Huawei: add HoldCharge

[CiNcH83]

SUN2000/SDongle Modbus Primer

Testing

States

• normal: battery charged with surplus only / discharged to home ... $${\color{lightgreen}OK}$$
• hold: only discharging locked / charging with surplus still possible ... $${\color{lightgreen}OK}$$
• charge: force-charge at maxchargepower from surplus/grid combined ... $${\color{lightgreen}OK}$$
• holdcharge: only charging locked / discharging still possible ... $${\color{lightgreen}OK}$$

State Transitions

while true; do curl -X 'POST' 'http://localhost:7070/api/batterymode/hold' -H 'accept: application/json' -d ''; sleep 30; done
while true; do curl -X 'POST' 'http://localhost:7070/api/batterymode/charge' -H 'accept: application/json' -d ''; sleep 30; done
while true; do curl -X 'POST' 'http://localhost:7070/api/batterymode/holdcharge' -H 'accept: application/json' -d ''; sleep 30; done
curl -X 'POST' 'http://localhost:7070/api/batterymode/normal' -H 'accept: application/json' -d ''

• normal → hold
• normal → charge
• normal → holdcharge
• hold → normal
• hold → charge
• hold → holdcharge
• holdcharge → normal
• holdcharge → hold
• holdcharge → charge
• charge → normal
• charge → hold
• charge → holdcharge

Register states are being checked with modbus-cli via evcc Modbus Proxy:

modbus 192.168.1.8:5021 -s 1 h@47075/I # max. charge power
modbus 192.168.1.8:5021 -s 1 h@47077/I # max. discharge power
modbus 192.168.1.8:5021 -s 1 h@47087/h # charge from grid enable
modbus 192.168.1.8:5021 -s 1 h@47100/h # force charge/discharge function
modbus 192.168.1.8:5021 -s 1 h@47246/h # force charge/discharge setting mode
modbus 192.168.1.8:5021 -s 1 h@47083/h # force charge/discharge period
modbus 192.168.1.8:5021 -s 1 h@47247/I # force charge power

Please beware that when checking states with FusionSolar portal, displayed settings might not reflect register settings immediately! Fetching the whole configuration via the portal also puts quite a lot of load on the SUN2000 which may cause Modbus errors for evcc.

Remarks

Old configs may still have the old default of 10000 for maxchargepower set which is higher than what most installed battery setups support (only the LUNA2000-S1 with 3 battery packs supports a higher charge power). While the Forcible charge power register (47247) allows out-of-bounds values to be set, trying to set it for the Maximum charging power register (47075) introduced in this PR fails. The introduced clamping mechanism takes care of that. So this is not a breaking change. Broken configs will stay broken though.

It would be great if Huawei had something similar to the SunSpec StorCtl_Mod register. With Huawei, every mode requires quite some registers to be written for cleaning up every potential previous mode/state as the number of modes increases. Implementing all modes (force-charge, force-discharge, lock, lock-charge, lock-discharge) via register 47100, also being guarded by a HW watchdog, would indeed be superb. It unfortunately only supports force-charge and force-discharge today.

Out of scope for this PR

• Race-condition when leaving charge which an expiring watchdog took care of [LINK]
  Fixed via Watchdog: don't re-assert stale value after reset #31039

[sourcery-ai]

Hey - I've left some high level feedback:

• The new holdcharge case duplicates the maxchargepower write logic that was added to the other cases; consider extracting a shared sequence or helper template to avoid repeating the same 47075/47077 write blocks in multiple branches.
• In the holdcharge sequence you explicitly zero 47075 while keeping 47077 at maxdischargepower; if this asymmetry is intentional, a brief inline comment explaining why discharge is left at max while charge is zeroed would make the behavior clearer to future maintainers.
• The watchdog handler’s new case: 4 uses a sleep with duration: 0s; if this case does not need any special handling, you can omit the branch entirely rather than adding a no-op step.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The new `holdcharge` case duplicates the `maxchargepower` write logic that was added to the other cases; consider extracting a shared sequence or helper template to avoid repeating the same `47075`/`47077` write blocks in multiple branches.
- In the `holdcharge` sequence you explicitly zero `47075` while keeping `47077` at `maxdischargepower`; if this asymmetry is intentional, a brief inline comment explaining why discharge is left at max while charge is zeroed would make the behavior clearer to future maintainers.
- The `watchdog` handler’s new `case: 4` uses a `sleep` with `duration: 0s`; if this case does not need any special handling, you can omit the branch entirely rather than adding a no-op step.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[CiNcH83]

Looks to be working. But I want to do some more stability testing, also see #30467.

[CiNcH83]

@andig

I am again experiencing a (reproducible) race-condition when switching from charge to hold. (or in fact when leaving charge). hold sets 47100 to 0. Then there seems to be another charge watchdog cycle which sets it to 1 again.

