dohertj2/lmxopcua
1
0
Fork 0
Code Issues 32 Pull Requests Actions Packages Projects Releases Wiki Activity

feat(mtconnect): IWritable via agent observation writes

Browse Source 
MTConnectDriver now implements IWritable over the Task-1 agent-client seam
(WriteObservationAsync -> the Agent's AllowPut POST surface). A write is
OBSERVATION INJECTION, not machine command: it sets what the Agent reports for
a DataItem, is visible to every consumer of that Agent, and is overwritten by
the next adapter/SHDR update. Off at two independent gates, both off by default
- the Agent's own AllowPut and the driver's write.enabled (Task 0).

Per-slot refusal order (never throws, strict N-in/N-out batch shape):
  write disabled          -> BadNotWritable
  no live client          -> BadNotConnected  (ReadAsync's existing convention)
  reference unresolved    -> BadNodeIdUnknown
  no resolvable device    -> BadNodeIdUnknown
  CONDITION category      -> BadNotWritable
  null / non-scalar value -> BadTypeMismatch
  agent refusal OR throw  -> BadCommunicationError
OperationCanceledException fills the remaining slots and returns. The Agent's
refusal text is logged at Warning and never enters DriverHealth.LastError
(type-only discipline); a refused write leaves health untouched.

Device-name resolution: TagBinding gains WriteTargetsByDataItemId, built in
BuildBinding from a probe-model walk that remembers which device each DataItem
was declared under (name, or id when nameless) plus its category. Falls back to
options.DeviceName; unresolvable -> BadNodeIdUnknown rather than a guess.
Values render via Convert.ToString(..., InvariantCulture) - so a Boolean writes
the invariant string "True"/"False", NOT "1"/"0". Arrays are refused rather
than rendered: Convert.ToString would emit the literal "System.Double[]", which
the Agent stores happily and which is invisible until read-back.

FINDING - the CurrentWrite bit does NOT key on SecurityClassification, and the
discovery stamping below is therefore cosmetic. Traced end to end: a browsed
leaf's SecurityClass survives into the browse session's AttributeInfo
(CapturedTreeBrowseSession.cs:53) and stops there - RawBrowseModal never reads
it, RawBrowseCommitMapper.MapLeaf hardcodes TagAccessLevel.Read, and Tag has no
SecurityClass column. AddressSpaceComposer:746/:809 sets Writable purely from
Tag.AccessLevel == ReadWrite; AddressSpaceApplier ANDs !IsArray;
OtOpcUaNodeManager.ComposeAccessLevel:2002 composes CurrentWrite from that
alone. Driver capability is never consulted at compose time either - IWritable
is checked only at runtime in DriverInstanceActor:755. Consequences: (a) raw
tags already committed keep their authored AccessLevel, so nothing this commit
does makes an existing node writable; (b) an operator must still set the tag to
ReadWrite for an MTConnect write to be reachable over OPC UA; (c) conversely a
ReadWrite tag on a non-IWritable driver has always advertised CurrentWrite and
failed only at the actor - pre-existing, not touched here.

Discovery stamps CanWrite(writeEnabled, dataItem) = write.enabled && !CONDITION
-> Operate, else ViewOnly - kept honest so the browse never advertises a tier
the write path would refuse (MQTT's CanWrite principle), with the caveat above
recorded verbatim at the call site.

Status-code constants are spelled WITHOUT MqttDriver's "Status" prefix, to
match the two this file already declares (BadCommunicationError,
BadNotConnected). StatusCodeParityTests strips either spelling and passes.

TDD: WriteAsync landed first as a NotImplementedException scaffold - 19 of 20
new tests red on behaviour, 1 (the capability assertion) green on the scaffold
- then implemented. MTConnectFactoryTests' "not_writable" characterization test
is rewritten to pin the new truth, and is the guard for the concrete-return
contract the factory remark explains.

Tests: 532 passed / 0 failed (was ~509). Full solution build: 0 errors,
0 warnings; OTOPCUA0001 clean.
This commit is contained in:
Joseph Doherty
2026-08-05 06:36:08 -04:00
parent 3b8b5a77e1
commit b29623cf99
6 changed files with 868 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectDiscoverTests.cs
+50 -5
View File
@@ -45,12 +45,16 @@ public sealed class MTConnectDiscoverTests
"dev1_system_cond",
];
private static MTConnectDriverOptions Opts(string? deviceName = null) =>
private static MTConnectDriverOptions Opts(string? deviceName = null, bool writeEnabled = false) =>
new()
{
AgentUri = AgentUri,
DeviceName = deviceName,
// Off unless a test says otherwise — the shipped default, and what every pre-write-back
// expectation in this file was written against.
Write = new MTConnectWriteOptions { Enabled = writeEnabled },
// Discovery is a pure read of the /probe device model: it must enumerate what the Agent
// declares, NOT what an operator has already authored. Left empty on purpose so a driver
// that streamed its authored tag table instead could not pass.
@@ -324,10 +328,10 @@ public sealed class MTConnectDiscoverTests
}
/// <summary>
/// MTConnect is a read-only protocol and this driver implements no <c>IWritable</c>, so every
/// browsed leaf is <see cref="SecurityClassification.ViewOnly"/> — the tier that is read-only
/// from OPC UA. Nothing is historized by discovery either: historization is authored per tag,
/// never inferred from a device model.
/// Write-back is off by default, so every browsed leaf is
/// <see cref="SecurityClassification.ViewOnly"/> — the tier that is read-only from OPC UA, and
/// exactly what this driver shipped. Nothing is historized by discovery either: historization
/// is authored per tag, never inferred from a device model.
/// </summary>
[Fact]
public async Task Discover_streams_every_leaf_view_only_and_not_historized()
@@ -341,6 +345,47 @@ public sealed class MTConnectDiscoverTests
cap.Variables.ShouldAllBe(v => v.Attr.Source == NodeSourceKind.Driver);
}
/// <summary>
/// With <c>write.enabled</c> on, the same probe model browses <see cref="SecurityClassification.Operate"/>
/// — but only for the leaves <c>WriteAsync</c> would actually accept. A browse that advertised a
/// tier the write path refuses is the "silent no-op in a new costume" failure MQTT's
/// <c>CanWrite</c> pins against; the flag is the ONLY difference between this test and the one
/// above, so a driver that ignored it could not pass both.
/// </summary>
[Fact]
public async Task Discover_streams_writable_leaves_as_operate_when_write_is_enabled()
{
var (driver, _) = await InitializedDriverAsync(Opts(writeEnabled: true));
var cap = await DiscoverAsync(driver);
var writable = cap.Variables.Where(v => !v.Attr.IsAlarm).ToList();
// Counted, not just ShouldAllBe: an empty sequence satisfies ShouldAllBe vacuously, so a
// discovery that streamed nothing at all would otherwise pass this test.
writable.Count.ShouldBe(FixtureDataItemIds.Length - 1);
writable.ShouldAllBe(v => v.Attr.SecurityClass == SecurityClassification.Operate);
}
/// <summary>
/// A CONDITION data item stays <see cref="SecurityClassification.ViewOnly"/> in <b>both</b>
/// modes: a condition PUT carries alarm semantics and a different <c>level|text</c> wire
/// format, so the write path refuses it and the browse must say so.
/// </summary>
/// <param name="writeEnabled">Whether the driver's write-back block is switched on.</param>
[Theory]
[InlineData(false)]
[InlineData(true)]
public async Task Discover_streams_a_condition_leaf_view_only_whether_or_not_write_is_enabled(bool writeEnabled)
{
var (driver, _) = await InitializedDriverAsync(Opts(writeEnabled: writeEnabled));
var cap = await DiscoverAsync(driver);
cap.Variables.Single(v => v.Attr.FullName == "dev1_system_cond")
.Attr.SecurityClass.ShouldBe(SecurityClassification.ViewOnly);
}
// ---- browse names ----
/// <summary>
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectFactoryTests.cs
+11 -5
View File
@@ -207,19 +207,25 @@ public sealed class MTConnectFactoryTests
}
/// <summary>
/// The other half of the capability pin: MTConnect's Agent surface is read-only, so the
/// instance must NOT be <see cref="IWritable"/>. If it ever became writable, the write gate
/// would start offering an operation the protocol cannot honour.
/// The other half of the capability pin: the instance IS <see cref="IWritable"/>, so the
/// runtime's write gate dispatches to it. This is the seam the concrete return type exists for
/// — the capability was added to the driver with no change to the factory, and a narrowed
/// return would have left it implemented but never dispatched.
/// </summary>
/// <remarks>
/// <b>Dispatchable is not enabled.</b> The config below authors no <c>write</c> block, so this
/// instance refuses every write with <c>BadNotWritable</c> — and the Agent's own
/// <c>AllowPut</c> is a second, independent gate. See <c>MTConnectWriteTests</c>.
/// </remarks>
[Fact]
public void Registered_delegate_returns_an_instance_that_is_not_writable()
public void Registered_delegate_returns_an_instance_that_dispatches_writes()
{
var registry = new DriverFactoryRegistry();
MTConnectDriverFactoryExtensions.Register(registry);
IDriver driver = registry.TryGet("MTConnect")!("mt1", "{\"agentUri\":\"http://a:5000\"}");
(driver is IWritable).ShouldBeFalse("the MTConnect Agent surface is read-only");
(driver is IWritable).ShouldBeTrue("MTConnect must dispatch writes (the Agent AllowPut surface)");
}
/// <summary>Registering into a null registry is a wiring bug, not a silent no-op.</summary>
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectWriteTests.cs
+356
View File
@@ -0,0 +1,356 @@
using Shouldly;
using Xunit;
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
namespace ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests;
/// <summary>
/// <see cref="MTConnectDriver"/>'s <see cref="IWritable"/> leg — the batch contract, the per-slot
/// refusals, and the one thing the driver has to get right before the Agent is even dialled: which
/// device a DataItem belongs to.
/// </summary>
/// <remarks>
/// <para>
/// <b>The write itself is the live gate's job.</b> Everything below stops at the
/// <see cref="IMTConnectAgentClient"/> seam and asserts what the driver <i>asked</i> the Agent
/// for (<see cref="CannedAgentClient.Writes"/>), never that a fake was called for its own sake —
/// the same philosophy as <c>MqttDriverWriteTests</c>. What IS worth pinning offline is
/// everything that decides whether a write should reach an Agent at all, because that is where
/// a silent no-op would hide.
/// </para>
/// <para>
/// <b>A write is observation injection, not machine command</b> (design §2.2): it sets what the
/// Agent reports for a DataItem, and the next adapter update overwrites it. That is why the
/// feature is off at two independent gates — the Agent's <c>AllowPut</c>, and this driver's
/// <c>write.enabled</c> — and why the disabled leg below is the first test in the file.
/// </para>
/// </remarks>
public sealed class MTConnectWriteTests
{
private const string AgentUri = "http://fixture-agent:5000";
/// <summary>The device the canned <c>Fixtures/probe.xml</c> declares — <c>id="dev1" name="VMC-3Axis"</c>.</summary>
private const string FixtureDeviceName = "VMC-3Axis";
private const string PositionDataItemId = "dev1_pos";
private const string ConditionDataItemId = "dev1_system_cond";
private const string PositionRawPath = "Plant/MTConnect/dev1/Position";
private const string ConditionRawPath = "Plant/MTConnect/dev1/SystemCondition";
private const string UnknownRawPath = "Plant/MTConnect/dev1/NotATag";
private const uint Good = 0x00000000u;
private const uint BadNodeIdUnknown = 0x80340000u;
private const uint BadNotWritable = 0x803B0000u;
private const uint BadTypeMismatch = 0x80740000u;
private const uint BadCommunicationError = 0x80050000u;
private const uint BadNotConnected = 0x808A0000u;
private static CancellationToken Ct => TestContext.Current.CancellationToken;
/// <summary>One authored raw tag bound to a DataItem id, as the browse-commit flow writes it.</summary>
private static RawTagEntry Entry(string rawPath, string dataItemId) =>
new(rawPath, $$"""{"fullName":"{{dataItemId}}"}""", WriteIdempotent: false, DeviceName: "dev1");
private static MTConnectDriverOptions Opts(
bool writeEnabled = true, string? deviceName = null, params RawTagEntry[] rawTags) =>
new()
{
AgentUri = AgentUri,
DeviceName = deviceName,
Tags = [],
RawTags = rawTags,
Write = new MTConnectWriteOptions { Enabled = writeEnabled },
};
private static (MTConnectDriver Driver, CannedAgentClient Client) NewDriver(
MTConnectDriverOptions options, RecordingDriverLogger? logger = null)
{
var client = CannedAgentClient.FromFixtures();
return (new MTConnectDriver(options, "mt1", _ => client, logger), client);
}
private static async Task<(MTConnectDriver Driver, CannedAgentClient Client)> InitializedAsync(
MTConnectDriverOptions options, RecordingDriverLogger? logger = null)
{
var (driver, client) = NewDriver(options, logger);
await driver.InitializeAsync("{}", Ct);
return (driver, client);
}
/// <summary>The ordinary rig: write on, one writable position tag and the fixture's CONDITION item.</summary>
private static Task<(MTConnectDriver Driver, CannedAgentClient Client)> WritableDriverAsync(
RecordingDriverLogger? logger = null) =>
InitializedAsync(
Opts(
writeEnabled: true,
deviceName: null,
Entry(PositionRawPath, PositionDataItemId),
Entry(ConditionRawPath, ConditionDataItemId)),
logger);
// ---- the capability itself ----
[Fact]
public void MTConnectDriver_implements_IWritable()
// The interface's presence IS the feature: without it the runtime's capability dispatch never
// offers this driver a write, whatever a node advertises.
=> NewDriver(Opts()).Driver.ShouldBeAssignableTo<IWritable>();
// ---- the two independent gates ----
[Fact]
public async Task A_driver_with_write_disabled_refuses_every_slot_without_dialling_the_agent()
{
// The default. An existing deployment that never authors the block keeps today's read-only
// behaviour EXACTLY — including making no request an Agent could log or accept.
var (driver, client) = await InitializedAsync(
Opts(writeEnabled: false, deviceName: null, Entry(PositionRawPath, PositionDataItemId)));
var results = await driver.WriteAsync([new WriteRequest(PositionRawPath, 42.5)], Ct);
results.Count.ShouldBe(1);
results[0].StatusCode.ShouldBe(BadNotWritable);
client.Writes.ShouldBeEmpty();
}
[Fact]
public async Task A_write_before_initialize_reports_not_connected()
{
// Matches ReadAsync: "this driver is not running" is the only true statement available, and it
// is NOT the same operator problem as an Agent that answered and refused.
var (driver, _) = NewDriver(Opts(true, null, Entry(PositionRawPath, PositionDataItemId)));
var results = await driver.WriteAsync([new WriteRequest(PositionRawPath, 42.5)], Ct);
results[0].StatusCode.ShouldBe(BadNotConnected);
}
[Fact]
public async Task A_write_after_shutdown_reports_not_connected()
{
var (driver, _) = await WritableDriverAsync();
await driver.ShutdownAsync(Ct);
var results = await driver.WriteAsync([new WriteRequest(PositionRawPath, 42.5)], Ct);
results[0].StatusCode.ShouldBe(BadNotConnected);
}
// ---- per-slot refusals ----
[Fact]
public async Task An_unknown_reference_is_refused_in_its_own_slot()
{
var (driver, client) = await WritableDriverAsync();
var results = await driver.WriteAsync([new WriteRequest(UnknownRawPath, 1.0)], Ct);
results[0].StatusCode.ShouldBe(BadNodeIdUnknown);
client.Writes.ShouldBeEmpty();
}
[Fact]
public async Task A_condition_data_item_is_refused_as_not_writable()
{
// CONDITION PUTs carry alarm semantics and a different level|text wire format — excluded from
// v1. BadNotWritable, not BadTypeMismatch: the value was never the problem.
var (driver, client) = await WritableDriverAsync();
var results = await driver.WriteAsync([new WriteRequest(ConditionRawPath, "NORMAL")], Ct);
results[0].StatusCode.ShouldBe(BadNotWritable);
client.Writes.ShouldBeEmpty();
}
[Fact]
public async Task A_null_value_is_refused_as_a_type_mismatch()
{
// A write IS a URL query value; there is no null to send. Refusing beats writing the empty
// string, which the Agent would accept as a real observation.
var (driver, client) = await WritableDriverAsync();
var results = await driver.WriteAsync([new WriteRequest(PositionRawPath, null)], Ct);
results[0].StatusCode.ShouldBe(BadTypeMismatch);
client.Writes.ShouldBeEmpty();
}
[Fact]
public async Task An_array_value_is_refused_as_a_type_mismatch()
{
// Convert.ToString would happily render "System.Double[]" and the Agent would happily store it:
// a silent corruption visible only on read-back. Array/TIME_SERIES writes are out of scope.
var (driver, client) = await WritableDriverAsync();
var results = await driver.WriteAsync([new WriteRequest(PositionRawPath, new[] { 1.0, 2.0 })], Ct);
results[0].StatusCode.ShouldBe(BadTypeMismatch);
client.Writes.ShouldBeEmpty();
}
// ---- what actually reaches the seam ----
[Fact]
public async Task An_accepted_write_reports_good_and_names_the_owning_device_and_data_item()
{
// The device NAME is resolved from the /probe model, not from config: the Agent's PUT surface
// is addressed per device, and a multi-device Agent must be addressed correctly with no extra
// authoring. The RawPath is translated to the DataItem id, exactly as the read path does.
var (driver, client) = await WritableDriverAsync();
var results = await driver.WriteAsync([new WriteRequest(PositionRawPath, 42.5)], Ct);
results[0].StatusCode.ShouldBe(Good);
client.Writes.ShouldHaveSingleItem()
.ShouldBe((FixtureDeviceName, PositionDataItemId, "42.5"));
}
[Fact]
public async Task A_value_is_rendered_with_the_invariant_culture()
{
// A comma decimal separator would be accepted by the Agent and read back as a different number.
var (driver, client) = await WritableDriverAsync();
await driver.WriteAsync([new WriteRequest(PositionRawPath, 1234.5678d)], Ct);
client.Writes.ShouldHaveSingleItem().Value.ShouldBe("1234.5678");
}
[Fact]
public async Task A_boolean_is_rendered_as_its_invariant_true_false_string()
{
// Boolean.ToString is culture-independent, so the invariant rendering is "True"/"False" — NOT
// "1"/"0". Documented because it is the value an MTConnect consumer will see verbatim.
var (driver, client) = await WritableDriverAsync();
await driver.WriteAsync([new WriteRequest(PositionRawPath, true)], Ct);
client.Writes.ShouldHaveSingleItem().Value.ShouldBe("True");
}
// ---- device-name resolution ----
[Fact]
public async Task A_data_item_the_probe_does_not_declare_falls_back_to_the_configured_device()
{
// The dataItemId-keyed authoring surface (the driver CLI) and any tag whose DataItem the Agent
// has since dropped: config is the only device name available, and it is a real one.
var (driver, client) = await InitializedAsync(
Opts(writeEnabled: true, deviceName: "ConfiguredDevice", Entry(PositionRawPath, "not_in_probe")));
var results = await driver.WriteAsync([new WriteRequest(PositionRawPath, 7)], Ct);
results[0].StatusCode.ShouldBe(Good);
client.Writes.ShouldHaveSingleItem().ShouldBe(("ConfiguredDevice", "not_in_probe", "7"));
}
[Fact]
public async Task A_data_item_with_no_resolvable_device_is_refused_as_an_unknown_node()
{
// Neither the probe model nor config names a device, so there is no URL to POST to. Refused
// rather than guessed: a wrong device path is a 404 the operator reads as "the Agent is broken".
var (driver, client) = await InitializedAsync(
Opts(writeEnabled: true, deviceName: null, Entry(PositionRawPath, "not_in_probe")));
var results = await driver.WriteAsync([new WriteRequest(PositionRawPath, 7)], Ct);
results[0].StatusCode.ShouldBe(BadNodeIdUnknown);
client.Writes.ShouldBeEmpty();
}
// ---- how the Agent's own answers map ----
[Fact]
public async Task An_agent_refusal_maps_to_a_communication_error_and_logs_the_agents_own_text()
{
var logger = new RecordingDriverLogger();
var (driver, client) = await WritableDriverAsync(logger);
client.WriteOutcome = new MTConnectWriteOutcome(Accepted: false, AgentError: "UNSUPPORTED: no such device");
var results = await driver.WriteAsync([new WriteRequest(PositionRawPath, 42.5)], Ct);
results[0].StatusCode.ShouldBe(BadCommunicationError);
logger.WarningsSnapshot().ShouldContain(w => w.Contains("UNSUPPORTED: no such device", StringComparison.Ordinal));
}
[Fact]
public async Task An_agent_refusal_never_reaches_the_health_LastError()
{
// Type-only discipline: LastError carries an exception TYPE, never Agent-authored text, and a
// refused write is not evidence the driver's read path is degraded either.
var (driver, client) = await WritableDriverAsync();
var before = driver.GetHealth();
client.WriteOutcome = new MTConnectWriteOutcome(Accepted: false, AgentError: "UNSUPPORTED: no such device");
await driver.WriteAsync([new WriteRequest(PositionRawPath, 42.5)], Ct);
var after = driver.GetHealth();
after.LastError.ShouldBe(before.LastError);
after.State.ShouldBe(before.State);
}
[Fact]
public async Task A_transport_failure_maps_to_a_communication_error_without_throwing()
{
var (driver, client) = await WritableDriverAsync();
client.WriteFailure = new HttpRequestException("connection refused");
var results = await driver.WriteAsync([new WriteRequest(PositionRawPath, 42.5)], Ct);
results[0].StatusCode.ShouldBe(BadCommunicationError);
}
// ---- batch contract ----
[Fact]
public async Task Cancellation_fills_the_remaining_slots_and_returns_the_batch()
{
// Never throws, even cancelled: the caller gets one status per request or the node-manager
// cannot tell which writes were attempted.
var (driver, client) = await WritableDriverAsync();
using var cts = new CancellationTokenSource();
await cts.CancelAsync();
var results = await driver.WriteAsync(
[new WriteRequest(PositionRawPath, 1.0), new WriteRequest(PositionRawPath, 2.0)], cts.Token);
results.Count.ShouldBe(2);
results.ShouldAllBe(r => r.StatusCode == BadCommunicationError);
client.Writes.ShouldBeEmpty();
}
[Fact]
public async Task One_bad_request_does_not_fail_the_others_in_the_batch()
{
// Mirrors ReadAsync: failing the whole call would let one stale reference reject every other
// write in the same OPC UA request.
var (driver, client) = await WritableDriverAsync();
var results = await driver.WriteAsync(
[
new WriteRequest(UnknownRawPath, 1.0),
new WriteRequest(ConditionRawPath, "NORMAL"),
new WriteRequest(PositionRawPath, 42.5),
],
Ct);
// Order-preserving, one status per request, each naming its own distinct cause.
results.Count.ShouldBe(3);
results[0].StatusCode.ShouldBe(BadNodeIdUnknown);
results[1].StatusCode.ShouldBe(BadNotWritable);
results[2].StatusCode.ShouldBe(Good);
client.Writes.ShouldHaveSingleItem().DataItemId.ShouldBe(PositionDataItemId);
}
[Fact]
public async Task An_empty_batch_is_answered_with_an_empty_result()
=> (await NewDriver(Opts()).Driver.WriteAsync([], Ct)).ShouldBeEmpty();
[Fact]
public async Task WriteAsync_rejects_a_null_batch()
=> await Should.ThrowAsync<ArgumentNullException>(
async () => await NewDriver(Opts()).Driver.WriteAsync(null!, Ct));
}