Switchbot Cloud: Fixed Circulator Fan on start error#165241
Switchbot Cloud: Fixed Circulator Fan on start error#165241XiaoLing-git wants to merge 6 commits intohome-assistant:devfrom
Conversation
|
Hey there @SeraphicRav, @laurence-presland, @Gigatrappeur, 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
Fixes a SwitchBot Cloud startup crash caused by treating the Circulator Fan device type as a sensor-capable device, which led to a KeyError during sensor platform setup.
Changes:
- Adjust device classification so
Circulator Fanis only set up as a fan (not as a sensor device). - Add/update test fixtures and fan tests to include both
Battery Circulator FanandCirculator Fandevice types.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
homeassistant/components/switchbot_cloud/__init__.py |
Stops adding Circulator Fan devices to the sensor setup list to avoid sensor-platform KeyError. |
tests/components/switchbot_cloud/__init__.py |
Introduces a separate fixture for Battery Circulator Fan vs Circulator Fan. |
tests/components/switchbot_cloud/test_fan.py |
Expands fan tests to cover both circulator fan device types. |
| entity_id = "fan.fan_1" | ||
| state = hass.states.get(entity_id) |
There was a problem hiding this comment.
The fan controller tests now exercise only the non-battery Circulator Fan path, and the Battery Circulator Fan command flow is no longer covered (beyond the coordinator-None case). Since this PR also changes fan command behavior, please add equivalent service-call tests for BATTERY_CIRCULATOR_FAN_INFO (and ideally assert the expected send_command arguments/sequence) to prevent regressions for the battery model.
| await self.send_api_command( | ||
| command=BatteryCirculatorFanCommands.SET_WIND_MODE, | ||
| parameters=str(BatteryCirculatorFanMode.DIRECT.value), | ||
| ) | ||
| await self.send_api_command( |
There was a problem hiding this comment.
Why is this being removed? (I'm not saying it shouldn't, but I would like to know the reasoning.)
There was a problem hiding this comment.
Can we parameterize the tests such that the battery powered fan is also tested?
| version="V1.0", | ||
| deviceId="battery-fan-id-1", | ||
| deviceName="battery-fan-1", | ||
| deviceType="Battery Circulator Fan", | ||
| hubDeviceId="test-hub-id", | ||
| ) | ||
|
|
||
| CIRCULATOR_FAN_INFO = Device( | ||
| version="V1.0", | ||
| deviceId="battery-fan-id-1", |
There was a problem hiding this comment.
the device id is the same as the battery-powerd fan
| deviceId="battery-fan-id-1", | |
| deviceId="fan-id-1", |
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
Proposed change
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: