Skip to content

Add hass_compat to feature #879

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

Closed
wants to merge 4 commits into from
Closed

Conversation

rytilahti
Copy link
Member

@rytilahti rytilahti commented Apr 28, 2024

This adds hass_compat (name suggestions welcome!) that allows defining extra attributes to homeassistant's *entitydescriptions for better and cleaner UX. At the moment this is just converted to dict and passed directly to *entitydescriptions.

  • Disable some diagnostic features per default
  • Hide some sensor information (like historical consumption) from the dashboard, while still keeping them active

This is based on #872 which needs to be merged first.
Fixes #842

image
image
image
image

This improves the temperature control features to allow implementing climate platform support for homeassistant
Also adds frostprotection module, which is basically also used to turn the thermostat on and off.
@sdb9696
Copy link
Collaborator

sdb9696 commented Apr 30, 2024

It would be good to close out the open question here from @bdraco home-assistant/core#111151 (comment). It might be helpful to list some of the things/scenarios we're currently unable to derive in HA. For example I thought it was going to be sufficient to use the Feature.Category.Debug to drive entity_registry_enabled_default and I'm not sure why we need entity_registry_visible_default because they both hide the entities and I don't believe we have a need for entities that are enabled & hidden.

For the DeviceClass and StateClass it does seem to suggest a lot of HA development knowledge when creating a feature as to how these are all treated in HA. Maybe we could maintain a set of FeatureName to class mappings in the HA integration for those we want to set specifically and then derive a sensible default for those not specified.

@rytilahti rytilahti force-pushed the feat/tempcontrol_improvements branch from c2f8cbb to 6d12158 Compare May 2, 2024 13:03
Base automatically changed from feat/tempcontrol_improvements to master May 2, 2024 13:05
@rytilahti
Copy link
Member Author

rytilahti commented May 2, 2024

I commented on that thread on this, but I agree that we should not proceed with this approach, but leave it to the homeassistant integration to do the mapping. Some of this can be done automatically, but for some we are going to need a mapping inside homeassistant. At least for those that require manual mapping, we are going to need to set the id manually to avoid breaking things if we one day decide to update the naming.

The difference between hiding & disabling is that the disabled entities won't be added to the state machine at all, but hidden ones are not added to the panes like the dashboard automatically. If you take a look at the screenshots in the description, you see that not all sensors (mainly the historical emeter ones) are not added, even when they are shown in the device page and recorded as expected.

On categorization, we could use the Debug to disable the entities, and moving the ones we want to be enabled to use Info, while mapping both Info and Debug to be entity_category=Diagnostics. Is that what you mean?

This would make us have the following categorization rules:
Primary: primary controls (state, brightness, etc.) & sensors (like temperature, emeter)
Config: secondary controls, entity_category=Config
Info: diagnostic information that is enabled per default in homeassistant
Debug: diagnostic information that is disabled per default

@sdb9696
Copy link
Collaborator

sdb9696 commented May 3, 2024

At least for those that require manual mapping, we are going to need to set the id manually to avoid breaking things if we one day decide to update the naming.

Or we set the id if/when we change the naming?

On categorization, we could use the Debug to disable the entities, and moving the ones we want to be enabled to use Info, while mapping both Info and Debug to be entity_category=Diagnostics. Is that what you mean?

Yes, I think this is simplest and we can always use overrides in HA if necessary.

This would make us have the following categorization rules: Primary: primary controls (state, brightness, etc.) & sensors (like temperature, emeter) Config: secondary controls, entity_category=Config Info: diagnostic information that is enabled per default in homeassistant Debug: diagnostic information that is disabled per default

Agreed. Then in HA we have a mapping of feature ids > (device_class, state_class) etc. The mappings can presumably live in each of the entity files as different entities have different description values.

@rytilahti
Copy link
Member Author

Or we set the id if/when we change the naming?

I think we should do this from the get-go, as it will also allow us to use the the ids as translation keys.

Yes, I think this is simplest and we can always use overrides in HA if necessary.

Okay, I'll proceed to create a new PR that updates the categorization to follow this plan.

Agreed. Then in HA we have a mapping of feature ids > (device_class, state_class) etc. The mappings can presumably live in each of the entity files as different entities have different description values.

Makes sense to me, thanks for your input! 👍

@rytilahti
Copy link
Member Author

Replaced with #904.

@rytilahti rytilahti closed this May 7, 2024
@rytilahti rytilahti deleted the feat/homeassistant_extras branch May 7, 2024 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow exposing extra feature metadata
2 participants