Skip to content

Fix dual-protocol robustness gaps (#13) - #22

Merged
fboundy merged 1 commit into
mainfrom
patch
Jul 3, 2026
Merged

fboundy merged 1 commit into
mainfrom
patch

Conversation

@fboundy

@fboundy fboundy commented Jul 3, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes all seven checklist items from issue #13 (dual-protocol robustness gaps found in the full repo review):

  1. Recovery edits lost across protocol switch — the recovery path (DataRetrieval._recover_sensor_group_after_modbus_failure) called replace_sensor_group directly on the concrete Modbus controller, bypassing the hub entirely. DataRetrieval now takes an optional hub reference and routes recovery through it; the hub's replace_sensor_group now applies to both modbus and cloud.
  2. enable_connection/disable_connection only touched the active protocol — now applied to both controllers via the hub.
  3. Duplicate config entries sharing one inverter serial would collide — async_migrate_to_serial_ids now checks for an existing entry already using the target serial before migrating, instead of silently creating a collision.
  4. _health_tick probes the standby link uncoordinated — added a skip-if-busy guard so overlapping timer fires (e.g. a slow probe of a dead link) can't race the health-tracking state.
  5. Broad except swallowed at debug level — get_modbus_updates now logs unexpected poll errors at warning with a traceback.
  6. Optimistic entity updates regardless of write success — the number entity's set_native_value fired the write into an untracked task and updated state regardless of outcome; converted to async_set_native_value which awaits the write directly (matching the time entity's existing correct behavior).
  7. SolisBinaryEntity.handle_modbus_update never pushes state — added schedule_update_ha_state() calls.

Test plan

  • tests/test_inverter_hub.py, tests/test_register_group_recovery.py, tests/test_migration.py, tests/test_switch_conflicts_requires.py: new coverage for each fix
  • tests/test_number.py, tests/test_s16_encoding.py: updated for the sync→async rename
  • Full suite: 294 passed
  • ruff check / ruff format --check clean

Closes #13

Recovery edits now route through the hub so a bad-register-block
recovery lands on both protocol controllers instead of being lost
when the hub switches; enable_connection/disable_connection likewise
apply to both. Duplicate config entries migrating to the same
inverter serial are now caught and skipped instead of silently
colliding. Overlapping health-tick probes of a dead standby link are
now guarded against. Unexpected poll exceptions are logged at warning
instead of debug. The number entity's write path no longer fires a
fire-and-forget task and optimistically updates regardless of
success; it awaits the write like the time entity already did. The
binary/switch entity now pushes state immediately on a register
update instead of relying on polling.

Fixes #13
@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Failed to generate code suggestions for PR

@fboundy
fboundy merged commit 7356be7 into main Jul 3, 2026
6 checks passed
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.

Dual-protocol robustness gaps found in code review

1 participant