-
-
Notifications
You must be signed in to change notification settings - Fork 101
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: Measurable attributes should be sensors #8
Comments
@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? |
@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. |
@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). 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.
things to consider for the various options:
This is not an easy problem imo. |
@balloob commented on Dec 13, 2017, 4:49 PM UTC: Comment by @armills Potentially less disruptive proposal that covers both issues: We create a new domain
|
@tboyce021 commented on Dec 13, 2017, 5:30 PM UTC: @NovapaX, my best attempt at the considerations:
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.
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
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.
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).
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? |
@BioSehnsucht commented on Dec 13, 2017, 6:51 PM UTC: If adding a new |
@rytilahti commented on Dec 14, 2017, 12:43 AM UTC:
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.
Each sensor has a state.
Each sensor would still exists under the
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.
I think one should be commanding the specific entitity, but that the main platform could provide global services where feasible.
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 So code-wise I would expect the following:
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 edit2: on API level the strip developer should hold the ownership of those entities, if I create a |
@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: some more notes: https://docs.google.com/presentation/d/12tu2G7lV4Ybph6_u7tz35WBI1pXQwcKsGF9SPj_ucPM/edit?usp=sharing |
@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 |
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. |
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. |
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. |
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 ? |
@balloob :
|
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) |
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 home-assistant/frontend#886 (comment), 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 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. |
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 home-assistant/core#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 |
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. |
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) |
That's a bug in the integration and should be fixed there. Adding range validation to every state update is way too costly. |
Your arguments can pretty much be solved by improving the documentation. Pull requests welcome to do that 👍 |
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) |
The documentation comment was as a response to:
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. |
@balloob are you suggesting that the current 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. |
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? |
Attributes are not "less important" states but instead are things that describe details of the current state. A light that is |
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:
You could then access any of those values using dot-notation in automations or elsewhere in the config. For example, 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 I'm not sure how the current domain structure of |
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. |
@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/core/issues/11118.
The text was updated successfully, but these errors were encountered: