Skip to content

Correctly handle "hours to show" for footer graph#8071

Merged
spacegaier merged 2 commits intohome-assistant:devfrom
spacegaier:issue-8055
Jan 26, 2021
Merged

Correctly handle "hours to show" for footer graph#8071
spacegaier merged 2 commits intohome-assistant:devfrom
spacegaier:issue-8055

Conversation

@spacegaier
Copy link
Copy Markdown
Member

Breaking change

Proposed change

This PR fixes two issues:

  1. The old coding did always check against 24 hours to see whether previously loaded history entries should be discarded rather than using the config parameter "hours to show".
  2. The old coding did not account for the "initial state" of the history. So if 12 hours should be shown, but only 13 hours ago and then 2 hours ago did state changes occur, the frontend receives also the history entry from 13 hours ago from the backend. So the initial graph rendering was always fine. However, during the subsequent updates (when only the delta of newly recorded history entries is retrieved from the backend), the frontend did discard all cached history entries that lie outside the 12 hours. That would mean that "initial state" history entry would be lost (and effectively only values from 2 hours ago until now would be rendered).
    This PR now ensures that the last previous history entry that now lies outside the "hours to show" is retained so that the graph has a correct "entry value" = value on the left border of the graph.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (thank you!)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example configuration

Additional information

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

@spacegaier
Copy link
Copy Markdown
Member Author

@zsarnett Since you did the implementation for the cached history, could you have a look at my changes if those look good to you

@bramkragten bramkragten requested a review from zsarnett January 4, 2021 09:05
Co-authored-by: Bram Kragten <mail@bramkragten.nl>
@spacegaier
Copy link
Copy Markdown
Member Author

@bramkragten I verified your proposals. They indeed work as the order of entries in this._stateHistory plays no role (the coordinates() logic seems to take care of the sorting on its own already), so my convoluted attempt to retain the order was not needed.

@spacegaier
Copy link
Copy Markdown
Member Author

@zsarnett Ping. It would really like to have that in the next beta 😄. The issue drives me nuts on my dashboard.

@spacegaier spacegaier merged commit 9b7d893 into home-assistant:dev Jan 26, 2021
@spacegaier spacegaier deleted the issue-8055 branch January 26, 2021 15:15
@bramkragten bramkragten mentioned this pull request Jan 27, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Jan 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Line graph card loose 24h older data on auto-update when showing 48h

3 participants