Improve BTicino support #497
Conversation
Firmware versions for these devices are always reported to be `1`, so the `FirmwareMixin` can be omitted. Both `BNIL` and `BNLD` do not report power consumption, so the `PowerMixin` should be omitted. Signed-off-by: Stephan Peijnik-Steinwender <[email protected]>
Signed-off-by: Stephan Peijnik-Steinwender <[email protected]>
Signed-off-by: Stephan Peijnik-Steinwender <[email protected]>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @speijnik - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟡 Testing: 1 issue found
- 🟢 Complexity: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| @@ -37,7 +44,7 @@ class BNMS(Shutter): | |||
| """BTicino motorized shade.""" | |||
|
|
|||
|
|
|||
| class BNAS(Shutter): | |||
| class BNAS(ShutterMixin, Module): | |||
There was a problem hiding this comment.
suggestion (code_clarification): Consider adding a comment explaining the use of ShutterMixin in BNAS.
Since ShutterMixin is being introduced, a brief comment on its role or functionality could help maintain clarity.
| class BNAS(ShutterMixin, Module): | |
| class BNAS(ShutterMixin, Module): | |
| """BTicino automatic shutter. | |
| ShutterMixin: Provides methods for controlling shutter position and state. | |
| """ |
| elif "BNTH" in self.device_types: | ||
| self.climate_type = DeviceType.BNTH |
There was a problem hiding this comment.
issue (code_refinement): Addition of BNTH device type handling is missing feature addition in the features set.
Consider adding relevant features to the features set for BNTH, similar to other device types.
| # BNTH is wired, so the room is always reachable | ||
| self.reachable = True |
There was a problem hiding this comment.
issue (edge_case_not_handled): Assumption that BNTH rooms are always reachable may not hold in all cases.
Consider edge cases where hardware or network issues might render the BNTH device unreachable despite being wired.
| self.reachable = raw_data.get("reachable") | ||
| if self.climate_type == DeviceType.BNTH: | ||
| # BNTH is wired, so the room is always reachable | ||
| self.reachable = True |
There was a problem hiding this comment.
issue (testing): Missing test for reachable property logic with BNTH devices.
A test should be added to confirm that the reachable property is set to True when the climate_type is BNTH, as this is a new behavior that needs verification.
Improves overall BTicino support for various devices: