docs(phase4d): S7 wide-types implementation plan + tasks.json
This commit is contained in:
Joseph Doherty
@@ -0,0 +1,171 @@
|
||||
# S7 Wide Types + Timer/Counter Implementation Plan
|
||||
|
||||
> **For Claude:** REQUIRED SUB-SKILL: Use superpowers-extended-cc:subagent-driven-development to implement this plan task-by-task.
|
||||
|
||||
**Goal:** Implement S7 read+write for Int64/UInt64/Float64(LReal)/String/DateTime and read for Timer/Counter, closing the two `stillpending.md` §2 S7 lines.
|
||||
|
||||
**Architecture:** Narrow types (Bool…Float32) keep the existing S7.Net string `ReadAsync`/`WriteAsync` path unchanged. Wide/structured/Timer/Counter route through the byte-buffer path (`ReadBytesAsync`/`WriteBytesAsync`, the 4c array mechanism) feeding pure `DecodeScalarBlock`/`EncodeScalarBlock` — `BinaryPrimitives` for 8-byte numerics, S7.Net's pure static `S7String`/`DateTime`/`Timer`/`Counter` `FromByteArray`/`ToByteArray` for the structured ones. Wide/structured types are byte-anchored (`DBB`/`MB`/`IB`/`QB` start byte; width from `DataType`). Init guards preserve fail-fast; the lossy `Int64/UInt64 → Int32` node mapping is corrected.
|
||||
|
||||
**Tech Stack:** C#/.NET 10, S7netplus 0.20.0, xUnit + Shouldly. All edits in `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/` (+ `.Contracts` if needed), `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/`, `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli/`, `docs/`.
|
||||
|
||||
**Branch:** `feat/stillpending-phase-4d-s7-wide-types` off master `12e114b3`. Design: `docs/plans/2026-06-17-stillpending-phase-4d-s7-wide-types-design.md` (`eb7f3c49`).
|
||||
|
||||
**Dependency graph:** `T1 → T2 → {T3, T4, T5} → T6 → T7 → T8`. T3/T4/T5 all build on T2's dispatch seam and share `S7Driver.cs`, so they serialize (NOT parallelizable).
|
||||
|
||||
**Hard rules:** stage by explicit path, never `git add .`; never stage `sql_login.txt` / `src/Server/.../pki/` / `pending.md` / `current.md` / `docker-dev/docker-compose.yml` / `stillpending.md`; never echo/commit secrets; no force-push; no `--no-verify`; NO EF migration; NO Commons wire/proto change; NO bUnit; `dangerouslyDisableSandbox` for all build/test/rig commands. When building concurrently with sibling agents, build ONLY the S7 driver/test project, and retry on `git index.lock`.
|
||||
|
||||
---
|
||||
|
||||
### Task 1: Init guards + MapDataType fix
|
||||
|
||||
**Classification:** standard
|
||||
**Estimated implement time:** ~4 min
|
||||
**Parallelizable with:** none
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs` (`UnimplementedDataTypes:350`, `RejectUnsupportedTagAddresses:306`, `RejectUnsupportedTagDataTypes:329`, `MapDataType:814`)
|
||||
- Test: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7TypeMappingTests.cs`, `S7DriverScaffoldTests.cs` (init-guard tests)
|
||||
|
||||
**What changes:**
|
||||
1. `UnimplementedDataTypes` → empty set `new()` (all five now implemented). Keep the field + `RejectUnsupportedTagDataTypes` as the seam, but it now rejects nothing from that set.
|
||||
2. `RejectUnsupportedTagAddresses` → remove the Timer/Counter rejection. Replace with a new combined init guard `RejectUnsupportedWideTypeConfigs` (or extend the existing method) that rejects, with clear remediation messages:
|
||||
- (a) `t.ArrayCount is >= 1` AND `DataType ∈ {Int64,UInt64,Float64,String,DateTime}` → "wide-type arrays are not yet supported".
|
||||
- (b) `DataType ∈ {Int64,UInt64,Float64,String,DateTime}` AND the parsed address `Size != S7Size.Byte` (and area is DB/M/I/Q) → "wide/structured S7 types must be byte-addressed (DBB/MB/IB/QB)".
|
||||
- (c) `Area == Timer` AND `DataType != Float64`, or `Area == Counter` AND `DataType != Int32` → "Timer tags must be DataType=Float64; Counter tags must be DataType=Int32".
|
||||
3. `MapDataType`: `S7DataType.Int64 => DriverDataType.Int64`, `S7DataType.UInt64 => DriverDataType.UInt64` (split off the lossy `Int32` line). Update the `Driver.S7-002`/`:822` comment.
|
||||
|
||||
**Steps (TDD):**
|
||||
1. Write failing tests in `S7TypeMappingTests.cs`: `MapDataType(Int64) == DriverDataType.Int64`, `MapDataType(UInt64) == DriverDataType.UInt64`. (Note `MapDataType` is `private static` — if no existing test reaches it, mirror however `S7TypeMappingTests` already exercises mapping, e.g. via `DiscoverAsync` + a capturing `IAddressSpaceBuilder`; otherwise make it `internal static` + `InternalsVisibleTo` if that pattern already exists in the project.)
|
||||
2. Write failing init-guard tests (construct an `S7Driver` with a config carrying each bad combo; assert `InitializeAsync`/the guard throws `NotSupportedException`/`FormatException` with the expected message fragment). Mirror how `S7DriverScaffoldTests` already drives init. A Float64 scalar at `DB1.DBB0` must NOT throw (positive case).
|
||||
3. Run: `dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests --filter "FullyQualifiedName~TypeMapping|FullyQualifiedName~Guard" -v minimal` (sandbox disabled). Expect FAIL.
|
||||
4. Implement the four changes above.
|
||||
5. Run the same filter. Expect PASS. Then run the full S7.Tests project to confirm no regression.
|
||||
6. Commit: `git add src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7TypeMappingTests.cs tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7DriverScaffoldTests.cs` then `git commit -m "feat(s7): unblock wide-type/Timer/Counter init guards + fix Int64/UInt64 node mapping"`.
|
||||
|
||||
---
|
||||
|
||||
### Task 2: 8-byte numerics (Int64/UInt64/LReal) scalar read+write
|
||||
|
||||
**Classification:** high-risk
|
||||
**Estimated implement time:** ~5 min
|
||||
**Parallelizable with:** none (introduces the dispatch seam T3/T4/T5 build on)
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs` (`ReadOneAsync:433`, `WriteOneAsync:722`, add `ReadScalarBlockAsync`/`WriteScalarBlockAsync`/`DecodeScalarBlock`/`EncodeScalarBlock`/`ScalarByteWidth`; reuse `ToS7NetArea:508`)
|
||||
- Test: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7DriverReadWriteTests.cs` (or a new `S7ScalarBlockTests.cs` mirroring `S7ArrayReadTests.cs`)
|
||||
|
||||
**What changes:**
|
||||
1. Add a `private static bool IsBufferType(S7TagDefinition tag, S7ParsedAddress addr)` → true for `DataType ∈ {Int64,UInt64,Float64,String,DateTime}` OR `addr.Area ∈ {Timer,Counter}`.
|
||||
2. In `ReadOneAsync`, after the array branch: `if (IsBufferType(tag, addr)) return await ReadScalarBlockAsync(plc, tag, addr, ct);` — else the existing `plc.ReadAsync` + `ReinterpretRawValue` path (unchanged).
|
||||
3. In `WriteOneAsync`: `if (IsBufferType(tag, addr)) { await WriteScalarBlockAsync(plc, tag, addr, value, ct); return; }` — else the existing `BoxValueForWrite` + `plc.WriteAsync` path. (Timer/Counter `IsBufferType` is true but writes land in T5 as `BadNotWritable`; here, for T2, only numeric write is wired — Timer/Counter/String/DateTime encode cases throw `NotSupportedException` until their task, matching the existing deferred-throw idiom.)
|
||||
4. `ReadScalarBlockAsync(plc, tag, addr, ct)`: `width = ScalarByteWidth(tag, addr)`; `block = await plc.ReadBytesAsync(ToS7NetArea(addr.Area), addr.DbNumber, addr.ByteOffset, width, ct)`; `return DecodeScalarBlock(tag, addr, block)`.
|
||||
5. `WriteScalarBlockAsync(...)`: `var bytes = EncodeScalarBlock(tag, value); await plc.WriteBytesAsync(ToS7NetArea(addr.Area), addr.DbNumber, addr.ByteOffset, bytes, ct)`.
|
||||
6. `ScalarByteWidth(tag, addr)`: Int64/UInt64/Float64 → 8; String → `tag.StringLength + 2`; DateTime → 8; Timer/Counter → 2. (Full table; T3/T4/T5 rely on it.)
|
||||
7. `DecodeScalarBlock` / `EncodeScalarBlock`: **pure**, `internal static`, switch on `(tag.DataType, addr.Area)`. T2 implements the Int64/UInt64/Float64 cases via `BinaryPrimitives.ReadInt64BigEndian`/`ReadUInt64BigEndian`/`ReadDoubleBigEndian` (decode) and `WriteInt64BigEndian`/`WriteUInt64BigEndian`/`WriteDoubleBigEndian` into a `new byte[8]` (encode, via `Convert.ToInt64`/`ToUInt64`/`ToDouble`). String/DateTime/Timer/Counter cases throw `NotSupportedException("… lands in a follow-up")` for now (filled by T3/T4/T5).
|
||||
|
||||
**Steps (TDD):**
|
||||
1. Write failing pure-function tests (mirror `S7ArrayReadTests.cs`): `DecodeScalarBlock` for Int64/UInt64/LReal over known big-endian 8-byte blocks; `EncodeScalarBlock` round-trip (decode∘encode identity) for each; assert big-endian order explicitly.
|
||||
2. Run filter `~ScalarBlock`. Expect FAIL (methods don't exist).
|
||||
3. Implement steps 1–7 above.
|
||||
4. Run filter → PASS. Run full S7.Tests → green (narrow read/write path unchanged).
|
||||
5. Commit the modified `S7Driver.cs` + the test file.
|
||||
|
||||
---
|
||||
|
||||
### Task 3: String read+write
|
||||
|
||||
**Classification:** standard
|
||||
**Estimated implement time:** ~4 min
|
||||
**Parallelizable with:** none (after T2)
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs` (`DecodeScalarBlock`/`EncodeScalarBlock` String cases)
|
||||
- Test: the T2 scalar-block test file
|
||||
|
||||
**What changes:** Fill the `(String, _)` cases. Decode: `S7.Net.Types.S7String.FromByteArray(block)` → `string`. Encode: `S7.Net.Types.S7String.ToByteArray(Convert.ToString(value) ?? "", tag.StringLength)` (reserved length = `StringLength`; confirm the exact 0.20.0 signature against the DLL — `S7String.ToByteArray(string value, int reservedLength)`). Width already handled by `ScalarByteWidth` (`StringLength + 2`).
|
||||
|
||||
**Steps (TDD):** failing test — build a known S7 STRING block `[maxLen][curLen][chars…]` and assert `DecodeScalarBlock` returns the string; assert `EncodeScalarBlock` produces a block whose header + chars match, and decode∘encode identity. Run filter FAIL → implement → PASS → full S7.Tests green → commit.
|
||||
|
||||
---
|
||||
|
||||
### Task 4: DateTime read+write (DATE_AND_TIME / DT)
|
||||
|
||||
**Classification:** standard
|
||||
**Estimated implement time:** ~4 min
|
||||
**Parallelizable with:** none (after T2)
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs` (`DecodeScalarBlock`/`EncodeScalarBlock` DateTime cases)
|
||||
- Test: the T2 scalar-block test file
|
||||
|
||||
**What changes:** Fill the `(DateTime, _)` cases. Decode: `S7.Net.Types.DateTime.FromByteArray(block)` → `System.DateTime`. Encode: `S7.Net.Types.DateTime.ToByteArray(Convert.ToDateTime(value))` (confirm 0.20.0 signature; the helper validates the 1990–2089 DT range — surface its exception as the existing catch maps it). Width = 8 (DT).
|
||||
|
||||
**Steps (TDD):** failing test — known 8-byte BCD DT block (e.g. `S7.Net.Types.DateTime.ToByteArray(new DateTime(2026,6,17,12,34,56))` as the fixture) → assert `DecodeScalarBlock` returns the same `DateTime`; decode∘encode identity. Run FAIL → implement → PASS → full S7.Tests green → commit.
|
||||
|
||||
---
|
||||
|
||||
### Task 5: Timer/Counter read
|
||||
|
||||
**Classification:** standard
|
||||
**Estimated implement time:** ~4 min
|
||||
**Parallelizable with:** none (after T2)
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs` (`DecodeScalarBlock` Timer/Counter cases; `EncodeScalarBlock` Timer/Counter → throw; `ToS7NetArea` already maps DB/M/I/Q — add Timer/Counter → `S7NetDataType.Timer`/`Counter`)
|
||||
- Test: the T2 scalar-block test file
|
||||
|
||||
**What changes:**
|
||||
1. `ToS7NetArea`: add `S7Area.Timer => S7NetDataType.Timer`, `S7Area.Counter => S7NetDataType.Counter` (it currently throws for them; now reachable for the buffer path).
|
||||
2. `DecodeScalarBlock` `(_, S7Area.Timer)` → `(double)S7.Net.Types.Timer.FromByteArray(block)` (seconds); `(_, S7Area.Counter)` → `(int)S7.Net.Types.Counter.FromByteArray(block)` (count). Confirm 0.20.0 return types against the DLL.
|
||||
3. `EncodeScalarBlock` Timer/Counter → `throw new NotSupportedException("S7 Timer/Counter writes are read-only this phase")` (T2 already routes Timer/Counter writes here; combined with the existing `WriteAsync` `NotSupportedException → BadNotSupported` mapping, and tags being marked read-only is preferable — set `Writable:false` is operator-config, so the throw is the backstop).
|
||||
4. `ReadBytesAsync` for Timer/Counter addresses by (Timer/Counter area, db=0, number, count). Confirm S7.Net's Timer/Counter byte semantics (each is a 2-byte word; `count` is the number of timers/counters, not bytes — verify against `ReadBytesAsync` vs the typed `Read(VarType.Timer)` overload; if `ReadBytesAsync` is byte-count-based for Timer/Counter, pass 2; if it's element-count-based, pass 1 — the implementer confirms and documents which).
|
||||
|
||||
**Steps (TDD):** failing test — feed a known 2-byte timer block (BCD S5TIME) → assert decoded seconds; known counter block → assert decoded count. Run FAIL → implement → PASS → full S7.Tests green → commit.
|
||||
|
||||
---
|
||||
|
||||
### Task 6: CLI help + docs + plan-record §2 clear
|
||||
|
||||
**Classification:** small
|
||||
**Estimated implement time:** ~3 min
|
||||
**Parallelizable with:** none (after T5)
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli/Commands/ReadCommand.cs:24-29`, `WriteCommand.cs`, `SubscribeCommand.cs` (drop the "Int64, UInt64, Float64, String, DateTime are not yet implemented … BadNotSupported" notes; keep the type list)
|
||||
- Modify: `docs/v2/driver-specs.md` §5 (byte-anchored wide-type addressing, supported-type table, Timer/Counter read, named deferrals), `docs/drivers/S7.md`
|
||||
- Test: `tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli.Tests/` only if a test pins the removed help text
|
||||
|
||||
**Steps:** grep the three CLI command files for the removed phrasing; update help strings + docs. Do NOT stage `stillpending.md` (record the §2 closure in the plan/`.tasks.json`). Build the CLI project + run `S7.Cli.Tests`. Commit by explicit path.
|
||||
|
||||
---
|
||||
|
||||
### Task 7: Full build + test + final integration review
|
||||
|
||||
**Classification:** standard
|
||||
**Estimated implement time:** ~5 min (mostly build/test wall-time)
|
||||
**Parallelizable with:** none
|
||||
|
||||
**Steps:**
|
||||
1. `dotnet build src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/...csproj` + the CLI csproj (sandbox disabled). Expect clean.
|
||||
2. `dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests` + `tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli.Tests`. Expect all green.
|
||||
3. Dispatch a **final integration reviewer** over `git diff 12e114b3..HEAD` for: codec-dispatch correctness (narrow path truly unchanged), big-endian byte order, byte-width table consistency, init-guard completeness (no wide type can leak to a `BadNotSupported`-on-read), the read-only Timer/Counter contract, and the 4c array path untouched. Apply fixes via fresh implementers.
|
||||
4. No commit unless fixes land.
|
||||
|
||||
---
|
||||
|
||||
### Task 8: Live `/run` acceptance + finish branch
|
||||
|
||||
**Classification:** standard
|
||||
**Estimated implement time:** operator/agent-driven
|
||||
**Parallelizable with:** none
|
||||
|
||||
**Steps:**
|
||||
1. Bring the S7 sim up if needed (`lmxopcua-fix up s7 s7_1500` — but note the rig's S7 sim is `10.100.0.35:1102`; the LOCAL docker-dev central server is what materializes nodes). Author an S7 equipment tag (LReal at `DB?.DBB?`, or Int64) on the local rig via AdminUI, deploy current configuration, and read it over the wire with Client.CLI. Best-effort: if the sim lacks populated 8-byte DBs, record a unit-proven gate + note (as 4c did for non-Modbus drivers).
|
||||
2. Use superpowers-extended-cc:finishing-a-development-branch → verify tests → merge to master (ff) + push. Delete the merged branch. Update `.tasks.json` (all completed + reviewFollowUps) and the backlog memory; commit + push the bookkeeping.
|
||||
|
||||
---
|
||||
|
||||
## Review follow-ups (record in `.tasks.json`, do not silently drop)
|
||||
|
||||
- Wide-type **arrays**, `S7WString`, `DTL` (`DateTimeLong`), and **Timer/Counter writes** are named deferrals.
|
||||
- Live `/run` for the non-headline types may be unit-proven only if the S7 sim lacks populated DBs.
|
||||
@@ -0,0 +1,21 @@
|
||||
{
|
||||
"planPath": "docs/plans/2026-06-17-stillpending-phase-4d-s7-wide-types.md",
|
||||
"designPath": "docs/plans/2026-06-17-stillpending-phase-4d-s7-wide-types-design.md",
|
||||
"designCommit": "eb7f3c49",
|
||||
"baseMaster": "12e114b3",
|
||||
"branch": "feat/stillpending-phase-4d-s7-wide-types",
|
||||
"scope": "S7 read+write for Int64/UInt64/Float64(LReal)/String/DateTime + read for Timer/Counter (AskUserQuestion: all five + Timer/Counter). Byte-buffer codec path + pure S7.Net.Types decoders; byte-anchored wide-type addressing; init-guard fail-fast; Int64/UInt64 node-mapping fix. OUT: wide-type arrays, S7WString, DTL, Timer/Counter writes.",
|
||||
"dependencyGraph": "T1 -> T2 -> {T3, T4, T5} -> T6 -> T7 -> T8 (T3/T4/T5 serialize: all build on T2's dispatch seam + share S7Driver.cs)",
|
||||
"tasks": [
|
||||
{"id": 1, "subject": "Init guards (empty UnimplementedDataTypes, drop Timer/Counter reject, add wide-array/non-Byte-address/Timer-Counter-DataType guards) + MapDataType Int64/UInt64 fix", "classification": "standard", "status": "pending"},
|
||||
{"id": 2, "subject": "8-byte numerics (Int64/UInt64/LReal) scalar read+write: codec dispatch + ReadScalarBlockAsync/WriteScalarBlockAsync + pure DecodeScalarBlock/EncodeScalarBlock", "classification": "high-risk", "status": "pending", "blockedBy": [1]},
|
||||
{"id": 3, "subject": "String read+write (S7String + StringLength)", "classification": "standard", "status": "pending", "blockedBy": [2]},
|
||||
{"id": 4, "subject": "DateTime read+write (DATE_AND_TIME / DT, 8-byte BCD)", "classification": "standard", "status": "pending", "blockedBy": [2]},
|
||||
{"id": 5, "subject": "Timer/Counter read (decode by area, read-only)", "classification": "standard", "status": "pending", "blockedBy": [2]},
|
||||
{"id": 6, "subject": "CLI help + docs (driver-specs §5, drivers/S7.md) + plan-record §2 clear", "classification": "small", "status": "pending", "blockedBy": [5]},
|
||||
{"id": 7, "subject": "Full build + S7 + S7.Cli test + final integration review", "classification": "standard", "status": "pending", "blockedBy": [6]},
|
||||
{"id": 8, "subject": "Live /run acceptance (S7 sim best-effort) + finish branch (merge to master + push)", "classification": "standard", "status": "pending", "blockedBy": [7]}
|
||||
],
|
||||
"reviewFollowUps": [],
|
||||
"lastUpdated": "2026-06-17"
|
||||
}
|
||||
Reference in New Issue
Block a user