SRNE: guarded TOU charge/discharge schedule-window writes (#67)

[arindahills]

Extends the OpenEMS#53 guarded settings-write path to the SRNE charge/discharge schedule registers, so time-of-use arbitrage (charge off-peak, discharge at peak) can be programmed through the same safe path. Revised to address the review; see the resolution comment below.

What

• Schedule windows: charge (0xE026/E027) + discharge (0xE02D/E02E), encoded hour*256+min, plus the schedule enable flags (0xE02C charge, 0xE033 discharge).
• A window is a coherent unit (ScheduleWindow): the contiguous start/stop pair is written as one atomic FC16 block and read-back verified; the enable is written last, only after the window verifies. enable=0 (disarm) is written first, independent of the window, so a failed/invalid window can never leave a schedule armed (disabled is the safe resting state).
• Validation: start and stop must be set together; start < stop (cross-midnight is rejected - the SRNE has no 24:00 or defined wrap, so end-of-day is 23:59); enable is 0/1 only.
• RTC (0xE034-036) exposed read-only so commissioning can verify the inverter clock before enabling any schedule.
• Same guards as Track changes to config OpenEMS/openems#53: default-off controlEnabled, verified-machine-state gate, one-shot, no auto-retry. Every target defaults to -1 (leave unchanged).

Tests (module :check green, JDK 21): ScheduleWindowTest drives the full write -> execute -> readback -> verify -> enable -> done lifecycle plus partial-failure-never-enables, execute-error, timeout, disarm-first, cross-midnight/zero-length/invalid/incomplete/enable-without-window/enable-out-of-range rejects. Component-level gate tests (control-disabled, unverified-state, queue) still pass.

Safety: offline code only, no live write; not commissioned. Plan + tariff economics in Bookstack "SRNE TOU arbitrage: plan (OpenEMS#67)"; commissioning is a supervised daytime step starting with a read-only on-site capture of the windows/enables/RTC. Companion to logger OpenEMS#67.

[tushabe]

I do not recommend onsite commissioning from this revision yet. CI is green and the scalar time validator is useful, but the schedule operation is not complete enough for a persistent production write.

Blocking findings:

1. The discharge schedule cannot be enabled. The live map records 0xE033=0 as the time-slot battery-discharge enable flag, but this PR neither reads nor writes 0xE033 (and likewise omits charge enable 0xE02C). Writing 0xE02D/E02E alone therefore does not establish that TOU discharge can run.

2. A window is split into independent one-register writes. Start and stop are reconciled by separate handlers and separate FC16 tasks. There is no pair-level validation, ordering guarantee, atomic block write, or rollback if one succeeds and the other fails. That can leave a persistent half-updated window. Validate the complete window first and update it as one coherent operation, or implement an explicit safe sequence with verified rollback.

3. The tests stop at QUEUED. They do not prove the exact FC16 frame(s), successful execution plus fresh readback, mismatch/timeout handling for a window, or partial-failure recovery. Add coverage for the complete start/stop operation and for control-disabled/unverified-state zero-write behavior specific to schedules.

4. RTC and interval semantics remain unresolved. OpenEMS#67 requires verifying the inverter RTC before relying on wall-clock execution. Also document and test how an intended 18:00–midnight window is represented: the validator rejects 24:00, while 18:00→00:00 is a cross-midnight pair whose validity is not currently assessed.

Please keep every new target at -1 and do not publish/commission this schedule-write image until these are resolved. A read-only onsite capture of 0xE026–0xE036, including enable flags and RTC, is still safe and useful.

[arindahills]

Revised to address all four review findings. Every schedule target still defaults to -1 (leave unchanged); this image is not for commissioning until you've re-reviewed.

Design change: the schedule window is now a coherent unit (ScheduleWindow) instead of independent per-register writes.

1. Enable flags — added charge-schedule-enable 0xE02C and discharge-schedule-enable 0xE033 as gated settings (chargeScheduleEnable, dischargeScheduleEnable, 0/1/-1). The enable is never a standalone afterthought; it is sequenced with the window (below).

2. Coherent / atomic window — the contiguous (start, stop) pair is written as a single atomic FC16 block (both elements set in the same cycle → merged into one frame), then both read-back verified. The enable flag is written last, only after both time registers verify. Any validation failure, execute error, or readback mismatch drives the window to FAILED and the enable is never raised — a mis-set or half-written window can never leave a schedule armed. Start and stop must be configured together; a half-specified window is rejected.

Direction-of-safety ordering for the enable:

• enable=0 (disarm) is written first, independent of the window — disabling always moves the inverter to the safe state, so a window typo or a failed window write can never block or delay it.
• enable=1 (arm) is written last, only after the window verifies and only with a complete configured window.

3. Deeper tests — the full lifecycle is now unit-tested in ScheduleWindowTest by driving the state machine directly (the test DummyModbusBridge does not execute writes, so component tests can only observe up to QUEUED): write → execute → fresh readback → verify → enable → done; partial-readback-failure proving the enable is never written; execute-error → FAILED; missing-readback timeout; disarm-first-then-reprogram; cross-midnight / zero-length / invalid / incomplete-window / enable-without-window / enable-out-of-range rejects; exact queued values pinned.

4. RTC + cross-midnight — cross-midnight windows (start >= stop) are rejected: the SRNE encodes hour*256+min with a max of 23:59 (5947) and no defined midnight wrap, so a window to midnight must stop at 23:59. The RTC (0xE034-036) is exposed read-only so commissioning can verify the inverter clock before any schedule is enabled.

I also ran an independent adversarial review pass over the revision, which caught and I fixed: (a) the enable register was not range-validated (a typo enable=2 could bypass the "arm requires a window" guard) — now rejected before any write, plus a metatype @Option constraint; (b) an explicit enable=0 could be dropped if a simultaneous window write failed — resolved by the disarm-first ordering above.

Per your guidance I have not commissioned this — all targets remain -1. A read-only on-site capture of 0xE026-0xE036 (windows + enables + RTC) is still the right next step before we program anything. Ready for re-review.

[tushabe]

Re-review at dcfb8a7a69cff7eeb84f42fb84abf0b0072085bf: the original four findings are substantially addressed, and both CI and a local ./gradlew :io.openems.edge.ess.srne:check pass.

One production-safety blocker remains:

Changing an already-enabled window does not disarm first. In reconcileFromIdle(), disarm-first only runs when the requested target is cfgEnable == 0. If the device currently reads actualEnable == 1 and the operator supplies a different complete window with cfgEnable == 1 (or leaves enable unmanaged at -1), enterWindowPhase() queues the FC16 start/stop write while the old schedule remains armed. The test windowMatchesButEnableAlreadySet currently codifies this path.

Please require a changed window to transition through disable → verified disabled → atomic window write → verified window → re-enable when requested. If enable is -1 and the existing schedule is armed, either reject the window change or safely disarm and restore the captured enable state; do not mutate a live enabled window.

Add tests for both:

• enabled device + changed window + cfgEnable=1: disable first, then re-arm last;
• enabled device + changed window + cfgEnable=-1: defined safe behavior with no live-window mutation.

Until that is fixed, the read-only onsite capture remains safe, but I still do not recommend schedule-write commissioning from this commit.

[arindahills]

Fixed the remaining blocker at e6782c05b9.

Never mutate a live enabled window. The reconcile is rebuilt around one invariant: the enable register must read 0 on the device whenever the window registers are written. It's now a convergence loop, and any change to a currently-armed window is sequenced exactly as you asked: disarm → verify-disabled → atomic window write → verify-window → re-arm. The window start/stop FC16 is only ever queued when the device reads enable == 0.

For enable == -1 on an armed device I took the "safely disarm and restore" option: the desired end-enable is captured once at the start (enable=0/1 from config; -1 captures the current device state), so a window change on an armed device is disarmed, rewritten, and restored to armed — never left silently disabled, never mutated in place. Capture-once means the post-disarm actualEnable == 0 can't turn a restore into a disable.

Tests: replaced windowMatchesButEnableAlreadySet (which codified the unsafe path) with the two you asked for:

• changedWindowOnArmedDeviceDisarmsFirstThenRearms — enabled device + changed window + enable=1: asserts the window element is untouched at DISABLE_QUEUED, then disarm → window write → re-arm last.
• changedWindowOnArmedDeviceWithUnmanagedEnableRestoresArmed — enabled device + changed window + enable=-1: disarm, rewrite, and the final enable write is 1 (captured armed state restored).

:check green on JDK 21, all tests pass. Targets remain -1; still not for commissioning until you're satisfied. Ready for re-review.

[tushabe]

Re-reviewed through 80137a794774d1195668f990bd4dbea5652c890a.

The remaining blocker is resolved:

• an armed schedule is disarmed and verified before any window FC16;
• the start/stop pair is written atomically and read back;
• re-arm happens last;
• enable=-1 captures and restores the original armed state;
• an enable flap before the window step re-disarms and never mutates the live window.

The new tests cover both requested enabled-device paths plus the flap case. I also ran ./gradlew :io.openems.edge.ess.srne:check locally: BUILD SUCCESSFUL.

Approved. Merge after the latest build-java check passes. For onsite commissioning, still begin with the planned read-only windows/enables/RTC capture, pin the resulting Edge image by digest, preserve the original block, and use a supervised reversible window test before enabling a real TOU period.