From f48f31cfc776d6ccd71fc16648e191d1e5947722 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sun, 26 Apr 2026 04:54:28 -0400 Subject: [PATCH] =?UTF-8?q?Auto:=20focas-f4b=20=E2=80=94=20cnc=5Fwrmacro?= =?UTF-8?q?=20+=20cnc=5Fwrparam=20writes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #269 --- docs/Driver.FOCAS.Cli.md | 41 +++- docs/drivers/FOCAS.md | 60 ++++- docs/v2/focas-deployment.md | 95 +++++++ .../v2/implementation/focas-simulator-plan.md | 93 +++++++ docs/v2/implementation/focas-wire-protocol.md | 80 ++++++ scripts/e2e/test-focas.ps1 | 63 ++++- .../FocasDriver.cs | 68 ++++- .../FocasDriverFactoryExtensions.cs | 18 ++ .../FocasDriverOptions.cs | 29 +++ .../FocasStatusMapper.cs | 12 +- .../FwlibFocasClient.cs | 32 +++ .../IFocasClient.cs | 33 +++ .../FakeFocasClient.cs | 45 ++++ .../FocasWriteMacroTests.cs | 201 +++++++++++++++ .../FocasWriteParameterTests.cs | 232 ++++++++++++++++++ 15 files changed, 1066 insertions(+), 36 deletions(-) create mode 100644 tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasWriteMacroTests.cs create mode 100644 tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasWriteParameterTests.cs diff --git a/docs/Driver.FOCAS.Cli.md b/docs/Driver.FOCAS.Cli.md index 79a9578..13317d0 100644 --- a/docs/Driver.FOCAS.Cli.md +++ b/docs/Driver.FOCAS.Cli.md @@ -110,23 +110,50 @@ Values parse per `--type` with invariant culture. Booleans accept ```powershell otopcua-focas-cli write -h 192.168.1.50 -a R100 -t Int16 -v 42 otopcua-focas-cli write -h 192.168.1.50 -a G50.3 -t Bit -v on -otopcua-focas-cli write -h 192.168.1.50 -a MACRO:500 -t Float64 -v 3.14 + +# MACRO: write — recipe / setpoint surface (server-side WriteOperate ACL) +otopcua-focas-cli write -h 192.168.1.50 -a MACRO:500 -t Int32 -v 42 + +# PARAM: write — commissioning surface (server-side WriteConfigure ACL, +# CNC must be in MDI mode + parameter-write switch enabled, else EW_PASSWD +# surfaces as BadUserAccessDenied) +otopcua-focas-cli write -h 192.168.1.50 -a PARAM:1815 -t Int32 -v 100 ``` PMC G/R writes land on a running machine — be careful which file you hit. Parameter writes may require the CNC to be in MDI mode with the parameter-write switch enabled. +#### Server-enforced ACL — issue #269, plan PR F4-b + +When the same write flows through the OtOpcUa server (rather than the CLI's +direct-to-CNC path), the server-layer ACL gates by tag kind: + +- `PARAM:` writes require **`WriteConfigure`** group membership — heavier + ACL because a misdirected parameter write can put the CNC in a bad + state. +- `MACRO:` writes require **`WriteOperate`** — matches the standard HMI + recipe / setpoint surface. +- PMC R/G/F writes require **`WriteOperate`**. + +The classification is declared by the FOCAS driver per tag and enforced by +`DriverNodeManager`; the driver itself never inspects user identity. See +[`docs/security.md`](security.md) for the full LDAP-group → permission +mapping, [`docs/v2/acl-design.md`](v2/acl-design.md) for the design, and +[`docs/v2/focas-deployment.md`](v2/focas-deployment.md) "Write safety" for +the operator pre-check runbook (MDI mode, parameter-write switch). + **Writes are non-idempotent by default** — a timeout after the CNC already applied the write will NOT auto-retry (plan decisions #44 + #45). -#### Server-side `Writes.Enabled` enforcement (issue #268, plan PR F4-a) +#### Server-side `Writes` enforcement (issue #268 F4-a + #269 F4-b) -The OtOpcUa server gates every FOCAS write behind two independent opt-ins: -`FocasDriverOptions.Writes.Enabled` (driver-level master switch, default -`false`) and `FocasTagDefinition.Writable` (per-tag, default `false`). When -either is off, the server-side `WriteAsync` short-circuits to -`BadNotWritable` before the wire client is touched. See +The OtOpcUa server gates every FOCAS write behind multiple independent +opt-ins: `FocasDriverOptions.Writes.Enabled` (driver-level master switch), +`Writes.AllowParameter` (PARAM kill switch — F4-b), `Writes.AllowMacro` +(MACRO kill switch — F4-b), and `FocasTagDefinition.Writable` (per-tag). +All default `false`; any one off short-circuits the server-side +`WriteAsync` to `BadNotWritable` before the wire client is touched. See [`docs/drivers/FOCAS.md`](drivers/FOCAS.md) "Writes (opt-in, off by default)" subsection + [`docs/v2/decisions.md`](v2/decisions.md) for the decision record. diff --git a/docs/drivers/FOCAS.md b/docs/drivers/FOCAS.md index dc0bd95..97b3a0f 100644 --- a/docs/drivers/FOCAS.md +++ b/docs/drivers/FOCAS.md @@ -54,9 +54,9 @@ giant request. Typical FANUC ring buffers cap at ~100 entries; the default surfacing the FWLIB struct fields directly into `FocasAlarmHistoryEntry`. -## Writes (opt-in, off by default) — issue #268, plan PR F4-a +## Writes (opt-in, off by default) — issue #268 (F4-a) + #269 (F4-b) -Writes ship behind two independent opt-ins. Both default off so a freshly +Writes ship behind multiple independent opt-ins. All default off so a freshly deployed FOCAS driver is read-only until the deployment makes a deliberate choice. Decision record: [`docs/v2/decisions.md`](../v2/decisions.md) → "FOCAS write-path opt-in". @@ -64,20 +64,49 @@ choice. Decision record: [`docs/v2/decisions.md`](../v2/decisions.md) → | Knob | Default | Effect when off | | --- | --- | --- | | `FocasDriverOptions.Writes.Enabled` *(driver-level master switch)* | `false` | Every entry in a `WriteAsync` batch short-circuits to `BadNotWritable` with status text `writes disabled at driver level`. Wire client never gets touched. | -| `FocasTagDefinition.Writable` *(per-tag opt-in)* | `false` | The per-tag check returns `BadNotWritable` for that tag even when the driver-level flag is on. | +| **`FocasDriverOptions.Writes.AllowParameter`** *(F4-b granular kill switch)* | **`false`** | **`PARAM:` writes return `BadNotWritable` with no wire client constructed. Defense in depth — even if `Enabled = true` an operator must explicitly opt into parameter writes per kind because a misdirected `cnc_wrparam` can put the CNC in a bad state.** | +| **`FocasDriverOptions.Writes.AllowMacro`** *(F4-b granular kill switch)* | **`false`** | **`MACRO:` writes return `BadNotWritable` with no wire client constructed. Macro writes are the normal HMI-driven recipe / setpoint surface; gating them separately from `AllowParameter` lets a deployment open MACRO without exposing the heavier PARAM write surface.** | +| `FocasTagDefinition.Writable` *(per-tag opt-in)* | `false` | The per-tag check returns `BadNotWritable` for that tag even when the driver-level flags are on. | -### Config shape +### Config shape — F4-b ```jsonc { - "Writes": { "Enabled": true }, + "Writes": { + "Enabled": true, + "AllowParameter": true, // F4-b — opt into cnc_wrparam + "AllowMacro": true // F4-b — opt into cnc_wrmacro + }, "Tags": [ { "Name": "RPM", "Address": "PARAM:1815", "DataType": "Int32", + "Writable": true, "WriteIdempotent": false }, + { "Name": "Recipe", "Address": "MACRO:500", "DataType": "Int32", "Writable": true, "WriteIdempotent": false } ] } ``` +### Server-layer ACL (LDAP groups) + +Per the [`docs/v2/acl-design.md`](../v2/acl-design.md) tier model, the FOCAS +driver only declares per-tag `SecurityClassification`; `DriverNodeManager` +applies the gate. The classification post-F4-b is: + +| Tag kind | Classification | LDAP group required (default mapping) | +| --- | --- | --- | +| `PARAM:N` writable | `Configure` | **`WriteConfigure`** | +| `MACRO:N` writable | `Operate` | `WriteOperate` | +| Other writable (PMC R/G/F/...) | `Operate` | `WriteOperate` | +| Non-writable | `ViewOnly` | (no write permission) | + +Parameter writes need the heavier `WriteConfigure` group because they're +mostly emergency commissioning territory; macro writes use `WriteOperate` +because they're the normal HMI recipe surface. The driver-level +`AllowParameter` / `AllowMacro` kill switches sit independently of ACL — an +operator-team kill switch the deployment can flip without redeploying ACL +group memberships. See [`docs/security.md`](../security.md) for the full +group/permission map. + `WriteIdempotent` is plumbed through Polly retry by the server-layer `CapabilityInvoker.ExecuteWriteAsync`. When `false` (default), failed writes are NOT auto-retried per plan decisions #44/#45 — a timeout that fires after @@ -86,14 +115,23 @@ non-idempotent action (alarm acks, M-code pulses, recipe steps). Flip `WriteIdempotent` on per tag for genuinely-idempotent writes (a parameter value that the operator simply wants forced to a target). -### Status-code semantics post-F4-a +### Status-code semantics post-F4-b -- `BadNotWritable` — driver-level `Writes.Enabled = false`, OR per-tag - `Writable = false`. Two distinct paths, same status code. +- `BadNotWritable` — one of: driver-level `Writes.Enabled = false`; per-tag + `Writable = false`; **`Writes.AllowParameter = false` for a `PARAM:` tag + (F4-b)**; **`Writes.AllowMacro = false` for a `MACRO:` tag (F4-b)**. Same + status code, four distinct paths — operators distinguish by checking the + knobs. +- `BadUserAccessDenied` — **F4-b** — the CNC reported `EW_PASSWD` + (parameter-write switch off / unlock required). F4-d will land the + unlock workflow on top of this surface; today the deployment instructs + the operator to flip the parameter-write switch on the CNC pendant. - `BadNotSupported` — both opt-ins flipped on, but the wire client doesn't - yet implement the kind being written. F4-a wires the dispatch surface; - F4-b/c land the actual macro / parameter / PMC writes for unimplemented - kinds, replacing those `BadNotSupported` responses with real wire calls. + implement the kind being written (e.g. older transport variant). F4-a + wired the generic dispatch; F4-b adds typed `WriteParameterAsync` / + `WriteMacroAsync` entry points whose default impls return + `BadNotSupported` so transports compiled against a stale `IFocasClient` + surface still build. - `BadNodeIdUnknown` — full-reference doesn't match any configured `FocasTagDefinition.Name`. - `BadCommunicationError` — wire failure (DLL not loaded, IPC peer dead, diff --git a/docs/v2/focas-deployment.md b/docs/v2/focas-deployment.md index 217e33b..20de016 100644 --- a/docs/v2/focas-deployment.md +++ b/docs/v2/focas-deployment.md @@ -43,3 +43,98 @@ The history projection emits each unseen entry through reported wall-clock — keep CNC clocks on UTC so the dedup key `(OccurrenceTime, AlarmNumber, AlarmType)` stays stable across DST transitions. + +## Write safety — issue #269, plan PR F4-b + +The FOCAS driver supports `cnc_wrparam` and `cnc_wrmacro` writes behind +multiple independent opt-ins. A misdirected parameter write can put the +CNC in a bad state, so the runbook below MUST be followed before flipping +the granular kill switches on. + +### Operator pre-checks (every deployment, every change) + +1. **CNC must be in MDI mode.** Most parameter writes fail with `EW_PASSWD` + (surfaces as `BadUserAccessDenied`) unless the CNC is in MDI. The + server-side write returns immediately with the access-denied status; no + value reaches the wire. +2. **Parameter-write switch enabled on the CNC pendant.** Even in MDI mode + protected parameters require the operator to physically enable the + parameter-write switch. Without it `cnc_wrparam` returns `EW_PASSWD`. + Plan PR F4-d will land an OPC UA-side unlock workflow; today the only + path is the pendant. +3. **Verify each tag's address against the FANUC manual.** Ranges vary per + CNC series; the + [`focas-version-matrix`](./focas-version-matrix.md) capability matrix + rejects out-of-range numbers at startup, but address-vs-meaning is the + operator's job. +4. **Dry run with `Writable = true` but `Writes.AllowParameter = false`.** + Staged opt-in catches mis-mapped tags: every PARAM write returns + `BadNotWritable` until you flip the granular flag, so you can confirm + the tag list before any wire write fires. + +### LDAP group requirements + +Per [`docs/security.md`](../security.md) the server-layer ACL maps +`SecurityClassification` to LDAP groups. Post-F4-b: + +| Tag kind | LDAP group required | +| --- | --- | +| `PARAM:N` (writable) | **`WriteConfigure`** — heaviest write tier; matches commissioning roles | +| `MACRO:N` (writable) | `WriteOperate` — standard HMI recipe / setpoint group | +| PMC R/G/F (writable) | `WriteOperate` | +| Read-only | `ReadOnly` | + +Per the `feedback_acl_at_server_layer` design note, the FOCAS driver +declares the classification but does NOT enforce it; `DriverNodeManager` +applies the gate before the driver's `WriteAsync` ever runs. A user +without `WriteConfigure` who attempts a `PARAM:` write gets +`BadUserAccessDenied` from the server with no driver-level audit entry — +the OPC UA layer's audit log catches it. + +### Audit-log expectations + +Every successful write produces: + +- An OPC UA AuditWriteEvent (server layer — see + [`docs/security.md`](../security.md) "Audit logging"). +- A FOCAS driver-level Serilog entry tagged `Driver=FOCAS DriverInstanceId=... + TagName=... Address=... ResultStatus=...`. +- A `Writes/LastWriteAt` and `Writes/LastWriteStatus` diagnostic counter + refresh on the device's `Diagnostics/` fixed-tree node (planned; + populated as F4-c lands). + +Failures to write (`BadUserAccessDenied`, `BadCommunicationError`, etc.) +produce the same audit entries with the failure status code so a +post-incident reviewer sees the same shape regardless of whether the write +succeeded. + +### Granular config example + +```jsonc +{ + "Drivers": { + "FOCAS": { + "Devices": [ + { "HostAddress": "focas://10.0.0.5:8193", "Series": "Series30i" } + ], + "Writes": { + "Enabled": true, + "AllowMacro": true, // recipe / setpoint writes — operator role + "AllowParameter": false // commissioning only — keep locked except during planned work + }, + "Tags": [ + { "Name": "Recipe.PartCount", "DeviceHostAddress": "focas://10.0.0.5:8193", + "Address": "MACRO:500", "DataType": "Int32", + "Writable": true, "WriteIdempotent": true }, + { "Name": "MaxFeedrate", "DeviceHostAddress": "focas://10.0.0.5:8193", + "Address": "PARAM:1815", "DataType": "Int32", + "Writable": false /* keep read-only until commissioning window */ } + ] + } + } +} +``` + +Flipping `AllowParameter` on for the commissioning window (and back off +afterward) is the recommended deployment cadence — the granular kill +switch is a lightweight runtime toggle, not a config-DB redeploy. diff --git a/docs/v2/implementation/focas-simulator-plan.md b/docs/v2/implementation/focas-simulator-plan.md index 6d7d324..7d40acc 100644 --- a/docs/v2/implementation/focas-simulator-plan.md +++ b/docs/v2/implementation/focas-simulator-plan.md @@ -29,6 +29,8 @@ command ids (and their request/response payloads) don't drift between the | `0x0010` | `pmc_rdpmcrng` | reads PMC byte ranges | | `0x0020` | `cnc_modal` | reads cached modal MSTB per profile | | ... | ... | ... | +| **`0x0102`** | **`cnc_wrparam`** | **mutates per-profile parameter map; returns `EW_PASSWD` (`11`) when the profile's `unlock_state` is off (sets up F4-d's unlock workflow) — issue #269, plan PR F4-b** | +| **`0x0103`** | **`cnc_wrmacro`** | **mutates per-profile macro map; integer-only writes for now (decimalPointCount=0) — issue #269, plan PR F4-b** | | **`0x0F1A`** | **`cnc_rdalmhistry`** | **dumps the per-profile alarm-history ring buffer (issue #267, plan PR F3-a)** | ## `cnc_rdalmhistry` mock behaviour @@ -100,3 +102,94 @@ Integration test `Series/AlarmHistoryProjectionTests.cs` will assert: These tests are blocked on the focas-mock + integration-test project landing; the unit-test coverage in `FocasAlarmProjectionTests` already exercises every same-process invariant. + +## `cnc_wrparam` / `cnc_wrmacro` mock behaviour — issue #269, plan PR F4-b + +When the focas-mock fixture lands, it MUST implement the contract below. +The .NET side already ships against this contract (`FwlibFocasClient.cs` +write helpers, `FakeFocasClient` round-trip support); writing the simulator +to the same shape lets the existing integration-test scaffolds at +`tests/.../IntegrationTests/Series/ParameterWriteTests.cs` and +`MacroWriteTests.cs` (when they materialise) light up without driver +changes. + +### Per-profile state + +Each profile owns: + +- `parameters: Dict[int, int]` — map from parameter number to current value. +- `macros: Dict[int, int]` — map from macro number to current scaled-int + value (decimal-point count fixed at 0 for F4-b). +- `unlock_state: bool` — defaults `False`. When `False`, every + `cnc_wrparam` returns `EW_PASSWD` (numeric `11`) regardless of + parameter. Macro writes are NOT gated by `unlock_state`. +- `last_write: Optional[LastWrite]` — most-recent successful + `(kind, number, value, ts)` tuple, surfaced via the admin endpoint + below for audit-log assertions. + +### `cnc_wrparam` request decode + +``` +[int16 LE datano][int16 LE axis][int8|int16|int32 LE value] +``` + +Width of the value field is determined by the request frame trailer +length per the table in +[`focas-wire-protocol.md`](./focas-wire-protocol.md). On +`unlock_state == False` short-circuit to `[int16 LE 11]` (`EW_PASSWD`). +Otherwise mutate `parameters[datano] = value`, set `last_write`, return +`[int16 LE 0]`. + +### `cnc_wrmacro` request decode + +``` +[int16 LE number][int16 LE length=8][int32 LE mcr_val][int16 LE dec_val] +``` + +Always accept (no `unlock_state` gate). Mutate +`macros[number] = mcr_val` (we ignore `dec_val` for F4-b — integer-only). +Return `[int16 LE 0]`. Round-trip: a subsequent `cnc_rdmacro(number)` +returns `(mcr_val, 0)`. + +### Admin endpoint — `POST /admin/mock_set_unlock_state` + +Toggles `unlock_state` for the F4-d unlock workflow tests. Without this, +F4-b parameter-write integration tests can't reproduce the +`EW_PASSWD` → `BadUserAccessDenied` mapping. + +``` +POST /admin/mock_set_unlock_state +{ "profile": "Series30i", "unlocked": true } +``` + +### Admin endpoint — `GET /admin/mock_get_last_write` + +Returns the simulator's view of the most-recent successful write, used by +F4-b audit-log integration assertions ("did the write actually reach the +fixture, and is the audit log capturing the right kind/number/value?"). + +``` +GET /admin/mock_get_last_write?profile=Series30i +-> +{ + "kind": "param", // "param" | "macro" + "number": 1815, + "value": 100, + "writtenAt": "2026-04-25T13:30:00Z" +} +``` + +When no write has happened the endpoint returns `null` rather than 404 so +the test helper can assert "no writes since fixture reset" without +exception handling. + +### Status + +focas-mock simulator has not landed yet (tracked separately from F4-b). +F4-b lands the .NET-side wire encoders + dispatch + status mapping +unconditionally; the integration-test scaffolds at +`tests/.../IntegrationTests/Series/ParameterWriteTests.cs` and +`MacroWriteTests.cs` are deferred until the simulator + integration-test +project land. Until then unit-test coverage in +`FocasWriteParameterTests` / `FocasWriteMacroTests` exercises every +same-process invariant against the in-memory `FakeFocasClient`. diff --git a/docs/v2/implementation/focas-wire-protocol.md b/docs/v2/implementation/focas-wire-protocol.md index 2451142..3f6ee11 100644 --- a/docs/v2/implementation/focas-wire-protocol.md +++ b/docs/v2/implementation/focas-wire-protocol.md @@ -19,6 +19,8 @@ Each FOCAS-equivalent call gets a stable wire-protocol command id. Ids are | `0x0003` | `cnc_rdmacro` | macro variable value | | `0x0004` | `cnc_rddiag` | diagnostic value | | ... | ... | ... | +| **`0x0102`** | **`cnc_wrparam`** | **IODBPSD parameter-write packet (issue #269, plan PR F4-b)** | +| **`0x0103`** | **`cnc_wrmacro`** | **ODBM macro-write packet (issue #269, plan PR F4-b)** | | `0x0F1A` | **`cnc_rdalmhistry`** | **ODBALMHIS alarm-history ring-buffer dump (issue #267, plan PR F3-a)** | ## ODBALMHIS — alarm history (`cnc_rdalmhistry`, command `0x0F1A`) @@ -74,3 +76,81 @@ DST transitions. The .NET decoder unstable anyway. - `msg_len` overrunning the payload truncates the entry list at the malformed entry rather than throwing. + +## IODBPSD — parameter write (`cnc_wrparam`, command `0x0102`) + +Issue #269, plan PR F4-b. The write-side payload is the **byte-symmetric +inverse of the `cnc_rdparam` read** — the same `IODBPSD` struct shape, and +the .NET wire client uses the read-side decoder reversed (`EncodeParamValue` +in `FwlibFocasClient.cs`) so the encoder/decoder are guaranteed to stay in +lock-step. + +### Request + +| Offset | Width | Field | +| --- | --- | --- | +| 0 | `int16 LE` | `datano` — parameter number (e.g. `1815`) | +| 2 | `int16 LE` | `type` — axis index (1-based; `0` = whole-CNC parameter) | +| 4 | `length` | `data` payload — width depends on parameter type | + +`length` (request frame trailer, drives `data` width): + +| FocasDataType | `length` | Payload encoding | +| --- | --- | --- | +| `Byte` | `4 + 1` | one signed byte at offset 4 | +| `Int16` | `4 + 2` | int16 LE at offset 4 | +| `Int32` | `4 + 4` | int32 LE at offset 4 | + +Bit-addressed parameters (`PARAM:1815/0` form) are not supported by F4-b +and surface as `BadNotSupported`; F4-c will land the read-modify-write +helper alongside the PMC bit RMW path. + +### Response + +Single `int16 LE` return code per the standard FWLIB convention: + +- `0` → `Good` +- `11` (`EW_PASSWD`) → **`BadUserAccessDenied`** (was `BadNotWritable` + pre-F4-b — see `FocasStatusMapper`). Means the parameter-write switch is + off or the CNC isn't in MDI mode; the F4-d unlock workflow will close + the loop on this from the OPC UA side. +- Other `EW_*` codes map per + [`FocasStatusMapper.MapFocasReturn`](../../src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasStatusMapper.cs). + +## ODBM — macro write (`cnc_wrmacro`, command `0x0103`) + +Issue #269, plan PR F4-b. The write-side payload mirrors the +`cnc_rdmacro` read shape: the same `(mcr_val, dec_val)` (integer + +decimal-point count) split, but emitted from the .NET side rather than +decoded. + +### Request + +| Offset | Width | Field | +| --- | --- | --- | +| 0 | `int16 LE` | `number` — macro variable number (e.g. `500`) | +| 2 | `int16 LE` | `length` — fixed at `8` for ODBM | +| 4 | `int32 LE` | `mcr_val` — scaled integer value | +| 8 | `int16 LE` | `dec_val` — decimal-point count | + +F4-b ships **integer-only writes** (`dec_val = 0`) to match the most +common HMI pattern; a future `WriteMacroScaled` overload will land if the +field calls for fractional macro setpoints. Read-side decoders apply +`mcr_val / 10^dec_val`, so a `dec_val = 0` write surfaces back as the +integer it was emitted as. + +### Response + +Same single-int16 envelope as `cnc_wrparam`. `EW_PASSWD` is rare on macro +writes (the gate-switch protection is parameter-specific) but the mapper +treats both kinds identically. + +### Symmetry note + +The plan carries a "byte layout symmetry" requirement — the encoder for +each kind is the read-side decoder reversed. Adding a new parameter type +(e.g. `Int64` parameters, when they ship) means extending both sides in +the same PR; the unit test +`FocasWriteParameterTests.ParameterWrite_round_trip_stores_value_visible_to_subsequent_read` +exercises encode → store → decode with the fake wire client and is the +canary for symmetry regressions. diff --git a/scripts/e2e/test-focas.ps1 b/scripts/e2e/test-focas.ps1 index 13465f1..a6ba777 100644 --- a/scripts/e2e/test-focas.ps1 +++ b/scripts/e2e/test-focas.ps1 @@ -28,12 +28,25 @@ NodeId at which the server publishes the Address. .PARAMETER Write - Issue #268 / plan PR F4-a — opts the script into the post-F4-a write - stages. F4-a ships the write infrastructure (driver-level Writes.Enabled - flag + per-tag Writable opt-in) without any actual wire writes; F4-b/c - populate this stage with real macro / parameter / PMC write coverage. - Until then the switch is a no-op marker so the e2e harness records that - the write surface was deliberately exercised (or skipped). + Issue #268 (F4-a) + #269 (F4-b) — opts the script into write stages. + Without -Write the script runs read-only probe / loopback / bridge + coverage. With -Write the script additionally exercises the F4-b + cnc_wrparam + cnc_wrmacro round-trip stages against the configured + -ParamAddress / -MacroAddress (default safe values). The wire writes + fire only when FOCAS_TRUST_WIRE=1 (already gated above) AND the + operator explicitly requests the write path. + +.PARAMETER ParamAddress + Parameter address for the F4-b write stage (default PARAM:1815). + Only used when -Write is supplied. Pick a parameter that's safe to + scribble on for your CNC setup — the default is benign for a stock + Fanuc 30i but every site differs. + +.PARAMETER MacroAddress + Macro variable for the F4-b write stage (default MACRO:500). Macro + writes are the lowest-risk write surface (no parameter-write switch + needed, no MDI mode required) so this stage runs whenever -Write is + supplied. #> param( @@ -42,7 +55,9 @@ param( [string]$Address = "R100", [string]$OpcUaUrl = "opc.tcp://localhost:4840", [Parameter(Mandatory)] [string]$BridgeNodeId, - [switch]$Write + [switch]$Write, + [string]$ParamAddress = "PARAM:1815", + [string]$MacroAddress = "MACRO:500" ) $ErrorActionPreference = "Stop" @@ -102,11 +117,35 @@ $results += Test-SubscribeSeesChange ` -ExpectedValue "$subValue" if ($Write) { - # F4-a no-op stage. Real per-kind write coverage lands in F4-b/c which extend the wire - # client past the BadNotSupported short-circuit + populate this branch with - # macro / parameter / PMC write assertions. Logged here so the harness records that the - # operator deliberately requested the write path. - Write-Host "[skip] -Write requested; F4-a ships infrastructure only — wire-write stages land in F4-b/c (issue #268)." + # F4-b — macro + parameter round-trip writes. Both stages use the same + # write-then-read shape the existing PMC stages use; the per-tag value + # comes back through Test-DriverLoopback's read step. + # + # Macro writes run unconditionally when -Write is supplied — no MDI / no + # parameter-write switch dependency, lowest-risk write surface on a CNC. + $macroValue = Get-Random -Minimum 100 -Maximum 9999 + $results += Test-DriverLoopback ` + -Cli $focasCli ` + -WriteArgs (@("write") + $commonFocas + @("-a", $MacroAddress, "-t", "Int32", "-v", $macroValue)) ` + -ReadArgs (@("read") + $commonFocas + @("-a", $MacroAddress, "-t", "Int32")) ` + -ExpectedValue "$macroValue" + + # Parameter writes only fire when the operator double-opts in via + # FOCAS_PARAM_WRITE=1. The CNC must be in MDI mode + parameter-write + # switch enabled or every write returns EW_PASSWD (BadUserAccessDenied); + # without an opt-in the script won't even attempt the write. F4-d will + # land an OPC UA-side unlock workflow that lets this stage run without + # the pendant. + if ($env:FOCAS_PARAM_WRITE -eq "1" -or $env:FOCAS_PARAM_WRITE -eq "true") { + $paramValue = Get-Random -Minimum 100 -Maximum 9999 + $results += Test-DriverLoopback ` + -Cli $focasCli ` + -WriteArgs (@("write") + $commonFocas + @("-a", $ParamAddress, "-t", "Int32", "-v", $paramValue)) ` + -ReadArgs (@("read") + $commonFocas + @("-a", $ParamAddress, "-t", "Int32")) ` + -ExpectedValue "$paramValue" + } else { + Write-Host "[skip] FOCAS_PARAM_WRITE not set — parameter-write stage requires the CNC to be in MDI mode + parameter-write switch enabled (see docs/v2/focas-deployment.md 'Write safety')." + } } Write-Summary -Title "FOCAS e2e" -Results $results diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs index 3f2b9d9..e800b36 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs @@ -515,9 +515,33 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, try { - var client = await EnsureConnectedAsync(device, cancellationToken).ConfigureAwait(false); var parsed = FocasAddress.TryParse(def.Address) ?? throw new InvalidOperationException($"FOCAS tag '{def.Name}' has malformed Address '{def.Address}'."); + + // PR F4-b (issue #269) — granular per-kind gates on top of Writes.Enabled + // and per-tag Writable. PARAM: tags require Writes.AllowParameter, + // MACRO: tags require Writes.AllowMacro. Both default false so a + // deployment that flips the master switch on without explicitly opting + // into a kind still gets BadNotWritable for that kind. Fires BEFORE + // EnsureConnectedAsync so a kind whose gate is closed doesn't even + // attempt to construct a wire client (mirrors the F4-a master-switch + // short-circuit). ACL note: PARAM tags surface + // SecurityClassification.Configure (server-layer requires + // WriteConfigure) and MACRO tags surface Operate (WriteOperate) — see + // DiscoverAsync. Driver-level gates and ACL are independent: this gate + // is a deployment-side kill switch, ACL is the per-session gate. + if (parsed.Kind == FocasAreaKind.Parameter && !_options.Writes.AllowParameter) + { + results[i] = new WriteResult(FocasStatusMapper.BadNotWritable); + continue; + } + if (parsed.Kind == FocasAreaKind.Macro && !_options.Writes.AllowMacro) + { + results[i] = new WriteResult(FocasStatusMapper.BadNotWritable); + continue; + } + + var client = await EnsureConnectedAsync(device, cancellationToken).ConfigureAwait(false); if (parsed.PathId > 1 && device.PathCount > 0 && parsed.PathId > device.PathCount) { results[i] = new WriteResult(FocasStatusMapper.BadOutOfRange); @@ -528,7 +552,19 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, await client.SetPathAsync(parsed.PathId, cancellationToken).ConfigureAwait(false); device.LastSetPath = parsed.PathId; } - var status = await client.WriteAsync(parsed, def.DataType, w.Value, cancellationToken).ConfigureAwait(false); + + // Dispatch through the typed entry points for PARAM/MACRO so the + // wire-client surface mirrors the per-kind opt-in shape; PMC and other + // kinds fall back to the generic WriteAsync path. + var status = parsed.Kind switch + { + FocasAreaKind.Parameter => await client.WriteParameterAsync( + parsed, def.DataType, w.Value, cancellationToken).ConfigureAwait(false), + FocasAreaKind.Macro => await client.WriteMacroAsync( + parsed, w.Value, cancellationToken).ConfigureAwait(false), + _ => await client.WriteAsync( + parsed, def.DataType, w.Value, cancellationToken).ConfigureAwait(false), + }; results[i] = new WriteResult(status); } catch (OperationCanceledException) { throw; } @@ -574,9 +610,7 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, DriverDataType: tag.DataType.ToDriverDataType(), IsArray: false, ArrayDim: null, - SecurityClass: tag.Writable - ? SecurityClassification.Operate - : SecurityClassification.ViewOnly, + SecurityClass: ClassifyTag(tag), IsHistorized: false, IsAlarm: false, WriteIdempotent: tag.WriteIdempotent)); @@ -765,6 +799,30 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, _ => DriverDataType.String, }; + /// + /// Plan PR F4-b (issue #269) — declare the per-tag write classification the + /// server-layer ACL gate (DriverNodeManager) consumes. Per the + /// feedback_acl_at_server_layer memory the driver only declares metadata; + /// enforcement happens at the server layer, not here. + /// + /// PARAM: tags → (server-layer requires WriteConfigure) + /// MACRO: tags → (server-layer requires WriteOperate) + /// Other writable tags (PMC) → + /// Non-writable tags → + /// + /// + internal static SecurityClassification ClassifyTag(FocasTagDefinition tag) + { + if (!tag.Writable) return SecurityClassification.ViewOnly; + var parsed = FocasAddress.TryParse(tag.Address); + return parsed?.Kind switch + { + FocasAreaKind.Parameter => SecurityClassification.Configure, + FocasAreaKind.Macro => SecurityClassification.Operate, + _ => SecurityClassification.Operate, + }; + } + private static string StatusReferenceFor(string hostAddress, string field) => $"{hostAddress}::Status/{field}"; diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriverFactoryExtensions.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriverFactoryExtensions.cs index 77a7084..417d37a 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriverFactoryExtensions.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriverFactoryExtensions.cs @@ -92,6 +92,12 @@ public static class FocasDriverFactoryExtensions Writes = new FocasWritesOptions { Enabled = dto.Writes?.Enabled ?? false, + // Plan PR F4-b (issue #269) — granular kill-switches on top of Enabled. + // Default false: even with Enabled=true the operator must explicitly opt + // into parameter and macro writes per kind. A bare Writes section with + // just { Enabled: true } keeps PARAM/MACRO writes locked. + AllowParameter = dto.Writes?.AllowParameter ?? false, + AllowMacro = dto.Writes?.AllowMacro ?? false, }, }; @@ -186,6 +192,18 @@ public static class FocasDriverFactoryExtensions internal sealed class FocasWritesDto { public bool? Enabled { get; init; } + + /// + /// Plan PR F4-b (issue #269). Default false — see + /// . + /// + public bool? AllowParameter { get; init; } + + /// + /// Plan PR F4-b (issue #269). Default false — see + /// . + /// + public bool? AllowMacro { get; init; } } internal sealed class FocasDeviceDto diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriverOptions.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriverOptions.cs index f50a495..8c678a7 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriverOptions.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriverOptions.cs @@ -54,6 +54,35 @@ public sealed record FocasWritesOptions /// "writes disabled at driver level". /// public bool Enabled { get; init; } = false; + + /// + /// Issue #269, plan PR F4-b — granular kill-switch for cnc_wrparam + /// parameter writes (defense in depth on top of and the + /// per-tag ). Default false: an + /// operator who flips on without explicitly opting into + /// parameter writes still gets + /// for every PARAM: tag. A misdirected parameter write can put the CNC + /// in a bad state, so the third opt-in keeps the blast radius bounded. + /// Server-layer ACL: PARAM: tags additionally surface a + /// classification + /// so the OPC UA gate requires WriteConfigure group membership; this + /// flag is the driver-level kill switch the operator team can flip without a + /// redeploy. + /// + public bool AllowParameter { get; init; } = false; + + /// + /// Issue #269, plan PR F4-b — granular kill-switch for cnc_wrmacro macro + /// variable writes (defense in depth on top of and the + /// per-tag ). Default false: + /// macro writes are gated separately from parameter writes because they're a + /// normal HMI-driven recipe / setpoint surface where parameter writes are + /// mostly emergency commissioning territory. + /// Server-layer ACL: MACRO: tags surface + /// so the OPC UA + /// gate requires WriteOperate group membership. + /// + public bool AllowMacro { get; init; } = false; } /// diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasStatusMapper.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasStatusMapper.cs index 565e900..411cf91 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasStatusMapper.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasStatusMapper.cs @@ -19,6 +19,16 @@ public static class FocasStatusMapper public const uint BadTimeout = 0x800A0000u; public const uint BadTypeMismatch = 0x80730000u; + /// + /// OPC UA BadUserAccessDenied. Surfaced when the CNC reports + /// EW_PASSWD (parameter-write switch off, MDI mode required, etc.) — the + /// deployment must escalate the operator's session to satisfy the write gate. + /// Plan PR F4-d will land the unlock workflow that lets operators flip the gate + /// from the OPC UA side; F4-b just maps the status code so clients can branch + /// on it. (Plan PR F4-b, issue #269.) + /// + public const uint BadUserAccessDenied = 0x801F0000u; + /// /// Map common FWLIB EW_* return codes. The values below match Fanuc's published /// numeric conventions (EW_OK=0, EW_FUNC=1, EW_NUMBER=3, EW_LENGTH=4, EW_ATTRIB=7, @@ -37,7 +47,7 @@ public static class FocasStatusMapper 7 => BadTypeMismatch, // EW_ATTRIB 8 => BadNodeIdUnknown, // EW_DATA — invalid data address 9 => BadCommunicationError, // EW_PARITY - 11 => BadNotWritable, // EW_PASSWD + 11 => BadUserAccessDenied, // EW_PASSWD — parameter-write switch off / unlock required (F4-d) -1 => BadDeviceFailure, // EW_BUSY -8 => BadInternalError, // EW_HANDLE — CNC handle not available -9 => BadNotSupported, // EW_VERSION — FWLIB vs CNC version mismatch diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FwlibFocasClient.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FwlibFocasClient.cs index 5a99cad..567f8c7 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FwlibFocasClient.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FwlibFocasClient.cs @@ -84,12 +84,44 @@ internal sealed class FwlibFocasClient : IFocasClient FocasAreaKind.Pmc when type == FocasDataType.Bit && address.BitIndex is int => await WritePmcBitAsync(address, Convert.ToBoolean(value), cancellationToken).ConfigureAwait(false), FocasAreaKind.Pmc => WritePmc(address, type, value), + // PR F4-b (issue #269) — route through the typed WriteParameterAsync / + // WriteMacroAsync entry points so the driver-level dispatch can apply the + // granular Writes.AllowParameter / Writes.AllowMacro gates without re-parsing + // the address kind. FocasAreaKind.Parameter => WriteParameter(address, type, value), FocasAreaKind.Macro => WriteMacro(address, value), _ => FocasStatusMapper.BadNotSupported, }; } + /// + /// Plan PR F4-b (issue #269) — typed parameter-write entry point. Backed by the + /// same helper as the kind-dispatched + /// path, so unit tests that go through the typed entry + /// point exercise the same wire encoding the production read/write loop hits. + /// + public Task WriteParameterAsync( + FocasAddress address, FocasDataType type, object? value, CancellationToken cancellationToken) + { + if (!_connected) return Task.FromResult(FocasStatusMapper.BadCommunicationError); + cancellationToken.ThrowIfCancellationRequested(); + return Task.FromResult(WriteParameter(address, type, value)); + } + + /// + /// Plan PR F4-b (issue #269) — typed macro-write entry point. Today + /// writes integer-only with + /// decimalPointCount = 0; a follow-up WriteMacroScaled overload + /// can land if fractional macro setpoints become a field requirement. + /// + public Task WriteMacroAsync( + FocasAddress address, object? value, CancellationToken cancellationToken) + { + if (!_connected) return Task.FromResult(FocasStatusMapper.BadCommunicationError); + cancellationToken.ThrowIfCancellationRequested(); + return Task.FromResult(WriteMacro(address, value)); + } + /// /// Read-modify-write one bit within a PMC byte. Acquires a per-byte semaphore so /// concurrent bit writes against the same byte serialise and neither loses its update. diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/IFocasClient.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/IFocasClient.cs index 4634c8a..33eb19a 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/IFocasClient.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/IFocasClient.cs @@ -43,6 +43,39 @@ public interface IFocasClient : IDisposable object? value, CancellationToken cancellationToken); + /// + /// Write a CNC parameter value via cnc_wrparam (FWLIB IODBPSD packet — + /// byte layout symmetric with the cnc_rdparam read side). Plan PR F4-b + /// (issue #269). The is parsed from a PARAM:N + /// tag string; drives the payload width (Byte / Int16 / + /// Int32). Default impl returns + /// so transports that haven't yet routed the write keep compiling. + /// EW_PASSWD from the CNC (parameter-write switch off / unlock required) + /// surfaces as ; F4-d will + /// wire the unlock workflow on top. + /// + Task WriteParameterAsync( + FocasAddress address, + FocasDataType type, + object? value, + CancellationToken cancellationToken) + => Task.FromResult(FocasStatusMapper.BadNotSupported); + + /// + /// Write a CNC macro variable value via cnc_wrmacro (FWLIB ODBM packet + /// symmetric with the cnc_rdmacro read side). Plan PR F4-b (issue #269). + /// The implementation encodes as (intValue, + /// decimalPointCount); today we ship integer-only (decimalPointCount = 0) + /// to match the most common HMI pattern, and a future WriteMacroScaled + /// overload can land if the field calls for fractional macro setpoints. + /// Default impl returns . + /// + Task WriteMacroAsync( + FocasAddress address, + object? value, + CancellationToken cancellationToken) + => Task.FromResult(FocasStatusMapper.BadNotSupported); + /// /// Cheap health probe — e.g. cnc_rdcncstat. Returns true when the CNC /// responds with any valid status. diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FakeFocasClient.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FakeFocasClient.cs index 7ff1622..6469abe 100644 --- a/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FakeFocasClient.cs +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FakeFocasClient.cs @@ -18,6 +18,20 @@ internal class FakeFocasClient : IFocasClient public Dictionary WriteStatuses { get; } = new(StringComparer.OrdinalIgnoreCase); public List<(FocasAddress addr, FocasDataType type, object? value)> WriteLog { get; } = new(); + /// + /// Plan PR F4-b (issue #269) — separate log of cnc_wrparam-shaped calls + /// observed via . Tests assert this list to + /// verify the driver routed PARAM writes through the typed entry point rather + /// than the generic dispatch. + /// + public List<(FocasAddress addr, FocasDataType type, object? value)> ParameterWriteLog { get; } = new(); + + /// + /// Plan PR F4-b (issue #269) — separate log of cnc_wrmacro-shaped calls + /// observed via . + /// + public List<(FocasAddress addr, object? value)> MacroWriteLog { get; } = new(); + public virtual Task ConnectAsync(FocasHostAddress address, TimeSpan timeout, CancellationToken ct) { ConnectCount++; @@ -46,6 +60,37 @@ internal class FakeFocasClient : IFocasClient return Task.FromResult(status); } + /// + /// Plan PR F4-b (issue #269) — typed parameter-write entry point. Records the + /// call in , persists the value into + /// at the canonical address (so a subsequent read returns + /// the written value), and resolves to if seeded + /// (lets a test simulate EW_PASSWD -> ). + /// + public virtual Task WriteParameterAsync( + FocasAddress address, FocasDataType type, object? value, CancellationToken ct) + { + if (ThrowOnWrite) throw Exception ?? new InvalidOperationException(); + ParameterWriteLog.Add((address, type, value)); + Values[address.Canonical] = value; + var status = WriteStatuses.TryGetValue(address.Canonical, out var s) ? s : FocasStatusMapper.Good; + return Task.FromResult(status); + } + + /// + /// Plan PR F4-b (issue #269) — typed macro-write entry point. See + /// for the per-canonical-address store / log shape. + /// + public virtual Task WriteMacroAsync( + FocasAddress address, object? value, CancellationToken ct) + { + if (ThrowOnWrite) throw Exception ?? new InvalidOperationException(); + MacroWriteLog.Add((address, value)); + Values[address.Canonical] = value; + var status = WriteStatuses.TryGetValue(address.Canonical, out var s) ? s : FocasStatusMapper.Good; + return Task.FromResult(status); + } + public List<(int number, int axis, FocasDataType type)> DiagnosticReads { get; } = new(); public virtual Task<(object? value, uint status)> ReadDiagnosticAsync( diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasWriteMacroTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasWriteMacroTests.cs new file mode 100644 index 0000000..9ca0809 --- /dev/null +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasWriteMacroTests.cs @@ -0,0 +1,201 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests; + +/// +/// Issue #269, plan PR F4-b — cnc_wrmacro coverage. The driver-level +/// Writes.AllowMacro kill switch sits on top of the F4-a +/// Writes.Enabled + per-tag Writable opt-ins. MACRO tags +/// surface (lighter ACL than PARAM) +/// because macro writes are the standard HMI-driven recipe / setpoint surface. +/// +[Trait("Category", "Unit")] +public sealed class FocasWriteMacroTests +{ + private const string Host = "focas://10.0.0.5:8193"; + + private static FocasDriver NewDriver( + FocasWritesOptions writes, + FocasTagDefinition[] tags, + out FakeFocasClientFactory factory) + { + factory = new FakeFocasClientFactory(); + return new FocasDriver(new FocasDriverOptions + { + Devices = [new FocasDeviceOptions(Host)], + Tags = tags, + Probe = new FocasProbeOptions { Enabled = false }, + Writes = writes, + }, "drv-1", factory); + } + + [Fact] + public async Task AllowMacro_false_returns_BadNotWritable_even_with_Enabled_and_Writable() + { + var drv = NewDriver( + writes: new FocasWritesOptions { Enabled = true, AllowMacro = false, AllowParameter = true }, + tags: + [ + new FocasTagDefinition("M500", Host, "MACRO:500", FocasDataType.Int32, Writable: true), + ], + out var factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + var results = await drv.WriteAsync( + [new WriteRequest("M500", 99)], CancellationToken.None); + + results.Single().StatusCode.ShouldBe(FocasStatusMapper.BadNotWritable); + // Gate fires pre-EnsureConnectedAsync so no wire client gets constructed at all. + factory.Clients.ShouldBeEmpty(); + } + + [Fact] + public async Task AllowMacro_true_dispatches_to_typed_WriteMacroAsync() + { + var drv = NewDriver( + writes: new FocasWritesOptions { Enabled = true, AllowMacro = true }, + tags: + [ + new FocasTagDefinition("M500", Host, "MACRO:500", FocasDataType.Int32, Writable: true), + ], + out var factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + var results = await drv.WriteAsync( + [new WriteRequest("M500", 99)], CancellationToken.None); + + results.Single().StatusCode.ShouldBe(FocasStatusMapper.Good); + var log = factory.Clients[0].MacroWriteLog; + log.Count.ShouldBe(1); + log[0].addr.Kind.ShouldBe(FocasAreaKind.Macro); + log[0].addr.Number.ShouldBe(500); + log[0].value.ShouldBe(99); + // Generic write log untouched — MACRO routes through the typed entry point. + factory.Clients[0].WriteLog.ShouldBeEmpty(); + } + + [Fact] + public async Task MacroWrite_round_trip_stores_value_visible_to_subsequent_read() + { + var drv = NewDriver( + writes: new FocasWritesOptions { Enabled = true, AllowMacro = true }, + tags: + [ + new FocasTagDefinition("M500", Host, "MACRO:500", FocasDataType.Int32, Writable: true), + ], + out var factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + await drv.WriteAsync([new WriteRequest("M500", 42)], CancellationToken.None); + + factory.Clients[0].Values["MACRO:500"].ShouldBe(42); + } + + [Fact] + public void Tag_classification_MACRO_yields_Operate() + { + // Server-layer ACL key — MACRO: tags require WriteOperate (per the + // F4-b plan note about lighter authorization for HMI recipe writes). + var tag = new FocasTagDefinition( + "M500", Host, "MACRO:500", FocasDataType.Int32, Writable: true); + + FocasDriver.ClassifyTag(tag).ShouldBe(SecurityClassification.Operate); + } + + [Fact] + public void FocasWritesOptions_default_AllowMacro_is_false() + { + new FocasWritesOptions().AllowMacro.ShouldBeFalse(); + new FocasDriverOptions().Writes.AllowMacro.ShouldBeFalse(); + } + + [Fact] + public void Dto_round_trip_preserves_AllowMacro() + { + const string json = """ + { + "Backend": "unimplemented", + "Devices": [{ "HostAddress": "focas://10.0.0.5:8193" }], + "Tags": [{ + "Name": "M", "DeviceHostAddress": "focas://10.0.0.5:8193", + "Address": "MACRO:500", "DataType": "Int32", "Writable": true + }], + "Writes": { "Enabled": true, "AllowMacro": true } + } + """; + + var drv = FocasDriverFactoryExtensions.CreateInstance("drv-1", json); + drv.InitializeAsync("{}", CancellationToken.None).GetAwaiter().GetResult(); + + var results = drv.WriteAsync( + [new WriteRequest("M", 42)], CancellationToken.None).GetAwaiter().GetResult(); + // unimplemented backend — anything except BadNotWritable proves the gate let + // the call through (likely BadCommunicationError from the no-op factory). + results.Single().StatusCode.ShouldNotBe(FocasStatusMapper.BadNotWritable); + } + + [Fact] + public void Dto_round_trip_preserves_both_granular_flags() + { + // Both flags must round-trip independently — a deployment that opts into + // PARAM only doesn't accidentally let MACRO writes through, and vice versa. + const string json = """ + { + "Backend": "unimplemented", + "Devices": [{ "HostAddress": "focas://10.0.0.5:8193" }], + "Writes": { "Enabled": true, "AllowParameter": true, "AllowMacro": true } + } + """; + + var drv = FocasDriverFactoryExtensions.CreateInstance("drv-1", json); + drv.InitializeAsync("{}", CancellationToken.None).GetAwaiter().GetResult(); + + // Both gates open: the driver no longer rejects either kind at the granular + // gate. Submit one of each and confirm neither maps to BadNotWritable + // (BadNodeIdUnknown when the tag isn't configured is fine — what matters is + // we stayed past the per-kind gate). + var results = drv.WriteAsync( + [ + new WriteRequest("ParamUnknown", 0), + new WriteRequest("MacroUnknown", 0), + ], CancellationToken.None).GetAwaiter().GetResult(); + + // Both writes hit BadNodeIdUnknown (tag-lookup fails) rather than the + // BadNotWritable short-circuit — confirms both flags reached the driver. + results.Count.ShouldBe(2); + results[0].StatusCode.ShouldBe(FocasStatusMapper.BadNodeIdUnknown); + results[1].StatusCode.ShouldBe(FocasStatusMapper.BadNodeIdUnknown); + } + + [Fact] + public async Task Per_kind_gate_does_not_affect_PMC_writes() + { + // Defense in depth: AllowParameter / AllowMacro stay locked but PMC writes + // (which already worked in F4-a) keep flowing through Writes.Enabled + + // per-tag Writable. This guards against regressing the F4-a surface. + var drv = NewDriver( + writes: new FocasWritesOptions + { + Enabled = true, + AllowParameter = false, + AllowMacro = false, + }, + tags: + [ + new FocasTagDefinition("R100", Host, "R100", FocasDataType.Byte, Writable: true), + ], + out var factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + var results = await drv.WriteAsync( + [new WriteRequest("R100", (sbyte)1)], CancellationToken.None); + + results.Single().StatusCode.ShouldBe(FocasStatusMapper.Good); + // PMC routes through the generic WriteAsync, not the typed entry points. + factory.Clients[0].WriteLog.Count.ShouldBe(1); + factory.Clients[0].ParameterWriteLog.ShouldBeEmpty(); + factory.Clients[0].MacroWriteLog.ShouldBeEmpty(); + } +} diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasWriteParameterTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasWriteParameterTests.cs new file mode 100644 index 0000000..d7f7f56 --- /dev/null +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasWriteParameterTests.cs @@ -0,0 +1,232 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests; + +/// +/// Issue #269, plan PR F4-b — cnc_wrparam coverage. The driver-level +/// Writes.AllowParameter kill switch sits on top of the F4-a +/// Writes.Enabled + per-tag Writable opt-ins; PARAM tags +/// additionally surface for the +/// server-layer ACL gate. +/// +[Trait("Category", "Unit")] +public sealed class FocasWriteParameterTests +{ + private const string Host = "focas://10.0.0.5:8193"; + + private static FocasDriver NewDriver( + FocasWritesOptions writes, + FocasTagDefinition[] tags, + out FakeFocasClientFactory factory) + { + factory = new FakeFocasClientFactory(); + return new FocasDriver(new FocasDriverOptions + { + Devices = [new FocasDeviceOptions(Host)], + Tags = tags, + Probe = new FocasProbeOptions { Enabled = false }, + Writes = writes, + }, "drv-1", factory); + } + + [Fact] + public async Task AllowParameter_false_returns_BadNotWritable_even_with_Enabled_and_Writable() + { + // F4-b — the granular kill switch defaults off so a redeployed driver with + // Writes.Enabled=true still keeps PARAM writes locked until the operator team + // explicitly opts in. This is the critical defense-in-depth assertion. + var drv = NewDriver( + writes: new FocasWritesOptions { Enabled = true, AllowParameter = false, AllowMacro = true }, + tags: + [ + new FocasTagDefinition("Param", Host, "PARAM:1815", FocasDataType.Int32, Writable: true), + ], + out var factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + var results = await drv.WriteAsync( + [new WriteRequest("Param", 42)], CancellationToken.None); + + results.Single().StatusCode.ShouldBe(FocasStatusMapper.BadNotWritable); + // Wire client was never even constructed because the gate short-circuited + // before EnsureConnectedAsync — defense in depth + lower blast radius if the + // wire client is misconfigured. + factory.Clients.ShouldBeEmpty(); + } + + [Fact] + public async Task AllowParameter_true_dispatches_to_typed_WriteParameterAsync() + { + var drv = NewDriver( + writes: new FocasWritesOptions { Enabled = true, AllowParameter = true }, + tags: + [ + new FocasTagDefinition("Param", Host, "PARAM:1815", FocasDataType.Int32, Writable: true), + ], + out var factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + var results = await drv.WriteAsync( + [new WriteRequest("Param", 42)], CancellationToken.None); + + results.Single().StatusCode.ShouldBe(FocasStatusMapper.Good); + var log = factory.Clients[0].ParameterWriteLog; + log.Count.ShouldBe(1); + log[0].addr.Kind.ShouldBe(FocasAreaKind.Parameter); + log[0].addr.Number.ShouldBe(1815); + log[0].type.ShouldBe(FocasDataType.Int32); + log[0].value.ShouldBe(42); + // Generic write log is untouched — PARAM goes through the typed entry point. + factory.Clients[0].WriteLog.ShouldBeEmpty(); + } + + [Fact] + public async Task ParameterWrite_round_trip_stores_value_visible_to_subsequent_read() + { + var drv = NewDriver( + writes: new FocasWritesOptions { Enabled = true, AllowParameter = true }, + tags: + [ + new FocasTagDefinition("Param", Host, "PARAM:1815", FocasDataType.Int32, Writable: true), + ], + out var factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + await drv.WriteAsync([new WriteRequest("Param", 42)], CancellationToken.None); + + // Round-trip: the next read sees the written value because the fake stores + // by canonical address. This exercises the same shape an integration test + // against focas-mock would: write -> read returns the written value. + factory.Clients[0].Values["PARAM:1815"].ShouldBe(42); + } + + [Fact] + public async Task EW_PASSWD_from_simulator_maps_to_BadUserAccessDenied() + { + // F4-b — when the CNC reports EW_PASSWD (parameter-write switch off / unlock + // required) the status code surfaces as BadUserAccessDenied rather than + // BadNotWritable. F4-d will land the unlock workflow on top of this surface. + var drv = NewDriver( + writes: new FocasWritesOptions { Enabled = true, AllowParameter = true }, + tags: + [ + new FocasTagDefinition("Param", Host, "PARAM:1815", FocasDataType.Int32, Writable: true), + ], + out var factory); + await drv.InitializeAsync("{}", CancellationToken.None); + // Seed the fake to mimic EW_PASSWD propagation (mapper assigns + // BadUserAccessDenied to that EW_* code post-F4-b). + factory.Customise = () => + { + var c = new FakeFocasClient(); + c.WriteStatuses["PARAM:1815"] = FocasStatusMapper.BadUserAccessDenied; + return c; + }; + + // Re-init now that the customiser is in place. + await drv.ShutdownAsync(CancellationToken.None); + await drv.InitializeAsync("{}", CancellationToken.None); + + var results = await drv.WriteAsync( + [new WriteRequest("Param", 42)], CancellationToken.None); + + results.Single().StatusCode.ShouldBe(FocasStatusMapper.BadUserAccessDenied); + } + + [Fact] + public void StatusMapper_EW_PASSWD_returns_BadUserAccessDenied() + { + // EW_PASSWD == 11 per FANUC FOCAS docs. Pre-F4-b this mapped to BadNotWritable; + // F4-b remaps it so clients can branch on user-vs-driver write rejection. + FocasStatusMapper.MapFocasReturn(11).ShouldBe(FocasStatusMapper.BadUserAccessDenied); + } + + [Fact] + public void Tag_classification_PARAM_yields_Configure() + { + // Server-layer ACL key — PARAM: tags require WriteConfigure group membership + // (vs MACRO: tags which require WriteOperate). Per the + // feedback_acl_at_server_layer memory the driver only declares classification; + // DriverNodeManager applies the gate. + var tag = new FocasTagDefinition( + "Param", Host, "PARAM:1815", FocasDataType.Int32, Writable: true); + + FocasDriver.ClassifyTag(tag).ShouldBe(SecurityClassification.Configure); + } + + [Fact] + public void Tag_classification_PARAM_non_writable_yields_ViewOnly() + { + // ViewOnly trumps the kind-based classification when the tag isn't writable. + var tag = new FocasTagDefinition( + "Param", Host, "PARAM:1815", FocasDataType.Int32, Writable: false); + + FocasDriver.ClassifyTag(tag).ShouldBe(SecurityClassification.ViewOnly); + } + + [Fact] + public void FocasWritesOptions_default_AllowParameter_is_false() + { + // Defense in depth: a fresh FocasWritesOptions has both granular kill + // switches off so a config-DB row that omits AllowParameter doesn't + // silently flip parameter writes on. + new FocasWritesOptions().AllowParameter.ShouldBeFalse(); + new FocasDriverOptions().Writes.AllowParameter.ShouldBeFalse(); + } + + [Fact] + public void Dto_round_trip_preserves_AllowParameter() + { + // JSON config -> FocasDriverOptions; the Writes.AllowParameter flag must + // survive the bootstrapper's deserialize step. Use a known-empty Tags config + // and a sentinel write to verify the gate reached the driver. + const string jsonAllowed = """ + { + "Backend": "unimplemented", + "Devices": [{ "HostAddress": "focas://10.0.0.5:8193" }], + "Tags": [{ + "Name": "P", "DeviceHostAddress": "focas://10.0.0.5:8193", + "Address": "PARAM:1815", "DataType": "Int32", "Writable": true + }], + "Writes": { "Enabled": true, "AllowParameter": true } + } + """; + + var drv = FocasDriverFactoryExtensions.CreateInstance("drv-1", jsonAllowed); + drv.InitializeAsync("{}", CancellationToken.None).GetAwaiter().GetResult(); + + // Real wire client is the unimplemented one — Connect throws, and the driver + // surfaces that as BadCommunicationError. The key is that the result is NOT + // BadNotWritable: that proves the AllowParameter gate didn't short-circuit. + var results = drv.WriteAsync( + [new WriteRequest("P", 42)], CancellationToken.None).GetAwaiter().GetResult(); + results.Single().StatusCode.ShouldNotBe(FocasStatusMapper.BadNotWritable); + } + + [Fact] + public void Dto_default_omitted_AllowParameter_keeps_safer_default() + { + // A Writes section with just { Enabled: true } must NOT silently flip the + // granular kill switches on. PARAM writes should still get BadNotWritable. + const string json = """ + { + "Backend": "unimplemented", + "Devices": [{ "HostAddress": "focas://10.0.0.5:8193" }], + "Tags": [{ + "Name": "P", "DeviceHostAddress": "focas://10.0.0.5:8193", + "Address": "PARAM:1815", "DataType": "Int32", "Writable": true + }], + "Writes": { "Enabled": true } + } + """; + + var drv = FocasDriverFactoryExtensions.CreateInstance("drv-1", json); + drv.InitializeAsync("{}", CancellationToken.None).GetAwaiter().GetResult(); + + var results = drv.WriteAsync( + [new WriteRequest("P", 42)], CancellationToken.None).GetAwaiter().GetResult(); + results.Single().StatusCode.ShouldBe(FocasStatusMapper.BadNotWritable); + } +} -- 2.49.1