-
Notifications
You must be signed in to change notification settings - Fork 99
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
States with forecasts #15
Comments
I think that this part of my quote is a pretty solid solution:
The state machine should store the current state of devices. It doesn't store history and neither should it store the future. If the frontend wants to display these, it should fetch it from an API. The weather UI and component should also be updated to do this. |
How can we then access the data for automations? |
Could you provide an example of what this API might look like? Would it look closest to:
What differentiates data which should be stored in state vs data that should be served via an API? The value of "forecast" seems to me like a state: it has a current value for a prediction of the future (it is not a future value). This is different from history, which is an (immutable) record of state at a certain point in time. It seems simplest to use state or state attributes for storing this data (see some discussion about keeping "extra" data in state attributes at this comment and below). That takes us back to the issue of "too much data going into the database".
@balloob would you mind sharing some of what those worms would be (or a link to past discussion)? Rather than selectively recording data, would it make sense to try purging data more intelligently by default? |
The big problem with not recording everything is that we get an incomplete history. Incomplete history will make machine learning on the data less effective. Machine learning will go wrong if we decide not to store those forecasts in the recorder yet we acted on them. I guess that does make a good argument to store the forecasts somewhere. However, the state machine is about the current state and I don't think that we should start conflating those concerns by including forecasts. By keeping the state machine simple, we keep the system fast. Automation access
If it's for the weather component, you could add a weather automation trigger. That trigger could then log the forecast API This should be A similar discussion came up in a PR recently for playlists of a media player. We can't store all available playlists and their ids in the state machine. They can just be fetched on demand through a view or when necessary inside the Conflating concerns Components should not be concerned with implementation details of other components. By going down this route, you will end up with an enormous list of 'things to consider' and still can't solve every use case. The group component is a great example of this going wrong. It groups binary sensor "ON" with a light "ON" state. Configuring other components inside an entity Hints to "not store" don't work well as components can store the data in different mechanisms. It is way better to describe the type of the data so that the component can decide for itself. An example of where this has gone wrong is the "hidden" attribute. Should this entity be hidden from a default view, a group or a view? |
Thank you for the detailed response!
I would argue that, for example, "next hour forecast" is a current state (or state attribute). A similar example is "next track" in a media player -- this is similar to a forecast, but IMO belongs in the state machine. Would you agree, at least with respect to "next track"? However, I understand that even if they might be conceptually similar, it could be desirable to remove forecast from state because of the performance impact.
That makes sense. It was useful for me to read that discussion and see the code here.
So, if someone wanted to trigger an automation based on electricity forecast, as in @Danielhiversen 's original thread, Daniel will now need to add an automation trigger. @balloob would you envision adding a |
Next track could be nice to have, although a lot of media players won't have access to this. Rare attributes are not good candidates to add to Home Assistant, we should not aim to support every feature everywhere. If we can get 90% covered, I would be happy. Things get complicated fast. Take for example the climate component. So there is a target temperature, except some devices allow setting a target range. Curren target temperature can be part of a schedule which the user also can override temporarily. We need to be able to describe all these feature sets, allow controlling them and be able to represent them in the state machine in such a way that a UI can be rendered that makes sense. And so when it comes to forecasts, do we allow next track? Do we allow a daily forecast for the next 10 days? Do we allow an hourly forecast for the next 3 days? By having it being put in a different place, the state machine does not have to worry about this. It also means that we can say: this is a forecast for this attribute or for the state itself etc. In that case you could also have a For automation triggers, it should be limited to components (sun, zone, mqtt) or concepts (state, event). If Daniel's electricity sensor is just a sensor, we should not add an automation trigger for that. We should instead introduce the concept of a forecast. Because if you just put it into the state, do you then want to add all this configuration to the state automation trigger to satisfy that use case? I can see this becoming a complicated config mess. |
The more I think about it, the more I think having a standalone forecast piece in Home Assistant is the right thing to do. That way we can have a forecast trigger but also allow frontend API to be more intelligent about what to fetch, like fetch forecast for next X period etc. Seems like a way cleaner solution than trying to cram that all into the current state machine and trying to match it with templates. |
I think also the https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/automation/litejet.py is fail. They should fire a event on component level and use the event trigger for it. |
I'm down to remove that, I don't even know if anyone will notice 🤔 |
Any further thought/progress on this? Some extremely powerful automations can be made with future values. |
Would love to see this as well, so adding my vote to this. |
This architecture issue is old, stale, and possibly obsolete. Things changed a lot over the years. Additionally, we have been moving to discussions for these architectural discussions. For that reason, I'm going to close this issue. ../Frenck |
In some cases it is useful to add some data array as an attribute. That will make the data available for the frontend and for automations. But this mean the data array are stored in the database, and easily may take a lot of disk space.
Currently we are adding the forecast data array as an attribute in the weather components.
In home-assistant/frontend#868 I wanted to add a data array as an attribute for visualization and automations. But it was not accepted since it easily may tak a lot of disk space. (But not more than the weather component already does)
One way to solve this, is to filter out all attributes starting with
_
from the database. Then we can renameforecast
in the weather component to_forecast
. This will allow to add useful data to the state for automations, but without having to store it in the database.@balloob did not like that solution (home-assistant/frontend#868 (comment)):
I understand his concerns, but I do not have any better idea.
I am hoping for some ideas of how we can solve this.
This issue might be relevante #11
Here a way of filtering out some sensors from being stored in the database might be useful.
The text was updated successfully, but these errors were encountered: