From fe981b0b7fce69c2f982ff7b88acf53c01b9158e Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 21 Apr 2026 10:31:55 -0400 Subject: [PATCH] =?UTF-8?q?Task=20#253=20follow-up=20=E2=80=94=20driver-si?= =?UTF-8?q?de=20e2e=20debug:=20port=20fixes=20+=20HR[200]=20scratch=20regi?= =?UTF-8?q?ster?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ran the driver CLIs against the live docker-compose fixtures to debug what actually works. Two real bugs surfaced: 1. `e2e-config.sample.json` pointed at the wrong simulator ports: - Modbus: 5502 → **5020** (pymodbus compose binding) - S7: 102 → **1102** (python-snap7, non-priv port) - AbCip: no port → now explicit **:44818** `test-modbus.ps1` default `-ModbusHost` also shipped with 5502; fixed to 5020. 2. Modbus loopback was off-by-2 because pymodbus `standard.json` makes HR[100] an auto-increment register (value ticks on every poll). Switched `test-modbus.ps1` to **HR[200]** (scratch range in the profile) + updated sample config `bridgeNodeId` to match. Also fixed the AbCip probe: `-t @raw_cpu_type` was rejected by the driver's TagPath parser ("malformed TagPath"). Probe now uses the user-supplied `-TagPath` for every family. Works against both ab_server and real controllers. Verified driver-side against live containers: - Modbus 5020: probe ✓, HR[200] write+read round-trip ✓ - AB CIP 44818: probe ✓, TestDINT write+read round-trip ✓ - S7 1102: probe ✓, DB1.DBW0 write+read round-trip ✓ ## Known blocker (stages 3-5) README now flags — and the 4 child issues under umbrella #209 track — that `src/ZB.MOM.WW.OtOpcUa.Server/Program.cs:98-104` only registers Galaxy + FOCAS driver factories. `DriverInstanceBootstrapper` silently skips any `DriverType` without a registered factory, so stages 3-5 (anything crossing the OPC UA server) can't work today even with a perfect Config DB seed. Issues #210-213 scope the factory + seed SQL work per driver. Co-Authored-By: Claude Opus 4.7 (1M context) --- scripts/e2e/README.md | 24 +++++++++++++++++++++++ scripts/e2e/e2e-config.sample.json | 11 +++++++---- scripts/e2e/test-abcip.ps1 | 14 ++++---------- scripts/e2e/test-modbus.ps1 | 31 +++++++++++++++++------------- 4 files changed, 53 insertions(+), 27 deletions(-) diff --git a/scripts/e2e/README.md b/scripts/e2e/README.md index aaf9e01..dde4e69 100644 --- a/scripts/e2e/README.md +++ b/scripts/e2e/README.md @@ -51,6 +51,30 @@ knows. Those NodeIds live in `e2e-config.json` (see below). The published tag must be **writable** — stages 4 + 5 will fail against a read-only tag. +## Status + +Stages 1 + 2 (driver-side probe + loopback) are verified end-to-end +against the pymodbus / ab_server / python-snap7 fixtures. Stages 3-5 +(anything crossing the OtOpcUa server) are **blocked** on server-side +driver factory wiring: + +- `src/ZB.MOM.WW.OtOpcUa.Server/Program.cs` only registers Galaxy + + FOCAS factories. `DriverInstanceBootstrapper` skips any `DriverType` + without a registered factory — so Modbus / AB CIP / AB Legacy / S7 / + TwinCAT rows in the Config DB are silently no-op'd even when the seed + is perfect. +- No Config DB seed script exists for non-Galaxy drivers; Admin UI is + currently the only path to author one. + +Tracking: **#209** (umbrella) → #210 (Modbus), #211 (AB CIP), #212 (S7), +#213 (AB Legacy, also hardware-gated — #222). Each child issue lists +the factory class to write + the seed SQL shape + the verification +command. + +Until those ship, stages 3-5 will fail with "read failed" (nothing +published at that NodeId) and `[FAIL]` the suite even on a running +server. + ## Prereqs 1. **OtOpcUa server** running on `opc.tcp://localhost:4840` (or pass diff --git a/scripts/e2e/e2e-config.sample.json b/scripts/e2e/e2e-config.sample.json index e7f5ae1..55ce8b2 100644 --- a/scripts/e2e/e2e-config.sample.json +++ b/scripts/e2e/e2e-config.sample.json @@ -2,13 +2,15 @@ "$comment": "Copy this file to e2e-config.json and replace the NodeIds with the ones your Config DB publishes. Fields named `opcUaUrl` override the -OpcUaUrl parameter on test-all.ps1 per-driver. Omit a top-level key to skip that driver.", "modbus": { - "endpoint": "127.0.0.1:5502", - "bridgeNodeId": "ns=2;s=Modbus/HR100", + "$comment": "Port 5020 matches tests/.../Modbus.IntegrationTests/Docker/docker-compose.yml — `docker compose --profile standard up -d`.", + "endpoint": "127.0.0.1:5020", + "bridgeNodeId": "ns=2;s=Modbus/HR200", "opcUaUrl": "opc.tcp://localhost:4840" }, "abcip": { - "gateway": "ab://127.0.0.1/1,0", + "$comment": "ab_server listens on port 44818 (default CIP/EIP). `docker compose --profile controllogix up -d`.", + "gateway": "ab://127.0.0.1:44818/1,0", "family": "ControlLogix", "tagPath": "TestDINT", "bridgeNodeId": "ns=2;s=AbCip/TestDINT" @@ -23,7 +25,8 @@ }, "s7": { - "endpoint": "127.0.0.1:102", + "$comment": "Port 1102 matches tests/.../S7.IntegrationTests/Docker/docker-compose.yml (python-snap7 needs non-priv port). `docker compose --profile s7_1500 up -d`. Real S7 PLCs listen on 102.", + "endpoint": "127.0.0.1:1102", "cpu": "S71500", "slot": 0, "address": "DB1.DBW0", diff --git a/scripts/e2e/test-abcip.ps1 b/scripts/e2e/test-abcip.ps1 index 7d33b45..c7fcad3 100644 --- a/scripts/e2e/test-abcip.ps1 +++ b/scripts/e2e/test-abcip.ps1 @@ -53,18 +53,12 @@ $opcUaCli = Get-CliInvocation ` $commonAbCip = @("-g", $Gateway, "-f", $Family) $results = @() -# Probe via @raw_cpu_type for ControlLogix; against ab_server this surfaces the -# string 'Controllogix'. Other families use the main TestDINT. -$probeTag = if ($Family -eq "ControlLogix" -or $Family -eq "CompactLogix" -or $Family -eq "GuardLogix") { - "@raw_cpu_type" -} else { - $TagPath -} -$probeType = if ($probeTag -eq "@raw_cpu_type") { "String" } else { "DInt" } - +# The AbCip driver's TagPath parser rejects CIP attribute syntax like +# `@raw_cpu_type` ("malformed TagPath"), so probe uses the real TagPath for +# every family. Works against ab_server + real controllers alike. $results += Test-Probe ` -Cli $abcipCli ` - -ProbeArgs (@("probe") + $commonAbCip + @("-t", $probeTag, "--type", $probeType)) + -ProbeArgs (@("probe") + $commonAbCip + @("-t", $TagPath, "--type", "DInt")) $writeValue = Get-Random -Minimum 1 -Maximum 9999 $results += Test-DriverLoopback ` diff --git a/scripts/e2e/test-modbus.ps1 b/scripts/e2e/test-modbus.ps1 index 7e68b29..85a9774 100644 --- a/scripts/e2e/test-modbus.ps1 +++ b/scripts/e2e/test-modbus.ps1 @@ -7,17 +7,22 @@ Five assertions: 1. `otopcua-modbus-cli probe` hits the simulator 2. Driver-loopback write + read-back via modbus-cli - 3. Forward bridge: modbus-cli writes HR[100], OPC UA client reads the bridged NodeId - 4. Reverse bridge: OPC UA client writes the NodeId, modbus-cli reads HR[100] + 3. Forward bridge: modbus-cli writes HR[200], OPC UA client reads the bridged NodeId + 4. Reverse bridge: OPC UA client writes the NodeId, modbus-cli reads HR[200] 5. Subscribe-sees-change: OPC UA subscription observes a modbus-cli write - Requires a running Modbus simulator on localhost:5502 (the pymodbus fixture - default per docs/drivers/Modbus-Test-Fixture.md) and a running OtOpcUa server - whose config DB has a Modbus DriverInstance bound to that simulator + a Tag - at HR[100] Int16 published under the NodeId passed via -BridgeNodeId. + Requires a running Modbus simulator on localhost:5020 (the pymodbus fixture + default — see tests/.../Modbus.IntegrationTests/Docker/docker-compose.yml) + and a running OtOpcUa server whose config DB has a Modbus DriverInstance + bound to that simulator + a Tag at HR[200] UInt16 published under the + NodeId passed via -BridgeNodeId. + + NOTE: HR[200] (not HR[100]) — pymodbus standard.json makes HR[100] an + auto-incrementing register that mutates every poll, so loopback writes + can't be verified there. .PARAMETER ModbusHost - Host:port of the Modbus simulator. Default 127.0.0.1:5502. + Host:port of the Modbus simulator. Default 127.0.0.1:5020. .PARAMETER OpcUaUrl Endpoint URL of the OtOpcUa server. Default opc.tcp://localhost:4840. @@ -31,7 +36,7 @@ #> param( - [string]$ModbusHost = "127.0.0.1:5502", + [string]$ModbusHost = "127.0.0.1:5020", [string]$OpcUaUrl = "opc.tcp://localhost:4840", [Parameter(Mandatory)] [string]$BridgeNodeId ) @@ -59,14 +64,14 @@ $results += Test-Probe ` $writeValue = Get-Random -Minimum 1 -Maximum 9999 $results += Test-DriverLoopback ` -Cli $modbusCli ` - -WriteArgs (@("write") + $commonModbus + @("-r", "HoldingRegisters", "-a", "100", "-t", "UInt16", "-v", $writeValue)) ` - -ReadArgs (@("read") + $commonModbus + @("-r", "HoldingRegisters", "-a", "100", "-t", "UInt16")) ` + -WriteArgs (@("write") + $commonModbus + @("-r", "HoldingRegisters", "-a", "200", "-t", "UInt16", "-v", $writeValue)) ` + -ReadArgs (@("read") + $commonModbus + @("-r", "HoldingRegisters", "-a", "200", "-t", "UInt16")) ` -ExpectedValue "$writeValue" $bridgeValue = Get-Random -Minimum 10000 -Maximum 19999 $results += Test-ServerBridge ` -DriverCli $modbusCli ` - -DriverWriteArgs (@("write") + $commonModbus + @("-r", "HoldingRegisters", "-a", "100", "-t", "UInt16", "-v", $bridgeValue)) ` + -DriverWriteArgs (@("write") + $commonModbus + @("-r", "HoldingRegisters", "-a", "200", "-t", "UInt16", "-v", $bridgeValue)) ` -OpcUaCli $opcUaCli ` -OpcUaUrl $OpcUaUrl ` -OpcUaNodeId $BridgeNodeId ` @@ -78,7 +83,7 @@ $results += Test-OpcUaWriteBridge ` -OpcUaUrl $OpcUaUrl ` -OpcUaNodeId $BridgeNodeId ` -DriverCli $modbusCli ` - -DriverReadArgs (@("read") + $commonModbus + @("-r", "HoldingRegisters", "-a", "100", "-t", "UInt16")) ` + -DriverReadArgs (@("read") + $commonModbus + @("-r", "HoldingRegisters", "-a", "200", "-t", "UInt16")) ` -ExpectedValue "$reverseValue" $subValue = Get-Random -Minimum 30000 -Maximum 39999 @@ -87,7 +92,7 @@ $results += Test-SubscribeSeesChange ` -OpcUaUrl $OpcUaUrl ` -OpcUaNodeId $BridgeNodeId ` -DriverCli $modbusCli ` - -DriverWriteArgs (@("write") + $commonModbus + @("-r", "HoldingRegisters", "-a", "100", "-t", "UInt16", "-v", $subValue)) ` + -DriverWriteArgs (@("write") + $commonModbus + @("-r", "HoldingRegisters", "-a", "200", "-t", "UInt16", "-v", $subValue)) ` -ExpectedValue "$subValue" Write-Summary -Title "Modbus e2e" -Results $results