Issue #6893 in rfxtrx#9130
Conversation
|
@Danielhiversen, thanks for your PR! By analyzing the history of the files in this pull request, we identified @fabaff, @badele and @ypollart to be potential reviewers. |
There was a problem hiding this comment.
line too long (84 > 79 characters)
There was a problem hiding this comment.
line too long (80 > 79 characters)
Danielhiversen
force-pushed
the
rfxtrx_issue
branch
from
5f9f84f to
2e35b7d
Compare
| raise vol.Invalid('Rfxtrx device {} is invalid: Invalid' | ||
| ' device id for {}'.format(key, value)) | ||
| except ImportError: | ||
| _LOGGER.warning('Restart Home Assistant to' |
There was a problem hiding this comment.
Shouldn't vol.Invalid be raised? Otherwise a possible invalid configuration could be passed as valid. What consequences would that have for downstream code?
There was a problem hiding this comment.
If we raise an error here, I think the rfxtrx lib never will be installed?
If the key is invalid the first time HA starts, the rfxtrx component should still load and install the rfxtrx lib, but the switch/sensor with an error will not be loaded.
pvizeli
left a comment
pvizeli
left a comment
There was a problem hiding this comment.
I think we should validate the config without external library. We validate the config every time to look correct but if that will work can be we only look on setup like all other component/platforms
|
Nice 👍 |
* upstream/dev: (113 commits) Fix fitbit error when trying to access token after upgrade. (home-assistant#9183) Upgrade sendgrid to 5.0.1 (home-assistant#9215) Upgrade pyasn1 to 0.3.3 and pyasn1-modules to 0.1.1 (home-assistant#9216) directv: extended discovery via REST api, bug fix (home-assistant#8800) Bayesian Binary Sensor (home-assistant#8810) Add cloud auth support (home-assistant#9208) Abode push events and lock, cover, and switch components (home-assistant#9095) Lint Sonarr tests Upgrade pymysensors to 0.11.1 (home-assistant#9212) Refactor rfxtrx (home-assistant#9117) Issue home-assistant#6893 in rfxtrx (home-assistant#9130) Support for season sensor (home-assistant#8958) Add counter component (home-assistant#9146) Fix and optimize digitalloggers platform (home-assistant#9203) Prevent error when no forecast data was available (home-assistant#9176) Add "status" to Sonarr sensor (home-assistant#9204) fix worldtidesinfo home-assistant#9184 (home-assistant#9201) Update pushbullet.py (home-assistant#9200) Fix dht22 when no data was read initially home-assistant#8976 (home-assistant#9198) Prevent iCloud exceptions in logfile (home-assistant#9179) ...
7 participants
Description:
From #6893 (comment) :
This might not be the best solution, so I am open for suggestions.
Merging this will make the make HA start for the first time without need of manually install PyRfxtrx, and still validate the config as good as possible with the available libraries.
Related issue (if applicable): fixes #6893