Add classic-auth AC charge/discharge time writes for Growatt Mix#176603
Open
albertteoh wants to merge 9 commits into
Open
Add classic-auth AC charge/discharge time writes for Growatt Mix#176603albertteoh wants to merge 9 commits into
albertteoh wants to merge 9 commits into
Conversation
Support writing AC charge and discharge time periods on classic
(username/password) auth Mix devices via the mixSet endpoint, alongside
the existing V1 (token) SPH path.
- coordinator: encode charge/discharge periods as the positional
param1-18 payload for update_mix_inverter_setting, surfacing library
errors and non-success responses as HomeAssistantError
- services: write handlers now match {"sph", "mix"} and branch on
api_version; classic auth falls back to safe defaults (all periods
off) since it has no verified live read path to merge against
- services: read handlers also match {"sph", "mix"} so classic devices
report token_auth_required instead of no_devices_configured
- tests: add coordinator-level unit tests pinning the param encoding and
error paths, plus service-layer classic-auth write/read tests
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
home-assistant
Bot
added
cla-needed
has-tests
integration: growatt_server
new-feature
labels
![home-assistant[bot]](https://avatars.githubusercontent.com/in/97978?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hi @albertteoh
It seems you haven't yet signed a CLA. Please do so here.
Once you do that we will be able to review and accept this pull request.
Thanks!
Contributor
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
Contributor
|
Hey there @johanzander, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
home-assistant
Bot
added
cla-recheck
cla-signed
and removed
cla-recheck
cla-needed
labels
Contributor
There was a problem hiding this comment.
Pull request overview
Adds classic-auth Growatt Mix AC charge/discharge schedule writes alongside existing V1 SPH support.
Changes:
- Adds classic Mix payload encoding and API calls.
- Expands service device matching and classic defaults.
- Adds coordinator and service tests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
coordinator.py |
Implements classic schedule writes. |
services.py |
Routes Mix devices and handles defaults. |
conftest.py |
Mocks classic write responses. |
test_coordinator.py |
Tests encoding and errors. |
test_services.py |
Tests classic service behavior. |
- Add a classic-auth read path for AC charge/discharge times, parsed from the coordinator's already-cached mix_detail data instead of falling back to fixed defaults on write. Write handlers now merge with real cached state for both auth types, matching V1 behaviour. - Restrict service device lookup to explicit (device_type, api_version) pairs (sph+v1, mix+classic) so a write can never be routed to the wrong device-specific endpoint. - Require an explicit success=True in classic API responses instead of defaulting missing/malformed responses to success. - Catch RequestException/JSONDecodeError around classic mixSet calls, not just GrowattError, so transport failures surface as HomeAssistantError instead of crashing the write. - Fix the classic discharge-write path to update the coordinator cache on success, matching the charge-write path. - Make the no_devices_configured message auth-neutral and update the write/read service descriptions to mention Mix devices. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
albertteoh
force-pushed
the
growatt-classic-auth-writes
branch
from
1e22130 to
6787520
Compare
Author
|
Pushed a follow-up commit addressing the Copilot review feedback:
Replied inline on each thread with details. One item I left open: the suggestion to move the All tests, ruff, mypy, and pylint pass locally (164 tests in |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
homeassistant/components/growatt_server/coordinator.py:624
- Move the classic Mix protocol implementation into
growattServerand call a library helper here. This positional endpoint encoding is device protocol logic and duplicates the library's SPH implementation; Home Assistant integrations should remain thin wrappers rather than carry temporary library workarounds.
return "00:00"
else:
return f"{hour:02d}:{minute:02d}"
The classic mix_detail() poll only returns telemetry (getEnergyProdAndCons_KW) and never included the AC charge/discharge schedule fields the previous read-path commit assumed were there - it only appeared to work because those fields were fabricated directly into the test mock. Real settings come from a separate endpoint, get_mix_inverter_settings() (getMixSetParams), fetched on demand and nested under obj.mixBean. Verified the response shape and field mapping against a real Mix inverter: most fields (forcedChargeTime*/forcedDischargeTime*/chargePowerCommand/ acChargeEnable/etc.) match 1:1, but the stop-SOC settings have two variants each (wchargeSOCLowLimit1/2, wdisChargeSOCLowLimit1/2); the "2" suffix is the one the app displays for the AC charge/discharge schedule. Also fixes a mistested classic write test that called update_ac_discharge_times under a test named for the charge path (leaving the charge path's GrowattError handling untested), and adds type annotations to test fixture parameters. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author
|
Pushed a follow-up fixing a real bug this review round caught: my previous commit's classic-auth read path assumed Fixed by fetching
Also fixed a mistested classic write test (named for the charge path but calling the discharge method, leaving charge's All 167 tests, ruff, mypy, and pylint pass locally. |
The classic write path builds a fixed-length positional param list from the caller's periods list with no length check, so a caller supplying other than 3 periods would silently misalign every param after it. Raise ValueError up front instead, matching the V1 SPH library's own validation for the same constraint. Also corrected the discharge path's comment, which claimed param1-18 (that's the charge path's count; the discharge payload is 2 header params + 15 period params = param1-17). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
get_mix_inverter_settings() failure responses (e.g. {"success": false})
have no obj.mixBean, which previously fell through to {} and was read
as all-zero/disabled settings. Combined with the write handlers'
read-merge-write pattern, a failed settings fetch could silently merge
in 0/100/disabled defaults and overwrite every field the caller didn't
specify, instead of aborting the write.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Why
Temporary workaround to #166817 until the token-based requests are fixed.
With token-based auth:
growatt_server.read_ac_charge_timesworkedgrowatt_server.write_ac_charge_timesdidn't work and returned the error:Growatt API error: Error during writing AC charge time periodsas noted in the issue API Error with growatt_server.write_ac_charge_times in 2026.4 beta for Growatt Server #166817.I used mitmproxy to see why my device (running ShinePhone) was able to update the ac charge times, and discovered it was using classic username/password auth.
I then tried to use classic username/password auth with Growatt Server on home_assistant, but when I tried
growatt_server.write_ac_charge_times, I got the error: "No SPH devices with token authentication are configured. Actions require SPH devices with V1 API access."This change allows for both
growatt_server.write_ac_charge_timesandgrowatt_server.read_ac_charge_times(and their discharge equivalents) to work with classic username/password auth on Mix devices. Reads are backed by the classicgetMixSetParamsendpoint (get_mix_inverter_settings) rather than the regular telemetry poll, since that poll doesn't include these settings — verified against a real Mix inverter.Breaking change
Proposed change
Adds support for reading and writing AC charge and discharge time periods on classic (username/password) auth Growatt Mix devices, alongside the existing V1 (token) SPH read/write path.
coordinator: encodes charge/discharge periods as the positionalparam1-18payload forupdate_mix_inverter_setting(classic writes), surfacing library errors, transport failures, and non-success responses asHomeAssistantError.coordinator: classic reads fetchget_mix_inverter_settings(getMixSetParams) on demand and parse the sameforcedChargeTime*/forcedDischargeTime*/chargePowerCommand/acChargeEnable/etc. fields the V1 SPH path already uses, plus the classic-specificwchargeSOCLowLimit2/wdisChargeSOCLowLimit2stop-SOC fields (confirmed against a real device — the classic API exposes two SOC-limit variants per direction;2is the one the app displays for the AC schedule).services: write handlers now match{("sph", "v1"), ("mix", "classic")}explicitly (rather than a flat device-type set) so a write can never be routed to the wrong device-specific endpoint, and always read-merge-write against real cached/fetched state for both auth types — a partial write no longer resets unspecified fields to fixed defaults.services: read handlers match the same explicit pairs and now return real classic Mix settings instead of raisingtoken_auth_required.services/strings:no_devices_configuredis now auth-neutral, and the write/read service descriptions mention Mix devices.tests: coordinator-level unit tests for the classic read/write param encoding and error paths (including transport failures and malformed responses), plus service-layer classic-auth read/write tests.Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: