Add Tahoma lock support#31311
Conversation
MartinHjelmare
changed the title
springstan
left a comment
springstan
left a comment
There was a problem hiding this comment.
Sorry did not see some things that need to be addressed 😅
springstan
left a comment
springstan
left a comment
There was a problem hiding this comment.
Looks good, thanks for implementing my suggested changes ✌😉
Codecov Report
@@ Coverage Diff @@
## dev #31311 +/- ##
==========================================
+ Coverage 94.61% 94.63% +0.02%
==========================================
Files 741 745 +4
Lines 53816 54048 +232
==========================================
+ Hits 50916 51148 +232
Misses 2900 2900
Continue to review full report at Codecov.
|
MartinHjelmare
left a comment
MartinHjelmare
left a comment
There was a problem hiding this comment.
Please address the comments in a new PR.
| if self._battery_level == "low": | ||
| _LOGGER.warning("Lock %s has low battery", self._name) | ||
| if self._battery_level == "verylow": | ||
| _LOGGER.error("Lock %s has very low battery", self._name) |
There was a problem hiding this comment.
We shouldn't log errors if the application is functioning normally. The low battery level is just a state at the device. It's not a problem with Home Assistant.
The user can make an automation that triggers an alert if the battery level attribute reaches a certain value.
| _LOGGER.error("Lock %s has very low battery", self._name) | ||
| if ( | ||
| self.tahoma_device.active_states.get("core:LockedUnlockedState") | ||
| == STATE_LOCKED |
There was a problem hiding this comment.
We shouldn't use the home assistant state constant to compare with the tahoma state API. These are two different APIs and either may change independently of the other.
Please define a separate string constant for the tahoma API to use here.
|
|
||
| def setup_platform(hass, config, add_entities, discovery_info=None): | ||
| """Set up the Tahoma lock.""" | ||
| controller = hass.data[TAHOMA_DOMAIN]["controller"] |
There was a problem hiding this comment.
Please add a guard clause here that checks if discovery_info is None and return if so.
|
Now I feel like I missed these important things 😅 but glad you looked over it @MartinHjelmare |
|
Don't worry. We're all learning here and that means we miss things and sometimes make mistakes etc. If we don't accept that we won't learn either. You're reviewing great! Keep it up! 👍 |
4 participants
Proposed change
Add support for Somfy's smartlock through the tahoma integration. If the smartlock has been added to the tahoma box, it will be detected and added to Home Assistant.
Type of change
Additional information
Checklist
black --fast homeassistant tests)no tests are done for tahoma in the current dev branch...
If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all..coveragerc.The integration reached or maintains the following Integration Quality Scale: