home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.
https://www.home-assistant.io
Apache License 2.0
71.34k stars 29.88k forks source link

RFC: Customization and attributes #10732

Closed andrey-git closed 5 years ago

andrey-git commented 6 years ago

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

Thoughts

tinloaf commented 6 years ago

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

andrey-git commented 6 years ago

@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 commented 6 years ago

/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 commented 6 years ago

@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 commented 6 years ago

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

OverloadUT commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

I would call group (4) metadata.

emlove commented 6 years ago

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 commented 6 years ago

To make sure nothing gets lost: please note https://github.com/home-assistant/home-assistant-polymer/pull/660#issuecomment-346162325 (just closed that PR)

rytilahti commented 6 years ago

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 commented 6 years ago

@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:

We have:

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 commented 6 years ago

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

andrey-git commented 6 years ago

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 commented 6 years ago

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:

andrey-git commented 6 years ago

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 commented 6 years ago

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:

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 commented 6 years ago

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

andrey-git commented 6 years ago

How are haaska_name and similar are used?

OverloadUT commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

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

andrey-git commented 6 years ago

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 commented 6 years ago

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:

andrey-git commented 6 years ago

There can still be a single Customization config panel that will write to both places, so for UI config users the change would be transparent.

tboyce021 commented 6 years ago

I would like to see some more consistency with what's shown as an attribute and what gets split into it's own entity, i.e. what's a "state attribute" vs a regular "state".

battery_level is an obvious one for me. Some components (e.g. roomba and owntracks) put battery_level as an attribute. For others (e.g. z-wave and my mqtt sensors from SmartThings) the battery level is it's own entity. The biggest issue currently with having it as an attribute is that it doesn't show up in the history and is slightly harder to work with in automations or show on the frontend in a separate "battery" view/group. The workaround for this is to create new template sensors that just extract the attribute into their own states.

Similarly, the nest component has config sections that do nothing but pull the attribute data into separate state entities for you. Unfortunately, with all of them enabled, it makes it difficult to see which sensors were created by nest since they're spread out over several different domains and don't all reference "nest". They only reference the device name, which may not contain the word "nest" and there can be several different devices all with completely different names. It would be nice to just have a single entry for each device with all the different "state attributes" grouped with that device.

Also, as was mentioned earlier, the "state attributes" don't currently have a good way to indicate the unit of measurement like regular states do. You either have to get it from another attribute or just decide what it is yourself. It's a bit odd to me that my climate.nest has a state of heat but the unit_of_measurement is °F. That unit of measurement is actually referring to the temperature attributes, not the state itself.

OverloadUT commented 6 years ago

@tboyce021's thoughts mirror my own. It's been days since I first mentioned it in a comment here, and I still feel that having the ability to have entities be "children" of other entities could be very nice. Then things like battery_level can ALWAYS be their own entities, but the UI can handle displaying them intuitively as, more or less, sub-entities of a main device.

balloob commented 6 years ago

It would totally be possible to add battery level to graphs if it is stored in an attribute. We already do this for climate entities. Although it is important not to forget the frontend, it should not drive any design decisions.

My issue with child entities is the complexity that we're adding.

andrey-git commented 6 years ago

The special hack for climate graph is a pain. I would prefer if we solved this without adding another hack.

I agree that the FE shouldn't drive BE design decisions, but the BE should expose enough data for a frontend

tboyce021 commented 6 years ago

I haven't looked at the code, but I would assume treating certain attributes as basically "less important" states would add it's own degree of complexity, but I have no idea which would add more.

The history and recorder components allow specifying a filter to include/exclude certain entities. How does this work for these state attributes? Does including the climate platform automatically include the operation mode, target temperature, current temperature, etc? If so, which ones does it include? It's not exactly clear (I honestly had no idea it even did this). And how would you go about filtering a subset of those out if you didn't want to save them in the database or show them in the history? Would we need to add configuration/logic to the history and recorder components to support this or is it somehow already supported?

I also would like these things to be presented more consistently. Is there a way to add an MQTT sensor that is a state attribute of an existing sensor? If all other components have battery level as an attribute, I would prefer to be able to do the same for my MQTT devices. Are we alright with adding something like an attributes config to all the MQTT components (and potentially others, e.g. template sensors) for specifying anything that would be considered an attribute?

binary_sensor:
- platform: mqtt
  state_topic: some_device/state
  state_attributes:
    battery_level:
      state_topic: some_device/battery
  static_attributes:
    firmware_version:
      state_topic: some_device/firmware

