Surface control commands the cloud refuses as failed service calls - #171
Open
OdynBrouwer wants to merge 1 commit into
Open
OdynBrouwer wants to merge 1 commit into
OdynBrouwer wants to merge 1 commit into
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #157
What's wrong
BackendApi's docstring for the semantic setters says "RaiseNotImplementedErrorfor 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 aset_device_state()that returnsFalseon failure, and both threw that return value away:aws_iot/api.py:set_device_state()returnsFalsewhen the v2 command endpoint fails and the v1 fallback fails too, andset_power/set_filter/set_heat/set_locked/set_jets/set_target_temperature/set_bubblesall 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: newBestwayApiException, 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,SmartSpaExceptionandBestwayExceptionnow derive from it; Gizwits' behaviour is otherwise untouched._apply_control(), which raises whenset_device_state()reports that the write didn't land.set_device_state()keeps itsboolreturn, 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 aHomeAssistantError, 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:truefor payloads it silently discards (unknown field, nonsensical value, empty payload) - that is documented inset_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
AGENTS.mddocuments the contract.Tests
tests/test_aws_iot_api.pyandtests/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 withHomeAssistantErrorinstead of reporting success.