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: Customization and attributes #10732

Closed
andrey-git opened this issue Nov 21, 2017 · 72 comments
Closed

RFC: Customization and attributes #10732

andrey-git opened this issue Nov 21, 2017 · 72 comments

Comments

@andrey-git
Copy link
Contributor

Currently all types of attributes got into the attributes dictionary of each entity.

I would divide those attributes into 5 types:

(1) Variable attributes that describe part of entity state. For example battery_level of some sensor, or a brightness of a light.

(2) Constant informational attributes, that will never change: node_id, manufacturer, supported_features, etc.

(3) Mixed visual-semantic attributes. For example groups are visual, but the ability to turn the group on/off makes them semantic. hidden is mostly visual, but it causes entities to be excluded from history which makes it semantic.

(4) Backend-controlled visual attributes: entity_picture, icon, device_class. Those are used purely for visuals, but backend sets them directly.

(5) Purely-visual attributes for the sake of custom UI.

Questions

  • Do we have more attributes of type (3)?

Thoughts

  • It would be great to eliminate type (3) completely. This mix is confusing and complicates things. For example what if I have a sensor I don't want to see in the UI, but do want to see in history?
  • We could split the attributes:
    • Type (1) stays as attributes.
    • Type (2) can be constant_attributes or properties.
    • Type (3) hopefully eliminated.
    • Type (4) either:
      • Move to a separate dictionary
      • Add device_subtype property and the frontend would set relevant properties on its own by domain + platform + subtype. This has the advantage of moving visuals out of the backend, and disadvantage that adding a custom platform will require adding frontend configuration. We could make this as simple as a json file however.
    • Type (5): Customization shouldn't be able to change type (1) & (2) attributes. It could change type (4) attribute or add a new one for custom UI sake.
@tinloaf
Copy link
Contributor

tinloaf commented Nov 21, 2017

Hi. I think it's a very good idea to clean up the attributes. Some thoughts:

  • Please don't name group 2 properties. It's already hard to not confuse attributes with properties (those python methods annotated with @property. Which are technically attributes in Python, ironically.) If we now have another type of thing with the same name…
  • For type 4:
    • What do you mean with "separate dictionary"? Something like attributes or constant_attributes?
    • If you take the device_subtype approach, then it's not possible to have dynamic symbols for entities, as it is currently the case with e.g. lights or batteries, is it? Would the entity change its subtype, e.g. from battery-70 to battery-60, to select between the different state-of-charge icons?
  • I don't see the distinction between 4 and 5. Both are purely visual, customizations are allowed to change both…?

@andrey-git
Copy link
Contributor Author

@tinloaf I agree regarding the name.

Yes, "separate dictionary" as visual_attributes or something.

Subtype is supposed to be constant. The "dynamics" would be implemented in the frontend, i.e. for a "battery" subtype - the frontend will pick an icon depending on battery_level attribute.

If (4) moves to a separate dictionary, then backend is still aware of things like "icon". Customization can override those or add new new attributes. If (4) moves to subtype, then customization can't override anything (there is nothing to override) but it can override frontend settings.

@OverloadUT
Copy link
Contributor

/me puts on his Product Manager hat
The main thing I see missing here is an issue statement. I get the desire to separate the data out in to categories, but what problem does this solve. What issue are people facing right now that we want to solve, before we even think about the technical solution?

@andrey-git
Copy link
Contributor Author

@OverloadUT For example there is discussion which attributes to show in the "more info" cards.
This approach makes a clear distinction between "types" of attributes.

@tinloaf
Copy link
Contributor

tinloaf commented Nov 21, 2017

@OverloadUT aside from what @andrey-git wrote, other things where I think a categorization of attributes makes sense:

  • History platforms currently store a lot of useless data. We don't need to record the manufacturer of a sensor in the database every time the sensor changes. Neither do we need to store its symbol.
  • The mqtt statestream platform, while being very useful, pushes a lot of useless stuff to MQTT - same as above. I don't need the manufacturer being published on MQTT if it never changes anyways.

