Skip to content

binary_sensor sensor_class to entity device_class#5860

Merged
balloob merged 3 commits into
home-assistant:devfrom
emlove:device-class
Feb 11, 2017
Merged

binary_sensor sensor_class to entity device_class#5860
balloob merged 3 commits into
home-assistant:devfrom
emlove:device-class

Conversation

@emlove
Copy link
Copy Markdown
Contributor

@emlove emlove commented Feb 11, 2017

Description:
This PR changes the binary_sensor property sensor_class to an Entity property device_class. I think this type of classification could be beneficial to expand to other components. My intended use case is to add this functionality to the cover component. Some examples being garage, window, door, gate, etc. I'm sure this could be useful for other components as well, though.

See also:
home-assistant/frontend#200
home-assistant/home-assistant.io#2005

@mention-bot
Copy link
Copy Markdown

@armills, thanks for your PR! By analyzing the history of the files in this pull request, we identified @fabaff, @pvizeli and @MartinHjelmare to be potential reviewers.

Comment thread tests/helpers/test_entity.py Outdated
state = self.hass.states.get(self.entity.entity_id)
assert state.attributes.get(ATTR_DEVICE_CLASS) == None
with patch('homeassistant.helpers.entity.Entity.device_class',
new='test_class'):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

continuation line over-indented for visual indent

Comment thread tests/helpers/test_entity.py Outdated
def test_device_class(self):
"""Test device class attribute."""
state = self.hass.states.get(self.entity.entity_id)
assert state.attributes.get(ATTR_DEVICE_CLASS) == None
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

comparison to None should be 'if cond is None:'

Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

I like this! Looks good besides the lint errors.

vol.Optional(CONF_PAYLOAD_OFF, default=DEFAULT_PAYLOAD_OFF): cv.string,
vol.Optional(CONF_PAYLOAD_ON, default=DEFAULT_PAYLOAD_ON): cv.string,
vol.Optional(CONF_SENSOR_CLASS, default=None):
vol.Any(vol.In(SENSOR_CLASSES), vol.SetTo(None)),
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

So I'm not sure why the schema was set up like this before. Is there a specific reason we want to silently ignore invalid classes?

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.

That's weird, we shouldn't. I have never seen SetTo before. (quick grep shows this is the only place it is used)

@balloob
Copy link
Copy Markdown
Member

balloob commented Feb 11, 2017

Love it! 🐬 amazing job.

@balloob balloob merged commit e877d57 into home-assistant:dev Feb 11, 2017
@emlove emlove deleted the device-class branch February 17, 2017 21:06
@pennyandsean
Copy link
Copy Markdown

pennyandsean commented Feb 28, 2017

This seemed to break the UI icons for binary sensor templates. Is that just me?

@emlove
Copy link
Copy Markdown
Contributor Author

emlove commented Feb 28, 2017

I had to clear the cache to get it to update.

@pennyandsean
Copy link
Copy Markdown

Ah yep that did the trick. Thanks Adam.

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.

8 participants