From ed0881d672b9f8ab1fa976582092b9caa70a9f35 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 7 Oct 2026 20:49:33 -0400 Subject: [PATCH] feat(ab): native managed transport is the default for AbCip and AbLegacy; libplctag remains as fallback Task 22 (spec section 8 P5), with the controller rulings that exclude PLC-5 and LogixPccc. - Transport is nullable on both option records; null (key absent) means "use the default" and is never serialized. Factory DTO parse: absent -> null; a bad value still fails via ParseEnum. - One resolver per driver (AbCipTransportResolver / AbLegacyTransportResolver), used by the driver constructor, the Test Connect probe and (through the driver) the CLI. AbCip: null -> Native. AbLegacy: null -> Native unless any device is Plc5 or LogixPccc, then the whole driver resolves to Libplctag. Explicit values always win; explicit Native with a Plc5/LogixPccc device logs one Warning at init. - Init log shows configured vs effective, e.g. transport=Native (default). - AdminUI forms: "Default (...)" / "Native (managed)" / "libplctag (fallback)". Default omits the key on save (reverses Task 19's explicit re-emit, which would have pinned every edited driver). - CLIs: --transport default|native|libplctag, default "default" via the resolver, so --plc-type Plc5 with no --transport runs libplctag. - Tests pinned to Libplctag where a default-path runtime was created: found by instrumenting the native factories with the current xUnit test name (one unit test, AbLegacyReinitConfigAdoption; plus the two fixture-gated AbCip probe E2E tests whose exact messages were proven on libplctag). TransportMatrix and the differential harness already pass explicit transports. - Switch/resolver/CLI/form tests are falsified by mutating the resolver (default -> Libplctag, exclusion removed, AbLegacy default -> Libplctag): each mutation turns them red. - Replaces the inline role-name literal in AbCipDriverFactoryExtensions' ParseEnum call with a named constant; RoleAuthorityGuardTests was already red on the base commit for that line. - Docs: AbCip.md / AbLegacy.md default + release note; CLAUDE.md AB section. Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- CLAUDE.md | 11 +- docs/drivers/AbCip.md | 46 +++++--- docs/drivers/AbLegacy.md | 45 ++++++-- .../AbCipCliTransport.cs | 32 ++++++ .../AbCipCommandBase.cs | 6 +- .../AbLegacyCliTransport.cs | 32 ++++++ .../AbLegacyCommandBase.cs | 8 +- .../AbCipDriverOptions.cs | 12 ++- .../AbCipDriver.cs | 19 ++-- .../AbCipDriverFactoryExtensions.cs | 11 +- .../AbCipDriverProbe.cs | 2 +- .../AbCipTransportResolver.cs | 36 +++++++ .../AbLegacyDriverOptions.cs | 13 ++- .../AbLegacyDriver.cs | 26 ++++- .../AbLegacyDriverFactoryExtensions.cs | 7 +- .../AbLegacyDriverProbe.cs | 2 +- .../AbLegacyTransportResolver.cs | 64 +++++++++++ .../Drivers/Forms/AbCipDriverForm.razor | 10 +- .../Drivers/Forms/AbLegacyDriverForm.razor | 10 +- .../AbCipTransportOptionTests.cs | 19 +++- .../BuildOptionsTests.cs | 63 ++++++++--- .../AbCipTransportInitLogTests.cs | 12 ++- .../AbCipTransportSwitchTests.cs | 47 +++++++- .../AbLegacyReinitConfigAdoptionTests.cs | 5 +- .../AbLegacyTransportInitLogTests.cs | 55 +++++++--- .../AbLegacyTransportSwitchTests.cs | 100 ++++++++++++++++-- .../AbCipDriverPageFormSerializationTests.cs | 42 +++++--- ...bLegacyDriverPageFormSerializationTests.cs | 42 +++++--- .../DriverProbeHandshakeE2eTests.cs | 7 +- 29 files changed, 653 insertions(+), 131 deletions(-) create mode 100644 src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/AbCipCliTransport.cs create mode 100644 src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli/AbLegacyCliTransport.cs create mode 100644 src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipTransportResolver.cs create mode 100644 src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyTransportResolver.cs diff --git a/CLAUDE.md b/CLAUDE.md index 74e3379b..6b595018 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -976,10 +976,13 @@ Dev/test LDAP is the **shared GLAuth** running on the Linux Docker host at `10.1 ## AB drivers: native transport (`Transport`) AbCip and AbLegacy have a second, managed wire transport (`Driver.Ab.Transport`) beside libplctag, chosen by -the driver-config key `transport` = `Libplctag` (default) | `Native` (string; takes effect when the driver is -recreated by a deploy; AdminUI "Transport" dropdown; CLIs `--transport`). **Opt-in:** the default stays -`Libplctag` until the real-press read-only gate (Task 21) runs and the default is flipped. Native writes are -verified against `ab_server` only. Client-visible: native reports `BadTypeMismatch` where libplctag returned +the driver-config key `transport` = `Native` | `Libplctag` (string; takes effect when the driver is +recreated by a deploy; AdminUI "Transport" dropdown; CLIs `--transport`). **Native is the default (Task 22):** +an ABSENT key resolves to `Native` via `AbCipTransportResolver` / `AbLegacyTransportResolver` (driver, probe and +CLI share them) — except an AbLegacy driver with any PLC-5 / `LogixPccc` device, which resolves to `Libplctag`. +"Default" is encoded by omitting the key; the AdminUI must never re-emit a concrete default. Evidence: AbCip +reads proven on a real CompactLogix (Z-35162, read-only); AbCip writes and all AbLegacy are `ab_server`-only. +Fall back with `"transport": "Libplctag"`. Client-visible: native reports `BadTypeMismatch` where libplctag returned Good garbage for a same-width mis-authored type (DINT authored, REAL returned). libplctag is confined to `Libplctag*.cs` adapters by `LibplctagConfinementGuardTests`; do not reference it elsewhere. See `docs/drivers/AbCip.md` §Transport, `docs/drivers/AbLegacy.md`, and diff --git a/docs/drivers/AbCip.md b/docs/drivers/AbCip.md index f273e9d5..7246db88 100644 --- a/docs/drivers/AbCip.md +++ b/docs/drivers/AbCip.md @@ -76,7 +76,7 @@ the driver's `DriverConfig` JSON. Key fields: - **`Devices`** — one `AbCipDeviceOptions` per PLC (`HostAddress`, `PlcFamily`, optional `DeviceName`, per-device `AllowPacking` / `ConnectionSize` overrides). - **`Tags`** — pre-declared `AbCipTagDefinition` list; `Members` for UDT fan-out, `SafetyTag` for GuardLogix safety-partition tags. - **`Probe`** — connectivity-probe `Enabled` / `Interval` / `Timeout` / `ProbeTagPath`. -- **`Transport`** — `Libplctag` (default) or `Native`; see [Transport](#transport-transport-option). +- **`Transport`** — absent (default ⇒ `Native`), `Native`, or `Libplctag` (fallback); see [Transport](#transport-transport-option). - **Discovery** — `EnableControllerBrowse` (`@tags` walk) and `EnableDeclarationOnlyUdtGrouping` (whole-UDT read fast path). - **Alarms** — `EnableAlarmProjection` + `AlarmPollInterval` for the ALMD projection. @@ -89,24 +89,46 @@ The driver has two interchangeable wire transports, selected by one `DriverConfi | Key | Values | Default | Serialization | |---|---|---|---| -| `transport` | `Libplctag` \| `Native` | `Libplctag` | string, case-insensitive on read, written as `"Libplctag"` / `"Native"` | +| `transport` | absent \| `Native` \| `Libplctag` | absent ⇒ `Native` | string, case-insensitive on read, written as `"Libplctag"` / `"Native"`; "use the default" is encoded by OMITTING the key | -- **`Libplctag`** (default) drives the libplctag native library, as before. -- **`Native`** is a managed in-process EtherNet/IP + CIP client (`Driver.Ab.Transport`). It is **opt-in**: - the default stays `Libplctag` until the real-press read-only gate has run and the default is flipped. +- **`Native`** (the default since Task 22) is a managed in-process EtherNet/IP + CIP client + (`Driver.Ab.Transport`). +- **`Libplctag`** (fallback) drives the libplctag native library, as before. +- An absent key means "use the default". It is resolved in one place, `AbCipTransportResolver`, which the + driver, the Test Connect probe and the CLI all go through. An explicit value always wins. - A bad value fails like a bad `PlcFamily`; the Test Connect probe reports "Config JSON is invalid". - A change takes effect when the driver instance is **recreated**. A deploy that changes `DriverConfig` does this (`DriverHostActor` respawns the child, #516). `ReinitializeAsync` alone keeps the transport it was constructed with. -- AdminUI: the **Transport** dropdown in the advanced section of the AbCip driver form ("libplctag - (default)" / "Native (managed)"). Saving another field preserves it. -- The driver logs `transport=` at Information on every init, including reconnect re-inits. AbCip +- AdminUI: the **Transport** dropdown in the advanced section of the AbCip driver form ("Default (Native)" / + "Native (managed)" / "libplctag (fallback)"). **Default omits the key on save**, so a defaulted driver stays + defaulted; saving another field preserves an explicit value and preserves absence. +- The driver logs `transport= (default|configured)` at Information on every init, e.g. + `transport=Native (default)`, including reconnect re-inits. AbCip drivers now receive the host logger (they previously ran on a null logger, so none of their warnings reached the node log); expect previously invisible warnings, e.g. a probe enabled with no probe tag path. -- CLI: `--transport native|libplctag` (default `libplctag`). +- CLI: `--transport default|native|libplctag` (default `default`, which goes through the same resolver). - libplctag is confined to `Libplctag*.cs` adapters, enforced by a source-scan guard (`LibplctagConfinementGuardTests`), so removing it later is mechanical. +### Release note — native is the default (Task 22) + +- **The native managed transport is now the default.** A driver config with no `transport` key runs + `Native`; libplctag remains available as the fallback. +- ⚠️ **Client-visible:** native reports **`BadTypeMismatch`** for a mis-authored same-width type (for example a + tag authored `DINT` where the controller holds a `REAL`), where libplctag returned Good garbage. Correct the + authored type; see [Native transport behaviour](#native-transport-behaviour). +- **To fall back**, set `"transport": "Libplctag"` in the driver config (or pick "libplctag (fallback)" in + the AdminUI dropdown). +- **When it takes effect:** a change of transport — including this default flip for an existing driver with + no key — takes effect when a deploy recreates the driver instance, not on `ReinitializeAsync`. +- **Evidence, and its limits:** AbCip **reads** (plus browse and templates) were proven on a production + CompactLogix 5069-L320ERS2 (**Z-35162**, read-only, 2026-10-08; see + [gate results](../plans/2026-10-07-native-ab-transport-gate-results.md) §Real press). AbCip **writes** are + simulator-proven only (`ab_server`), as is all of AbLegacy. +- AbLegacy has its own exclusion: a defaulted AbLegacy driver with any PLC-5 or `logixpccc` device stays on + libplctag (see [AbLegacy.md §Transport](AbLegacy.md#transport-transport-option)). + ## Native transport behaviour - **One CIP connection and one request scheduler per device** (`DeviceSessionPool`), shared by reads, @@ -137,15 +159,15 @@ The driver has two interchangeable wire transports, selected by one `DriverConfi ## Known gaps - ⚠️ **Native writes are verified against `ab_server` only (simulator-only), never a real controller.** The - real-press gate (Task 21) is **read-only** and **has not run yet**; this page makes no real-controller - claim for the native transport. Results land in + real-press gate (Task 21) was **read-only**: it proved native reads, browse and templates on Z-35162 + (2026-10-08) and nothing else. Results: [native-ab-transport-gate-results](../plans/2026-10-07-native-ab-transport-gate-results.md). - **libplctag cannot be deleted** until a bench controller allows write verification. - Wide-struct fragmented writes (8-byte alignment) are unit-tested only, never proven on a controller (#567). - Native browse: listing pages are unpaced, and template reads outside an enumeration open a connection per call (#572). - Micro800, routed and `logixpccc` paths are covered by `ab_server` only. The Micro800 profile passes - natively; the planned default flip excludes `logixpccc`. + natively. The AbLegacy default excludes `logixpccc` (and PLC-5). - Follow-up issues for this workstream: #564 to #577. - Fixed along the way (pre-existing on master): `ab://host:44818/` with an empty CIP path (Micro800, unrouted MicroLogix) failed on libplctag with `ErrorBadData` because the adapter sent `Path=""`; it now diff --git a/docs/drivers/AbLegacy.md b/docs/drivers/AbLegacy.md index b6afa87b..5479deb0 100644 --- a/docs/drivers/AbLegacy.md +++ b/docs/drivers/AbLegacy.md @@ -93,32 +93,57 @@ binds from the driver's `DriverConfig` JSON: - **`Devices`** — one `AbLegacyDeviceOptions` per PLC (`HostAddress`, `PlcFamily`, optional `DeviceName`). - **`Tags`** — pre-declared `AbLegacyTagDefinition` list (`Name`, `DeviceHostAddress`, `Address`, `DataType`, `Writable`, `WriteIdempotent`). - **`Probe`** — connectivity-probe `Enabled` / `Interval` / `Timeout` / `ProbeAddress`. -- **`Transport`** — `Libplctag` (default) or `Native`; see [Transport](#transport-transport-option). +- **`Transport`** — absent (default ⇒ `Native`, but `Libplctag` when any device is PLC-5 / `LogixPccc`), `Native`, or `Libplctag` (fallback); see [Transport](#transport-transport-option). Full per-field descriptions live in the contracts assembly. The JSON skeleton is reproduced in [docs/v2/driver-specs.md §4](../v2/driver-specs.md). ## Transport (`transport` option) -Same switch as AB CIP: `transport` = `Libplctag` (default) \| `Native`, a string (case-insensitive on read, -written as `"Libplctag"` / `"Native"`). `Native` is a managed PCCC-over-EtherNet/IP client -(`Driver.Ab.Transport`) and is **opt-in**; the default stays `Libplctag` pending the real-press gate and the -default flip. A change takes effect when the driver instance is recreated (a deploy that changes -`DriverConfig`); the AdminUI form has a **Transport** dropdown in the advanced section, and the driver logs -`transport=` on every init. CLI: `--transport native|libplctag`. See +Same switch as AB CIP: `transport` = absent \| `Native` \| `Libplctag`, a string (case-insensitive on read, +written as `"Libplctag"` / `"Native"`; "use the default" is encoded by omitting the key). `Native` is a managed +PCCC-over-EtherNet/IP client (`Driver.Ab.Transport`). + +**Default resolution** (one place, `AbLegacyTransportResolver`, used by the driver, the Test Connect probe and +the CLI): an absent key resolves to `Native`, **unless any configured device has `PlcFamily` `Plc5` or +`LogixPccc`** — then the whole driver resolves to `Libplctag`, because the transport is chosen once per driver, +not per device. An explicit value always wins: explicit `Native` with a PLC-5 / `LogixPccc` device is allowed +(an operator choice) and logs one Warning at init naming the limitation. + +A change takes effect when the driver instance is recreated (a deploy that changes `DriverConfig`). The +AdminUI form's **Transport** dropdown offers "Default (Native; libplctag for PLC-5 / LogixPccc)" (omits the +key on save), "Native (managed)" and "libplctag (fallback)". The driver logs +`transport= (default|configured)` on every init, e.g. `transport=Libplctag (default; PLC-5/LogixPccc +device present)`. CLI: `--transport default|native|libplctag` (default `default`, so `--plc-type Plc5` with no +`--transport` runs libplctag). See [AbCip.md §Transport](AbCip.md#transport-transport-option) for the shared behaviour (one pooled connection per device, Forward Open throttle, routing). Native routing: SLC500/MicroLogix/PLC-5 are direct; `logixpccc` uses UnconnectedSend and its SLC framing is **unverified**. +### Release note — native is the default (Task 22) + +- **The native managed transport is now the default** for SLC 500 and MicroLogix. A driver config with no + `transport` key runs `Native`; libplctag remains available as the fallback. +- **Exclusion:** a defaulted driver with **any PLC-5 or `LogixPccc` device stays on libplctag**. Native PLC-5 + support is N-file reads only, and `LogixPccc` framing is unverified. Explicit `Native` is still honoured + there, with a Warning at init. +- ⚠️ **Client-visible:** native reports **`BadTypeMismatch`** for a mis-authored same-width type, where + libplctag returned Good garbage (see [AbCip.md §Native transport behaviour](AbCip.md#native-transport-behaviour)). +- **To fall back**, set `"transport": "Libplctag"` in the driver config. +- **When it takes effect:** when a deploy recreates the driver instance (not on `ReinitializeAsync`). An existing + driver with no key moves to Native on its next recreate. +- **Evidence, and its limits:** all of AbLegacy is **simulator-proven only** (`ab_server`). No real SLC, + MicroLogix or PLC-5 has been exercised. (The real-press gate, Z-35162 2026-10-08, covered AbCip reads only.) + ### Native known gaps - ⚠️ **Simulator-only.** PCCC reads and writes are verified against `ab_server` only. No real SLC/MicroLogix/ - PLC-5 has been exercised, and the real-press gate (Task 21) is read-only and has not run yet. + PLC-5 has been exercised; the real-press gate (Task 21) was read-only and AbCip-only. - **PLC-5 native is N-file reads only.** Every other file type and all PLC-5 writes are refused, currently - surfacing as `BadCommunicationError` rather than `BadNotSupported` (#571). The planned default flip excludes - PLC-5 and `logixpccc`. + surfacing as `BadCommunicationError` rather than `BadNotSupported` (#571). The default excludes PLC-5 and + `logixpccc` (they resolve to libplctag unless `Native` is set explicitly). - Status-bit sub-element writes (e.g. `T4:0/DN`, `C5:0/DN`) are Unsupported natively (they would clobber the control word); nothing goes on the wire. Sub-element arrays are Unsupported natively. L-file bit indexes above 15 are allowed. diff --git a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/AbCipCliTransport.cs b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/AbCipCliTransport.cs new file mode 100644 index 00000000..3a66f770 --- /dev/null +++ b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/AbCipCliTransport.cs @@ -0,0 +1,32 @@ +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli; + +/// +/// The CLI's --transport choice. leaves +/// null so the driver resolves it through +/// exactly as an absent transport key in a deployed config does. +/// +public enum AbCipCliTransport +{ + /// Use the driver's default (resolved by ). + Default, + + /// The managed native transport. + Native, + + /// The libplctag fallback transport. + Libplctag, +} + +/// Maps a onto the driver option. +public static class AbCipCliTransportExtensions +{ + /// Returns the configured transport: null for . + /// The CLI choice. + /// The value for . + public static AbCipTransport? ToConfigured(this AbCipCliTransport choice) => choice switch + { + AbCipCliTransport.Native => AbCipTransport.Native, + AbCipCliTransport.Libplctag => AbCipTransport.Libplctag, + _ => null, + }; +} diff --git a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/AbCipCommandBase.cs b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/AbCipCommandBase.cs index dc4e6b45..e0f61ed0 100644 --- a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/AbCipCommandBase.cs +++ b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/AbCipCommandBase.cs @@ -28,8 +28,8 @@ public abstract class AbCipCommandBase : DriverCommandBase /// Gets the transport stack used to reach the PLC. [CommandOption("transport", Description = - "Transport stack: libplctag (default) or native (managed EtherNet/IP + CIP).")] - public AbCipTransport Transport { get; init; } = AbCipTransport.Libplctag; + "Transport stack: default (= native), native (managed EtherNet/IP + CIP) or libplctag (fallback).")] + public AbCipCliTransport Transport { get; init; } = AbCipCliTransport.Default; /// Gets the per-operation timeout in milliseconds. [CommandOption("timeout-ms", Description = "Per-operation timeout in ms (default 5000).")] @@ -73,7 +73,7 @@ public abstract class AbCipCommandBase : DriverCommandBase WriteIdempotent: t.WriteIdempotent, DeviceName: t.DeviceHostAddress))], Timeout = Timeout, - Transport = Transport, + Transport = Transport.ToConfigured(), Probe = new AbCipProbeOptions { Enabled = false }, EnableControllerBrowse = false, EnableAlarmProjection = false, diff --git a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli/AbLegacyCliTransport.cs b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli/AbLegacyCliTransport.cs new file mode 100644 index 00000000..43aec121 --- /dev/null +++ b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli/AbLegacyCliTransport.cs @@ -0,0 +1,32 @@ +namespace ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli; + +/// +/// The CLI's --transport choice. leaves +/// null so the driver resolves it through +/// exactly as an absent transport key in a deployed config does. +/// +public enum AbLegacyCliTransport +{ + /// Use the driver's default (resolved by ). + Default, + + /// The managed native transport. + Native, + + /// The libplctag fallback transport. + Libplctag, +} + +/// Maps a onto the driver option. +public static class AbLegacyCliTransportExtensions +{ + /// Returns the configured transport: null for . + /// The CLI choice. + /// The value for . + public static AbLegacyTransport? ToConfigured(this AbLegacyCliTransport choice) => choice switch + { + AbLegacyCliTransport.Native => AbLegacyTransport.Native, + AbLegacyCliTransport.Libplctag => AbLegacyTransport.Libplctag, + _ => null, + }; +} diff --git a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli/AbLegacyCommandBase.cs b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli/AbLegacyCommandBase.cs index 36074744..2354b91a 100644 --- a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli/AbLegacyCommandBase.cs +++ b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli/AbLegacyCommandBase.cs @@ -27,9 +27,9 @@ public abstract class AbLegacyCommandBase : DriverCommandBase /// Gets the transport stack used to reach the PLC. [CommandOption("transport", Description = - "Transport stack: libplctag (default) or native (managed EtherNet/IP + PCCC; PLC-5 N-file reads only, " + - "no status-bit writes, LogixPccc unverified).")] - public AbLegacyTransport Transport { get; init; } = AbLegacyTransport.Libplctag; + "Transport stack: default, native or libplctag. default = native, except libplctag for --plc-type " + + "Plc5 / LogixPccc (native there is PLC-5 N-file reads only, no status-bit writes, LogixPccc unverified).")] + public AbLegacyCliTransport Transport { get; init; } = AbLegacyCliTransport.Default; /// Gets the per-operation timeout in milliseconds. [CommandOption("timeout-ms", Description = "Per-operation timeout in ms (default 5000).")] @@ -62,7 +62,7 @@ public abstract class AbLegacyCommandBase : DriverCommandBase RawTags = [.. tags.Select(t => new RawTagEntry( t.Name, AbLegacyTagDefinitionFactory.ToTagConfig(t), t.WriteIdempotent, DeviceName: t.DeviceHostAddress))], Timeout = Timeout, - Transport = Transport, + Transport = Transport.ToConfigured(), Probe = new AbLegacyProbeOptions { Enabled = false }, }; diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/AbCipDriverOptions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/AbCipDriverOptions.cs index 4a75c297..295491ee 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/AbCipDriverOptions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/AbCipDriverOptions.cs @@ -33,14 +33,20 @@ public sealed class AbCipDriverOptions public IReadOnlyList RawTags { get; init; } = []; /// - /// Which transport stack the driver, its Test Connect probe and the CLI use. Defaults to - /// . + /// Which transport stack the driver, its Test Connect probe and the CLI use. null (the + /// transport key absent) means "use the default", which is ; + /// an explicit value always wins. The effective transport is resolved in one place, + /// AbCipTransportResolver. Set to fall back. /// Changing this takes effect when the driver instance is RECREATED, not on /// ReinitializeAsync: the factories are chosen once in the constructor and held across /// Shutdown+Initialize. A deploy that changes the driver config stops and respawns the instance /// (DriverSpawnPlanner, #516), so an operator edit does recreate it. /// - public AbCipTransport Transport { get; init; } = AbCipTransport.Libplctag; + /// Null is never written: an absent key IS the "use the default" encoding, so a round-trip + /// through any serializer (the AdminUI form included) keeps a defaulted driver defaulted rather than + /// pinning it to whatever the default happened to be on the day it was saved. + [System.Text.Json.Serialization.JsonIgnore(Condition = System.Text.Json.Serialization.JsonIgnoreCondition.WhenWritingNull)] + public AbCipTransport? Transport { get; init; } /// Per-device probe settings. Falls back to defaults when omitted. public AbCipProbeOptions Probe { get; init; } = new(); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs index 39c3985b..534f90bc 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs @@ -137,9 +137,9 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, /// Initializes a new instance of the class. /// The driver configuration options. /// A unique identifier for this driver instance. - /// Optional factory for creating tag runtimes; uses libplctag default if null. - /// Optional factory for enumerating tags; uses libplctag default if null. - /// Optional factory for reading UDT templates; uses libplctag default if null. + /// Optional factory for creating tag runtimes; when null the resolved transport's (see ) is used. + /// Optional factory for enumerating tags; when null the resolved transport's is used. + /// Optional factory for reading UDT templates; when null the resolved transport's is used. /// Optional logger; uses null logger if not provided. public AbCipDriver(AbCipDriverOptions options, string driverInstanceId, IAbCipTagFactory? tagFactory = null, @@ -152,8 +152,9 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, _driverInstanceId = driverInstanceId; // Chosen once here and held across ReinitializeAsync (Shutdown+Initialize): a changed // Transport in a reinit config is inert; a deploy recreates the instance instead. - _transport = options.Transport; - var defaults = AbCipTransportFactories.For(options.Transport); + _transport = AbCipTransportResolver.Resolve(options); + _transportDescription = AbCipTransportResolver.Describe(options.Transport); + var defaults = AbCipTransportFactories.For(_transport); _tagFactory = tagFactory ?? defaults.Tags; _enumeratorFactory = enumeratorFactory ?? defaults.Enumerators; _templateReaderFactory = templateReaderFactory ?? defaults.Templates; @@ -179,10 +180,16 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, /// The tag factory chosen at construction; exposed so tests can pin transport selection. internal IAbCipTagFactory TagFactory => _tagFactory; + /// The effective transport resolved at construction (see ). + internal AbCipTransport EffectiveTransport => _transport; + // The transport chosen at construction (see the ctor); logged at every init so an operator can tell // from the node log which stack serves this instance. private readonly AbCipTransport _transport; + // Configured-vs-effective rendering of _transport for the init log, e.g. "Native (default)". + private readonly string _transportDescription; + /// Upper bound on the poll-loop failure backoff — adopts the S7-proven 30 s cap fleet-wide (05/STAB-8). private static readonly TimeSpan PollBackoffCap = TimeSpan.FromSeconds(30); @@ -289,7 +296,7 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, _health = new DriverHealth(DriverState.Initializing, null, null); _logger.LogInformation( "AbCip driver {DriverInstanceId} initializing: transport={Transport} tagFactory={TagFactory}", - _driverInstanceId, _transport, _tagFactory.GetType().Name); + _driverInstanceId, _transportDescription, _tagFactory.GetType().Name); try { if (!string.IsNullOrWhiteSpace(driverConfigJson)) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverFactoryExtensions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverFactoryExtensions.cs index 7819ec72..4016d6d5 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverFactoryExtensions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverFactoryExtensions.cs @@ -96,8 +96,11 @@ public static class AbCipDriverFactoryExtensions ProbeTagPath = dto.Probe?.ProbeTagPath, }, Timeout = PositiveTimeoutOrDefault(dto.TimeoutMs, 2_000), - Transport = ParseEnum(dto.Transport, "driver", driverInstanceId, "Transport", - fallback: AbCipTransport.Libplctag), + // Absent ⇒ null ("use the default", resolved by AbCipTransportResolver); a bad value still + // fails exactly as any other bad enum field does. + Transport = string.IsNullOrWhiteSpace(dto.Transport) + ? null + : ParseEnum(dto.Transport, DriverConfigOwner, driverInstanceId, "Transport"), EnableControllerBrowse = dto.EnableControllerBrowse ?? false, EnableAlarmProjection = dto.EnableAlarmProjection ?? false, AlarmPollInterval = TimeSpan.FromMilliseconds(dto.AlarmPollIntervalMs ?? 1_000), @@ -120,6 +123,10 @@ public static class AbCipDriverFactoryExtensions private static TimeSpan PositiveTimeoutOrDefault(int? ms, int defaultMs) => TimeSpan.FromMilliseconds(ms is int v and > 0 ? v : defaultMs); + // Names the owner of a driver-level enum field in ParseEnum's error text. A named constant rather + // than an inline literal: RoleAuthorityGuardTests forbids a bare role-name string literal outside RoleParser. + private const string DriverConfigOwner = "(driver config)"; + private static T ParseEnum(string? raw, string? tagName, string driverInstanceId, string field, T? fallback = null) where T : struct, Enum { diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverProbe.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverProbe.cs index 7fe144c3..a6baf074 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverProbe.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverProbe.cs @@ -123,7 +123,7 @@ public sealed class AbCipDriverProbe : IDriverProbe ConnectionSize: firstDevice?.ConnectionSize ?? profile.DefaultConnectionSize, UseConnectedMessaging: profile.SupportsConnectedMessaging); - var rt = (_tagFactory ?? AbCipTransportFactories.For(opts.Transport).Tags).Create(p); + var rt = (_tagFactory ?? AbCipTransportFactories.For(AbCipTransportResolver.Resolve(opts)).Tags).Create(p); try { await rt.InitializeAsync(cts.Token); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipTransportResolver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipTransportResolver.cs new file mode 100644 index 00000000..07a7f889 --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipTransportResolver.cs @@ -0,0 +1,36 @@ +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip; + +/// +/// The ONE place an AbCip driver's effective transport is decided from its configured +/// . The driver constructor, the Test Connect probe and the +/// CLI (through the driver) all resolve through here, so they cannot disagree about what an absent +/// transport key means. +/// +public static class AbCipTransportResolver +{ + /// The transport an absent / null transport key selects. + public const AbCipTransport Default = AbCipTransport.Native; + + /// Returns the effective transport: an explicit value always wins; null means . + /// The configured transport, or null for "use the default". + /// The transport the driver runs on. + public static AbCipTransport Resolve(AbCipTransport? configured) => configured ?? Default; + + /// Returns the effective transport for . + /// The driver options. + /// The transport the driver runs on. + public static AbCipTransport Resolve(AbCipDriverOptions options) + { + ArgumentNullException.ThrowIfNull(options); + return Resolve(options.Transport); + } + + /// + /// Operator-facing description of the configured-vs-effective transport for the init log line: + /// Native (default) when the key was absent, the bare transport name when it was explicit. + /// + /// The configured transport, or null. + /// A short description. + public static string Describe(AbCipTransport? configured) => + configured is null ? $"{Resolve(configured)} (default)" : $"{configured} (configured)"; +} diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/AbLegacyDriverOptions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/AbLegacyDriverOptions.cs index 15c8b754..781bd0f1 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/AbLegacyDriverOptions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/AbLegacyDriverOptions.cs @@ -26,14 +26,21 @@ public sealed class AbLegacyDriverOptions public IReadOnlyList RawTags { get; init; } = []; /// - /// Which transport stack the driver, its Test Connect probe and the CLI use. Defaults to - /// . + /// Which transport stack the driver, its Test Connect probe and the CLI use. null (the + /// transport key absent) means "use the default": , UNLESS + /// any configured device is a PLC-5 or LogixPccc (limited native support), in which case the whole + /// driver resolves to . An explicit value always wins. The + /// effective transport is resolved in one place, AbLegacyTransportResolver. /// Changing this takes effect when the driver instance is RECREATED, not on /// ReinitializeAsync: the tag factory is chosen once in the constructor and held across /// Shutdown+Initialize. A deploy that changes the driver config stops and respawns the instance /// (DriverSpawnPlanner, #516), so an operator edit does recreate it. /// - public AbLegacyTransport Transport { get; init; } = AbLegacyTransport.Libplctag; + /// Null is never written: an absent key IS the "use the default" encoding, so a round-trip + /// through any serializer (the AdminUI form included) keeps a defaulted driver defaulted rather than + /// pinning it to whatever the default happened to be on the day it was saved. + [System.Text.Json.Serialization.JsonIgnore(Condition = System.Text.Json.Serialization.JsonIgnoreCondition.WhenWritingNull)] + public AbLegacyTransport? Transport { get; init; } /// Gets or sets the probe (connectivity check) options. public AbLegacyProbeOptions Probe { get; init; } = new(); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs index a6b1dcf8..6f132660 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs @@ -70,8 +70,9 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover _driverInstanceId = driverInstanceId; // Chosen once here and held across ReinitializeAsync (Shutdown+Initialize): a changed // Transport in a reinit config is inert; a deploy recreates the instance instead. - _transport = options.Transport; - _tagFactory = tagFactory ?? (options.Transport == AbLegacyTransport.Native + _transport = AbLegacyTransportResolver.Resolve(options); + _transportDescription = AbLegacyTransportResolver.Describe(options.Transport, _transport); + _tagFactory = tagFactory ?? (_transport == AbLegacyTransport.Native ? new NativeAbLegacyTagFactory() : new LibplctagLegacyTagFactory()); _logger = logger ?? NullLogger.Instance; @@ -93,10 +94,16 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover /// The tag factory chosen at construction; exposed so tests can pin transport selection. internal IAbLegacyTagFactory TagFactory => _tagFactory; + /// The effective transport resolved at construction (see ). + internal AbLegacyTransport EffectiveTransport => _transport; + // The transport chosen at construction (see the ctor); logged at every init so an operator can tell // from the node log which stack serves this instance. private readonly AbLegacyTransport _transport; + // Configured-vs-effective rendering of _transport for the init log, e.g. "Native (default)". + private readonly string _transportDescription; + /// Upper bound on the poll-loop failure backoff — adopts the S7-proven 30 s cap fleet-wide (05/STAB-8). private static readonly TimeSpan PollBackoffCap = TimeSpan.FromSeconds(30); @@ -136,7 +143,7 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover _health = new DriverHealth(DriverState.Initializing, null, null); _logger.LogInformation( "AbLegacy driver {DriverInstanceId} initializing: transport={Transport} tagFactory={TagFactory}", - _driverInstanceId, _transport, _tagFactory.GetType().Name); + _driverInstanceId, _transportDescription, _tagFactory.GetType().Name); try { // #516: re-parse the supplied DriverConfig so a config change delivered through the IDriver @@ -145,6 +152,19 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover if (HasConfigBody(driverConfigJson)) _options = AbLegacyDriverFactoryExtensions.ParseOptions(_driverInstanceId, driverConfigJson); + // The resolver only defaults a PLC-5 / LogixPccc driver to libplctag; an operator may still pick + // Native explicitly (or a reinit may add such a device to a driver resolved Native). Allowed, but + // the limitation is client-visible, so name it once per init. + if (_transport == AbLegacyTransport.Native + && AbLegacyTransportResolver.NativeLimitedFamilies(_options.Devices) is { Count: > 0 } limited) + { + _logger.LogWarning( + "AbLegacy driver {DriverInstanceId} runs the Native transport against {Families} device(s); native " + + "support is limited there (PLC-5: N-file reads only; LogixPccc: framing unverified). Set " + + "\"transport\": \"Libplctag\" to use the fallback stack.", + _driverInstanceId, string.Join(", ", limited)); + } + foreach (var device in _options.Devices) { var addr = AbLegacyHostAddress.TryParse(device.HostAddress) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriverFactoryExtensions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriverFactoryExtensions.cs index dc8b0aff..f6963b4d 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriverFactoryExtensions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriverFactoryExtensions.cs @@ -97,8 +97,11 @@ public static class AbLegacyDriverFactoryExtensions ProbeAddress = dto.Probe?.ProbeAddress ?? "S:0", }, Timeout = PositiveTimeoutOrDefault(dto.TimeoutMs, 2_000), - Transport = ParseEnum(dto.Transport, driverInstanceId, "Transport", - fallback: AbLegacyTransport.Libplctag), + // Absent ⇒ null ("use the default", resolved by AbLegacyTransportResolver against the devices); + // a bad value still fails exactly as any other bad enum field does. + Transport = string.IsNullOrWhiteSpace(dto.Transport) + ? null + : ParseEnum(dto.Transport, driverInstanceId, "Transport"), }; return options; diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriverProbe.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriverProbe.cs index 2856a491..d1b05167 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriverProbe.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriverProbe.cs @@ -106,7 +106,7 @@ public sealed class AbLegacyDriverProbe : IDriverProbe TagName: tagName, Timeout: timeout); - var factory = _tagFactory ?? (opts.Transport == AbLegacyTransport.Native + var factory = _tagFactory ?? (AbLegacyTransportResolver.Resolve(opts) == AbLegacyTransport.Native ? new NativeAbLegacyTagFactory() : new LibplctagLegacyTagFactory()); var rt = factory.Create(p); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyTransportResolver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyTransportResolver.cs new file mode 100644 index 00000000..fdadc7c5 --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyTransportResolver.cs @@ -0,0 +1,64 @@ +using ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.PlcFamilies; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbLegacy; + +/// +/// The ONE place an AbLegacy driver's effective transport is decided from its configured +/// and its devices. The driver constructor, the Test +/// Connect probe and the CLI (through the driver) all resolve through here. +/// An absent key means — UNLESS any configured device is a +/// or , whose native support +/// is limited (PLC-5: N-file reads only; LogixPccc: framing unverified). The tag factory is chosen once +/// per DRIVER, not per device, so one such device resolves the whole driver to +/// . An explicit value always wins (an operator choice). +/// +public static class AbLegacyTransportResolver +{ + /// Returns true for a family whose native transport support is limited. + /// The PLC family. + /// True for PLC-5 and LogixPccc. + public static bool IsNativeLimited(AbLegacyPlcFamily family) => + family is AbLegacyPlcFamily.Plc5 or AbLegacyPlcFamily.LogixPccc; + + /// Returns the distinct native-limited families among . + /// The configured devices. + /// The limited families present, in declaration order. + public static IReadOnlyList NativeLimitedFamilies(IEnumerable devices) + { + ArgumentNullException.ThrowIfNull(devices); + return [.. devices.Select(d => d.PlcFamily).Where(IsNativeLimited).Distinct()]; + } + + /// Returns the effective transport for a configured value and device set. + /// The configured transport, or null for "use the default". + /// The configured devices. + /// The transport the driver runs on. + public static AbLegacyTransport Resolve(AbLegacyTransport? configured, IEnumerable devices) + { + ArgumentNullException.ThrowIfNull(devices); + if (configured is { } explicitValue) return explicitValue; + return devices.Any(d => IsNativeLimited(d.PlcFamily)) ? AbLegacyTransport.Libplctag : AbLegacyTransport.Native; + } + + /// Returns the effective transport for . + /// The driver options. + /// The transport the driver runs on. + public static AbLegacyTransport Resolve(AbLegacyDriverOptions options) + { + ArgumentNullException.ThrowIfNull(options); + return Resolve(options.Transport, options.Devices); + } + + /// + /// Operator-facing description of the configured-vs-effective transport for the init log line, e.g. + /// Native (default), Libplctag (default; PLC-5/LogixPccc device present) or + /// Native (configured). + /// + /// The configured transport, or null. + /// The effective transport. + /// A short description. + public static string Describe(AbLegacyTransport? configured, AbLegacyTransport effective) => + configured is not null ? $"{effective} (configured)" + : effective == AbLegacyTransport.Libplctag ? $"{effective} (default; PLC-5/LogixPccc device present)" + : $"{effective} (default)"; +} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/AbCipDriverForm.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/AbCipDriverForm.razor index 616ad7d4..d817277b 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/AbCipDriverForm.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/AbCipDriverForm.razor @@ -35,10 +35,11 @@
- + + -
Which protocol stack talks to the controller. Takes effect on the next deploy.
+
Which protocol stack talks to the controller. Default omits the setting so the driver picks; takes effect on the next deploy.
@@ -175,7 +176,10 @@ { // Operation public int TimeoutSeconds { get; set; } = 2; - public AbCipTransport Transport { get; set; } = AbCipTransport.Libplctag; + // Null = "Default": the transport key is OMITTED on save so the driver resolves it (and the AbLegacy + // PLC-5 / LogixPccc exclusion still applies). Never re-emit a concrete default here — that would pin + // every edited driver to it. + public AbCipTransport? Transport { get; set; } public bool EnableControllerBrowse { get; set; } = false; public bool EnableDeclarationOnlyUdtGrouping { get; set; } = false; diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/AbLegacyDriverForm.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/AbLegacyDriverForm.razor index b3bd135e..b5397a76 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/AbLegacyDriverForm.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/AbLegacyDriverForm.razor @@ -22,10 +22,11 @@
- + + -
Which protocol stack talks to the controller. Takes effect on the next deploy.
+
Which protocol stack talks to the controller. Default omits the setting so the driver picks; takes effect on the next deploy.
@@ -140,7 +141,10 @@ { // Operation public int TimeoutSeconds { get; set; } = 2; - public AbLegacyTransport Transport { get; set; } = AbLegacyTransport.Libplctag; + // Null = "Default": the transport key is OMITTED on save so the driver resolves it (and the AbLegacy + // PLC-5 / LogixPccc exclusion still applies). Never re-emit a concrete default here — that would pin + // every edited driver to it. + public AbLegacyTransport? Transport { get; set; } // Probe public bool ProbeEnabled { get; set; } = true; diff --git a/tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli.Tests/AbCipTransportOptionTests.cs b/tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli.Tests/AbCipTransportOptionTests.cs index 6c36ce95..3e2c6b92 100644 --- a/tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli.Tests/AbCipTransportOptionTests.cs +++ b/tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli.Tests/AbCipTransportOptionTests.cs @@ -14,26 +14,34 @@ public sealed class AbCipTransportOptionTests private sealed class CapturingCommand : AbCipCommandBase { public static AbCipTransport? Captured; + public static bool Ran; public override ValueTask ExecuteAsync(IConsole console) { + Ran = true; Captured = BuildOptions([]).Transport; return default; } } - /// Verifies the default is libplctag when the flag is absent. + /// + /// With no flag the CLI leaves the option null ("use the default"), and the driver's resolver makes + /// that Native — the same path an absent transport key takes in a deployed config. + /// [Fact] - public async Task Transport_defaults_to_libplctag() + public async Task Transport_defaults_to_the_resolver_default_native() { CapturingCommand.Captured = null; + CapturingCommand.Ran = false; using var console = new FakeInMemoryConsole(); var app = new CliApplicationBuilder().AddCommand(typeof(CapturingCommand)).UseConsole(console).Build(); var exit = await app.RunAsync(["capture-transport", "--gateway", "ab://1.2.3.4/1,0"]); exit.ShouldBe(0, console.ReadErrorString()); - CapturingCommand.Captured.ShouldBe(AbCipTransport.Libplctag); + CapturingCommand.Ran.ShouldBeTrue(); + CapturingCommand.Captured.ShouldBeNull(); + AbCipTransportResolver.Resolve(CapturingCommand.Captured).ShouldBe(AbCipTransport.Native); } /// Verifies CliFx parses --transport (case-insensitive) into the options. @@ -41,8 +49,10 @@ public sealed class AbCipTransportOptionTests [InlineData("native", AbCipTransport.Native)] [InlineData("Native", AbCipTransport.Native)] [InlineData("libplctag", AbCipTransport.Libplctag)] - public async Task Transport_flag_is_parsed_by_the_cli(string value, AbCipTransport expected) + [InlineData("default", null)] + public async Task Transport_flag_is_parsed_by_the_cli(string value, AbCipTransport? expected) { + CapturingCommand.Ran = false; CapturingCommand.Captured = null; using var console = new FakeInMemoryConsole(); var app = new CliApplicationBuilder().AddCommand(typeof(CapturingCommand)).UseConsole(console).Build(); @@ -50,6 +60,7 @@ public sealed class AbCipTransportOptionTests var exit = await app.RunAsync(["capture-transport", "--gateway", "ab://1.2.3.4/1,0", "--transport", value]); exit.ShouldBe(0, console.ReadErrorString()); + CapturingCommand.Ran.ShouldBeTrue(); CapturingCommand.Captured.ShouldBe(expected); } } diff --git a/tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli.Tests/BuildOptionsTests.cs b/tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli.Tests/BuildOptionsTests.cs index a3bc9231..a806eebf 100644 --- a/tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli.Tests/BuildOptionsTests.cs +++ b/tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli.Tests/BuildOptionsTests.cs @@ -161,42 +161,79 @@ public sealed class BuildOptionsTests options.Devices[0].PlcFamily.ShouldBe(AbLegacyPlcFamily.Plc5); } - /// Verifies the transport defaults to libplctag and --transport reaches the options. - [Fact] - public void BuildOptions_defaults_to_libplctag_and_forwards_transport() + /// + /// With no --transport the CLI leaves the option null and the driver's resolver decides: Native + /// for an SLC 500, libplctag for --plc-type Plc5 / LogixPccc (Task 22 exclusion). An explicit + /// choice is forwarded verbatim and wins. + /// + [Theory] + [InlineData(AbLegacyPlcFamily.Slc500, AbLegacyTransport.Native)] + [InlineData(AbLegacyPlcFamily.MicroLogix, AbLegacyTransport.Native)] + [InlineData(AbLegacyPlcFamily.Plc5, AbLegacyTransport.Libplctag)] + [InlineData(AbLegacyPlcFamily.LogixPccc, AbLegacyTransport.Libplctag)] + public void BuildOptions_default_transport_goes_through_the_resolver(AbLegacyPlcFamily family, AbLegacyTransport expected) { - var def = new TestCommand { Gateway = "ab://192.168.1.20/1,0" }; - def.Build([]).Transport.ShouldBe(AbLegacyTransport.Libplctag); + var options = new TestCommand { Gateway = "ab://192.168.1.20/1,0", PlcType = family }.Build([]); + options.Transport.ShouldBeNull(); + AbLegacyTransportResolver.Resolve(options).ShouldBe(expected); - var native = new TestCommand { Gateway = "ab://192.168.1.20/1,0", Transport = AbLegacyTransport.Native }; - native.Build([]).Transport.ShouldBe(AbLegacyTransport.Native); + var native = new TestCommand + { + Gateway = "ab://192.168.1.20/1,0", PlcType = family, Transport = AbLegacyCliTransport.Native, + }.Build([]); + native.Transport.ShouldBe(AbLegacyTransport.Native); + AbLegacyTransportResolver.Resolve(native).ShouldBe(AbLegacyTransport.Native); } - /// Verifies CliFx really parses --transport native (case-insensitive) into the property. + /// + /// The plan's CLI case end to end through CliFx: --plc-type plc5 with no --transport + /// resolves to libplctag; the same invocation for an SLC resolves Native (falsification control). + /// + [Theory] + [InlineData("plc5", AbLegacyTransport.Libplctag)] + [InlineData("logixpccc", AbLegacyTransport.Libplctag)] + [InlineData("slc500", AbLegacyTransport.Native)] + public async Task Plc_type_without_transport_flag_resolves_through_the_resolver(string plcType, AbLegacyTransport expected) + { + CapturingCommand.Options = null; + using var console = new FakeInMemoryConsole(); + var app = new CliApplicationBuilder().AddCommand(typeof(CapturingCommand)).UseConsole(console).Build(); + + var exit = await app.RunAsync(["capture-transport", "--gateway", "ab://1.2.3.4/1,0", "--plc-type", plcType]); + + exit.ShouldBe(0, console.ReadErrorString()); + CapturingCommand.Options.ShouldNotBeNull(); + CapturingCommand.Options.Transport.ShouldBeNull(); + AbLegacyTransportResolver.Resolve(CapturingCommand.Options).ShouldBe(expected); + } + + /// Verifies CliFx really parses --transport (case-insensitive) into the property. [Theory] [InlineData("native", AbLegacyTransport.Native)] [InlineData("Native", AbLegacyTransport.Native)] [InlineData("libplctag", AbLegacyTransport.Libplctag)] - public async Task Transport_flag_is_parsed_by_the_cli(string value, AbLegacyTransport expected) + [InlineData("default", null)] + public async Task Transport_flag_is_parsed_by_the_cli(string value, AbLegacyTransport? expected) { - CapturingCommand.Captured = null; + CapturingCommand.Options = null; using var console = new FakeInMemoryConsole(); var app = new CliApplicationBuilder().AddCommand(typeof(CapturingCommand)).UseConsole(console).Build(); var exit = await app.RunAsync(["capture-transport", "--gateway", "ab://1.2.3.4/1,0", "--transport", value]); exit.ShouldBe(0, console.ReadErrorString()); - CapturingCommand.Captured.ShouldBe(expected); + CapturingCommand.Options.ShouldNotBeNull(); + CapturingCommand.Options.Transport.ShouldBe(expected); } [Command("capture-transport")] private sealed class CapturingCommand : AbLegacyCommandBase { - public static AbLegacyTransport? Captured; + public static AbLegacyDriverOptions? Options; public override System.Threading.Tasks.ValueTask ExecuteAsync(IConsole console) { - Captured = BuildOptions([]).Transport; + Options = BuildOptions([]); return default; } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipTransportInitLogTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipTransportInitLogTests.cs index 3c54b962..91049efa 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipTransportInitLogTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipTransportInitLogTests.cs @@ -12,13 +12,15 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests; [Trait("Category", "Unit")] public sealed class AbCipTransportInitLogTests { - private static string Config(string transport) => - $$"""{"transport":"{{transport}}","probe":{"enabled":false},"devices":[{"hostAddress":"ab://10.0.0.1:44818/1,0","plcFamily":"ControlLogix"}]}"""; + private static string Config(string? transport) => + $$"""{{{(transport is null ? "" : $"\"transport\":\"{transport}\",")}}"probe":{"enabled":false},"devices":[{"hostAddress":"ab://10.0.0.1:44818/1,0","plcFamily":"ControlLogix"}]}"""; + /// The init line names the configured-vs-effective transport (Task 22). [Theory] - [InlineData("Native", AbCipTransport.Native)] - [InlineData("Libplctag", AbCipTransport.Libplctag)] - public async Task Init_logs_the_transport_through_the_registered_logger_factory(string json, AbCipTransport expected) + [InlineData("Native", "Native (configured)")] + [InlineData("Libplctag", "Libplctag (configured)")] + [InlineData(null, "Native (default)")] + public async Task Init_logs_the_transport_through_the_registered_logger_factory(string? json, string expected) { var captured = new List(); using var factory = new CapturingLoggerFactory(captured); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipTransportSwitchTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipTransportSwitchTests.cs index 0b274df2..5375101d 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipTransportSwitchTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipTransportSwitchTests.cs @@ -14,17 +14,23 @@ public sealed class AbCipTransportSwitchTests private static string Config(string? transportJson) => $$"""{{{(transportJson is null ? "" : $"\"transport\":{transportJson},")}}"devices":[{{Device}}]}"""; - /// Transport values select the factory trio; absent defaults to libplctag. + /// + /// Transport values select the factory trio; absent parses to null ("use the default") and the driver + /// resolves that to Native (Task 22). An explicit Libplctag still wins. + /// [Theory] [InlineData("\"Native\"", AbCipTransport.Native)] [InlineData("\"native\"", AbCipTransport.Native)] [InlineData("\"Libplctag\"", AbCipTransport.Libplctag)] - [InlineData(null, AbCipTransport.Libplctag)] + [InlineData(null, AbCipTransport.Native)] public void Config_transport_selects_the_factory(string? json, AbCipTransport expected) { - AbCipDriverFactoryExtensions.ParseOptions("ab1", Config(json)).Transport.ShouldBe(expected); + var parsed = AbCipDriverFactoryExtensions.ParseOptions("ab1", Config(json)); + if (json is null) parsed.Transport.ShouldBeNull(); + else parsed.Transport.ShouldBe(expected); var driver = AbCipDriverFactoryExtensions.CreateInstance("ab1", Config(json)); + driver.EffectiveTransport.ShouldBe(expected); if (expected == AbCipTransport.Native) driver.TagFactory.ShouldBeOfType(); else @@ -119,6 +125,41 @@ public sealed class AbCipTransportSwitchTests lib.Message!.ShouldNotContain("Connection lost"); } + /// + /// The probe resolves an ABSENT transport through the same resolver as the driver: with no key it takes + /// the native path (native failure text), and an explicit Libplctag on the same listener does not. + /// + [Fact] + public async Task Probe_with_no_transport_resolves_like_the_driver_to_native() + { + using var listener = new TcpListener(IPAddress.Loopback, 0); + listener.Start(); + var port = ((IPEndPoint)listener.LocalEndpoint).Port; + _ = Task.Run(async () => + { + while (true) + { + try { (await listener.AcceptTcpClientAsync(TestContext.Current.CancellationToken)).Dispose(); } + catch { return; } + } + }, TestContext.Current.CancellationToken); + + var device = $$"""{"hostAddress":"ab://127.0.0.1:{{port}}/1,0"}"""; + var probe = new AbCipDriverProbe(); + + var absent = await probe.ProbeAsync($$"""{"devices":[{{device}}]}""", TimeSpan.FromSeconds(3), + TestContext.Current.CancellationToken); + var lib = await probe.ProbeAsync($$"""{"transport":"Libplctag","devices":[{{device}}]}""", TimeSpan.FromSeconds(3), + TestContext.Current.CancellationToken); + listener.Stop(); + + AbCipTransportResolver.Resolve(AbCipDriverFactoryExtensions.ParseOptions("p", $$"""{"devices":[{{device}}]}""")) + .ShouldBe(AbCipTransport.Native); + absent.Ok.ShouldBeFalse(); + absent.Message!.ShouldContain("CIP handshake failed: Connection lost"); + lib.Message!.ShouldNotContain("Connection lost"); + } + /// The probe deserialises with its own string-enum converter; a bad transport is a config error. [Fact] public async Task Probe_with_a_bad_transport_reports_invalid_config() diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyReinitConfigAdoptionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyReinitConfigAdoptionTests.cs index 4aaa3feb..1894c9c9 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyReinitConfigAdoptionTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyReinitConfigAdoptionTests.cs @@ -39,9 +39,12 @@ public sealed class AbLegacyReinitConfigAdoptionTests [Fact] public async Task Reinitialize_with_an_empty_document_keeps_the_constructor_options() { + // Pinned to libplctag (Task 22): this init starts the default probe loop against 10.0.0.1 with no + // injected factory, and was written and proven on libplctag. Pinning keeps it off the process-wide + // native DeviceSessionPool rather than silently changing what it exercises. var driver = AbLegacyDriverFactoryExtensions.CreateInstance( "ab1", - """{"devices":[{"hostAddress":"ab://10.0.0.1:44818/1,0","plcFamily":"Slc500"}]}""", + """{"transport":"Libplctag","devices":[{"hostAddress":"ab://10.0.0.1:44818/1,0","plcFamily":"Slc500"}]}""", loggerFactory: null); await Should.NotThrowAsync(() => driver.ReinitializeAsync("{}", CancellationToken.None)); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyTransportInitLogTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyTransportInitLogTests.cs index f16ffb3c..223656d5 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyTransportInitLogTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyTransportInitLogTests.cs @@ -12,44 +12,73 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests; [Trait("Category", "Unit")] public sealed class AbLegacyTransportInitLogTests { - private static string Config(string transport) => - $$"""{"transport":"{{transport}}","probe":{"enabled":false},"devices":[{"hostAddress":"ab://10.0.0.1:44818/1,0","plcFamily":"Slc500"}]}"""; + private static string Config(string? transport, string family = "Slc500") => + $$"""{{{(transport is null ? "" : $"\"transport\":\"{transport}\",")}}"probe":{"enabled":false},"devices":[{"hostAddress":"ab://10.0.0.1:44818/1,0","plcFamily":"{{family}}"}]}"""; + /// The init line names the configured-vs-effective transport (Task 22). [Theory] - [InlineData("Native", AbLegacyTransport.Native)] - [InlineData("Libplctag", AbLegacyTransport.Libplctag)] - public async Task Init_logs_the_transport_through_the_registered_logger_factory(string json, AbLegacyTransport expected) + [InlineData("Native", "Slc500", "Native (configured)")] + [InlineData("Libplctag", "Slc500", "Libplctag (configured)")] + [InlineData(null, "Slc500", "Native (default)")] + [InlineData(null, "Plc5", "Libplctag (default; PLC-5/LogixPccc device present)")] + public async Task Init_logs_the_transport_through_the_registered_logger_factory( + string? json, string family, string expected) { - var captured = new List(); + var captured = await InitAndCaptureAsync(Config(json, family)); + captured.ShouldContain(m => m.Message.Contains("drv-t19") && m.Message.Contains($"transport={expected}")); + } + + /// + /// An explicit Native against a PLC-5 / LogixPccc device is an allowed operator choice, but logs ONE + /// Warning naming the limitation. The defaulted and explicit-libplctag rows are the falsification + /// controls: same device families, no warning. + /// + [Theory] + [InlineData("Native", "Plc5", true)] + [InlineData("Native", "LogixPccc", true)] + [InlineData(null, "Plc5", false)] + [InlineData("Libplctag", "LogixPccc", false)] + [InlineData("Native", "Slc500", false)] + public async Task Explicit_Native_on_a_limited_family_warns_once(string? json, string family, bool warns) + { + var captured = await InitAndCaptureAsync(Config(json, family)); + var warnings = captured.Where(m => m.Level == LogLevel.Warning && m.Message.Contains("native support is limited")) + .ToList(); + warnings.Count.ShouldBe(warns ? 1 : 0); + if (warns) warnings[0].Message.ShouldContain(family); + } + + private static async Task> InitAndCaptureAsync(string config) + { + var captured = new List<(LogLevel, string)>(); using var factory = new CapturingLoggerFactory(captured); - var driver = AbLegacyDriverFactoryExtensions.CreateInstance("drv-t19", Config(json), factory); + var driver = AbLegacyDriverFactoryExtensions.CreateInstance("drv-t19", config, factory); try { - await driver.InitializeAsync(Config(json), TestContext.Current.CancellationToken); + await driver.InitializeAsync(config, TestContext.Current.CancellationToken); } finally { await driver.ShutdownAsync(TestContext.Current.CancellationToken); } - - captured.ShouldContain(m => m.Contains("drv-t19") && m.Contains($"transport={expected}")); + lock (captured) return [.. captured]; } - private sealed class CapturingLoggerFactory(List sink) : ILoggerFactory + private sealed class CapturingLoggerFactory(List<(LogLevel, string)> sink) : ILoggerFactory { public ILogger CreateLogger(string categoryName) => new CapturingLogger(sink); public void AddProvider(ILoggerProvider provider) { } public void Dispose() { } } - private sealed class CapturingLogger(List sink) : ILogger + private sealed class CapturingLogger(List<(LogLevel, string)> sink) : ILogger { public IDisposable? BeginScope(TState state) where TState : notnull => null; public bool IsEnabled(LogLevel logLevel) => true; public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) { - lock (sink) sink.Add(formatter(state, exception)); + lock (sink) sink.Add((logLevel, formatter(state, exception))); } } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyTransportSwitchTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyTransportSwitchTests.cs index 711bcbcf..3705e60d 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyTransportSwitchTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyTransportSwitchTests.cs @@ -14,23 +14,74 @@ public sealed class AbLegacyTransportSwitchTests private static string Config(string? transportJson) => $$"""{{{(transportJson is null ? "" : $"\"transport\":{transportJson},")}}"devices":[{{Device}}]}"""; - /// Transport values select the factory type; absent defaults to libplctag. + private static string Config(string? transportJson, params string[] families) + { + var devices = string.Join(",", families.Select((f, i) => + $"{{\"hostAddress\":\"ab://10.0.0.{i + 1}:44818/1,0\",\"plcFamily\":\"{f}\"}}")); + var transport = transportJson is null ? "" : $"\"transport\":{transportJson},"; + return $"{{{transport}\"devices\":[{devices}]}}"; + } + + /// + /// Transport values select the factory type; absent parses to null ("use the default") and the driver + /// resolves that to Native for an SLC 500 (Task 22). An explicit Libplctag still wins. + /// [Theory] [InlineData("\"Native\"", AbLegacyTransport.Native)] [InlineData("\"native\"", AbLegacyTransport.Native)] [InlineData("\"Libplctag\"", AbLegacyTransport.Libplctag)] - [InlineData(null, AbLegacyTransport.Libplctag)] + [InlineData(null, AbLegacyTransport.Native)] public void Config_transport_selects_the_factory(string? json, AbLegacyTransport expected) { - AbLegacyDriverFactoryExtensions.ParseOptions("ab1", Config(json)).Transport.ShouldBe(expected); + var parsed = AbLegacyDriverFactoryExtensions.ParseOptions("ab1", Config(json)); + if (json is null) parsed.Transport.ShouldBeNull(); + else parsed.Transport.ShouldBe(expected); var driver = AbLegacyDriverFactoryExtensions.CreateInstance("ab1", Config(json)); + driver.EffectiveTransport.ShouldBe(expected); if (expected == AbLegacyTransport.Native) driver.TagFactory.ShouldBeOfType(); else driver.TagFactory.ShouldBeOfType(); } + /// + /// The exclusion: an absent key resolves the WHOLE driver to libplctag when any device is a PLC-5 or a + /// LogixPccc (limited native support); the factory is driver-level, so one such device among SLCs + /// is enough. The SLC/MicroLogix-only rows are the falsification control — they must resolve Native. + /// + [Theory] + [InlineData(AbLegacyTransport.Libplctag, "Plc5")] + [InlineData(AbLegacyTransport.Libplctag, "LogixPccc")] + [InlineData(AbLegacyTransport.Libplctag, "Slc500", "Plc5")] + [InlineData(AbLegacyTransport.Libplctag, "MicroLogix", "LogixPccc")] + [InlineData(AbLegacyTransport.Native, "Slc500")] + [InlineData(AbLegacyTransport.Native, "MicroLogix")] + [InlineData(AbLegacyTransport.Native, "Slc500", "MicroLogix")] + public void Absent_transport_excludes_PLC5_and_LogixPccc(AbLegacyTransport expected, params string[] families) + { + var json = Config(null, families); + AbLegacyTransportResolver.Resolve(AbLegacyDriverFactoryExtensions.ParseOptions("ab1", json)).ShouldBe(expected); + + var driver = AbLegacyDriverFactoryExtensions.CreateInstance("ab1", json); + driver.EffectiveTransport.ShouldBe(expected); + if (expected == AbLegacyTransport.Native) + driver.TagFactory.ShouldBeOfType(); + else + driver.TagFactory.ShouldBeOfType(); + } + + /// An explicit value always wins over the exclusion, in both directions. + [Theory] + [InlineData("\"Native\"", "Plc5", AbLegacyTransport.Native)] + [InlineData("\"Native\"", "LogixPccc", AbLegacyTransport.Native)] + [InlineData("\"Libplctag\"", "Slc500", AbLegacyTransport.Libplctag)] + public void Explicit_transport_wins_over_the_exclusion(string json, string family, AbLegacyTransport expected) + { + var driver = AbLegacyDriverFactoryExtensions.CreateInstance("ab1", Config(json, family)); + driver.EffectiveTransport.ShouldBe(expected); + } + /// A bad value fails the way an existing bad enum field (PlcFamily) does. [Fact] public void Bad_transport_fails_like_a_bad_PlcFamily() @@ -81,14 +132,14 @@ public sealed class AbLegacyTransportSwitchTests using var listener = new TcpListener(IPAddress.Loopback, 0); listener.Start(); var port = ((IPEndPoint)listener.LocalEndpoint).Port; - var accept = Task.Run(async () => + _ = Task.Run(async () => { while (true) { try { (await listener.AcceptTcpClientAsync(TestContext.Current.CancellationToken)).Dispose(); } catch { return; } } - }); + }, TestContext.Current.CancellationToken); string Cfg(string t) => $$"""{"transport":"{{t}}","devices":[{"hostAddress":"ab://127.0.0.1:{{port}}/1,0"}]}"""; @@ -102,7 +153,44 @@ public sealed class AbLegacyTransportSwitchTests native.Message.ShouldNotBeNull(); // "Connection lost" is the managed EncapsulationClient's wording; libplctag reports a bare status name. native.Message!.ShouldContain("PCCC handshake failed: Connection lost"); - lib.Message.ShouldNotContain("Connection lost"); + lib.Message!.ShouldNotContain("Connection lost"); + } + + /// + /// The probe resolves an ABSENT transport through the same resolver as the driver: an SLC 500 with no + /// key takes the native path (native failure text), a PLC-5 with no key takes libplctag (no native + /// text) — against the same listener, so only the resolution differs. + /// + [Fact] + public async Task Probe_with_no_transport_resolves_like_the_driver() + { + using var listener = new TcpListener(IPAddress.Loopback, 0); + listener.Start(); + var port = ((IPEndPoint)listener.LocalEndpoint).Port; + _ = Task.Run(async () => + { + while (true) + { + try { (await listener.AcceptTcpClientAsync(TestContext.Current.CancellationToken)).Dispose(); } + catch { return; } + } + }, TestContext.Current.CancellationToken); + + string Cfg(string family) => + $$"""{"devices":[{"hostAddress":"ab://127.0.0.1:{{port}}/1,0","plcFamily":"{{family}}"}]}"""; + var probe = new AbLegacyDriverProbe(); + + var slc = await probe.ProbeAsync(Cfg("Slc500"), TimeSpan.FromSeconds(3), TestContext.Current.CancellationToken); + var plc5 = await probe.ProbeAsync(Cfg("Plc5"), TimeSpan.FromSeconds(3), TestContext.Current.CancellationToken); + listener.Stop(); + + AbLegacyTransportResolver.Resolve(AbLegacyDriverFactoryExtensions.ParseOptions("p", Cfg("Slc500"))) + .ShouldBe(AbLegacyTransport.Native); + AbLegacyTransportResolver.Resolve(AbLegacyDriverFactoryExtensions.ParseOptions("p", Cfg("Plc5"))) + .ShouldBe(AbLegacyTransport.Libplctag); + slc.Ok.ShouldBeFalse(); + slc.Message!.ShouldContain("PCCC handshake failed: Connection lost"); + plc5.Message!.ShouldNotContain("Connection lost"); } /// The probe deserialises with its own string-enum converter; a bad transport is a config error. diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/AbCipDriverPageFormSerializationTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/AbCipDriverPageFormSerializationTests.cs index e338494b..b52af00f 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/AbCipDriverPageFormSerializationTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/AbCipDriverPageFormSerializationTests.cs @@ -108,7 +108,8 @@ public sealed class AbCipDriverPageFormSerializationTests [Theory] [InlineData(AbCipTransport.Libplctag)] [InlineData(AbCipTransport.Native)] - public void Transport_survives_FromOptions_ToOptions(AbCipTransport transport) + [InlineData(null)] + public void Transport_survives_FromOptions_ToOptions(AbCipTransport? transport) { var original = new AbCipDriverOptions { Transport = transport }; var back = AbCipDriverForm.FormModel.FromOptions(original).ToOptions([]); @@ -123,35 +124,52 @@ public sealed class AbCipDriverPageFormSerializationTests json.ShouldContain("\"transport\":\"Native\""); } - [Fact] - public void Editing_an_unrelated_field_preserves_a_saved_Native_transport() + [Theory] + [InlineData("Native", AbCipTransport.Native)] + [InlineData("Libplctag", AbCipTransport.Libplctag)] + public void Editing_an_unrelated_field_preserves_an_explicit_transport(string savedValue, AbCipTransport expected) { // The regression the review found: FromOptions/ToOptions dropped Transport, so an operator who set - // Native was silently reverted to libplctag the next time they saved ANY other edit. - const string saved = """{"transport":"Native","timeout":"00:00:02","devices":[]}"""; + // a transport was silently reverted the next time they saved ANY other edit. + var saved = $$"""{"transport":"{{savedValue}}","timeout":"00:00:02","devices":[]}"""; var loaded = AbCipDriverForm.TryDeserialize(saved); loaded.ShouldNotBeNull(); - loaded.Transport.ShouldBe(AbCipTransport.Native); + loaded.Transport.ShouldBe(expected); var form = AbCipDriverForm.FormModel.FromOptions(loaded); form.TimeoutSeconds = 7; var json = AbCipDriverForm.Serialize(form.ToOptions(loaded.Devices)); - json.ShouldContain("\"transport\":\"Native\""); + json.ShouldContain($"\"transport\":\"{savedValue}\""); json.ShouldContain("\"timeout\":\"00:00:07\""); } [Fact] - public void A_config_without_transport_loads_as_Libplctag_and_re_emits_it_explicitly() + public void A_config_without_transport_loads_as_Default_and_an_unrelated_edit_keeps_the_key_absent() { - // Chosen behaviour: an absent key round-trips as an explicit "Libplctag" (the form always writes the - // field). The dropdown therefore shows "libplctag (default)". + // Task 22 reverses Task 19's "re-emit an explicit Libplctag": that would pin every edited driver to + // a concrete transport and bypass the driver-side default (and AbLegacy's PLC-5 / LogixPccc + // exclusion). Absent loads as Default (null) and a save that edits something else still omits it. var loaded = AbCipDriverForm.TryDeserialize("""{"probeTimeoutSeconds":5}"""); loaded.ShouldNotBeNull(); var form = AbCipDriverForm.FormModel.FromOptions(loaded); + form.Transport.ShouldBeNull(); + + form.TimeoutSeconds = 7; + var json = AbCipDriverForm.Serialize(form.ToOptions(loaded.Devices)); + json.ShouldNotContain("transport", Case.Insensitive); + json.ShouldContain("\"timeout\":\"00:00:07\""); + } + + [Fact] + public void Choosing_Default_over_an_explicit_transport_drops_the_key() + { + var loaded = AbCipDriverForm.TryDeserialize("""{"transport":"Libplctag"}"""); + loaded.ShouldNotBeNull(); + var form = AbCipDriverForm.FormModel.FromOptions(loaded); form.Transport.ShouldBe(AbCipTransport.Libplctag); - var json = AbCipDriverForm.Serialize(form.ToOptions(loaded.Devices)); - json.ShouldContain("\"transport\":\"Libplctag\""); + form.Transport = null; // the "Default (...)" dropdown option + AbCipDriverForm.Serialize(form.ToOptions(loaded.Devices)).ShouldNotContain("transport", Case.Insensitive); } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/AbLegacyDriverPageFormSerializationTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/AbLegacyDriverPageFormSerializationTests.cs index e77da8ad..2cfa184d 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/AbLegacyDriverPageFormSerializationTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/AbLegacyDriverPageFormSerializationTests.cs @@ -104,7 +104,8 @@ public sealed class AbLegacyDriverPageFormSerializationTests [Theory] [InlineData(AbLegacyTransport.Libplctag)] [InlineData(AbLegacyTransport.Native)] - public void Transport_survives_FromOptions_ToOptions(AbLegacyTransport transport) + [InlineData(null)] + public void Transport_survives_FromOptions_ToOptions(AbLegacyTransport? transport) { var original = new AbLegacyDriverOptions { Transport = transport }; var back = AbLegacyDriverForm.FormModel.FromOptions(original).ToOptions([]); @@ -119,35 +120,52 @@ public sealed class AbLegacyDriverPageFormSerializationTests json.ShouldContain("\"transport\":\"Native\""); } - [Fact] - public void Editing_an_unrelated_field_preserves_a_saved_Native_transport() + [Theory] + [InlineData("Native", AbLegacyTransport.Native)] + [InlineData("Libplctag", AbLegacyTransport.Libplctag)] + public void Editing_an_unrelated_field_preserves_an_explicit_transport(string savedValue, AbLegacyTransport expected) { // The regression the review found: FromOptions/ToOptions dropped Transport, so an operator who set - // Native was silently reverted to libplctag the next time they saved ANY other edit. - const string saved = """{"transport":"Native","timeout":"00:00:02","devices":[]}"""; + // a transport was silently reverted the next time they saved ANY other edit. + var saved = $$"""{"transport":"{{savedValue}}","timeout":"00:00:02","devices":[]}"""; var loaded = AbLegacyDriverForm.TryDeserialize(saved); loaded.ShouldNotBeNull(); - loaded.Transport.ShouldBe(AbLegacyTransport.Native); + loaded.Transport.ShouldBe(expected); var form = AbLegacyDriverForm.FormModel.FromOptions(loaded); form.TimeoutSeconds = 7; var json = AbLegacyDriverForm.Serialize(form.ToOptions(loaded.Devices)); - json.ShouldContain("\"transport\":\"Native\""); + json.ShouldContain($"\"transport\":\"{savedValue}\""); json.ShouldContain("\"timeout\":\"00:00:07\""); } [Fact] - public void A_config_without_transport_loads_as_Libplctag_and_re_emits_it_explicitly() + public void A_config_without_transport_loads_as_Default_and_an_unrelated_edit_keeps_the_key_absent() { - // Chosen behaviour: an absent key round-trips as an explicit "Libplctag" (the form always writes the - // field). The dropdown therefore shows "libplctag (default)". + // Task 22 reverses Task 19's "re-emit an explicit Libplctag": that would pin every edited driver to + // a concrete transport and bypass the driver-side default (and AbLegacy's PLC-5 / LogixPccc + // exclusion). Absent loads as Default (null) and a save that edits something else still omits it. var loaded = AbLegacyDriverForm.TryDeserialize("""{"probeTimeoutSeconds":5}"""); loaded.ShouldNotBeNull(); var form = AbLegacyDriverForm.FormModel.FromOptions(loaded); + form.Transport.ShouldBeNull(); + + form.TimeoutSeconds = 7; + var json = AbLegacyDriverForm.Serialize(form.ToOptions(loaded.Devices)); + json.ShouldNotContain("transport", Case.Insensitive); + json.ShouldContain("\"timeout\":\"00:00:07\""); + } + + [Fact] + public void Choosing_Default_over_an_explicit_transport_drops_the_key() + { + var loaded = AbLegacyDriverForm.TryDeserialize("""{"transport":"Libplctag"}"""); + loaded.ShouldNotBeNull(); + var form = AbLegacyDriverForm.FormModel.FromOptions(loaded); form.Transport.ShouldBe(AbLegacyTransport.Libplctag); - var json = AbLegacyDriverForm.Serialize(form.ToOptions(loaded.Devices)); - json.ShouldContain("\"transport\":\"Libplctag\""); + form.Transport = null; // the "Default (...)" dropdown option + AbLegacyDriverForm.Serialize(form.ToOptions(loaded.Devices)).ShouldNotContain("transport", Case.Insensitive); } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/DriverProbeHandshakeE2eTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/DriverProbeHandshakeE2eTests.cs index 850b6553..7ce24bce 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/DriverProbeHandshakeE2eTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/DriverProbeHandshakeE2eTests.cs @@ -182,7 +182,9 @@ public sealed class DriverProbeHandshakeE2eTests // exercising the one path the comment above says it does not exercise — and reporting it as a // failure of the fixture rather than of the config shape. var result = await new AbCipDriverProbe().ProbeAsync( - $"{{\"Devices\":[{{\"HostAddress\":\"ab://{DockerHost}:{AbCipPort}/1,0\"}}]," + + // Pinned to libplctag (Task 22 flipped the default to Native): the exact messages below were + // proven against ab_server on libplctag; the native path's ab_server coverage is TransportMatrix. + $"{{\"Transport\":\"Libplctag\",\"Devices\":[{{\"HostAddress\":\"ab://{DockerHost}:{AbCipPort}/1,0\"}}]," + $"\"RawTags\":[{{\"RawPath\":\"Plant/AbCip/dev1/TestDINT\"," + $"\"TagConfig\":\"{{\\\"tagPath\\\":\\\"TestDINT\\\",\\\"dataType\\\":\\\"DInt\\\"}}\"," + $"\"WriteIdempotent\":false,\"DeviceName\":\"ab://{DockerHost}:{AbCipPort}/1,0\"}}]}}", @@ -217,7 +219,8 @@ public sealed class DriverProbeHandshakeE2eTests { SkipUnless(DockerHost, AbCipPort); var result = await new AbCipDriverProbe().ProbeAsync( - $"{{\"Devices\":[{{\"HostAddress\":\"ab://{DockerHost}:{AbCipPort}/1,0\"}}],\"RawTags\":[]}}", + // Pinned to libplctag (Task 22): the 0x04 → ErrorNotFound reasoning above is libplctag's. + $"{{\"Transport\":\"Libplctag\",\"Devices\":[{{\"HostAddress\":\"ab://{DockerHost}:{AbCipPort}/1,0\"}}],\"RawTags\":[]}}", Timeout, Ct); result.Ok.ShouldBeTrue($"Probe message: {result.Message}");