-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Infer a limited history chart from state object #19176
Conversation
@@ -114,7 +114,7 @@ export class StateHistoryChartTimeline extends LitElement { | |||
config: this.hass.config, | |||
}, | |||
}, | |||
suggestedMin: this.startTime, | |||
min: this.startTime.getTime(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need getTime
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Struggling to remember exactly, I think maybe at one time the custom timeline controller didn't handle Date
correctly, but I might have fixed that at some point. I'll see if it works just as well with removing it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't find any issue, removed it 🤷
{ | ||
s: hass.states[entity].state, | ||
a: hass.states[entity].attributes, | ||
lu: new Date(hass.states[entity].last_updated).getTime() / 1000, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be last_changed
and not last_updated
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I convinced myself that last_updated would be more correct, in case we were displaying a chart with attributes (like climate).
If we use last_changed, then we would render that the attributes were constant all the way back to the time of the last state change, but that would be possibly inaccurate, as the attributes may have been changing after that point.
If we cutoff the graph at the time of last attribute change, then we display potentially less data, but guarantee that what is shown is actually correct.
src/data/history.ts
Outdated
]; | ||
} | ||
}); | ||
Object.keys(stateHistory).forEach((entityId) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we combine Object.keys(stateHistory)
and entityIds
at the top so we dont have to go over stateHistory
again?
Could this PR cause this behaviour? #20008 |
Proposed change
If we have a very stable sensor that rarely changes, eventually its last recorded datapoint in the states database may be purged, at which point there is no recorded information for that entity.
If user has a history graph of that entity, at this point it will no longer render, the entity will just silently disappear from the chart, or if that is the only entity, it will display the message "No state history found".
This is suboptimal as it leaves users to think their entity has somehow broken or gone unavailable.
To improve this situation, I believe we can use the information from the entity state object to infer a limited history for an entity which can be displayed in the history chart. Given that we still know the entity's state, and the last updated timestamp, we can show an accurate chart for that entity instead of letting it disappear.
Type of change
Example configuration
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed: