Fix TypeError in UniFi LED brightness control#155936
Conversation
|
Hey there @Kane610, 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 refactors the brightness conversion logic in the UniFi light integration by extracting inline brightness calculations into dedicated utility functions. The changes improve code maintainability and readability without altering the functional behavior.
Key Changes
- Extracted brightness conversion logic into three new helper functions:
convert_brightness_to_unifi(),convert_brightness_to_ha(), andget_device_brightness_or_default() - Refactored
async_device_led_control_fn()to use the new helper functions with clearer control flow - Simplified
async_update_state()to use the new conversion function
| """Get device's current LED brightness. Defaults to 100 (full brightness) if not set.""" | ||
| if device.led_override_color_brightness is None: | ||
| return 100 | ||
| return int(device.led_override_color_brightness) |
There was a problem hiding this comment.
The function get_device_brightness_or_default returns a UniFi brightness value (0-100) but may return the device's current brightness value without validation. If device.led_override_color_brightness is a string (as accepted by convert_brightness_to_ha), this could raise a ValueError. Consider adding validation or clarifying the expected type to ensure consistency with the type hint annotation that expects int | str | None in the companion function.
| """Get device's current LED brightness. Defaults to 100 (full brightness) if not set.""" | |
| if device.led_override_color_brightness is None: | |
| return 100 | |
| return int(device.led_override_color_brightness) | |
| """Get device's current LED brightness. Defaults to 100 (full brightness) if not set or invalid.""" | |
| value = device.led_override_color_brightness | |
| if value is None: | |
| return 100 | |
| try: | |
| return int(value) | |
| except (ValueError, TypeError): | |
| return 100 |
There was a problem hiding this comment.
This is a valid concern. Why is the conversion needed in the first place? That sounds like either a logic or typing issue?
There was a problem hiding this comment.
@frenck its super hard to tell, I was initially able to reproduce the issue after the initial update to HA 2025.11, but unfortunately now fail to reproduce at all, clean installation, reinitialization etc. doesn't help.
This was the initial stacktrace
[31m2025-11-06 10:03:52.668 ERROR (MainThread) [homeassistant.components.websocket_api.http.connection] [546689866048] Unexpected exception
Traceback (most recent call last):
File "/usr/src/homeassistant/homeassistant/components/websocket_api/commands.py", line 270, in handle_call_service
response = await hass.services.async_call(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...<7 lines>...
)
^
File "/usr/src/homeassistant/homeassistant/core.py", line 2816, in async_call
response_data = await coro
^^^^^^^^^^
File "/usr/src/homeassistant/homeassistant/core.py", line 2859, in _execute_service
return await target(service_call)
^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/src/homeassistant/homeassistant/helpers/service.py", line 830, in entity_service_call
single_response = await _handle_entity_call(
^^^^^^^^^^^^^^^^^^^^^^^^^^
hass, entity, func, data, call.context
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
)
^
File "/usr/src/homeassistant/homeassistant/helpers/service.py", line 902, in _handle_entity_call
result = await task
^^^^^^^^^^
File "/usr/src/homeassistant/homeassistant/components/light/__init__.py", line 651, in async_handle_light_on_service
await light.async_turn_on(**filter_turn_on_params(light, params))
File "/usr/src/homeassistant/homeassistant/components/unifi/light.py", line 140, in async_turn_on
await self.entity_description.control_fn(self.hub, self._obj_id, True, **kwargs)
File "/usr/src/homeassistant/homeassistant/components/unifi/light.py", line 72, in async_device_led_control_fn
DeviceSetLedStatus.create(
~~~~~~~~~~~~~~~~~~~~~~~~~^
device=device,
^^^^^^^^^^^^^^
...<2 lines>...
color=color,
^^^^^^^^^^^^
)
^
File "/usr/local/lib/python3.13/site-packages/aiounifi/models/device.py", line 847, in create
if not (0 <= brightness <= 100):
^^^^^^^^^^^^^^^^^^^^^^
My assumption is that due to the usage of non-official APIs from UniFi, at some point the API returns a wrong type.
As the library implementation does not typeguard, a wrong type may end up in HA.
Why This Matters: When async_device_led_control_fn is called without ATTR_BRIGHTNESS (e.g., just turning on the light, like in the issue (#155892) mentioned), it uses the device's current brightness from the UniFi API.
I've added a try-catch block with some logging so we have more information if it would happen again in the future, but this should fix the issue some users are facing.
There was a problem hiding this comment.
My assumption is that due to the usage of non-official APIs from UniFi, at some point the API returns a wrong type.
As the library implementation does not typeguard, a wrong type may end up in HA.
With that in mind, the type conversion is unexpected. It masks the actual issue.
There was a problem hiding this comment.
@Kane610 do you think this could be the behavior of the UniFi API?
There was a problem hiding this comment.
There has been bugs before so it is not impossible. lets add the check and logging in the library instead and keep the integration code clean. It should be the responsibility of the lubrary
| """Get device's current LED brightness. Defaults to 100 (full brightness) if not set.""" | ||
| if device.led_override_color_brightness is None: | ||
| return 100 | ||
| return int(device.led_override_color_brightness) |
There was a problem hiding this comment.
This is a valid concern. Why is the conversion needed in the first place? That sounds like either a logic or typing issue?
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
|
Closing as solution masks the actual problem; Library should be improved instead. |
Proposed change
Fixes a TypeError in the UniFi LED brightness control.
brightnessis aintType 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: