Skip to content

PR-04: Modbus soil sensor over RS485 (esp-modbus)#10

Merged
cryptotomte merged 11 commits into
mainfrom
004-modbus-soil-sensor
Jul 2, 2026
Merged

PR-04: Modbus soil sensor over RS485 (esp-modbus)#10
cryptotomte merged 11 commits into
mainfrom
004-modbus-soil-sensor

Conversation

@cryptotomte

@cryptotomte cryptotomte commented Jul 2, 2026

Copy link
Copy Markdown
Owner

Replaces the legacy hand-rolled SP3485 Modbus client with esp-modbus 2.1.2 behind ported IModbusClient/ISoilSensor interfaces, per docs/prd/PR-04-modbus-soil-sensor.md and spec specs/004-modbus-soil-sensor/.

What's in here

  • New sensors component (actuators pattern): pure-logic ModbusSoilSensor (decode/validation/calibration, host-tested), EspModbusClient (the only hardware touchpoint, CMake-excluded on the linux host-test target), LockedSoilSensor mutex decorator, MockModbusClient/MockSoilSensor test fixtures.
  • Parity §5 faithful: 9 holding registers (slave 0x01, fn 0x03) in one transaction, scaling incl. signed temperature, range validation (moisture 0–100 / temp −40–80 / pH 3–9 → error 5; EC/NPK deliberately unenforced), 3000 ms response timeout, no retry, real-bus availability probe, statistics counters, calibration with legacy semantics (local factor + best-effort register write, non-fatal on write failure).
  • Dual-board RS485: UART_MODE_RS485_HALF_DUPLEX on both boards — rev1 drives DE via hardware RTS (GPIO 25), rev2 configures no direction pin (compile-time proven: the pin macro does not exist on rev2). rev2 hardware requirements from the 2026-07-02 design review: FW-2 (RX internal pull-up against floating THVD1426 RO) and FW-4 (TX echo suppressed by half-duplex receive gating; electrical verification at PR-14).
  • Console diagnostics: soil, rs485test, soil_cal_moisture/ph/ec.
  • Input hygiene beyond parity: calibration rejects non-finite/non-positive references and factors exceeding the uint16 register encoding (legacy had UB here); console float parsing rejects nan/inf.
  • Host tests: 29 soil-sensor tests (suite total 79, 0 failures) — decode incl. negative temperature, all validation branches incl. boundary inclusivity, all-or-nothing publish, no-retry invariant, timeout/exception/recovery paths, lazy-init failure modes, full calibration contract.

Documented parity divergences (see specs/004-modbus-soil-sensor/plan.md Risks)

  1. Slave exception codes: esp-modbus 2.1.2 does not surface the exception number — all non-timeout failures map to bus error 2 (timeout stays distinct, which is the safety-bearing part of FR-010). Legacy used 100+n.
  2. Write echo: esp-modbus validates FC06 response framing (address/function/CRC) but does not compare the echoed value byte-for-byte; the legacy client verified the full 8-byte echo.
  3. setTimeout: applies at initialize() time only (esp-modbus takes the timeout at create).

Review

Full agent review (code, tests, silent failures, comments, type design) + fixer pass + clean verification re-review at Checkpoint 3. Verified: host suite 79/0 on the linux target, both board targets build green from clean checkout, dependencies.lock stable, frozen legacy untouched.

HIL checklist — merge gate ⚠️

