home-assistant / architecture

Repo to discuss Home Assistant architecture
311 stars 101 forks source link

RFC: Measurable attributes should be sensors #8

Closed ghost closed 3 years ago

ghost commented 6 years ago

@balloob commented on Dec 13, 2017, 8:54 AM UTC:

#10732 started having 2 conversations in 1. This splits out the conversation about having attributes that contain measurements be represented as standalone sensors.

This issue was moved by @balloob from home-assistant/home-assistant/issues/11118.

ghost commented 6 years ago

@balloob commented on Dec 13, 2017, 8:55 AM UTC:

So one thing that I don't see mentioned in the discussion at all is our abstract base classes. We define how a thermostat works, the developer just fills in the properties and methods and you get a nice UI and, because all thermostats follow the same base class, a unified interface to control any thermostat of any brand. For people proposing we split all attributes as sensors, would you expect the components to figure out which sensors and entities are being maintained by an integration?

ghost commented 6 years ago

@tboyce021 commented on Dec 13, 2017, 9:58 AM UTC:

I may be misunderstanding because I don't know much about the core code, but I think this is where the idea of a "device" level comes into play. The thermostat would be a "device" and all states related to it's operation would be stored under it. Most of these would be predefined (temperature, target temperature, battery level, etc.) but we may want to consider allowing additional states that could be accessed for automation or controlled by custom UI if we can't realistically handle special cases. Each device type would require certain states to be present on the device in order to operate (temperature, target temperature, etc. for thermostats).

Essentially, I think what I'm envisioning is basically making the state field a dict and moving any dynamic attributes into that dict along with the single state value that we currently have. Entity then become the same as "device". I'm not sure if we need a way to specify a primary state or not. I don't know what it would really be used for other than UI, which could be handled elsewhere.

ghost commented 6 years ago

@NovapaX commented on Dec 13, 2017, 11:19 AM UTC:

Yes. That would be my way of modeling that data too.

The only problem is that currently every entity is a member of a singly domain (switch, light, sensor). But wall plug can switch and measure. So what domain should that be put in? The attribute could be part of a domain, or the entity could be part of multiple domains.

Say I have a power strip with 6 switchable outlets and power + total energy measurement on each outlet. It also has combined power+total energy measurement, and fancy controllable RGB light. How should this be exposed?

  1. as 21 different entities and states (6 switches, 14 sensors and 1 light) with duplicate device info about model/manufacturer/connection state
  2. as 22 different entities: 1 parent state (with device info) + 21 child entities
  3. as 8 different entities: 6 switches (with additional attributes for power measurement in their state) + 1 sensor for the total power measurement + 1 light. (with duplicate device info in the state)
  4. as 8 different entities: 1 parent entity with device info and total power measurement attributes) + 7 child entities (6 switches and 1 light)
  5. as 1 device entity with 21 "entity attributes" in its state.

things to consider for the various options:

This is not an easy problem imo.

ghost commented 6 years ago

@balloob commented on Dec 13, 2017, 4:49 PM UTC:

Comment by @armills (he posted in the wrong issue)


Potentially less disruptive proposal that covers both issues:

We create a new domain device, for which each entity represents a single IoT device. (i.e. 1 multisensor, 1 thermostat, 1 smart bulb, 1 switch, etc.) The attributes on this entity can store all of the metadata. (firmware, brand, etc.) The existing entities can use an attribute device_id to associate themselves with this device.

ghost commented 6 years ago

@tboyce021 commented on Dec 13, 2017, 5:30 PM UTC:

@NovapaX, my best attempt at the considerations:

  • How many states are getting updated on a new readout of the power strip

I think it would be nice if it was a single state change event for the "device" (i.e. the entire strip) mostly just for performance reasons, but obviously automations that trigger based on a single state (e.g. sensor.power_strip_outlet_1_power) need to trigger appropriately.

  • How easy would it be to use the different readouts as triggers for automations

Unless I'm missing something, I haven't found a way to use the current attributes as triggers without using a template trigger, so either way this should make that easier sensor.power_strip_outlet_1_power and sensor.power_strip.outlet_1_power are nearly identical in terms of writing the templates, but obviously the first one matches what we have currently.

  • How are the entities displayed by default in the UI

