Add a fake WindowCovering accessory script#165
Conversation
Add 5sec delay for closing the fake shutter
set_info_service for bridge
Quick assign basic accessory information
Codecov Report
@@ Coverage Diff @@
## dev #165 +/- ##
=======================================
Coverage 58.14% 58.14%
=======================================
Files 16 16
Lines 1639 1639
Branches 165 165
=======================================
Hits 953 953
Misses 652 652
Partials 34 34 |
cdce8p
left a comment
cdce8p
left a comment
There was a problem hiding this comment.
I'm not sure why you've created a completely new file for the Fake WindowCovering. Why not integrate it into busy_home.py?
| 'TargetPosition', setter_callback=self.set_target_position) | ||
|
|
||
| self.char_state = serv_cover.configure_char( | ||
| 'PositionState', setter_callback=self.set_position_state) |
There was a problem hiding this comment.
You don't need to assign this char a local variable. The setter_callback is never called, so you can remove it. Maybe add value=2 here as a parameter.
| 'PositionState', setter_callback=self.set_position_state) | ||
|
|
||
| self.char_cur_pos = serv_cover.configure_char( | ||
| 'CurrentPosition', setter_callback=self.set_current_position) |
There was a problem hiding this comment.
The setter_callback is also never called, so it can be removed.
| logging.info("WindowCovering TargetPosition value: %s", value) | ||
| self.get_service('WindowCovering')\ | ||
| .get_characteristic('TargetPosition')\ | ||
| .set_value(value) |
There was a problem hiding this comment.
Don't set the char to the value that it posted. This isn't necessary. Just remove it.
| logging.info("WindowCovering CurrentPosition value: %s", value) | ||
| self.get_service('WindowCovering')\ | ||
| .get_characteristic('CurrentPosition')\ | ||
| .set_value(value) |
There was a problem hiding this comment.
You've assigned CurrentPosition a local var. Why not use it:
self.char_cur_pos.set_value(value)| logging.info("WindowCovering CurrentPosition value: %s", value) | ||
| self.get_service('WindowCovering')\ | ||
| .get_characteristic('CurrentPosition')\ | ||
| .set_value(value) |
There was a problem hiding this comment.
The methods set_position_state and set_current_position can be removed, since they are never called.
| self.set_info_service(firmware_revision=2, manufacturer="Brand", | ||
| model="Shutter", serial_number="0123456789") | ||
|
|
||
| # Add the WindowCovering service. Also add optional characteristics to it. |
There was a problem hiding this comment.
This comment and the one about assigning accessory infos can be removed as well.
|
@lboue Will you be addressing the raised issues or should I take care of these after a merge maybe? |
|
@ikalchev I don't like the idea of merging a PR that isn't finished yet, only to finish it later. Instead consider committing the changes directly to the PR branch. Maintainers do generally have the permission to do so. |
|
OK. I will fix all that items ASAP. |
4 participants
Should I remove the 2 following function too?