Splitting modes in switch and watchdog does not seem to be a good idea?

[andig]

Sounds as if the watchdog reset happens too late? It would be helpful if we could have a demo config that allows to reproduce this without having a Huawei inverter.

[andig]

Fix options, roughly in order of preference:

1. Harden the watchdog plugin (needed regardless): re-check ctx.Err() inside the mutex-protected tick callback, and ideally make stopWdt synchronous (wait for goroutine exit). This kills the stale-tick-after-cancel race, but cannot fix the sequence-level race since the switch writes never pass through the watchdog's mutex.
2. Give the watchdog two actions — e.g. set: (executed once on value change, under the mutex, after the old timer is stopped) plus a separate refresh:/keep-alive config (executed per tick). The template then becomes a single watchdog { set: , refresh: <47100-only switch> }, the sequence disappears, and both write paths are serialized correctly. This is the principled fix matching what the template is trying to express.
3. Template-only workaround: move the whole mode switch inside the watchdog's set. Correctly serialized and self-healing, but rewrites all charge registers every 15s — probably tolerable on modbus TCP, ugly nonetheless, and it still needs fix 1.

Imho 3 would be easier to understand unless this kills any eeprom register?

[CiNcH83]

Imho 3 would be easier to understand unless this kills any eeprom register?

Hope not. I am just trying to only do as much as necessary to keep the burden on those devices as low as possible...

[CiNcH83]

BTW... So far I haven't realized that error because as soon as the 1 minute watchdog expires, the forcible charging is disabled anyway. I just realized now because I am actively checking the register states after a mode switch...

[CiNcH83]

I measured response times when writing the respective registers and they indeed seem costly, between 500ms and multiple seconds(!) per register.

[CiNcH83]

So how do you want to proceed here? I think it makes a ton of sense to have a setup and a periodic phase for a state. Plus I want to keep the numbers of registers written low as writes are indeed expensive for Huawei.

I will finish testing holdcharge over the weekend. The race-condition when leaving charge is also in the master/release. But after the 1m watchdog expires, the forced charge is stopped anyway.

[andig]

Up to you. Accept the broken config or do this.

[CiNcH83]

Finished all testing now (test plan see initial posting). Watchdog race-condition looks to be fixed via #31039.

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• The go blocks that clamp maxchargepower to the rated max (register 37046) are repeated three times; consider extracting this into a reusable template/include or YAML anchor to reduce duplication and keep future changes consistent.
• The case values for the different modes (including the new 4 // holdcharge) are hard-coded magic numbers; adding a centralized mapping or constants for these mode IDs would make the intent clearer and reduce the risk of mismatches with the API/batterymode layer.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The `go` blocks that clamp `maxchargepower` to the rated max (register 37046) are repeated three times; consider extracting this into a reusable template/include or YAML anchor to reduce duplication and keep future changes consistent.
- The `case` values for the different modes (including the new `4 // holdcharge`) are hard-coded magic numbers; adding a centralized mapping or constants for these mode IDs would make the intent clearer and reduce the risk of mismatches with the API/batterymode layer.

## Individual Comments

### Comment 1
<location path="templates/definition/meter/huawei-sun2000-hybrid.yaml" line_range="352-355" />
<code_context>
+                        address: 47075 # Max. Charge Power
+                        type: writemultiple
+                        encoding: uint32
+                script: |
+                  limit := {{ .maxchargepower }}
+                  out := rated
+                  if limit < rated { out = limit }
+                  out
               # register 47077 is range-checked: a maxdischargepower above the
</code_context>
<issue_to_address>
**issue (bug_risk):** Consider guarding against negative maxchargepower when writing to a uint32 register

`limit` is an `int` but the register is `uint32`. If `maxchargepower` is ever negative (e.g. via misconfiguration), the value will be encoded as an invalid `uint32` (wrap/rejection risk). Please clamp the lower bound explicitly, e.g.:

```go
limit := {{ .maxchargepower }}
if limit < 0 { limit = 0 }
out := rated
if limit < rated { out = limit }
out
```

The same lower-bound clamp is needed for the corresponding discharge-power blocks that write to `uint32` registers.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

issue (bug_risk): Consider guarding against negative maxchargepower when writing to a uint32 register

limit is an int but the register is uint32. If maxchargepower is ever negative (e.g. via misconfiguration), the value will be encoded as an invalid uint32 (wrap/rejection risk). Please clamp the lower bound explicitly, e.g.:

limit := {{ .maxchargepower }}
if limit < 0 { limit = 0 }
out := rated
if limit < rated { out = limit }
out

The same lower-bound clamp is needed for the corresponding discharge-power blocks that write to uint32 registers.