PR-11: Watering controller (host-tested application logic)#16
Merged
Conversation
Spec-kit artifacts for PR-11 (pure, 100%-host-tested WateringController + ReservoirController), approved at Checkpoint 2. Records the resolved decisions: soak gate enforced (from burst END), manual cap 300s, QUIRK-1 mode semantics, reservoir truth table + invalid-row + post-abort cooldown (FR-012a, deliberate divergence), fail-safe never delayed by the gate, 30s staleness, snapshot-helper enablers, periodic soil reader. No implementation yet. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…hot helpers
Phase 1+2 of PR-11:
- control component skeleton (pure, header-only until US1/US2 sources land)
- consistent-snapshot helpers on LockedSoil/Env/LevelSensor (one locked call ->
{validity + values}, closing the read()-then-getters cross-call gap; non-blocking,
snapshot caches read() outcome, no bus I/O). Resolves the TODO(PR-11) markers.
- MockSoilSensor scriptSuccessfulRead/scriptFailedRead (coherent FIFO, back-compat)
- host control test suites registered (stubs)
Verified: host 252/0, rev1 + rev2 build (0 errors) + board-config verified.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-safe (host-tested) Phase 3 / US1 (pure, host-tested MVP): - WateringController.tick(): fail-safe FIRST (soil unavailable / stale >30s / invalid moisture in auto -> immediate stop + logFailsafe, never delayed by the soak gate), then the watering decision (start at <=low when soak elapsed; stop at >=high). Soak measured from burst END (edge-detects the pump's own timed self-stop). Thresholds/ duration/soak read from config each tick. Gate on read result, not placeholder. lastValidSoilMs recorded before the stale test (fixes a first-read deadlock). - 14 host tests (8 automatic/soak + 6 fail-safe incl. not-delayed-by-soak + graceful degradation) On-target read()/isAvailable() vs snapshot integration is US3. Reservoir is US2. Verified: host 266/0 (+14), rev1 + rev2 build + board-config verified. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pure, board-independent reservoir fill controller: - level truth table (invalid/implausible rows -> no action; dry+dry -> fill) - running safety: stop on high-wet; pump cap aborts at 300 s (MaxRuntimeForced) - post-abort cooldown (kReservoirRefillCooldownMs=60 s): after a max-runtime abort, suppress a new auto fill until cooldown elapses even if still low-dry (guards a stuck high sensor / empty source); normal high-wet stop does not arm it; manual fill bypasses it - feature gate: !enabled forces the pump off and skips all logic 14 host tests (test_reservoir.cpp): all truth-table rows, stop-on-high, max-fill abort, disabled-forces-off, cooldown arm/bypass. Host 280/0; rev1 + rev2 build green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
WateringController gains the operator-override and telemetry-logging paths (still pure, host-tested): - startManual(int) clamps to [1,300] s, runs the plant pump, sets an override flag that exempts the run from the automatic fail-safe; stop() clears it. A pump self-stop (duration/cap) clears the override on the next tick. - tick() now logs env+soil telemetry via maybeLogData() BEFORE the mode / fail-safe branches, so telemetry is recorded regardless of watering state. Soil is read once per tick and reused by both logging and the decision. - maybeLogData() gates on IWallClock::isTimeSet() (never logs a 1970 epoch), honours getDataLogIntervalMs(), stamps IWallClock::nowEpoch(), logs env (temperature/humidity/pressure) and soil (moisture/temperature/ph/ec) plus NPK only when >= 0. soil_humidity is intentionally NOT logged: getHumidity() is identical to getMoisture() (parity), so the max distinct-metric set is exactly 10 = kMaxMetrics (3 env + 4 soil base + 3 NPK), no data loss. Constructor now injects IEnvironmentalSensor& (env) so the controller is the single host-tested owner of periodic data logging. +9 host tests (manual bypass/clamp/clear, auto-not-manual, log cadence/epoch/ NPK filter/time-not-set gate/failsafe-path). Host 289/0; rev1 + rev2 green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…arget
Adds the decision-layer watering task and constructs the controllers in
app_main (T013/T014/T015):
- new main/watering_task.{h,cpp}: a watchdog-subscribed FreeRTOS task that
ticks at config.getSensorReadIntervalMs() (floored at 1 s). Each cycle it
calls WateringController::tick() and (rev1) ReservoirController::tick(). The
controller-as-reader does the periodic soil Modbus read here, refreshing the
LockedSoilSensor cache the PR-09 /sensors endpoint serves (this is why no
separate soil-reader task is needed). Blocking bus I/O is isolated on this
task, off the 10 Hz safety loop.
- app_main constructs WateringController (+ ReservoirController on rev1) as
function-local statics over the live Locked* wrappers, after the sensors and
before console registration, and starts the watering task after sensor_task.
pumps_force_off() stays first; the 10 Hz enforcement loop is unchanged (it
still owns precise pump-timing). The API mode flag reaches the controller
purely through config (getWateringEnabled(), read each tick) — no direct
ApiServer<->controller call.
- reservoir tick mapping (rev1): tick(true, getWateringEnabled()) — the pump
always exists so enabled is always true; automatic level control is gated by
the same mode flag, so manual mode suspends auto-fill while manual API fills
still work. There is deliberately no dedicated auto-level config flag.
- main/CMakeLists: +watering_task.cpp, +control in PRIV_REQUIRES.
rev1 + rev2 build green; host suite unchanged (289/0, control component
untouched).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The diag console rs485test command drove the raw EspModbusClient directly, bypassing LockedSoilSensor's mutex. With PR-11's periodic soil reader now issuing bus transactions from the watering task, that was a cross-task RS485 bus-transaction race (documented TODO at diag_console rs485test). Fix: new header-only LockedModbusClient (IModbusClient decorator, one mutex around every call, same pattern as LockedSoilSensor/LockedWaterPump). app_main wraps the EspModbusClient in it and injects the wrapper everywhere — the ModbusSoilSensor's transactions and rs485test now share one mutex, so no two bus transactions overlap. Lock order is always (soil mutex -> modbus mutex) or (modbus mutex alone); the wrapper never calls back into the sensor, so no deadlock. Per-transaction serialization suffices: interleaving complete request/response transactions on the bus is safe, only overlapping ones corrupt. rs485test code is unchanged (the fix is purely the injected locked instance); its stale TODO comment is updated to note the resolution. rev1 + rev2 build green; host suite unchanged (289/0). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…e checklists - firmware/CLAUDE.md: new "Watering controller (feature 011)" section (control component, pure/host-tested logic, soak-from-burst-end + enforcement, fail-safe precedence, manual=flag+300 s cap, reservoir truth table + cooldown divergence, data-log metric set incl. why soil_humidity is dropped, snapshot helpers, controller-as-reader periodic soil, watering task + watchdog, LockedModbusClient race fix). - specs/011/checklists/hil.md: rev1 bench-rig smoke (live soil, automatic + soak, fail-safe precedence, manual override, reservoir fill/stop/abort/cooldown, logging, isolation, rs485 race) with the deferral path. - specs/011/checklists/branch-coverage.md: every decision/safety branch mapped to its covering host test; records the green run (289/0, both boards, sizes). Size (T019): rev1 1037.3 KiB, rev2 1039.4 KiB in the 1536 KiB OTA slot (~32 % margin). No new managed deps (T018). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…gaps Addresses the Checkpoint-3 review findings. M1 (watchdog boot-loop): watering_task fed the WDT once per loop while sleeping the full, operator-configurable getSensorReadIntervalMs() — set above the ~20 s task-WDT timeout it would panic-reboot and boot-loop (NVS-persisted). Now uses chunked feed-and-sleep (1 s slices, feed each) and ticks once per full period, so a large configured cadence can no longer starve the watchdog. M2 (redundant blocking probe + dead snapshot helpers): the controller did read() then a second blocking isAvailable() RS485 probe per tick — a transient glitch on that probe after a good read could spuriously fail-safe-stop the pump. Promoted snapshot() to the ISoilSensor interface (ModbusSoilSensor tracks read history; LockedSoilSensor locks + delegates; MockSoilSensor implements it). tick() now reads once then consumes ONE coherent snapshot() — no second probe, no torn tuple; availability = ever-read-ok (never-read -> soil-unavailable, worked-then- lost -> soil-stale). maybeLogData logs from the same snapshot. The snapshot helpers are now live, not dead code, and the header/manual-flag comments match the code. Hardening: watering_task_start now logs a durable EventLogger::logFailsafe( "watering-task-start-failed") on xTaskCreate failure (visible in /api/v1/events). Test gaps (the deliverable is 100 % branch coverage): +T-A high-threshold soak origin, +T-B reservoir auto-level-off suppresses fill (manual still works), +T-C drop-sensitive 10-metric cap assertion (rejectedWrites==0), +NPK phosphorus/potassium negative, +env-read-failure branch. branch-coverage.md updated. Docs: retagged stale TODO(PR-11) markers to PR-14 (diag_console env/level/power, LockedPowerSensor); CLAUDE.md snapshot payload note qualified (error is soil-only). Host 293/0; rev1 + rev2 build green. Co-Authored-By: Claude Opus 4.8 <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
1 participant
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.
Feature 011 — the watering-decision layer, the master-PRD's 100 %-host-tested logic PR. New pure
controlcomponent driven entirely over the interfaces + injected clocks.What's here
WateringController— pulsed automatic watering: start at moisture ≤ low when the soak pause (measured from the burst END, enforced — FR-003 divergence) has elapsed; stop at ≥ high. Fail-safe (soil unavailable / stale > 30 s / moisture out of range) is checked BEFORE the soak gate and never delayed by it. Manual override (cap 300 s, exempt from fail-safe). Periodic env+soil data-logging (epoch-stamped, gated onisTimeSet(), NPK only when ≥ 0;soil_humidityomitted as it duplicatessoil_moisture, keeping the set at exactlykMaxMetrics=10).ReservoirController(rev1) — level truth table (invalid/implausible → no action), stop-on-high, 300 s max-fill abort, and a post-abort cooldown (FR-012a, the approved divergence — guards a stuck high sensor / empty source). Host-tested regardless of board.main/watering_task.cpp, a watchdog-subscribed task at the sensor cadence. The controller-as-reader does the periodic soil read (refreshing the/sensorscache), isolated off the 10 Hz pump-timing loop. The API mode flag reaches the controller only throughconfig(FR-017 isolation).LockedModbusClient— serializes the consolers485testprobe with the periodic reader (fixes the documented cross-task RS485 race).snapshot()promoted toISoilSensor— the controller reads once then consumes one coherent, non-blocking snapshot (no redundant blockingisAvailable()bus probe).Verification
specs/011-.../checklists/branch-coverage.md.docs/checkpoints/cp3-011-watering-controller.md.HIL
Deferred to the rev1 bench rig (
specs/011-.../checklists/hil.md) — the merge gate is the host suite + both board builds, per the project's host-test-PR policy.Safety
pumps_force_off()remains first inapp_main; the purecontrolcomponent has noesp_*includes; the 10 Hz enforcement loop is unchanged. Merge with a merge commit (never squash — spec-kit branch chaining).🤖 Generated with Claude Code