diff --git a/.specify/feature.json b/.specify/feature.json index 229797a..9101c8f 100644 --- a/.specify/feature.json +++ b/.specify/feature.json @@ -1,3 +1,3 @@ { - "feature_directory": "specs/003-nvs-littlefs-storage" + "feature_directory": "specs/004-modbus-soil-sensor" } diff --git a/firmware/CLAUDE.md b/firmware/CLAUDE.md index e074f3e..24e18b9 100644 --- a/firmware/CLAUDE.md +++ b/firmware/CLAUDE.md @@ -28,8 +28,10 @@ must stay green. ### Host tests (linux preview target) -Pump enforcement logic is unit-tested natively, no ESP32 needed. The test -executable's exit code equals the Unity failure count (CI gate, job +Logic is unit-tested natively, no ESP32 needed: pump enforcement, config +store, data storage and the soil sensor decode/validation/calibration +(`test_soil_sensor.cpp`, real `ModbusSoilSensor` over `MockModbusClient`). +The test executable's exit code equals the Unity failure count (CI gate, job `host-test`): ```bash @@ -59,12 +61,19 @@ firmware/ │ │ └── include/board/board.h │ ├── interfaces/ # Header-only, NO IDF deps (host-includable) │ │ └── include/interfaces/ # IActuator, IWaterPump, ITimeProvider, -│ │ # IConfigStore, IDataStorage +│ │ # IConfigStore, IDataStorage, +│ │ # IModbusClient, ISoilSensor │ ├── actuators/ # Pump drivers │ │ ├── include/actuators/ # WaterPump (pure C++ logic), GpioWaterPump, │ │ │ # EspTimeProvider (esp32-only header), │ │ │ # testing/ (MockWaterPump, FakeTimeProvider) │ │ └── src/ # GpioWaterPump.cpp excluded on linux target +│ ├── sensors/ # RS485 Modbus soil sensor (feature 004) +│ │ ├── include/sensors/ # ModbusSoilSensor (pure C++ logic), +│ │ │ # EspModbusClient, LockedSoilSensor, +│ │ │ # testing/ (MockModbusClient, MockSoilSensor) +│ │ └── src/ # EspModbusClient.cpp + esp-modbus dep +│ │ # excluded on linux target │ └── storage/ # Config + data persistence (feature 003) │ ├── include/storage/ # NvsConfigStore, LittleFsDataStorage (POSIX, │ │ # host-runnable), StorageMount (esp32-only), @@ -74,7 +83,8 @@ firmware/ │ # on linux target (esp_littlefs has no port) └── test_apps/ └── host/ # Host test app (linux preview target, Unity): - # pump + config store + data storage suites + # pump + config store + data storage + + # soil sensor (test_soil_sensor.cpp) suites ``` Future components (drivers, controllers, web server) are added as siblings @@ -115,6 +125,16 @@ config get | set | wifi | wifi-clear | factory- storage stats | log | query [t0 t1] | event | events [n] ``` +Feature 004 adds the soil sensor commands (HIL verification path, same +thin-wrapper rule; a failed calibration-register write is reported as +non-fatal — legacy parity): + +``` +soil # one read(); 7 values or error code +rs485test # raw 1-register Modbus probe + statistics +soil_cal_moisture | soil_cal_ph | soil_cal_ec +``` + ## Storage (config + data persistence) Feature 003 (PR-06). Two redesigned, host-includable interfaces in @@ -146,7 +166,19 @@ data is migrated; on-disk formats diverge from legacy by design — see The diagnostic console (`ws>`) exposes `config` and `storage` subcommands for the HIL verification path (see below). -## Board abstraction +## Soil sensor (RS485 Modbus) + +Feature 004 (PR-08). `components/sensors/` splits the driver at the +`IModbusClient` interface: `ModbusSoilSensor` is pure C++ +(decode/scaling/validation/calibration, host-tested against +`MockModbusClient`), `EspModbusClient` is the only hardware touchpoint +(esp-modbus RTU master, UART RS485 half-duplex on both boards, RX pull-up +for the rev2 SHDN̅/hi-Z case) and is excluded from the linux build. The +**esp-modbus dependency is pinned `==2.1.2`** in the component's +`idf_component.yml` and rule-gated to `target != linux`, keeping it out of +the host-test dependency graph. Cross-task access goes through +`LockedSoilSensor` (console REPL now, main-loop reader in PR-11) — same +pattern as the other Locked* wrappers. Two board revisions exist, selected via Kconfig (`main/Kconfig.projbuild`): @@ -189,7 +221,9 @@ must fit in 1.5MB per slot. logging. Check IDF errors with `ESP_ERROR_CHECK` or explicit handling. - **Include guards:** `WATERINGSYSTEM_PATH_FILE_H` (e.g. `WATERINGSYSTEM_BOARD_BOARD_H`). -- **Managed dependencies** are pinned exactly in `main/idf_component.yml`; +- **Managed dependencies** are pinned exactly in `main/idf_component.yml` + AND `components/sensors/idf_component.yml` (esp-modbus is pinned `==2.1.2` + in both — bump the two in lockstep or the resolver conflicts); `dependencies.lock` is committed, `managed_components/` is not. ## Testing strategy diff --git a/firmware/components/board/include/board/board.h b/firmware/components/board/include/board/board.h index 087750c..d60bcd4 100644 --- a/firmware/components/board/include/board/board.h +++ b/firmware/components/board/include/board/board.h @@ -39,6 +39,9 @@ #define BOARD_PIN_RS485_RX 17 #define BOARD_HAS_RS485_DE 1 #define BOARD_PIN_RS485_DE 25 +/* Modbus RTU runs on UART2 at 9600 baud 8N1 (parity: legacy Serial2, + * docs/parity-checklist.md §5). */ +#define BOARD_RS485_UART_PORT 2 /* Pumps (MOSFET gates, active high) */ #define BOARD_PIN_MAIN_PUMP 26 @@ -83,6 +86,10 @@ * is 0: any reference that is not guarded by #if BOARD_HAS_RS485_DE becomes * a compile error instead of undefined behavior (e.g. 1ULL << -1) or a * silently dropped ESP_ERR_INVALID_ARG at runtime. */ +/* Modbus RTU runs on UART2 at 9600 baud 8N1 (parity: legacy Serial2, + * docs/parity-checklist.md §5). The UART number is a parity fact, not part + * of the provisional rev2 pin map — no TODO(SYNC1). */ +#define BOARD_RS485_UART_PORT 2 /* Pumps (MOSFET gates, active high) */ #define BOARD_PIN_MAIN_PUMP 26 // TODO(SYNC1): final rev2 pin map frozen at hardware sync 1 diff --git a/firmware/components/interfaces/include/interfaces/IModbusClient.h b/firmware/components/interfaces/include/interfaces/IModbusClient.h new file mode 100644 index 0000000..0923cba --- /dev/null +++ b/firmware/components/interfaces/include/interfaces/IModbusClient.h @@ -0,0 +1,116 @@ +// SPDX-FileCopyrightText: 2026 Cryptotomte +// SPDX-License-Identifier: AGPL-3.0-or-later +/** + * @file IModbusClient.h + * @brief Modbus RTU master interface for the RS485 sensor bus. + * + * Ported from the frozen Arduino firmware (include/communication/ + * IModbusClient.h) with the same method surface; Arduino-era base-class + * baggage is trimmed. Normative contract: + * specs/004-modbus-soil-sensor/contracts/interfaces.md. + * + * Error codes reported by getLastError() + * (specs/004-modbus-soil-sensor/data-model.md): + * + * 0 OK + * 1 not initialized + * 2 bus/communication error (CRC, framing, truncated response; also + * covers slave exceptions collapsed by implementations that cannot + * surface the exception number — see 100+n) + * 3 timeout (no response within the configured timeout) + * 5 range validation failed — set by the sensor layer on top of this + * interface, never by IModbusClient implementations themselves + * 100+n Modbus slave exception n (when the implementation can surface it + * — EspModbusClient collapses slave exceptions onto code 2, + * research.md R6; only test mocks emit 100+n today; consumers must + * not branch on 100+n) + * + * Part of the header-only `interfaces` component: no IDF includes allowed. + */ + +#ifndef WATERINGSYSTEM_INTERFACES_IMODBUSCLIENT_H +#define WATERINGSYSTEM_INTERFACES_IMODBUSCLIENT_H + +#include + +/** + * @brief Modbus RTU master: single-attempt register reads/writes. + */ +class IModbusClient { +public: + virtual ~IModbusClient() = default; + + /** + * @brief Bring up the bus (UART/transceiver). Must precede any transfer. + * + * @return true on success; false leaves the client unusable (subsequent + * transfers fail with error 1). + */ + virtual bool initialize() = 0; + + /** + * @brief Read holding registers (Modbus function 0x03). + * + * Performs exactly ONE bus attempt — no internal retry (parity, + * docs/parity-checklist.md §5); recovery comes from the caller's read + * cadence. On failure returns false and getLastError() carries the code. + * + * @param deviceAddress Modbus slave address. + * @param startRegister First holding register address. + * @param count Number of consecutive registers to read. + * @param buffer Caller-owned array of at least `count` elements; filled + * only on success. + * @return true if all requested registers were read. + */ + virtual bool readHoldingRegisters(uint8_t deviceAddress, uint16_t startRegister, + uint16_t count, uint16_t* buffer) = 0; + + /** + * @brief Write a single holding register (Modbus function 0x06). + * + * Exactly one bus attempt, no retry (parity). Success means the + * addressed slave returned a well-formed FC06 response (address, + * function and CRC validated) — a sent-but-not-acknowledged write + * reports false. Implementations are NOT required to compare the echoed + * register/value byte-for-byte against the request (the legacy client + * did; see the parity-divergence note in EspModbusClient.cpp). + * + * @param deviceAddress Modbus slave address. + * @param registerAddress Holding register address. + * @param value Value to write. + * @return true if the slave acknowledged the write with a well-formed + * response. + */ + virtual bool writeSingleRegister(uint8_t deviceAddress, uint16_t registerAddress, + uint16_t value) = 0; + + /** + * @brief Error code of the most recent operation (0 = OK; table above). + */ + virtual int getLastError() = 0; + + /** + * @brief Set the response timeout for subsequent transfers. + * + * Parity default is 3000 ms (docs/parity-checklist.md §5). + * Implementations may apply the value at initialize() time only + * (EspModbusClient does — research.md R5). + * + * @param timeoutMs Timeout in milliseconds. + */ + virtual void setTimeout(uint32_t timeoutMs) = 0; + + /** + * @brief Cumulative transaction statistics. + * + * Every readHoldingRegisters()/writeSingleRegister() call increments + * exactly one of the two counters; reading the statistics increments + * neither. + * + * @param successCount Out: number of successful transfers. + * @param errorCount Out: number of failed transfers. + */ + virtual void getStatistics(uint32_t* successCount, uint32_t* errorCount) = 0; +}; + +#endif /* WATERINGSYSTEM_INTERFACES_IMODBUSCLIENT_H */ diff --git a/firmware/components/interfaces/include/interfaces/ISoilSensor.h b/firmware/components/interfaces/include/interfaces/ISoilSensor.h new file mode 100644 index 0000000..3c0f1aa --- /dev/null +++ b/firmware/components/interfaces/include/interfaces/ISoilSensor.h @@ -0,0 +1,135 @@ +// SPDX-FileCopyrightText: 2026 Cryptotomte +// SPDX-License-Identifier: AGPL-3.0-or-later +/** + * @file ISoilSensor.h + * @brief Interface for the RS485 Modbus soil sensor (7 quantities). + * + * Ported from the frozen Arduino firmware (include/sensors/ISoilSensor.h) + * with one deliberate trim: legacy setValidRange/isWithinValidRange are NOT + * ported — validation ranges are the fixed parity constants (moisture + * 0–100 %, temperature −40–80 °C, pH 3–9) and no caller in the legacy + * firmware ever changed them at runtime. Normative contract: + * specs/004-modbus-soil-sensor/contracts/interfaces.md; register map, + * scaling and error codes: specs/004-modbus-soil-sensor/data-model.md. + * + * Validity contract (FR-005): a false return from read() means the data is + * invalid. The getters keep returning the last-good values, so consumers + * MUST gate on the read result / getLastError() — never on value + * plausibility. + * + * Concurrency: implementations are unsynchronized by design; cross-task + * consumers (main loop + console REPL) wrap them in the LockedSoilSensor + * decorator, same pattern as LockedWaterPump/LockedConfigStore. + * + * Part of the header-only `interfaces` component: no IDF includes allowed. + */ + +#ifndef WATERINGSYSTEM_INTERFACES_ISOILSENSOR_H +#define WATERINGSYSTEM_INTERFACES_ISOILSENSOR_H + +/** + * @brief Soil sensor: atomic multi-quantity reads with parity validation. + */ +class ISoilSensor { +public: + virtual ~ISoilSensor() = default; + + /** + * @brief Prepare the sensor (brings up the underlying Modbus client if + * needed). Implementations may initialize lazily — read()/isAvailable()/ + * calibrate*() attempt initialization themselves when it has not + * happened yet — so calling this first is recommended, not required. + */ + virtual bool initialize() = 0; + + /** + * @brief Read all quantities in ONE 9-register transaction + * (registers 0x0000–0x0008), single bus attempt, no retry. + * + * The reading is atomic: either every value is decoded, scaled and + * range-validated and the getters are refreshed, or the call fails and + * the last-good getter values remain untouched. false means the data is + * INVALID (bus error, timeout, exception or range validation failure — + * see getLastError()); consumers gate on this result, never on how + * plausible the getter values look. + * + * @return true if a fully valid reading was taken. + */ + virtual bool read() = 0; + + /** + * @brief Probe sensor presence with a REAL 1-register bus read (parity). + * + * Every call performs an actual bus transaction — never a cached or + * derived state. Recovery from earlier failures is implicit: a sensor + * that responds again is available again. + */ + virtual bool isAvailable() = 0; + + /** + * @brief Error code of the most recent read()/initialize()/calibrate*() + * (0 = OK). + * + * isAvailable() never touches this code. Note that a calibrate*() + * returning true may still leave a nonzero code from the non-fatal + * calibration-register write (see the calibration block below). + * + * Same table as IModbusClient; code 5 (range validation failed) is + * produced by this layer, never by the client. See + * specs/004-modbus-soil-sensor/data-model.md. + */ + virtual int getLastError() = 0; + + // Values from the most recent successful read(). Before the FIRST + // successful read() they are meaningless placeholders, and after a + // failed read() they still hold the previous good reading — consumers + // gate on the read() result, never on the values. + + /// Soil moisture in percent (0–100). + virtual float getMoisture() = 0; + + /// Soil temperature in °C, signed (−40–80). + virtual float getTemperature() = 0; + + /** + * @brief Humidity in percent — identical to getMoisture() (parity). + * + * The sensor reports a single moisture/humidity quantity in register + * 0x0000; the legacy firmware exposed it under both names. + */ + virtual float getHumidity() = 0; + + /// Soil pH (validated 3–9). + virtual float getPH() = 0; + + /// Electrical conductivity in µS/cm (not range-enforced, parity). + virtual float getEC() = 0; + + /// Nitrogen content in mg/kg (not range-enforced, parity). + virtual float getNitrogen() = 0; + + /// Phosphorus content in mg/kg (not range-enforced, parity). + virtual float getPhosphorus() = 0; + + /// Potassium content in mg/kg (not range-enforced, parity). + virtual float getPotassium() = 0; + + // Calibration (CP1 decision A — legacy semantics). Each call computes a + // local correction factor (reference / current raw reading), applies it + // to every subsequent read, and best-effort writes the factor to the + // sensor's calibration register (0x0100/0x0101/0x0102) — a failed + // sensor-register write is NON-FATAL and does not fail the call: the + // call returns true while getLastError() reports the write error. + // Factors are RAM-only for now; persistence wired in PR-09/PR-11. + + /// Calibrate moisture against a reference value in percent. + virtual bool calibrateMoisture(float referenceValue) = 0; + + /// Calibrate pH against a reference value. + virtual bool calibratePH(float referenceValue) = 0; + + /// Calibrate EC against a reference value in µS/cm. + virtual bool calibrateEC(float referenceValue) = 0; +}; + +#endif /* WATERINGSYSTEM_INTERFACES_ISOILSENSOR_H */ diff --git a/firmware/components/sensors/CMakeLists.txt b/firmware/components/sensors/CMakeLists.txt new file mode 100644 index 0000000..224f082 --- /dev/null +++ b/firmware/components/sensors/CMakeLists.txt @@ -0,0 +1,28 @@ +# sensors — Modbus soil sensor driver layer. +# +# ModbusSoilSensor.cpp is pure C++ (decode/validation/calibration logic) and +# builds on every target, including the linux preview target used by the host +# test suite. EspModbusClient.cpp is the only hardware touchpoint (esp-modbus +# master + UART RS485 half-duplex + RX pull-up) and is excluded — together +# with its driver/esp-modbus dependencies — when building for linux +# (research.md R7, same mechanism as storage/actuators). +if(${IDF_TARGET} STREQUAL "linux") + idf_component_register( + SRCS "src/ModbusSoilSensor.cpp" + INCLUDE_DIRS "include" + REQUIRES interfaces board + ) +else() + # Target build only: the esp-modbus client. esp-modbus is a managed + # component (pinned in this component's idf_component.yml, rule-gated to + # non-linux targets; registered under its namespaced component name). + # PRIV: esp-modbus and driver headers appear only in + # src/EspModbusClient.cpp / private code, never in this component's + # public headers (same rule as storage's littlefs). + idf_component_register( + SRCS "src/ModbusSoilSensor.cpp" "src/EspModbusClient.cpp" + INCLUDE_DIRS "include" + REQUIRES interfaces board + PRIV_REQUIRES espressif__esp-modbus esp_driver_uart esp_driver_gpio + ) +endif() diff --git a/firmware/components/sensors/idf_component.yml b/firmware/components/sensors/idf_component.yml new file mode 100644 index 0000000..1c15502 --- /dev/null +++ b/firmware/components/sensors/idf_component.yml @@ -0,0 +1,12 @@ +# Managed dependencies of the sensors component, pinned exactly for +# reproducible builds (constitution III). +# +# esp-modbus is target-only: EspModbusClient.cpp is excluded from the linux +# host build (see CMakeLists.txt), so the dependency is rule-gated to keep +# esp-modbus out of the host-test dependency graph. +dependencies: + idf: ">=6.0.0" + espressif/esp-modbus: + version: "==2.1.2" + rules: + - if: "target != linux" diff --git a/firmware/components/sensors/include/sensors/EspModbusClient.h b/firmware/components/sensors/include/sensors/EspModbusClient.h new file mode 100644 index 0000000..f05d358 --- /dev/null +++ b/firmware/components/sensors/include/sensors/EspModbusClient.h @@ -0,0 +1,94 @@ +// SPDX-FileCopyrightText: 2026 Cryptotomte +// SPDX-License-Identifier: AGPL-3.0-or-later +/** + * @file EspModbusClient.h + * @brief IModbusClient over esp-modbus 2.1.2 (Modbus RTU master, RS485). + * + * ESP32-ONLY: excluded from the linux-target build together with the + * esp-modbus/driver dependencies (see this component's CMakeLists.txt). + * This is the sensors component's single hardware touchpoint — all + * business logic lives above IModbusClient (research.md R7). + * + * PRIV rule: esp-modbus and driver headers appear only in the .cpp, never + * here — the esp-modbus master handle is held as an opaque pointer. + * + * UART port and pins come from board/board.h inside the .cpp; the + * `#if BOARD_HAS_RS485_DE` direction-control difference between rev1 + * (RTS-driven DE) and rev2 (THVD1426 auto-direction) lives in exactly one + * place there (research.md R2). + * + * Concurrency: unsynchronized, like every base implementation in this + * codebase. As of feature 004 the client is only reached from the console + * REPL task (directly by `rs485test` and through LockedSoilSensor by + * `soil`); a second consumer task would need a locking wrapper. PR-11's + * main-loop reader adds exactly that second task — `rs485test`'s raw access + * must be routed through a locked client wrapper (or the sensor) then. + */ + +#ifndef WATERINGSYSTEM_SENSORS_ESPMODBUSCLIENT_H +#define WATERINGSYSTEM_SENSORS_ESPMODBUSCLIENT_H + +#include + +#include "interfaces/IModbusClient.h" + +/** + * @brief Modbus RTU master on the board's RS485 UART (9600 8N1, parity). + */ +class EspModbusClient : public IModbusClient { +public: + /// Parity response timeout (docs/parity-checklist.md §5). + static constexpr uint32_t kDefaultTimeoutMs = 3000; + + EspModbusClient() = default; + + /// Tears down the esp-modbus master stack (mbc_master_delete). + ~EspModbusClient() override; + + EspModbusClient(const EspModbusClient&) = delete; + EspModbusClient& operator=(const EspModbusClient&) = delete; + + /** + * @brief Bring up the esp-modbus master on the RS485 UART. + * + * create → uart_set_pin (RTS = DE pin iff BOARD_HAS_RS485_DE) → start → + * uart_set_mode(RS485 half-duplex) → RX pull-up (FW-2). Idempotent; + * on failure every partially-created resource is torn down again. + */ + bool initialize() override; + + bool readHoldingRegisters(uint8_t deviceAddress, uint16_t startRegister, + uint16_t count, uint16_t* buffer) override; + + bool writeSingleRegister(uint8_t deviceAddress, uint16_t registerAddress, + uint16_t value) override; + + int getLastError() override; + + /** + * @brief Set the response timeout for subsequent transfers. + * + * esp-modbus 2.1.2 exposes no documented runtime timeout setter, so the + * value is applied at initialize() time only (research.md R5, risk + * documented): calls after initialize() are stored but take effect only + * on a future re-initialization. No caller changes the timeout at + * runtime today. + */ + void setTimeout(uint32_t timeoutMs) override; + + void getStatistics(uint32_t* successCount, uint32_t* errorCount) override; + +private: + /// One bus transaction via mbc_master_send_request + bookkeeping. + bool sendRequest(uint8_t deviceAddress, uint8_t command, + uint16_t registerAddress, uint16_t count, void* data); + + void* mbHandle_ = nullptr; ///< opaque esp-modbus master handle + bool initialized_ = false; + int lastError_ = 0; + uint32_t timeoutMs_ = kDefaultTimeoutMs; + uint32_t successCount_ = 0; + uint32_t errorCount_ = 0; +}; + +#endif /* WATERINGSYSTEM_SENSORS_ESPMODBUSCLIENT_H */ diff --git a/firmware/components/sensors/include/sensors/LockedSoilSensor.h b/firmware/components/sensors/include/sensors/LockedSoilSensor.h new file mode 100644 index 0000000..55662f5 --- /dev/null +++ b/firmware/components/sensors/include/sensors/LockedSoilSensor.h @@ -0,0 +1,151 @@ +// SPDX-FileCopyrightText: 2026 Cryptotomte +// SPDX-License-Identifier: AGPL-3.0-or-later +/** + * @file LockedSoilSensor.h + * @brief Mutex-serializing ISoilSensor decorator (header-only). + * + * WHY THIS EXISTS: the soil sensor will be reached from more than one + * FreeRTOS task — the diag console REPL task issues soil/calibration + * commands while application tasks (watering controller in PR-11) run + * periodic reads. ModbusSoilSensor itself is deliberately unsynchronized + * pure C++ (host-testable), so its plain float members would race: read() + * publishes eight values member-by-member, and a concurrent getter could + * observe a half-published reading (fresh moisture with stale pH). This + * decorator wraps an ISoilSensor and takes a mutex around every interface + * call, serializing all access (pattern of actuators/LockedWaterPump.h and + * storage/LockedConfigStore.h). + * + * USAGE RULE: once a sensor is wrapped, the underlying sensor must ONLY be + * accessed through the wrapper — every call site (boot wiring, console + * registration, controllers, ...) goes through the LockedSoilSensor, never + * through the wrapped object directly. + * + * SCOPE: this decorator provides PER-CALL atomicity only, not cross-call. + * A read-then-get sequence spanning two calls (read() then getMoisture()) + * is NOT protected against an interleaving read() from another task in + * between — another task may refresh (or invalidate) the values first. + * Such sequences need higher-level coordination (a caller-held lock or + * single-owner task). + * + * Pure C++ ( is available via pthread on ESP-IDF and on the linux + * preview target), so the decorator is host-testable. + */ + +#ifndef WATERINGSYSTEM_SENSORS_LOCKEDSOILSENSOR_H +#define WATERINGSYSTEM_SENSORS_LOCKEDSOILSENSOR_H + +#include + +#include "interfaces/ISoilSensor.h" + +/** + * @brief ISoilSensor decorator that serializes every call with a mutex. + * + * Composition, not inheritance from a concrete sensor: the base class + * stays pure (no locking) and the host tests are unchanged. The wrapped + * sensor must outlive this object. + */ +class LockedSoilSensor : public ISoilSensor { +public: + /// Wrap @p sensor; the wrapped sensor must outlive this object. + explicit LockedSoilSensor(ISoilSensor& sensor) : sensor_(sensor) {} + + LockedSoilSensor(const LockedSoilSensor&) = delete; + LockedSoilSensor& operator=(const LockedSoilSensor&) = delete; + + bool initialize() override + { + std::lock_guard lock(mutex_); + return sensor_.initialize(); + } + + bool read() override + { + std::lock_guard lock(mutex_); + return sensor_.read(); + } + + bool isAvailable() override + { + std::lock_guard lock(mutex_); + return sensor_.isAvailable(); + } + + int getLastError() override + { + std::lock_guard lock(mutex_); + return sensor_.getLastError(); + } + + float getMoisture() override + { + std::lock_guard lock(mutex_); + return sensor_.getMoisture(); + } + + float getTemperature() override + { + std::lock_guard lock(mutex_); + return sensor_.getTemperature(); + } + + float getHumidity() override + { + std::lock_guard lock(mutex_); + return sensor_.getHumidity(); + } + + float getPH() override + { + std::lock_guard lock(mutex_); + return sensor_.getPH(); + } + + float getEC() override + { + std::lock_guard lock(mutex_); + return sensor_.getEC(); + } + + float getNitrogen() override + { + std::lock_guard lock(mutex_); + return sensor_.getNitrogen(); + } + + float getPhosphorus() override + { + std::lock_guard lock(mutex_); + return sensor_.getPhosphorus(); + } + + float getPotassium() override + { + std::lock_guard lock(mutex_); + return sensor_.getPotassium(); + } + + bool calibrateMoisture(float referenceValue) override + { + std::lock_guard lock(mutex_); + return sensor_.calibrateMoisture(referenceValue); + } + + bool calibratePH(float referenceValue) override + { + std::lock_guard lock(mutex_); + return sensor_.calibratePH(referenceValue); + } + + bool calibrateEC(float referenceValue) override + { + std::lock_guard lock(mutex_); + return sensor_.calibrateEC(referenceValue); + } + +private: + ISoilSensor& sensor_; + mutable std::mutex mutex_; +}; + +#endif /* WATERINGSYSTEM_SENSORS_LOCKEDSOILSENSOR_H */ diff --git a/firmware/components/sensors/include/sensors/ModbusSoilSensor.h b/firmware/components/sensors/include/sensors/ModbusSoilSensor.h new file mode 100644 index 0000000..0b2f279 --- /dev/null +++ b/firmware/components/sensors/include/sensors/ModbusSoilSensor.h @@ -0,0 +1,143 @@ +// SPDX-FileCopyrightText: 2026 Cryptotomte +// SPDX-License-Identifier: AGPL-3.0-or-later +/** + * @file ModbusSoilSensor.h + * @brief Pure C++ soil sensor logic over an injected IModbusClient. + * + * Ported from the frozen Arduino firmware (src/sensors/ModbusSoilSensor.cpp, + * read-only reference): register map, decode/scaling (incl. signed + * temperature), fixed parity validation ranges, real-bus-read availability + * probe and the legacy calibration semantics. Register map, scaling and + * error codes are normative in specs/004-modbus-soil-sensor/data-model.md. + * + * This class contains NO hardware access — every bus transaction goes + * through the injected IModbusClient, so the decode/validation/calibration + * logic is compiled and unit-tested on the IDF linux preview target against + * MockModbusClient (constitution II). + * + * Concurrency: unsynchronized by design (host-testable); cross-task + * consumers (console REPL + future main-loop controller) wrap it in + * LockedSoilSensor and access it only through the wrapper. + */ + +#ifndef WATERINGSYSTEM_SENSORS_MODBUSSOILSENSOR_H +#define WATERINGSYSTEM_SENSORS_MODBUSSOILSENSOR_H + +#include + +#include "interfaces/IModbusClient.h" +#include "interfaces/ISoilSensor.h" + +/** + * @brief ISoilSensor over Modbus RTU holding registers (data-model.md). + * + * One read() = one 9-register transaction (0x0000–0x0008), decoded, scaled + * and range-validated atomically: on any failure the last-good getter + * values remain untouched and getLastError() carries the cause. + */ +class ModbusSoilSensor : public ISoilSensor { +public: + /// Parity slave address (docs/parity-checklist.md §5). + static constexpr uint8_t kDefaultDeviceAddress = 0x01; + + /** + * @brief Construct the sensor over an injected Modbus client. + * + * @param client Modbus master used for every transaction; must outlive + * this object (same injection style as WaterPump's + * ITimeProvider). + * @param deviceAddress Modbus slave address of the sensor. + */ + explicit ModbusSoilSensor(IModbusClient& client, + uint8_t deviceAddress = kDefaultDeviceAddress); + + ~ModbusSoilSensor() override = default; + + ModbusSoilSensor(const ModbusSoilSensor&) = delete; + ModbusSoilSensor& operator=(const ModbusSoilSensor&) = delete; + + // ISoilSensor + bool initialize() override; + bool read() override; + bool isAvailable() override; + int getLastError() override; + + float getMoisture() override; + float getTemperature() override; + float getHumidity() override; + float getPH() override; + float getEC() override; + float getNitrogen() override; + float getPhosphorus() override; + float getPotassium() override; + + // Calibration (legacy-exact port, research.md R8); host-tested in + // test_soil_sensor.cpp (calibration suite). + bool calibrateMoisture(float referenceValue) override; + bool calibratePH(float referenceValue) override; + bool calibrateEC(float referenceValue) override; + +private: + // Register map (data-model.md; legacy include/sensors/ModbusSoilSensor.h). + static constexpr uint16_t kRegHumidity = 0x0000; ///< 0.1 % (= moisture) + static constexpr uint16_t kRegTemperature = 0x0001; ///< 0.1 °C, signed + static constexpr uint16_t kRegEc = 0x0002; ///< 1 µS/cm + static constexpr uint16_t kRegPh = 0x0003; ///< 0.1 pH + static constexpr uint16_t kRegNitrogen = 0x0004; ///< 1 mg/kg + static constexpr uint16_t kRegPhosphorus = 0x0005; ///< 1 mg/kg + static constexpr uint16_t kRegPotassium = 0x0006; ///< 1 mg/kg + // 0x0007 (salinity) and 0x0008 (TDS) are read but not exposed (parity). + + /// Calibration factor registers (best-effort writes, factor ×100). + static constexpr uint16_t kRegMoistureCalib = 0x0100; + static constexpr uint16_t kRegPhCalib = 0x0101; + static constexpr uint16_t kRegEcCalib = 0x0102; + + /// One transaction covers registers 0x0000–0x0008. + static constexpr uint16_t kReadRegisterCount = 9; + + // Fixed parity validation ranges (the legacy setValidRange defaults; + // runtime range changes were trimmed — see interfaces/ISoilSensor.h). + static constexpr float kMoistureMin = 0.0f; + static constexpr float kMoistureMax = 100.0f; + static constexpr float kTemperatureMin = -40.0f; + static constexpr float kTemperatureMax = 80.0f; + static constexpr float kPhMin = 3.0f; + static constexpr float kPhMax = 9.0f; + + /** + * @brief Shared legacy calibration flow (legacy :207-322, three + * near-identical bodies folded into one). + * + * Reads one raw register, computes factor = reference / (raw / rawScale), + * stores it into @p factor and best-effort writes factor ×100 to + * @p calibRegister (a failed write is NON-FATAL: logged, lastError set, + * call still succeeds — parity). + */ + bool calibrate(uint16_t rawRegister, float rawScale, + uint16_t calibRegister, float& factor, + float referenceValue, const char* quantity); + + IModbusClient& client_; + uint8_t deviceAddress_; + bool initialized_ = false; + int lastError_ = 0; + + // Last-good reading (published only by a fully successful read()). + float moisture_ = 0.0f; + float temperature_ = 0.0f; + float humidity_ = 0.0f; + float ph_ = 0.0f; + float ec_ = 0.0f; + float nitrogen_ = 0.0f; + float phosphorus_ = 0.0f; + float potassium_ = 0.0f; + + // Calibration factors (RAM-only for now; persistence wired in + // PR-09/PR-11). + float moistureCalibrationFactor_ = 1.0f; + float phCalibrationFactor_ = 1.0f; + float ecCalibrationFactor_ = 1.0f; +}; + +#endif /* WATERINGSYSTEM_SENSORS_MODBUSSOILSENSOR_H */ diff --git a/firmware/components/sensors/include/sensors/testing/MockModbusClient.h b/firmware/components/sensors/include/sensors/testing/MockModbusClient.h new file mode 100644 index 0000000..90e6ab4 --- /dev/null +++ b/firmware/components/sensors/include/sensors/testing/MockModbusClient.h @@ -0,0 +1,217 @@ +// SPDX-FileCopyrightText: 2026 Cryptotomte +// SPDX-License-Identifier: AGPL-3.0-or-later +/** + * @file MockModbusClient.h + * @brief Scriptable IModbusClient test double (header-only). + * + * Serves the ModbusSoilSensor host tests: script register payloads per + * (address, startRegister, count), queue per-call outcomes (success, + * timeout, bus error, slave exception) for fail-then-recover scenarios, + * and assert on the recorded call log (no-retry invariant, real-read + * availability probe, calibration register writes) and on the + * one-increment-per-call statistics. Never compiled into target builds + * (only included from test code). No IDF includes. + */ + +#ifndef WATERINGSYSTEM_SENSORS_TESTING_MOCKMODBUSCLIENT_H +#define WATERINGSYSTEM_SENSORS_TESTING_MOCKMODBUSCLIENT_H + +#include +#include +#include + +#include "interfaces/IModbusClient.h" + +/** + * @brief IModbusClient over scripted payloads, instrumented for tests. + * + * Outcome selection per read/write call: the front of `outcomeQueue` is + * consumed if non-empty, otherwise `defaultOutcome` applies (error-code + * convention: kOk = success). Reads that succeed return the payload + * scripted for their exact (address, startRegister, count) key, else the + * default payload, else all-zero registers. + */ +class MockModbusClient : public IModbusClient { +public: + // Error codes (data-model.md table) as readable test constants. + static constexpr int kOk = 0; + static constexpr int kErrNotInitialized = 1; + static constexpr int kErrBus = 2; ///< CRC/framing/truncated + static constexpr int kErrTimeout = 3; + static constexpr int kErrExceptionBase = 100; ///< 100 + n for slave exception n + + /// One recorded bus call (in `calls`, chronological). + struct Call { + enum class Type { Read, Write }; + Type type; + uint8_t deviceAddress; + uint16_t startRegister; ///< registerAddress for writes + uint16_t count; ///< register count (reads); 1 for writes + uint16_t value; ///< written value (writes); 0 for reads + bool succeeded; ///< outcome reported to the caller + }; + + // Instrumentation (public, MockConfigStore style). + std::vector calls; ///< every read/write, in call order + std::vector timeoutCalls; ///< every setTimeout() argument + int initializeCalls = 0; + bool initializeResult = true; ///< false: initialize() fails + int defaultOutcome = kOk; ///< used when outcomeQueue is empty + + // -- Scripting -------------------------------------------------------- + + /** + * @brief Script the payload returned for reads matching exactly + * (deviceAddress, startRegister, count == values.size()). + * Re-scripting the same key replaces the previous payload. + */ + void setRegisters(uint8_t deviceAddress, uint16_t startRegister, + std::vector values) + { + for (auto& entry : scripted_) { + if (entry.deviceAddress == deviceAddress && + entry.startRegister == startRegister && + entry.values.size() == values.size()) { + entry.values = std::move(values); + return; + } + } + scripted_.push_back({deviceAddress, startRegister, std::move(values)}); + } + + /** + * @brief Fallback payload for successful reads with no exact-key script; + * the first `count` values are returned (must hold at least `count`, + * shorter defaults fall through to all-zero registers). + */ + void setDefaultRegisters(std::vector values) + { + defaultRegisters_ = std::move(values); + } + + /** + * @brief Queue the outcome for the NEXT read/write call (FIFO). + * + * kOk = success; kErrTimeout, kErrBus, kErrExceptionBase + n, ... force + * that failure. Queue e.g. {kErrTimeout, kOk} for a fail-then-recover + * scenario. When the queue is empty, defaultOutcome applies. + */ + void queueOutcome(int errorCode) { outcomeQueue_.push_back(errorCode); } + + // -- IModbusClient ----------------------------------------------------- + + bool initialize() override + { + ++initializeCalls; + initialized_ = initializeResult; + return initialized_; + } + + bool readHoldingRegisters(uint8_t deviceAddress, uint16_t startRegister, + uint16_t count, uint16_t* buffer) override + { + Call call{Call::Type::Read, deviceAddress, startRegister, count, 0, false}; + if (!initialized_) { + return finish(call, kErrNotInitialized); + } + const int outcome = nextOutcome(); + if (outcome != kOk) { + return finish(call, outcome); + } + fillPayload(deviceAddress, startRegister, count, buffer); + return finish(call, kOk); + } + + bool writeSingleRegister(uint8_t deviceAddress, uint16_t registerAddress, + uint16_t value) override + { + Call call{Call::Type::Write, deviceAddress, registerAddress, 1, value, false}; + if (!initialized_) { + return finish(call, kErrNotInitialized); + } + return finish(call, nextOutcome()); + } + + int getLastError() override { return lastError_; } + + void setTimeout(uint32_t timeoutMs) override + { + timeoutCalls.push_back(timeoutMs); + } + + void getStatistics(uint32_t* successCount, uint32_t* errorCount) override + { + if (successCount != nullptr) { + *successCount = successCount_; + } + if (errorCount != nullptr) { + *errorCount = errorCount_; + } + } + +private: + struct ScriptedRead { + uint8_t deviceAddress; + uint16_t startRegister; + std::vector values; + }; + + int nextOutcome() + { + if (outcomeQueue_.empty()) { + return defaultOutcome; + } + const int outcome = outcomeQueue_.front(); + outcomeQueue_.erase(outcomeQueue_.begin()); + return outcome; + } + + void fillPayload(uint8_t deviceAddress, uint16_t startRegister, + uint16_t count, uint16_t* buffer) const + { + for (const auto& entry : scripted_) { + if (entry.deviceAddress == deviceAddress && + entry.startRegister == startRegister && + entry.values.size() == count) { + for (uint16_t i = 0; i < count; ++i) { + buffer[i] = entry.values[i]; + } + return; + } + } + if (defaultRegisters_.has_value() && defaultRegisters_->size() >= count) { + for (uint16_t i = 0; i < count; ++i) { + buffer[i] = (*defaultRegisters_)[i]; + } + return; + } + for (uint16_t i = 0; i < count; ++i) { + buffer[i] = 0; + } + } + + /// Record the call, apply the one-increment-per-call statistics + /// contract, set lastError, and return the call result. + bool finish(Call& call, int errorCode) + { + call.succeeded = (errorCode == kOk); + calls.push_back(call); + lastError_ = errorCode; + if (call.succeeded) { + ++successCount_; + } else { + ++errorCount_; + } + return call.succeeded; + } + + bool initialized_ = false; + int lastError_ = kOk; + uint32_t successCount_ = 0; + uint32_t errorCount_ = 0; + std::vector outcomeQueue_; + std::vector scripted_; + std::optional> defaultRegisters_; +}; + +#endif /* WATERINGSYSTEM_SENSORS_TESTING_MOCKMODBUSCLIENT_H */ diff --git a/firmware/components/sensors/include/sensors/testing/MockSoilSensor.h b/firmware/components/sensors/include/sensors/testing/MockSoilSensor.h new file mode 100644 index 0000000..5389959 --- /dev/null +++ b/firmware/components/sensors/include/sensors/testing/MockSoilSensor.h @@ -0,0 +1,115 @@ +// SPDX-FileCopyrightText: 2026 Cryptotomte +// SPDX-License-Identifier: AGPL-3.0-or-later +/** + * @file MockSoilSensor.h + * @brief Scriptable ISoilSensor test double (header-only). + * + * Serves the host tests of soil-sensor CONSUMERS (watering controller / + * sensor manager, PR-11): set the seven quantity values and the + * initialize()/read()/isAvailable() results, script the error code, and + * assert on the call counters. The sensor's own decode/validation logic is + * tested against the REAL ModbusSoilSensor via MockModbusClient — never + * through this mock. Never compiled into target builds (only included from + * test code). No IDF includes. + */ + +#ifndef WATERINGSYSTEM_SENSORS_TESTING_MOCKSOILSENSOR_H +#define WATERINGSYSTEM_SENSORS_TESTING_MOCKSOILSENSOR_H + +#include + +#include "interfaces/ISoilSensor.h" + +/** + * @brief ISoilSensor returning scripted values, instrumented for tests. + * + * All state is public (MockModbusClient/MockConfigStore style): assign the + * result fields and quantity values before driving the consumer, then + * assert on the counters/recorded arguments. The getters serve the scripted + * values unconditionally — like the real sensor after a failed read() they + * keep returning the last values, so consumers must gate on the read() + * result / lastError, never on value plausibility (FR-005). + */ +class MockSoilSensor : public ISoilSensor { +public: + // -- Scripted results --------------------------------------------------- + + bool initializeResult = true; ///< returned by initialize() + bool readResult = true; ///< returned by read() + bool isAvailableResult = true; ///< returned by isAvailable() + bool calibrateResult = true; ///< returned by all three calibrate*() + int lastError = 0; ///< returned by getLastError() + + // -- Scripted quantity values (served by the getters) -------------------- + + float moisture = 0.0f; + float temperature = 0.0f; + float humidity = 0.0f; ///< the real sensor keeps this ≡ moisture (parity) + float ph = 0.0f; + float ec = 0.0f; + float nitrogen = 0.0f; + float phosphorus = 0.0f; + float potassium = 0.0f; + + // -- Instrumentation ----------------------------------------------------- + + int initializeCalls = 0; + int readCalls = 0; + int isAvailableCalls = 0; + /// Every calibrate*() reference-value argument, in call order (the + /// vector size doubles as the per-quantity call counter). + std::vector calibrateMoistureCalls; + std::vector calibratePhCalls; + std::vector calibrateEcCalls; + + // -- ISoilSensor --------------------------------------------------------- + + bool initialize() override + { + ++initializeCalls; + return initializeResult; + } + + bool read() override + { + ++readCalls; + return readResult; + } + + bool isAvailable() override + { + ++isAvailableCalls; + return isAvailableResult; + } + + int getLastError() override { return lastError; } + + float getMoisture() override { return moisture; } + float getTemperature() override { return temperature; } + float getHumidity() override { return humidity; } + float getPH() override { return ph; } + float getEC() override { return ec; } + float getNitrogen() override { return nitrogen; } + float getPhosphorus() override { return phosphorus; } + float getPotassium() override { return potassium; } + + bool calibrateMoisture(float referenceValue) override + { + calibrateMoistureCalls.push_back(referenceValue); + return calibrateResult; + } + + bool calibratePH(float referenceValue) override + { + calibratePhCalls.push_back(referenceValue); + return calibrateResult; + } + + bool calibrateEC(float referenceValue) override + { + calibrateEcCalls.push_back(referenceValue); + return calibrateResult; + } +}; + +#endif /* WATERINGSYSTEM_SENSORS_TESTING_MOCKSOILSENSOR_H */ diff --git a/firmware/components/sensors/src/EspModbusClient.cpp b/firmware/components/sensors/src/EspModbusClient.cpp new file mode 100644 index 0000000..d62d675 --- /dev/null +++ b/firmware/components/sensors/src/EspModbusClient.cpp @@ -0,0 +1,255 @@ +// SPDX-FileCopyrightText: 2026 Cryptotomte +// SPDX-License-Identifier: AGPL-3.0-or-later +/** + * @file EspModbusClient.cpp + * @brief esp-modbus 2.1.2 Modbus RTU master implementation. + * + * Target-only translation unit (excluded from the linux host build). The + * only file in the sensors component that touches esp-modbus, UART and + * GPIO APIs. Setup sequence and RS485 half-duplex rationale: research.md + * R1/R2/R3/R4. + */ + +#include "sensors/EspModbusClient.h" + +#include "board/board.h" +#include "driver/gpio.h" +#include "driver/uart.h" +#include "esp_log.h" + +// esp-modbus umbrella header (managed component, pinned ==2.1.2). +#include "mbcontroller.h" + +static const char *TAG = "esp_modbus_client"; + +namespace { + +/** + * @brief Map an esp-modbus/esp_err_t result onto the IModbusClient error + * table (data-model.md, research.md R6). + * + * Kept as ONE helper so the mapping can be refined in a single place. + * + * parity divergence R6: esp-modbus 2.1.2's mbc_master_send_request does not + * surface the Modbus slave exception code to the caller — exceptions and + * other invalid responses come back as generic esp_err_t failures. The + * legacy 100+n exception granularity is therefore collapsed onto error 2 + * (bus/communication error) here. The binding FR-010 requirement — distinct + * from timeout — still holds (timeout maps to 3). + */ +int map_esp_err(esp_err_t err) +{ + switch (err) { + case ESP_OK: + return 0; + case ESP_ERR_TIMEOUT: + return 3; // No response within the configured timeout. + case ESP_ERR_INVALID_STATE: + return 1; // Stack not initialized/started. + default: + return 2; // CRC/framing/invalid response/slave exception class. + } +} + +/** + * @brief mbc_master_delete with a logged (never discarded) failure; nulls + * the handle. Shared by the initialize() failure paths and the destructor. + */ +void delete_master_logged(void*& handle) +{ + const esp_err_t err = mbc_master_delete(handle); + if (err != ESP_OK) { + ESP_LOGE(TAG, "mbc_master_delete failed: %s", esp_err_to_name(err)); + } + handle = nullptr; +} + +} // namespace + +EspModbusClient::~EspModbusClient() +{ + if (mbHandle_ != nullptr) { + delete_master_logged(mbHandle_); + } +} + +bool EspModbusClient::initialize() +{ + if (initialized_) { + return true; + } + + // R1: esp-modbus 2.x serial master, Modbus RTU 9600 8N1, parity + // response timeout (3000 ms default, member set via setTimeout()). + mb_communication_info_t comm_info = {}; + comm_info.ser_opts.port = static_cast(BOARD_RS485_UART_PORT); + comm_info.ser_opts.mode = MB_RTU; + comm_info.ser_opts.baudrate = 9600; + comm_info.ser_opts.data_bits = UART_DATA_8_BITS; + comm_info.ser_opts.stop_bits = UART_STOP_BITS_1; + comm_info.ser_opts.parity = MB_PARITY_NONE; + comm_info.ser_opts.uid = 0; // master + comm_info.ser_opts.response_tout_ms = timeoutMs_; + + esp_err_t err = mbc_master_create_serial(&comm_info, &mbHandle_); + if (err != ESP_OK || mbHandle_ == nullptr) { + ESP_LOGE(TAG, "mbc_master_create_serial failed: %s", + esp_err_to_name(err)); + mbHandle_ = nullptr; + lastError_ = 1; + return false; + } + + // R2: UART pins from board.h. rev1 drives the transceiver DE via the + // UART RTS line (hardware-timed around each frame); rev2 has no DE pin + // (THVD1426 auto-direction) so RTS stays unrouted. This #if is the one + // place the board direction-control difference exists. + err = uart_set_pin(static_cast(BOARD_RS485_UART_PORT), + BOARD_PIN_RS485_TX, BOARD_PIN_RS485_RX, +#if BOARD_HAS_RS485_DE + BOARD_PIN_RS485_DE, +#else + UART_PIN_NO_CHANGE, +#endif + UART_PIN_NO_CHANGE); + if (err != ESP_OK) { + ESP_LOGE(TAG, "uart_set_pin failed: %s", esp_err_to_name(err)); + delete_master_logged(mbHandle_); + lastError_ = 1; + return false; + } + + err = mbc_master_start(mbHandle_); + if (err != ESP_OK) { + ESP_LOGE(TAG, "mbc_master_start failed: %s", esp_err_to_name(err)); + delete_master_logged(mbHandle_); + lastError_ = 1; + return false; + } + + // R2/R3: RS485 half-duplex mode on BOTH boards. Besides the automatic + // RTS/DE framing on rev1, the TX-gated receive path suppresses the rev2 + // THVD1426 TX echo (RE̅ grounded, receiver always on): echo bytes are + // physically simultaneous with transmission and never reach the driver. + // The esp-modbus RTU T3.5 frame resynchronization is the fallback for + // residual tail bytes. To be electrically verified on rev2 (HIL, PR-14). + err = uart_set_mode(static_cast(BOARD_RS485_UART_PORT), + UART_MODE_RS485_HALF_DUPLEX); + if (err != ESP_OK) { + ESP_LOGE(TAG, "uart_set_mode(RS485 half-duplex) failed: %s", + esp_err_to_name(err)); + delete_master_logged(mbHandle_); + lastError_ = 1; + return false; + } + + // R4 (FW-2): internal pull-up on the RX pin, unconditionally on both + // boards. On rev2 the THVD1426 SHDN̅ tracks SENS_PWR_EN — RO goes hi-Z + // when the sensor power domain is off and the RX GPIO would otherwise + // float into the UART, producing garbage bytes. Harmless on rev1 + // (ADM3485 RO drives push-pull). + err = gpio_pullup_en(static_cast(BOARD_PIN_RS485_RX)); + if (err != ESP_OK) { + ESP_LOGE(TAG, "gpio_pullup_en(RX) failed: %s", esp_err_to_name(err)); + delete_master_logged(mbHandle_); + lastError_ = 1; + return false; + } + + ESP_LOGI(TAG, + "Modbus RTU master up: UART%d 9600 8N1, timeout %lu ms, " + "RS485 half-duplex", + BOARD_RS485_UART_PORT, static_cast(timeoutMs_)); + initialized_ = true; + lastError_ = 0; + return true; +} + +bool EspModbusClient::sendRequest(uint8_t deviceAddress, uint8_t command, + uint16_t registerAddress, uint16_t count, + void* data) +{ + // Exactly one bus attempt per call, exactly one counter increment + // (interface contract; no retry — parity). + if (!initialized_) { + lastError_ = 1; + ++errorCount_; + return false; + } + + mb_param_request_t request = {}; + request.slave_addr = deviceAddress; + request.command = command; + request.reg_start = registerAddress; + request.reg_size = count; + + const esp_err_t err = mbc_master_send_request(mbHandle_, &request, data); + lastError_ = map_esp_err(err); + if (err != ESP_OK) { + ++errorCount_; + ESP_LOGW(TAG, + "request failed: cmd=0x%02x addr=%u reg=0x%04x n=%u: %s " + "(error %d)", + static_cast(command), + static_cast(deviceAddress), + static_cast(registerAddress), + static_cast(count), esp_err_to_name(err), + lastError_); + return false; + } + ++successCount_; + return true; +} + +bool EspModbusClient::readHoldingRegisters(uint8_t deviceAddress, + uint16_t startRegister, + uint16_t count, uint16_t* buffer) +{ + // Modbus function 0x03: esp-modbus fills the caller buffer with the + // register values only on success. + return sendRequest(deviceAddress, 0x03, startRegister, count, buffer); +} + +bool EspModbusClient::writeSingleRegister(uint8_t deviceAddress, + uint16_t registerAddress, + uint16_t value) +{ + // parity divergence (write-echo): esp-modbus 2.1.2 validates the FC06 + // response framing (length/address/function/CRC) but performs NO + // comparison of the echoed register/value against the request. The + // legacy client verified the full 8-byte echo + // (src/communication/SP3485ModbusClient.cpp:266-355, + // docs/parity-checklist.md §5). Same mechanism as R6 above: the gap is + // inside mbc_master_send_request, documented rather than patched. + uint16_t writeValue = value; + return sendRequest(deviceAddress, 0x06, registerAddress, 1, &writeValue); +} + +int EspModbusClient::getLastError() +{ + return lastError_; +} + +void EspModbusClient::setTimeout(uint32_t timeoutMs) +{ + timeoutMs_ = timeoutMs; + if (initialized_) { + // R5: no documented runtime timeout setter in esp-modbus 2.1.2 — + // the stored value takes effect on a future re-initialization only. + ESP_LOGW(TAG, + "setTimeout(%lu) after initialize(): applied at next " + "initialization only", + static_cast(timeoutMs)); + } +} + +void EspModbusClient::getStatistics(uint32_t* successCount, + uint32_t* errorCount) +{ + if (successCount != nullptr) { + *successCount = successCount_; + } + if (errorCount != nullptr) { + *errorCount = errorCount_; + } +} diff --git a/firmware/components/sensors/src/ModbusSoilSensor.cpp b/firmware/components/sensors/src/ModbusSoilSensor.cpp new file mode 100644 index 0000000..da02969 --- /dev/null +++ b/firmware/components/sensors/src/ModbusSoilSensor.cpp @@ -0,0 +1,319 @@ +// SPDX-FileCopyrightText: 2026 Cryptotomte +// SPDX-License-Identifier: AGPL-3.0-or-later +/** + * @file ModbusSoilSensor.cpp + * @brief Soil sensor decode/validation/calibration logic (pure C++). + * + * Uses ESP_LOG only — the log component is simulated on the IDF linux + * preview target, so this file builds and runs in the host test suite + * (same rule as actuators/WaterPump.cpp). + * + * Port reference: src/sensors/ModbusSoilSensor.cpp (frozen Arduino + * firmware, read-only). Legacy behaviors confirmed and ported exactly: + * + * - The moisture calibration factor is NOT applied in read(): legacy + * computes/stores/writes it in calibrateMoisture() but read() publishes + * humidity = raw / 10 with the comment "No calibration factor for + * humidity yet", and moisture = humidity. Ported as-is (research.md R8 + * "moisture factor exists in legacy and is ported as-is"; the data-model + * table's "× moisture factor" describes the factor's existence, not a + * legacy read-path multiplication). + * - pH and EC factors ARE applied in read(), and validation runs on the + * FACTORED values (legacy :103-128 validates after multiplying). + * - Validation covers moisture, temperature and pH only; EC/N/P/K are not + * range-enforced in read() (parity — legacy checks exactly moisture, + * temperature, humidity, ph; the humidity check is dropped here because + * humidity ≡ moisture makes it redundant). + * - Lazy initialization: read()/isAvailable()/calibrate*() attempt + * initialize() when not yet initialized (legacy :75-81, :134-137). + * - The availability/initialization probe reads ONE register at 0x0000 + * (legacy REG_HUMIDITY, :65 and :140) — not 0x0001. + * + * Error codes follow the new normative table (data-model.md) instead of the + * legacy ad-hoc codes 4/6..14: client failures propagate the client's error + * (2/3/100+n), range validation failure is 5 (parity), and the legacy + * "raw value too low for calibration" (7/10/13) maps onto 5 as well. + */ + +#include "sensors/ModbusSoilSensor.h" + +#include + +#include "esp_log.h" + +static const char *TAG = "soilsensor"; + +ModbusSoilSensor::ModbusSoilSensor(IModbusClient& client, uint8_t deviceAddress) + : client_(client), + deviceAddress_(deviceAddress) +{ +} + +bool ModbusSoilSensor::initialize() +{ + // Idempotent, like the legacy sensor (:49-51). + if (initialized_) { + return true; + } + + if (!client_.initialize()) { + // Legacy code 2 ("Modbus client initialization failed") — coincides + // with the normative table's bus/communication error. + lastError_ = 2; + ESP_LOGE(TAG, "initialize failed: Modbus client init error"); + return false; + } + + // Verify the sensor answers: one real register read (legacy :63-68; + // legacy hardcoded error 3, here the client's actual error code — a + // silent sensor reports 3 = timeout either way). + uint16_t testRegister = 0; + if (!client_.readHoldingRegisters(deviceAddress_, kRegHumidity, 1, + &testRegister)) { + lastError_ = client_.getLastError(); + ESP_LOGW(TAG, "initialize failed: sensor probe error %d", lastError_); + return false; + } + + initialized_ = true; + lastError_ = 0; + return true; +} + +bool ModbusSoilSensor::read() +{ + // Lazy initialization (legacy :77-81). A failure here is already + // logged inside initialize() (both exits) and lastError_ is set there. + if (!initialized_ && !initialize()) { + return false; + } + + // One 9-register transaction, single bus attempt (FR-004/parity). + uint16_t registerValues[kReadRegisterCount] = {}; + if (!client_.readHoldingRegisters(deviceAddress_, kRegHumidity, + kReadRegisterCount, registerValues)) { + // Client failure: propagate the client's error code (legacy used + // the ad-hoc code 4) and leave the last-good values untouched. + lastError_ = client_.getLastError(); + ESP_LOGW(TAG, "read failed: bus error %d", lastError_); + return false; + } + + // Decode + scale into locals first — the members are published only + // after validation passes (all-or-nothing, FR-005). + // + // 0x0000 humidity/moisture 0.1 % — NO moisture calibration factor in + // the read path (legacy parity, see file header). + const float humidity = static_cast(registerValues[0]) / 10.0f; + const float moisture = humidity; + // 0x0001 temperature 0.1 °C, SIGNED 16-bit (legacy :98-99). + const float temperature = + static_cast(static_cast(registerValues[1])) / 10.0f; + // 0x0002 EC 1 µS/cm, calibration factor applied (legacy :102-103). + const float ec = + static_cast(registerValues[2]) * ecCalibrationFactor_; + // 0x0003 pH 0.1, calibration factor applied (legacy :106-107). + const float ph = + (static_cast(registerValues[3]) / 10.0f) * phCalibrationFactor_; + // 0x0004–0x0006 N/P/K 1 mg/kg, unscaled. + const float nitrogen = static_cast(registerValues[4]); + const float phosphorus = static_cast(registerValues[5]); + const float potassium = static_cast(registerValues[6]); + // 0x0007 salinity and 0x0008 TDS are read but not exposed (parity). + + // Validate AFTER decode/scaling, on the factored values (legacy + // :121-128 order). Failure publishes nothing. + if (moisture < kMoistureMin || moisture > kMoistureMax || + temperature < kTemperatureMin || temperature > kTemperatureMax || + ph < kPhMin || ph > kPhMax) { + lastError_ = 5; // Range validation failed (parity error 5). + ESP_LOGW(TAG, + "read failed: range validation (moisture=%.1f temp=%.1f " + "ph=%.1f)", + static_cast(moisture), + static_cast(temperature), static_cast(ph)); + return false; + } + + // Publish atomically w.r.t. this object: plain members are fine — + // cross-task exclusion is LockedSoilSensor's job. + humidity_ = humidity; + moisture_ = moisture; + temperature_ = temperature; + ec_ = ec; + ph_ = ph; + nitrogen_ = nitrogen; + phosphorus_ = phosphorus; + potassium_ = potassium; + + lastError_ = 0; + return true; +} + +bool ModbusSoilSensor::isAvailable() +{ + // Lazy initialization doubles as the probe (legacy :136-138). + if (!initialized_) { + return initialize(); + } + + // REAL 1-register bus read on every call, never cached (parity, + // legacy :140 — register 0x0000). Legacy passed nullptr as the buffer; + // the new IModbusClient contract requires a caller-owned buffer. + uint16_t testRegister = 0; + const bool available = client_.readHoldingRegisters( + deviceAddress_, kRegHumidity, 1, &testRegister); + if (!available) { + // Probe failure is not latched (next call probes again) and does + // not touch lastError_ — read() owns the reading's error state. + ESP_LOGW(TAG, "availability probe failed: error %d", + client_.getLastError()); + } + return available; +} + +int ModbusSoilSensor::getLastError() +{ + return lastError_; +} + +float ModbusSoilSensor::getMoisture() +{ + return moisture_; +} + +float ModbusSoilSensor::getTemperature() +{ + return temperature_; +} + +float ModbusSoilSensor::getHumidity() +{ + // Humidity ≡ moisture for this sensor (register 0x0000, parity). + return humidity_; +} + +float ModbusSoilSensor::getPH() +{ + return ph_; +} + +float ModbusSoilSensor::getEC() +{ + return ec_; +} + +float ModbusSoilSensor::getNitrogen() +{ + return nitrogen_; +} + +float ModbusSoilSensor::getPhosphorus() +{ + return phosphorus_; +} + +float ModbusSoilSensor::getPotassium() +{ + return potassium_; +} + +// Calibration — faithful port of legacy :207-322 (three identical flows +// folded into one helper). Host-tested in test_soil_sensor.cpp (calibration +// suite). +bool ModbusSoilSensor::calibrate(uint16_t rawRegister, float rawScale, + uint16_t calibRegister, float& factor, + float referenceValue, const char* quantity) +{ + // Input hygiene, not parity: legacy accepted any float here. A NaN/inf + // or non-positive reference would poison the factor (and NaN silently + // passes every < comparison below), so reject it up front. + if (!std::isfinite(referenceValue) || referenceValue <= 0.0f) { + lastError_ = 5; + ESP_LOGW(TAG, "calibrate %s failed: invalid reference %.3f", + quantity, static_cast(referenceValue)); + return false; + } + + // Lazy initialization (legacy :209-213). + if (!initialized_ && !initialize()) { + return false; + } + + // Fresh raw reading of the single quantity register (legacy errors + // 6/9/12 — here the client's actual error code). + uint16_t rawRegisterValue = 0; + if (!client_.readHoldingRegisters(deviceAddress_, rawRegister, 1, + &rawRegisterValue)) { + lastError_ = client_.getLastError(); + ESP_LOGW(TAG, "calibrate %s failed: raw read error %d", quantity, + lastError_); + return false; + } + + const float currentRawValue = + static_cast(rawRegisterValue) / rawScale; + + // Avoid division by zero (legacy :226-229; legacy codes 7/10/13 map + // onto the normative range-validation code 5). + if (currentRawValue < 0.01f) { + lastError_ = 5; + ESP_LOGW(TAG, "calibrate %s failed: raw value too low (%.3f)", + quantity, static_cast(currentRawValue)); + return false; + } + + const float newFactor = referenceValue / currentRawValue; + + // Input hygiene, not parity: legacy static_cast this product straight to + // uint16_t (UB for values outside 0..65535). A factor that large is + // physically absurd, so reject it BEFORE it is stored or encoded (the + // reference guard above makes negative factors unreachable). + if (newFactor * 100.0f > 65535.0f) { + lastError_ = 5; + ESP_LOGW(TAG, "calibrate %s failed: factor %.3f out of encodable " + "range", quantity, static_cast(newFactor)); + return false; + } + + factor = newFactor; + + // Best-effort factor write (×100) to the sensor's calibration register + // — NON-FATAL on failure: the factor is still used locally (parity, + // legacy :234-241; legacy codes 8/11/14 map onto the client's error). + const uint16_t calibFactorRegValue = + static_cast(factor * 100.0f); + if (!client_.writeSingleRegister(deviceAddress_, calibRegister, + calibFactorRegValue)) { + lastError_ = client_.getLastError(); + ESP_LOGW(TAG, + "calibrate %s: sensor register write failed (error %d), " + "factor %.3f kept locally", + quantity, lastError_, static_cast(factor)); + } else { + lastError_ = 0; + } + + ESP_LOGI(TAG, "calibrate %s: factor=%.3f", quantity, + static_cast(factor)); + return true; +} + +bool ModbusSoilSensor::calibrateMoisture(float referenceValue) +{ + return calibrate(kRegHumidity, 10.0f, kRegMoistureCalib, + moistureCalibrationFactor_, referenceValue, "moisture"); +} + +bool ModbusSoilSensor::calibratePH(float referenceValue) +{ + return calibrate(kRegPh, 10.0f, kRegPhCalib, phCalibrationFactor_, + referenceValue, "ph"); +} + +bool ModbusSoilSensor::calibrateEC(float referenceValue) +{ + // EC raw value is unscaled (legacy :300-301) — rawScale 1. + return calibrate(kRegEc, 1.0f, kRegEcCalib, ecCalibrationFactor_, + referenceValue, "ec"); +} diff --git a/firmware/dependencies.lock b/firmware/dependencies.lock index d9bed83..01808f4 100644 --- a/firmware/dependencies.lock +++ b/firmware/dependencies.lock @@ -27,6 +27,6 @@ direct_dependencies: - espressif/esp-modbus - idf - joltwallet/littlefs -manifest_hash: e580dfb7518fc7d4915da3d29c857a79126d5584a4d47fdbbb6abf4a64e8b96e +manifest_hash: a75946f2281bc7a0ba1812f2e56a4b4bed5255f5aeae0f1f46c455ecd24902e7 target: esp32 version: 3.0.0 diff --git a/firmware/main/CMakeLists.txt b/firmware/main/CMakeLists.txt index 6aac901..97ff861 100644 --- a/firmware/main/CMakeLists.txt +++ b/firmware/main/CMakeLists.txt @@ -2,7 +2,7 @@ idf_component_register( SRCS "app_main.cpp" "diag_console.cpp" PRIV_REQUIRES board esp_driver_gpio esp_app_format actuators interfaces console esp_timer - storage nvs_flash + storage nvs_flash sensors ) # Build-time littlefs image of the committed seed directory diff --git a/firmware/main/app_main.cpp b/firmware/main/app_main.cpp index 687addc..58e6f9c 100644 --- a/firmware/main/app_main.cpp +++ b/firmware/main/app_main.cpp @@ -30,6 +30,9 @@ #include "actuators/EspTimeProvider.h" #include "actuators/GpioWaterPump.h" #include "actuators/LockedWaterPump.h" +#include "sensors/EspModbusClient.h" +#include "sensors/LockedSoilSensor.h" +#include "sensors/ModbusSoilSensor.h" #include "storage/LittleFsDataStorage.h" #include "storage/LockedConfigStore.h" #include "storage/LockedDataStorage.h" @@ -170,9 +173,32 @@ extern "C" void app_main(void) static_cast(stats.usedBytes / 1024), static_cast(stats.totalBytes / 1024)); + // RS485 Modbus soil sensor (feature 004). Not safety-critical: a failed + // client init is logged and the system keeps running — the sensor layer + // reports invalid data and recovers on later attempts (US2 semantics). + // Function-local statics after pumps_force_off() (boot fail-safe rule), + // sensor wrapped in the mutex-serializing decorator: accessed from the + // console REPL task now and the main-loop controller in PR-11, so EVERY + // sensor access goes through the wrapper. No periodic read task in + // feature 004 (arrives with PR-11) — reads happen on console command + // only. + static EspModbusClient modbus_client; + static ModbusSoilSensor soil_sensor_raw(modbus_client); + static LockedSoilSensor soil_sensor(soil_sensor_raw); + + if (modbus_client.initialize()) { + ESP_LOGI(TAG, "RS485 Modbus client up (UART%d)", + BOARD_RS485_UART_PORT); + } else { + ESP_LOGE(TAG, "RS485 Modbus client init failed (error %d) — " + "soil sensor unavailable until recovery", + modbus_client.getLastError()); + } + // Serial diagnostic REPL (rig testing; contracts/serial-diagnostic.md). diag_console_register_pumps(plant, reservoir); diag_console_register_storage(config, storage); + diag_console_register_soil(soil_sensor, modbus_client); esp_err_t err = diag_console_start(); if (err != ESP_OK) { // Console is a diagnostic aid, not a safety function: log and keep diff --git a/firmware/main/diag_console.cpp b/firmware/main/diag_console.cpp index c1f3c39..1a826b7 100644 --- a/firmware/main/diag_console.cpp +++ b/firmware/main/diag_console.cpp @@ -27,6 +27,17 @@ * storage event # category = u8 (1..255) * storage events [n] # newest-first, default 10 * + * Soil sensor commands (HIL verification path for feature 004; console + * contract in specs/004-modbus-soil-sensor/contracts/interfaces.md — the + * calibration commands print no factor value: no factor getter exists, + * see cmd_soil_calibrate): + * + * soil # one read(); 7 values or error + * rs485test # raw 1-register probe + statistics + * soil_cal_moisture # calibrate against a reference + * soil_cal_ph # value; a failed calibration- + * soil_cal_ec # register write is NON-FATAL + * * Handler exit codes follow the esp_console convention: 0 on OK, 1 on ERR. * * State is plain pointers/PODs set from app_main — no non-trivial static @@ -35,6 +46,7 @@ #include "diag_console.h" +#include #include #include #include @@ -47,6 +59,8 @@ #include "interfaces/IConfigStore.h" #include "interfaces/IDataStorage.h" +#include "interfaces/IModbusClient.h" +#include "interfaces/ISoilSensor.h" #include "interfaces/IWaterPump.h" namespace { @@ -71,6 +85,11 @@ PumpSlot s_slots[2] = { IConfigStore *s_config = nullptr; IDataStorage *s_storage = nullptr; +// Soil sensor + Modbus client (set from app_main; the sensor is expected +// to be the LockedSoilSensor decorator). Same trivial-initialization rule. +ISoilSensor *s_soil = nullptr; +IModbusClient *s_modbus = nullptr; + const char *stop_reason_str(StopReason reason) { switch (reason) { @@ -198,12 +217,13 @@ bool parse_u32(const char *arg, uint32_t &out) return true; } -/// Strict float parse; false on garbage (range checks are the store's job). +/// Strict float parse; false on garbage or non-finite input — nan/inf never +/// reach a float consumer (finite range checks are the consumer's job). bool parse_float(const char *arg, float &out) { char *end = nullptr; out = strtof(arg, &end); - return end != arg && *end == '\0'; + return end != arg && *end == '\0' && std::isfinite(out); } void print_config(const IConfigStore &config) @@ -430,6 +450,152 @@ int storage_cmd(int argc, char **argv) return print_storage_usage(); } +// --- soil / rs485test commands (feature 004 HIL verification path) ------ + +/// Error-code names per the data-model.md error table (feature 004). +const char *soil_error_str(int error) +{ + switch (error) { + case 0: + return "ok"; + case 1: + return "not_initialized"; + case 2: + return "bus_error"; + case 3: + return "timeout"; + case 5: + return "range_validation"; + default: + return error >= 100 ? "slave_exception" : "unknown"; + } +} + +/// `soil`: one read() through the locked sensor; all 7 values or the error. +int soil_cmd(int argc, char **argv) +{ + (void)argv; + if (argc != 1) { + printf("ERR usage: soil\n"); + return 1; + } + if (s_soil == nullptr) { + printf("ERR soil sensor not available\n"); + return 1; + } + if (!s_soil->read()) { + const int error = s_soil->getLastError(); + printf("ERR read failed: error %d (%s)\n", error, + soil_error_str(error)); + return 1; + } + printf("OK moisture=%.1f %% temp=%.1f C ec=%.0f uS/cm ph=%.1f " + "n=%.0f mg/kg p=%.0f mg/kg k=%.0f mg/kg\n", + static_cast(s_soil->getMoisture()), + static_cast(s_soil->getTemperature()), + static_cast(s_soil->getEC()), + static_cast(s_soil->getPH()), + static_cast(s_soil->getNitrogen()), + static_cast(s_soil->getPhosphorus()), + static_cast(s_soil->getPotassium())); + return 0; +} + +/// `rs485test`: one raw 1-register probe (slave 0x01, register 0x0000 — +/// the parity availability probe) + cumulative transaction statistics. +/// +/// TODO(PR-11): this drives the raw, unsynchronized EspModbusClient BELOW +/// LockedSoilSensor's mutex; once PR-11 adds the main-loop reader this +/// becomes a cross-task race — route it through a locked client wrapper +/// (or through the sensor) then. +int rs485test_cmd(int argc, char **argv) +{ + (void)argv; + if (argc != 1) { + printf("ERR usage: rs485test\n"); + return 1; + } + if (s_modbus == nullptr) { + printf("ERR modbus client not available\n"); + return 1; + } + uint16_t value = 0; + const bool ok = s_modbus->readHoldingRegisters(0x01, 0x0000, 1, &value); + uint32_t successCount = 0; + uint32_t errorCount = 0; + s_modbus->getStatistics(&successCount, &errorCount); + if (ok) { + printf("OK reg 0x0000 = %u, stats: success=%lu error=%lu\n", + static_cast(value), + static_cast(successCount), + static_cast(errorCount)); + return 0; + } + const int error = s_modbus->getLastError(); + printf("ERR probe failed: error %d (%s), stats: success=%lu error=%lu\n", + error, soil_error_str(error), + static_cast(successCount), + static_cast(errorCount)); + return 1; +} + +/// Shared `soil_cal_*` flow: one calibrate*() call through the locked +/// sensor. ModbusSoilSensor exposes no factor getter (factors are private, +/// RAM-only for now), so the output reports success/failure plus the +/// legacy write-result semantics: a true return with a non-zero last error +/// means the factor is applied locally but the best-effort write to the +/// sensor's calibration register failed (NON-FATAL, parity). +int cmd_soil_calibrate(int argc, char **argv, const char *name, + bool (ISoilSensor::*calibrate)(float)) +{ + if (s_soil == nullptr) { + printf("ERR soil sensor not available\n"); + return 1; + } + if (argc != 2) { + printf("ERR usage: %s \n", name); + return 1; + } + float reference = 0.0f; + if (!parse_float(argv[1], reference)) { + printf("ERR reference-value: not a number\n"); + return 1; + } + if (!(s_soil->*calibrate)(reference)) { + const int error = s_soil->getLastError(); + printf("ERR calibration failed: error %d (%s)\n", error, + soil_error_str(error)); + return 1; + } + const int error = s_soil->getLastError(); + if (error != 0) { + printf("OK calibration applied (sensor register write failed, " + "non-fatal: error %d (%s))\n", + error, soil_error_str(error)); + } else { + printf("OK calibration applied\n"); + } + return 0; +} + +int soil_cal_moisture_cmd(int argc, char **argv) +{ + return cmd_soil_calibrate(argc, argv, "soil_cal_moisture", + &ISoilSensor::calibrateMoisture); +} + +int soil_cal_ph_cmd(int argc, char **argv) +{ + return cmd_soil_calibrate(argc, argv, "soil_cal_ph", + &ISoilSensor::calibratePH); +} + +int soil_cal_ec_cmd(int argc, char **argv) +{ + return cmd_soil_calibrate(argc, argv, "soil_cal_ec", + &ISoilSensor::calibrateEC); +} + } // namespace void diag_console_register_pumps(IWaterPump& plant, IWaterPump& reservoir) @@ -444,6 +610,12 @@ void diag_console_register_storage(IConfigStore& config, IDataStorage& storage) s_storage = &storage; } +void diag_console_register_soil(ISoilSensor& sensor, IModbusClient& client) +{ + s_soil = &sensor; + s_modbus = &client; +} + esp_err_t diag_console_start(void) { esp_console_repl_t *repl = nullptr; @@ -504,5 +676,78 @@ esp_err_t diag_console_start(void) return err; } + const esp_console_cmd_t cmd_soil = { + .command = "soil", + .help = "soil — one soil sensor read (7 values or error code)", + .hint = nullptr, + .func = &soil_cmd, + .argtable = nullptr, + .func_w_context = nullptr, + .context = nullptr, + }; + err = esp_console_cmd_register(&cmd_soil); + if (err != ESP_OK) { + return err; + } + + const esp_console_cmd_t cmd_rs485test = { + .command = "rs485test", + .help = "rs485test — raw 1-register Modbus probe + statistics", + .hint = nullptr, + .func = &rs485test_cmd, + .argtable = nullptr, + .func_w_context = nullptr, + .context = nullptr, + }; + err = esp_console_cmd_register(&cmd_rs485test); + if (err != ESP_OK) { + return err; + } + + const esp_console_cmd_t cmd_soil_cal_moisture = { + .command = "soil_cal_moisture", + .help = "soil_cal_moisture — calibrate moisture " + "against a reference in %", + .hint = nullptr, + .func = &soil_cal_moisture_cmd, + .argtable = nullptr, + .func_w_context = nullptr, + .context = nullptr, + }; + err = esp_console_cmd_register(&cmd_soil_cal_moisture); + if (err != ESP_OK) { + return err; + } + + const esp_console_cmd_t cmd_soil_cal_ph = { + .command = "soil_cal_ph", + .help = "soil_cal_ph — calibrate pH against a " + "reference", + .hint = nullptr, + .func = &soil_cal_ph_cmd, + .argtable = nullptr, + .func_w_context = nullptr, + .context = nullptr, + }; + err = esp_console_cmd_register(&cmd_soil_cal_ph); + if (err != ESP_OK) { + return err; + } + + const esp_console_cmd_t cmd_soil_cal_ec = { + .command = "soil_cal_ec", + .help = "soil_cal_ec — calibrate EC against a " + "reference in uS/cm", + .hint = nullptr, + .func = &soil_cal_ec_cmd, + .argtable = nullptr, + .func_w_context = nullptr, + .context = nullptr, + }; + err = esp_console_cmd_register(&cmd_soil_cal_ec); + if (err != ESP_OK) { + return err; + } + return esp_console_start_repl(repl); } diff --git a/firmware/main/diag_console.h b/firmware/main/diag_console.h index fe6343a..e0d3bda 100644 --- a/firmware/main/diag_console.h +++ b/firmware/main/diag_console.h @@ -14,6 +14,8 @@ #include "esp_err.h" #include "interfaces/IConfigStore.h" #include "interfaces/IDataStorage.h" +#include "interfaces/IModbusClient.h" +#include "interfaces/ISoilSensor.h" #include "interfaces/IWaterPump.h" /** @@ -35,6 +37,18 @@ void diag_console_register_pumps(IWaterPump& plant, IWaterPump& reservoir); void diag_console_register_storage(IConfigStore& config, IDataStorage& storage); +/** + * @brief Register the soil sensor + Modbus client the `soil`/`rs485test` + * commands operate on (HIL verification path for feature 004). + * + * Pass the LockedSoilSensor decorator, never the raw sensor — the console + * handlers run on the REPL task, concurrently with the main-loop reader + * arriving in PR-11. The client may be passed raw: in this PR it is only + * reached from the REPL task (directly and via the locked sensor). Must be + * called before diag_console_start(); plain pointer registration. + */ +void diag_console_register_soil(ISoilSensor& sensor, IModbusClient& client); + /** * @brief Start the UART REPL (prompt "ws>") and register the commands. * diff --git a/firmware/main/idf_component.yml b/firmware/main/idf_component.yml index dd74fdf..88b3fe0 100644 --- a/firmware/main/idf_component.yml +++ b/firmware/main/idf_component.yml @@ -1,6 +1,7 @@ # Managed dependencies (IDF Component Registry), pinned for reproducible builds. -# Not yet used by app_main — pinned here so dependency resolution is validated -# in CI from phase 0 onwards. +# Used by app_main via components/sensors since feature 004. Keep the esp-modbus +# version in lockstep with components/sensors/idf_component.yml (both pin +# ==2.1.2; bump together or the resolver conflicts). dependencies: idf: ">=6.0.0" espressif/esp-modbus: "==2.1.2" diff --git a/firmware/test_apps/host/main/CMakeLists.txt b/firmware/test_apps/host/main/CMakeLists.txt index 7785cc2..841ba31 100644 --- a/firmware/test_apps/host/main/CMakeLists.txt +++ b/firmware/test_apps/host/main/CMakeLists.txt @@ -3,5 +3,6 @@ idf_component_register( "test_water_pump.cpp" "test_config_store.cpp" "test_data_storage.cpp" - REQUIRES unity actuators interfaces storage nvs_flash + "test_soil_sensor.cpp" + REQUIRES unity actuators interfaces storage nvs_flash sensors ) diff --git a/firmware/test_apps/host/main/test_main.cpp b/firmware/test_apps/host/main/test_main.cpp index cfef728..dca0839 100644 --- a/firmware/test_apps/host/main/test_main.cpp +++ b/firmware/test_apps/host/main/test_main.cpp @@ -16,6 +16,7 @@ void run_water_pump_tests(void); void run_config_store_tests(void); void run_data_storage_tests(void); +void run_soil_sensor_tests(void); // Unity requires setUp/tearDown definitions (shared by all suites). extern "C" void setUp(void) {} @@ -27,5 +28,6 @@ extern "C" void app_main(void) run_water_pump_tests(); run_config_store_tests(); run_data_storage_tests(); + run_soil_sensor_tests(); std::exit(UNITY_END()); } diff --git a/firmware/test_apps/host/main/test_soil_sensor.cpp b/firmware/test_apps/host/main/test_soil_sensor.cpp new file mode 100644 index 0000000..af1ca8b --- /dev/null +++ b/firmware/test_apps/host/main/test_soil_sensor.cpp @@ -0,0 +1,789 @@ +// SPDX-FileCopyrightText: 2026 Cryptotomte +// SPDX-License-Identifier: AGPL-3.0-or-later +/** + * @file test_soil_sensor.cpp + * @brief Host tests for the ModbusSoilSensor register decode (linux target). + * + * Tests the REAL decode/scaling logic (ModbusSoilSensor) via + * MockModbusClient (scripted register payloads + call recording). + * Registered via run_soil_sensor_tests() from the shared Unity runner + * (test_main.cpp); the process exit code equals the failure count and is + * the CI gate. + * + * Coverage maps to tasks.md T006 + T014 / quickstart.md §1: the + * data-model.md scaling table incl. the signed-temperature decode, + * humidity ≡ moisture, the one-transaction-per-read() invariant, and the + * US2 fault paths — timeout, range validation (error 5, all-or-nothing + * publish), exception propagation, no-retry, implicit recovery, lazy + * (re)initialization, real-bus availability probe, statistics and the + * setTimeout client contract. T021 adds the calibration flows: factor + * computation from a fresh 1-register raw read, the ×100 best-effort + * calibration-register write (NON-FATAL on failure), read-path application + * for pH/EC (the moisture factor is stored/written but NOT applied — legacy + * parity), the raw-too-low guard (error 5) and range validation running on + * the FACTORED values. + */ + +#include +#include + +#include "unity.h" + +#include "sensors/ModbusSoilSensor.h" +#include "sensors/testing/MockModbusClient.h" + +namespace { + +constexpr uint8_t kAddr = 0x01; +constexpr uint16_t kStartReg = 0x0000; +constexpr uint16_t kRegCount = 9; + +// Calibration raw-read registers (data-model.md; 1-register reads — a +// distinct mock script key from the 9-register data payload above) and the +// calibration factor registers written with factor ×100. +constexpr uint16_t kRegEc = 0x0002; +constexpr uint16_t kRegPh = 0x0003; +constexpr uint16_t kRegMoistureCalib = 0x0100; +constexpr uint16_t kRegPhCalib = 0x0101; +constexpr uint16_t kRegEcCalib = 0x0102; + +/// Baseline valid payload: moisture 55.0 %, temperature 23.5 °C, +/// EC 1200 µS/cm, pH 6.8, N 45, P 30, K 120 (+ salinity/TDS, unexposed). +std::vector goodPayload() +{ + return {550, 235, 1200, 68, 45, 30, 120, 7, 9}; +} + +/// Fresh mock + sensor per test; initialization (client init + 1-register +/// probe) is part of the fixture, and the recorded call log is cleared so +/// each test asserts only on its own transactions. NOTE: the fixture's +/// probe read still counts in the mock STATISTICS (success 1 / error 0 +/// baseline) — only the call log is cleared. +struct Fixture { + MockModbusClient mock; + ModbusSoilSensor sensor{mock, kAddr}; + + explicit Fixture(std::vector payload) + { + mock.setRegisters(kAddr, kStartReg, std::move(payload)); + TEST_ASSERT_TRUE(sensor.initialize()); + mock.calls.clear(); + } +}; + +} // namespace + +// -------------------------------------------------------------------------- +// Known 9-register payload decodes per the data-model scaling table, +// including a negative temperature (0xFF38 = -200 raw = -20.0 °C) +// -------------------------------------------------------------------------- +static void test_decode_known_payload_negative_temperature(void) +{ + // regs 0x0000..0x0008: humidity/moisture, temp, EC, pH, N, P, K, + // salinity, TDS. + Fixture f({550, 0xFF38, 1200, 68, 45, 30, 120, 7, 9}); + + TEST_ASSERT_TRUE(f.sensor.read()); + TEST_ASSERT_EQUAL_INT(0, f.sensor.getLastError()); + + TEST_ASSERT_EQUAL_FLOAT(55.0f, f.sensor.getMoisture()); // 550 / 10 + TEST_ASSERT_EQUAL_FLOAT(-20.0f, f.sensor.getTemperature()); // int16 -200 / 10 + TEST_ASSERT_EQUAL_FLOAT(1200.0f, f.sensor.getEC()); // unscaled + TEST_ASSERT_EQUAL_FLOAT(6.8f, f.sensor.getPH()); // 68 / 10 + TEST_ASSERT_EQUAL_FLOAT(45.0f, f.sensor.getNitrogen()); + TEST_ASSERT_EQUAL_FLOAT(30.0f, f.sensor.getPhosphorus()); + TEST_ASSERT_EQUAL_FLOAT(120.0f, f.sensor.getPotassium()); + + // Salinity (reg 0x0007 = 7) and TDS (reg 0x0008 = 9) are read but not + // exposed: ISoilSensor has no getter for them, so there is nothing + // further to assert beyond the 7 values above (deliberate parity trim). +} + +// -------------------------------------------------------------------------- +// Positive signed temperature decodes the same way (235 raw = 23.5 °C) +// -------------------------------------------------------------------------- +static void test_decode_positive_temperature(void) +{ + Fixture f({420, 235, 800, 65, 12, 8, 40, 3, 4}); + + TEST_ASSERT_TRUE(f.sensor.read()); + TEST_ASSERT_EQUAL_FLOAT(23.5f, f.sensor.getTemperature()); + TEST_ASSERT_EQUAL_FLOAT(42.0f, f.sensor.getMoisture()); + TEST_ASSERT_EQUAL_FLOAT(6.5f, f.sensor.getPH()); +} + +// -------------------------------------------------------------------------- +// Humidity ≡ moisture (parity: single quantity in register 0x0000) +// -------------------------------------------------------------------------- +static void test_humidity_equals_moisture(void) +{ + Fixture f({550, 0xFF38, 1200, 68, 45, 30, 120, 7, 9}); + + TEST_ASSERT_TRUE(f.sensor.read()); + TEST_ASSERT_EQUAL_FLOAT(f.sensor.getMoisture(), f.sensor.getHumidity()); + TEST_ASSERT_EQUAL_FLOAT(55.0f, f.sensor.getHumidity()); +} + +// -------------------------------------------------------------------------- +// One read() = exactly ONE bus transaction: all 9 registers in one +// readHoldingRegisters(0x01, 0x0000, 9) call (FR-004, no retry) +// -------------------------------------------------------------------------- +static void test_read_is_one_nine_register_transaction(void) +{ + Fixture f({550, 235, 1200, 68, 45, 30, 120, 7, 9}); + + TEST_ASSERT_TRUE(f.sensor.read()); + + TEST_ASSERT_EQUAL(1, f.mock.calls.size()); + const MockModbusClient::Call &call = f.mock.calls.front(); + TEST_ASSERT_EQUAL(static_cast(MockModbusClient::Call::Type::Read), + static_cast(call.type)); + TEST_ASSERT_EQUAL_UINT8(kAddr, call.deviceAddress); + TEST_ASSERT_EQUAL_UINT16(kStartReg, call.startRegister); + TEST_ASSERT_EQUAL_UINT16(kRegCount, call.count); + TEST_ASSERT_TRUE(call.succeeded); +} + +// ========================================================================== +// Fault paths (T014, US2): timeout / validation / exception / no-retry / +// recovery / statistics / availability probe / setTimeout contract +// ========================================================================== + +// -------------------------------------------------------------------------- +// Timeout: read() fails with error 3 and the last-good values remain +// untouched — getters after a failed read are stale, not fresh (FR-005) +// -------------------------------------------------------------------------- +static void test_timeout_fails_read_and_keeps_last_good_values(void) +{ + Fixture f(goodPayload()); + TEST_ASSERT_TRUE(f.sensor.read()); + + f.mock.queueOutcome(MockModbusClient::kErrTimeout); + TEST_ASSERT_FALSE(f.sensor.read()); + TEST_ASSERT_EQUAL_INT(MockModbusClient::kErrTimeout, + f.sensor.getLastError()); + + // Last-good reading still served (validity is carried by the read() + // result + error code, never by the values). + TEST_ASSERT_EQUAL_FLOAT(55.0f, f.sensor.getMoisture()); + TEST_ASSERT_EQUAL_FLOAT(23.5f, f.sensor.getTemperature()); + TEST_ASSERT_EQUAL_FLOAT(1200.0f, f.sensor.getEC()); + TEST_ASSERT_EQUAL_FLOAT(6.8f, f.sensor.getPH()); +} + +// -------------------------------------------------------------------------- +// Range validation: moisture > 100 % (raw 1500 = 150.0 %) → error 5, +// nothing published +// -------------------------------------------------------------------------- +static void test_out_of_range_moisture_rejected_error5(void) +{ + Fixture f(goodPayload()); + TEST_ASSERT_TRUE(f.sensor.read()); + + f.mock.setRegisters(kAddr, kStartReg, {1500, 235, 800, 68, 1, 2, 3, 0, 0}); + TEST_ASSERT_FALSE(f.sensor.read()); + TEST_ASSERT_EQUAL_INT(5, f.sensor.getLastError()); + + TEST_ASSERT_EQUAL_FLOAT(55.0f, f.sensor.getMoisture()); + TEST_ASSERT_EQUAL_FLOAT(55.0f, f.sensor.getHumidity()); +} + +// -------------------------------------------------------------------------- +// Range validation: temperature < -40 °C (raw 0xFE0C = int16 -500 = +// -50.0 °C) → error 5, nothing published +// -------------------------------------------------------------------------- +static void test_out_of_range_temperature_rejected_error5(void) +{ + Fixture f(goodPayload()); + TEST_ASSERT_TRUE(f.sensor.read()); + + f.mock.setRegisters(kAddr, kStartReg, + {550, 0xFE0C, 800, 68, 1, 2, 3, 0, 0}); + TEST_ASSERT_FALSE(f.sensor.read()); + TEST_ASSERT_EQUAL_INT(5, f.sensor.getLastError()); + + TEST_ASSERT_EQUAL_FLOAT(23.5f, f.sensor.getTemperature()); +} + +// -------------------------------------------------------------------------- +// Range validation: pH > 9 (raw 95 = 9.5) → error 5, and the publish is +// ALL-OR-NOTHING: in-range fields of the same payload (EC, N/P/K) are +// rejected together with the offending one (FR-005) +// -------------------------------------------------------------------------- +static void test_out_of_range_ph_rejected_all_or_nothing(void) +{ + Fixture f(goodPayload()); + TEST_ASSERT_TRUE(f.sensor.read()); + + // pH 9.5 out of range; EC 999 and N/P/K 77/88/99 are individually fine. + f.mock.setRegisters(kAddr, kStartReg, + {550, 235, 999, 95, 77, 88, 99, 0, 0}); + TEST_ASSERT_FALSE(f.sensor.read()); + TEST_ASSERT_EQUAL_INT(5, f.sensor.getLastError()); + + TEST_ASSERT_EQUAL_FLOAT(6.8f, f.sensor.getPH()); + TEST_ASSERT_EQUAL_FLOAT(1200.0f, f.sensor.getEC()); // not 999 + TEST_ASSERT_EQUAL_FLOAT(45.0f, f.sensor.getNitrogen()); // not 77 + TEST_ASSERT_EQUAL_FLOAT(30.0f, f.sensor.getPhosphorus()); + TEST_ASSERT_EQUAL_FLOAT(120.0f, f.sensor.getPotassium()); +} + +// -------------------------------------------------------------------------- +// Range validation: temperature > 80 °C (raw 850 = 85.0 °C) → error 5, +// nothing published (upper bound of the parity temperature range) +// -------------------------------------------------------------------------- +static void test_out_of_range_high_temperature_rejected_error5(void) +{ + Fixture f(goodPayload()); + TEST_ASSERT_TRUE(f.sensor.read()); + + f.mock.setRegisters(kAddr, kStartReg, {550, 850, 800, 68, 1, 2, 3, 0, 0}); + TEST_ASSERT_FALSE(f.sensor.read()); + TEST_ASSERT_EQUAL_INT(5, f.sensor.getLastError()); + + TEST_ASSERT_EQUAL_FLOAT(23.5f, f.sensor.getTemperature()); +} + +// -------------------------------------------------------------------------- +// Range validation: pH < 3 (raw 25 = 2.5) → error 5, nothing published +// (lower bound of the parity pH range) +// -------------------------------------------------------------------------- +static void test_out_of_range_low_ph_rejected_error5(void) +{ + Fixture f(goodPayload()); + TEST_ASSERT_TRUE(f.sensor.read()); + + f.mock.setRegisters(kAddr, kStartReg, {550, 235, 800, 25, 1, 2, 3, 0, 0}); + TEST_ASSERT_FALSE(f.sensor.read()); + TEST_ASSERT_EQUAL_INT(5, f.sensor.getLastError()); + + TEST_ASSERT_EQUAL_FLOAT(6.8f, f.sensor.getPH()); +} + +// -------------------------------------------------------------------------- +// Boundary inclusivity: the parity range limits themselves are VALID — +// moisture 100.0 % (raw 1000), temperature -40.0 °C (raw 0xFE70 = -400) +// and pH 3.0 (raw 30) in one payload; pH 9.0 (raw 90) too. One step past +// the limit (moisture 100.1 %, raw 1001) is invalid with error 5. +// -------------------------------------------------------------------------- +static void test_range_boundaries_are_inclusive(void) +{ + Fixture f({1000, 0xFE70, 800, 30, 1, 2, 3, 0, 0}); + + TEST_ASSERT_TRUE(f.sensor.read()); + TEST_ASSERT_EQUAL_INT(0, f.sensor.getLastError()); + TEST_ASSERT_EQUAL_FLOAT(100.0f, f.sensor.getMoisture()); + TEST_ASSERT_EQUAL_FLOAT(-40.0f, f.sensor.getTemperature()); + TEST_ASSERT_EQUAL_FLOAT(3.0f, f.sensor.getPH()); + + // Upper pH bound (9.0) is valid too. + f.mock.setRegisters(kAddr, kStartReg, {1000, 0xFE70, 800, 90, 1, 2, 3, 0, 0}); + TEST_ASSERT_TRUE(f.sensor.read()); + TEST_ASSERT_EQUAL_FLOAT(9.0f, f.sensor.getPH()); + + // One step past the moisture limit: 100.1 % → error 5. + f.mock.setRegisters(kAddr, kStartReg, {1001, 0xFE70, 800, 90, 1, 2, 3, 0, 0}); + TEST_ASSERT_FALSE(f.sensor.read()); + TEST_ASSERT_EQUAL_INT(5, f.sensor.getLastError()); + TEST_ASSERT_EQUAL_FLOAT(100.0f, f.sensor.getMoisture()); +} + +// -------------------------------------------------------------------------- +// EC/N/P/K are NOT range-enforced on read (parity, FR-004 second sentence): +// extreme values — up to the uint16 maximum — are published as valid +// -------------------------------------------------------------------------- +static void test_ec_npk_not_range_enforced(void) +{ + Fixture f({550, 235, 65535, 68, 65535, 65535, 65535, 0, 0}); + + TEST_ASSERT_TRUE(f.sensor.read()); + TEST_ASSERT_EQUAL_INT(0, f.sensor.getLastError()); + TEST_ASSERT_EQUAL_FLOAT(65535.0f, f.sensor.getEC()); + TEST_ASSERT_EQUAL_FLOAT(65535.0f, f.sensor.getNitrogen()); + TEST_ASSERT_EQUAL_FLOAT(65535.0f, f.sensor.getPhosphorus()); + TEST_ASSERT_EQUAL_FLOAT(65535.0f, f.sensor.getPotassium()); +} + +// -------------------------------------------------------------------------- +// Modbus slave exception: the sensor layer propagates the client's error +// code VERBATIM (here 100+2 = 102). The real EspModbusClient coarsens +// exceptions to code 2 (esp-modbus 2.1.2 hides the exception number — +// documented parity divergence R6); this test pins the sensor-layer +// propagation contract, not the client mapping. +// -------------------------------------------------------------------------- +static void test_modbus_exception_propagates_verbatim(void) +{ + Fixture f(goodPayload()); + + f.mock.queueOutcome(MockModbusClient::kErrExceptionBase + 2); + TEST_ASSERT_FALSE(f.sensor.read()); + TEST_ASSERT_EQUAL_INT(102, f.sensor.getLastError()); +} + +// -------------------------------------------------------------------------- +// No retry: a failed read() is exactly ONE bus attempt (FR-004) — the +// fixture cleared the init probe, so the call log holds only this read +// -------------------------------------------------------------------------- +static void test_failed_read_is_single_bus_attempt(void) +{ + Fixture f(goodPayload()); + + f.mock.queueOutcome(MockModbusClient::kErrTimeout); + TEST_ASSERT_FALSE(f.sensor.read()); + + TEST_ASSERT_EQUAL(1, f.mock.calls.size()); + const MockModbusClient::Call &call = f.mock.calls.front(); + TEST_ASSERT_EQUAL(static_cast(MockModbusClient::Call::Type::Read), + static_cast(call.type)); + TEST_ASSERT_EQUAL_UINT16(kRegCount, call.count); + TEST_ASSERT_FALSE(call.succeeded); +} + +// -------------------------------------------------------------------------- +// Implicit recovery: fail (timeout) then succeed — the next read() returns +// fresh values with NO re-initialization (no extra probe transaction, no +// second client initialize; there is no permanent-failure latch) +// -------------------------------------------------------------------------- +static void test_read_recovers_after_failure_without_reinit(void) +{ + Fixture f(goodPayload()); + + f.mock.queueOutcome(MockModbusClient::kErrTimeout); + TEST_ASSERT_FALSE(f.sensor.read()); + + TEST_ASSERT_TRUE(f.sensor.read()); // defaultOutcome kOk + TEST_ASSERT_EQUAL_INT(0, f.sensor.getLastError()); + TEST_ASSERT_EQUAL_FLOAT(55.0f, f.sensor.getMoisture()); + TEST_ASSERT_EQUAL_FLOAT(23.5f, f.sensor.getTemperature()); + + // Exactly the two 9-register reads — no 1-register probe in between. + TEST_ASSERT_EQUAL(2, f.mock.calls.size()); + TEST_ASSERT_EQUAL_UINT16(kRegCount, f.mock.calls[0].count); + TEST_ASSERT_EQUAL_UINT16(kRegCount, f.mock.calls[1].count); + TEST_ASSERT_EQUAL_INT(1, f.mock.initializeCalls); +} + +// -------------------------------------------------------------------------- +// Statistics: exactly one counter increments per client call, on both +// outcomes (IModbusClient contract; fixture probe = baseline 1 success) +// -------------------------------------------------------------------------- +static void test_statistics_count_one_per_call(void) +{ + Fixture f(goodPayload()); + + uint32_t successCount = 0; + uint32_t errorCount = 0; + f.mock.getStatistics(&successCount, &errorCount); + TEST_ASSERT_EQUAL_UINT32(1, successCount); // fixture init probe + TEST_ASSERT_EQUAL_UINT32(0, errorCount); + + TEST_ASSERT_TRUE(f.sensor.read()); + f.mock.getStatistics(&successCount, &errorCount); + TEST_ASSERT_EQUAL_UINT32(2, successCount); + TEST_ASSERT_EQUAL_UINT32(0, errorCount); + + f.mock.queueOutcome(MockModbusClient::kErrTimeout); + TEST_ASSERT_FALSE(f.sensor.read()); + f.mock.getStatistics(&successCount, &errorCount); + TEST_ASSERT_EQUAL_UINT32(2, successCount); + TEST_ASSERT_EQUAL_UINT32(1, errorCount); +} + +// -------------------------------------------------------------------------- +// isAvailable(): a REAL 1-register bus read of 0x0000 on EVERY call — +// never cached (FR-011); result reflects the live outcome, and a sensor +// that answers again is available again +// -------------------------------------------------------------------------- +static void test_is_available_performs_real_probe_every_call(void) +{ + Fixture f(goodPayload()); + + TEST_ASSERT_TRUE(f.sensor.isAvailable()); + TEST_ASSERT_EQUAL(1, f.mock.calls.size()); + const MockModbusClient::Call &probe = f.mock.calls.front(); + TEST_ASSERT_EQUAL(static_cast(MockModbusClient::Call::Type::Read), + static_cast(probe.type)); + TEST_ASSERT_EQUAL_UINT8(kAddr, probe.deviceAddress); + TEST_ASSERT_EQUAL_UINT16(0x0000, probe.startRegister); + TEST_ASSERT_EQUAL_UINT16(1, probe.count); + + // Second call hits the bus again (not a cached true) and reports the + // live failure... + f.mock.queueOutcome(MockModbusClient::kErrTimeout); + TEST_ASSERT_FALSE(f.sensor.isAvailable()); + TEST_ASSERT_EQUAL(2, f.mock.calls.size()); + + // ...without clobbering getLastError(): the probe result is carried by + // the return value only — the fixture's error state (0 after the init) + // is UNCHANGED by the failed probe (read() owns the reading's error). + TEST_ASSERT_EQUAL_INT(0, f.sensor.getLastError()); + + // ...and the next call recovers implicitly (no failure latch). + TEST_ASSERT_TRUE(f.sensor.isAvailable()); + TEST_ASSERT_EQUAL(3, f.mock.calls.size()); +} + +// -------------------------------------------------------------------------- +// setTimeout(): reaches the client and is recorded verbatim (FR-006 at the +// IModbusClient contract level — ModbusSoilSensor itself never sets the +// timeout; the parity 3000 ms default lives in the client) +// -------------------------------------------------------------------------- +static void test_set_timeout_reaches_client(void) +{ + MockModbusClient mock; + mock.setTimeout(1234); + + TEST_ASSERT_EQUAL(1, mock.timeoutCalls.size()); + TEST_ASSERT_EQUAL_UINT32(1234, mock.timeoutCalls.front()); +} + +// -------------------------------------------------------------------------- +// Lazy initialization: a failed client initialize() → read() fails with +// error 2 and NO bus transaction; once the client comes up, the next +// read() initializes (probe) and delivers fresh values +// -------------------------------------------------------------------------- +static void test_lazy_init_client_failure_then_recovery(void) +{ + MockModbusClient mock; + mock.setRegisters(kAddr, kStartReg, goodPayload()); + mock.initializeResult = false; + ModbusSoilSensor sensor(mock, kAddr); + + TEST_ASSERT_FALSE(sensor.read()); + TEST_ASSERT_EQUAL_INT(2, sensor.getLastError()); // client init failed + TEST_ASSERT_EQUAL(0, mock.calls.size()); // never reached the bus + + mock.initializeResult = true; + TEST_ASSERT_TRUE(sensor.read()); + TEST_ASSERT_EQUAL_INT(0, sensor.getLastError()); + TEST_ASSERT_EQUAL_FLOAT(55.0f, sensor.getMoisture()); + + // Recovery read = 1-register init probe + the 9-register data read. + TEST_ASSERT_EQUAL(2, mock.calls.size()); + TEST_ASSERT_EQUAL_UINT16(1, mock.calls[0].count); + TEST_ASSERT_EQUAL_UINT16(kRegCount, mock.calls[1].count); +} + +// -------------------------------------------------------------------------- +// Lazy initialization: a failed init PROBE keeps the sensor uninitialized +// (read() aborts after the probe — no data transaction) and the next +// read() re-runs the full initialization before reading +// -------------------------------------------------------------------------- +static void test_lazy_init_probe_failure_then_recovery(void) +{ + MockModbusClient mock; + mock.setRegisters(kAddr, kStartReg, goodPayload()); + ModbusSoilSensor sensor(mock, kAddr); + + mock.queueOutcome(MockModbusClient::kErrTimeout); // hits the init probe + TEST_ASSERT_FALSE(sensor.read()); + TEST_ASSERT_EQUAL_INT(MockModbusClient::kErrTimeout, + sensor.getLastError()); + TEST_ASSERT_EQUAL(1, mock.calls.size()); // probe only, no 9-reg read + TEST_ASSERT_EQUAL_UINT16(1, mock.calls[0].count); + + TEST_ASSERT_TRUE(sensor.read()); + TEST_ASSERT_EQUAL_FLOAT(55.0f, sensor.getMoisture()); + + // Second read re-initialized: probe (1 reg) + data read (9 regs). + TEST_ASSERT_EQUAL(3, mock.calls.size()); + TEST_ASSERT_EQUAL_UINT16(1, mock.calls[1].count); + TEST_ASSERT_EQUAL_UINT16(kRegCount, mock.calls[2].count); + TEST_ASSERT_EQUAL_INT(2, mock.initializeCalls); +} + +// ========================================================================== +// Calibration (T021, US3): factor computation + ×100 register write, +// read-path application (pH/EC yes, moisture no — parity), non-fatal write +// failure, raw-too-low guard, validation on factored values +// ========================================================================== + +// -------------------------------------------------------------------------- +// calibratePH(): fresh 1-register raw read of 0x0003, factor = +// reference / (raw / 10), factor ×100 written to 0x0101, and the factor IS +// applied to every subsequent read() +// -------------------------------------------------------------------------- +static void test_calibrate_ph_factor_write_and_read_effect(void) +{ + Fixture f(goodPayload()); + TEST_ASSERT_TRUE(f.sensor.read()); + TEST_ASSERT_EQUAL_FLOAT(6.8f, f.sensor.getPH()); // pre-calibration + f.mock.calls.clear(); + + // Raw pH register scripted as 68 → 6.8; reference 7.0 → factor 7.0/6.8. + f.mock.setRegisters(kAddr, kRegPh, {68}); + TEST_ASSERT_TRUE(f.sensor.calibratePH(7.0f)); + TEST_ASSERT_EQUAL_INT(0, f.sensor.getLastError()); + + // Exactly one 1-register raw read + one factor write (×100 truncated: + // 7.0/6.8 = 1.0294… → 102), already initialized so no extra probe. + TEST_ASSERT_EQUAL(2, f.mock.calls.size()); + const MockModbusClient::Call &raw = f.mock.calls[0]; + TEST_ASSERT_EQUAL(static_cast(MockModbusClient::Call::Type::Read), + static_cast(raw.type)); + TEST_ASSERT_EQUAL_UINT16(kRegPh, raw.startRegister); + TEST_ASSERT_EQUAL_UINT16(1, raw.count); + const MockModbusClient::Call &write = f.mock.calls[1]; + TEST_ASSERT_EQUAL(static_cast(MockModbusClient::Call::Type::Write), + static_cast(write.type)); + TEST_ASSERT_EQUAL_UINT8(kAddr, write.deviceAddress); + TEST_ASSERT_EQUAL_UINT16(kRegPhCalib, write.startRegister); + TEST_ASSERT_EQUAL_UINT16(1, write.count); + TEST_ASSERT_EQUAL_UINT16(102, write.value); + TEST_ASSERT_TRUE(write.succeeded); + + // Read-path effect: raw 68 → 6.8 × factor = 7.0. + TEST_ASSERT_TRUE(f.sensor.read()); + TEST_ASSERT_EQUAL_FLOAT(7.0f, f.sensor.getPH()); +} + +// -------------------------------------------------------------------------- +// calibrateEC(): same flow with unscaled raw (0x0002, rawScale 1), factor +// ×100 written to 0x0102, factor applied on subsequent reads +// -------------------------------------------------------------------------- +static void test_calibrate_ec_factor_write_and_read_effect(void) +{ + Fixture f(goodPayload()); + TEST_ASSERT_TRUE(f.sensor.read()); + TEST_ASSERT_EQUAL_FLOAT(1200.0f, f.sensor.getEC()); // pre-calibration + f.mock.calls.clear(); + + // Raw EC register scripted as 1000 (unscaled); reference 1500 → + // factor 1.5, written as 150. + f.mock.setRegisters(kAddr, kRegEc, {1000}); + TEST_ASSERT_TRUE(f.sensor.calibrateEC(1500.0f)); + TEST_ASSERT_EQUAL_INT(0, f.sensor.getLastError()); + + TEST_ASSERT_EQUAL(2, f.mock.calls.size()); + const MockModbusClient::Call &write = f.mock.calls[1]; + TEST_ASSERT_EQUAL(static_cast(MockModbusClient::Call::Type::Write), + static_cast(write.type)); + TEST_ASSERT_EQUAL_UINT16(kRegEcCalib, write.startRegister); + TEST_ASSERT_EQUAL_UINT16(150, write.value); + + // Read-path effect: 1200 × 1.5 = 1800. + TEST_ASSERT_TRUE(f.sensor.read()); + TEST_ASSERT_EQUAL_FLOAT(1800.0f, f.sensor.getEC()); +} + +// -------------------------------------------------------------------------- +// calibrateMoisture(): the factor is computed and written to 0x0100 (×100) +// but NOT applied in read() — legacy parity: read() publishes raw / 10 with +// "No calibration factor for humidity yet" (see ModbusSoilSensor.cpp header) +// -------------------------------------------------------------------------- +static void test_calibrate_moisture_factor_written_but_not_applied(void) +{ + Fixture f(goodPayload()); + TEST_ASSERT_TRUE(f.sensor.read()); + TEST_ASSERT_EQUAL_FLOAT(55.0f, f.sensor.getMoisture()); + f.mock.calls.clear(); + + // Raw moisture register scripted as 500 → 50.0 %; reference 55.0 → + // factor 1.1, written as 110. + f.mock.setRegisters(kAddr, kStartReg, {500}); // 1-register key + TEST_ASSERT_TRUE(f.sensor.calibrateMoisture(55.0f)); + TEST_ASSERT_EQUAL_INT(0, f.sensor.getLastError()); + + TEST_ASSERT_EQUAL(2, f.mock.calls.size()); + const MockModbusClient::Call &write = f.mock.calls[1]; + TEST_ASSERT_EQUAL(static_cast(MockModbusClient::Call::Type::Write), + static_cast(write.type)); + TEST_ASSERT_EQUAL_UINT16(kRegMoistureCalib, write.startRegister); + TEST_ASSERT_EQUAL_UINT16(110, write.value); + + // NO read-path effect (parity): raw 550 still publishes 55.0, not 60.5. + TEST_ASSERT_TRUE(f.sensor.read()); + TEST_ASSERT_EQUAL_FLOAT(55.0f, f.sensor.getMoisture()); + TEST_ASSERT_EQUAL_FLOAT(55.0f, f.sensor.getHumidity()); +} + +// -------------------------------------------------------------------------- +// Failed calibration-register WRITE is NON-FATAL (parity): calibrate*() +// still returns true, getLastError() carries the write error, and the +// factor is applied locally on the next read() +// -------------------------------------------------------------------------- +static void test_calibrate_write_failure_nonfatal_factor_applied(void) +{ + Fixture f(goodPayload()); + TEST_ASSERT_TRUE(f.sensor.read()); + f.mock.calls.clear(); + + f.mock.setRegisters(kAddr, kRegPh, {68}); + f.mock.queueOutcome(MockModbusClient::kOk); // raw read succeeds + f.mock.queueOutcome(MockModbusClient::kErrBus); // factor write fails + TEST_ASSERT_TRUE(f.sensor.calibratePH(7.0f)); // still succeeds + TEST_ASSERT_EQUAL_INT(MockModbusClient::kErrBus, f.sensor.getLastError()); + + // The write WAS attempted (and failed) — best-effort, single attempt. + TEST_ASSERT_EQUAL(2, f.mock.calls.size()); + const MockModbusClient::Call &write = f.mock.calls[1]; + TEST_ASSERT_EQUAL(static_cast(MockModbusClient::Call::Type::Write), + static_cast(write.type)); + TEST_ASSERT_EQUAL_UINT16(kRegPhCalib, write.startRegister); + TEST_ASSERT_FALSE(write.succeeded); + + // Factor kept locally despite the failed sensor-register write. + TEST_ASSERT_TRUE(f.sensor.read()); + TEST_ASSERT_EQUAL_FLOAT(7.0f, f.sensor.getPH()); + TEST_ASSERT_EQUAL_INT(0, f.sensor.getLastError()); +} + +// -------------------------------------------------------------------------- +// Raw value < 0.01 → calibration fails with error 5 (division-by-zero +// guard, legacy codes 7/10/13 mapped onto 5), NO factor write, and the +// previous factor (1.0) stays in effect +// -------------------------------------------------------------------------- +static void test_calibrate_raw_too_low_error5_no_write(void) +{ + Fixture f(goodPayload()); + TEST_ASSERT_TRUE(f.sensor.read()); + f.mock.calls.clear(); + + f.mock.setRegisters(kAddr, kRegPh, {0}); // raw 0.0 < 0.01 + TEST_ASSERT_FALSE(f.sensor.calibratePH(7.0f)); + TEST_ASSERT_EQUAL_INT(5, f.sensor.getLastError()); + + // Only the 1-register raw read reached the bus — no write recorded. + TEST_ASSERT_EQUAL(1, f.mock.calls.size()); + TEST_ASSERT_EQUAL(static_cast(MockModbusClient::Call::Type::Read), + static_cast(f.mock.calls.front().type)); + + // Factor unchanged: the next read still publishes the unfactored 6.8. + TEST_ASSERT_TRUE(f.sensor.read()); + TEST_ASSERT_EQUAL_FLOAT(6.8f, f.sensor.getPH()); +} + +// -------------------------------------------------------------------------- +// Validation runs on the FACTORED values (legacy validates after +// multiplying): a factor that pushes pH out of the 3–9 parity range makes +// the next read() fail with error 5 and publish nothing +// -------------------------------------------------------------------------- +static void test_calibration_factor_subject_to_range_validation(void) +{ + Fixture f(goodPayload()); + TEST_ASSERT_TRUE(f.sensor.read()); + TEST_ASSERT_EQUAL_FLOAT(6.8f, f.sensor.getPH()); + + // Raw pH 30 → 3.0; reference 9.0 → factor 3.0 (write value 300). + f.mock.setRegisters(kAddr, kRegPh, {30}); + TEST_ASSERT_TRUE(f.sensor.calibratePH(9.0f)); + + // Data payload pH 6.8 × 3.0 = 20.4 > 9 → range validation rejects the + // whole reading; the last-good (pre-calibration) values survive. + TEST_ASSERT_FALSE(f.sensor.read()); + TEST_ASSERT_EQUAL_INT(5, f.sensor.getLastError()); + TEST_ASSERT_EQUAL_FLOAT(6.8f, f.sensor.getPH()); + TEST_ASSERT_EQUAL_FLOAT(55.0f, f.sensor.getMoisture()); +} + +// -------------------------------------------------------------------------- +// Failed calibration raw READ is fatal: calibrate*() fails with the +// client's error verbatim, exactly one bus attempt (the raw read), no +// factor write, and the next read() still publishes UNfactored values +// -------------------------------------------------------------------------- +static void test_calibrate_raw_read_failure_error_propagated_no_write(void) +{ + Fixture f(goodPayload()); + TEST_ASSERT_TRUE(f.sensor.read()); + f.mock.calls.clear(); + + f.mock.queueOutcome(MockModbusClient::kErrTimeout); // hits the raw read + TEST_ASSERT_FALSE(f.sensor.calibratePH(7.0f)); + TEST_ASSERT_EQUAL_INT(MockModbusClient::kErrTimeout, + f.sensor.getLastError()); + + // Exactly the one failed raw read — no factor write attempted. + TEST_ASSERT_EQUAL(1, f.mock.calls.size()); + TEST_ASSERT_EQUAL(static_cast(MockModbusClient::Call::Type::Read), + static_cast(f.mock.calls.front().type)); + TEST_ASSERT_FALSE(f.mock.calls.front().succeeded); + + // Factor unchanged: the next read still publishes the unfactored 6.8. + TEST_ASSERT_TRUE(f.sensor.read()); + TEST_ASSERT_EQUAL_FLOAT(6.8f, f.sensor.getPH()); +} + +// -------------------------------------------------------------------------- +// Input-hygiene guard (not parity): a non-finite or non-positive reference +// is rejected up front with error 5 — no bus transaction, no factor write, +// factor unchanged +// -------------------------------------------------------------------------- +static void test_calibrate_rejects_invalid_reference(void) +{ + Fixture f(goodPayload()); + TEST_ASSERT_TRUE(f.sensor.read()); + f.mock.calls.clear(); + + TEST_ASSERT_FALSE(f.sensor.calibratePH(NAN)); + TEST_ASSERT_EQUAL_INT(5, f.sensor.getLastError()); + TEST_ASSERT_EQUAL(0, f.mock.calls.size()); // guard fires before the bus + + TEST_ASSERT_FALSE(f.sensor.calibratePH(-1.0f)); + TEST_ASSERT_EQUAL_INT(5, f.sensor.getLastError()); + TEST_ASSERT_EQUAL(0, f.mock.calls.size()); + + // Factor unchanged: the next read still publishes the unfactored 6.8. + TEST_ASSERT_TRUE(f.sensor.read()); + TEST_ASSERT_EQUAL_FLOAT(6.8f, f.sensor.getPH()); +} + +// -------------------------------------------------------------------------- +// Input-hygiene guard (not parity): a factor whose ×100 register encoding +// would exceed uint16 (float→uint16 overflow was UB in the legacy cast) is +// rejected with error 5 BEFORE being stored — no write, factor unchanged. +// Raw pH 1 → 0.1 (passes the raw-too-low guard); reference 7000 → factor +// 70000 → ×100 = 7,000,000 > 65535. +// -------------------------------------------------------------------------- +static void test_calibrate_factor_overflow_rejected_error5(void) +{ + Fixture f(goodPayload()); + TEST_ASSERT_TRUE(f.sensor.read()); + f.mock.calls.clear(); + + f.mock.setRegisters(kAddr, kRegPh, {1}); + TEST_ASSERT_FALSE(f.sensor.calibratePH(7000.0f)); + TEST_ASSERT_EQUAL_INT(5, f.sensor.getLastError()); + + // Only the raw read reached the bus — the overflowing factor was never + // written. + TEST_ASSERT_EQUAL(1, f.mock.calls.size()); + TEST_ASSERT_EQUAL(static_cast(MockModbusClient::Call::Type::Read), + static_cast(f.mock.calls.front().type)); + + // Factor NOT applied: the next read still publishes the unfactored 6.8. + TEST_ASSERT_TRUE(f.sensor.read()); + TEST_ASSERT_EQUAL_FLOAT(6.8f, f.sensor.getPH()); +} + +void run_soil_sensor_tests(void) +{ + RUN_TEST(test_decode_known_payload_negative_temperature); + RUN_TEST(test_decode_positive_temperature); + RUN_TEST(test_humidity_equals_moisture); + RUN_TEST(test_read_is_one_nine_register_transaction); + RUN_TEST(test_timeout_fails_read_and_keeps_last_good_values); + RUN_TEST(test_out_of_range_moisture_rejected_error5); + RUN_TEST(test_out_of_range_temperature_rejected_error5); + RUN_TEST(test_out_of_range_ph_rejected_all_or_nothing); + RUN_TEST(test_out_of_range_high_temperature_rejected_error5); + RUN_TEST(test_out_of_range_low_ph_rejected_error5); + RUN_TEST(test_range_boundaries_are_inclusive); + RUN_TEST(test_ec_npk_not_range_enforced); + RUN_TEST(test_modbus_exception_propagates_verbatim); + RUN_TEST(test_failed_read_is_single_bus_attempt); + RUN_TEST(test_read_recovers_after_failure_without_reinit); + RUN_TEST(test_statistics_count_one_per_call); + RUN_TEST(test_is_available_performs_real_probe_every_call); + RUN_TEST(test_set_timeout_reaches_client); + RUN_TEST(test_lazy_init_client_failure_then_recovery); + RUN_TEST(test_lazy_init_probe_failure_then_recovery); + RUN_TEST(test_calibrate_ph_factor_write_and_read_effect); + RUN_TEST(test_calibrate_ec_factor_write_and_read_effect); + RUN_TEST(test_calibrate_moisture_factor_written_but_not_applied); + RUN_TEST(test_calibrate_write_failure_nonfatal_factor_applied); + RUN_TEST(test_calibrate_raw_too_low_error5_no_write); + RUN_TEST(test_calibration_factor_subject_to_range_validation); + RUN_TEST(test_calibrate_raw_read_failure_error_propagated_no_write); + RUN_TEST(test_calibrate_rejects_invalid_reference); + RUN_TEST(test_calibrate_factor_overflow_rejected_error5); +} diff --git a/specs/004-modbus-soil-sensor/checklists/hil.md b/specs/004-modbus-soil-sensor/checklists/hil.md new file mode 100644 index 0000000..88e361c --- /dev/null +++ b/specs/004-modbus-soil-sensor/checklists/hil.md @@ -0,0 +1,62 @@ +# HIL Checklist: Modbus Soil Sensor (004) — rev1 bench rig + +**Purpose**: hardware-in-the-loop verification of PR-04 at Checkpoint 3 (Paul, bench rig) +**Rig**: ESP32 devkit + RS485 5 Click (ADM3485, manual DE) + NPK soil sensor, 9600 8N1, TX=GPIO16 RX=GPIO17 DE=GPIO25 +**Build**: rev1 target (`sdkconfig.board.rev1_devkit`), flash per `firmware/CLAUDE.md` +**Reference**: acceptance criteria `docs/prd/PR-04-modbus-soil-sensor.md`; spec SC-002/003/005 + +## A. Basic readings (US1, SC-002) + +- [ ] A1. Boot with sensor connected; console `soil` → all 7 values printed + (moisture %, temperature °C, EC µS/cm, pH, N/P/K mg/kg), no error +- [ ] A2. Values match the Arduino unit's readings for the same probe/soil + (within one scaling step, i.e. ±0.1 for moisture/temp/pH, ±1 for EC/NPK) +- [ ] A3. Temperature sign check: chill the probe tip (or verify a plausible + positive value and confirm sign handling was host-tested) — reading is + signed-correct, no 6553.x-style wraparound +- [ ] A4. `rs485test` → success outcome, success counter increments across + repeated runs + +## B. Fault injection & recovery (US2, SC-003) + +- [ ] B1. Disconnect the RS485 A/B pair; `soil` → error after ~3 s (timeout, + error 3), no crash, no watchdog reset, error logged on console +- [ ] B2. Repeat `soil` twice while disconnected → same failure each time, + error counter increments each attempt (no retry storms, no lockup) +- [ ] B3. Reconnect A/B; next `soil` → succeeds without reboot or manual + reinit (SC-003 automatic recovery) +- [ ] B4. Power-cycle the SENSOR only (if rig allows): `soil` fails while + sensor boots, recovers on a later read + +## C. RS485 frame integrity (plan risk 1 — hardware RTS vs TXS0108E margins) + +- [ ] C1. With scope or logic analyzer on DE (GPIO 25) and A/B (optional but + recommended, parity §5 open HIL item): DE asserts before the first start + bit and releases after the last stop bit; request frames are untruncated + at 9600 baud. If truncation appears: STOP, report — fallback is manual + GPIO DE control (research R2), goes back through fixer +- [ ] C2. 20× `soil` in a row → 20 successes (no intermittent framing losses; + check counters via `rs485test`) + +## D. Calibration commands (US4/FR-012, optional bench check) + +- [ ] D1. `soil_cal_ph ` with a known buffer (or plausible dummy + reference) → `OK calibration applied` (or the documented non-fatal + write-failure variant); subsequent `soil` reflects the factored pH +- [ ] D2. `soil_cal_moisture ` → OK, but subsequent `soil` moisture + UNCHANGED (parity: moisture factor stored, never applied on read — + expected behavior, not a bug) + +## E. Regression guard + +- [ ] E1. Pumps still OFF at boot (watch outputs during A1 boot) — safety + invariant untouched by this PR +- [ ] E2. Existing console commands (pump/storage) still respond normally + +## Deferred to PR-14 (rev2 bring-up — NOT this checklist) + +- rev2 THVD1426 echo suppression at 9600 baud (FW-4, research R3) +- rev2 RX pull-up effect with SENS_PWR_EN off (FW-2) +- rev2 auto-direction frame integrity + +**Sign-off**: date + result per item as PR comment (pattern from PR #7). diff --git a/specs/004-modbus-soil-sensor/checklists/requirements.md b/specs/004-modbus-soil-sensor/checklists/requirements.md new file mode 100644 index 0000000..12138fa --- /dev/null +++ b/specs/004-modbus-soil-sensor/checklists/requirements.md @@ -0,0 +1,42 @@ +# Specification Quality Checklist: Modbus Soil Sensor over RS485 + +**Purpose**: Validate specification completeness and quality before proceeding to planning +**Created**: 2026-07-02 +**Feature**: [spec.md](../spec.md) + +## Content Quality + +- [x] No implementation details (languages, frameworks, APIs) +- [x] Focused on user value and business needs +- [x] Written for non-technical stakeholders +- [x] All mandatory sections completed + +## Requirement Completeness + +- [x] No [NEEDS CLARIFICATION] markers remain +- [x] Requirements are testable and unambiguous +- [x] Success criteria are measurable +- [x] Success criteria are technology-agnostic (no implementation details) +- [x] All acceptance scenarios are defined +- [x] Edge cases are identified +- [x] Scope is clearly bounded +- [x] Dependencies and assumptions identified + +## Feature Readiness + +- [x] All functional requirements have clear acceptance criteria +- [x] User scenarios cover primary flows +- [x] Feature meets measurable outcomes defined in Success Criteria +- [x] No implementation details leak into specification + +## Notes + +- "Implementation details" caveat (same stance as specs 002/003): register addresses, + scaling factors, timeout values, GPIO numbers and the `BOARD_HAS_RS485_DE` flag are + **parity contract facts** (`docs/parity-checklist.md` §5) and hardware facts from the + rev2 design review — they define WHAT the system must do, not HOW. References to + esp-modbus/interface names appear only in the Input quote and in Assumptions + (explicitly non-binding "who implements it" note). +- Calibration scope was raised as the single clarification question at Checkpoint 1 + and confirmed by Paul 2026-07-02 (include, exact legacy semantics) — recorded in + the spec's Clarifications section. diff --git a/specs/004-modbus-soil-sensor/contracts/interfaces.md b/specs/004-modbus-soil-sensor/contracts/interfaces.md new file mode 100644 index 0000000..2d3e1f4 --- /dev/null +++ b/specs/004-modbus-soil-sensor/contracts/interfaces.md @@ -0,0 +1,76 @@ +# Interface Contracts: Modbus Soil Sensor (004) + +**Date**: 2026-07-02. These are the host-includable pure C++ contracts (constitution +II). Ported from the legacy headers (`include/communication/IModbusClient.h`, +`include/sensors/ISoilSensor.h`, read-only reference) with the same method surface; +Arduino-era base-class baggage is trimmed to what the new codebase defines. + +## interfaces/IModbusClient.h + +```cpp +class IModbusClient { +public: + virtual ~IModbusClient() = default; + virtual bool initialize() = 0; + virtual bool readHoldingRegisters(uint8_t deviceAddress, uint16_t startRegister, + uint16_t count, uint16_t* buffer) = 0; + virtual bool writeSingleRegister(uint8_t deviceAddress, uint16_t registerAddress, + uint16_t value) = 0; + virtual int getLastError() = 0; // 0 = OK; see data-model error table + virtual void setTimeout(uint32_t timeoutMs) = 0; + virtual void getStatistics(uint32_t* successCount, uint32_t* errorCount) = 0; +}; +``` + +Contract notes: +- `readHoldingRegisters`/`writeSingleRegister` perform exactly ONE bus attempt; + false ⇒ `getLastError()` is set. No internal retry (parity). +- `writeSingleRegister` success means the addressed slave returned a well-formed + FC06 response (address/function/CRC validated); implementations are NOT required + to compare the echoed register/value byte-for-byte against the request (the + legacy client did — documented parity divergence, see EspModbusClient.cpp and + plan.md Risks item 6). +- Statistics: every call increments exactly one of success/error. + +## interfaces/ISoilSensor.h + +```cpp +class ISoilSensor { +public: + virtual ~ISoilSensor() = default; + virtual bool initialize() = 0; + virtual bool read() = 0; // one 9-register transaction + virtual bool isAvailable() = 0; // real 1-register bus probe (parity) + virtual int getLastError() = 0; + // Values from the most recent successful read(): + virtual float getMoisture() = 0; // % + virtual float getTemperature() = 0; // °C, signed + virtual float getHumidity() = 0; // ≡ getMoisture() (parity) + virtual float getPH() = 0; + virtual float getEC() = 0; // µS/cm + virtual float getNitrogen() = 0; // mg/kg + virtual float getPhosphorus() = 0; // mg/kg + virtual float getPotassium() = 0; // mg/kg + // Calibration (CP1 decision A — legacy semantics): + virtual bool calibrateMoisture(float referenceValue) = 0; + virtual bool calibratePH(float referenceValue) = 0; + virtual bool calibrateEC(float referenceValue) = 0; +}; +``` + +Contract notes: +- `read()` false ⇒ data invalid; last-good getter values MUST NOT be mistaken for + fresh (PR-11 gates on the read result/error, per spec FR-005). +- Legacy `setValidRange`/`isWithinValidRange` are NOT ported: ranges are the fixed + parity constants (moisture 0–100, temp −40–80, pH 3–9); no caller in the legacy + firmware ever changed them at runtime. Recorded as a deliberate trim. +- `LockedSoilSensor` decorator provides per-call mutual exclusion (REPL vs main + loop), same pattern/caveats as `LockedWaterPump`/`LockedConfigStore`. + +## Console contract (HIL surface, FR-013) + +| Command | Behavior | +|---|---| +| `soil` | One `read()`; prints all 7 values or error code + name | +| `rs485test` | Raw 1-register probe; prints outcome + success/error counters | +| `soil_cal_moisture ` / `soil_cal_ph ` / `soil_cal_ec ` | Runs calibration; prints success/error plus whether the non-fatal sensor-register write succeeded (no factor value — factors are private, deliberate) | diff --git a/specs/004-modbus-soil-sensor/data-model.md b/specs/004-modbus-soil-sensor/data-model.md new file mode 100644 index 0000000..eacbf36 --- /dev/null +++ b/specs/004-modbus-soil-sensor/data-model.md @@ -0,0 +1,69 @@ +# Data Model: Modbus Soil Sensor over RS485 (004) + +**Date**: 2026-07-02 | **Spec**: [spec.md](spec.md) | **Research**: [research.md](research.md) + +## SoilReading (value object) + +Produced atomically from one 9-register transaction; either fully valid or an error. + +| Field | Type | Source | Validation (read-fails-if) | +|---|---|---|---| +| moisture | float % | reg 0x0000 ÷10 (moisture factor NOT applied on read — legacy parity; `calibrateMoisture` stores/writes the factor but the read path never uses it) | outside 0–100 | +| temperature | float °C | reg 0x0001 as **int16_t** ÷10 | outside −40–80 | +| ec | float µS/cm | reg 0x0002 ×1, × EC factor | not enforced (parity) | +| ph | float | reg 0x0003 ÷10, × pH factor | outside 3–9 | +| nitrogen | float mg/kg | reg 0x0004 ×1 | not enforced (parity) | +| phosphorus | float mg/kg | reg 0x0005 ×1 | not enforced (parity) | +| potassium | float mg/kg | reg 0x0006 ×1 | not enforced (parity) | +| (salinity) | — | reg 0x0007 read, not exposed | — | +| (tds) | — | reg 0x0008 read, not exposed | — | + +Humidity ≡ moisture for this sensor (`getHumidity()` returns moisture, parity). + +**Invariant**: a failed read (bus error, timeout, exception, out-of-range) leaves +the last-good values untouched but flags the sensor invalid; getters after a failed +read must not be presented as fresh (validity flag + error code carry the truth — +consumers in PR-11 gate on validity, never on value plausibility). + +## Error codes (ISoilSensor / IModbusClient contract) + +| Code | Meaning | Parity anchor | +|---|---|---| +| 0 | OK | — | +| 1 | Not initialized | legacy | +| 2 | Bus/communication error (CRC, framing, truncated) | legacy | +| 3 | Timeout (no response within 3000 ms) | legacy | +| 5 | Range validation failed | legacy error 5 | +| 100+n | Modbus slave exception n (when surfaced by esp-modbus; else single generic exception code, documented divergence) | legacy 100+n | + +## CalibrationFactor (per quantity: moisture, pH, EC) + +| Field | Type | Notes | +|---|---|---| +| factor | float, default 1.0 | `reference / rawReading` at calibration time | +| sensor register | 0x0100 / 0x0101 / 0x0102 | best-effort write ×100 via fn 0x06; write failure non-fatal | + +Lifecycle: set by `calibrate*(reference)`; applied on every subsequent read; RAM +only in this PR (persistence → PR-09/PR-11). + +## TransactionStatistics + +`successCount`, `errorCount` (uint32), one increment per `IModbusClient` call +outcome; exposed via `getStatistics()` and the `rs485test` console dump. + +## RS485 board profile (existing, consumed not defined) + +From `firmware/components/board/board.h`: `BOARD_PIN_RS485_TX/RX`, +`BOARD_HAS_RS485_DE` (+ `BOARD_PIN_RS485_DE` iff 1). UART port number is a +`sensors`-component Kconfig/board fact (UART2, parity). + +## State transitions (sensor availability) + +``` +UNINITIALIZED --initialize() ok--> READY +READY --read ok--> READY (valid data, successCount++) +READY --read fail--> READY (invalid flag + error code, errorCount++) [no retry] +any --isAvailable()--> performs real 1-register bus read (parity), no cached state +``` + +No permanent failure state: recovery is implicit in the next read (US2). diff --git a/specs/004-modbus-soil-sensor/plan.md b/specs/004-modbus-soil-sensor/plan.md new file mode 100644 index 0000000..b6c579f --- /dev/null +++ b/specs/004-modbus-soil-sensor/plan.md @@ -0,0 +1,185 @@ +# Implementation Plan: Modbus Soil Sensor over RS485 + +**Branch**: `004-modbus-soil-sensor` | **Date**: 2026-07-02 | **Spec**: [spec.md](spec.md) + +**Input**: Feature specification from `specs/004-modbus-soil-sensor/spec.md` + +## Summary + +Replace the legacy hand-rolled SP3485 Modbus client with esp-modbus 2.1.2 behind +ported `IModbusClient`/`ISoilSensor` interfaces. Decode/validation/calibration logic +lives in a pure-C++ `ModbusSoilSensor` (host-tested); the only hardware-touching +class is `EspModbusClient`, which configures the board-selected UART in RS485 +half-duplex mode — rev1 drives DE via hardware RTS (GPIO 25), rev2 configures no +direction pin, gets its TX echo suppressed by the half-duplex receive gating (FW-4) +and its RX pin pulled up against a floating transceiver output (FW-2). Console +gains `soil`, `rs485test` and calibration commands for HIL. Design decisions and +their rationale: [research.md](research.md). + +## Technical Context + +**Language/Version**: C++ (modern, RAII, no Arduino layers) on ESP-IDF v6.0.1 +(pinned docker image `espressif/idf:v6.0.1`) + +**Primary Dependencies**: `espressif/esp-modbus==2.1.2` (new pin, added to the +`sensors` component's `idf_component.yml`; `dependencies.lock` updated +deliberately), ESP-IDF UART/GPIO drivers, esp_console (existing REPL) + +**Storage**: N/A (calibration factors RAM-only this PR; persistence → PR-09/PR-11) + +**Testing**: host tests on IDF linux preview target (`firmware/test_apps/host`, +exit code = number of failures), CI via esp-idf-ci-action with `target: linux` +(known pitfall: default IDF_TARGET aborts `set-target linux`); HIL checklist on the +rev1 bench rig at Checkpoint 3 + +**Target Platform**: ESP32-WROOM-32E (rev1 devkit rig + rev2 custom PCB), dual +board targets via Kconfig `BOARD_REV1_DEVKIT`/`BOARD_REV2` + +**Project Type**: ESP-IDF component set within existing `firmware/` project + +**Performance Goals**: 9600 baud 8N1 bus; one 9-register transaction well under the +5 s legacy read cadence; no busy-waiting in the driver + +**Constraints**: parity contract `docs/parity-checklist.md` §5 (register map, +scaling, ranges, 3000 ms timeout, NO retry, real-read availability probe, +statistics); rev2 hardware requirements FW-2/FW-4 (spec FR-008/FR-009); pumps/ +safety layer untouched by this PR + +**Scale/Scope**: 1 slave device (addr 0x01), 9 holding registers + 3 calibration +registers; ~2 new interfaces, 1 new component, 3–5 console commands + +## Constitution Check + +*GATE: evaluated pre-Phase 0 and re-checked post-Phase 1 design — PASS (no +violations, no Complexity Tracking entries).* + +- **I. Safety First**: PASS — no pump paths touched. The driver's contribution to + safety is the validity/error contract (FR-005) that PR-11's fail-safe consumes; + invalid-on-timeout and range-validation failure are host-tested here. +- **II. Host-Testability**: PASS — all decode/validation/calibration logic in pure + `ModbusSoilSensor` behind `IModbusClient`; only `EspModbusClient` touches IDF + APIs and contains no business logic; mocks provided; host suite extended in CI. + Echo suppression sits below the interface boundary (UART hardware mode) — argued + in research.md R3, HIL-verified rather than host-tested. +- **III. Reproducible Builds**: PASS — esp-modbus exactly `==2.1.2`, + `dependencies.lock` committed, both board targets built in CI from clean checkout. +- **IV. Frozen Legacy**: PASS — legacy files are read-only porting reference; no + modification. +- **V. Checkpoint-Gated Workflow**: PASS — CP1 held (calibration scope, answered A); + this plan stops at CP2; implementation via implementer subagent; review + CP3 + before commit/PR. +- **VI. English Outward**: PASS — all artifacts/code/commits in English. +- **Additional constraints**: board differences only via `board` component flags + (single `#if BOARD_HAS_RS485_DE` site in `EspModbusClient`); `ESP_LOG*` with + component tag; include guards `WATERINGSYSTEM_*_H`; no partition changes. + +## Project Structure + +### Documentation (this feature) + +```text +specs/004-modbus-soil-sensor/ +├── spec.md +├── plan.md # This file +├── research.md # Phase 0 — 9 resolved decisions (R1–R9) +├── data-model.md # Phase 1 — SoilReading, error codes, calibration, stats +├── contracts/ +│ └── interfaces.md # Phase 1 — IModbusClient, ISoilSensor, console contract +├── quickstart.md # Phase 1 — host-test/build/HIL validation guide +├── checklists/ +│ └── requirements.md +└── tasks.md # Phase 2 (/speckit-tasks — not created by /speckit-plan) +``` + +### Source Code (repository root) + +```text +firmware/ +├── components/ +│ ├── interfaces/include/interfaces/ +│ │ ├── IModbusClient.h # NEW — ported, pure C++ +│ │ └── ISoilSensor.h # NEW — ported, trimmed (see contracts) +│ ├── sensors/ # NEW component (PR-02 actuators pattern) +│ │ ├── CMakeLists.txt # excludes EspModbusClient on linux target +│ │ ├── idf_component.yml # espressif/esp-modbus==2.1.2 +│ │ ├── include/sensors/ +│ │ │ ├── ModbusSoilSensor.h # pure logic: decode/validate/calibrate +│ │ │ ├── LockedSoilSensor.h # mutex decorator (REPL vs main loop) +│ │ │ ├── EspModbusClient.h # esp-modbus + UART/RS485 + pull-up +│ │ │ └── testing/ +│ │ │ ├── MockModbusClient.h # scripted responses/timeouts/exceptions +│ │ │ └── MockSoilSensor.h # for PR-11 consumers +│ │ └── src/ +│ │ ├── ModbusSoilSensor.cpp +│ │ └── EspModbusClient.cpp # target-only +│ └── board/include/board/board.h # + BOARD_RS485_UART_PORT in both profiles (analyze I1) +├── main/ +│ ├── app_main.cpp # wire EspModbusClient + LockedSoilSensor +│ └── diag_console.cpp # + soil, rs485test, soil_cal_* commands +└── test_apps/host/main/ + ├── test_soil_sensor.cpp # NEW — decode/validate/timeout/calibration + └── CMakeLists.txt # register new test file +``` + +**Structure Decision**: one new `sensors` component following the `actuators` +component layout (pure base + hardware class + `Locked*` decorator + `testing/` +mocks); interface headers join the existing `interfaces` component. `EspModbusClient` +is CMake-excluded from the linux/host target the same way `storage` excludes +esp_littlefs (research R7). + +## Key Design Decisions (from research.md) + +| # | Decision | Spec FR | +|---|---|---| +| R1 | esp-modbus 2.x handle API, `mbc_master_send_request` raw requests — no CID dictionary | FR-002 | +| R2 | `UART_MODE_RS485_HALF_DUPLEX` on both boards; RTS = DE pin on rev1, no RTS on rev2; HIL verifies TXS0108E margins (fallback: manual GPIO DE) | FR-007 | +| R3 | rev2 echo suppressed by half-duplex RX gating; no app-level scrubber; PR-14 HIL verifies | FR-008 | +| R4 | Unconditional internal pull-up on RX pin in client init | FR-009 | +| R5 | `response_tout_ms = 3000`; esp-modbus event-driven receive ≈ legacy "extend while arriving"; strictly no retry | FR-006 | +| R6 | esp_err_t → legacy-shaped error codes; 100+n exception range when surfaced, else documented divergence | FR-010 | +| R7 | Component layout (above); decode/validation pure & host-tested | FR-001/014 | +| R8 | Calibration legacy-exact (CP1 answer A), RAM-only factors | FR-012 | +| R9 | Console: `soil`, `rs485test`, `soil_cal_*` via LockedSoilSensor | FR-013 | + +## Risks & Open Items (outcomes recorded during implementation, T028) + +1. **RTS timing vs TXS0108E margins (R2)** — OPEN, HIL item C1/C2 in + `checklists/hil.md`; documented fallback (manual GPIO DE) stays behind + `IModbusClient`. +2. **esp-modbus runtime timeout setter (R5)** — CONFIRMED DIVERGENCE: 2.1.2 takes + the timeout via `ser_opts.response_tout_ms` at create time; `setTimeout()` + applies at initialize() only (documented in code; no runtime caller exists). +3. **Exception-code granularity (R6)** — CONFIRMED DIVERGENCE: 2.1.2 + `mbc_master_send_request` does not surface the slave exception number; all + non-timeout failures map to bus error 2 (distinct-from-timeout holds, which is + the safety-bearing part of FR-010). The sensor layer propagates client codes + verbatim, so 100+n granularity returns automatically if a future client + surfaces it. Noted for the PR description. +4. **Kconfig timeout clamp (R5)** — RESOLVED: `FMB_MASTER_TIMEOUT_MS_RESPOND` + range is 150–30000 ms (default 10000); 3000 fits, no override needed (T002). +5. **rev2 echo behavior unverifiable until PR-14** — accepted per spec assumption; + T3.5 resync is the fallback layer. rev2 target build verified green with + `CONFIG_BOARD_REV2=y` and no DE-pin reference (compile-time proof, T020). +6. **Write-echo verification (FC06)** — CONFIRMED DIVERGENCE (same mechanism as + R6): 2.1.2 `mbc_master_send_request` validates the FC06 response framing + (length/address/function/CRC) but performs no comparison of the echoed + register/value against the request; the legacy client verified the full + 8-byte echo (`src/communication/SP3485ModbusClient.cpp:266-355`, + `docs/parity-checklist.md` §5). Documented in EspModbusClient.cpp and the + interface contract (writeSingleRegister doc softened accordingly). + +Additional parity fact confirmed during the port (also corrected in +data-model.md): the legacy read path never applies the moisture calibration +factor (`calibrateMoisture` stores/writes it only); pH/EC are validated on +factored values. Both ported as-is. + +## Agent context update + +Skipped deliberately: root `CLAUDE.md` carries no `` markers in this +repo and unprompted root-CLAUDE.md edits are blocked by policy; +`.specify/feature.json` (already pointing at `specs/004-modbus-soil-sensor`) is what +downstream spec-kit commands use. + +## Complexity Tracking + +No constitution violations — table intentionally empty. diff --git a/specs/004-modbus-soil-sensor/quickstart.md b/specs/004-modbus-soil-sensor/quickstart.md new file mode 100644 index 0000000..88ff0d9 --- /dev/null +++ b/specs/004-modbus-soil-sensor/quickstart.md @@ -0,0 +1,51 @@ +# Quickstart Validation: Modbus Soil Sensor (004) + +Prerequisites: docker (espressif/idf:v6.0.1). OneDrive tree cannot be mounted by +Docker Desktop — rsync to /tmp first (PR-06 lesson): + +```bash +rsync -a --delete --exclude build --exclude managed_components --exclude sdkconfig \ + "$PWD/firmware/" /tmp/ws004-firmware/ +``` + +## 1. Host tests (CI-equivalent) — [CI] acceptance criteria + +```bash +docker run --rm -v /tmp/ws004-firmware:/project -w /project/test_apps/host \ + espressif/idf:v6.0.1 bash -c "idf.py --preview set-target linux && idf.py build \ + && ./build/host_tests.elf" +``` + +Expected: exit code 0; suite includes soil-sensor cases: decode of a known +9-register payload (incl. negative temperature), range-validation failures +(moisture/temp/pH), invalid-on-timeout via MockModbusClient, exception-code +mapping, calibration factor computation + best-effort write, statistics counters. + +## 2. Both board targets build — [CI] acceptance criterion + +```bash +# rev1 (repeat with BOARD_REV2 for rev2; fullclean + rm sdkconfig between boards) +docker run --rm -v /tmp/ws004-firmware:/project -w /project espressif/idf:v6.0.1 \ + bash -c "idf.py fullclean; rm -f sdkconfig; \ + echo CONFIG_BOARD_REV1_DEVKIT=y >> sdkconfig.defaults.local && idf.py build" +``` + +Expected: both builds green; rev2 build contains no `BOARD_PIN_RS485_DE` reference +(compile-time guaranteed). + +## 3. HIL on the rev1 bench rig — Checkpoint 3 checklist (Paul) + +Rig: devkit + RS485 5 Click + soil sensor at 9600 8N1 (TX 16 / RX 17 / DE 25). + +| # | Step | Expected | +|---|---|---| +| 1 | Flash, open console, run `soil` | All 7 values printed; plausible vs Arduino unit on same probe (SC-002) | +| 2 | Compare each value with the Arduino unit | Match within one scaling step; temperature sign correct | +| 3 | `rs485test` | Success outcome, counters increment | +| 4 | Disconnect A/B, run `soil` | Error after ~3 s (timeout), flagged invalid, logged; no crash/watchdog (SC-003) | +| 5 | Reconnect A/B, run `soil` | Next read succeeds, no reboot needed | +| 6 | `soil_cal_ph ` with known buffer (optional) | Factor updated; write result reported; subsequent `soil` reflects factor | +| 7 | Scope/logic-analyzer on DE (optional) | Frames untruncated at 9600 baud (R2 risk check, parity §5 HIL item) | + +rev2-specific items (echo suppression, RX pull-up effect with SENS_PWR_EN off) are +validated at PR-14 bring-up per spec assumption. diff --git a/specs/004-modbus-soil-sensor/research.md b/specs/004-modbus-soil-sensor/research.md new file mode 100644 index 0000000..9de5d4d --- /dev/null +++ b/specs/004-modbus-soil-sensor/research.md @@ -0,0 +1,185 @@ +# Research: Modbus Soil Sensor over RS485 (004) + +**Date**: 2026-07-02 | **Spec**: [spec.md](spec.md) + +All NEEDS CLARIFICATION items from Technical Context are resolved here. Sources: +esp-modbus docs (context7, `/espressif/esp-modbus`), legacy client +(`src/communication/SP3485ModbusClient.cpp`, read-only reference), parity contract +(`docs/parity-checklist.md` §5), rev2 design review 2026-07-02 (FW-2/FW-4). + +## R1: esp-modbus API generation — 2.x object API + +**Decision**: Use the esp-modbus 2.x handle-based API: `mbc_master_create_serial()` +with `mb_communication_info_t.ser_opts` (port, `MB_RTU`, 9600 baud, +`UART_DATA_8_BITS`, `UART_STOP_BITS_1`, `MB_PARITY_NONE`, +`response_tout_ms = 3000`), then `mbc_master_send_request()` with +`mb_param_request_t` for raw register access. No CID/data-dictionary layer. + +**Rationale**: Version pin 2.1.2 is mandated by the PRD. The request-based API maps +1:1 onto the ported `IModbusClient` (`readHoldingRegisters` → command 0x03, +`writeSingleRegister` → command 0x06) without inventing a parameter dictionary the +system doesn't need — the soil sensor is one device with one fixed register window. +`mbc_master_send_request` returns `ESP_ERR_TIMEOUT` on no-response, distinct codes +for invalid responses, satisfying FR-005/FR-010 error discrimination. + +**Alternatives considered**: +- *CID data dictionary (`mbc_master_get_parameter`)*: adds a static descriptor table + and float conversion machinery for no benefit — decode/scaling must live in + host-testable `ModbusSoilSensor` logic anyway (parity scaling is bespoke). +- *Raw UART port of the legacy client*: rejected by the PRD (esp-modbus is the point + of this PR); would re-own CRC/framing/timing code that esp-modbus already provides. + +## R2: RS485 direction control — UART half-duplex mode on both boards + +**Decision**: Configure the Modbus UART in `UART_MODE_RS485_HALF_DUPLEX` on **both** +boards, differing only in the RTS pin wiring: +- rev1 (`BOARD_HAS_RS485_DE == 1`): `uart_set_pin(..., rts = BOARD_PIN_RS485_DE)` — + the UART peripheral drives DE via RTS automatically around each frame. +- rev2 (`BOARD_HAS_RS485_DE == 0`): `uart_set_pin(..., rts = UART_PIN_NO_CHANGE)` — + no direction pin exists; the THVD1426 auto-directs. + +Call order per the pinned esp-modbus serial-master example: create → `uart_set_pin` +→ start → `uart_set_mode(UART_MODE_RS485_HALF_DUPLEX)`. + +**Rationale**: Satisfies FR-007 with zero application-level `#ifdef` logic beyond the +pin selection (constitution: board differences only via the board component). The +`#if BOARD_HAS_RS485_DE` guard lives in one place in `EspModbusClient`; rev2 builds +never reference `BOARD_PIN_RS485_DE` (it is undefined there — compile-time enforced +by the existing board.h sanity checks). + +**Risk (HIL-verified)**: hardware-timed RTS switches within microseconds of frame +start/end, whereas the legacy driver gave the TXS0108E-shifted DE a 50 µs (2× +applied = 100 µs) assert margin and 50 µs release margin. At 9600 baud one bit is +104 µs, and the TXS0108E propagates in nanoseconds, so margins should hold — but +this is exactly the parity-checklist §5 open HIL item ("frames still complete +without truncation"). If HIL shows truncation on rev1, fallback is manual DE via a +GPIO toggled around `uart_wait_tx_done()` (legacy-equivalent timing), still behind +`IModbusClient`. + +**Alternatives considered**: manual GPIO DE control as primary (rejected: reproduces +the code esp-modbus/UART hardware already provides and adds a race-prone task-timing +dependency); `UART_MODE_RS485_APP_CTRL` (rejected: lowest-level option, no need). + +## R3: rev2 TX echo (FW-4) — suppressed by half-duplex mode, verified at HIL + +**Decision**: Rely on `UART_MODE_RS485_HALF_DUPLEX` receive gating for echo removal: +in this mode the ESP32 UART ignores the receive path while the transmitter is +active, so the echo the THVD1426's always-on receiver (RE̅ grounded) feeds back +during transmission never reaches the driver. No application-level echo scrubber is +written up front. A rev2 HIL checklist item (deferred to PR-14 with the rest of +rev2 electrical validation, per spec assumption) verifies no echo bytes leak; the +esp-modbus RTU state machine's T3.5 frame resynchronization is the second line of +defense against residual tail bytes. + +**Rationale**: The echo is physically simultaneous with transmission (transceiver +loopback, not a store-and-forward reflection), so TX-gated receive removal is +complete by construction. Writing a speculative echo-scrubber above esp-modbus would +be dead code below our own test boundary — and FR-008's observable contract is +"only the sensor's reply is parsed", not "an echo scrubber exists". + +**Spec note (FR-014)**: this resolves "echo discarding" at a layer beneath +host-testable code; host tests therefore cover decode/validation/timeout/exception +paths against the mock client, and echo correctness is a HIL concern (PR-14). The +spec's FR-014 echo-discard mention is satisfied by testing that the sensor logic +parses exactly one well-formed reply per transaction from the mock — the +host-observable equivalent. + +## R4: RX pull-up (FW-2) — unconditional `gpio_pullup_en` on the RX pin + +**Decision**: After UART pin setup, enable the internal pull-up on +`BOARD_PIN_RS485_RX` (IO17) unconditionally on both boards, in `EspModbusClient` +initialization. + +**Rationale**: FR-009. On rev2 the THVD1426 SHDN̅ tracks `SENS_PWR_EN`; RO goes hi-Z +when the sensor domain is off and IO17 would otherwise float into the UART RX, +producing garbage bytes/noise. The transceiver's RO output drives through a weak +pull-up when active, so leaving it enabled permanently is harmless on both boards +(rev1's ADM3485 RO likewise drives push-pull). Doing it in the client (not the board +component) keeps the board component a pure pin/flag table, per PR-02's design. + +## R5: Timeout semantics — `response_tout_ms = 3000`, parity equivalence documented + +**Decision**: Set `ser_opts.response_tout_ms = 3000` at create time and route +`IModbusClient::setTimeout()` to the esp-modbus runtime timeout setter if 2.1.2 +exposes one; if it does not, `setTimeout` is honored at initialization only and +documented as such (implementer verifies against the pinned component; the console +never changes the timeout at runtime today). Verify that Kconfig +`CONFIG_FMB_MASTER_TIMEOUT_MS_RESPOND`'s upper bound does not clamp 3000 in +`sdkconfig.defaults`. + +**Rationale**: FR-006 requires a 3000 ms default (parity). The legacy +"timeout extended while bytes arrive" behavior is provided equivalently by +esp-modbus's event-driven receive path (inter-character T1.5 / frame T3.5 timers +mean a trickling frame is not cut off mid-reception); the observable contract — +slow-but-arriving replies survive, absent replies fail at ~3000 ms — holds. Exactly +one bus attempt per call: esp-modbus master performs no application-level retry for +`send_request`, matching the no-retry parity rule; the implementer MUST NOT add +retry loops. + +## R6: Error-code mapping — esp_err_t → legacy-shaped error codes + +**Decision**: `EspModbusClient` maps esp-modbus results onto the `IModbusClient` +error contract: 0 = OK, distinct codes for timeout (`ESP_ERR_TIMEOUT`), invalid +response/CRC (`ESP_ERR_INVALID_RESP`-class), and Modbus slave exceptions. Slave +exceptions map to the legacy 100+n range when the underlying API surfaces the +exception code; if 2.1.2 reports exceptions only as a generic invalid-response +error, they map to a single documented "slave exception/invalid response" code. + +**Rationale**: FR-010's binding requirement is *distinct from timeout* (fail-safe +diagnostics need to distinguish "sensor absent" from "sensor confused"). The legacy +100+exception granularity is preserved when the API allows; otherwise the coarser +mapping is recorded as a parity divergence in the PR (same mechanism as PR-06's +documented divergences). Statistics counters (FR-013) are counted in +`EspModbusClient` exactly like the legacy client: one success or one error per call. + +## R7: Component layout & decode placement — new `sensors` component, pure logic base + +**Decision**: Follow the PR-02 actuators pattern: +- `firmware/components/interfaces/include/interfaces/`: `IModbusClient.h`, + `ISoilSensor.h` (ported, pure C++, host-includable; drop the legacy `ISensor` + Arduino heritage in favor of the minimal base the new codebase needs). +- New `firmware/components/sensors/`: `ModbusSoilSensor` (pure logic: register + decode incl. signed temperature, scaling, range validation, calibration factors, + availability probe — depends only on `IModbusClient`), `LockedSoilSensor` + (mutex decorator, REPL + future controller access), `testing/MockModbusClient.h` + + `testing/MockSoilSensor.h`. +- `EspModbusClient` (the only file touching esp-modbus/UART/GPIO APIs) in the same + `sensors` component but excluded from the linux/host build via the same + CMake target-guard used by `storage` for esp_littlefs. +- esp-modbus dependency pinned `espressif/esp-modbus==2.1.2` in the component's + `idf_component.yml`; `dependencies.lock` updated deliberately (constitution III). + +**Rationale**: Constitution II — everything above `IModbusClient` is host-testable +by construction; the hardware-touching client contains no business logic. The +decode/validation/calibration logic is exactly what PR-11's fail-safe tests consume. + +**Alternatives considered**: separate `modbus` component for the client (rejected +for now: one consumer; can be split when the level sensors/INA226 PR needs shared +bus infrastructure — noted for PR-05). + +## R8: Calibration semantics (CP1 answer A) — legacy-exact + +**Decision**: Port legacy behavior verbatim: `calibrateMoisture/PH/EC(reference)` +compute `factor = reference / rawReading` from a fresh read, apply the factor +locally to subsequent reads (EC and pH per parity map; moisture factor exists in +legacy and is ported as-is), and best-effort write the factor (×100, per legacy +encoding) to sensor registers 0x0100/0x0101/0x0102 via function 0x06 with echo +verification — a failed write logs a warning and returns success for the local +part (non-fatal, parity). Factors live in memory; persistence is wired when +configuration consumers land (PR-09/PR-11). + +**Rationale**: CP1 decision (option A), parity checklist §5. Implementer ports the +exact legacy formula from `src/sensors/ModbusSoilSensor.cpp:207-322` (read-only +reference) rather than re-deriving it. + +## R9: Console diagnostics — extend `diag_console.cpp` + +**Decision**: Add `soil` (full decoded reading + validity/error), `rs485test` (raw +single-transaction bus probe + statistics counters dump) to the existing esp-console +REPL in `firmware/main/diag_console.cpp`, following the command style PR-02/PR-06 +established. Calibration console commands (`soil_cal_moisture ` etc.) included +since calibration is in scope. + +**Rationale**: FR-013; same HIL surface Paul already uses on the rig. Access to the +shared sensor goes through `LockedSoilSensor` (REPL vs future main-loop, the PR-02 +race lesson). diff --git a/specs/004-modbus-soil-sensor/spec.md b/specs/004-modbus-soil-sensor/spec.md new file mode 100644 index 0000000..4103b62 --- /dev/null +++ b/specs/004-modbus-soil-sensor/spec.md @@ -0,0 +1,308 @@ +# Feature Specification: Modbus Soil Sensor over RS485 + +**Feature Branch**: `004-modbus-soil-sensor` + +**Created**: 2026-07-02 + +**Status**: Draft + +**Input**: User description: "Replace the hand-rolled SP3485 Modbus client with esp-modbus +(pinned 2.1.2) behind IModbusClient/ISoilSensor interfaces, supporting both rev1 (manual DE +via RTS on GPIO 25) and rev2 (THVD1426 auto-direction, no DE pin) RS485 hardware, per +docs/prd/PR-04-modbus-soil-sensor.md (authoritative mini-PRD; ground truth for behavior is +docs/parity-checklist.md §5) plus hardware-driven requirements FW-2 and FW-4 from the rev2 +design review 2026-07-02." + +## Clarifications + +### Session 2026-07-02 + +- Q: Are the legacy calibration commands (moisture/pH/EC factor from reference value, + best-effort write to sensor registers 0x0100–0x0102 via function 0x06) in scope for + PR-04, given the mini-PRD scope list omits them but `docs/parity-checklist.md` §5 + lists them under this sensor? → A: Include with exact legacy semantics (option A); + factors held in memory this PR, store persistence wired when configuration + consumers land (PR-09/PR-11). Confirmed by Paul at Checkpoint 1. + +## User Scenarios & Testing *(mandatory)* + +### User Story 1 - Soil readings on the bench rig (Priority: P1) + +Paul flashes the rev1 rig, which has the real RS485 soil sensor wired to it. From the +serial console he triggers a soil reading and gets all seven used values (moisture, +temperature, EC, pH, N, P, K) with the same numbers the production Arduino unit shows +for the same sensor — including correct negative temperatures. A bus-level diagnostic +command shows raw transaction health for troubleshooting wiring. + +**Why this priority**: Soil moisture is the primary input to every watering decision +(PR-11); a driver that reads the real sensor correctly on the rig is the core +deliverable of this PR and the second hardware-in-the-loop milestone of phase 1. + +**Independent Test**: Flash the rig with the sensor attached, issue the soil diagnostic +command, compare each value against the Arduino unit's readings for the same probe. + +**Acceptance Scenarios**: + +1. **Given** the rig with the soil sensor connected, **When** the operator requests a + soil reading, **Then** all nine registers are fetched in one transaction and the + seven used values are reported with correct scaling (moisture ÷10 → %, temperature + ÷10 → °C signed, EC ×1 µS/cm, pH ÷10, N/P/K ×1 mg/kg). +2. **Given** a sensor reporting a below-zero temperature, **When** the value is read, + **Then** it is reported as the correct negative number (signed interpretation). +3. **Given** the rig, **When** the operator runs the bus diagnostic command, **Then** + the output shows transaction success/failure and enough detail to distinguish "no + response" from "bad response". + +--- + +### User Story 2 - Sensor faults yield invalid data, never wrong data (Priority: P2) + +A wire comes loose in the greenhouse (or the sensor dies). The system does not crash, +does not hang, and does not present stale or garbage numbers as truth: the reading is +flagged invalid with a logged error, and when the wire is reconnected the very next +read cycle recovers without a reboot. The downstream fail-safe logic (PR-11) can rely +on this validity signal to stop watering. + +**Why this priority**: Constitution principle I (Safety First) — invalid sensor data +must stop the pumps in automatic mode. That fail-safe is only as good as the validity +flag this driver produces. + +**Independent Test**: On the rig, disconnect the RS485 A/B pair mid-operation, observe +invalid readings + logged errors and no crash; reconnect and observe automatic recovery +on a subsequent read. + +**Acceptance Scenarios**: + +1. **Given** a connected sensor, **When** the A/B pair is disconnected and a read is + attempted, **Then** the read fails with a timeout error after the configured + response timeout (3000 ms default), the result is flagged invalid, and the failure + is logged — no crash, no watchdog reset. +2. **Given** a disconnected sensor, **When** the pair is reconnected, **Then** the next + read attempt succeeds without any manual intervention or restart. +3. **Given** a sensor response with an out-of-range value (moisture outside 0–100 %, + temperature outside −40–80 °C, pH outside 3–9), **When** the reading is validated, + **Then** the read fails with a distinct validation error and no partial values are + presented as valid. +4. **Given** a failing read, **When** the driver reports the failure, **Then** exactly + one bus attempt was made (no automatic retry — recovery comes from the caller's + read cadence, parity with the Arduino client). +5. **Given** the sensor answers with a Modbus exception response, **When** the reply is + parsed, **Then** it maps to a distinct error code (not a generic timeout). + +--- + +### User Story 3 - One driver, two RS485 hardware generations (Priority: P3) + +An AI developer (or Paul) builds the firmware for either board revision. On rev1 the +transceiver needs explicit transmit/receive direction control on a dedicated pin with +safe switching margins; on rev2 the transceiver switches direction automatically, has +no direction pin at all, and — because its receiver is always on — every transmitted +frame is heard back on the receive line. The same driver serves both: the board +abstraction decides direction handling, the rev2 build discards its own transmit echo +before parsing replies, and the receive line is kept from floating when the rev2 +switched sensor power domain is off. + +**Why this priority**: Dual-board support is the reason this driver is being rewritten +at all (the legacy client is rev1-only). The rev2-specific behaviors (echo, floating +RX) come straight from the 2026-07-02 hardware design review and must be encoded now so +rev2 bring-up (PR-14) is a validation exercise, not a rewrite. + +**Independent Test**: Build both board variants in CI; verify the rev1 binary +configures the direction pin and the rev2 binary does not; verify by host test that +exactly one well-formed reply is parsed per transaction (single-reply parsing via +the mock — echo suppression itself is hardware receive-gating, verified +electrically at PR-14 per plan decision R3/FR-014). + +**Acceptance Scenarios**: + +1. **Given** a rev1 build, **When** the driver initializes, **Then** direction control + uses the board-defined direction pin (GPIO 25) and transmitted frames complete + without truncation at 9600 baud (direction-switch margins hold). +2. **Given** a rev2 build, **When** the driver initializes, **Then** no direction pin + is configured or touched (the pin macro does not even exist — compile-time + guarantee). +3. **Given** a rev2 build, **When** a request frame is transmitted, **Then** the echo + of that frame never reaches the reply parser and only the sensor's reply is + parsed — echo suppression is hardware receive-gating (RS485 half-duplex TX + gating, plan decision R3), verified electrically at PR-14; host tests verify + single-reply parsing (exactly one well-formed reply per transaction) via the + mock (FR-014). +4. **Given** a rev2 board with the sensor power domain switched off, **When** the + receive line would otherwise float, **Then** the internal pull-up on the RX pin + keeps the line idle-high so no garbage bytes accumulate. + +--- + +### User Story 4 - Sensor behavior testable without hardware (Priority: P4) + +An AI developer changes decode, validation or error-handling logic and runs the host +test suite in CI. Register decoding (including signed temperature), range validation, +invalid-on-timeout behavior and single-reply parsing are verified against mock +implementations — no devkit, no sensor, failures block the merge. + +**Why this priority**: Constitution principle II (Host-Testability). The mock soil +sensor created here is also the input PR-11's watering-controller tests build on. + +**Independent Test**: Run the host test suite on a machine with no hardware attached; +decode/validation/timeout tests pass deterministically. + +**Acceptance Scenarios**: + +1. **Given** a mock Modbus client returning a known 9-register payload, **When** the + sensor logic decodes it, **Then** every value matches the expected scaled result, + including a negative temperature case. +2. **Given** a mock client that times out, **When** a reading is requested, **Then** + the result is invalid with a timeout error and no stale values leak through. +3. **Given** a mock client returning out-of-range values, **When** the reading is + validated, **Then** the read fails with a validation error. +4. **Given** CI on a clean checkout, **When** the host tests run, **Then** they + complete without any hardware or device-specific environment. + +--- + +### Edge Cases + +- Spurious/noise bytes on the bus (e.g. from the rev2 sensor domain powering on or + timing-offset garbage) must not permanently poison subsequent transactions: the next + well-formed transaction succeeds. (The legacy client tolerates leading garbage by + scanning for the address+function pattern; equivalent resilience is required.) +- Echo handling when the reply arrives back-to-back with the echo: the boundary between + echoed request and real reply must not be misparsed. +- A reply shorter than expected (truncated frame) or with a bad CRC fails the read with + an error — never a partially-decoded "valid" reading. +- Response timeout while bytes are still trickling in: the timeout window is extended + while reception is in progress (parity), so a slow-but-arriving reply is not cut off. +- Registers 0x0007 (salinity) and 0x0008 (TDS) are read as part of the block but not + exposed as values (parity: read but unused). +- Concurrent read requests (e.g. console diagnostic while a periodic read is active in + a later PR): transactions must serialize on the bus, never interleave. +- Sensor availability probing must be a real bus read (parity), not a cached flag — + otherwise a dead sensor looks alive. + +## Requirements *(mandatory)* + +### Functional Requirements + +- **FR-001**: Modbus client and soil sensor functionality MUST be exposed through + hardware-independent interfaces (ported `IModbusClient`/`ISoilSensor`, pure C++, no + hardware-SDK types) usable from host tests without any hardware headers. +- **FR-002**: The soil sensor driver MUST read device address 0x01 with function 0x03, + one transaction of 9 holding registers starting at 0x0000, over the board-defined + RS485 UART at 9600 baud 8N1. Pin source of truth is the board component (rev1: + TX 16 / RX 17 / DE 25 per `docs/parity-checklist.md`; `docs/hardware.md` is known + swapped — QUIRK 6). +- **FR-003**: Register decoding MUST apply the parity scaling map: 0x0000 moisture ÷10 + → % (moisture ≡ humidity for this sensor); 0x0001 temperature ÷10 → °C **signed**; + 0x0002 EC ×1 → µS/cm × calibration factor; 0x0003 pH ÷10 × calibration factor; + 0x0004–0x0006 N/P/K ×1 → mg/kg; 0x0007–0x0008 read but unused. +- **FR-004**: Range validation MUST fail the read (distinct error) when moisture is + outside 0–100 %, temperature outside −40–80 °C, or pH outside 3–9. EC and N/P/K + ranges are NOT enforced on read (parity with the Arduino validation). +- **FR-005**: Every reading MUST carry an explicit validity signal (valid data XOR + error code) that downstream fail-safe logic (PR-11) can consume; a failed read MUST + never present stale or partial values as valid. +- **FR-006**: The response timeout MUST default to 3000 ms and be configurable; the + timeout window MUST be extended while response bytes are arriving. Exactly **one** + bus attempt per call — the driver MUST NOT retry automatically (parity; + `docs/parity-checklist.md` §5). Any future retry mechanism would be a deliberate + behavior change, out of scope here. +- **FR-007**: Transmit/receive direction handling MUST be selected by the board + component's `BOARD_HAS_RS485_DE` flag: rev1 drives the board-defined direction pin + around each transmission such that frames complete without truncation at 9600 baud + (the legacy 50 µs assert/release margins are the reference behavior); rev2 configures + **no** direction pin, and rev2 builds MUST NOT reference a direction pin at all + (compile-time enforced by the existing board sanity checks). +- **FR-008**: On boards without a direction pin (rev2), the driver MUST discard the + echo of its own transmitted frame from the receive path before parsing the reply + (THVD1426 receiver is always enabled; FW-4, rev2 design review 2026-07-02). +- **FR-009**: The internal pull-up on the RS485 RX pin MUST be enabled so the line does + not float when the transceiver's receiver output is high-impedance (on rev2 the + transceiver shuts down with the switched sensor power domain; FW-2, rev2 design + review 2026-07-02). Enabling it unconditionally on both boards is acceptable — the + receiver output drives through it. +- **FR-010**: Malformed traffic MUST degrade gracefully: bad CRC, truncated frames, + Modbus exception responses (mapped to distinct error codes, parity: 100 + exception + code) and spurious leading bytes each fail only the current transaction; the next + well-formed transaction MUST succeed without reinitialization. +- **FR-011**: Sensor availability checks MUST perform an actual bus read (parity), not + return cached state. +- **FR-012**: Calibration MUST follow parity: per-quantity calibration factors for + moisture, pH and EC are computed locally from an operator-supplied reference value + and best-effort written to sensor registers 0x0100–0x0102 (function 0x06, echo + verified); a failed sensor write is non-fatal (the local factor still applies). +- **FR-013**: Serial diagnostics equivalent to the legacy `rs485test` and `soil` + commands MUST exist on the rig console: trigger a raw bus test transaction and a + full decoded soil reading, and expose the client's success/error statistics counters + (parity) for troubleshooting. +- **FR-014**: Mock implementations of the Modbus client and soil sensor MUST exist for + host tests, sufficient to deterministically test register decoding (incl. signed + temperature), range validation, invalid-on-timeout, exception mapping and + single-reply parsing (exactly one well-formed reply parsed per transaction — the + host-observable equivalent of rev2 echo discarding, which is resolved at the + hardware receive-gating layer per plan decision R3 and verified electrically at + PR-14) in CI. +- **FR-015**: Both board targets MUST build green in CI — rev1 with the direction pin + configured, rev2 without. + +### Key Entities + +- **Soil reading**: the seven used values (moisture %, temperature °C, EC µS/cm, pH, + N/P/K mg/kg) plus validity/error state; produced atomically from one 9-register + transaction. +- **Modbus transaction**: one request/response exchange with a single device — address, + function, register window, outcome (success, timeout, CRC error, exception, + validation failure), contributing to success/error statistics. +- **RS485 board profile**: the board component's UART pins and direction-control flag + (`BOARD_HAS_RS485_DE`); the single source of truth the driver configures itself from. +- **Calibration factor**: per-quantity multiplier (moisture, pH, EC) derived from an + operator reference value; applied locally on read, best-effort mirrored to the + sensor. + +## Success Criteria *(mandatory)* + +### Measurable Outcomes + +- **SC-001**: Both board variants build green in CI from a clean checkout; the rev2 + binary provably contains no direction-pin handling (compile-time check). +- **SC-002**: On the rig, a soil reading returns all seven used values matching the + Arduino unit's readings for the same sensor (same probe, same soil) on every + attempt of the HIL checklist, including the signed-temperature spot check. +- **SC-003**: Disconnecting the A/B pair produces flagged-invalid readings and logged + errors with zero crashes or watchdog resets; reconnection recovers automatically on + a subsequent read with no operator action (HIL checklist). +- **SC-004**: Host test suite covering decode, validation, timeout and single-reply + parsing (echo suppression itself is hardware receive-gating, verified + electrically at PR-14 per plan decision R3/FR-014) runs in CI with zero hardware + dependencies and passes deterministically (no flaky reruns). +- **SC-005**: Operator can run the bus diagnostic and soil reading commands on the + first attempt using documented commands, and the output is sufficient to distinguish + wiring faults from sensor faults (HIL checklist). + +## Assumptions + +- **Calibration is in scope** (confirmed at Checkpoint 1, see Clarifications): + included with exact legacy semantics (local factor + best-effort sensor write) since + `docs/parity-checklist.md` §5 lists the calibration commands as `[HOST]` items under + this sensor and no other PR covers Modbus writes. Calibration factors are held in + memory in this PR; persisting them via the configuration store is wired when + configuration consumers land (PR-09/PR-11). +- **Read cadence stays out of scope**: the legacy 5 s periodic read loop is + controller-level behavior (PR-11). This PR delivers on-demand reads (console + diagnostics + API for later PRs); no periodic task is added. +- **rev2 electrical validation is deferred**: THVD1426 behavior at 9600 baud on real + rev2 hardware (echo timing, auto-direction margins) is validated at PR-14 bring-up. + This PR covers rev2 at build level and echo/pull-up logic at host-test level, per + the mini-PRD's out-of-scope note. +- **The switched sensor power domain (`SENS_PWR_EN`) is not managed here**: rev2 power + gating of the sensor rail belongs to the board/power layer (rev2 board profile, + PR-14). This driver only guarantees it behaves correctly when the domain is off + (pull-up, FR-009) and after it turns on. +- **esp-modbus internals may satisfy some requirements natively** (e.g. direction + control via RTS, echo suppression in half-duplex mode): where the pinned component + already provides a required behavior, verifying that behavior counts as + implementing the requirement — the observable contract above is what is binding, + not who implements it. +- The legacy client's tolerant response parsing (scanning the first bytes for the + address+function pattern) is a means, not an end: the binding requirement is + garbage-resilience per FR-010, achieved by whatever mechanism the new client stack + provides. diff --git a/specs/004-modbus-soil-sensor/tasks.md b/specs/004-modbus-soil-sensor/tasks.md new file mode 100644 index 0000000..72b5840 --- /dev/null +++ b/specs/004-modbus-soil-sensor/tasks.md @@ -0,0 +1,221 @@ +# Tasks: Modbus Soil Sensor over RS485 + +**Input**: Design documents from `specs/004-modbus-soil-sensor/` +**Prerequisites**: plan.md, research.md (R1–R9), data-model.md, contracts/interfaces.md, quickstart.md + +**Tests**: included — the spec's [CI] acceptance criteria explicitly require host +tests (FR-014); test tasks precede/accompany implementation per constitution II. + +**Process rules (PR-06 lessons)**: implementer agent missions are WRITE-ONLY — no +docker builds inside agent missions. Every task marked **[VERIFY-MAIN]** is executed +by the orchestrator in the main session via Bash (docker + rsync-to-/tmp pattern, +see quickstart.md). Commit after each phase at minimum; tasks are sized to survive +agent death. + +**Organization**: tasks grouped by user story (US1 readings, US2 fault handling, +US3 dual-board, US4 host-testability) so each story is independently verifiable. + +## Phase 1: Setup + +- [x] T001 Create `sensors` component skeleton: `firmware/components/sensors/CMakeLists.txt` + (REQUIRES `interfaces`, `board`; register `src/ModbusSoilSensor.cpp` always and + `src/EspModbusClient.cpp` only when `IDF_TARGET != linux`, same guard style as + `firmware/components/storage/CMakeLists.txt`) and + `firmware/components/sensors/idf_component.yml` pinning `espressif/esp-modbus: "==2.1.2"` +- [x] T002 [VERIFY-MAIN] Confirm `CONFIG_FMB_MASTER_TIMEOUT_MS_RESPOND` bounds allow + 3000 ms in esp-modbus 2.1.2 Kconfig (inspect managed component after a + dependency fetch) and add any required override to `firmware/sdkconfig.defaults` + (plan risk 4) + +## Phase 2: Foundational (blocking prerequisites for all stories) + +- [x] T003 [P] Port `IModbusClient` to + `firmware/components/interfaces/include/interfaces/IModbusClient.h` per + contracts/interfaces.md (pure C++, guard `WATERINGSYSTEM_INTERFACES_IMODBUSCLIENT_H`, + SPDX header, contract notes as doc comments: one attempt/no retry, write echo + verification, statistics semantics) +- [x] T004 [P] Port `ISoilSensor` to + `firmware/components/interfaces/include/interfaces/ISoilSensor.h` per + contracts/interfaces.md (trimmed surface — no setValidRange/isWithinValidRange; + document the trim and the read()/validity contract) +- [x] T005 [P] Create `MockModbusClient` in + `firmware/components/sensors/include/sensors/testing/MockModbusClient.h`: + scriptable register payloads per (address, startRegister, count), forced + timeout/CRC/exception errors, call recording (incl. writeSingleRegister log), + statistics counters — style of `actuators/testing` and `storage/testing` mocks + +**Checkpoint**: interfaces + mock exist — all story phases can start. + +## Phase 3: User Story 1 — Soil readings on the bench rig (P1) 🎯 MVP + +**Goal**: real sensor read on the rig: all 9 registers in one transaction, parity +scaling incl. signed temperature, console `soil`/`rs485test` output. + +**Independent test**: quickstart.md §1 decode tests green + §3 HIL steps 1–3. + +- [x] T006 [P] [US1] Write host tests for register decode in + `firmware/test_apps/host/main/test_soil_sensor.cpp`: known 9-register payload + decodes to expected moisture/temp/EC/pH/N/P/K (data-model scaling table), + negative temperature case (0xFF38 → −20.0 °C), humidity ≡ moisture, + salinity/TDS not exposed; register in + `firmware/test_apps/host/main/CMakeLists.txt` +- [x] T007 [US1] Implement `ModbusSoilSensor` (pure logic) in + `firmware/components/sensors/include/sensors/ModbusSoilSensor.h` + + `firmware/components/sensors/src/ModbusSoilSensor.cpp`: initialize/read + (one readHoldingRegisters(0x01, 0x0000, 9) call), decode per data-model.md, + getters from last successful read, real-bus-read isAvailable() (1 register, + parity), getLastError; port reference `src/sensors/ModbusSoilSensor.cpp` + (READ-ONLY legacy) +- [x] T008 [US1] Implement `EspModbusClient` core in + `firmware/components/sensors/include/sensors/EspModbusClient.h` + + `firmware/components/sensors/src/EspModbusClient.cpp`: R1 create/start sequence + (`mbc_master_create_serial`, ser_opts 9600 8N1 `response_tout_ms=3000`, UART + port + pins from `board/board.h` — add `BOARD_RS485_UART_PORT` (2, parity + UART2) to BOTH board profiles in + `firmware/components/board/include/board/board.h`, analyze finding I1), + `uart_set_mode(UART_MODE_RS485_HALF_DUPLEX)`, + RTS = `BOARD_PIN_RS485_DE` under `#if BOARD_HAS_RS485_DE` else no RTS pin (R2), + readHoldingRegisters/writeSingleRegister via `mbc_master_send_request` + (commands 0x03/0x06), esp_err_t → error-code mapping per data-model.md (R6), + statistics counters +- [x] T009 [US1] Add `LockedSoilSensor` decorator in + `firmware/components/sensors/include/sensors/LockedSoilSensor.h` (per-call + mutex, pattern of `actuators/LockedWaterPump.h`; document per-call-not- + cross-call atomicity like `storage/Locked*`) +- [x] T010 [US1] Wire sensor into app in `firmware/main/app_main.cpp`: + construct `EspModbusClient` + `ModbusSoilSensor` + `LockedSoilSensor` after + storage init; boot log line with client init result; no periodic read task + (out of scope, PR-11) +- [x] T011 [US1] Add console commands `soil` and `rs485test` in + `firmware/main/diag_console.cpp` per contracts/interfaces.md console contract + (values or error code+name; raw probe + statistics dump), going through + `LockedSoilSensor`/client +- [x] T012 [VERIFY-MAIN] [US1] Host suite green (quickstart §1) — decode tests pass, + exit code 0 +- [x] T013 [VERIFY-MAIN] [US1] rev1 target builds green (quickstart §2, rev1 half) + +**Checkpoint**: MVP flashable — HIL steps 1–3 executable by Paul at CP3. + +## Phase 4: User Story 2 — Faults yield invalid data, never wrong data (P2) + +**Goal**: timeout/validation/exception paths flag invalid + log, one attempt only, +auto-recovery on next read. + +**Independent test**: host tests for all fault paths green; HIL steps 4–5. + +- [x] T014 [P] [US2] Extend `firmware/test_apps/host/main/test_soil_sensor.cpp` with + fault-path tests: timeout → read() false + timeout error + getters not + presented as fresh; out-of-range moisture/temp/pH → validation error 5; + Modbus exception → distinct 100+n (or documented generic) code; exactly ONE + client call per read() on failure (no retry — assert via mock call recording); + recovery: failing mock then working mock → next read() true; statistics + increment correctness; isAvailable() performs a real 1-register bus read + (assert via mock call recording — FR-011, analyze C1); setTimeout() reaches + the client (mock passthrough assert — FR-006, analyze C2) +- [x] T015 [US2] Implement fault handling in + `firmware/components/sensors/src/ModbusSoilSensor.cpp`: range validation per + data-model table (fail read, error 5, EC/NPK unenforced per parity), validity + flag semantics, ESP_LOG error on every failed read (tag `soil_sensor`), + no-retry invariant, lazy availability recovery (no permanent-failure state) +- [x] T016 [US2] Verify/complete error discrimination in + `firmware/components/sensors/src/EspModbusClient.cpp`: timeout vs invalid- + response vs exception mapping (R6); if 2.1.2 hides the slave exception code, + map to the single documented exception code and note the parity divergence in + a code comment + PR notes +- [x] T017 [VERIFY-MAIN] [US2] Host suite green including all fault-path tests + +**Checkpoint**: fail-safe contract (FR-005/FR-006/FR-010) host-proven. + +## Phase 5: User Story 3 — One driver, two RS485 generations (P3) + +**Goal**: rev2 build without DE pin, echo handled, RX pull-up active; rev1 +unaffected. + +**Independent test**: both targets build (quickstart §2); rev2 binary has no DE +reference; pull-up call present. + +- [x] T018 [US3] Add FW-2 RX pull-up in + `firmware/components/sensors/src/EspModbusClient.cpp`: after pin setup, + `gpio_pullup_en(BOARD_PIN_RS485_RX)` unconditionally with comment referencing + FW-2/THVD1426 SHDN̅–SENS_PWR_EN coupling (R4) +- [x] T019 [US3] Document FW-4 echo strategy in + `firmware/components/sensors/src/EspModbusClient.cpp` (comment at + uart_set_mode site): half-duplex RX gating suppresses THVD1426 echo on rev2, + T3.5 resync as fallback, PR-14 HIL verifies (R3); confirm no rev1-only + assumptions in the shared path +- [x] T020 [VERIFY-MAIN] [US3] Both board targets build green from clean config + (quickstart §2) and `strings`/grep of rev2 build objects show no + `BOARD_PIN_RS485_DE` usage compiled in (board.h sanity would have failed the + build — record the check output) + +**Checkpoint**: FR-007/008/009/015 satisfied at build+code level; electrical proof +deferred to PR-14 per spec assumption. + +## Phase 6: User Story 4 — Testable without hardware + calibration (P4) + +**Goal**: complete host-test surface incl. calibration (CP1 answer A) and the mock +soil sensor PR-11 will consume; CI runs it all. + +**Independent test**: full host suite green in CI on linux target. + +- [x] T021 [P] [US4] Extend `firmware/test_apps/host/main/test_soil_sensor.cpp` with + calibration tests: factor = reference/raw from fresh read; factor applied to + subsequent reads (EC, pH, moisture); best-effort write to 0x0100/0x0101/0x0102 + fn 0x06 with legacy ×100 encoding (assert via mock write log); write failure + → calibrate returns success for local part + factor still applied (parity + non-fatal) +- [x] T022 [US4] Implement calibration in + `firmware/components/sensors/src/ModbusSoilSensor.cpp` + + `ModbusSoilSensor.h`: `calibrateMoisture/PH/EC` porting the exact legacy + formula from `src/sensors/ModbusSoilSensor.cpp:207-322` (READ-ONLY reference); + factors RAM-only (R8) +- [x] T023 [P] [US4] Create `MockSoilSensor` in + `firmware/components/sensors/include/sensors/testing/MockSoilSensor.h` + (settable values/validity/errors — the PR-11 consumer fixture) +- [x] T024 [US4] Add calibration console commands `soil_cal_moisture`, + `soil_cal_ph`, `soil_cal_ec` in `firmware/main/diag_console.cpp` per console + contract (factor + write-result reporting) +- [x] T025 [VERIFY-MAIN] [US4] Full host suite green (all soil tests + existing 50 + tests, exit 0); confirm CI workflow needs no change beyond the new files + (esp-idf-ci-action already runs `test_apps/host` with `target: linux`) + +**Checkpoint**: all [CI] acceptance criteria met. + +## Phase 7: Polish & cross-cutting + +- [x] T026 [P] Write HIL checklist to + `specs/004-modbus-soil-sensor/checklists/hil.md` from quickstart §3 (steps, + expected outcomes, sign-off boxes; note rev2 items deferred to PR-14) for + Paul at Checkpoint 3 +- [x] T027 [P] Update `firmware/CLAUDE.md`: `sensors` component summary, esp-modbus + pin, console command additions, host-test file list +- [x] T028 Record parity divergences (if any hit during implementation: exception- + code granularity R6, setTimeout runtime support R5, RTS-timing fallback R2) + in code comments + `specs/004-modbus-soil-sensor/plan.md` Risks section + updates +- [x] T029 [VERIFY-MAIN] Final clean-checkout verification: rsync fresh copy, + both board builds + host suite from scratch (quickstart §1+§2), commit + `dependencies.lock` change deliberately (constitution III) + +## Dependencies & execution order + +- Phase 1 → Phase 2 → Phase 3 (US1) → Phase 4 (US2) → Phase 5 (US3) → Phase 6 + (US4) → Phase 7. Stories after US1 are logically independent but share + `ModbusSoilSensor.cpp`/`EspModbusClient.cpp`, so run sequentially (single + implementer at a time per file; [P] marks the safe parallel writes). +- T002 can run any time before T012. T005 blocks T006/T014/T021. T008 blocks + T010–T013. Console tasks (T011, T024) depend on wiring (T010). + +## Parallel opportunities + +- Phase 2: T003, T004, T005 (three different files). +- T006 (tests) alongside T007/T008 (different files). +- Phase 6: T021 ∥ T023; Phase 7: T026 ∥ T027. + +## Implementation strategy + +MVP = Phase 1–3 (US1): flashable rig build with `soil`/`rs485test` — already +HIL-demonstrable. Then US2 (safety contract), US3 (rev2 deltas), US4 +(calibration + full CI surface), polish. Implementer missions per phase (write-only, +one commit per phase minimum); orchestrator runs every [VERIFY-MAIN] task in the +main session between missions; fixer agent handles review findings at CP3.