There's also still the issue of attributes needing to specify a unit of measurement, especially for components that may expose multiple state attributes with different units. Roomba, for example, has attributes for position, error, cleaning_time and cleaned_area. All of these could be useful for automation or history, but none of them specify the unit of measurement. I assume cleaning_time is in minutes and cleaned_area is either square feet or square meters, but there's nothing to indicate for sure.

I personally don't care what approach is used, I would just like to be able to have similar functionality and flexibility with state attributes as I do with regular states, and for things to be consistent across the various different platforms and components.

balloob commented 6 years ago

The recorder should always record every state change completely. If something changed that was not related to a state, it should not have fired a state change. Neither should it have been set via hass.states.set_state -> it shouldn't live on the state.

BioSehnsucht commented 6 years ago

Is it possible to expose data to the UI (such as via attributes) for display without recording it? For example, showing the hardware or software version of a device might be useful to display in the details of an entity, but might not be useful to log / graph.

balloob commented 6 years ago

So I think that for now I propose the following solution. This is a solution that will keep our API simple, which I think is one of our strengths.

andrey-git commented 6 years ago

custom UI uses states for configuration lives outside of HASS because there is no built-in way to do it otherwise :)

NovapaX commented 6 years ago

About the entity and attributes (like battery levels) discussion above: I've always felt an entity should also be a 'real-life distinguishable' entity. So a battery level should always be an attribute of a entity (like a actual physical remote or sensor) and not be a separate entity/sensor. if the history/recorder does not handle that properly, that component should be changed. I've mentioned my nest protect smoke-detectors earlier, 1 device generates 4 separate entities (sensors) and that is not desired behavior IMO. But also a wall plug should always be one entity, the different attributes like 'power consumption', 'voltage', 'current' should never be separate sensors/entities. I know this may be a separate issue, but having clear 'attribute guidelines' are part of the solution. (the only issue I see here is that more configuration is needed to expose these relevant attributes in the front-end)

My solution for now would be: keep everything in the state machine, but start to group the different kind of attributes. If the need really arises it should be fairly easy to start extracting those groups into separate kinds of stores. And make sure no unneccesary entities (and states) are created that actually should be attributes of an entity. That should reduce a lot of clutter and noise in the state machine.

If we're talking about the event bus I think there are better ways to reduce bandwidth issues without extracting stuff out of the state machine. Like filtering unnessary attribute groups, not always sending both the full old and new states on a state-change, and extracting some very often triggered state-changes to seperate events which let the receivers pull the rest of the neccesary info out of the state-machine only if needed. With regards to that last one, I'm thinking about the sun component, there should be better ways to use that then triggering a state change every 1 minute. (I actually don't really know why it does that, the state doesn't even change every minute.)

tinloaf commented 6 years ago

I fully agree with @NovapaX - I think if we try to improve how states / attributes work, we should try to achieve a consensus about when (and I vote for "never") to break out attributes as separate entities. I've seen many discussions about this in PRs (and been part of some), and I think coming to a general decision about this would also benefit the unification of the idea about what states / attributes are. Any thoughts on this, @balloob ?

OverloadUT commented 6 years ago

Now see, I feel exactly the opposite here.

I think requiring the user to know "what physical device is measuring UV index in the living room" in order to determine if it's an entity or an attribute is silly. It's a sensor, so it should be a sensor entity.

To put it another way: If I have two devices that track motion and vibration respectively in the room, and then I replace those two devices with a multisensor that tracks both things in a single device, nothing should change in Hass. There were two things being measured before, and there are two things being measured now. Two sensors before, and two sensors after.

I really REALLY don't like the idea that power and energy readings would be attributes on a switch. The switch is a switch for turning on and off. The energy and power readings are sensors because they are sensing something and reporting the changes.

It would also create problems when you're trying to determine what exactly "real-life distinguishable" means. If you have a weather station that tracks wind speed and wind direction (one physical device) and temperature (a second physical device), under this policy wind speed would be an entity, with direction being an attribute, but temperature would be a separate sensor. To a user, this would be very confusing: why on earth are two of my weather readings under one entity, but the third is its own? Just because it's a discrete device on the roof?

All of these examples are saying the same thing: To me, the whole point of a big fancy controller like Hass is to abstract away the nitty gritty of how various HA platforms and sensors and whatnot work.

I would be more in favor of eliminating non-static attributes entirely than going the other way around.

NovapaX commented 6 years ago

That makes sense too. But when I add my smoke detector to hass I expect to see one entity state card with a state being 'safe' and maybe some additional secondary info. I don't expect to see separate 'conectivity status' and 'battery level' sensor state cards for every device.

