diff --git a/docs/Driver.FOCAS.Cli.md b/docs/Driver.FOCAS.Cli.md index 13317d0..e11aab9 100644 --- a/docs/Driver.FOCAS.Cli.md +++ b/docs/Driver.FOCAS.Cli.md @@ -120,6 +120,18 @@ otopcua-focas-cli write -h 192.168.1.50 -a MACRO:500 -t Int32 -v 42 otopcua-focas-cli write -h 192.168.1.50 -a PARAM:1815 -t Int32 -v 100 ``` +> **WARNING — `write -a G50.3 -t Bit -v on` is a read-modify-write.** +> The wire call `pmc_wrpmcrng` is byte-addressed; the driver reads the +> parent byte at `G50` first, sets bit 3, and writes the byte back. Other +> bits in `G50` that the ladder is concurrently updating may be clobbered +> by the byte we read a millisecond ago. Coordinate via a ladder-side +> handshake when this matters. **PMC writes also bypass the ladder's +> normal MDI-mode protection** — a misdirected bit can move motion or +> latch a feedhold the moment it lands. Verify e-stop is live and the +> machine is in JOG mode before issuing the first PMC write of a +> session. See [`docs/drivers/FOCAS.md`](drivers/FOCAS.md) "PMC bit-write +> read-modify-write semantics" for the full RMW flow. + 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. @@ -146,17 +158,17 @@ 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` enforcement (issue #268 F4-a + #269 F4-b) +#### Server-side `Writes` enforcement (issue #268 F4-a + #269 F4-b + #270 F4-c) 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. +(MACRO kill switch — F4-b), `Writes.AllowPmc` (PMC kill switch — F4-c), +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. **The CLI bypasses the server-side flag.** `otopcua-focas-cli write` is a per-invocation operator tool — it sets `Writes.Enabled = true` locally for diff --git a/docs/drivers/FOCAS.md b/docs/drivers/FOCAS.md index 97b3a0f..2a883ff 100644 --- a/docs/drivers/FOCAS.md +++ b/docs/drivers/FOCAS.md @@ -54,7 +54,7 @@ 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 (F4-a) + #269 (F4-b) +## Writes (opt-in, off by default) — issue #268 (F4-a) + #269 (F4-b) + #270 (F4-c) 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 @@ -66,22 +66,54 @@ choice. Decision record: [`docs/v2/decisions.md`](../v2/decisions.md) → | `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. | | **`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.** | +| **`FocasDriverOptions.Writes.AllowPmc`** *(F4-c granular kill switch)* | **`false`** | **PMC writes (R/G/F/D/X/Y/K/A/E/T/C letters, both Bit and Byte) return `BadNotWritable` with no wire client constructed. PMC is ladder working memory — a mistargeted bit can move motion, latch a feedhold, or flip a safety interlock, so PMC writes are gated separately from PARAM/MACRO so an operator team can open PARAM (commissioning) without exposing the much higher-blast-radius PMC 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 — F4-b +> **PMC SAFETY CALLOUT** — PMC is the FANUC ladder's working memory. A +> mistargeted bit can move motion (a Y-coil writing to a servo enable), +> latch a feedhold (an internal R-relay the ladder ANDs with cycle-start), +> or flip a safety interlock (an X-input shadow). **Treat PMC writes the +> same way you'd treat editing a live ladder:** verify e-stop is live and +> the machine is in jog mode before issuing the first write of a session. +> The driver gates these writes behind THREE independent opt-ins +> (`Writes.Enabled` + `Writes.AllowPmc` + per-tag `Writable`) precisely +> because the blast radius is higher than parameter writes. + +### PMC bit-write read-modify-write semantics — F4-c + +The FOCAS wire call `pmc_wrpmcrng` is **byte-addressed** — there is no +sub-byte write primitive. When the driver receives a write request on a +`Bit` tag (e.g. `R100.3`), it: + +1. Reads the parent byte via `pmc_rdpmcrng` (1 byte at `R100`). +2. Masks the target bit (set: `current | (1 << bit)`; clear: `current & ~(1 << bit)`). +3. Writes the modified byte back via `pmc_wrpmcrng` (1 byte at `R100`). + +A **per-byte semaphore** serialises concurrent bit writes against the same +byte so two updates that race never lose one another's bit. RMW means **a +PMC bit write reads first, then writes back the whole byte** — if the ladder +is also writing to that byte at the same instant, there is a small window +where the driver's value can clobber the ladder's. Operators who care about +this race must coordinate the write through a ladder-side handshake (e.g. +the operator sets a request bit, the ladder reads + clears it). + +### Config shape — F4-c ```jsonc { "Writes": { "Enabled": true, "AllowParameter": true, // F4-b — opt into cnc_wrparam - "AllowMacro": true // F4-b — opt into cnc_wrmacro + "AllowMacro": true, // F4-b — opt into cnc_wrmacro + "AllowPmc": true // F4-c — opt into pmc_wrpmcrng (incl. RMW bit writes) }, "Tags": [ { "Name": "RPM", "Address": "PARAM:1815", "DataType": "Int32", "Writable": true, "WriteIdempotent": false }, { "Name": "Recipe", "Address": "MACRO:500", "DataType": "Int32", - "Writable": true, "WriteIdempotent": false } + "Writable": true, "WriteIdempotent": false }, + { "Name": "StartFlag", "Address": "R100.3", "DataType": "Bit", + "Writable": true, "WriteIdempotent": true } ] } ``` @@ -119,9 +151,9 @@ value that the operator simply wants forced to a target). - `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. + (F4-b)**; **`Writes.AllowMacro = false` for a `MACRO:` tag (F4-b)**; + **`Writes.AllowPmc = false` for a PMC tag (F4-c)**. Same status code, + five 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 diff --git a/docs/v2/focas-deployment.md b/docs/v2/focas-deployment.md index 20de016..e5c9376 100644 --- a/docs/v2/focas-deployment.md +++ b/docs/v2/focas-deployment.md @@ -44,12 +44,13 @@ 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 +## Write safety — issue #269 (PARAM/MACRO, F4-b) + issue #270 (PMC, F4-c) -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. +The FOCAS driver supports `cnc_wrparam`, `cnc_wrmacro`, and `pmc_wrpmcrng` +writes behind multiple independent opt-ins. A misdirected parameter write +can put the CNC in a bad state; a misdirected PMC write can move motion or +latch a feedhold. The runbook below MUST be followed before flipping any +of the granular kill switches on. ### Operator pre-checks (every deployment, every change) @@ -72,6 +73,35 @@ the granular kill switches on. `BadNotWritable` until you flip the granular flag, so you can confirm the tag list before any wire write fires. +### PMC pre-checks (in addition to the above) — F4-c + +PMC writes have a higher blast radius than PARAM/MACRO writes because PMC +is the ladder's working memory — bits in R/G/F/D directly drive servo +enables, feedhold latches, and safety interlocks. Before flipping +`Writes.AllowPmc` on: + +1. **E-stop verified live + reachable.** The first PMC write of a session + should be issued with the operator's hand on the e-stop. PMC writes + bypass the ladder's normal MDI-mode protections; a misdirected bit can + move motion the moment it lands on the wire. +2. **Machine in JOG mode (or equivalent low-energy mode).** Auto / MEM + modes interpret PMC state immediately; JOG / MDI surface symptoms + slowly enough that the e-stop is the recovery path. **Never issue the + first PMC write of a deployment in Auto.** +3. **Audit the PMC tag list against the ladder print-out.** `R100.3` on + one machine is "homing complete"; on another it's "feedhold released". + The driver has no way to distinguish — the ladder source is the only + ground truth. +4. **Bit writes are read-modify-write — see + [`docs/drivers/FOCAS.md`](../drivers/FOCAS.md) "PMC bit-write read-modify-write semantics".** + `pmc_wrpmcrng` is byte-addressed; the driver reads the parent byte + first, masks the target bit, and writes the byte back. Concurrent + ladder writes to the same byte create a small race window. Coordinate + through a ladder-side handshake when this matters. +5. **Dry run with `Writable = true` but `Writes.AllowPmc = false`.** Same + staged-opt-in pattern as PARAM/MACRO — confirm tag mapping before any + PMC byte hits the wire. + ### LDAP group requirements Per [`docs/security.md`](../security.md) the server-layer ACL maps @@ -108,6 +138,14 @@ 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. +**Audit PMC writes specifically.** Because PMC writes have the highest blast +radius of the three write kinds, ops should set up a saved-search / +dashboard query for `Driver=FOCAS` + `Address` matching the PMC letter +prefixes (`R*`, `G*`, `F*`, `D*`, `Y*`, etc.) and review on the same +cadence as ladder change reviews. A spike in PMC write rate or a write +to an address outside the audited tag list is the leading indicator of a +misconfigured client or compromised credential. + ### Granular config example ```jsonc @@ -120,7 +158,8 @@ succeeded. "Writes": { "Enabled": true, "AllowMacro": true, // recipe / setpoint writes — operator role - "AllowParameter": false // commissioning only — keep locked except during planned work + "AllowParameter": false, // commissioning only — keep locked except during planned work + "AllowPmc": false // PMC writes — keep locked unless the deployment specifically needs them }, "Tags": [ { "Name": "Recipe.PartCount", "DeviceHostAddress": "focas://10.0.0.5:8193", @@ -128,13 +167,19 @@ succeeded. "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 */ } + "Writable": false /* keep read-only until commissioning window */ }, + { "Name": "OperatorRequest", "DeviceHostAddress": "focas://10.0.0.5:8193", + "Address": "R100.3", "DataType": "Bit", + "Writable": false /* keep PMC read-only until ladder handshake reviewed */ } ] } } } ``` -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. +Flipping `AllowParameter` / `AllowPmc` on for the commissioning window +(and back off afterward) is the recommended deployment cadence — the +granular kill switches are lightweight runtime toggles, not config-DB +redeploys. PMC in particular should default OFF in production and only +flip on for windows where the ladder team has signed off on the write +path. diff --git a/docs/v2/implementation/focas-simulator-plan.md b/docs/v2/implementation/focas-simulator-plan.md index 7d40acc..e3c407b 100644 --- a/docs/v2/implementation/focas-simulator-plan.md +++ b/docs/v2/implementation/focas-simulator-plan.md @@ -31,6 +31,7 @@ command ids (and their request/response payloads) don't drift between the | ... | ... | ... | | **`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** | +| **`0x0104`** | **`pmc_wrpmcrng`** | **mutates per-profile PMC byte tables; byte-aligned writes preserve untouched bytes; bit-level writes never reach the simulator (driver wraps with RMW) — issue #270, plan PR F4-c** | | **`0x0F1A`** | **`cnc_rdalmhistry`** | **dumps the per-profile alarm-history ring buffer (issue #267, plan PR F3-a)** | ## `cnc_rdalmhistry` mock behaviour @@ -183,13 +184,97 @@ 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. +## `pmc_wrpmcrng` mock behaviour — issue #270, plan PR F4-c + +The simulator keeps a per-profile PMC byte table keyed by `(addr_type, +byte_address)` — the same map the existing `pmc_rdpmcrng` handler reads +from. The write handler mutates the same map so a subsequent read sees +the written bytes. + +### Per-profile state + +Each profile carries: + +```python +pmc: Dict[int, bytearray] # addr_type -> bytearray (one per PMC letter, default 256 bytes each) +``` + +`addr_type` is the PMC area code (R=5, G=4, F=3, D=8, X=1, Y=2, K=10, +A=11, E=12, T=6, C=7); the existing `pmc_rdpmcrng` fixture seeds the +defaults (zeros + a few canned bits per the dl205-style profile fixtures). + +### `pmc_wrpmcrng` request decode + +| Offset | Width | Field | +| --- | --- | --- | +| 0 | int16 LE | `addr_type` | +| 2 | int16 LE | `data_type` (must be `0` = byte; the driver only emits byte writes) | +| 4 | uint16 LE | `datano_s` | +| 6 | uint16 LE | `datano_e` | +| 8 | bytes | `data[]` — `(datano_e - datano_s + 1)` bytes | + +Handler steps: + +1. Look up the per-profile bytearray for `addr_type` (allocate on first + write, default 256 zeros). +2. **Validate** `0 <= datano_s <= datano_e < len(bytearray)` — otherwise + return `EW_NUMBER` (`4`). +3. **Validate** `len(data) == datano_e - datano_s + 1` — otherwise + return `EW_LENGTH` (`14`). +4. **Validate** `data_type == 0` — otherwise return `EW_DATA` (`9`) + because the driver only ever emits byte writes (bit writes wrap with + driver-side RMW so they reach the simulator as 1-byte writes). +5. Copy `data[]` into `bytearray[datano_s:datano_e+1]`. Other bytes + in the array are untouched. +6. Update `last_write` admin-endpoint state (kind=`pmc`, address-type, + start byte, length, bytes). +7. Return `ew_status = 0`. + +### Round-trip invariant + +The simulator MUST satisfy: + +``` +write(R, [10..12], [0xAA, 0xBB, 0xCC]); read(R, [10..12]) == [0xAA, 0xBB, 0xCC] +``` + +and the **byte-isolation invariant**: + +``` +write(R, [11], [0xFF]); bytes[10] == prior bytes[10] && bytes[12] == prior bytes[12] +``` + +The integration tests `Series/PmcRangeWriteTests.cs` and +`Series/PmcBitRmwIntegrationTests.cs` assert both shapes. + +### Admin endpoint — `GET /admin/mock_get_last_write` extension + +The `last_write` payload gains a `kind: "pmc"` variant: + +``` +{ + "kind": "pmc", + "addr_type": 5, // R + "datano_s": 100, + "datano_e": 100, + "bytes": "0x08", // hex-encoded + "writtenAt": "2026-04-25T13:30:00Z" +} +``` + +Bit-level writes never appear here as a separate kind — they reach the +simulator as 1-byte writes after the driver's RMW wrapper, so the audit +shape is identical to a byte write at the same address. + ### 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`. +focas-mock simulator has not landed yet (tracked separately from F4-b / +F4-c). F4-b + F4-c land the .NET-side wire encoders + dispatch + status +mapping unconditionally; the integration-test scaffolds at +`tests/.../IntegrationTests/Series/ParameterWriteTests.cs`, +`MacroWriteTests.cs`, `PmcRangeWriteTests.cs`, and +`PmcBitRmwIntegrationTests.cs` are deferred until the simulator + +integration-test project land. Until then unit-test coverage in +`FocasWriteParameterTests` / `FocasWriteMacroTests` / +`FocasWritePmcTests` 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 3f6ee11..686a119 100644 --- a/docs/v2/implementation/focas-wire-protocol.md +++ b/docs/v2/implementation/focas-wire-protocol.md @@ -21,6 +21,7 @@ Each FOCAS-equivalent call gets a stable wire-protocol command id. Ids are | ... | ... | ... | | **`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)** | +| **`0x0104`** | **`pmc_wrpmcrng`** | **IODBPMC PMC range-write packet (issue #270, plan PR F4-c)** | | `0x0F1A` | **`cnc_rdalmhistry`** | **ODBALMHIS alarm-history ring-buffer dump (issue #267, plan PR F3-a)** | ## ODBALMHIS — alarm history (`cnc_rdalmhistry`, command `0x0F1A`) @@ -154,3 +155,62 @@ 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. + +## IODBPMC — PMC range write (`pmc_wrpmcrng`, command `0x0104`) + +Issue #270, plan PR F4-c. The write-side payload is the read-side +`pmc_rdpmcrng` IODBPMC packet with the data direction inverted: the +caller fills the `data[]` byte run and the simulator / Fwlib32 stores +it; the response is the small status envelope rather than the populated +data buffer the read side returns. + +### Request + +| Offset | Width | Field | +| --- | --- | --- | +| 0 | `int16 LE` | `type_a` — PMC address-type code (R=5, G=4, F=3, D=8, X=1, Y=2, K=10, A=11, E=12, T=6, C=7) | +| 2 | `int16 LE` | `type_d` — data type (`0` = byte; only byte writes are issued — bit writes wrap the byte path with a read-modify-write helper) | +| 4 | `uint16 LE` | `datano_s` — first byte address (inclusive) | +| 6 | `uint16 LE` | `datano_e` — last byte address (inclusive) — `(datano_e - datano_s + 1)` is the byte count | +| 8 | `bytes` | `data[]` — payload, exactly `(datano_e - datano_s + 1)` bytes | + +The header is 8 bytes; the FWLIB `IODBPMC.data` field caps at 32 bytes +(40-byte total per call), so larger ranges are chunked into 32-byte +sub-calls by the wire client. The simulator MUST honour the same chunk +ceiling so chunked-vs-single round-trips produce the same final bytes. + +### Response + +Same single-int16 envelope as `cnc_wrparam` / `cnc_wrmacro`: + +| Offset | Width | Field | +| --- | --- | --- | +| 0 | `int16 LE` | `ew_status` — `0` = success, non-zero = FANUC `EW_*` | + +`EW_NOOPT` (option not installed), `EW_NUMBER` (out-of-range address), +`EW_LENGTH` (chunk size mismatch) are the typical failures the simulator +reproduces; the mapper translates them to OPC UA status codes the same +way the read-side does. + +### Bit-level RMW (driver-side, no extra wire op) + +`pmc_wrpmcrng` is **byte-addressed** — there is no sub-byte write op on +the wire. Bit writes go through `IFocasClient.WritePmcBitAsync` which: + +1. Issues a 1-byte `pmc_rdpmcrng` to fetch the parent byte. +2. Masks the target bit (set: OR; clear: AND-NOT). +3. Issues a 1-byte `pmc_wrpmcrng` with the modified byte. + +A per-byte semaphore in `FwlibFocasClient` serialises concurrent bit +writes against the same byte so two updates that race never lose one +another's bit. The simulator's handler implements the same byte-aligned +semantics — bit writes never reach it as a separate frame. + +### Symmetry note + +The encoder is the `pmc_rdpmcrng` decoder reversed: the read side parses +`(type_a, type_d, datano_s, datano_e)` from the request and emits the +data buffer in the response; the write side parses all five fields plus +the data buffer from the request and emits a status int16 in the +response. Tests `FocasWritePmcTests.PMC_*` exercise the round-trip on +the fake wire client. diff --git a/scripts/e2e/test-focas.ps1 b/scripts/e2e/test-focas.ps1 index a6ba777..b2c861c 100644 --- a/scripts/e2e/test-focas.ps1 +++ b/scripts/e2e/test-focas.ps1 @@ -47,6 +47,15 @@ writes are the lowest-risk write surface (no parameter-write switch needed, no MDI mode required) so this stage runs whenever -Write is supplied. + +.PARAMETER PmcBitAddress + PMC bit address for the F4-c bit-write round-trip stage (default + R100.3). Only fires when -Write is supplied AND the operator + double-opts in via FOCAS_PMC_WRITE=1, mirroring the FOCAS_PARAM_WRITE + gate. PMC writes have a higher blast radius than PARAM/MACRO (a + mistargeted bit can move motion or latch a feedhold) so the gate is + off by default — see docs/v2/focas-deployment.md "Write safety / PMC + pre-checks". #> param( @@ -57,7 +66,8 @@ param( [Parameter(Mandatory)] [string]$BridgeNodeId, [switch]$Write, [string]$ParamAddress = "PARAM:1815", - [string]$MacroAddress = "MACRO:500" + [string]$MacroAddress = "MACRO:500", + [string]$PmcBitAddress = "R100.3" ) $ErrorActionPreference = "Stop" @@ -146,6 +156,27 @@ if ($Write) { } 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')." } + + # F4-c — PMC bit round-trip. PMC writes have a higher blast radius + # than PARAM/MACRO (a mistargeted bit can move motion or latch a + # feedhold) so the stage is gated on a separate FOCAS_PMC_WRITE=1 + # opt-in. The bit write exercises the driver's read-modify-write + # path: write 'on' -> read returns 'on'; write 'off' -> read returns + # 'off'. Both halves run so a regression in either branch is caught. + if ($env:FOCAS_PMC_WRITE -eq "1" -or $env:FOCAS_PMC_WRITE -eq "true") { + $results += Test-DriverLoopback ` + -Cli $focasCli ` + -WriteArgs (@("write") + $commonFocas + @("-a", $PmcBitAddress, "-t", "Bit", "-v", "on")) ` + -ReadArgs (@("read") + $commonFocas + @("-a", $PmcBitAddress, "-t", "Bit")) ` + -ExpectedValue "True" + $results += Test-DriverLoopback ` + -Cli $focasCli ` + -WriteArgs (@("write") + $commonFocas + @("-a", $PmcBitAddress, "-t", "Bit", "-v", "off")) ` + -ReadArgs (@("read") + $commonFocas + @("-a", $PmcBitAddress, "-t", "Bit")) ` + -ExpectedValue "False" + } else { + Write-Host "[skip] FOCAS_PMC_WRITE not set — PMC bit-write round-trip is off by default because a mistargeted PMC bit can move motion or latch a feedhold (see docs/v2/focas-deployment.md 'PMC pre-checks')." + } } 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 e800b36..6b0bda9 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs @@ -540,6 +540,19 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, results[i] = new WriteResult(FocasStatusMapper.BadNotWritable); continue; } + // PR F4-c (issue #270) — granular gate for PMC writes. PMC is ladder + // working memory; a mistargeted bit can move motion or latch a feedhold + // so the operator team must explicitly opt in via Writes.AllowPmc on + // top of Writes.Enabled + per-tag Writable. Defaults to false so a + // deployment that flips the master switch on without touching the PMC + // gate still gets BadNotWritable for every PMC tag. ACL note: PMC tags + // surface SecurityClassification.Operate (server-layer requires + // WriteOperate) — see ClassifyTag. + if (parsed.Kind == FocasAreaKind.Pmc && !_options.Writes.AllowPmc) + { + 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) @@ -553,18 +566,47 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, device.LastSetPath = parsed.PathId; } - // 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 + // Dispatch through the typed entry points for PARAM/MACRO/PMC so the + // wire-client surface mirrors the per-kind opt-in shape. PMC bit + // writes route through the WritePmcBitAsync RMW helper so the wire + // client only ever sees byte-aligned pmc_wrpmcrng calls (PR F4-c, + // issue #270). The fallback generic WriteAsync path is preserved for + // kinds that don't have a typed entry point yet, plus the unit-test + // FakeFocasClient that overrides WriteAsync directly. + uint status; + if (parsed.Kind == FocasAreaKind.Parameter) { - 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), - }; + status = await client.WriteParameterAsync( + parsed, def.DataType, w.Value, cancellationToken).ConfigureAwait(false); + } + else if (parsed.Kind == FocasAreaKind.Macro) + { + status = await client.WriteMacroAsync( + parsed, w.Value, cancellationToken).ConfigureAwait(false); + } + else if (parsed.Kind == FocasAreaKind.Pmc + && def.DataType == FocasDataType.Bit + && parsed.BitIndex is int bit + && parsed.PmcLetter is string letter) + { + status = await client.WritePmcBitAsync( + letter, parsed.PathId, parsed.Number, bit, + Convert.ToBoolean(w.Value), cancellationToken).ConfigureAwait(false); + } + else if (parsed.Kind == FocasAreaKind.Pmc + && def.DataType == FocasDataType.Byte + && parsed.PmcLetter is string byteLetter) + { + var b = unchecked((byte)Convert.ToSByte(w.Value)); + status = await client.WritePmcRangeAsync( + byteLetter, parsed.PathId, parsed.Number, new[] { b }, + cancellationToken).ConfigureAwait(false); + } + else + { + status = await client.WriteAsync( + parsed, def.DataType, w.Value, cancellationToken).ConfigureAwait(false); + } results[i] = new WriteResult(status); } catch (OperationCanceledException) { throw; } diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriverFactoryExtensions.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriverFactoryExtensions.cs index 417d37a..265d44b 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriverFactoryExtensions.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriverFactoryExtensions.cs @@ -98,6 +98,11 @@ public static class FocasDriverFactoryExtensions // just { Enabled: true } keeps PARAM/MACRO writes locked. AllowParameter = dto.Writes?.AllowParameter ?? false, AllowMacro = dto.Writes?.AllowMacro ?? false, + // Plan PR F4-c (issue #270) — granular kill-switch for pmc_wrpmcrng. + // Default false: PMC is ladder working memory; a mistargeted bit can + // move motion or latch a feedhold so the operator team must explicitly + // opt in even with Enabled=true. + AllowPmc = dto.Writes?.AllowPmc ?? false, }, }; @@ -204,6 +209,12 @@ public static class FocasDriverFactoryExtensions /// . /// public bool? AllowMacro { get; init; } + + /// + /// Plan PR F4-c (issue #270). Default false — see + /// . + /// + public bool? AllowPmc { 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 8c678a7..dc700a0 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriverOptions.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriverOptions.cs @@ -83,6 +83,20 @@ public sealed record FocasWritesOptions /// gate requires WriteOperate group membership. /// public bool AllowMacro { get; init; } = false; + + /// + /// Issue #270, plan PR F4-c — granular kill-switch for pmc_wrpmcrng PMC + /// range writes (and the bit-level read-modify-write that wraps it). Default + /// false: PMC is ladder working memory — a mistargeted bit can move + /// motion, latch a feedhold, or flip a safety interlock. Even with + /// on and a tag's + /// flag flipped on, PMC writes stay locked until this third opt-in fires. + /// Server-layer ACL: PMC tags surface + /// so the OPC UA + /// gate requires WriteOperate group membership; this flag is the driver- + /// level kill switch the operator team can flip without a redeploy. + /// + public bool AllowPmc { get; init; } = false; } /// diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FwlibFocasClient.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FwlibFocasClient.cs index 567f8c7..5af8243 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FwlibFocasClient.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FwlibFocasClient.cs @@ -123,8 +123,10 @@ internal sealed class FwlibFocasClient : IFocasClient } /// - /// 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. + /// Read-modify-write one bit within a PMC byte (Plan PR F4-c, issue #270). + /// Acquires a per-byte semaphore so concurrent bit writes against the same + /// byte serialise and neither loses its update. The wire call is byte-addressed + /// so we read the parent byte, mask the target bit, then write the byte back. /// private async Task WritePmcBitAsync( FocasAddress address, bool newValue, CancellationToken cancellationToken) @@ -151,19 +153,8 @@ internal sealed class FwlibFocasClient : IFocasClient ? (byte)(current | (1 << bit)) : (byte)(current & ~(1 << bit)); - // Write the updated byte. - var writeBuf = new FwlibNative.IODBPMC - { - TypeA = addrType, - TypeD = FocasPmcDataType.Byte, - DatanoS = (ushort)address.Number, - DatanoE = (ushort)address.Number, - Data = new byte[40], - }; - writeBuf.Data[0] = updated; - - var writeRet = FwlibNative.PmcWrPmcRng(_handle, 8 + 1, ref writeBuf); - return writeRet == 0 ? FocasStatusMapper.Good : FocasStatusMapper.MapFocasReturn(writeRet); + // Write the updated byte via pmc_wrpmcrng (1-byte range). + return WritePmcRange(addrType, address.Number, new[] { updated }); } finally { @@ -171,6 +162,52 @@ internal sealed class FwlibFocasClient : IFocasClient } } + /// + /// Plan PR F4-c (issue #270) — typed PMC-range write entry point. Writes a + /// contiguous run of bytes via pmc_wrpmcrng. The FWLIB IODBPMC.Data + /// payload caps at ~40 bytes so larger ranges are chunked into 32-byte + /// sub-calls, mirroring the read-side shape. + /// + public Task WritePmcRangeAsync( + string letter, int pathId, int startByte, byte[] bytes, CancellationToken cancellationToken) + { + if (!_connected) return Task.FromResult(FocasStatusMapper.BadCommunicationError); + cancellationToken.ThrowIfCancellationRequested(); + if (bytes is null || bytes.Length == 0) return Task.FromResult(FocasStatusMapper.Good); + var addrType = FocasPmcAddrType.FromLetter(letter) + ?? throw new InvalidOperationException($"Unknown PMC letter '{letter}'."); + return Task.FromResult(WritePmcRange(addrType, startByte, bytes)); + } + + /// + /// Synchronous PMC range write helper — chunked at 32 bytes so each + /// pmc_wrpmcrng call fits inside the FWLIB IODBPMC.Data 40-byte + /// window (8-byte header + 32-byte payload). Stops on the first non-zero + /// EW_* return so a partial write doesn't claim Good. + /// + private uint WritePmcRange(short addrType, int startByte, byte[] bytes) + { + const int chunkBytes = 32; + var offset = 0; + while (offset < bytes.Length) + { + var thisChunk = Math.Min(chunkBytes, bytes.Length - offset); + var writeBuf = new FwlibNative.IODBPMC + { + TypeA = addrType, + TypeD = FocasPmcDataType.Byte, + DatanoS = (ushort)(startByte + offset), + DatanoE = (ushort)(startByte + offset + thisChunk - 1), + Data = new byte[40], + }; + Array.Copy(bytes, offset, writeBuf.Data, 0, thisChunk); + var ret = FwlibNative.PmcWrPmcRng(_handle, (ushort)(8 + thisChunk), ref writeBuf); + if (ret != 0) return FocasStatusMapper.MapFocasReturn(ret); + offset += thisChunk; + } + return FocasStatusMapper.Good; + } + public Task GetPathCountAsync(CancellationToken cancellationToken) { if (!_connected) return Task.FromResult(1); diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/IFocasClient.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/IFocasClient.cs index 33eb19a..3776c3c 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/IFocasClient.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/IFocasClient.cs @@ -232,6 +232,49 @@ public interface IFocasClient : IDisposable int depth, CancellationToken cancellationToken) => Task.FromResult>(Array.Empty()); + /// + /// Write a contiguous range of PMC bytes in a single wire call (FOCAS + /// pmc_wrpmcrng) for the given starting at + /// , copying every byte from . + /// Plan PR F4-c (issue #270). The wire call is byte-addressed; bit-level writes + /// are handled upstream by the read-modify-write + /// wrapper which performs pmc_rdpmcrng + bit mask + this method on a + /// per-byte semaphore (so two concurrent bit writes against the same byte don't + /// lose one another's update). + /// Default impl returns so + /// transport variants that haven't yet routed the write keep compiling — those + /// variants surface BadNotSupported on PMC writes until the wire client is + /// extended. + /// + Task WritePmcRangeAsync( + string letter, int pathId, int startByte, byte[] bytes, CancellationToken cancellationToken) + => Task.FromResult(FocasStatusMapper.BadNotSupported); + + /// + /// Read-modify-write one bit within a PMC byte (Plan PR F4-c, issue #270). The + /// wire call pmc_wrpmcrng is byte-addressed, so the driver reads the + /// parent byte first, masks the target bit, then writes the byte back. Default + /// impl uses + + /// so transport variants get correct RMW semantics for free; the FWLIB-backed + /// client overrides this with a per-byte semaphore so two concurrent bit writes + /// against the same byte serialise. + /// + async Task WritePmcBitAsync( + string letter, int pathId, int byteAddress, int bitIndex, bool newValue, + CancellationToken cancellationToken) + { + if (bitIndex is < 0 or > 7) return FocasStatusMapper.BadOutOfRange; + var (buf, status) = await ReadPmcRangeAsync(letter, pathId, byteAddress, 1, cancellationToken) + .ConfigureAwait(false); + if (status != FocasStatusMapper.Good || buf is null || buf.Length < 1) return status; + var current = buf[0]; + var updated = newValue + ? (byte)(current | (1 << bitIndex)) + : (byte)(current & ~(1 << bitIndex)); + return await WritePmcRangeAsync(letter, pathId, byteAddress, new[] { updated }, cancellationToken) + .ConfigureAwait(false); + } + /// /// Read a contiguous range of PMC bytes in a single wire call (FOCAS /// pmc_rdpmcrng with byte data type) for the given 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 6469abe..247ea89 100644 --- a/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FakeFocasClient.cs +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FakeFocasClient.cs @@ -91,6 +91,44 @@ internal class FakeFocasClient : IFocasClient return Task.FromResult(status); } + /// + /// Plan PR F4-c (issue #270) — typed PMC range-write entry point. Records + /// the call in and applies the bytes to + /// at (letter, pathId) so a subsequent + /// sees the updated bytes (round-trip + /// shape). Status looked up by the canonical PMC address (e.g. R100) + /// of the first byte if seeded; otherwise Good. + /// + public List<(string Letter, int PathId, int StartByte, byte[] Bytes)> PmcRangeWriteLog { get; } = new(); + + public virtual Task WritePmcRangeAsync( + string letter, int pathId, int startByte, byte[] bytes, CancellationToken ct) + { + if (ThrowOnWrite) throw Exception ?? new InvalidOperationException(); + var copy = bytes.ToArray(); + PmcRangeWriteLog.Add((letter, pathId, startByte, copy)); + // Persist into PmcByteRanges so subsequent range reads see the write — this + // mirrors the simulator round-trip the integration tests check. + var key = (letter.ToUpperInvariant(), pathId); + if (!PmcByteRanges.TryGetValue(key, out var src)) + { + src = new byte[startByte + copy.Length]; + PmcByteRanges[key] = src; + } + else if (src.Length < startByte + copy.Length) + { + var grown = new byte[startByte + copy.Length]; + Array.Copy(src, 0, grown, 0, src.Length); + src = grown; + PmcByteRanges[key] = src; + } + Array.Copy(copy, 0, src, startByte, copy.Length); + // Status seeded by canonical PMC address of the first byte (no bit index). + var canonical = $"{letter.ToUpperInvariant()}{startByte}"; + var status = WriteStatuses.TryGetValue(canonical, out var sx) ? sx : 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/FocasPmcBitRmwTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasPmcBitRmwTests.cs index ffabc79..674a7d3 100644 --- a/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasPmcBitRmwTests.cs +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasPmcBitRmwTests.cs @@ -9,43 +9,32 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests; public sealed class FocasPmcBitRmwTests { /// - /// Fake client simulating PMC byte storage + exposing it as a sbyte so RMW callers can - /// observe the read-modify-write round-trip. ReadAsync for a Bit with bitIndex surfaces - /// the current bit; WriteAsync stores the full byte the driver issues. + /// Fake client simulating PMC byte storage as a single 1024-byte buffer. Post-F4-c + /// (issue #270) the FOCAS driver routes PMC writes through the typed + /// + + /// entry points — the bit path performs RMW via ReadPmcRangeAsync + + /// WritePmcRangeAsync, so this fake overrides those to drive a shared + /// buffer the tests can assert against. + /// is the unit-test surface; we mirror writes to + /// too so any helper that reads from there sees the same source of truth. /// private sealed class PmcRmwFake : FakeFocasClient { public byte[] PmcBytes { get; } = new byte[1024]; - public override Task<(object? value, uint status)> ReadAsync( - FocasAddress address, FocasDataType type, CancellationToken ct) + public override Task<(byte[]? buffer, uint status)> ReadPmcRangeAsync( + string letter, int pathId, int startByte, int byteCount, CancellationToken ct) { - if (address.Kind == FocasAreaKind.Pmc && type == FocasDataType.Byte) - return Task.FromResult(((object?)(sbyte)PmcBytes[address.Number], FocasStatusMapper.Good)); - if (address.Kind == FocasAreaKind.Pmc && type == FocasDataType.Bit && address.BitIndex is int bit) - return Task.FromResult(((object?)((PmcBytes[address.Number] & (1 << bit)) != 0), FocasStatusMapper.Good)); - return base.ReadAsync(address, type, ct); + var buf = new byte[byteCount]; + Array.Copy(PmcBytes, startByte, buf, 0, byteCount); + return Task.FromResult<(byte[]?, uint)>((buf, FocasStatusMapper.Good)); } - public override Task WriteAsync( - FocasAddress address, FocasDataType type, object? value, CancellationToken ct) + public override Task WritePmcRangeAsync( + string letter, int pathId, int startByte, byte[] bytes, CancellationToken ct) { - // Driver writes the full byte after RMW (type==Byte with full byte value), OR a raw - // bit write (type==Bit, bitIndex non-null) — depending on how the driver routes it. - if (address.Kind == FocasAreaKind.Pmc && type == FocasDataType.Byte) - { - PmcBytes[address.Number] = (byte)Convert.ToSByte(value); - return Task.FromResult(FocasStatusMapper.Good); - } - if (address.Kind == FocasAreaKind.Pmc && type == FocasDataType.Bit && address.BitIndex is int bit) - { - var current = PmcBytes[address.Number]; - PmcBytes[address.Number] = Convert.ToBoolean(value) - ? (byte)(current | (1 << bit)) - : (byte)(current & ~(1 << bit)); - return Task.FromResult(FocasStatusMapper.Good); - } - return base.WriteAsync(address, type, value, ct); + Array.Copy(bytes, 0, PmcBytes, startByte, bytes.Length); + return Task.FromResult(FocasStatusMapper.Good); } } @@ -64,7 +53,7 @@ public sealed class FocasPmcBitRmwTests Devices = [new FocasDeviceOptions("focas://10.0.0.5:8193")], Tags = writableTags, Probe = new FocasProbeOptions { Enabled = false }, - Writes = new FocasWritesOptions { Enabled = true }, + Writes = new FocasWritesOptions { Enabled = true, AllowPmc = true }, }, "drv-1", factory); return (drv, fake); } diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasReadWriteTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasReadWriteTests.cs index b3a8f66..6b60d99 100644 --- a/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasReadWriteTests.cs +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasReadWriteTests.cs @@ -17,9 +17,10 @@ public sealed class FocasReadWriteTests Tags = tags, Probe = new FocasProbeOptions { Enabled = false }, // F4-a flipped Writable + Writes.Enabled defaults to false for safer-by-default - // posture (issue #268). The legacy read-write test fixture opts both back on so - // existing assertions exercise the same wire path the original tests covered. - Writes = new FocasWritesOptions { Enabled = true }, + // posture (issue #268). F4-c added AllowPmc on the same shape (issue #270). The + // legacy read-write test fixture opts everything back on so existing assertions + // exercise the same wire path the original tests covered. + Writes = new FocasWritesOptions { Enabled = true, AllowPmc = true }, }, "drv-1", factory); return (drv, factory); } @@ -218,7 +219,7 @@ public sealed class FocasReadWriteTests new FocasTagDefinition("B", "focas://10.0.0.5:8193", "R101", FocasDataType.Byte, Writable: false), ], Probe = new FocasProbeOptions { Enabled = false }, - Writes = new FocasWritesOptions { Enabled = true }, + Writes = new FocasWritesOptions { Enabled = true, AllowPmc = true }, }, "drv-1", factory); await drv.InitializeAsync("{}", CancellationToken.None); diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasWriteInfrastructureTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasWriteInfrastructureTests.cs index 940c315..4800418 100644 --- a/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasWriteInfrastructureTests.cs +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasWriteInfrastructureTests.cs @@ -69,10 +69,11 @@ public sealed class FocasWriteInfrastructureTests public async Task DriverLevel_Writes_enabled_per_tag_Writable_true_dispatches_to_wire_client() { // F4-a's wire dispatch surface is unchanged — when both flags are flipped, the call - // reaches the (fake) wire client, which by default returns Good. F4-b will introduce - // BadNotSupported branches for kinds the wire layer hasn't implemented yet. + // reaches the (fake) wire client, which by default returns Good. F4-b/F4-c add per-kind + // gates (AllowParameter / AllowMacro / AllowPmc); PMC byte writes route through the + // typed WritePmcRangeAsync entry point post-F4-c so we assert on PmcRangeWriteLog. var drv = NewDriver( - writes: new FocasWritesOptions { Enabled = true }, + writes: new FocasWritesOptions { Enabled = true, AllowPmc = true }, tags: [ new FocasTagDefinition("X", "focas://10.0.0.5:8193", "R100", FocasDataType.Byte, Writable: true), @@ -84,7 +85,7 @@ public sealed class FocasWriteInfrastructureTests [new WriteRequest("X", (sbyte)1)], CancellationToken.None); results.Single().StatusCode.ShouldBe(FocasStatusMapper.Good); - factory.Clients[0].WriteLog.Count.ShouldBe(1); + factory.Clients[0].PmcRangeWriteLog.Count.ShouldBe(1); } [Fact] diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasWriteMacroTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasWriteMacroTests.cs index 9ca0809..e7021ac 100644 --- a/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasWriteMacroTests.cs +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasWriteMacroTests.cs @@ -170,17 +170,20 @@ public sealed class FocasWriteMacroTests } [Fact] - public async Task Per_kind_gate_does_not_affect_PMC_writes() + public async Task Per_kind_gate_does_not_cross_contaminate_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. + // (gated by F4-c's AllowPmc) keep flowing when the operator opted in to PMC + // alone. Pre-F4-c this test asserted PMC needed no per-kind gate; post-F4-c + // it asserts AllowPmc is the gate that matters for PMC, independent of the + // PARAM/MACRO gates. var drv = NewDriver( writes: new FocasWritesOptions { Enabled = true, AllowParameter = false, AllowMacro = false, + AllowPmc = true, }, tags: [ @@ -193,8 +196,8 @@ public sealed class FocasWriteMacroTests [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); + // PMC routes through the typed WritePmcRangeAsync entry point post-F4-c. + factory.Clients[0].PmcRangeWriteLog.Count.ShouldBe(1); factory.Clients[0].ParameterWriteLog.ShouldBeEmpty(); factory.Clients[0].MacroWriteLog.ShouldBeEmpty(); } diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasWritePmcTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasWritePmcTests.cs new file mode 100644 index 0000000..d8191be --- /dev/null +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasWritePmcTests.cs @@ -0,0 +1,292 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests; + +/// +/// Issue #270, plan PR F4-c — pmc_wrpmcrng coverage. The driver-level +/// Writes.AllowPmc kill switch sits on top of the F4-a +/// Writes.Enabled + per-tag Writable opt-ins. PMC bit writes +/// additionally exercise the read-modify-write helper (pmc_wrpmcrng is +/// byte-addressed; the wire never sees a sub-byte write). PMC tags surface +/// for the server-layer ACL gate. +/// +[Trait("Category", "Unit")] +public sealed class FocasWritePmcTests +{ + 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); + } + + /// + /// Variant that pre-seeds a fake's PMC byte storage (so an RMW test can verify + /// the read-side picked up the prior byte before the bit mask). The customiser + /// fires once per device — sufficient for the single-device tests below. + /// + private static FocasDriver NewDriverWithSeededPmc( + FocasWritesOptions writes, + FocasTagDefinition[] tags, + string letter, + int pathId, + byte[] seed, + out FakeFocasClientFactory factory) + { + factory = new FakeFocasClientFactory + { + Customise = () => + { + var c = new FakeFocasClient(); + c.PmcByteRanges[(letter.ToUpperInvariant(), pathId)] = (byte[])seed.Clone(); + return c; + }, + }; + 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 AllowPmc_false_returns_BadNotWritable_even_with_Enabled_and_Writable() + { + // F4-c — the granular kill switch defaults off so a redeployed driver with + // Writes.Enabled=true still keeps PMC writes locked until the operator team + // explicitly opts in. PMC is ladder working memory; defense-in-depth is + // critical because a mistargeted bit can move motion or latch a feedhold. + var drv = NewDriver( + writes: new FocasWritesOptions { Enabled = true, AllowPmc = false }, + tags: + [ + new FocasTagDefinition("Coil", Host, "R100", FocasDataType.Byte, Writable: true), + ], + out var factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + var results = await drv.WriteAsync( + [new WriteRequest("Coil", (sbyte)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. + factory.Clients.ShouldBeEmpty(); + } + + [Fact] + public async Task AllowPmc_true_byte_write_dispatches_to_typed_WritePmcRangeAsync() + { + var drv = NewDriver( + writes: new FocasWritesOptions { Enabled = true, AllowPmc = true }, + tags: + [ + new FocasTagDefinition("Coil", Host, "R100", FocasDataType.Byte, Writable: true), + ], + out var factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + var results = await drv.WriteAsync( + [new WriteRequest("Coil", (sbyte)42)], CancellationToken.None); + + results.Single().StatusCode.ShouldBe(FocasStatusMapper.Good); + var log = factory.Clients[0].PmcRangeWriteLog; + log.Count.ShouldBe(1); + log[0].Letter.ShouldBe("R"); + log[0].StartByte.ShouldBe(100); + log[0].Bytes.ShouldBe(new byte[] { 42 }); + // Generic WriteAsync path is untouched — PMC byte goes through the typed entry point. + factory.Clients[0].WriteLog.ShouldBeEmpty(); + } + + [Fact] + public async Task PMC_bit_write_set_RMW_preserves_zero_byte_writes_only_target_bit() + { + // Prior byte = 0b0000_0000; set bit 3 → write byte = 0b0000_1000. + var drv = NewDriverWithSeededPmc( + writes: new FocasWritesOptions { Enabled = true, AllowPmc = true }, + tags: [new FocasTagDefinition("G50_3", Host, "G50.3", FocasDataType.Bit, Writable: true)], + letter: "G", pathId: 1, + seed: PmcBuffer(byteAddr: 50, value: 0b0000_0000), + out var factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + var results = await drv.WriteAsync( + [new WriteRequest("G50_3", true)], CancellationToken.None); + + results.Single().StatusCode.ShouldBe(FocasStatusMapper.Good); + var log = factory.Clients[0].PmcRangeWriteLog.Single(); + log.Letter.ShouldBe("G"); + log.StartByte.ShouldBe(50); + log.Bytes.ShouldBe(new byte[] { 0b0000_1000 }); + } + + [Fact] + public async Task PMC_bit_write_set_preserves_other_bits_already_set() + { + // Prior byte = 0b1111_0000; set bit 0 → write byte = 0b1111_0001. + var drv = NewDriverWithSeededPmc( + writes: new FocasWritesOptions { Enabled = true, AllowPmc = true }, + tags: [new FocasTagDefinition("R50_0", Host, "R50.0", FocasDataType.Bit, Writable: true)], + letter: "R", pathId: 1, + seed: PmcBuffer(byteAddr: 50, value: 0b1111_0000), + out var factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + await drv.WriteAsync([new WriteRequest("R50_0", true)], CancellationToken.None); + + var log = factory.Clients[0].PmcRangeWriteLog.Single(); + log.Bytes.ShouldBe(new byte[] { 0b1111_0001 }); + } + + [Fact] + public async Task PMC_bit_write_clear_preserves_other_bits() + { + // Prior byte = 0b1111_1111; clear bit 0 → write byte = 0b1111_1110. + var drv = NewDriverWithSeededPmc( + writes: new FocasWritesOptions { Enabled = true, AllowPmc = true }, + tags: [new FocasTagDefinition("R50_0", Host, "R50.0", FocasDataType.Bit, Writable: true)], + letter: "R", pathId: 1, + seed: PmcBuffer(byteAddr: 50, value: 0xFF), + out var factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + await drv.WriteAsync([new WriteRequest("R50_0", false)], CancellationToken.None); + + var log = factory.Clients[0].PmcRangeWriteLog.Single(); + log.Bytes.ShouldBe(new byte[] { 0b1111_1110 }); + } + + [Fact] + public async Task Multiple_consecutive_bit_writes_in_same_byte_serialise() + { + // Each bit write does its own RMW (Read range -> mask -> Write range). Eight + // consecutive bit-set writes on R100 starting from 0 must compose to 0xFF. + var tags = Enumerable.Range(0, 8) + .Select(b => new FocasTagDefinition($"Bit{b}", Host, $"R100.{b}", FocasDataType.Bit, Writable: true)) + .ToArray(); + var drv = NewDriverWithSeededPmc( + writes: new FocasWritesOptions { Enabled = true, AllowPmc = true }, + tags: tags, + letter: "R", pathId: 1, + seed: PmcBuffer(byteAddr: 100, value: 0), + out var factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + for (var b = 0; b < 8; b++) + await drv.WriteAsync([new WriteRequest($"Bit{b}", true)], CancellationToken.None); + + var fake = factory.Clients[0]; + // 8 RMW round-trips, each writing the cumulative byte back. + fake.PmcRangeWriteLog.Count.ShouldBe(8); + fake.PmcByteRanges[("R", 1)][100].ShouldBe((byte)0xFF); + // Every write hit the same byte address 100. + fake.PmcRangeWriteLog.ShouldAllBe(e => e.StartByte == 100 && e.Bytes.Length == 1); + } + + [Fact] + public void Tag_classification_PMC_writable_yields_Operate() + { + // Server-layer ACL key — PMC tags require WriteOperate group membership + // (mirrors MACRO; PARAM is the higher-friction Configure tier). + var tag = new FocasTagDefinition( + "Coil", Host, "R100.3", FocasDataType.Bit, Writable: true); + + FocasDriver.ClassifyTag(tag).ShouldBe(SecurityClassification.Operate); + } + + [Fact] + public void Tag_classification_PMC_non_writable_yields_ViewOnly() + { + var tag = new FocasTagDefinition( + "Coil", Host, "R100.3", FocasDataType.Bit, Writable: false); + + FocasDriver.ClassifyTag(tag).ShouldBe(SecurityClassification.ViewOnly); + } + + [Fact] + public void FocasWritesOptions_default_AllowPmc_is_false() + { + // Defense in depth: a fresh FocasWritesOptions has every granular kill + // switch off. A config row that omits AllowPmc must NOT silently flip PMC + // writes on. + new FocasWritesOptions().AllowPmc.ShouldBeFalse(); + new FocasDriverOptions().Writes.AllowPmc.ShouldBeFalse(); + } + + [Fact] + public void Dto_round_trip_preserves_AllowPmc() + { + // JSON config -> FocasDriverOptions; the Writes.AllowPmc flag must survive + // the bootstrapper's deserialize step. Sentinel: a write at a configured + // PMC tag should NOT short-circuit to BadNotWritable when AllowPmc=true is + // set in the JSON (the unimplemented backend will surface BadCommunicationError + // instead). + const string jsonAllowed = """ + { + "Backend": "unimplemented", + "Devices": [{ "HostAddress": "focas://10.0.0.5:8193" }], + "Tags": [{ + "Name": "P", "DeviceHostAddress": "focas://10.0.0.5:8193", + "Address": "R100", "DataType": "Byte", "Writable": true + }], + "Writes": { "Enabled": true, "AllowPmc": true } + } + """; + + var drv = FocasDriverFactoryExtensions.CreateInstance("drv-1", jsonAllowed); + drv.InitializeAsync("{}", CancellationToken.None).GetAwaiter().GetResult(); + + var results = drv.WriteAsync( + [new WriteRequest("P", (sbyte)1)], CancellationToken.None).GetAwaiter().GetResult(); + // Key assertion: NOT BadNotWritable — that proves the AllowPmc gate didn't short-circuit. + results.Single().StatusCode.ShouldNotBe(FocasStatusMapper.BadNotWritable); + } + + [Fact] + public void Dto_default_omitted_AllowPmc_keeps_safer_default() + { + // A Writes section with just { Enabled: true } must NOT silently flip the + // granular kill switch on. PMC 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": "R100", "DataType": "Byte", "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", (sbyte)1)], CancellationToken.None).GetAwaiter().GetResult(); + results.Single().StatusCode.ShouldBe(FocasStatusMapper.BadNotWritable); + } + + private static byte[] PmcBuffer(int byteAddr, byte value) + { + // Allocate enough buffer to hold the byteAddr index, fill the chosen byte. + var buf = new byte[byteAddr + 1]; + buf[byteAddr] = value; + return buf; + } +}