Skip to content

Surface control commands the cloud refuses as failed service calls - #171

Open
OdynBrouwer wants to merge 1 commit into
cdpuk:mainfrom
OdynBrouwer:fix/control-errors
Open

OdynBrouwer wants to merge 1 commit into
cdpuk:mainfrom
OdynBrouwer:fix/control-errors

Conversation

@OdynBrouwer

Copy link
Copy Markdown

Fixes #157

What's wrong

BackendApi's docstring for the semantic setters says "Raise NotImplementedError for a feature the device/backend combination doesn't support - never silently no-op."

Gizwits honoured the failure half of that: _do_control_post() raises when the API rejects a control post. The AWS IoT and SmartSpa backends didn't. Both built their writes on a set_device_state() that returns False on failure, and both threw that return value away:

  • aws_iot/api.py: set_device_state() returns False when the v2 command endpoint fails and the v1 fallback fails too, and set_power / set_filter / set_heat / set_locked / set_jets / set_target_temperature / set_bubbles all discarded it.
  • smartspa/api.py: same shape, same seven setters.

So a rejected command looked like success: the service call returned normally, the entity showed the optimistic value for the ~8 s optimistic timeout, then silently reverted, and the only trace was a WARNING-level log line the user is unlikely to see.

What this changes

  • backend.py: new BestwayApiException, the shared base for every backend's own exception type. The backends import no Home Assistant, so this is what the entity layer can catch without naming a specific backend.
  • AwsIotException, SmartSpaException and BestwayException now derive from it; Gizwits' behaviour is otherwise untouched.
  • Both V02 backends route their writes through a new _apply_control(), which raises when set_device_state() reports that the write didn't land. set_device_state() keeps its bool return, so the v2 -> v1 fallback inside it and any other internal caller are unaffected.
  • BestwayEntity.async_control() is the single place that turns that into a HomeAssistantError, and every entity action (switch on/off, select option, number value, climate hvac_mode / set_temperature) now goes through it. A refusal fails the service call with the backend's message, e.g. Device 'Hydrojet Pro' rejected the command: {'power_state': 1}.

What this can't catch

An accepted envelope is not proof of a write. The SmartSpa gateway answers 200/data:true for payloads it silently discards (unknown field, nonsensical value, empty payload) - that is documented in set_device_state() and unchanged here. This surfaces what each backend can actually detect, which is what the issue describes: auth expired, device offline, malformed payload, both endpoints failing.

Also

  • The optimistic value still clears on its own timeout rather than being rolled back immediately on failure. Rolling that back would mean touching seven call sites in four platform files for a cosmetic gain, so it's deliberately out of scope.
  • AGENTS.md documents the contract.

Tests

  • tests/test_aws_iot_api.py and tests/test_smartspa_api.py: a refusal raises for every setter, parametrised so a setter added later is covered too, plus a check that the message names the spa rather than its MAC.
  • tests/test_entity_wiring.py: switch, select, number and climate each fail the service call with HomeAssistantError instead of reporting success.

The semantic setters of the AWS IoT and SmartSpa backends threw away
set_device_state()'s bool return, so a rejected write looked like success: the
service call returned normally, the entity showed the optimistic value for the
~8 s optimistic timeout and then quietly reverted, and the only trace was a
WARNING-level log line. Gizwits already raised on a failed control post, so the
two V02 backends were the odd ones out - the BackendApi docstring promises
"never silently no-op".

Backends stay free of Home Assistant imports, so the entity layer cannot name
any single backend's exception type. A shared BestwayApiException in backend.py
is what it catches instead: each backend's own base exception (AwsIotException,
SmartSpaException, BestwayException) derives from it, and the semantic setters
send their write through _apply_control(), which raises when the write didn't
land. BestwayEntity.async_control() is then the single place that turns it into
a HomeAssistantError, which is what makes HA fail the service call and show the
backend's message instead of reporting success.

Worth knowing when reading this: an accepted envelope is still no proof of a
write. The SmartSpa gateway answers 200/data:true for payloads it silently
discards, so this catches what each backend can actually detect - the read-back
requirement is documented in set_device_state().

Tests: a refusal raises for every setter on both backends, the message names the
spa rather than its MAC, and every entity type (switch, select, number, climate)
fails the service call rather than reporting success.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Failed control commands are silent on AWS IoT and SmartSpa backends

1 participant