@OverloadUT
Copy link
Contributor

Thanks that's helpful.

I agree with your overall suggestion to move to 3 types of attributes: state attributes (which can change) and static attributes (which never change). The third and final category is all of the stuff related to how Hass uses and displays the entity, and perhaps could even be called "customization" or something similar.

As for hidden affecting history, to me that is a design issue with the history component itself, and should not be considered an issue with the hidden property. I would separate that issue from the one being solved here.

@tinloaf
Copy link
Contributor

tinloaf commented Nov 21, 2017

I agree. hidden should be split into hidden and not_recorded or something. This would have the added benefit that you could remove e.g. template sensors from history. Since their state at any time can be computed from the state of all other entities, recording them is mostly useless.

@happyleavesaoc
Copy link
Contributor

I would call group (4) metadata.

@emlove
Copy link
Contributor

emlove commented Nov 21, 2017

Great write-up!

I'd throw out another name possibility of (1):state attributes and (2):entity attributes.

Another possible win is that by making this distinction we could make the websocket connection more intelligent wrt bandwidth usage. Category (2) attributes could be transmitted once when the state is first seen, then don't need to be transmitted with every state update. With this distinction it could be implemented in home-assistant-js-websocket transparently to polymer or other client consumers.

Also just bikeshedding, but I'd suggest that device_class belongs in category (2). Although it's currently only a visual distinction, it's not tightly coupled to our frontend implementation like icon/entity_picture are. It's just additional classification that describes the entity that clients can use how they choose.

@tinloaf
Copy link
Contributor

tinloaf commented Nov 21, 2017

To make sure nothing gets lost: please note home-assistant/frontend#660 (comment) (just closed that PR)

@rytilahti
Copy link
Member

While we are at it (making larger changes to attribute handling), would it make sense to explore ways to expose type (or unit) information for attributes in some form? Or would it be better just to enforce specific units for specific attributes?

