Add DEVICE_CLASS_BATTERY_CHARGING to binary_sensor

[bdraco]

Context

The consensus in https://github.com/home-assistant/core/pull/33432 was that a battery charging sensor should be a binary_sensor.

In order to make this automatically discoverable by homekit there needs to be a DEVICE_CLASS_BATTERY_CHARING in binary sensor so we can tell what is a battery and what is a charge sensor. https://github.com/home-assistant/core/pull/33519

Proposal

Add DEVICE_CLASS_BATTERY_CHARING to binary_sensor

Consequences

homekit battery sensor setup can now be done without manual configuration. The frontend already discovers DEVICE_CLASS_BATTERY, and should be able to benefit from this change as well to show if the device is charging.

[Jc2k]

Don’t some batteries already know whether or not they are charging and change their icon accordingly? Are we saying they shouldn’t do that or that they should also know whether or not they are charging separately to this new sensor type?

[bdraco]

Don’t some batteries already know whether or not they are charging and change their icon accordingly?

Yes, this wouldn't change that, homekit is still looking at ATTR_BATTERY_CHARGING if there is no linked binary sensor.

Are we saying they shouldn’t do that or that they should also know whether or not they are charging separately to this new sensor type?

This change will add the ability to look at the binary_sensor for the charging state.

[dshokouhi]

Not sure if it counts here but the android app also shows the battery charging state but it is a sensor and not a binary sensor. Not to be confused with the actual battery level, the state sensor has several states it can be to reflect if it is plugged, charging or discharging. Should we look at all integrations and decide what fits the majority? How many integrations do we have that fit this criteria?

[Jc2k]

This change will add the ability to look at the binary_sensor for the charging state.

Right but are we saying the existing mechanisms are deprecated/removed (icon and attribute)? Or that we should do both?

Right now I have a list of battery entities in a panel. I can tell what’s happening with them at a glance. Will I need to have 2 entities per list in the panel if we go down this route?

[bdraco]

This change will add the ability to look at the binary_sensor for the charging state.

Right but are we saying the existing mechanisms are deprecated/removed (icon and attribute)? Or that we should do both?

AFAICT, the frontend doesn't know about battery charging.

root@ha-dev:~/frontend# git grep -i ATTR_BATTERY_CHARGING
root@ha-dev:~/frontend# git grep -i battery_charging

Right now I have a list of battery entities in a panel. I can tell what’s happening with them at a glance. Will I need to have 2 entities per list in the panel if we go down this route?

The registry won't know about ATTR_BATTERY_CHARGING state so we need a way to find the charging sensor. https://github.com/home-assistant/core/pull/33519

@balloob @frenck Can you weigh in here.

[Jc2k]

Right. There is a utility function that maps a battery state to an icon and that knows about charging.

https://github.com/home-assistant/core/blob/dev/homeassistant/helpers/icon.py

So the frontend doesn’t know directly... but it does know.

[bdraco]

@Jc2k for clarity, if there is any deprecation it would be ATTR_BATTERY_CHARGING not battery level so it shouldn't affect icon_for_battery_level

There are only a few integrations that set ATTR_BATTERY_CHARGING

components/google_maps/device_tracker.py:                ATTR_BATTERY_CHARGING: person.charging,
components/life360/device_tracker.py:            ATTR_BATTERY_CHARGING: _bool_attr_from_int(loc.get("charge")),
components/logi_circle/camera.py:            state[ATTR_BATTERY_CHARGING] = self._camera.charging
components/logi_circle/sensor.py:            state[ATTR_BATTERY_CHARGING] = self._camera.charging
components/tesla/__init__.py:            attr[ATTR_BATTERY_CHARGING] = self.tesla_device.battery_charging()

[Jc2k]

Yes but that function knows about charging doesn’t it???, I think it’s weird that the battery sensor icon knows about charging and but we are saying charging/not charging lives on another entity.

(I am not as invested in this as I may be coming across I just don’t know if I’m being clear about what I mean).

[bdraco]

Not sure if it counts here but the android app also shows the battery charging state but it is a sensor and not a binary sensor. Not to be confused with the actual battery level, the state sensor has several states it can be to reflect if it is plugged, charging or discharging. Should we look at all integrations and decide what fits the majority? How many integrations do we have that fit this criteria?

The iOS app is also implementing this as a separate sensor instead of a binary_sensor

Looking over the code base, the two most common are:

1. Battery charging as a property of a battery sensor
2. Battery charging sensors that are implemented as sensor

[bdraco]

Yes but that function knows about charging doesn’t it???, I think it’s weird that the battery sensor icon knows about charging and but we are saying charging/not charging lives on another entity.

(I am not as invested in this as I may be coming across I just don’t know if I’m being clear about what I mean).

I don't have a strong opinion on this, but others may: https://github.com/home-assistant/core/pull/33432#issuecomment-606842460 https://github.com/home-assistant/core/pull/33432#issuecomment-606040047

I'm just as happy to implement https://github.com/home-assistant/core/pull/33519 by looking at ATTR_BATTERY_CHARGING on a battery sensor, in fact is makes it far simpler and a lot less code.

[frenck]

One good reason for separating it is for users to be able to automate it easily.

From a state perspective, "the level of the battery charged" vs "is currently charging or not", are 2 different states. I think it is valid to separate those into different (binary) sensors.

[balloob]

I'm also for splitting it up. It's been the direction that we're heading: each measurement of a device is represented as a new entity.