Add support for Wearable device status for xiaomi-ble#166322
Conversation
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
|
Hey there @Jc2k, @Ernst79, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
There was a problem hiding this comment.
Pull request overview
This PR extends the Xiaomi BLE integration’s sensor mapping to expose additional wearable-status sensors needed for newer Xiaomi wearables (e.g., MiBand 10).
Changes:
- Add sensor descriptions for wearable charging state, sleep state, and wearing status.
- Assign appropriate icons for the new wearable-status sensors.
| # Charging state for Wearable device | ||
| (ExtendedSensorDeviceClass.CHARGING_STATE, None): SensorEntityDescription( | ||
| key=str(ExtendedSensorDeviceClass.CHARGING_STATE), | ||
| icon="mdi:battery-charging", | ||
| ), |
There was a problem hiding this comment.
Add coverage in tests/components/xiaomi_ble/test_sensor.py to verify the new wearable sensors (charging state, sleep state, device wearing status) are created and expose the expected friendly name/icon and state values from an advertisement payload.
| # Charging state for Wearable device | ||
| (ExtendedSensorDeviceClass.CHARGING_STATE, None): SensorEntityDescription( | ||
| key=str(ExtendedSensorDeviceClass.CHARGING_STATE), | ||
| icon="mdi:battery-charging", | ||
| ), |
There was a problem hiding this comment.
Update these wearable state sensors to be modeled as enum sensors and align the processor typing accordingly. These values are discrete strings (e.g., "Charging"/"Full", "Awake"), but the sensor update pipeline is currently typed/cast as float | None; set device_class=SensorDeviceClass.ENUM with options for the allowed states and update the PassiveBluetoothDataUpdate/XiaomiPassiveBluetoothDataProcessor generics and native_value return type to use StateType (or include str) instead of float so the annotations match actual values.
| # Charging state for Wearable device | ||
| (ExtendedSensorDeviceClass.CHARGING_STATE, None): SensorEntityDescription( | ||
| key=str(ExtendedSensorDeviceClass.CHARGING_STATE), | ||
| icon="mdi:battery-charging", | ||
| ), | ||
| # Sleep state for Wearable device | ||
| (ExtendedSensorDeviceClass.SLEEP_STATE, None): SensorEntityDescription( | ||
| key=str(ExtendedSensorDeviceClass.SLEEP_STATE), | ||
| icon="mdi:sleep", | ||
| ), | ||
| # Device wearing status for Wearable device | ||
| (ExtendedSensorDeviceClass.DEVICE_WEARING_STATUS, None): SensorEntityDescription( | ||
| key=str(ExtendedSensorDeviceClass.DEVICE_WEARING_STATUS), | ||
| icon="mdi:watch", | ||
| ), |
There was a problem hiding this comment.
@Ernst79 Might know more about this, but aren't these all binary sensors as they sounds like on/off values? Is there a way for xiaomi_ble to handle such values?
There was a problem hiding this comment.
Charging state can have 3 values.
if xobj[0] == 0:
charging_state = "Charging"
elif xobj[0] == 1:
charging_state = "Not Charging"
elif xobj[0] == 2:
charging_state = "Full"
I think the others also have more than two possible values.
There was a problem hiding this comment.
So we don't really have a device class for full, but we do have one for charging, and that also is taken into account for example in the UI
There was a problem hiding this comment.
So if we want to give users the best experience, I'd say that it should be a binary sensor (maybe 1 for charging not charging and 1 for full), but if that is hard to architect in this integration then this would be fine as well I guess
There was a problem hiding this comment.
Yes, as you said, it's difficult to represent three states with a binary sensor.
Sleep status and wearing status might be possible, but that would require adding new states in binary sensor. I don't know if that is easy.
There was a problem hiding this comment.
Please hold with any changes, as I don't agree with Joost. In my opinion, we should keep it as one sensor. Anyways I have put this PR on the agenda of the next core meeting on Tuesday. Will report back afterwards.
For this reason, I will also draft it.
There was a problem hiding this comment.
@edenhaus If adding the sleep state to the binary sensor description, it is necessary to consider how to distinguish whether "sleep" refers to the device sleeping or a person sleeping, as this may cause confusion for translators. Since my native language is Chinese, the terms "sleep" and "wake" have already caused me confusion.
There was a problem hiding this comment.
Okay, charger state can be a single sensor, but the other 2 sound like binary description.
Could it help if the entity name is "Wearer sleep state"? Or something along those lines to indicate that we're talking about the person wearing the device being asleep as opposed to the device itself?
There was a problem hiding this comment.
@joostlek I updated the charger state and added options to specify settings. The other two were changed to binary sensors, and the local language range is now limited to the xiaomi_ble to avoid ambiguity.
| assert await hass.config_entries.async_unload(entry.entry_id) | ||
| await hass.async_block_till_done() | ||
|
|
||
| assert entry.data[CONF_SLEEPY_DEVICE] is True |
There was a problem hiding this comment.
This assertion is incorrect for the M2456B1 device being tested. The Xiaomi Smart Band 10 (M2456B1) is not a sleepy device, so CONF_SLEEPY_DEVICE should not be set to True. This assertion was likely copied from the previous test_sleepy_device_restore_state test but does not apply here. This test will fail if executed.
| assert entry.data[CONF_SLEEPY_DEVICE] is True | |
| assert entry.data.get(CONF_SLEEPY_DEVICE) is not True |
| assert await hass.config_entries.async_unload(entry.entry_id) | ||
| await hass.async_block_till_done() | ||
|
|
||
| assert entry.data[CONF_SLEEPY_DEVICE] is True |
There was a problem hiding this comment.
This assertion is incorrect for the M2456B1 device being tested. The Xiaomi Smart Band 10 (M2456B1) is not a sleepy device, so CONF_SLEEPY_DEVICE should not be set to True. This assertion was likely copied from the previous test_sleepy_device_restore_state test but does not apply here. This test will fail if executed.
| assert entry.data[CONF_SLEEPY_DEVICE] is True | |
| assert entry.data.get(CONF_SLEEPY_DEVICE) is not True |
| ExtendedBinarySensorDeviceClass.ASLEEP: BinarySensorEntityDescription( | ||
| key=str(ExtendedBinarySensorDeviceClass.ASLEEP), | ||
| icon="mdi:sleep", | ||
| name="Asleep", | ||
| translation_key="asleep", | ||
| ), | ||
| ExtendedBinarySensorDeviceClass.CHILDLOCK: BinarySensorEntityDescription( | ||
| key=ExtendedBinarySensorDeviceClass.CHILDLOCK, | ||
| ), | ||
| ExtendedBinarySensorDeviceClass.DEVICE_FORCIBLY_REMOVED: BinarySensorEntityDescription( | ||
| key=ExtendedBinarySensorDeviceClass.DEVICE_FORCIBLY_REMOVED, | ||
| device_class=BinarySensorDeviceClass.PROBLEM, | ||
| ), | ||
| ExtendedBinarySensorDeviceClass.DOOR_LEFT_OPEN: BinarySensorEntityDescription( | ||
| key=ExtendedBinarySensorDeviceClass.DOOR_LEFT_OPEN, | ||
| device_class=BinarySensorDeviceClass.PROBLEM, | ||
| ), | ||
| ExtendedBinarySensorDeviceClass.DOOR_STUCK: BinarySensorEntityDescription( | ||
| key=ExtendedBinarySensorDeviceClass.DOOR_STUCK, | ||
| device_class=BinarySensorDeviceClass.PROBLEM, | ||
| ), | ||
| ExtendedBinarySensorDeviceClass.FINGERPRINT: BinarySensorEntityDescription( | ||
| key=ExtendedBinarySensorDeviceClass.FINGERPRINT, | ||
| icon="mdi:fingerprint", | ||
| ), | ||
| ExtendedBinarySensorDeviceClass.KNOCK_ON_THE_DOOR: BinarySensorEntityDescription( | ||
| key=ExtendedBinarySensorDeviceClass.KNOCK_ON_THE_DOOR, | ||
| ), | ||
| ExtendedBinarySensorDeviceClass.PRY_THE_DOOR: BinarySensorEntityDescription( | ||
| key=ExtendedBinarySensorDeviceClass.PRY_THE_DOOR, | ||
| device_class=BinarySensorDeviceClass.TAMPER, | ||
| ), | ||
| ExtendedBinarySensorDeviceClass.TOOTHBRUSH: BinarySensorEntityDescription( | ||
| key=ExtendedBinarySensorDeviceClass.TOOTHBRUSH, | ||
| ), | ||
| ExtendedBinarySensorDeviceClass.WEARING: BinarySensorEntityDescription( | ||
| key=str(ExtendedBinarySensorDeviceClass.WEARING), | ||
| icon="mdi:watch-variant", | ||
| name="Wearing", | ||
| translation_key="wearing", | ||
| ), |
There was a problem hiding this comment.
For consistency with other ExtendedBinarySensorDeviceClass entries in the dictionary (ANTILOCK, ARMED, CHILDLOCK, etc.), the key parameter should use the enum value directly without str() conversion. While both approaches may work, using str() only for these two entries is inconsistent with the established pattern in this file.
| key=ExtendedBinarySensorDeviceClass.TOOTHBRUSH, | ||
| ), | ||
| ExtendedBinarySensorDeviceClass.WEARING: BinarySensorEntityDescription( | ||
| key=str(ExtendedBinarySensorDeviceClass.WEARING), |
There was a problem hiding this comment.
Fix inconsistency: remove str() conversion to match the pattern used by ASLEEP and other ExtendedBinarySensorDeviceClass entries.
| key=str(ExtendedBinarySensorDeviceClass.WEARING), | |
| key=ExtendedBinarySensorDeviceClass.WEARING, |
ad4aa1b to
0afede5
Compare
| key=str(ExtendedSensorDeviceClass.CHARGING_STATE), | ||
| device_class=SensorDeviceClass.ENUM, | ||
| icon="mdi:battery-charging", | ||
| options=["Charging", "Not Charging", "Full", "Unknown"], |
There was a problem hiding this comment.
- Are we able to make these snake_case?
- Can we map
unknowntoNone? (Otherwise the automation editor shows Unknown twice
There was a problem hiding this comment.
1.To maintain consistency with other sensors in xiaomi_ble, snake_case was not used. I will try using snake_case, and if I encounter unsolvable issues, I will remove the options attribute.
2. I will change "Unknown" to "Other" to distinguish it from "None".
There was a problem hiding this comment.
- When you use snake case, you can use the translations to translate it
- I would argue that Unknown means the same as unknown, so no need to map it to Other IMO
There was a problem hiding this comment.
@joostlek Sorry, I encountered some unsolvable issues, so I abandoned this pull request.
| key=str(ExtendedSensorDeviceClass.CHARGING_STATE), | ||
| device_class=SensorDeviceClass.ENUM, | ||
| icon="mdi:battery-charging", | ||
| options=["Charging", "Not Charging", "Full", "Unknown"], |
There was a problem hiding this comment.
- When you use snake case, you can use the translations to translate it
- I would argue that Unknown means the same as unknown, so no need to map it to Other IMO
Proposed change
This PR adds three
SENSOR_DESCRIPTIONSfor Xiaomi wearable devices, which are required for the Xiaomi MiBand 10.CHARGING_STATE- Sensor: Not charging, Charging, Full and UnknownASLEEP- Binary sensor: On means Asleep, Off means AwakeWEARING- Binary sensor: On means Wearing, Off means Not Wearing,Depedency bump will be done in a separate PR: #167384
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: