[F34 partial] mxaccess-asb: fix collect_asbidata_payloads + add Active flag
rust / build / test / clippy / fmt (push) Has been cancelled

Investigation via examples/asb-relay.rs middleman captured the full
S→C bytes of a working PublishResponse from the .NET probe against
MxDataProvider. Decoder fix verified by regression test against the
captured fixture; one further wire-format gap surfaced and is filed.

Closed in this commit:

1. collect_asbidata_payloads filtered out empty <ASBIData/> elements
   so positional payload[N] indexing collapsed when Status was
   empty-but-present. The wire form for PublishResponse is:
     <Status><ASBIData/></Status>          ← empty placeholder
     <Values><ASBIData>{bytes}</ASBIData></Values>
   Our decoder lost the positional info and read Values as Status,
   then panicked on the malformed parse. Fix: always push every
   <ASBIData> element (empty or not) so payloads[0]=Status and
   payloads[1]=Values stay aligned. New regression test
   tests/publish_capture.rs runs the full decode chain over the
   captured wire bytes (305-byte frame at
   tests/fixtures/publish-response-with-value.bin) and asserts
   values.len() == 1.

2. MinimalMonitoredItem.active: Option<bool> + new with_active()
   constructor. The .NET reference's MxAsbDataClient.AddMonitoredItems
   defaults to active: true (cs:441). Without <Active>true</Active>
   on the wire, MxDataProvider treats the subscription as inactive
   and Publish polls return empty Values. Both binary build and
   canonical XML emitters now conditionally emit <Active> when
   active.is_some(). Shared push_monitored_item_body helper
   eliminates the duplicate MonitoredItem encoder between
   AddMonitoredItems and DeleteMonitoredItems builders.

3. SampleInterval unit: clarified as **milliseconds** in
   MinimalMonitoredItem.sample_interval doc + the example
   (sample_interval_ticks → sample_interval_ms = 1000). Matches the
   .NET reference's `ulong sampleInterval = 1000` default.

Open: F34's deeper finding — `MonitoredItem`'s wire schema is
DataContract field-suffix names (`activeField`, `bufferedField`,
`itemField`, `sampleIntervalField`, etc., per the per-session NBFX
dictionary the .NET probe declares), NOT XmlSerializer property
names (`Active`, `Buffered`, `Item`, `SampleInterval`). Our binary
NBFX builder still uses the property names, so MxDataProvider
silently fails to register monitored items — successField=true with
a 0-length Status array. The fix needs a complete rebuild of
build_add_monitored_items_request_body and
build_delete_monitored_items_request_body to use the field-suffix
names plus emit the *Specified siblings (activeFieldSpecified,
idFieldSpecified, etc.) as their own elements. The HMAC canonical
XML side is unaffected (XmlSerializer naming is correct there;
verified byte-equal to .NET via the F28 fixtures). Detailed in
design/followups.md F34's "Open" section.

