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

Update sensor enabled by default logic to be more granular #3315

Merged
merged 1 commit into from
Feb 15, 2023

Conversation

dshokouhi
Copy link
Member

@dshokouhi dshokouhi commented Feb 7, 2023

Summary

As we have had more battery sensors get added the amount of sensors enabled by default has grown. Some of the battery sensors were created based on requests and probably not used by majority of users. Updating the logic to move from disabled at the manager level to be enabled at the individual sensor level. Default is still false

So now the new default sensors will be:

  • battery_level
  • battery_state
  • charger_type

Screenshots

image

Link to pull request in Documentation repository

Documentation: home-assistant/companion.home-assistant#907

Any other notes

Not sure if this should be marked as a breaking change since this only impacts new installs.

If you are planning on testing this PR make sure to use a device name never used in your HA instance otherwise you will see the old sensors enabled by default.

@jpelgrom
Copy link
Member

jpelgrom commented Feb 7, 2023

Not sure if this should be marked as a breaking change since this only impacts new installs.

Existing users won't notice a difference so it is not a breaking change :)

@jpelgrom
Copy link
Member

jpelgrom commented Feb 7, 2023

If we're keeping charger_type, I'd also keep is_charging, or if the aim is to add as few sensors as possible I expect more people will simply want to know "is my device charging?" than "how is my device charging?".

(Why isn't the charger type an attribute?)

@dshokouhi
Copy link
Member Author

dshokouhi commented Feb 7, 2023

If we're keeping charger_type, I'd also keep is_charging, or if the aim is to add as few sensors as possible I expect more people will simply want to know "is my device charging?" than "how is my device charging?".

The goal is to have less sensors that are not redundant. In this case is_charging is actually derived from battery_state so it makes sense to include more details than the opposite to begin with. Personally I like the full state because then you know your battery is done charging. 100% != a full battery :)

(Why isn't the charger type an attribute?)

That would be a separate state :)

@JBassett
Copy link
Collaborator

Once merge conflict is fixed it will be merged.

@JBassett JBassett enabled auto-merge (squash) February 15, 2023 01:34
@dshokouhi dshokouhi force-pushed the sensor_enabled_default branch from 374bd8d to bd80fd4 Compare February 15, 2023 01:44
@JBassett JBassett merged commit fd1ae9e into home-assistant:master Feb 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants