From da33f27544cc942ba34ded38de7602162c6516c3 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 8 Oct 2026 14:25:45 -0400 Subject: [PATCH 1/2] feat(mtconnect): serve TIME_SERIES observations as variable-length double[] (#627) Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- docs/drivers/MTConnect.md | 42 +++++-- .../MTConnectDtos.cs | 13 ++- .../MTConnectObservationIndex.cs | 51 +++++++-- .../MTConnectStreamsParser.cs | 24 +++- .../Uns/TagEditors/MTConnectTagConfigModel.cs | 17 ++- .../MTConnectObservationIndexTests.cs | 104 ++++++++++++++---- .../MTConnectReadTests.cs | 7 +- .../Uns/MTConnectTagConfigModelTests.cs | 11 ++ 8 files changed, 229 insertions(+), 40 deletions(-) diff --git a/docs/drivers/MTConnect.md b/docs/drivers/MTConnect.md index 1068984b..a7659305 100644 --- a/docs/drivers/MTConnect.md +++ b/docs/drivers/MTConnect.md @@ -230,10 +230,37 @@ Other status codes an observation can surface: |---|---| | `BadTypeMismatch` (`0x80740000`) | The Agent's text isn't a value of the tag's coerced type at all | | `BadOutOfRange` (`0x803C0000`) | The Agent reported a number the coerced type can't represent | -| `BadNotSupported` (`0x803D0000`) | A shape this build doesn't materialize — a `TIME_SERIES` vector, or a structured `DATA_SET`/`TABLE` observation (deferred to P1.5; see [Known limitations](#known-limitations)) | +| `BadDecodingError` (`0x80070000`) | A `TIME_SERIES` vector whose element count disagrees with the observation's own `sampleCount` (or the tag's fixed `ArrayDim`) — see [TIME_SERIES](#time_series-arrays-627) | +| `BadNotSupported` (`0x803D0000`) | A shape this build doesn't materialize — a structured `DATA_SET`/`TABLE` observation (decided: stays unsupported, #627), or an array tag declared as anything other than Float64 | | `BadWaitingForInitialData` | Tag is authored but the Agent hasn't reported it yet | | `BadNodeIdUnknown` | DataItem id is neither authored nor ever observed | +### TIME_SERIES arrays (#627) + +A `SAMPLE` DataItem with `representation="TIME_SERIES"` is served as a +**variable-length `double[]`** (`ValueRank = 1`, no fixed `ArrayDimensions` — +a live Agent never declares `sampleCount` on the DataItem, see Known +limitations 4; a probe that does declare one yields a fixed `ArrayDim`). +Parsing: the observation text is split on any whitespace, each token parsed +invariant-culture. The result is **all-or-nothing** — one non-numeric / +non-finite token is `BadTypeMismatch`, a literal `UNAVAILABLE` token or an +empty body is `BadNoCommunication`, and a count that disagrees with the +observation's own `sampleCount` attribute (or a fixed `ArrayDim`) is +`BadDecodingError`. A partial vector is never served as Good. Only a +**Float64** array tag is served; any other declared element type stays +`BadNotSupported`. An authored tag needs `"isArray": true` and +`"dataType": "Float64"` in its TagConfig (the AdminUI editor shows an advisory +for any other array type; a browse-commit does not stamp `isArray`, so set it +in the Tag modal's array section). + +**Sample rate is not exposed on the served node.** The observation's +`sampleRate` and `sampleCount` attributes are parsed (`MTConnectObservation`) +but the OPC UA node has no description/property slot through `DriverAttributeInfo`, +and a sibling property node would need a new address-space mechanism. Decision: +document it — the rate is a property of the Agent's DataItem (`sampleRate` in +`/probe`), read it there. **`DATA_SET` / `TABLE` stay `BadNotSupported`** (decision, #627): they are +key/value maps with no scalar or array OPC UA shape that preserves the keys. + ### Subscribe — the `/sample` pump One shared `/sample` long-poll stream per driver instance (the Agent streams @@ -410,7 +437,7 @@ for anything beyond values-and-conditions. `/probe` metadata a browse-commit stamped (`mtCategory`/`mtType`), and shows an advisory line — it never blocks a save (the deploy gate does not refuse it) and is a no-op for hand-authored blobs with no stamped metadata. `representation` - is not on the model, so TIME_SERIES arrays are not covered. Beyond that, + is not on the model; TIME_SERIES arrays get their own `isArray` advisory (declare Float64). Beyond that, nothing validates a raw tag's declared OPC UA `DataType` against the blob's coercion type. The `/probe`-sourced inference (`MTConnectDataTypeInference`) narrows how often an author picks a wrong @@ -427,9 +454,11 @@ for anything beyond values-and-conditions. declare a `sampleCount`, is unrealistic on this point and exists only to pin the parsing rule itself. 5. **CONDITION is a plain `String`, not a native Part 9 alarm** (see above). -6. **`TIME_SERIES` arrays and structured `DATA_SET`/`TABLE` observations - surface as `BadNotSupported`**, not an approximation — see the Quality - mapping table. +6. **Structured `DATA_SET`/`TABLE` observations surface as + `BadNotSupported`**, not an approximation — see the Quality mapping table. + `TIME_SERIES` is served as `double[]` (#627, see + [TIME_SERIES arrays](#time_series-arrays-627)); the sample rate is not + surfaced on the node. ## Deferred (not in this build) @@ -447,8 +476,7 @@ for anything beyond values-and-conditions. - **P1.5 fast-follow** (design [§9](../plans/2026-07-15-mtconnect-driver-design.md#9-phasing--effort)): CONDITION → native OPC UA Part 9 alarms via `IAlarmSource` (the Galaxy - native-alarm pattern); `TIME_SERIES` SAMPLE arrays materialized as real - OPC UA arrays; EVENT controlled-vocabulary values → OPC UA enumerations. + native-alarm pattern); EVENT controlled-vocabulary values → OPC UA enumerations. - **P2 (on demand): SHDR adapter ingest.** A `SourceMode: "Agent" | "Shdr"` switch that opens the raw pipe-delimited SHDR TCP socket directly. Loses auto-discovery (no device model without an Agent in front, so diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDtos.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDtos.cs index 740a2d71..0158722b 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDtos.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDtos.cs @@ -158,11 +158,22 @@ public sealed record MTConnectStreamsResult( /// false, the shape of every ordinary scalar observation. /// /// +/// +/// The observation's own sampleCount attribute — the number of values a TIME_SERIES +/// vector claims to carry (#627). The index checks the parsed vector against it. null when absent. +/// +/// +/// The observation's own sampleRate attribute (samples per second) on a TIME_SERIES +/// observation. Carried for the parser contract; not yet surfaced on the served node (see +/// docs/drivers/MTConnect.md). null when absent. +/// public sealed record MTConnectObservation( string DataItemId, string Value, DateTime TimestampUtc, - bool IsStructured = false); + bool IsStructured = false, + int? SampleCount = null, + double? SampleRate = null); /// /// Recursive helpers over the device/component tree. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectObservationIndex.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectObservationIndex.cs index c68a870c..7e95659d 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectObservationIndex.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectObservationIndex.cs @@ -49,8 +49,8 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.MTConnect; /// /// /// Shapes this build does not represent are coded BadNotSupported with a null -/// value rather than approximated: a TIME_SERIES vector (array materialization is -/// deferred to P1.5) and a DATA_SET / TABLE observation. The latter carries its +/// value rather than approximated: a non-Float64 array tag and a DATA_SET / TABLE +/// observation (a Float64 TIME_SERIES vector is served as double[], #627). The latter carries its /// real content in <Entry key=…> child elements, which reach a text-valued /// observation concatenated with the keys discarded — a two-entry set reads as the single /// token "12" — and because the type inference correctly demotes those @@ -85,9 +85,12 @@ internal sealed class MTConnectObservationIndex /// The Agent reported a number the authored type cannot represent. private const uint BadOutOfRange = 0x803C0000u; - /// The observation's shape is real data this build cannot represent (TIME_SERIES vectors). + /// The observation's shape is real data this build cannot represent (a DATA_SET/TABLE, or a non-Float64 array tag). private const uint BadNotSupported = 0x803D0000u; + /// A TIME_SERIES vector whose element count disagrees with its declared sampleCount. + private const uint BadDecodingError = 0x80070000u; + /// The Agent reported text that is not a value of the authored type at all. private const uint BadTypeMismatch = 0x80740000u; @@ -268,13 +271,11 @@ internal sealed class MTConnectObservationIndex _tags.TryGetValue(observation.DataItemId, out var tag); - // TIME_SERIES vectors arrive as one space-separated string. P1 does not materialize OPC UA - // arrays (deferred to P1.5), and the honest answer is "real data I cannot represent" — not a - // type mismatch, and emphatically not the first element parsed out and served as Good. - // Checked AFTER the sentinel so an unavailable array tag still reports the truthful no-comms. + // TIME_SERIES vectors arrive as one whitespace-separated string (#627). Checked AFTER the + // sentinel so an unavailable array tag still reports the truthful no-comms. if (tag is { IsArray: true }) { - return new DataValueSnapshot(null, BadNotSupported, sourceTimestamp, serverTimestamp); + return BuildArraySnapshot(observation, tag, sourceTimestamp, serverTimestamp); } // An unauthored observation has no declared target type; String is the only coercion that @@ -286,6 +287,40 @@ internal sealed class MTConnectObservationIndex : new DataValueSnapshot(null, status, sourceTimestamp, serverTimestamp); } + /// + /// Parses a TIME_SERIES vector into a double[]. Only + /// arrays are served (the MTConnect TIME_SERIES element type is a floating-point sample); any other + /// authored array type is BadNotSupported rather than an approximation. All-or-nothing: one + /// unparseable or non-finite token, or a count that disagrees with the observation's own + /// sampleCount (or the tag's fixed ArrayDim), makes the whole vector Bad — a partial + /// vector served as Good would be a silently wrong waveform. + /// + private static DataValueSnapshot BuildArraySnapshot( + MTConnectObservation observation, MTConnectTagDefinition tag, DateTime sourceTimestamp, DateTime serverTimestamp) + { + DataValueSnapshot Bad(uint status) => new(null, status, sourceTimestamp, serverTimestamp); + + if (tag.DriverDataType != DriverDataType.Float64) return Bad(BadNotSupported); + + var tokens = observation.Value.Split((char[]?)null, StringSplitOptions.RemoveEmptyEntries); + var values = new double[tokens.Length]; + for (var i = 0; i < tokens.Length; i++) + { + if (string.Equals(tokens[i], UnavailableSentinel, StringComparison.Ordinal)) return Bad(BadNoCommunication); + + if (!double.TryParse(tokens[i], NumberStyles.Float, CultureInfo.InvariantCulture, out values[i]) || + !double.IsFinite(values[i])) + { + return Bad(BadTypeMismatch); + } + } + + if (observation.SampleCount is { } declared && declared != values.Length) return Bad(BadDecodingError); + if (tag.ArrayDim > 0 && tag.ArrayDim != values.Length) return Bad(BadDecodingError); + + return new DataValueSnapshot(values, Good, sourceTimestamp, serverTimestamp); + } + /// /// Converts the Agent's text to the tag's authored type. Every parse is /// : MTConnect is an invariant-format wire, and a diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectStreamsParser.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectStreamsParser.cs index b0d6bdff..06a9a5cf 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectStreamsParser.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectStreamsParser.cs @@ -377,6 +377,8 @@ internal static class MTConnectStreamsParser string? dataItemId = null; var timestampUtc = default(DateTime); string? conditionState = null; + int? sampleCount = null; + double? sampleRate = null; if (isObservation) { @@ -385,6 +387,24 @@ internal static class MTConnectStreamsParser timestampUtc = ReadTimestampUtc(reader, dataItemId); conditionState = isCondition ? ConditionState(name) : null; + // TIME_SERIES metadata rides the observation element itself. Read here, before the subtree + // walk moves the reader off this element's attributes. Lenient: a vendor's unparseable + // attribute is dropped rather than failing the whole document (the vector is checked + // against the declared sampleCount only when one parses). + if (!isCondition) + { + sampleCount = int.TryParse( + MTConnectXml.OptionalAttribute(reader, "sampleCount"), + NumberStyles.Integer, + CultureInfo.InvariantCulture, + out var count) && count >= 0 ? count : null; + sampleRate = double.TryParse( + MTConnectXml.OptionalAttribute(reader, "sampleRate"), + NumberStyles.Float, + CultureInfo.InvariantCulture, + out var rate) && double.IsFinite(rate) ? rate : null; + } + // The slot is reserved BEFORE the subtree is read, and filled after: an observation is // only complete once its text and its child-element flag are known, but appending it // then would report a nested observation ahead of the one enclosing it. The tree walk @@ -430,7 +450,9 @@ internal static class MTConnectStreamsParser // MTConnectObservation.IsStructured. Never true for a CONDITION: its value comes from // the element NAME, so child content cannot corrupt it, and flagging one would throw // away a perfectly well-determined state. - IsStructured: !isCondition && hasElements); + IsStructured: !isCondition && hasElements, + SampleCount: sampleCount, + SampleRate: sampleRate); } /// diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/MTConnectTagConfigModel.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/MTConnectTagConfigModel.cs index d1d3997a..647930e0 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/MTConnectTagConfigModel.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/MTConnectTagConfigModel.cs @@ -229,10 +229,25 @@ public sealed class MTConnectTagConfigModel /// passed as null; it can only DEMOTE an item to String (DATA_SET/TABLE), which cannot create a false /// warning because a numeric declaration on a String-inferred item is warned either way, and a SAMPLE /// inferred Float64 is only warned about for a non-numeric, non-String declaration or an integer one. - /// TIME_SERIES arrays are not detectable without it and are not covered. + /// TIME_SERIES arrays are covered separately by the isArray rule in . /// /// Zero or more human-readable warnings. public IReadOnlyList Warnings() + { + // #627: an array (TIME_SERIES) tag is served as double[] only. Independent of the probe metadata — + // the array flag is authored on the tag, and any other declared element type reads BadNotSupported. + var arrayWarning = _bag.TryGetPropertyValue("isArray", out var arr) + && arr is JsonValue av && av.TryGetValue(out var isArray) && isArray + && DataType != DriverDataType.Float64 + ? $"Data type {DataType} on an array tag: MTConnect TIME_SERIES arrays are served as Float64 (double[]) only; " + + "declare Float64 or the tag reads BadNotSupported. Saving is still allowed." + : null; + + var scalar = ScalarWarnings(); + return arrayWarning is null ? scalar : [arrayWarning, .. scalar]; + } + + private IReadOnlyList ScalarWarnings() { if (string.IsNullOrWhiteSpace(MtCategory)) { return []; } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectObservationIndexTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectObservationIndexTests.cs index 253a491f..84657e3e 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectObservationIndexTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectObservationIndexTests.cs @@ -30,6 +30,7 @@ public sealed class MTConnectObservationIndexTests private const uint BadOutOfRange = 0x803C0000u; private const uint BadNotSupported = 0x803D0000u; private const uint BadTypeMismatch = 0x80740000u; + private const uint BadDecodingError = 0x80070000u; /// Every dataItemId both /current fixtures report. private static readonly string[] AllFixtureDataItemIds = @@ -48,7 +49,7 @@ public sealed class MTConnectObservationIndexTests /// Wraps one synthetic observation in the minimal legal streams result. private static MTConnectStreamsResult Synthetic( - string dataItemId, string value, DateTime? timestampUtc = null, bool isStructured = false) => + string dataItemId, string value, DateTime? timestampUtc = null, bool isStructured = false, int? sampleCount = null) => new( InstanceId: 1L, NextSequence: 2L, @@ -57,7 +58,8 @@ public sealed class MTConnectObservationIndexTests dataItemId, value, timestampUtc ?? new DateTime(2026, 7, 24, 12, 0, 0, DateTimeKind.Utc), - isStructured)]); + isStructured, + sampleCount)]); private static MTConnectTagDefinition Tag(string id, DriverDataType type) => new(id, type); @@ -412,13 +414,9 @@ public sealed class MTConnectObservationIndexTests // -------------------------------------------------------------------------------- TIME_SERIES - /// - /// P1 defers TIME_SERIES array materialization to P1.5. An array-authored tag therefore - /// reports BadNotSupported rather than pretending: the vector is real data this build - /// cannot represent, which is neither a type mismatch nor a comms failure. - /// + /// #627: a Float64 array tag serves the whole vector as double[], Good. [Fact] - public void Time_series_authored_as_an_array_reports_BadNotSupported() + public void Time_series_authored_as_a_float64_array_parses_to_double_array() { var idx = new MTConnectObservationIndex( [new MTConnectTagDefinition("dev1_vibration_ts", DriverDataType.Float64, IsArray: true, ArrayDim: 10)]); @@ -426,25 +424,93 @@ public sealed class MTConnectObservationIndexTests var snap = idx.Get("dev1_vibration_ts"); - snap.StatusCode.ShouldBe(BadNotSupported); + snap.StatusCode.ShouldBe(Good); + snap.Value.ShouldBeOfType() + .ShouldBe([1.1, 1.2, 1.3, 1.4, 1.5, 1.6, 1.7, 1.8, 1.9, 2.0]); + } + + /// The variable-length shape (no declared ArrayDim) is served the same way. + [Fact] + public void Time_series_without_a_declared_length_is_served_variable_length() + { + var idx = new MTConnectObservationIndex( + [new MTConnectTagDefinition("ts", DriverDataType.Float64, IsArray: true)]); + idx.Apply(Synthetic("ts", "1 2.5\n 3")); + + var snap = idx.Get("ts"); + + snap.StatusCode.ShouldBe(Good); + snap.Value.ShouldBeOfType().ShouldBe([1.0, 2.5, 3.0]); + } + + [Fact] + public void Time_series_count_mismatching_the_observations_sampleCount_is_Bad() + { + var idx = new MTConnectObservationIndex( + [new MTConnectTagDefinition("ts", DriverDataType.Float64, IsArray: true)]); + idx.Apply(Synthetic("ts", "1 2 3", sampleCount: 4)); + + var snap = idx.Get("ts"); + + snap.StatusCode.ShouldBe(BadDecodingError); snap.Value.ShouldBeNull(); } - /// - /// The falsifiable half of the deferral: the vector must never be silently reduced to its - /// first element. A Good 1.1 here would be a wrong value an operator would trust. - /// [Fact] - public void Time_series_vector_is_never_silently_parsed_as_its_first_element() + public void Time_series_count_mismatching_a_fixed_ArrayDim_is_Bad() { var idx = new MTConnectObservationIndex( - [new MTConnectTagDefinition("dev1_vibration_ts", DriverDataType.Float64, IsArray: true, ArrayDim: 10)]); + [new MTConnectTagDefinition("ts", DriverDataType.Float64, IsArray: true, ArrayDim: 5)]); + idx.Apply(Synthetic("ts", "1 2 3")); + + idx.Get("ts").StatusCode.ShouldBe(BadDecodingError); + } + + [Theory] + [InlineData("1 2 abc")] + [InlineData("1 NaN 3")] + [InlineData("1 UNAVAILABLE 3")] + public void Time_series_with_an_unusable_token_is_Bad_never_a_partial_vector(string text) + { + var idx = new MTConnectObservationIndex( + [new MTConnectTagDefinition("ts", DriverDataType.Float64, IsArray: true)]); + idx.Apply(Synthetic("ts", text)); + + var snap = idx.Get("ts"); + + snap.StatusCode.ShouldNotBe(Good); + snap.Value.ShouldBeNull(); + } + + [Fact] + public void Empty_time_series_reports_no_communication() + { + var idx = new MTConnectObservationIndex( + [new MTConnectTagDefinition("ts", DriverDataType.Float64, IsArray: true)]); + idx.Apply(Synthetic("ts", " ")); + + idx.Get("ts").StatusCode.ShouldBe(BadNoCommunication); + } + + [Fact] + public void A_non_float64_array_tag_stays_BadNotSupported() + { + var idx = new MTConnectObservationIndex( + [new MTConnectTagDefinition("dev1_vibration_ts", DriverDataType.Int32, IsArray: true)]); idx.Apply(ParseFixture(CurrentFixture)); - var snap = idx.Get("dev1_vibration_ts"); + idx.Get("dev1_vibration_ts").StatusCode.ShouldBe(BadNotSupported); + } - snap.Value.ShouldNotBe(1.1); - snap.StatusCode.ShouldNotBe(Good); + /// The parser carries the observation's own sampleCount / sampleRate attributes. + [Fact] + public void Parser_carries_time_series_sampleCount_and_sampleRate() + { + var obs = ParseFixture(CurrentFixture).Observations.Single(o => o.DataItemId == "dev1_vibration_ts"); + + obs.SampleCount.ShouldBe(10); + obs.SampleRate.ShouldBe(100.0); + ParseFixture(CurrentFixture).Observations.Single(o => o.DataItemId == "dev1_pos").SampleCount.ShouldBeNull(); } /// @@ -452,7 +518,7 @@ public sealed class MTConnectObservationIndexTests /// representable state and must report it rather than the deferral code. /// [Fact] - public void Unavailable_wins_over_the_unsupported_array_shape() + public void Unavailable_array_tag_reports_no_communication() { var idx = new MTConnectObservationIndex( [new MTConnectTagDefinition("dev1_vibration_ts", DriverDataType.Float64, IsArray: true, ArrayDim: 10)]); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectReadTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectReadTests.cs index ad9288dd..b8891260 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectReadTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectReadTests.cs @@ -219,15 +219,16 @@ public sealed class MTConnectReadTests res[0].SourceTimestampUtc.ShouldNotBeNull(); } - /// A TIME_SERIES vector is real data this build cannot represent (P1.5 deferral). + /// A probe-declared TIME_SERIES item is read end to end as a double[] (#627). [Fact] - public async Task Read_of_a_time_series_ref_reports_BadNotSupported() + public async Task Read_of_a_time_series_ref_returns_the_double_vector() { var (driver, _) = await InitializedDriverAsync(); var res = await driver.ReadAsync(["dev1_vibration_ts"], TestContext.Current.CancellationToken); - res[0].StatusCode.ShouldBe(BadNotSupported); + res[0].StatusCode.ShouldBe(0u); + res[0].Value.ShouldBeOfType().Length.ShouldBe(10); } /// A blank reference is data, not a programming error — it codes Bad, it does not throw. diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/MTConnectTagConfigModelTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/MTConnectTagConfigModelTests.cs index a40e62d7..6b31fd40 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/MTConnectTagConfigModelTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/MTConnectTagConfigModelTests.cs @@ -328,4 +328,15 @@ public sealed class MTConnectTagConfigModelTests public void Warnings_is_a_noop_without_probe_metadata() => MTConnectTagConfigModel.FromJson("{\"fullName\":\"x\",\"dataType\":\"Boolean\"}") .Warnings().ShouldBeEmpty(); + + [Fact] + public void Warnings_accepts_a_float64_array_for_time_series_and_flags_other_array_types() + { + MTConnectTagConfigModel.FromJson("{\"fullName\":\"x\",\"dataType\":\"Float64\",\"isArray\":true}") + .Warnings().ShouldBeEmpty(); + + var m = MTConnectTagConfigModel.FromJson("{\"fullName\":\"x\",\"dataType\":\"Int32\",\"isArray\":true}"); + m.Warnings().ShouldHaveSingleItem().ShouldContain("Float64"); + m.Validate().ShouldBeNull(); // advisory only + } } From dcc827ca8c5fa048d0f5d86f5e0bcda3761df0a3 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 8 Oct 2026 14:29:50 -0400 Subject: [PATCH 2/2] =?UTF-8?q?feat(mtconnect):=20one=20agent=20client=20p?= =?UTF-8?q?er=20driver=20=E2=80=94=20a=20second=20/raw=20Device=20is=20a?= =?UTF-8?q?=20deploy=20error=20(#628)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- docs/drivers/MTConnect.md | 31 ++++++---- .../Validation/DraftValidator.cs | 37 +++++++++++ .../Uns/RawTreeService.cs | 10 ++- .../DraftValidatorMTConnectDeviceTests.cs | 61 +++++++++++++++++++ .../Uns/RawTreeServiceWaveBTests.cs | 19 ++++++ 5 files changed, 144 insertions(+), 14 deletions(-) create mode 100644 tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorMTConnectDeviceTests.cs diff --git a/docs/drivers/MTConnect.md b/docs/drivers/MTConnect.md index a7659305..210bc10f 100644 --- a/docs/drivers/MTConnect.md +++ b/docs/drivers/MTConnect.md @@ -410,14 +410,24 @@ for anything beyond values-and-conditions. `GetHostStatuses()` fresh, and the result rides `DriverHealthChanged` to the AdminUI `/hosts` page (no SQL; the `DriverHostStatus` table was dropped). See `docs/Telemetry.md`. -2. **`RawTagEntry.DeviceName` is accepted on the wire shape but ignored for - routing.** One driver instance owns exactly one Agent client scoped by - `MTConnectDriverOptions.DeviceName` (the top-level config key); the - per-tag `RawTagEntry.DeviceName` field the v3 raw-tag identity carries is - never read by the driver. Two `/raw` Devices authored under one MTConnect - driver instance both resolve to the same one Agent connection. Real - per-device routing (a client per device) would be a design change, not a - bug fix. +2. **One MTConnect driver instance = one Agent client = at most one `/raw` + Device (#628, decision (b)).** The driver never reads + `RawTagEntry.DeviceName`; every raw tag binds through the single Agent client + scoped by `MTConnectDriverOptions.DeviceName`. The v3 raw tree always has a + Device level (a new driver auto-creates `Device1`), and that single-Device + layout is the normal, valid shape — its `DeviceConfig` is merged up into the + driver config by `DriverDeviceConfigMerger` (this is how the endpoint lands). + With **two or more** Devices under one MTConnect driver there is no merge-up + and no routing, so which Device supplies the endpoint is ambiguous. That is + now a **deploy-time `DraftValidator` error** (`MTConnectMultipleDevices`), + and the AdminUI refuses to *create* the second Device + (`RawTreeService.CreateDeviceAsync`, same message): "MTConnect binds all tags + through one agent client per driver instance; author one MTConnect driver per + agent device." Option (a), a client per Device, was rejected as a design + change with no demand. A multi-device Agent is served by several driver + instances, each with its own `DeviceName` scope. The driver's init-time + Warning for differing `RawTagEntry.DeviceName` values stays as defence in + depth for configs that predate the rule. **The write path is the one place a device name is resolved per DataItem**, and it does not use `RawTagEntry.DeviceName` either: it reads the owning @@ -426,11 +436,6 @@ for anything beyond values-and-conditions. So `MTConnectDriverOptions.DeviceName` does double duty — request scope for reads/browse, fallback write target for writes. - **It is no longer silent (#613).** When the instance's raw tags carry two or - more distinct non-blank `RawTagEntry.DeviceName` values, driver init logs one - Warning naming them and the driver-level scope they collapse into. A single - value is not reported (it is just the `/raw` Device's own name). Real - per-device routing remains deferred. 3. **Authoring only *warns* when a declared coercion type cannot hold the inferred one (#613).** The AdminUI tag editor (`MTConnectTagConfigModel.Warnings()`) compares the declared type with `MTConnectDataTypeInference` over the diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftValidator.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftValidator.cs index 3543bce2..3f5bebab 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftValidator.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftValidator.cs @@ -41,6 +41,7 @@ public static class DraftValidator ValidateEquipReferenceResolution(draft, errors); ValidateCalculationTags(draft, errors); ValidateSqlConnectionStringNotPersisted(draft, errors); + ValidateMTConnectSingleDevice(draft, errors); ValidateNodeAclScopes(draft, errors); return errors; } @@ -343,6 +344,42 @@ public static class DraftValidator } } + /// Error code for an MTConnect driver instance that has more than one /raw Device (#628). + public const string MTConnectMultipleDevicesCode = "MTConnectMultipleDevices"; + + /// + /// The one message both the deploy gate and the authoring-time guard (RawTreeService.CreateDeviceAsync) + /// use, so the two can never describe different rules. + /// + /// The driver instance id or name to cite. + public static string MTConnectMultipleDevicesMessage(string driverLabel) => + $"MTConnect driver '{driverLabel}' has more than one Device. MTConnect binds all tags through one agent " + + "client per driver instance and does not route by /raw Device, and with several Devices the Device's " + + "DeviceConfig (agent URI / device name) is no longer merged into the driver config, so which Device would " + + "supply the endpoint is ambiguous. Author one MTConnect driver per agent device, each with a single Device."; + + /// + /// #628 (decision (b)): an MTConnect driver instance owns exactly one agent client, so a second /raw + /// Device under it is a genuine ambiguity — the driver ignores RawTagEntry.DeviceName, and + /// DriverDeviceConfigMerger only merges a Device's DeviceConfig up to the driver config when + /// there is exactly ONE Device. The normal single-Device layout (every driver gets a default + /// Device1) stays valid. + /// + private static void ValidateMTConnectSingleDevice(DraftSnapshot draft, List errors) + { + foreach (var driver in draft.DriverInstances) + { + if (!string.Equals(driver.DriverType, Core.Abstractions.DriverTypeNames.MTConnect, StringComparison.Ordinal)) + continue; + var count = draft.Devices.Count(d => + string.Equals(d.DriverInstanceId, driver.DriverInstanceId, StringComparison.Ordinal)); + if (count <= 1) continue; + errors.Add(new(MTConnectMultipleDevicesCode, + MTConnectMultipleDevicesMessage(driver.Name) + $" ({count} Devices found.)", + driver.DriverInstanceId)); + } + } + /// WP7 Calculation-driver deploy gates. For every tag bound to a Calculation driver: /// /// scriptId existence — the tag's TagConfig.scriptId must be present and resolve to diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawTreeService.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawTreeService.cs index 3e7d77fa..e5992fbe 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawTreeService.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawTreeService.cs @@ -5,6 +5,7 @@ using ZB.MOM.WW.OtOpcUa.AdminUI.Uns.TagEditors; using ZB.MOM.WW.OtOpcUa.Commons.Types; using ZB.MOM.WW.OtOpcUa.Configuration; using ZB.MOM.WW.OtOpcUa.Configuration.Entities; +using ZB.MOM.WW.OtOpcUa.Configuration.Validation; using ZB.MOM.WW.OtOpcUa.Core.Abstractions; namespace ZB.MOM.WW.OtOpcUa.AdminUI.Uns; @@ -615,9 +616,16 @@ public sealed class RawTreeService(IDbContextFactory dbF await using var db = await dbFactory.CreateDbContextAsync(ct); - if (!await db.DriverInstances.AnyAsync(d => d.DriverInstanceId == driverInstanceId, ct)) + var owner = await db.DriverInstances.FirstOrDefaultAsync(d => d.DriverInstanceId == driverInstanceId, ct); + if (owner is null) return new UnsMutationResult(false, $"Driver '{driverInstanceId}' not found."); + // #628: MTConnect owns one agent client per driver — mirror the deploy gate + // (DraftValidator.ValidateMTConnectSingleDevice) so the second Device is refused where it is authored. + if (string.Equals(owner.DriverType, DriverTypeNames.MTConnect, StringComparison.Ordinal) + && await db.Devices.AnyAsync(dev => dev.DriverInstanceId == driverInstanceId, ct)) + return new UnsMutationResult(false, DraftValidator.MTConnectMultipleDevicesMessage(owner.Name)); + if (await db.Devices.AnyAsync(dev => dev.DriverInstanceId == driverInstanceId && dev.Name == name, ct)) return new UnsMutationResult(false, $"A device named '{name}' already exists on this driver."); diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorMTConnectDeviceTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorMTConnectDeviceTests.cs new file mode 100644 index 00000000..670ee35e --- /dev/null +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorMTConnectDeviceTests.cs @@ -0,0 +1,61 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Configuration.Entities; +using ZB.MOM.WW.OtOpcUa.Configuration.Validation; + +namespace ZB.MOM.WW.OtOpcUa.Configuration.Tests; + +/// +/// Gitea #628 (decision b): an MTConnect driver instance binds every tag through one agent client, so a +/// second /raw Device under it is a deploy error — while the normal single-Device layout stays valid. +/// +[Trait("Category", "Unit")] +public sealed class DraftValidatorMTConnectDeviceTests +{ + private const string Code = DraftValidator.MTConnectMultipleDevicesCode; + + private static DriverInstance Driver(string id, string type) => new() + { + DriverInstanceId = id, ClusterId = "c", Name = id, DriverType = type, DriverConfig = "{}", + }; + + private static Device Dev(string id, string driverId) => new() + { + DeviceId = id, DriverInstanceId = driverId, Name = id, DeviceConfig = "{}", + }; + + private static DraftSnapshot Draft(DriverInstance[] drivers, params Device[] devices) => new() + { + GenerationId = 1, ClusterId = "c", DriverInstances = drivers, Devices = devices, + }; + + [Fact] + public void Two_devices_under_one_MTConnect_driver_is_a_deploy_error() + { + var errors = DraftValidator.Validate(Draft([Driver("mt", "MTConnect")], Dev("d1", "mt"), Dev("d2", "mt"))); + + var e = errors.Where(x => x.Code == Code).ShouldHaveSingleItem(); + e.Context.ShouldBe("mt"); + e.Message.ShouldContain("one MTConnect driver per agent device"); + } + + [Fact] + public void The_normal_single_device_layout_stays_valid() + => DraftValidator.Validate(Draft([Driver("mt", "MTConnect")], Dev("d1", "mt"))) + .ShouldNotContain(x => x.Code == Code); + + [Fact] + public void A_driver_with_no_device_is_not_this_rules_concern() + => DraftValidator.Validate(Draft([Driver("mt", "MTConnect")])) + .ShouldNotContain(x => x.Code == Code); + + [Fact] + public void Several_devices_under_a_non_MTConnect_driver_are_unaffected_and_devices_are_counted_per_driver() + { + var errors = DraftValidator.Validate(Draft( + [Driver("mb", "Modbus"), Driver("mt1", "MTConnect"), Driver("mt2", "MTConnect")], + Dev("a", "mb"), Dev("b", "mb"), Dev("c", "mt1"), Dev("d", "mt2"))); + + errors.ShouldNotContain(x => x.Code == Code); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawTreeServiceWaveBTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawTreeServiceWaveBTests.cs index 3315b79c..63febc95 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawTreeServiceWaveBTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawTreeServiceWaveBTests.cs @@ -421,6 +421,25 @@ public sealed class RawTreeServiceWaveBTests result.Error!.ShouldContain("already exists"); } + [Fact] + public async Task CreateDevice_refuses_a_second_device_under_an_MTConnect_driver_but_not_under_others() + { + var (service, dbName) = Seeded(); + var mt = await service.CreateDriverAsync(RawTreeTestDb.ClusterId, null, "mt-1", "MTConnect", "{}"); + mt.Ok.ShouldBeTrue(); + + // CreateDriver auto-creates Device1, so the very next device is the ambiguous second one (#628). + var refused = await service.CreateDeviceAsync(mt.CreatedId!, "Device2", "{}"); + + refused.Ok.ShouldBeFalse(); + refused.Error!.ShouldContain("one MTConnect driver per agent device"); + + // Positive control: the same call against a non-MTConnect driver still succeeds. + (await service.CreateDeviceAsync(RawTreeTestDb.FolderedDriverId, "Device9", "{}")).Ok.ShouldBeTrue(); + using var db = RawTreeTestDb.CreateNamed(dbName); + db.Devices.Count(d => d.DriverInstanceId == mt.CreatedId).ShouldBe(1); + } + // ------------------------------------------------------------- Merged probe config (Test connect) [Fact]