I agree things are arbitrary, the multisensor example is spot on. I would expect those to be two separate sensors indeed.

But when I add my smart meter, 9 (ungrouped) entities get added to the state machine. That may not be the biggest problem for end users, but they are not grouped in any way so it takes quite some work to get that sorted out in the UI, and then still it takes a lot of space for info of which I (and probably almost every user) actually only want a summary state-card and more info in the more-info-panel. And should a meter-update (every 10 seconds) really trigger 9 state-object-changes when it could (and probably should) just be one event?

Of course everybody can start creating custom UI for these kind of things, but I think it's just bad UX.

EDIT: I'm not saying it should be one way or the other. I'm just saying there should be clear guidelines as to when a separate entity is warranted and which things should just be attributes. And what state.state should be like. (for which state.summary would be a better name then IMO) For a smoke detectors I just expect a state of 'safe/ok' or 'alarm' And for my smart meter I expect the state to be the calculated power consumption (positive or negative)

pvizeli commented 6 years ago

I would prefere the info data to merge it to state.info after it comes from statemachine.

balloob commented 6 years ago

So I'm more with @OverloadUT here.

@NovapaX we should not conflate the concerns of the user interface and the backend. It's totally possible for the backend to have it all split up and the frontend to still show it grouped. It also allows cool things like what we do with our generic thermostat. It takes in a switch that controls a heater and sensor and mimics a thermostat by turning the switch on when it gets too cold.

This reminds me of this great video that I saw the other day about 1st order and 2nd order synthetic sensors: https://youtu.be/aqbKrrru2co?t=217 (the whole video is worth a watch btw)

Alright, back to topic:

We have done a great job to make sure that the core is not aware of what it is used for. This has allowed the core to stay small and lean. Just a little over 1100 lines including an async and a sync API.

We're breaking from this when we want the core to become aware of which attributes it stores and what they mean. It also means that it will limit what custom components can achieve, as their things might not "known" by the core. That's not a good path to take.

With hass.info we are adding a new piece to the core where we can store static info for entities that are not related to the current state. Think manufacturer, firmware version or if an entity should be synced over to Google Assistant. This doesn't mean that the info can't get updated, it's just that the frontend or other consumers don't have to listen to all changes because they can assume it's static and not being up to date is not the end of the world.

NovapaX commented 6 years ago

Yes, I suppose you are right and I was trying to solve the issue from the wrong angle. But for UX sake there must be guidelines and a little focus as to how things are exposed by default components and what UI is used. We should just be able to assume state.battery_level is the battery level of the device (and a value between 0 and 100 or maybe a couple of keywords), and if its not there, there is no battery level to report in the UI or elsewhere. Convention over configuration.