I would personally be alright with either displaying all entities within a group representing the device or just displaying them all individually. Displaying them individually by default may be easiest and I'm guessing as long as there's a way to link the entities together, custom UI or something could easily group them for you if you want.

  • Which services should I be able to call on what entity

For the most part, the services would match the entities (switch services for each switch, and light services for the light). If the strip supports anything beyond those, it could just have its own service (fancy_power_strip.do_something).

  • How to define the base classes (domains?) on the different entities.

This part I don't really know. But, look at something like the Nest component or any other hub component that creates entities in multiple domains. Could we use something along those lines?

ghost commented 6 years ago

@BioSehnsucht commented on Dec 13, 2017, 6:51 PM UTC:

If adding a new device domain as @balloob proposes, then regarding the issue of whether UI should group all members of the device or not, probably stick to the naive default behavior we have now (list all things by domain), then add an option for when creating a custom view that lets you do something like group.device.device_id to create a group of just the entities that are part of that device_id

ghost commented 6 years ago

@rytilahti commented on Dec 14, 2017, 12:43 AM UTC:

Say I have a power strip with 6 switchable outlets and power + total energy measurement on each outlet. It also has combined power+total energy measurement, and fancy controllable RGB light. How should this be exposed?

as 8 different entities: 1 parent entity with device info and total power measurement attributes) + 7 child entities (6 switches and 1 light)

This is the closest that I think how it should work. If each outlet has a energy measurement possibility, the sensors do belong under the switch entity, not under the power strip as is the case with the light.

One power strip entity (or device as discussed above), consisting of:
  * Switch[6]
    - turn_on()
    - turn_off()
    - switch_custom_service()
    - .. and_other_switch_domain_functionalities ..
    * Sensors:
          - temperature sensor
          - voltage sensor
          - ..
  * Children of Switch light entity
How many states are getting updated on a new readout of the power strip.

Each sensor has a state.

How easy would it be to use the different readouts as triggers for automations

Each sensor would still exists under the sensor domain, so they should be trackable by any automation. The strip itself would be a logical composition of its sub-entities.

How are the entities displayed by default in the UI

The platform developer of this "strip" device has to decide how the information is best exposed to the users. Although preferably the UI will have a tab containing access to all of its sub-entities. A summarized form however should be decided by the developer, and be kept concise and useful.

Which services should I be able to call on what entity

I think one should be commanding the specific entitity, but that the main platform could provide global services where feasible.

How to define the base classes (domains?) on the different entities.

The base classes / domains of sub-entities shall follow the structure of existing related domains. E.g. light of the strip should follow the API of light domain (where it also resides). The switch #1 is still a switch, implementing its interface.

So code-wise I would expect the following:

  1. Strip entity creates 1x light.<strips_id>_<some_random_uuid> and 6x switch.<strips_id>_<some_random_uuid> entities.
  2. All those entities are passed a reference to the strip entity, which is how they interface and get updated by the strip.
  3. Each switch and light provide the functionality those domains already do.
  4. The developer of this strip platform will implement turn_off as he sees fit. If the switches support separate switching on/off, the turn_off should be targeted on the wanted switch. If that is not the case, the switches themselves shall not implement turn_off, as it is a functionality of that strip.

edit: to add, I like @armills proposal for how this probably should work on the low-level. Although I'm unsure if it makes sense to add a new domain instead of choosing the most suitable and use it as a base? In case of this strip example it would be a switch.

edit2: on API level the strip developer should hold the ownership of those entities, if I create a switch.tplink I should be able to add sub-entities (and update them as needed) for those sensors without requiring to construct a full-blown component.

ghost commented 6 years ago

@c727 commented on Feb 15, 2018, 6:40 PM UTC:

I think this topic needs some more attention as we run into issues around this again and again: Imo we should have separate entities for each state, here is a concept for a light domain: un22benannt

some more notes: https://docs.google.com/presentation/d/12tu2G7lV4Ybph6_u7tz35WBI1pXQwcKsGF9SPj_ucPM/edit?usp=sharing

balloob commented 6 years ago

@c727 can you name an issue that we run into again and again?