Bench-rig verification per specs/004-modbus-soil-sensor/checklists/hil.md runs when Paul is back at the rig; results will be posted as a PR comment (same pattern as #7). Do not merge before HIL is green.

  • A. Basic readings match the Arduino unit (SC-002)
  • B. A/B disconnect → invalid + logged, auto-recovery on reconnect (SC-003)
  • C. RS485 frame integrity — hardware RTS vs TXS0108E margins (plan risk R2, scope)
  • D. Calibration commands (optional bench check)
  • E. Regression guard: pumps OFF at boot, existing console commands intact

🤖 Generated with Claude Code

cryptotomte and others added 11 commits July 2, 2026 10:29
@cryptotomte @claude
docs(spec): PR-04 modbus soil sensor specification
a6c7989 
Spec for the esp-modbus driver replacing the legacy SP3485 client:
9-register parity read map, fail-safe validity semantics, dual-board
RS485 direction handling, and rev2 hardware requirements FW-2 (RX
pull-up against floating RO) and FW-4 (TX echo discard) from the
2026-07-02 design review. One open clarification (calibration scope)
pending at Checkpoint 1.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cryptotomte @claude
docs(spec): record Checkpoint 1 clarification — calibration in scope
4fc71bc 
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cryptotomte @claude
docs(plan): PR-04 implementation plan, research, data model, contract…
b34b36c 
…s, quickstart

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cryptotomte @claude
docs(tasks): PR-04 task breakdown — 29 tasks across 7 phases
a693c0f 
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cryptotomte @claude
docs(spec): apply CP2 analyze remediations I1/I2/C1/C2
23b2db8 
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cryptotomte @claude
feat(firmware): sensors component skeleton, IModbusClient/ISoilSensor…
ac34c53 
… interfaces, mock client

T001+T003-T005: component CMake with linux-target exclusion of the
hardware client, esp-modbus ==2.1.2 rule-gated to non-linux targets,
ported pure-C++ interfaces with parity contract notes, scriptable
MockModbusClient with call recording for the host test suite.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cryptotomte @claude
feat(firmware): Modbus soil sensor MVP — driver, esp-modbus client, c…
9e76c6b 
…onsole diagnostics

T006-T013 (US1): pure-logic ModbusSoilSensor ported from legacy with
confirmed parity quirks (moisture factor not applied on read; pH/EC
validated on factored values; availability probes register 0x0000),
EspModbusClient on esp-modbus 2.1.2 with RS485 half-duplex on both
boards (RTS=DE on rev1, none on rev2), FW-2 RX pull-up, FW-4 echo note,
LockedSoilSensor decorator, app wiring, soil/rs485test console commands,
BOARD_RS485_UART_PORT in both board profiles, four decode host tests.

Verified: host suite 54/0 on linux target; rev1 board build green;
FMB timeout Kconfig range 150-30000 accommodates 3000 ms (T002);
dependencies.lock manifest hash updated.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cryptotomte @claude
test(firmware): soil sensor fault-path host tests — invalid data neve…
fe20d55 
…r wrong data

T014-T017 (US2): 12 tests covering timeout/range/exception rejection with
all-or-nothing publish, no-retry invariant, recovery without reinit,
real-bus availability probing, lazy-init failure paths, statistics and
setTimeout contracts. One logging gap fixed in isAvailable(). Host suite
66/0 on linux target.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cryptotomte @claude
feat(firmware): soil calibration tests, MockSoilSensor, console calib…
e522bc3 
…ration commands

T018-T027: FW-2/FW-4 confirmations (written in phase 3, verified),
rev2 board build green with compile-time proof of no DE-pin reference,
6 calibration host tests (factor computation, x100 register write,
moisture-factor-not-applied parity, non-fatal write failure, validation
on factored values), MockSoilSensor fixture for PR-11, soil_cal_*
console commands, HIL checklist, firmware/CLAUDE.md sensors section,
plan risk outcomes recorded (R5/R6 divergences documented, R2 open for
HIL). Host suite 72/0.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cryptotomte @claude
docs(spec): all 29 tasks complete — final clean verification green
c733211 
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cryptotomte @claude
fix(sensors): CP3 review fixes — calibration input hygiene, write-ech…
36b512b 
…o divergence, doc accuracy

A1: reject non-finite/non-positive calibration references and factors
exceeding the uint16 register encoding (UB before; NaN factors defeated
range validation and poisoned readings as valid); console parse_float
rejects non-finite input. A2+B1: 7 new host tests (untested validation
branches temp>80/pH<3, boundary inclusivity, EC/NPK non-enforcement
parity, calibration raw-read failure, invalid-reference and factor-
overflow guards) + isAvailable lastError assert. A3: write-echo claim
corrected — esp-modbus 2.1.2 validates FC06 framing only, no echoed-
value comparison; documented as parity divergence (plan risk 6).
B2: initialize() teardown paths log mbc_master_delete failures.
B3-B6: interface doc scoping (100+n collapse, setTimeout timing, lazy
init, getLastError scope), comment-rot fixes, console argc checks +
TODO(PR-11) on rs485test concurrency, spec/contract sync (single-reply
parsing wording, no-factor console output).

Verified: host suite 79/0 on linux target; rev1 + rev2 builds green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cryptotomte cryptotomte merged commit 4e102e7 into main Jul 2, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@cryptotomte