On the other hand, storing everything as separate entities is fine too and for the core doesn't really matter if a battery level is stored as a separate entity or an attribute. (if it's changed separately and acted upon a lot I suppose it could as well be a separate sensor.) But it is a bit confusing you could have switch.kitchen_light and sensor.kitchen_light_switch_battery_level two kind of related entities completely uncoupled from each other in the state machine. If there was some kind of grouping or relational info in the state machine it would be a lot easier for UI to group things accordingly. (and for future custom UI components/wizzard to automatically select related entities. The platforms are still and always should be responsible for what is stored in state and info. With that I fully agree.

The core doesn't need to know what attributes it stores and what they mean. But there should be consistency and caution in the platforms about which, and how much, entities are created. For all I care the hass.info could as well just be 'hass.state[entity].info' Or maybe it should actually have been. hass.entities[id].state and hass.entities[x].info but that would be a major architectural and code change and we probably don't want to open that can of worms.

tboyce021 commented 6 years ago

I agree with @novapaX mostly in the need for consistency. It's rather annoying trying to figure out what is considered a separate state and what is considered an attribute.

I rather like the approach SmartThings uses. The UI lists every devices, which could be handled by the frontend so no change needed for that. Under each device, there's a list of "current states", this could just be a list of every state/entity that gets created by the device, including battery level, switch status, temperature, etc. Everything that changes dynamically is considered a state, so there's no need to figure out if battery level or temperature is a state or attribute for each individual device.

With that, there's essentially no need for "attributes" on each state/entity (some of which are created by the same device, so things like firmware and battery level either get duplicated or only stored with one of the states and not the others). Instead, attributes would be device-specific. SmartThings handles this with a separate "data" section under each device, which stores things like manufacturer and model info. They also have a separate, "firmware" section, but for simplicity we could probably leave that as part of data/info/attributes/whatever we call it.

So I also agree with the last comment that something like hass.entities[id].states and hass.entities[id].info would make sense, or for even more clarity, hass.devices[id].states[state_id] and hass.devices[id].info[info_id]. Of course, we could still provide a way to access the same information via something like hass.states[device_id]['battery_level']. Taking that one step further, we could even leave the current API and just make entity_id a concatination of device_id and state_id and it would look identical to what we already have in many cases today: hass.states['device_id_battery_level'].

With that, it's obvious that anything that changes dynamically is a state and on the state machine and anything that is just informational is in the info. No need for this state vs. attribute stuff.

BioSehnsucht commented 6 years ago

I like the idea of a separate info for things which aren't relevant to the state.

In regards to the issue of multiple sensors/devices being related to one physical/logical device (such as the example given of kitchen light switch which also has a sensor for it's battery level), perhaps add a property / attribute related_entities (or whatever name works best) which is a Dict/List/whatever of all the entities related to the device? Then the UI could use this to find all the related and group them if desired, or not - but the core doesn't have to do anything about it, the Dict or whatever is managed by the platform/component that created the entities.

OverloadUT commented 6 years ago

Some sort of relationship mapping between entities has been suggested several times (albeit in different forms) in this thread by different people. My early suggestion was to have "children" entities of a parent entity.

It really feels like something like this would be very beneficial, at least for UI purposes.

rytilahti commented 6 years ago

So some thoughts (I'm mostly agreeing with @OverloadUT & @tboyce021 here):

  1. I would also like having children-entities ("sensors"), which should also be initializable by the platform in question. That alone is the reason switch.tplink does not currently expose sensor entities; it would have been too burdensome split it out to multiple pieces of code, while also making the code less maintainable. The current way of encoding the units of measurement into attribute names feels really hacky, especially when we have sensors which provide a proper interface. As others have mentioned already, this would also simplify building UIs (UI elements presenting entities, among them their sensors) by making integration of sensors from a specific device into one state-card a breeze.

  2. The child-entities should be linked also to their parent. This will also simplify the API usage for external apps (I'm unsure how e.g. floorplan currently finds out what sensor entities belong to which entity, or is there no link at all?), as all it would require is to go through the child entities when looking for a battery sensor of "kitchen.sink". Or to put it other way around, it would allow going through all "battery" sensors and figuring out the parent device if needed (e.g. for notifying that it's time to change battery of entity X).

  3. I think that the platform developers are more knowledgeable than the users to choose which sensors should be exposed (per default at least) to the state tracking, that is "template sensors" is "a hack" to fix the lack of proper ways for exposing this (implicit) knowledge, so here I disagree with @NovapaX's "prefer attributes over entities" -- we should be explicit about what unit is a certain value is. The developer should have an explicit way for stating that the "total_consumption" of "kitchen.sink" is delivered as watts, leaving it to the presenter to do a conversion if wanted. Furthermore, elevating an attribute to a sensor to be tracked should be configurable, if someone really wants to track static "firmware version" information all along.

Short example, instead of the following in the config:

    kitchen_amps:
      value_template: '{{ states.switch.tplink_kitchen.attributes["current"].split(" ")[0] | float }}'
      unit_of_measurement: 'A'

I would prefer to have tplink_kitchen.current sensor any day. The author of switch.tplink should know, that current is reported in units of 'A'.

  1. I don't like the idea of having a separate info living inside of hass namespace. This will further complicate the lives of developers (especially of those using the rest api) with no apparent reason -- I feel like those are attributes of the entity, not some separately living properties. If info is really wanted, there should at least be a clear link between the entity and this information, so that the information can be linked without knowledge of internal workings of the hass daemon.

I'm really starting to like the idea of @OverloadUT about getting rid of non-static attributes (they should be sensors anyway, right?) and keeping the static attributes as part of the entities, as wild as it may sound. To put it shortly: I would be for making the attributes (where it makes sense) behave like sensors with measurement units and all, which would then be exposed for recording. This way the developer may choose to expose the units without converting it themselves and just letting homeassistant (or anyone else using this information) to handle the conversion as needed/wanted (this is something quite a lot of javascript-based frontends do e.g. with date information, right? The original value being stored as a UNIX timestamp, which gets converted by the UI to wanted timezone etc.).

tl;dr: 1) introduce linking between parent and children entities, useful for GUI and external API users, 2) make sensory information be real sensors with units, this could be done by introducing units to attributes (and allow elevation to real sensors if configured so), 3) make developers choose which attributes/sensors are exposed per default.