Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Device manufacturer / model reporting #26

Closed
ubnt-marc-khouri opened this issue May 14, 2018 · 11 comments
Closed

RFC: Device manufacturer / model reporting #26

ubnt-marc-khouri opened this issue May 14, 2018 · 11 comments

Comments

@ubnt-marc-khouri
Copy link

Goal

Report device manufacturer and model in a standardized way, accessible to other components and the frontend. For example, allow showing whether a Sonos player is a Play:1 vs Playbar. Many components/platforms already have this info, but either report it in state_attributes, or not at all.

Proposal

  1. homeassistant/helpers/entity.py: Extend Entity to include a manufacturer_name and model_name property (similar to unique_id)
  2. homeassistant/helpers/entity_platform.py: Pass the manufacturer/model information to the Entity Registry
  3. entity registry: Store the mfg / model info, so that other components may access it
  4. Platforms are extended to expose this information, for example in Hue:
# HueLight class
# https://github.com/home-assistant/home-assistant/blob/0.69.1/homeassistant/components/light/hue.py#L201
+    @property
+    def manufacturer_name(self):
+        """Return the manufacturer of the Hue light."""
+        return self.light.manufacturername
+
+    @property
+    def model_name(self):
+        """Return the model of the Hue light."""
+        return self.light.productname
@balloob
Copy link
Member

balloob commented May 14, 2018

I like this idea a lot . We won't pollute the state machine with it. I think the entity registry is an excellent place to change this as it won't ever change (contrary to say, firmware version).

@ubnt-marc-khouri
Copy link
Author

Thanks for the feedback! I'll stay tuned for any other comments, and probably prepare a PR near the end of the week

@fabaff
Copy link
Member

fabaff commented May 15, 2018

I like it too. Having additional details at hand can help to debug issues. I'm thinking about the TP-Link switch support that broke with new firmware or the no-name/relabeled devices from Asia which belong to family of identical devices which are just sold under different names.

@dgomes
Copy link

dgomes commented May 15, 2018

I also like the idea, actually some components such as camera already have brand and model properties:

https://github.com/home-assistant/home-assistant/blob/de50d5d9c1c0375c5e46042f3168d356c73f6aa1/homeassistant/components/camera/__init__.py#L254

@balloob
Copy link
Member

balloob commented May 16, 2018

We should make sure to align those. Note that this property will only be used for entities with a unique id.

@ubnt-marc-khouri
Copy link
Author

As pointed out by @dgomes , the current Camera Entity class uses brand and model, whereas my proposal is manufacturer_name and model_name.

Any strong opinions on naming? I slightly prefer manufacturer_name over brand, but I don't feel strongly about it

@Kane610
Copy link
Member

Kane610 commented May 16, 2018

I prefer it as Zigbee does it; uses modelid and manufacturer

@balloob
Copy link
Member

balloob commented May 17, 2018

The downside of model id is that we can't just show it in the frontend.

@Kane610
Copy link
Member

Kane610 commented May 18, 2018

@balloob I see them as the same thing

@balloob
Copy link
Member

balloob commented May 18, 2018

Interesting. I would expect id to reference an identifier, name to reference a name.

I think that we should have the names contain the source + the attribute. So I wouldn't like manufacturer, I would prefer manufacturer_name. I think for model, initially I would be interested in the model_name, not the model_id. I would want to show a list of devices in Home Assistant.

@MartinHjelmare
Copy link
Member

@balloob we can close this I think. It was solved with the device registry.

See #36.

@balloob balloob closed this as completed Mar 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants