From 77d09bf64e217207429ed6b5e435be07e40f9739 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sat, 18 Apr 2026 12:45:21 -0400 Subject: [PATCH] =?UTF-8?q?Phase=203=20PR=2025=20=E2=80=94=20modbus-test-p?= =?UTF-8?q?lan.md:=20integration-test=20playbook=20with=20per-device=20qui?= =?UTF-8?q?rk=20catalog.=20ModbusPal=20is=20the=20chosen=20simulator;=20Au?= =?UTF-8?q?tomationDirect=20DL205=20is=20the=20first=20target=20device=20c?= =?UTF-8?q?lass=20with=206=20pending=20quirks=20to=20document=20and=20cove?= =?UTF-8?q?r=20with=20named=20tests=20(word=20order=20for=2032-bit=20value?= =?UTF-8?q?s,=20register-zero=20access=20policy,=20coil=20addressing=20bas?= =?UTF-8?q?e,=20maximum=20registers=20per=20FC03,=20response=20framing=20u?= =?UTF-8?q?nder=20sustained=20load,=20exception=20code=20on=20protected-bi?= =?UTF-8?q?t=20coil=20write).=20Each=20quirk=20placeholder=20has=20a=20pro?= =?UTF-8?q?posed=20test=20name=20so=20the=20user's=20validation=20work=20t?= =?UTF-8?q?ranslates=20directly=20into=20integration=20tests.=20Test=20con?= =?UTF-8?q?ventions=20section=20codifies=20the=20named-per-quirk=20pattern?= =?UTF-8?q?,=20skip-when-unreachable=20guard,=20real=20ModbusTcpTransport?= =?UTF-8?q?=20usage,=20and=20inter-test=20isolation.=20Sets=20up=20the=20h?= =?UTF-8?q?arness-and-catalog=20structure=20future=20device=20families=20(?= =?UTF-8?q?Allen-Bradley=20Micrologix,=20Siemens=20S7-1200=20Modbus=20gate?= =?UTF-8?q?way,=20Schneider=20M340,=20whatever=20the=20user=20hits)=20will?= =?UTF-8?q?=20slot=20into=20=E2=80=94=20same=20per-device=20catalog=20shap?= =?UTF-8?q?e,=20cross-device=20patterns=20section=20for=20recurring=20quir?= =?UTF-8?q?ks=20that=20can=20get=20promoted=20into=20driver=20defaults.=20?= =?UTF-8?q?Next=20concrete=20PRs=20proposed:=20PR=2026=20for=20the=20integ?= =?UTF-8?q?ration=20test=20project=20scaffold=20+=20DL205=20profile=20+=20?= =?UTF-8?q?fixture=20with=20skip-guard=20+=20one=20smoke=20test,=20PR=2027?= =?UTF-8?q?+=20for=20the=20individual=20confirmed=20quirks=20one-per-PR.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/v2/modbus-test-plan.md | 103 ++++++++++++++++++++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100644 docs/v2/modbus-test-plan.md diff --git a/docs/v2/modbus-test-plan.md b/docs/v2/modbus-test-plan.md new file mode 100644 index 0000000..3301ae0 --- /dev/null +++ b/docs/v2/modbus-test-plan.md @@ -0,0 +1,103 @@ +# Modbus driver — test plan + device-quirk catalog + +The Modbus TCP driver unit tests (PRs 21–24) cover the protocol surface against an +in-memory fake transport. They validate the codec, state machine, and function-code +routing against a textbook Modbus server. That's necessary but not sufficient: real PLC +populations disagree with the spec in small, device-specific ways, and a driver that +passes textbook tests can still misbehave against actual equipment. + +This doc is the harness-and-quirks playbook. It's what gets wired up in the +`tests/Driver.Modbus.IntegrationTests` project when we ship that (PR 26 candidate). + +## Harness + +**Chosen simulator: ModbusPal** (Java, scriptable). Rationale: +- Scriptable enough to mimic device-specific behaviors (non-standard register + layouts, custom exception codes, intentional response delays). +- Runs locally, no CI dependency. Tests skip when `localhost:502` (or the configured + simulator endpoint) isn't reachable. +- Free + long-maintained — physical PLC bench is unavailable in most dev + environments, and renting cloud PLCs isn't worth the per-test cost. + +**Setup pattern** (not yet codified in a script — will land alongside the integration +test project): +1. Install ModbusPal, load the per-device `.xmpp` profile from + `tests/Driver.Modbus.IntegrationTests/ModbusPal/` (TBD directory). +2. Start the simulator listening on `localhost:502` (or override via + `MODBUS_SIM_ENDPOINT` env var). +3. `dotnet test` the integration project — tests auto-skip when the endpoint is + unreachable, so forgetting to start the simulator doesn't wedge CI. + +## Per-device quirk catalog + +### AutomationDirect DL205 + +First known target device. Quirks to document and cover with named tests (to be +filled in when user validates each behavior in ModbusPal with a DL205 profile): + +- **Word order for 32-bit values**: _pending_ — confirm whether DL205 uses ABCD + (Modbus TCP standard) or CDAB (Siemens-style word-swap) for Int32/UInt32/Float32. + Test name: `DL205_Float32_word_order_is_CDAB` (or `ABCD`, whichever proves out). +- **Register-zero access**: _pending_ — some DL205 configurations reject FC03 at + register 0 with exception code 02 (illegal data address). If confirmed, the + integration test suite verifies `ModbusProbeOptions.ProbeAddress` default of 0 + triggers the rejection and operators must override; test name: + `DL205_FC03_at_register_0_returns_IllegalDataAddress`. +- **Coil addressing base**: _pending_ — DL205 documentation sometimes uses 1-based + coil addresses; verify the driver's zero-based addressing matches the physical + PLC without an off-by-one adjustment. +- **Maximum registers per FC03**: _pending_ — Modbus spec caps at 125; some DL205 + models enforce a lower limit (e.g., 64). Test name: + `DL205_FC03_beyond_max_registers_returns_IllegalDataValue`. +- **Response framing under sustained load**: _pending_ — the driver's + single-flight semaphore assumes the server pairs requests/responses by + transaction id; at least one DL205 firmware revision is reported to drop the + TxId under load. If reproduced in ModbusPal we add a retry + log-and-continue + path to `ModbusTcpTransport`. +- **Exception code on coil write to a protected bit**: _pending_ — some DL205 + setups protect internal coils; the driver should surface the PLC's exception + PDU as `BadNotWritable` rather than `BadInternalError`. + +_User action item_: as each quirk is validated in ModbusPal, replace the _pending_ +marker with the confirmed behavior and file a named test in the integration suite. + +### Future devices + +One section per device class, same shape as DL205. Quirks that apply across +multiple devices (e.g., "all AB PLCs use CDAB") can be noted in the cross-device +patterns section below once we have enough data points. + +## Cross-device patterns + +Once multiple device catalogs accumulate, quirks that recur across two or more +vendors get promoted into driver defaults or opt-in options: + +- _(empty — filled in as catalogs grow)_ + +## Test conventions + +- **One named test per quirk.** `DL205_word_order_is_CDAB_for_Float32` is easier to + diagnose on failure than a generic `Float32_roundtrip`. The `DL205_` prefix makes + filtering by device class trivial (`--filter "DisplayName~DL205"`). +- **Skip with a clear SkipReason.** Follow the pattern from + `GalaxyRepositoryLiveSmokeTests`: check reachability in the fixture, capture + a `SkipReason` string, and have each test call `Assert.Skip(SkipReason)` when + it's set. Don't throw — skipped tests read cleanly in CI logs. +- **Use the real `ModbusTcpTransport`.** Integration tests exercise the wire + protocol end-to-end. The in-memory `FakeTransport` from the unit test suite is + deliberately not used here — its value is speed + determinism, which doesn't + help reproduce device-specific issues. +- **Don't depend on ModbusPal state between tests.** Each test resets the + simulator's register bank or uses a unique address range. Avoid relying on + "previous test left value at register 10" setups that flake when tests run in + parallel or re-order. + +## Next concrete PRs + +- **PR 26 — Integration test project + DL205 profile scaffold**: creates + `tests/Driver.Modbus.IntegrationTests`, imports the ModbusPal profile (or + generates it from JSON), adds the fixture with skip-when-unreachable, plus + one smoke test that reads a register. No DL205-specific assertions yet — that + waits for the user to validate each quirk. +- **PR 27+**: one PR per confirmed DL205 quirk, landing the named test + any + driver-side adjustment (e.g., retry on dropped TxId) needed to pass it.