SRNE: guarded output-priority (E204) + BMS-enable (E215) writes (#71)

[arindahills]

Adds output source priority (E204, e.g. SBU) and BMS-comms enable (E215) to the guarded write allow-list, so "switch to battery at a SoC" (SBU + BMS-SoC) can be set fully from OpenEMS instead of the panel.

Tracked by nfe-modbus-energy-logger OpenEMS#71 (sub-issue of OpenEMS#56, immediate source transfer).

What

• E204 / E215 as guarded scalar writes: controlEnabled default off, verified-machine-state gate, target default -1, one-shot, read-back verified, bounded-range validated. Same envelope as Track changes to config OpenEMS/openems#53.

Safety: output priority is an "arm", so it is gated

E204 = SBU transfers the load onto the battery — effectively the enable for battery discharge. Mirroring the schedule feature's verified-then-enable, reconcileOutputPriority() writes E204 only after its prerequisites (the switchToLineSoc/switchToBatterySoc reserve band and bmsCommunication, where configured) have read-back verified OR already read the configured value on the device. A failed or pending reserve-floor write can never arm battery discharge against a wrong reserve.

⚠️ Do NOT set these targets yet — enum unverified

The value→mode mapping is not verified in the SRNE manuals: the generic Modbus manual says E204 is 0..2, but the live unit reads 3 and the ASP operation manual lists four modes (UTI/SUB/SOL/SBU) by name only; E215 similarly (generic 0/1 vs operation-manual DIS/485/CAN). So the ranges here are bounded only (E204 0..3, E215 0..2) and read-back verify only confirms the register accepted the number, not that it is the intended mode. A wrong-but-in-range value (an output mode that drops the load, or disabling BMS) would be accepted. This is safe only while targets stay -1. The enum MUST be captured on the unit (set each mode via LCD, read E204; confirm the BMS register) and recorded before any live write — see OpenEMS#71.

Tests (:check green, JDK 21)

• E204/E215 queue-on-change and reject at the exact boundary (E204 3 ok / 4 rejected; E215 2 ok / 3 rejected).
• testOutputPriorityPrerequisiteGate — the gate helper across unmanaged / verified / already-correct / pending / failed.
• testOutputPriorityArmsWhenPrerequisiteAlreadyCorrect — proves E204 still arms when a managed prerequisite already matches the device (guards against a gate deadlock in the disciplined "set band first, add priority later" workflow).

Ran an independent adversarial reviewer over this before opening (it caught the missing gate, the missing E215 tests, and a gate deadlock — all fixed). Targets remain -1; not for commissioning until OpenEMS#71 confirms the enum.

[tushabe]

I do not recommend merging the arm path as written yet. The module check passes locally and the default -1 targets are safe, but the prerequisite gate has two production-safety gaps.

1. Pending or failed prerequisites can pass the gate. isPrerequisiteSettled() returns true whenever actual == configuredTarget, regardless of handler state. That means QUEUED, AWAITING_READBACK, or even FAILED can arm E204 if the channel happens to show the target. Conversely, VERIFIED returns true even when the current channel is null or has drifted away. This does not enforce the stated “verified or already correct” contract. Please make the state distinction explicit:

   • unmanaged: settled only if intentionally allowed by the policy below;
   • IDLE: settled only when the current read equals the target;
   • VERIFIED: settled only while the current read still equals the target;
   • QUEUED, AWAITING_READBACK, FAILED, UNDEFINED: never settled.

   Add exact tests for FAILED + matching actual, AWAITING_READBACK + matching actual, and VERIFIED + drift/null.

2. outputPriority can arm with no safety prerequisites configured. The current queue test sets only outputPriority; all SoC-band and BMS targets are -1, so the gate treats all three as settled and transfers the load using unknown existing reserve/BMS settings. For an operation described as an arm, require an explicit complete prerequisite set—at minimum switchToLineSoc, switchToBatterySoc, and the verified BMS mode—before a changed E204 can queue. Also validate the reserve pair as a coherent band (line threshold below battery-return threshold) before arming. If the intended policy is to accept pre-existing device settings instead, those settings need explicit validated values and live reads, not unconditional -1 => true.

The enum mapping still must be captured onsite before any write, as OpenEMS#71 says. A read-only capture is safe; keep E204 and E215 targets at -1 until that evidence is recorded.

[arindahills]

Both gaps fixed at 1925da4fc4. Thanks — these were real; my own pre-PR reviewer missed them because its tests only exercised the handler state with a non-matching read.

1. State bypass. isPrerequisiteSettled now short-circuits on state first: a prerequisite is settled only when its handler is at rest (IDLE = already correct, or VERIFIED) and the device currently reads the configured target. QUEUED/AWAITING_READBACK/FAILED/UNDEFINED never settle even if the read momentarily matches; a VERIFIED value that has drifted (or a null read) is not settled. Tests now cover exactly those: FAILED+match, AWAITING+match, QUEUED+match, UNDEFINED+match, VERIFIED+drift, VERIFIED+null.

2. Arming with no prerequisites. A changed outputPriority is now rejected (→ FAILED) unless switchToLineSoc, switchToBatterySoc and bmsCommunication are all configured, and the reserve band is coherent (switchToLineSoc strictly below switchToBatterySoc). Then each must be settled per (1). It never arms on unknown existing device settings. Tests: testOutputPriorityRejectedWithoutPrerequisites, testOutputPriorityRejectedWithIncoherentBand (line > battery), testOutputPriorityRejectedWithEqualBand (line == battery boundary).

Distinction preserved: a misconfiguration (missing prereq / incoherent band) rejects to terminal FAILED; a genuinely pending prerequisite holds (no reject) and arms once it settles.

One thing for your awareness (out of scope here, pre-dates this PR): the coherence check gates the arm, but reconcile(4)/(5) still write individually-valid-but-incoherent switchToLineSoc/switchToBatterySoc to E01F/E020 independently — harmless while SBU isn't armed, but an incoherent band would land on the device if the unit were already in battery-priority mode set outside OpenEMS. Happy to add band-coherence to those writes in a follow-up if you'd like. Targets remain -1; enum still to be captured on-unit per OpenEMS#71.

[tushabe]

Re-review at 1925da4fc475f8de70a66773b07066bec5cfd9f3: the two requested gate fixes are correct, the new state matrix is appropriate, and CI is green.

One remaining production-safety blocker follows from the interaction noted in your response:

Reject an incoherent reserve band before either persistent threshold is queued. reconcileSafeSettings() still calls the scalar reconciles for E01F and E020 before reconcileOutputPriority() checks line < battery. With an invalid pair, both individually valid values can therefore be written to the inverter even though the E204 arm subsequently enters FAILED. If the unit is already in SBU/battery-priority mode from its panel configuration, that is not harmless; the bad reserve band becomes active immediately. It also leaves persistent drift after a rejected operation.

When outputPriority is requested, validate that the complete reserve pair is present, individually bounded, and coherent before reconciling either E01F or E020. On failure, queue no writes for the pair, BMS mode, or E204 and expose terminal FAILED. Add a component test that supplies an incoherent but individually in-range pair and asserts the write elements/handlers for E01F and E020 never queue, rather than only asserting aggregate FAILED.

The enum mapping remains an onsite read-only prerequisite, and all targets should remain -1 until it is recorded.

[arindahills]

Fixed at 21c5baa69e (rebased onto your main merge).

Your finding — a rejected arm no longer touches the reserve band. reconcileSafeSettings used to write E01F/E020 before the arm's coherence check. That's gone: the reserve band, BMS and E204 are now one operation (reconcileSourceTransfer). When an arm is requested the whole set is validated up front — all three present, individually in range, band coherent (switchToLineSoc below switchToBatterySoc) — and on any failure nothing is written (not the band, not BMS, not E204), reporting terminal FAILED. New test testIncoherentBandArmWritesNothingToReserveBand asserts the reserve-band handlers stay IDLE (never queue) on a rejected arm, not just the aggregate.

I also closed the related door you'd expect next. A bad band is dangerous whenever the unit is in SBU (its normal backup posture, settable from the panel), so an incoherent band is now rejected on the no-arm path too when both thresholds are configured (rejectBand). Test testIncoherentBandRejectedWithoutArm.

One deliberate boundary, for your call. Coherence is enforced when both thresholds are OpenEMS-managed. If an operator sets only one threshold (leaving the other unmanaged) such that it inverts against the device's current value, that single write isn't blocked — the principle is "OpenEMS validates what OpenEMS manages," and validating a managed write against transient unmanaged device state is a more fragile design (it can reject a legitimate mid-configuration). The device value is readable, so I can close this too if you'd prefer — but I'd rather not couple a write to unmanaged device state without your nod. Flagging it as a conscious decision rather than leaving it silent.

Enum still an onsite read-only prerequisite (OpenEMS#71); targets remain -1. Build green, all tests pass.

[tushabe]

Re-reviewed through 21c5baa69e897e8f95de9058c7568bd8acc520ef.

The remaining blocker is resolved:

• source transfer validates the complete reserve band, BMS target, and E204 target before any related write queues;
• an invalid arm leaves E01F/E020/E215 handlers IDLE and E204 terminal FAILED;
• incoherent complete reserve bands are also rejected when no arm is requested;
• the state-aware prerequisite checks from the prior revision remain intact.

I ran ./gradlew :io.openems.edge.ess.srne:check locally: BUILD SUCCESSFUL.

Approved. Merge after the latest build-java check passes. The consciously retained single-threshold behavior should be documented as a follow-up hardening item: if only E01F or E020 is managed, coherence against the unmanaged live counterpart is not currently enforced.

For onsite work, E204/E215 remain read-only discovery until OpenEMS#71 records the verified enum mapping; keep both targets at -1 until then.