Skip to content

deCONZ - new sensor attribute 'on' and new sensor GenericFlag#15247

Merged
Kane610 merged 6 commits into
home-assistant:devfrom
Kane610:deconz-new-attribute-new-sensor
Jul 2, 2018
Merged

deCONZ - new sensor attribute 'on' and new sensor GenericFlag#15247
Kane610 merged 6 commits into
home-assistant:devfrom
Kane610:deconz-new-attribute-new-sensor

Conversation

@Kane610
Copy link
Copy Markdown
Member

@Kane610 Kane610 commented Jul 1, 2018

Description:

New attribute 'on' for sensors
New sensor GenericFlag added through updated dependency

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

if self._sensor.battery:
attr[ATTR_BATTERY_LEVEL] = self._sensor.battery
if self._sensor.on is not None:
attr[ATTR_ON] = self._sensor.on
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.

How is is_on different from is_tripped and reachable ? Shouldn't on just impact the available property?

Copy link
Copy Markdown
Member Author

@Kane610 Kane610 Jul 1, 2018

Choose a reason for hiding this comment

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

I was contemplating whether or not. But I decided to go with this way. The on is if the device should report status or not. This is something controlled by the user who can still decide to change 'on' from false to true, so I don't think that reachable is applicable here since in that context you wouldn't expect you could change the 'on' value.

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.

Would it be preferable if I called it enabled or keep with how zigbee names it?

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.

Yes, we should try to stick to how the source names things.

@Kane610
Copy link
Copy Markdown
Member Author

Kane610 commented Jul 2, 2018

Thanks @balloob !

@Kane610 Kane610 merged commit bedd2d7 into home-assistant:dev Jul 2, 2018
@ghost ghost removed the in progress label Jul 2, 2018
@Kane610 Kane610 deleted the deconz-new-attribute-new-sensor branch July 2, 2018 21:15
awarecan pushed a commit to awarecan/home-assistant that referenced this pull request Jul 16, 2018
@balloob balloob mentioned this pull request Jul 20, 2018
girlpunk pushed a commit to girlpunk/home-assistant that referenced this pull request Sep 4, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Dec 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants