Skip to content

Rebuild on current upstream: drop redundant SPH/throttle, keep shared-session re-login - #34

Closed
johanzander wants to merge 1 commit into
masterfrom
chore/rebuild-on-vendor-sync
Closed

Rebuild on current upstream: drop redundant SPH/throttle, keep shared-session re-login#34
johanzander wants to merge 1 commit into
masterfrom
chore/rebuild-on-vendor-sync

Conversation

@johanzander

Copy link
Copy Markdown
Owner

Summary

master had drifted behind current upstream home-assistant/core in ways it shouldn't have — this rebuilds on vendor-sync (a clean current snapshot of upstream's growatt_server component) and re-applies only what's genuinely still needed.

Dropped (now redundant with upstream):

  • SPH device support (sph.py + wiring in coordinator/services/sensor) — duplicated your own home-assistant/core#165314, merged 2026-03-16. Upstream's canonical version is used instead.
  • throttle.py's ApiThrottleManager — verified it was never actually wired into the setup flow (_handle_throttled_setup had zero call sites, ApiThrottleManager was never instantiated anywhere). Dead code with its own test file. Dropped, along with tests/test_throttle.py and the now-unused mock_throttle_manager autouse fixture / no_throttle_mock marker.
  • Everything else — took upstream's more current version wholesale (translation_key-based exceptions, the periodic new/removed-device auto-discovery scanner, general refactors, diagnostics.py).

Kept, because it's real and still load-bearing:

  • Every GrowattCoordinator for a config entry now shares one authenticated API instance via config_entry.runtime_data.api, instead of each coordinator creating and logging into its own. Upstream's coordinator currently calls api.login() on every single update cycle for Classic API — with N devices that's N independent logins every scan interval, which is very likely the actual root cause of the rate-limiting problems this integration has fought historically.
  • GrowattCoordinator._async_re_login: reactive re-authentication on session expiry (detected via JSONDecodeError — the server returns an HTML login page instead of JSON), guarded by a shared asyncio.Lock + 60s cooldown (runtime_data.login_lock / last_login_time) so concurrent coordinators don't all re-login at once.

Also fixed: two except ValueError, TypeError:-style syntax errors (invalid Python 3 syntax) that are present in upstream home-assistant/core's dev branch itself right now — found because the file wouldn't compile otherwise. Worth reporting upstream separately.

Test plan

  • ruff check passes clean on every changed file.
  • python -m py_compile passes clean on every changed file (also how the two upstream syntax errors were caught).
  • Not run: the pytest-homeassistant-custom-component suite. This dev environment's package index caps at homeassistant==2025.1.4, which predates this component's current structure (no AUTH_API_TOKEN, etc.) — confirmed this is a pre-existing environment limitation, not something introduced here, by running unmodified master against the same environment and getting an identical import failure. Please run the real test suite locally before merging.

…-session re-login

master had drifted behind upstream home-assistant/core's growatt_server
component in several ways it shouldn't have:

- SPH device support (sph.py, coordinator/service wiring) duplicated what's
  now upstream's own canonical implementation — merged via PR #165314
  (home-assistant/core#165314), authored by @johanzander. Dropped in favor
  of upstream's version.
- throttle.py's ApiThrottleManager (disk-persisted rate-limit guard) was
  never actually wired into the setup flow — dead code with its own test
  file but zero call sites. Dropped. Upstream's own ConfigEntryNotReady-based
  handling for V1-API rate-limit errors covers that path instead.
- Everything else (translation_key-based exceptions, the periodic
  new/removed-device auto-discovery scanner, general refactors) — took
  upstream's more current version wholesale.

What's kept, because it's real and still needed:
- Every GrowattCoordinator for a config entry now shares ONE authenticated
  API instance via config_entry.runtime_data.api, instead of each
  coordinator creating and logging into its own. Upstream's coordinator logs
  in on every single update cycle for Classic API — with N devices that's
  N independent logins every scan interval, which is very likely the actual
  root cause of the rate-limiting this integration has fought historically.
- GrowattCoordinator._async_re_login: reactive re-authentication on session
  expiry (a JSONDecodeError — the server returns an HTML login page instead
  of JSON), guarded by a shared asyncio.Lock + 60s cooldown
  (runtime_data.login_lock / last_login_time) so concurrent coordinators
  don't all re-login at once.

Also fixes two `except ValueError, TypeError:`-style syntax errors
(invalid Python 3 syntax) present in the current upstream source itself —
found because reconstructing on top of it wouldn't otherwise compile.

Not verified by the test suite: this environment's package index caps at
homeassistant==2025.1.4, which predates this component's current structure
(AUTH_API_TOKEN and more), so pytest-homeassistant-custom-component can't
actually run here — confirmed pre-existing by running unmodified master
against the same environment with identical failure. Verified instead via
ruff (clean) and py_compile (clean) on every changed file.
@johanzander

Copy link
Copy Markdown
Owner Author

Superseded by a cleaner split: #35

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.

1 participant