Live verification of the F34-partial bonus context:
  - Read still returns 99 end-to-end via canonical XML signing.
  - AddMonitoredItems still returns Status[0] = 0 items
    (server doesn't recognize our DataContract-misnamed payload).
  - Publish still returns 0 values (the F34-open consequence).
  - All other 13 canonical-XML signed ops succeed at the request
    level (no SOAP faults, no HMAC rejections).

Workspace: mxaccess-asb 95 → 96 (+1 capture-driven decoder test);
default-feature clippy clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Joseph Doherty
2026-05-06 02:49:11 -04:00
parent 0771664092
commit fb40e4c20b
6 changed files with 212 additions and 81 deletions
+22 -3
View File
@@ -74,9 +74,28 @@ move to `## Resolved` with a date + commit hash.
For the per-step body of every line listed in the cumulative execution log, see the matching commit message — each commit is a single F-number step with its own scope, fixtures, test count delta, and follow-up notes. The detailed per-step write-ups previously inlined here added little beyond what `git show <hash>` provides.
### F34 — Publish response: decoder reads 0 values where .NET sees real values
**Severity:** P2 — only affects the F26 stream's data flow against MxDataProvider; the canonical-XML signing for Publish itself is verified working (server accepts the request, returns a non-fault response).
**Source:** Live `cargo run -p mxaccess --example asb-subscribe` against the local AVEVA install, 2026-05-06.
### F34 — `MonitoredItem` wire format: DataContract field-suffix names, not XmlSerializer property names
**Severity:** P2 — only affects the F26 stream's data flow against MxDataProvider; canonical-XML HMAC signing for the operation is verified working (server accepts the request, returns a non-fault response).
**Source:** Live `cargo run -p mxaccess --example asb-subscribe` + `examples/asb-relay.rs` capture, 2026-05-06.
**Two sub-issues, one closed and one open.**
**Closed: `decode_publish_response` filtered empty `<ASBIData/>` placeholders out of the positional payload list.** Captured the full S→C bytes of a working `PublishResponse` via `examples/asb-relay.rs` between the .NET probe and MxDataProvider (fixture stashed at `crates/mxaccess-asb/tests/fixtures/publish-response-with-value.bin`). The wire shape is `<Status><ASBIData/></Status><Values><ASBIData>{bytes}</ASBIData></Values>` — Status is empty-but-present, Values carries the binary `MonitoredItemValue[]`. Our `collect_asbidata_payloads` previously skipped the empty Status, shifting Values down to index `0` where the decoder mis-read it as Status and corrupted the parse. Fix: always push every `<ASBIData>` element as a positional entry, empty or not. `tests/publish_capture.rs` runs the full decode chain over the real wire bytes and asserts `values.len() == 1`. **Verified 2026-05-06.**
**Open: AddMonitoredItems / DeleteMonitoredItems request bodies use the wrong element-name form on the binary NBFX wire.** Live capture of the .NET probe's `AddMonitoredItems` request exposes a per-session NBFX dictionary declaring these strings *(verbatim, in declaration order)*: `activeField`, `activeFieldSpecified`, `bufferedField`, `itemField`, `contextNameField`, `idField`, `idFieldSpecified`, `nameField`, `referenceTypeField`, `typeField`, `sampleIntervalField`, `timeDeadbandField`, `timeDeadbandFieldSpecified`, `userDataField`, `lengthField`, `payloadField`, `valueDeadbandField`. These are the `[DataMember(Name = "...")]` private-field names from `AsbContracts.cs:940-965` — the wire form chosen by `DataContractSerializer` for non-`IAsbCustomSerializableType` types like `MonitoredItem`. Our `build_add_monitored_items_request_body` (and `build_delete_monitored_items_request_body`) emits XmlSerializer-property names like `<Active>`, `<Buffered>`, `<Item>`, `<SampleInterval>` — those are the *canonical XML for HMAC* shape (XmlSerializer-derived), which is correct for the signing input but wrong for the binary wire payload. MxDataProvider silently fails to register monitored items whose field names don't match its DataContract schema, returns a 0-length `Status` array (`successField=true`, `resultCode=0`, but no items actually registered), and consequently the `Publish` poll loop forever returns empty `Values`.
**The dual-format world**: ASB requests have *two* element-name conventions on the wire:
- **HMAC canonical XML** (input to `AsbAuthenticator::Sign`): XmlSerializer-derived names — `<Active>`, `<Items>`, `<MonitoredItem>` etc. Driven by `[XmlElement(...)]` and property names. Our `xml_canonical` emitter is byte-equal to .NET here (F28 step 2 fixtures verify).
- **Binary NBFX body** (the actual wire request): DataContractSerializer-derived names — `<activeField>`, `<bufferedField>`, etc. Driven by private-field `[DataMember(Name = "...")]`. Our builders for AddMonitoredItems / DeleteMonitoredItems are wrong here.
For ops where the body is purely `IAsbCustomSerializableType` arrays (Read, Register, Unregister), no DataContract names appear — every payload is wrapped as `<Items><ASBIData>{bytes}</ASBIData></Items>` (binary fast-path) and our builders are correct. The DataContract schema only matters for ops carrying non-`IAsbCustomSerializable` types like `MonitoredItem` and `WriteValue`.
**Resolves when:** `build_add_monitored_items_request_body` and `build_delete_monitored_items_request_body` rewrite each `MonitoredItem` child as the DataContract field-suffix names (`activeField` instead of `Active`, etc.), with the `*Specified` siblings emitted as their own elements (`activeFieldSpecified`, `idFieldSpecified`, etc.). The dictionary-id pre-population that .NET's WCF binary writer uses to compress these long strings is a perf optimisation; an inline-string emit will work for correctness. Likely the same fix applies to `WriteBasicRequest`'s `WriteValue[]? Values` field (also non-`IAsbCustomSerializable`) — that's a future capture-and-verify pass.
**Bonus context discovered while debugging F34:**
- `MinimalMonitoredItem` gained an `active: Option<bool>` field with a `with_active(item, interval, active)` constructor. Without `<Active>true</Active>` on the wire, MxDataProvider treats the subscription as inactive even when AddMonitoredItems "succeeds" — F26 stream then never sees values. (Once the field-name fix lands, this becomes the determining factor for whether values flow.)
- `SampleInterval` unit corrected from "100-ns ticks" to **milliseconds** in the example + the `MinimalMonitoredItem.sample_interval` doc — matches `MxAsbDataClient.cs:441`'s `ulong sampleInterval = 1000` default.
- `result_code = 32` is `AsbErrorCode.PublishComplete` (`AsbResultMapping.cs:37`), informational not fatal — `ToResult:122-129` treats it like `Success`. F26 stream's `publish_loop` narrowed to bail only on `RESULT_CODE_INVALID_CONNECTION_ID = 1`.
**Evidence.** Side-by-side comparison after the .NET probe and Rust client both ran `--subscribe` against the same `TestChildObject.TestInt` tag: