Skip to content

deCONZ - Fix service device refresh calling state update#30920

Merged
balloob merged 1 commit into
home-assistant:devfrom
Kane610:deconz-ignore_update_signal_when_refreshing_devices
Jan 17, 2020
Merged

deCONZ - Fix service device refresh calling state update#30920
balloob merged 1 commit into
home-assistant:devfrom
Kane610:deconz-ignore_update_signal_when_refreshing_devices

Conversation

@Kane610
Copy link
Copy Markdown
Member

@Kane610 Kane610 commented Jan 17, 2020

Description:

Unintentional behavioural change when updating data management in pydeconz. This allows controlling whether or not an update should be ignored.

Related issue (if applicable): fixes #30890

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the [development checklist][dev-checklist]

@Kane610 Kane610 added this to the 0.104.2 milestone Jan 17, 2020
@Kane610 Kane610 self-assigned this Jan 17, 2020
@callback
def async_update_callback(self):
def async_update_callback(self, ignore_update=False):
"""Sensor state updated."""
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't you check here for ignore_update too ?

Copy link
Copy Markdown
Member Author

@Kane610 Kane610 Jan 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No it is just tracker for battery sensors, by not evaluating ignore_update we increase the chance of creating new battery sensors which would be a good thing :)

def async_update_callback(self, force_update=False, ignore_update=False):
"""Update the sensor's state."""
changed = set(self._device.changed_keys)
if ignore_update:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems kinda weird. Wouldn't it be possible to just not call async_update_callback if ignore_update is True ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thought was to not have that logic in the library, but you think that would be better?

Copy link
Copy Markdown
Member

@balloob balloob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Going to merge this for the hotfix, we can discuss the comment afterwards.

@balloob balloob merged commit c9db21f into home-assistant:dev Jan 17, 2020
@balloob balloob mentioned this pull request Jan 17, 2020
@home-assistant home-assistant deleted a comment from fastender Jan 18, 2020
@Kane610
Copy link
Copy Markdown
Member Author

Kane610 commented Jan 18, 2020

@fastender it works here, be a bit more constructive please

@Kane610 Kane610 deleted the deconz-ignore_update_signal_when_refreshing_devices branch January 18, 2020 06:51
@lock lock Bot locked and limited conversation to collaborators Jan 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Calling deconz.device_refresh leads to toggling lights

3 participants