I'm not a fan of splitting up entities by their attributes. It makes a lot more sense as it currently is. I was looking at the web of things spec and they had 2 things different: they didn't have state that describes the overall state and they had units of measurements per attribute.

dgomes commented 6 years ago

jumping into the discussion:

I'm currently working on a weather.platform and its clear that state (current weather condition) is the least relevant information for me, Temperature, Humidity, Pressure is far more relevant. And if I want a chart of those, I'll have to create sensor.templates for each of them.

balloob commented 6 years ago

That's incorrect. You just need to update the chart code to take it into consideration. For example, for climate components we'll plot both target and current temperature.

Also, we should not have the frontend guide our backend decisions. Making changes like this impacts everything. If we can solve it in the frontend, we should do so as it impacts nothing.

balloob commented 6 years ago

I also talked in the polymer repo about adding entities to the history component. The history component could target either the history of an entity state but one could also add functionality to plot a state attribute.

dgomes commented 6 years ago

I agree that the frontend shouldn't guide backend decisions. 100%

But currently it feels that the frontend is simply making up for less generic options in the backend. The web of things points towards that more generic approach.

Lets rephrase the issue: To the the backend, why does it matter that state is a single property and attributes is a bag of properties ?

c727 commented 6 years ago

@balloob :

dgomes commented 6 years ago

I share the issues of @c727 but don't think that making everything a sensor is the way to go. It's just a too big of a flat space. We should instead workout attributes into something that can be easier to use to solve the aforementioned issues.

(in my personal setup half of the sensors are actually attributes from entities that need to be exposed for automation purposes)

balloob commented 6 years ago

Complex domains like plant are 2nd order components. They take "raw" data and process it to other info. Same as the statistic sensor, the threshold sensor etc etc. We always had a very strict rule of components not using other components for UI purposes or even being frontend aware. Things will end up not fitting the use case, configuration options get added to make it happen, and we get in config hell. For example, take https://github.com/home-assistant/home-assistant-polymer/pull/886#issuecomment-365241074, the user didn't like our date time input, decided to roll it's own with 2 sliders. However, in that use case the sliders need more config options to have his slider look better. We can't be adding that stuff or things go south very quickly.

Any argument for restructuring the core because of something frontend related, is moot.

And what's up with the extracting attributes for automation purposes? You can use the numeric_state trigger with a value_template and point it at any attribute for example. If the automations need more flexibility, let's see if we can fix that. We can add automation triggers without having to change the core.

Did either of you ever written an automation in pure Python with our event helpers (which power the automation component)? So much is possible, yet we didn't expose every little thing in YAML because it, again, turns complex.

Home Assistant has been around for 4 years. One reason that for example @dgomes, who is recently new to our community, is able to pick up our code base so quick and able to contribute, is because we are keeping it simple. Complexity is something that can never be reversed once merged into the codebase.

c727 commented 6 years ago

