Add support for SwitchBot Ceiling Lights#159072
Conversation
home-assistant
Bot
added
cla-signed
integration: switchbot_cloud
new-feature
small-pr
|
Hey there @SeraphicRav, @laurence-presland, @Gigatrappeur, @XiaoLing-git, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
|
I have started working on this ages ago, but then kinda... forgot about it because things were working in my local fork; and in the meantime the code has shifted meaningfully, and the changes required now are very small :) |
| async def _send_color_temperature_command(self, color_temp_kelvin: int) -> None: | ||
| """Send a color temperature command.""" | ||
| await self.send_api_command( | ||
| CeilingLightCommands.SET_COLOR_TEMPERATURE, |
There was a problem hiding this comment.
These, arguably, could be the same commands as RGBWWLightCommands, since they use the same actual commands (here: https://github.com/SeraphicCorp/py-switchbot-api/blob/main/switchbot_api/commands.py#L294C30-L294C49).
But they're separate on the py-switchbot-api side, so I've left them distinct here too. Maybe we should unify that somehow?
|
I'll take a look at adding some tests later today. |
There was a problem hiding this comment.
Pull request overview
This PR adds support for SwitchBot Ceiling Light and Ceiling Light Pro devices to the SwitchBot Cloud integration. The implementation introduces a new entity class that supports color temperature control with brightness adjustment.
Key changes:
- Adds
SwitchBotCloudCeilingLightentity class with color temperature support (2700-6500K range) - Registers "Ceiling Light" and "Ceiling Light Pro" device types in the device discovery logic
- Implements device-specific commands using
CeilingLightCommandsfrom the switchbot_api library
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| homeassistant/components/switchbot_cloud/light.py | Adds SwitchBotCloudCeilingLight class with color temperature and brightness control, updates factory function to handle ceiling light device types |
| homeassistant/components/switchbot_cloud/init.py | Registers "Ceiling Light" and "Ceiling Light Pro" device types for light platform discovery |
| class SwitchBotCloudCeilingLight(SwitchBotCloudLight): | ||
| """Representation of SwitchBot Ceiling Light.""" | ||
|
|
||
| _attr_max_color_temp_kelvin = 6500 | ||
| _attr_min_color_temp_kelvin = 2700 | ||
|
|
||
| _attr_supported_color_modes = {ColorMode.COLOR_TEMP} | ||
|
|
||
| async def _send_brightness_command(self, brightness: int) -> None: | ||
| """Send a brightness command.""" | ||
| await self.send_api_command( | ||
| CeilingLightCommands.SET_BRIGHTNESS, | ||
| parameters=str(value_map_brightness(brightness)), | ||
| ) | ||
|
|
||
| async def _send_color_temperature_command(self, color_temp_kelvin: int) -> None: | ||
| """Send a color temperature command.""" | ||
| await self.send_api_command( | ||
| CeilingLightCommands.SET_COLOR_TEMPERATURE, | ||
| parameters=str(color_temp_kelvin), | ||
| ) | ||
|
|
There was a problem hiding this comment.
The new SwitchBotCloudCeilingLight class lacks test coverage. Tests should be added to verify:
- Turn on/off functionality
- Brightness control using
CeilingLightCommands.SET_BRIGHTNESS - Color temperature control using
CeilingLightCommands.SET_COLOR_TEMPERATURE - Supported color modes (only COLOR_TEMP)
- Temperature range enforcement (2700-6500K)
Other light types in this integration have comprehensive tests (see test_strip_light_turn_on, test_rgbww_light_turn_on). Similar tests should be added for ceiling lights.
|
|
||
|
|
There was a problem hiding this comment.
The base class async_turn_on method sets _attr_color_mode = ColorMode.RGB when adjusting brightness (line 88) or when turning on without parameters (line 97). However, SwitchBotCloudCeilingLight only supports ColorMode.COLOR_TEMP. This mismatch could cause the entity to report an incorrect color mode.
Consider overriding async_turn_on in SwitchBotCloudCeilingLight to set the correct color mode, or adjust the base class logic to check the supported color modes before setting the color mode.
| async def async_turn_on(self, **kwargs: Any) -> None: | |
| """Turn on the ceiling light with the correct color mode.""" | |
| # Only COLOR_TEMP is supported | |
| if (brightness := kwargs.get("brightness")) is not None: | |
| await self._send_brightness_command(brightness) | |
| if (color_temp := kwargs.get("color_temp_kelvin")) is not None: | |
| await self._send_color_temperature_command(color_temp) | |
| if not kwargs: | |
| await self.send_api_command(CommonCommands.TURN_ON) | |
| self._attr_is_on = True | |
| self._attr_color_mode = ColorMode.COLOR_TEMP | |
| await asyncio.sleep(AFTER_COMMAND_REFRESH) | |
| await self.coordinator.async_request_refresh() |
joostlek
left a comment
joostlek
left a comment
There was a problem hiding this comment.
Would it be possible to add tests to avoid regressions in the future?
|
Yes, sorry! Will do over the weekend, $Job got in the way in between. |
|
Added some tests (full-disclosure: written with help of AI; though I did manually review them and made sure they made sense); and fixed the bug that Copilot noticed above; I did notice HASS complaining about this in my local instance in practice: |
|
If/when this is approved, Draft PR for docs update: home-assistant/home-assistant.io#42694 |
| await self._send_color_temperature_command(color_temp_kelvin) | ||
| else: | ||
| self._attr_color_mode = ColorMode.RGB | ||
| self._attr_color_mode = self._get_default_color_mode() |
There was a problem hiding this comment.
The whole _get_default_color_mode() is here to preserve the existing logic of updating the ColorMode here; but I am not sure why it's here in the first place.
Would just not updating the _attr_color_mode here (and in the brightness is not None case?) at all would be a better move? I don't fullly understand HASS internals to understand what the implications of that are, but it feels weird to always update this, even when only touching the brightness?
Proposed change
Adds support for Ceiling Light devices for SwitchBot Cloud integration.
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: