Skip to content
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

Filter unrecorded entities from history panel #19621

Merged
merged 4 commits into from
May 29, 2024

Conversation

karwosts
Copy link
Contributor

@karwosts karwosts commented Feb 2, 2024

Proposed change

Don't show unrecorded entities in the history panel target picker.

Could probably expand this to more places (logbook, history graph card, alert in more-info dialog), but just starting simple for a proof of concept.

Requires home-assistant/core#92640

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:

hass: HomeAssistant
): Promise<RecordedEntities> => {
if (recordedEntitiesCache) {
return recordedEntitiesCache;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means new entities will never show up, I don't think we should do this, it will cause users to wonder why their new entity is not showing up in the picker...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oof, right. Maybe invert the list when we fetch it, and cache the list of things which are currently excluded?

I didn't start with caching, but I thought it would be nice as this could be kind of a big list to retrieve repeatedly, and it doesn't change much in a meaningful way (at least without someone editing their config.yaml and restarting).

Maybe also inquire about inverting the polarity of the API, to fetch excluded entities instead of recorded ones. I feel like that's generally going to be a much smaller list in the average usecase.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be easier to just get the filters that are set by the user in config, we can then use

export const generateFilter = (
to filter the entities...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe leave that comment in the core thread so we can collect all the ideas in one place?

I do kind of like that we're now getting visibility into exactly what the backend thinks is recorded, instead of reproducing the filtering logic ourselves, as we've already identified a bug as a result of that. But this could work too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we don't support things like globs in frontend...

Copy link
Contributor Author

@karwosts karwosts Feb 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok flipped the logic around a bit. I still cache the result, but at the moment I get back the api response, I also cache a list of any entity that is present in hass.states, but not in the recorded response. And then use that to exclude from the picker.

So any new entities created after the cache was fetched would not be excluded.

bdraco also suggested we can use this information to populate the picker with entities which are orphaned/missing from the states table, but still have history in the database. Haven't done that yet, but the plumbing is there to do so.

I think hass.states is the right thing to check against right? I don't need to do anything additionally with entity registry?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think hass.states is the right thing to check against right? I don't need to do anything additionally with entity registry?

Yeah that should be fine 👍

@Mariusthvdb
Copy link
Contributor

Mariusthvdb commented Feb 2, 2024

Proposed change

Don't show unrecorded entities in the history panel target picker.

Could probably expand this to more places (logbook, history graph card, alert in more-info dialog),

just letting you know the latter there was the whole reason for our custom plugin custom-more-info.... filter those empty placeholders for History and Logbook..which show on entities that are not recorded in the first place.

If core would do that, that would be awesome indeed.

Also, Ive chatted about 'importing' the HA recorder settings into our plugin, but, as that is a pure Frontend thing, it doesnt read the backend recorder settings....

maybe this new PR can change that at some point. Or, make it unnecessary, which would be even better.

bramkragten
bramkragten previously approved these changes Feb 5, 2024
@bramkragten bramkragten added wait for backend and removed Backend Change Required Requires a Backend Core Code Change labels Feb 5, 2024
@ildar170975
Copy link
Contributor

Currently History panel can show a history for not-recorded entity - if the entity is changing while the History view is open.
For instance, this entity is excluded from Recorder - yet we can see it's "streamlined" data realtime:

image

Disabling a possibility to select a not-recorded entity will kill this feature.
If you about to filter a list - suggest to add a checkbox like "show only recorded" ("on" by default).

Btw, a possibility to show history for not-recorded entities is also available in more-info:

image

And keeping this possibility for a History view could be rather useful for monitoring.

@karwosts
Copy link
Contributor Author

karwosts commented Mar 3, 2024

Currently History panel can show a history for not-recorded entity - if the entity is changing while the History view is open. For instance, this entity is excluded from Recorder - yet we can see it's "streamlined" data realtime:
Disabling a possibility to select a not-recorded entity will kill this feature. If you about to filter a list - suggest to add a checkbox like "show only recorded" ("on" by default).

Believe you can still type any entity id manually in the dropdown in history to keep the current behavior, it just won't be populated in the dropdown list for easy picking.

@ildar170975
Copy link
Contributor

still type any entity id manually

Better than nothing.

@silamon silamon added the backend merged The backend PR for this frontend PR has been merged label May 15, 2024
@bramkragten bramkragten merged commit d886700 into home-assistant:dev May 29, 2024
13 checks passed
@karwosts karwosts deleted the history-filter-unrecorded branch May 29, 2024 20:07
karwosts added a commit to karwosts/frontend that referenced this pull request May 31, 2024
bramkragten pushed a commit that referenced this pull request Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend merged The backend PR for this frontend PR has been merged cla-signed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants