Create the AC Charge Start/End Battery SOC entities on grid-tied families

[notexpected]

Summary

The reg-160/161 AC Charge SOC window entities (ACChargeStartBatterySOCNumber /
ACChargeEndBatterySOCNumber, added for EG4_OFFGRID in #331) are also live —
and currently invisible — on grid-tied families. This PR creates them in the
grid-tied setup branch too: Start enabled by default, End created
disabled by default pending a verified grid-tied write (the PR #332 review
noted reg-161 writes observed rejected on grid-tied hardware).

Hardware evidence (FlexBOSS21, EG4_HYBRID, fw FAAB-2727, local WiFi dongle)

Register 160 initiates AC charging whenever battery SOC is below its value,
regardless of the reg-120 ACChargeType selector and of the AC-charge time
windows. Observed over three consecutive nights with ACChargeType = "according to time" (reg 120 low bits 0x52) and AC Charge enabled (reg 21
bit 7):

• reg 160 = 90 (factory): 6 kW grid charges started at exactly SOC 90,
  outside every configured window (battery oscillated 90↔100 overnight), and
  an in-progress forced discharge was overridden at SOC 90. Net effect: the
  battery was pinned high around the clock and every ToU schedule silently
  defeated.
• reg 160 = 10: no grid charge started inside a valid enabled window
  (01:00–03:00) with the battery at SOC 19 — 19 > 10, so the trigger never
  fired. The official 18KPV Modbus protocol says regs 160/161 are "valid after
  selecting ACChg according to SOC"; this firmware applies the start-SOC gate
  in time mode as well.
• reg 160 local dongle read and write verified on this unit (90 → 10 with
  read-back); reg 161 reads correctly (100), write untested here.

The EG4 portal/app exposes the same field for this family as "Start AC
Charge SOC(%)", so users can set it (or ship with the factory 90) while
Home Assistant has no way to see why their charge schedule isn't honored.
That makes the entity arguably more important on grid-tied units than on
EG4_OFFGRID, where it's at least the family's primary, documented control.

Changes

• number.py: grid-tied/fail-open setup branch also creates
  ACChargeStartBatterySOCNumber (enabled) and ACChargeEndBatterySOCNumber
  (entity_registry_enabled_default = False on this branch); class docstrings
  updated from "EG4_OFFGRID only" to both families with the evidence above.
  EG4_OFFGRID behavior unchanged.
• tests/test_number_entities.py: grid-tied setup test now expects 25
  entities with the pair present (Start enabled-default, End disabled-default
  asserted); fail-open test updated likewise (23 entities).
• CHANGELOG.md: Unreleased entry.

Testing

• pytest tests/test_number_entities.py passes.
• ruff check / ruff format --check clean on the touched files (no new
  findings vs main).
• Live validation pending on the reporting FlexBOSS21 once released — the same
  unit that produced the register evidence.

Claude AI

Note, this code was generated by Claude Code and I manually reviewed the cod prior to sending PR.

[btli]

Code review

Not ready to merge. Adversarial review (gpt-5.6-sol at ultra) found six issues; I verified the two blocking ones directly against the source. The register evidence in the PR body is solid and the premise is right — I independently confirmed on a second FlexBOSS21 through the cloud path that both parameters exist and read (HOLD_AC_CHARGE_START_BATTERY_SOC = 5, HOLD_AC_CHARGE_END_BATTERY_SOC = 0, alongside reg 67 = 80). The problems are in how the entities are wired up, not in whether the registers are real.

1. On the very setup that produced the evidence, the new entity can never show a value. Local polling only widens the 158 read to cover 160/161 on EG4_OFFGRID:

eg4_web_monitor/custom_components/eg4_web_monitor/coordinator_local.py

Lines 605 to 616 in 92c22c9

	*ac_first_range, # (152, 6) on EG4_OFFGRID only (GH #295)
	# AC charge start/stop voltage (158-159); on EG4_OFFGRID the
	# read widens to cover the AC-charge SOC window (160-161,
	# GH #331) consumed by the family's AC Charge Start/End
	# Battery SOC numbers — same widen-don't-split rationale as
	# (64, 26): one Modbus read either way. Both registers
	# surface under pylxpweb's name-map keys
	# (HOLD_AC_CHARGE_{START,END}_BATTERY_SOC; reg 161 named
	# from 0.9.36b28 — older releases emit the raw "161"
	# fallback key, which nothing consumes until the pin bump).
	(158, 4 if is_offgrid else 2),
	(169, 1), # On-grid end-of-discharge voltage (HOLD_ONGRID_EOD_VOLTAGE)

Both entities read exclusively from the parameter cache with no inverter-property fallback, so a pure-LOCAL grid-tied FlexBOSS21 — the dongle-connected unit in the PR body — gets an enabled Start entity stuck at unknown. That is precisely the "user can't see why their schedule isn't honored" problem the PR exists to solve. The gate needs to widen for the families that now create the entity. test_coordinator_local.py:395 currently pins the old exclusion for EG4_HYBRID, so it will need updating too.

2. Using the else branch as a positive capability gate inverts the project's convention. This is documented in the predicates themselves:

eg4_web_monitor/custom_components/eg4_web_monitor/utils.py

Lines 267 to 287 in 92c22c9

	def is_offgrid_family(device_data: dict[str, Any]) -> bool:
	"""Return True when a device is positively identified as EG4_OFFGRID.
	Fails open (False) when features are missing or the family is unknown, so
	family-based suppression never removes entities from devices that were
	not positively identified as 12000XP/6000XP-class hardware.
	"""
	features = device_data.get("features") or {}
	return bool(features.get("inverter_family") == INVERTER_FAMILY_EG4_OFFGRID)
	def is_hybrid_family(device_data: dict[str, Any]) -> bool:
	"""Return True when a device is positively identified as EG4_HYBRID.
	Fails closed (False) when features are missing or the family is unknown —
	the Generator/Off-Grid/Peak Shaving schedules were verified on EG4_HYBRID
	hardware and are only created there (plus EG4_OFFGRID for Generator).
	"""
	features = device_data.get("features") or {}
	return bool(features.get("inverter_family") == INVERTER_FAMILY_EG4_HYBRID)

is_offgrid_family() fails open specifically so that family-based suppression never strips entities from unidentified hardware. is_hybrid_family() fails closed and is what positively-verified-on-hybrid-hardware capabilities use — time.py:83 applies exactly that rule for the schedule families, and test_time_entities.py:418 explicitly excludes LXP and unknown families when the evidence is hybrid-only. The if offgrid: … else: here lumps EG4_HYBRID, LXP and unrecognized hardware together on one FlexBOSS21's evidence. is_hybrid_family() is the convention-consistent gate.

Non-blocking, but worth addressing:

3. Start's range disagrees with the library. The entity permits 0–100%, while pylxpweb's register-160 definition and its hybrid setter cap Start at 90% (inverter_holding.py:1211). The generic writer bypasses that setter, so the mismatch is real; whether firmware clamps or rejects 91–100 is untested.

4. Reg 161 as a grid-tied "End" isn't what the library models. pylxpweb models the grid-tied SOC window as Start = 160 and End = reg 67, with a paired setter (hybrid.py:803). 161 reading correctly doesn't establish it as an independent grid-tied stop control. Since it ships disabled anyway, dropping it from the grid-tied branch until its behavior is verified costs nothing. Relatedly, _get_related_entity_types() currently links 160 and 161 to each other while excluding 67 — if 160/67 are the real grid-tied pair, that grouping is backwards.

5. An accepted no-op still reports success. Loud failures are handled (pylxpweb raises on HTTP errors and success:false), but a success:true response is trusted without comparing the refreshed register to the requested value, and refresh failures are swallowed. So a cloud write that the portal acknowledges but doesn't apply surfaces to the user as success while reg 160 stays at 90. Given grid-tied cloud writes are untested, an equality-checked readback would be the cheap insurance before Start ships enabled to pure-CLOUD installs.

6. Factual wording. The PR and changelog say reg-161 grid-tied writes were "observed rejected", citing the #332 note — but that note records them as read-only, and this codebase deliberately distinguishes the two. CLAUDE.md:452 also still reads "EG4_OFFGRID family only, #332" for these registers, which this change contradicts without updating.

[notexpected]

Thank you for the thorough review — all six items are addressed in ff15f54. Per item:

1. Local polling — the register-158 read now widens to 160-161 for the families that create the entity: (158, 4 if (is_offgrid or is_hybrid) else 2) in coordinator_local.py, so a pure-LOCAL grid-tied unit's Start entity reads from the cache it is served from. test_ac_charge_soc_window_registers_by_family replaces the old exclusion pin (EG4_OFFGRID and EG4_HYBRID both widen; unknown families keep the voltage-only read, mirroring the entity gate).

2. Fail-closed gate — grid-tied creation now sits behind is_hybrid_family() (the time.py schedule-family convention), not the else branch. LXP and unidentified hardware are excluded until verified; the fail-open suppression test once again asserts NO window entities without positive family detection.

3. Start range — writes cap at 90% (AC_CHARGE_START_BATTERY_SOC_MAX, sourced from pylxpweb's reg-160 definition and its hybrid setter bound). Reads keep the tolerant 0-100 window so an out-of-spec register value still displays rather than blanking.

4. Reg 161 dropped from grid-tied — the End entity is EG4_OFFGRID-only again; pylxpweb's 160+67 pairing is now reflected in the related-refresh sets (a reg-67 write refreshes Start and vice versa, alongside the existing 160/161 pairing that still applies on EG4_OFFGRID).

5. Equality-checked readback — _write_cloud_named_parameter takes a verify_register: after success: true the register is re-read (pylxpweb invalidates its parameter cache on successful writes, so the read is fresh) and a definite mismatch raises instead of reporting success. An unreadable readback only logs — a flaky read must not fail a write that in all likelihood landed. All three outcomes are pinned in tests.

6. Wording — "observed rejected" corrected to the fix: off-grid AC charge SOC uses portal-verified start/end params; SOC Limit gated (#331) #332 note's actual record (read-only) everywhere it appeared, and the CLAUDE.md register-table row now reflects the Start/End family split.

Adversarial follow-up — mirroring your process, an independent second review was run against the fix itself before pushing. It verified all six items against pylxpweb 0.9.39b4 source (including that the readback's key naming, cache freshness via invalidate_cache_for_device, exception scope, and optimistic-state rollback are correct) and caught four things now folded in: the rewritten changelog claimed "EG4_OFFGRID behavior is unchanged," which was false — the 90 cap and the readback apply there too, so an off-grid Start value above 90 written through an earlier beta becomes one-way (still displays, re-writable only at ≤90); docs/CONFIGURATION.md still described the window as off-grid-only; the widened (158, 4) hybrid read now carries a comment noting its single-model evidence and the blast radius if a family member NAKs past 159 (with a split-read escape hatch); and two test pins were added (the End entity's reg-161 readback arming, and the LXP exclusion asserted directly).

Full suite (2262), CI-pinned ruff, and strict mypy are clean. Same disclosure as the PR body: this response and the fix commits were written by Claude Fable 5 (model claude-fable-5, working in Claude Code) and reviewed by me before posting.

[btli]

Code review — fix round

Five of the six findings are fully resolved. Adversarial review (gpt-5.6-sol at xhigh) confirmed each against the code rather than the commit message:

1. Local polling — the read widens 158–159 to 158–161 for EG4_OFFGRID and positively-identified EG4_HYBRID, exactly the families that create the entity. The block is precisely 158–161 and doesn't overlap the next block at 169.
2. Gate — grid-tied Start creation now uses the fails-closed is_hybrid_family(); LXP and unknown families are excluded, matching the convention time.py follows.
3. 90% cap — exposed and enforced, matching pylxpweb. Reads keep the 0–100 validation window, so a legacy value like 95 is published unchanged rather than clamped or blanked. That's the right call.
4. Reg 161 — grid-tied now creates Start plus the existing reg-67 limit rather than a reg-161 End, and related refreshes link Start to reg 67. This matches how the library models the window.
5. Wording — code and docs now say reg 161 was observed read-only, and CLAUDE.md:452 distinguishes Start and End family scope correctly. A three-way merge simulation confirmed no semantic conflict with the ID-Formats correction that landed on main separately.

Finding 5 is partially resolved, plus two smaller items.

1. The readback can report a successful write as failed. The write is followed by an immediate read with no settle delay and no retry, and the first stale value raises. pylxpweb's own write-and-verify guidance waits ~2s for propagation, and its dongle verifier treats mismatches as diagnostic precisely because delayed clamping and concurrent propagation legitimately differ. So a write the portal accepted and applied can surface to the user as a failure if the immediate read is still stale.

2. An accepted no-op can still report success — finding 5 isn't fully closed. A definite numeric mismatch now fails, but read exceptions are swallowed, and an absent key or non-numeric value skips verification entirely. On the Hybrid fallback path the requested value is then seeded into the local cache as if confirmed. So success:true followed by a timeout, an omitted key or a malformed readback still reports success. The CHANGELOG's unconditional claim that an unapplied write now reports failure overstates what the code guarantees — worth narrowing to match.

3. Some of the new tests don't kill the mutation they're meant to. test_cloud_write_readback_match_succeeds and test_cloud_write_readback_failure_does_not_fail_write never assert read_parameters was called, so both pass with verification removed entirely. The changed Start rows in the non-integer and in-range-boundary tests also pass under the old 100 maximum. And nothing pins the promised 91–100 legacy read behaviour — the read tests cover 90 and 150 only.

None of these three block on their own; 1 is the one I'd fix first, since a false failure on a write that did apply is worse for a user than the no-op case it's guarding against — they'll retry a write that already succeeded.

[notexpected]

Thanks — all three are addressed in e629dec, with item 1 fixed first for the reason you gave (a false failure on a write that did apply is worse than the no-op it guards, since the user retries a write that already landed).

1. The readback no longer fails a slow-to-apply write. The first read is still taken immediately — the fast path for a write that lands at once — but a mismatch there is no longer decisive. Only a value that STAYS wrong across a settle re-read (_READBACK_SETTLE_SECONDS = 2 s, matching pylxpweb's write-and-verify guidance) fails the write. So a write the portal accepted and applied a beat later reads stale once, then correct, and reports success.

2. The claim is narrowed to what the code guarantees. You're right that a definite numeric mismatch is all that fails: a readback that cannot testify — a read that raises, an absent key, or a non-numeric value — is trusted rather than failed, and the Hybrid path then seeds the optimistic value as before. I kept that fail-safe direction (it's the counterweight to item 1) and instead narrowed the CHANGELOG: it now says the readback narrows the acknowledged-but-unapplied no-op class without eliminating it — a success:true whose readback times out or omits the key still reports success; only a register that keeps reading a different value does not. The docstring says the same.

3. The tests now bite.

• test_cloud_write_readback_match_succeeds and ..._failure_does_not_fail_write now assert read_parameters was actually awaited (with the right register), so deleting the verification fails them.
• New test_cloud_write_readback_recovers_after_settle pins item 1 — a stale-then-correct readback succeeds without a false failure — and ..._absent_key_trusts_write pins the documented trust path.
• The 90 write cap gets an explicit boundary test (90 accepted, 91 rejected) independent of the shared coerce-int parametrization, and a new parametrized read test pins the 91–100 legacy behaviour: a register value above the write cap still DISPLAYS (reads keep the tolerant 0–100 window) rather than blanking. The read tests now cover 91/95/100, not just 90 and 150.

Full suite 2268, ruff and strict mypy clean.

Same disclosure as before: this response and the fix commit were written by Claude Fable 5 (model claude-fable-5, working in Claude Code) and reviewed by me on Claude Opus 4.8 (model claude-opus-4-8) before posting.