I checked the WoT docs (https://iot.mozilla.org/wot/#web-thing-description) that you mentioned, balloob. I see similarities to both concepts there. Maybe exposing attributes is indeed the wrong approach and unnecessary. My intention was how can we log attribute changes without logging all attributes each time. But I like the idea of grouping all states as "properties" (which brings us back to https://github.com/home-assistant/home-assistant/issues/10732 ) and assigning a type and unit to each property. My intention also was to make attributes(states) basic types like number, string, Boolean and get rid of those arrays and objects there

balloob commented 6 years ago

So all attributes actually already have a unit attached to them, although they are not part of the state itself. We know brightness for a light is 0..255, media player volume is 0..1 etc.

dgomes commented 6 years ago

You say attributes already have units, but they are loosely attached (its just semantics), care taking the units of so many physical entities is nightmarish, and prone to mismatch: e.g. volume is usually 0 to 11 🎸

If for instance I'm writing a climate or weather component, there is state = temperature and proper unit_of_temperature property. But if I take a pressure god knows which unit to stick (mPa, atm, etc)

balloob commented 6 years ago

That's a bug in the integration and should be fixed there. Adding range validation to every state update is way too costly.

balloob commented 6 years ago

Your arguments can pretty much be solved by improving the documentation. Pull requests welcome to do that 👍

dgomes commented 6 years ago

Don't understand... my example was rhetoric (1 to 11 is what you find in guitar amps 🎸 , not im Home Cinemas 🎦 )

Of course one can document what units are being used, but to show all the attribute units in a climate component I need to develop a custom_ui for it. Shouldn't the unit be a property of the the attribute.

A "quick fix" would be for all int/floats to be converted into tuples (value, unit)

balloob commented 6 years ago

The documentation comment was as a response to:

If for instance I'm writing a climate or weather component, there is state = temperature and proper unit_of_temperature property. But if I take a pressure god knows which unit to stick (mPa, atm, etc)

Just a unit is not enough. What about the range? Or maybe it's a string with extra values? These things should not be stored in the state machine. If you want to have this available somewhere, feel free to write a schema definition of the state and its attributes. You can base it on the current documentation which is in the docs of each property on the entity base classes.

tboyce021 commented 6 years ago

@balloob are you suggesting that the current unit_of_measurement property shouldn't exist either? Units for the state itself should be obtainable from the docs just as easily as for properties, right? Where would the history component get the units if they're not available via the API in any way?

Regardless of where the units are defined, I don't see why it should be done differently for state vs. attributes. Attributes just seem like "less important" states in essence.

That was my original sentiment for suggesting this. Currently, states and attributes are treated and handled differently. There's some hoop jumping required to work with attributes that doesn't exist when working with states (need to use templates for automation triggers, currently no history graphs, no units information available from the API, etc.). This causes some confusion because after people learn how to work with states, they then need to learn some additional skills to work with attributes. This just seems odd to me since many attributes only differ from states in that they're considered "secondary", even though whether or not that holds depends on how each person is using the component. What's secondary to one person may be the primary use for another.

The other thing that is somewhat odd is that some components (eg. MQTT sensors) have no way to set attributes (at least not an easy way), despite also potentially reporting things like battery level that are frequently an attribute. This results in some components having certain things as attributes while others have to keep them as a separate state. This can obviously be fixed by having attribute topics for MQTT and other components with this issue, but in the current implementation I just find it odd.

tboyce021 commented 6 years ago

I personally don't care too much how this is handled, whether it's a bunch of separate entities, states grouped by "device", or just better support for working with attributes. I just don't really understand the need for the distinction between state and measurable attributes. They're both just data reported by some component, right?

balloob commented 6 years ago

Attributes are not "less important" states but instead are things that describe details of the current state. A light that is on can have a color, a brightness, etc. I would say that battery level should totally be an attribute.

cgarwood commented 6 years ago

This would be a major architecture change, but what about creating a single entity per device that contains all of the sensor values/states/attributes, instead of a bunch of separate sensor entities.

Take a zwave multisensor for example. Currently you get separate sensors for motion, temperature, humidity, luminance, alarm_level, alarm_type, burglar, etc. What if, instead, you just had a single entity that contained all of this detail like such:

{
  "temperature" : {
    "value" : 71.6
    "unit_of_measurement" : "F"
  },
  "humidity" : {
    "value" : 52
    "unit_of_measurement" : "%"
  }
  "luminance" : {
    "value" : 123
    "unit_of_measurement" : "lux"
  }
  "motion" : false
  "alarm_type" : 22
  "alarm_level" : 0
  "burglar" : 8
  "sourcenodeid" : 0
  "battery_level" : {
    "value" : 98
    "unit_of_measurement" : "%"
  }
}

You could then access any of those values using dot-notation in automations or elsewhere in the config. For example, living_room_multisensor.motion or living_room_multisensor.battery_level.value

Everything would be treated the same as a state, no more template sensors to pull out attributes. You would add the different bits to the frontend using the same dot notation, so instead of the current setup of sensor.living_room_multisensor_temperature you'd just include living_room_multisensor.temperature in the group/view.

I'm not sure how the current domain structure of sensor,binary_sensor,etc would fit in with this architecture.

frenck commented 3 years ago

This issue has been long overdue.

We actually enforce this original request in PRs nowadays. We prefer having actual entities over attributes. However, we made attributes more accessible as most automation triggers/conditions can now work with entity attributes as well.