fix: resolve entity unavailability, stale tokens, and unresponsive controls#109
Merged
cdpuk merged 13 commits intocdpuk:mainfrom Apr 12, 2026
Merged
Conversation
The Bestway/Gizwits cloud API frequently reports is_online as false even when the device is functioning normally and controllable via the official app. The API continues returning valid device state data regardless of this flag. This causes all spa entities to become permanently unavailable in Home Assistant despite the integration successfully polling data. Remove the is_online check from the base entity available property. The connectivity binary sensor continues to report the raw is_online value as a diagnostic indicator. Fixes cdpuk#89, cdpuk#93, cdpuk#100
Stored tokens expire server-side without any expiry field the integration can check. When a stale token is present, the integration skipped authentication and proceeded to refresh_bindings(), which failed with 'Token is not authorized', leaving all entities unavailable. Always request a fresh token on startup. Re-authenticating is a single POST request and avoids silent auth failures.
Switches now update the UI immediately on toggle, then schedule a non-blocking background refresh. Previously, toggles waited for a full API round-trip (2-10s) before reflecting the change, causing the UI to appear unresponsive or 'bounce' the toggle back. Changes: - switch.py: Add optimistic state tracking. UI updates instantly, cleared when real data arrives from coordinator poll. - climate.py: Replace blocking async_refresh() with non-blocking async_request_refresh() for hvac mode and temperature changes. - select.py: Add async_request_refresh() after bubbles selection (was missing entirely).
When the API returns a partial state (e.g., after a timeout or during startup before the first full poll), the Tunit key may be missing from attrs. Using dict indexing causes a KeyError that spams the logs. Use .get() to safely default to Celsius.
Cover the 4 fixes with unit tests: - Entity available when is_online=False (the core fix) - Entity unavailable when device missing or coordinator fails - Switch optimistic state tracking (_assumed_state, _optimistic_state) - Switch optimistic state cleared on coordinator update - Climate Tunit .get() safety with missing/present/zero values Tests follow existing repo patterns and require the same pytest-homeassistant-custom-component framework (CI only, not Windows).
_handle_coordinator_update calls async_write_ha_state which requires a real HA instance. Patch it in the unit test since we only need to verify the optimistic state is cleared.
test_entity_fixes.py -> test_availability_and_controls.py
Contributor
Author
|
@cdpuk, could you please review? |
Owner
|
Thanks for the collection of fixes - I don't have a V2 device so very much appreciate community contributions. I'll give this a quick check over the next few days. Could you just take a look at why the |
- Remove duplicate __init__ in switch.py (ruff F811) - Remove unused imports in tests (ruff F401) - Apply ruff formatting to entity.py, __init__.py, tests
test_config_flow: The bypass_setup_fixture patched async_setup_entry but not async_unload_entry, causing KeyError during teardown. Added unload patch. The remaining xfail is a daemon thread from asyncio internals (_run_safe_shutdown_loop) that the test framework rejects - pre-existing on main, not caused by our changes. __init__.py: Use .get() in async_unload_entry to handle the case where hass.data[DOMAIN] was never populated (e.g., failed setup). test.yaml: Fix CI workflow - branch was 'master' (repo uses 'main'), and coverage target was 'givenergy_local' (copy-paste from another project).
.get('Tunit') returned None (falsy) when the key was absent, falling
through to Fahrenheit. Use .get('Tunit', 1) so missing key defaults to
Celsius, matching the no-status fallback behavior.
Rewrote Tunit tests to exercise the actual AirjetV01HydrojetSpaThermostat
entity instead of duplicating the if/else logic inline.
The upstream verify_cleanup fixture races with a daemon thread spawned by shutdown_default_executor(). Override it in test_config_flow.py to join lingering threads before teardown assertions. Removes the xfail marker - test_successful_config_flow now passes deterministically. 78 passed, 0 xfailed.
Contributor
Author
|
@cdpuk no worries. I added a few more fixes and updated the PR description. The precommit check is now passing. |
Collapse multi-line constructor calls that fit on one line, matching ruff v0.15.1 from pre-commit config.
The event_loop fixture was removed in newer pytest-asyncio versions used by CI. Rewrite verify_cleanup override to take no parameters - just track threads before/after yield and join any daemon threads (like _run_safe_shutdown_loop) before the upstream assertion runs.
cdpuk
approved these changes
Apr 12, 2026
This was referenced Apr 12, 2026
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.
Summary
Four code fixes for V02 (AWS IoT) device support, plus repo maintenance fixes for tests and CI. Addresses persistent entity unavailability, authentication failures, sluggish dashboard controls, and log spam from missing keys.
Files changed: 8 | +365 -34 | 78 tests passing
Code fixes
1. Ignore unreliable
is_onlineflag (entity.py)The Bestway/Gizwits cloud API frequently reports
is_online: falseeven when the device is functioning normally and controllable via the official app. The API continues returning valid device state data regardless of this flag.The base
BestwayEntity.availableproperty gated all entities onself.bestway_device.is_online, causing all spa entities to become permanently unavailable. The connectivity binary sensor continues to report the rawis_onlinevalue as a diagnostic indicator.Fixes #89, #93, #100
2. Always re-authenticate on startup for AWS IoT (
__init__.py)Stored tokens expire server-side without any expiry field the integration can check. When a stale token existed, the integration skipped authentication and proceeded to
refresh_bindings(), which failed with "Token is not authorized", leaving all entities unavailable until the next HA restart.Now always requests a fresh token on startup. Re-authenticating is a single POST request (cheap).
Related: #86
3. Optimistic state updates for responsive controls (
switch.py,climate.py,select.py)Switch toggles waited for a full API round-trip (2-10s) before reflecting the state change in the UI. If the post-command refresh failed (e.g., temporary API timeout), the UI reverted the toggle, requiring the user to toggle twice.
async_request_refresh()(non-blocking, debounced) instead ofasync_refresh()(blocking).4. Safe attribute access for
Tunitkey (climate.py)When the API returns a partial state (e.g., after a timeout or during startup before the first full poll), the
Tunitkey may be missing fromattrs. Using dict indexing caused aKeyErrorthat spammed the logs (~200 occurrences over 4 days). Changed to.get("Tunit", 1)to safely default to Celsius (matching the no-status fallback behavior).Repo maintenance fixes
5. Defensive
async_unload_entry(__init__.py)Use
.get(DOMAIN, {}).pop(entry.entry_id, None)to handle the case wherehass.data[DOMAIN]was never populated (e.g., failed setup). PreventsKeyErrorduring unload of partially-initialized entries.6. Config flow test teardown (
test_config_flow.py)async_setup_entryandasync_unload_entryin thebypass_setup_fixtureto prevent teardownKeyErrorwhen HA manages entries that were never fully initialized.verify_cleanupfixture override that joins daemon threads before the upstream thread assertion runs. The upstreamverify_cleanupraces with_run_safe_shutdown_loopfrom asyncio'sshutdown_default_executor().7. CI workflow fixes (
.github/workflows/test.yaml)Fixed two copy-paste bugs from the
givenergy_localproject:branches: [master]->branches: [main](push-triggered CI never ran)--cov custom_components.givenergy_local->--cov custom_components.bestway(coverage measured wrong package)Testing
Tested on a V02 Airjet spa (UltraFit, product_id T53NN8) over 5 days (April 5-10):
is_onlinevalueTest suite: 78 passed, 0 errors
11 regression tests in
test_availability_and_controls.pyexercising the actual entity classes:is_online=True,is_online=False, missing device, coordinator failure_assumed_stateflag, optimistic turn on, state cleared on coordinator updateCompatibility
availableproperty change applies to the shared base class, and climate/switch changes use the same patterns for both backends.