A use-case is that currently there are some platforms (switch.tplink comes to my mind) which expose non-numeral values (see https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/switch/tplink.py#L89) just for pretty-printing them in the UI, which forces one to do template tricks to get rid of the unit part.

Wrt metadata type (or 'constant attributes') proposal, would that be the place to expose information such as backend module name, its version, and bug reporting facilities? The use-case would be to allow adding "report a bug" / see platform information button, which would be allow prefilling issue descriptions (and thus allow automatic tagging of for component & platform owners), and making it feasible for end-users to check the version. This would also act as a sort of attribution and acknowledge the work done by backend lib developers.

@OverloadUT
Copy link
Contributor

@rytilahti: I feel like this starts to get down a rabbit hole of attributes getting the features that states have to begin with, which says to me we have a design issue with the way attributes are being used in the first place.

Here's a crazy idea: What if we move "state attributes" (aka, attributes that change, like battery level, etc) to their own sensor entities, and allow entities to have parents and children for the purpose of UI grouping (and potentially even the way they are accessed via entity_id references).

So instead of:

  • sensor.living_room_motion: detected
    ** (attribute) battery_level: 60%

We have:

  • sensor.living_room_motion: detected
  • sensor.living_room_motion_battery: 60

And then living_room_motion_battery can be configured (automatically when a platform is generating these or manually via configure) as a child entity of the main living_room_motion device. The UI would more or less display this the same way we do today with battery being a visible attribute when you look at the motion sensor device, BUT we gain all of the benefits of having the battery level being its own entity (such as unit of measurement designation, the thought that spawned this whole post.)

Many platforms already work this way. Dark Sky creates a whole bunch of sensors rather than a single sensor with a ton of attributes. To me, I don't see a lot of consistency with regards to when a "sub-state" is its own sensor vs an attribute on a parent entity.

@OverloadUT
Copy link
Contributor

To summarize the above, I guess there are only two changes in my suggestion:

  • Allow sensors to be attached to parent entities, so that they show up a lot like attributes do today
  • Start enforcing a design standard that any attributes that are really states themselves be split in to separate sensors and use the new parent feature

@andrey-git
Copy link
Contributor Author

An issue no one touched yet is the group, which is both a semantic and a visual thing.
We could split it into "semantic" groups, where a service call on a group is propagated to all members, and "visual" groups that are configured at the frontend and the backend is not aware of them.

@OverloadUT
Copy link
Contributor

Groups are their own entities; they aren't attributes.

But even if they are in some cases (?), I don't think making a distinction between semantic and visual properties has a lot of value. That's all "metadata" - stuff that Hass uses to do things (display or functionality, that's all just Hass stuff). That's why I suggested three types of Lists that can be on an entity:

  • State Attributes (mutable, relevant to the state of the entity)
  • Static Attributes (immutable info about the entity)
  • Metadata (stuff Hass uses to do things: hidden, device_class)

@andrey-git
Copy link
Contributor Author

Yes, groups are entities, but they also affect other entities presentation (entities in groups are excluded from default_view for example).
If I "just" want to "group" things visually I need to create an entity (which will tracks state, etc.)
This is separate from attributes, but in the same domain of semantic/visual mix.

@balloob
Copy link
Member

balloob commented Nov 22, 2017

I realize that this has become more of a braindump then I wanted

@andrey-git Great write up! I want to add a few minor updates to your list.

The first category we should call "State attributes": attributes that explain something extra about the current state. If a light is on, it will mention the color or brightness.

The second category is something that people like to add to their platforms and that we do a good job at not allowing. (I just removed the Tradfri ones that accidentally made it in #10399) However, I think that having something besides the state machine available for this would be a good idea. Changing this data would not fire any event nor would it be part of the serialized state object.

I would consider category 3 ("hidden") to be part of 4. These should indeed not change often. Things like icons representing the state of a device should all be dealt with in the frontend. I think that we can even do a better job here and not have the camera component change entity_picture every 15 seconds.

Category 5, "Purely-visual attributes". I prefer to refer to them as UI configuration attributes. I think that we have 1 as part of Home Assistant: control for the group entity. I don't like this category. It makes frontend development extremely hard because all of a sudden the UI can be in so many different states. I'm fine with them being used for custom UI as the authors of custom UI can choose themselves how complicated they will allow it to get.

Extra category 6: Directions for services that sync with Home Assistant: haaska_name, haaska_hidden. Built-in component that uses this is Alexa Smart Home. I'll make sure that this gets removed.

Thanks for putting the product manager hat on @OverloadUT, that's a good hat to carry around 👍


Home Assistant architecture is very easy to understand. By keeping it simple, we have been able to:

  • allow developers to easily get started with development
  • allow powerful extensions of the system like recorder and MQTT statestream without having to change anything about the architecture
  • The architecture also supports replication (although we deprecated our implementation)
  • get a whole bunch of dashboards, apps and other user interfaces for Home Assistant because it's so easy to integrate
  • Want to try how something looks? Want to test an automation? Just change the state in the state machine.

I'm of the opinion that this simplicity has been one of the key drivers behind the success of Home Assistant. And simplicity is something that I will want to keep fighting for.

Simple systems are easy to understand, develop for and extend. However, the problem with each growing system is that it eventually will outgrow itself and we'll need a fundamental core change to be able to grow the next 10x. Past changes that fall in this category are the change from threaded to asyncio and on the frontend from NuclearJS + EventSource to WebSocket. Changing the way attributes work is however another magnitude bigger size change. This will impact everything that works/interfaces with Home Assistant. So let's be very thoughtful about this.


Would it be an idea that once we are able to decide on the categories that exist, we create a spreadsheet with known attributes and their categories?

@balloob
Copy link
Member

balloob commented Nov 22, 2017

btw, I wouldn't mind jumping on a voice call. Writing whole books is not always very helpful.

@andrey-git
Copy link
Contributor Author

How are haaska_name and similar are used?

@OverloadUT
Copy link
Contributor

OverloadUT commented Nov 22, 2017

How are haaska_name and similar are used?

For this example specifically, it is used to present a different name of the entity to voice interface services. You might want a group called "Living Room Front Track Lighting" inside Home Assistant, because that's what it is. But for voice interface purposes, you just want that called "Living Room Lights" because it's much easier and more natural to say.

@andrey-git
Copy link
Contributor Author

I think control is the same as icon and entity_picture, i.e. something purely visual that the backend is aware of.

While type (2) shouldn't be part of state machine, I think it should be part of the API.

Here is the list of attributes that Entity extract from properties:
unit_of_measurement: Used by the backend for conversion and by frontend for display.
name, icon, entity_picture, hidden: visual properties (once we make hidden visual-only)
assumed_state: I'm not sure, does backend care about this or is it visual only.
supported_features, device_class: static attributes

@andrey-git
Copy link
Contributor Author

@OverloadUT so it is not used by HA backend or HA frontend, just by external clients? So it fits in the same category as the rest of "for the sake of custom UI" attributes.

@emlove
Copy link
Contributor

emlove commented Nov 22, 2017

I'll throw out another thought for some brainstorming food:

We also have the case to deal with where we have smart home devices that create multiple entities in hass (i.e. zwave multisensor, etc.). The zwave component ended up moving the shared device state attributes (connection strength, etc.) into a completely separate entity in the zwave domain, so these weren't duplicated. Considering this use-case as well might help lead us to the right solution.

@tinloaf
Copy link
Contributor

tinloaf commented Nov 22, 2017

Another thought: What are the conceptual and technical differences between the string in state.state and the string in state.attributes[ATTR_WHATEVER]? The only real distinction I can come up with is that state.state is being used to give a short summary of what the entity is currently doing / measuring. But that's actually just a visual thing, isn't it? So, if we do a major refactoring anyway, why not get rid of state.state and move it into state.attributes[ATTR_SUMMARY] (or something). The UI then knows to use this attribute (if present) in state cards etc.

This would solve the problem with "physical things that have no state". For example, currently clickable devices ("buttons") are weird: Some platforms represent them as binary_sensor, some don't create entities for them at all but only fire some event when the button is pressed. In the latter case, you need an additional somewhat weird entity to report attributes like battery level, etc. If state.state was removed and state.attributes[ATTR_SUMMARY] was optional, these things could become entities like everything else and just not set ATTR_SUMMARY.

@balloob
Copy link
Member

balloob commented Nov 23, 2017

@andrey-git supported_features is dynamic. Based on the current state of a device, the features it support might change.

@tinloaf I am not a fan of that. Think about how confusing we're going to make it for users to get started. What is the most important attribute? With state.state it's clear that this is the actual state. It's the first thing you would say when describing a device. Light is on / off etc. We should keep it simple.

I guess my post was too long but the gist of the last part was this:

Let's go for an evolutionary approach versus a revolutionary approach.

We should not aim for a major refactor just to make the case of storing non-state data for an entity easier. The main purpose of Home Assistant is storing the state of entities. If we make that more complex, we're not doing the right thing.

I am a fan of a static data but they should not contain anything that would be needed for our frontend to render it. It would be extra information the user might be interested in (model, vendor, firmware).

Does that mean that custom UI config variables will be stored on the state machine? Yep but that's not an issue we have to solve for. This is a choice the custom UI authors make. They could have people put it as an object on the DOM too. (with this I mean that it should still be possible, just shouldn't have to perse be easier)

@andrey-git
Copy link
Contributor Author

Custom UI customization can be huge and currently the only option of not putting it in the state machine is to use a custom component and put the config there instead of using customize.

@balloob
Copy link
Member

balloob commented Nov 28, 2017

Another option for customize UI is to have a config file that you load as an HTML import.

@andrey-git
Copy link
Contributor Author

It will add an additional round trip.
Also writing configs in html is hard.

We could have a "customize" (another name needed) component that exports a config without putting it in the entities.

@balloob
Copy link
Member

balloob commented Nov 28, 2017

So I have toyed with that idea. We could move all Alexa/Google Assistant/Emulated Hue/Haaska config there too. My only concern is that now there are 2 places that hold configuration for a component. google_assistant: and homeassistant:/entity_config:

@balloob
Copy link
Member

balloob commented Dec 13, 2017

@armills I've moved your comment to #11118

@emlove
Copy link
Contributor

emlove commented Dec 13, 2017

@balloob I don't think it's the wrong issue. That would eliminate the need to even add hass.info.

@balloob
Copy link
Member

balloob commented Dec 13, 2017

Oh, I guess you are right there, oops 😶 .

I think in that case I still wouldn't want to store component entity configuration in the state machine.

@emlove
Copy link
Contributor

emlove commented Dec 13, 2017

Hah, no problem. Appreciate the effort to at least try and wrangle this conversation!

Definitely agree on the entity configuration, but I think if that's the only problem left we can come up with a better solution for that on its own.

If possible I'd really like to not introduce more concepts to core hass. This feels like it knocks out a lot of the problems, while still living within our current infrastructure.

@pvizeli
Copy link
Member

pvizeli commented Dec 13, 2017

With last statement what it should stored inside hass.info (from @balloob) I think we don't need merge it to state like entity.info.

For custom UI we need support a interface to fetch this data over the API.

I'm also fine with that but I'm feel also like @armills. It solve this problem but I'm not 100% sure if we have not a deeper concept problem.

Anyway maybe we can use hass.data['info'][entity_id] without make a new store and can use a helpers to handle it. So we can call this function with hass.helpers.info.yx

@rytilahti
Copy link
Member

My opinion wrt. firmware versions and manufacturers is that entity attributes/properties (such as firmware version, manufacturer, etc.) should belong to that entity and not some external place. If the issue is polluting the recorder, maybe we could think a way to avoid exposing those attributes to the recorder? Storing homeassistant's internal entity configuration could very well go outside from the entities themselves, but wouldn't it be more clear if that would be a part of the entity (as it logically feels to be), but which also should be ignored for state tracking purposes?

Regarding to configuration elements of entities themselves, I think those should also "belong" to an entity. In the future I would like to see a way for the platform developer to expose platform's extra services in a nice way for both API and REST API users. These should not definitely be part of the state, but should be accessible through the entity interface.

As it stands, adding those services into a global namespace breaks the encapsulation both on the code (separating descriptions out from the platform) and on the logical level (the available, platform-specific services have just prefixed names with no real way finding more about them). Considering that I would be an UI developer, providing our users a way to access some extra functionality like setting a custom fan speed or reseting the "exchange filter" counters after a change, to achieve this for all possible devices is just waste of time.

However, considering that these types of actions would very well be useful for our users, letting the developer of the platform to expose services (again defined inside the "main platform"/"entity") with some meta-data (type: "range? slider?", min_value: 0, max_value: 100) would enable UI developers to add such simple services e.g. into a separate "extra actions" tab. This (and introspectable interfaces in general) would also be beneficial for the REST API users. If one wants to access services offered by some platform it is currently very cumbersome to do so in appdaemon.

Now that I have written that out, I feel I need to apologize for taking it towards something which may be worth of another RFC, but I think this shows that there is an inherent need to have some type of non-state related attributes not in a globally shared container but inside the entities themselves.

@pvizeli
Copy link
Member

pvizeli commented Dec 15, 2017

After some discussion with @balloob I write follow statment:

I only vote for this if we try to solve real problems and make not only a useless core change. Yes 1-5% usercase for a new stuff is not realy height.

That means:

  1. customize should not able to write new attributes
  2. hasd.info data need to be fetch able over API for external system like UI, hasska

Only if the new stuff support this, we have a nice feature they will bring realy usable cases.

@BioSehnsucht
Copy link
Contributor

I don't really know core well, but have done some component work / am working on a component/platform for a new integration.

Is there a reason for hass.info versus adding an info Dict property to the entities (i.e., from a code perspective on the entity, same as attributes, just different name / purpose)? Or is this the same idea (I've assumed that accessing hass.states...some_entity.attributes is accessing some_entity.attributes directly, but maybe it's not?)

@balloob
Copy link
Member

balloob commented Jan 3, 2018

The idea would be that info doesn't live on the state object, as it is not part of the state of a device. So there is a difference between hass.states.get(entity_id) and hass.info.get(entity_id).

Something that I have been thinking about that is somewhat related to this (as it impacts attributes), is the Web Thing API. It's a draft standard API for the internet of things on the web. (It doesn't cover all the things I would expect from an API but it's a draft so we can have an impact) We should probably move talking about the Web Things API to a new topic if people are interested. (leave a 👍 on this comment to indicate interest)

@balloob
Copy link
Member

balloob commented Jan 9, 2018

Update:

I have removed configuration from customize from both Alexa and Google Assistant. Emulated Hue is still using customize.

Based on all the discussion here, I thought of a related idea: the entity registry. More info here: #11533

@andrey-git
Copy link
Contributor Author

@balloob
How about creating a "component" for storing "customization" that won't go on the state machine?
It

This is for the sake of custom UI components or alternate clients altogether: Right now it requires each author to write and each user to install a custom component.

@balloob
Copy link
Member

balloob commented Jan 15, 2018

I prefer if we can keep it out of the state machine because it causes side effects, like people complaining the recorder is bad.

What if we have a special component in which you can store whatever data structure you want and we add special code to the frontend to load it once (inside <home-assistant>#connectionChanged via an API call if it sees that the specific component is loaded?

@tboyce021
Copy link
Contributor

Somewhat related to this discussion but not sure if I should create a new issue or not:

Some components (I'm specifically looking at template_sensor) have config options to set various UI-related attributes. With all this talk of separating the concerns of frontend and backend, it seems like these should not be config options. In fact, a lot of them can already be achieved in other ways, specifically

  • friendly_name can be set via customize. A name config option is really only needed if it affects the entity_id, not just the display name.
  • template_icon can be done via custom UI. The main difference I see is that template_icon is done server side and with custom UI it's (I think) done client side (correct me if I'm wrong here).
  • entity_picture_template I assume can also be done with custom UI.

Since these are all fairly trivial to accomplish using the alternative methods (custom UI stuff being trivial with @andrey-git's repo) , would it be good to start removing these config options so people can stop using them and switch to the alternative methods?

@c727
Copy link
Contributor

c727 commented Feb 1, 2018

I like @andrey-git 's idea about creating such a "customization" "component". I would name it customizer but this name is already blocked 😆

Loading the data clientside on <home-assistant>#connectionChanged also sounds great. <-- That's something I meant when I aksed where to store the data client side for custom cards for different cards (full/state/more-info)

My PR for the custom_card (here) component has some limitations as it allows only storing config for entities created by the component. I can modify it to do both, allow the creation of custom_cards and provide config for any entity...

Or are you already working on a (better) solution, Andrey?

maybe it should be part of frontend?

frontend:
  custom_config:
    "light.light1":
      key1: val1
  custom_card:
    "card":
      "full_card": test-card
      "state_card": xxx

@balloob
Copy link
Member

balloob commented Feb 2, 2018

I think that adding it to the frontend component is not a bad idea. And then in the Polymer part, when stuff is loaded, see if the data was set and if so, fetch it with API call.

@balloobbot
Copy link

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates.

Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍

@balloobbot
Copy link

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates.

Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍

@balloob
Copy link
Member

balloob commented Oct 26, 2018

I think that we can close this issue:

  • With device registry, we want things like battery level to be their own entity.
  • UI customizations can be added directly to Lovelace config, also for custom cards

@balloob balloob closed this as completed Oct 26, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Feb 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests