Fix node internal state handling #363
Conversation
…-sleep nodes, just send the message (theolind#362)
… state and to set desired state (theolind#362)
…ew value is set to None upon confirmation from node receive (theolind#362)
…des - it is handled by the `update_child_value` method internally (theolind#362)
… in case node responds with something different than the desired state value (theolind#362)
…for both regular and smart sleep sensors processing (theolind#362)
… the default ack value for the `set` message type (motivation - give a way to match regular and smart sleep behaviour) (theolind#362)
…the value request message from the node (theolind#362)
|
@MartinHjelmare take a look, please |
|
I'll have a look later in the week. Thanks! |
mysensors/__init__.py
Outdated
| @@ -39,6 +38,7 @@ def __init__(self, event_callback=None, protocol_version="1.4"): | |||
| self.protocol_version = protocol_version | |||
| self.sensors = {} | |||
| self.tasks = None | |||
| self.use_ack_when_set_values = kwargs.get("use_ack_when_set_values", False) | |||
There was a problem hiding this comment.
I think this option belongs in set_child_values per message, and not a new gateway attribute. We already supported it before per message.
There was a problem hiding this comment.
@MartinHjelmare this is a workaround for smart sleep nodes.
The gateway's set_child_values method still allows to configure the ACK parameter, but for smart sleep nodes it doesn't work, because we convert set message into the new state value (which has the ChildSensor type and doesn't allow to store message parameters).
E.g. right now Home Assistant integration always requests echo messages during set_child_values calls, but for smart sleep nodes it does nothing.
Other approach would be to use a specialized type to store the new state, which would allow to save message parameters in addition to the value.
Or maybe allow to configure sensor to always use ACK for some types of messages.
There was a problem hiding this comment.
We could consider storing all or latest message instead of the state and replaying the message(s).
There was a problem hiding this comment.
@MartinHjelmare yes, I had the same idea, but it requires more changes. Plus storing the desired state instead of just a message is helpful when you need to reply to a state request from the node (handle_req function).
There was a problem hiding this comment.
@MartinHjelmare I think I've addressed all your comments except this one. We can revert this change, if you want, and leave that fix for the next PR.
There was a problem hiding this comment.
Yeah, let's remove it for now.
There was a problem hiding this comment.
@MartinHjelmare done
Co-authored-by: Martin Hjelmare <[email protected]>
Co-authored-by: Martin Hjelmare <[email protected]>
…eter for the default ack value for the `set` message type (motivation - give a way to match regular and smart sleep behaviour) (theolind#362)" This reverts commit d8fe478.
…heolind#362)" This reverts commit 33deb7d.
MartinHjelmare
changed the title
|
Please update the PR description after the latest commit. |
|
@MartinHjelmare oh, right. Done. |
|
I think we should mention in the PR description that the Sensor method |
|
@MartinHjelmare updated. Should not be the breaking change since that method was not part of the public API, as far as I understand it. All state changes should be routed through the gateway class ( |
|
Technically all non private methods are part of the API. |
|
@MartinHjelmare is current description ok? |
|
Thanks! |
| child = self.new_state[child_id] | ||
| value = child.values.get(value_type) if child else None | ||
|
|
||
| if value: |
There was a problem hiding this comment.
I think we need to check for None explicitly here since the value may be an empty string, which is valid.
There was a problem hiding this comment.
@MartinHjelmare I've created a new PR - #406
Closes #362.
Setmessages to smart sleep nodes will be repeated until the response from the node will be received.set_child_valuemethod was removed and replaced and split intoupdate_child_valueandset_child_desired_statemethods (the first one updates the internal state immediately and the second one sets the target state for smart sleep nodes)