From 40c74938298fc732f2de2e29b3ba133817f2f476 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 14:10:12 -0400 Subject: [PATCH] fix(twincat): BOOL-within-word writes RMW the parent at its own width (H18) The bit write read and wrote the parent word as a 32-bit UDInt with no parent check: a bit beyond a 16-bit parent was written into neighbouring bytes, and a signed parent with its sign bit set read back >= 2^31, so the checked conversion turned any bit write into BadOutOfRange. The write now goes through ITwinCATClient.WriteBitAsync: the ADS client resolves the parent symbol, checks the bit against it (TwinCATBitAccess. CheckParent, as the notification path already did), reads exactly parent.ByteSize raw bytes, flips one bit (TwinCATBitAccess.ApplyBitWrite) and writes the bytes back by index group/offset or variable handle. A bit beyond the parent is BadOutOfRange, a non-integer parent BadTypeMismatch, a short parent read BadCommunicationError (never written back), all without a health write. The per-parent RMW lock stays in the driver. Refs #739 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT --- .../AdsTwinCATClient.cs | 55 +++++++++++++++++ .../ITwinCATClient.cs | 16 +++++ .../TwinCATBitAccess.cs | 30 ++++++++++ .../TwinCATDriver.cs | 31 +++------- .../FakeTwinCATClient.cs | 34 +++++++++++ .../TwinCATBitAccessTests.cs | 46 +++++++++++++++ .../TwinCATReadWriteTests.cs | 59 ++++++++++++------- 7 files changed, 227 insertions(+), 44 deletions(-) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/AdsTwinCATClient.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/AdsTwinCATClient.cs index 53481b5e..a143d018 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/AdsTwinCATClient.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/AdsTwinCATClient.cs @@ -482,6 +482,61 @@ internal sealed class AdsTwinCATClient : ITwinCATClient } } + /// + public async Task WriteBitAsync(string parentPath, int bit, bool value, CancellationToken cancellationToken) + { + try + { + // Resolve the parent so the RMW happens at its OWN width (the notification path's model): + // the old 32-bit UDInt round-trip wrote a 16-bit parent's neighbours and overflowed on a + // signed parent with its sign bit set. + var (parent, _, symbol, status) = await DescribeSymbolAsync(parentPath, cancellationToken) + .ConfigureAwait(false); + if (status != TwinCATStatusMapper.Good) return status; + if (TwinCATBitAccess.CheckParent(bit, parent) is not null) + return TwinCATBitAccess.ApplyBitWrite(parent, ReadOnlySpan.Empty, bit, value, out _); + + var (bytes, readStatus) = await ReadRawAsync(parentPath, symbol!, parent.ByteSize, cancellationToken) + .ConfigureAwait(false); + if (readStatus != TwinCATStatusMapper.Good) return readStatus; + var modify = TwinCATBitAccess.ApplyBitWrite(parent, bytes, bit, value, out var updated); + if (modify != TwinCATStatusMapper.Good) return modify; + return await WriteRawAsync(parentPath, symbol!, updated, cancellationToken).ConfigureAwait(false); + } + catch (AdsErrorException ex) + { + return MapAndSignal((uint)ex.ErrorCode); + } + } + + /// + /// Writes raw bytes to a resolved symbol — the mirror of : by its index + /// group/offset, or through a fresh variable handle for a handle-addressed symbol (0xF005). + /// + private async Task WriteRawAsync( + string symbolPath, IAdsSymbol symbol, ReadOnlyMemory data, CancellationToken cancellationToken) + { + if (symbol.IndexGroup != SymbolValueByHandle) + { + var direct = await _client.WriteAsync(symbol.IndexGroup, symbol.IndexOffset, data, cancellationToken) + .ConfigureAwait(false); + return direct.ErrorCode == AdsErrorCode.NoError ? TwinCATStatusMapper.Good : MapAndSignal((uint)direct.ErrorCode); + } + + var handle = await _client.CreateVariableHandleAsync(symbolPath, cancellationToken).ConfigureAwait(false); + if (handle.ErrorCode != AdsErrorCode.NoError) return MapAndSignal((uint)handle.ErrorCode); + try + { + var write = await _client.WriteAsync(handle.Handle, data, cancellationToken).ConfigureAwait(false); + return write.ErrorCode == AdsErrorCode.NoError ? TwinCATStatusMapper.Good : MapAndSignal((uint)write.ErrorCode); + } + finally + { + try { await _client.DeleteVariableHandleAsync(handle.Handle, CancellationToken.None).ConfigureAwait(false); } + catch { /* best-effort release */ } + } + } + /// public async Task ProbeAsync(CancellationToken cancellationToken) { diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/ITwinCATClient.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/ITwinCATClient.cs index 3849edd0..1ffeb6fa 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/ITwinCATClient.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/ITwinCATClient.cs @@ -119,6 +119,22 @@ public interface ITwinCATClient : IDisposable object? value, CancellationToken cancellationToken); + /// + /// Sets or clears one bit of an integer parent word — the write half of a BOOL-within-word tag + /// (MAIN.Flags.3; ADS has no symbol for WORD.N). A read-modify-write of the parent's raw + /// bytes at the parent's OWN resolved width (), so a 16-bit + /// parent is never written as 32 bits and a signed parent's sign bit is just a bit. The caller + /// serialises concurrent writers to the same parent; the PLC program writing the word between the + /// read and the write is inherent to RMW. + /// + /// The parent word's ADS symbol path (the bit path without its .N). + /// The bit index (0 = least significant bit of the first byte). + /// The bit value to write. + /// Cancellation token for the read and the write. + /// The mapped OPC UA status: Good; the parent read's status; BadOutOfRange for a bit + /// beyond the parent; BadTypeMismatch for a parent that is not an integer word. + Task WriteBitAsync(string parentPath, int bit, bool value, CancellationToken cancellationToken); + /// /// Cheap health probe — returns true when the target's AMS state is reachable. /// Used by 's probe loop. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATBitAccess.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATBitAccess.cs index 186e1cee..277bd317 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATBitAccess.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATBitAccess.cs @@ -32,6 +32,36 @@ internal static class TwinCATBitAccess : $"bit {bit} is outside the {parent.ByteSize * 8}-bit parent {parent.TypeName}"; } + /// + /// The modify step of a bit write's read-modify-write: checks the bit against the parent + /// () and returns the parent's bytes — exactly parent.ByteSize of them — + /// with one bit set or cleared. Works on raw bytes, so the parent's width and signedness play no part. + /// + /// The parent symbol's resolved shape. + /// The parent's current bytes as read from the PLC. + /// The bit index. + /// The bit value. + /// The bytes to write back when the result is Good; otherwise empty. + /// Good; BadOutOfRange for a bit beyond the parent; BadTypeMismatch for a parent + /// that is not an integer word; BadCommunicationError for a read shorter than the parent + /// (writing it back would zero the missing bytes). + public static uint ApplyBitWrite(TwinCATPlcSymbolShape parent, ReadOnlySpan current, int bit, bool value, + out byte[] updated) + { + updated = []; + if (CheckParent(bit, parent) is not null) + return parent.Kind != TwinCATPlcKind.Scalar || parent.IsFloat + ? TwinCATStatusMapper.BadTypeMismatch + : TwinCATStatusMapper.BadOutOfRange; + if (current.Length < parent.ByteSize) return TwinCATStatusMapper.BadCommunicationError; + + var bytes = current[..parent.ByteSize].ToArray(); + var mask = (byte)(1 << (bit % 8)); + bytes[bit / 8] = value ? (byte)(bytes[bit / 8] | mask) : (byte)(bytes[bit / 8] & ~mask); + updated = bytes; + return TwinCATStatusMapper.Good; + } + /// Reads bit of a little-endian parent word. /// The parent word's raw bytes. /// The bit index (0 = least significant bit of the first byte). diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs index 7c95a8f0..2a5d4d94 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs @@ -511,38 +511,21 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery var client = await EnsureConnectedAsync(device, cancellationToken).ConfigureAwait(false); var parsed = TwinCATSymbolPath.TryParse(def.SymbolPath); - // BOOL-within-word write — read-modify-write of the parent word. Mirrors the bit-read - // (AdsTwinCATClient.ReadValueAsync) which reads the parent as uint: read the parent as - // UDInt (-> uint), flip the bit, write it back, all under a per-parent lock. + // BOOL-within-word write — a read-modify-write of the parent word at the parent's OWN + // width and on raw bytes (the client resolves the parent and checks the bit against it, the + // notification path's model), under a per-parent lock so concurrent bit writers to the same + // word don't lose each other's updates. if (def.DataType == TwinCATDataType.Bool && parsed?.BitIndex is int bit) { var parentPath = (parsed with { BitIndex = null }).ToAdsSymbolName(); + var bitValue = Convert.ToBoolean(w.Value); var gate = _bitRmwLocks.GetOrAdd( $"{def.DeviceHostAddress}|{parentPath}", static _ => new SemaphoreSlim(1, 1)); await gate.WaitAsync(cancellationToken).ConfigureAwait(false); try { - var (parentValue, readStatus) = await client.ReadValueAsync( - parentPath, TwinCATDataType.UDInt, null, null, cancellationToken).ConfigureAwait(false); - if (readStatus != TwinCATStatusMapper.Good) - { - results[i] = new WriteResult(readStatus); - } - else if (parentValue is null) - { - // Good status but no value — treating null as 0 would write a zeroed - // word and clear every bit set on the device. Surface a Bad status and - // write nothing rather than corrupt the parent word. - results[i] = new WriteResult(TwinCATStatusMapper.BadCommunicationError); - } - else - { - var word = Convert.ToUInt32(parentValue); - var updated = Convert.ToBoolean(w.Value) ? word | (1u << bit) : word & ~(1u << bit); - var writeStatus = await client.WriteValueAsync( - parentPath, TwinCATDataType.UDInt, null, updated, cancellationToken).ConfigureAwait(false); - results[i] = new WriteResult(writeStatus); - } + results[i] = new WriteResult(await client.WriteBitAsync( + parentPath, bit, bitValue, cancellationToken).ConfigureAwait(false)); } finally { diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/FakeTwinCATClient.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/FakeTwinCATClient.cs index 2ad33e42..e31340a5 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/FakeTwinCATClient.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/FakeTwinCATClient.cs @@ -136,6 +136,40 @@ internal class FakeTwinCATClient : ITwinCATClient return Task.FromResult(status); } + /// Raw bytes of bit-write parent words, by parent path (#739 H18). The byte count is the parent's width. + public Dictionary ParentWords { get; } = new(StringComparer.OrdinalIgnoreCase); + /// Optional parent shapes; absent = an unsigned integer word as wide as its entry. + public Dictionary ParentShapes { get; } = new(StringComparer.OrdinalIgnoreCase); + /// Gets the log of every call. + public List<(string parent, int bit, bool value)> BitWriteLog { get; } = new(); + /// When set, yields between its read and its write so a missing + /// per-parent lock in the driver loses updates. + public bool YieldInBitWrite { get; set; } + + /// Simulates the parent-width bit RMW with the driver's own . + /// The parent word's symbol path. + /// The bit index. + /// The bit value. + /// The cancellation token. + /// The write status. + public virtual async Task WriteBitAsync(string parentPath, int bit, bool value, CancellationToken ct) + { + if (ThrowOnWrite) throw Exception ?? new InvalidOperationException(); + lock (BitWriteLog) BitWriteLog.Add((parentPath, bit, value)); + if (ReadStatuses.TryGetValue(parentPath, out var readStatus) && readStatus != TwinCATStatusMapper.Good) + return readStatus; + byte[] current; + lock (ParentWords) current = ParentWords.TryGetValue(parentPath, out var w) ? [.. w] : []; + var shape = ParentShapes.TryGetValue(parentPath, out var sh) + ? sh + : new TwinCATPlcSymbolShape(TwinCATPlcKind.Scalar, current.Length == 0 ? 4 : current.Length, false, "UDINT"); + if (YieldInBitWrite) await Task.Yield(); + var status = TwinCATBitAccess.ApplyBitWrite(shape, current, bit, value, out var updated); + if (status != TwinCATStatusMapper.Good) return status; + lock (ParentWords) ParentWords[parentPath] = updated; + return WriteStatuses.TryGetValue(parentPath, out var s) ? s : TwinCATStatusMapper.Good; + } + /// Simulates probing the connection status. /// The cancellation token. /// A task that returns the probe result. diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATBitAccessTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATBitAccessTests.cs index 3dc22507..a5b339c8 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATBitAccessTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATBitAccessTests.cs @@ -55,4 +55,50 @@ public sealed class TwinCATBitAccessTests TwinCATBitAccess.CheckParent(0, TwinCATPlcSymbolShape.Classify("ST_X", DataTypeCategory.Struct, 8)).ShouldNotBeNull(); TwinCATBitAccess.CheckParent(31, TwinCATPlcSymbolShape.Classify("DWORD", DataTypeCategory.Primitive, 4)).ShouldBeNull(); } + + // ---- #739 H18: the bit WRITE is a read-modify-write at the parent's own width ---- + + /// A bit write flips exactly one bit of the parent's raw bytes at the parent's width. + [Fact] + public void ApplyBitWrite_flips_one_bit_at_the_parent_width() + { + var word = TwinCATPlcSymbolShape.Classify("WORD", DataTypeCategory.Primitive, 2); + TwinCATBitAccess.ApplyBitWrite(word, [0x01, 0x00], 9, true, out var set).ShouldBe(TwinCATStatusMapper.Good); + set.ShouldBe(new byte[] { 0x01, 0x02 }); + TwinCATBitAccess.ApplyBitWrite(word, [0xFF, 0xFF], 3, false, out var cleared).ShouldBe(TwinCATStatusMapper.Good); + cleared.ShouldBe(new byte[] { 0xF7, 0xFF }); + } + + /// + /// A signed parent with its sign bit set is plain bytes — the old UDInt round-trip read it back as + /// a value ≥ 2³¹ and a checked conversion turned ANY bit write into BadOutOfRange. + /// + [Fact] + public void ApplyBitWrite_on_a_negative_signed_parent_preserves_the_sign_bit() + { + var intWord = TwinCATPlcSymbolShape.Classify("INT", DataTypeCategory.Primitive, 2); + TwinCATBitAccess.ApplyBitWrite(intWord, [0x00, 0x80], 0, true, out var updated).ShouldBe(TwinCATStatusMapper.Good); + updated.ShouldBe(new byte[] { 0x01, 0x80 }); + } + + /// A bit beyond the parent is BadOutOfRange; a non-integer parent BadTypeMismatch; nothing is produced. + [Fact] + public void ApplyBitWrite_refuses_a_bit_the_parent_does_not_have() + { + var word = TwinCATPlcSymbolShape.Classify("UINT", DataTypeCategory.Primitive, 2); + TwinCATBitAccess.ApplyBitWrite(word, [0, 0], 16, true, out var none).ShouldBe(TwinCATStatusMapper.BadOutOfRange); + none.ShouldBeEmpty(); + var real = TwinCATPlcSymbolShape.Classify("REAL", DataTypeCategory.Primitive, 4); + TwinCATBitAccess.ApplyBitWrite(real, [0, 0, 0, 0], 1, true, out _).ShouldBe(TwinCATStatusMapper.BadTypeMismatch); + } + + /// A parent read shorter than the parent's width is never written back (it would zero bytes). + [Fact] + public void ApplyBitWrite_refuses_a_short_parent_buffer() + { + var dword = TwinCATPlcSymbolShape.Classify("DWORD", DataTypeCategory.Primitive, 4); + TwinCATBitAccess.ApplyBitWrite(dword, [0xFF, 0xFF], 1, true, out var none) + .ShouldBe(TwinCATStatusMapper.BadCommunicationError); + none.ShouldBeEmpty(); + } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATReadWriteTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATReadWriteTests.cs index c81e7b88..d7dbcc0d 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATReadWriteTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATReadWriteTests.cs @@ -296,23 +296,23 @@ public sealed class TwinCATReadWriteTests factory.Clients[0].DisposeCount.ShouldBe(1); } - // ---- BOOL-within-word RMW writes ---- + // ---- BOOL-within-word RMW writes (#739 H18: at the parent's own width, on raw bytes) ---- - /// A BOOL-within-word set reads the parent word, ORs the bit, writes it back as UDInt. + /// A BOOL-within-word set read-modify-writes the parent word's raw bytes through WriteBitAsync. [Fact] - public async Task Bit_set_RMWs_parent_word_as_UDInt() + public async Task Bit_set_RMWs_the_parent_word_bytes() { var (drv, factory) = NewDriver( new TwinCATTagDefinition("Flag", "ads://5.23.91.23.1.1:851", "MAIN.Flags.3", TwinCATDataType.Bool)); - factory.Customise = () => new FakeTwinCATClient { Values = { ["MAIN.Flags"] = 0b0001u } }; + factory.Customise = () => new FakeTwinCATClient { ParentWords = { ["MAIN.Flags"] = [0b0001, 0, 0, 0] } }; await drv.InitializeAsync("{}", CancellationToken.None); var results = await drv.WriteAsync([new WriteRequest("Flag", true)], CancellationToken.None); results.Single().StatusCode.ShouldBe(TwinCATStatusMapper.Good); - factory.Clients[0].Values["MAIN.Flags"].ShouldBe(0b1001u); - factory.Clients[0].WriteLog.ShouldContain(e => - e.symbol == "MAIN.Flags" && e.type == TwinCATDataType.UDInt && e.bit == null); + factory.Clients[0].ParentWords["MAIN.Flags"].ShouldBe(new byte[] { 0b1001, 0, 0, 0 }); + factory.Clients[0].BitWriteLog.ShouldBe([("MAIN.Flags", 3, true)]); + factory.Clients[0].WriteLog.ShouldBeEmpty(); // no 32-bit UDInt round-trip any more } /// A BOOL-within-word clear preserves the other bits in the parent word. @@ -321,12 +321,12 @@ public sealed class TwinCATReadWriteTests { var (drv, factory) = NewDriver( new TwinCATTagDefinition("Flag", "ads://5.23.91.23.1.1:851", "MAIN.Flags.3", TwinCATDataType.Bool)); - factory.Customise = () => new FakeTwinCATClient { Values = { ["MAIN.Flags"] = 0xFFFFu } }; + factory.Customise = () => new FakeTwinCATClient { ParentWords = { ["MAIN.Flags"] = [0xFF, 0xFF] } }; await drv.InitializeAsync("{}", CancellationToken.None); await drv.WriteAsync([new WriteRequest("Flag", false)], CancellationToken.None); - factory.Clients[0].Values["MAIN.Flags"].ShouldBe(0xFFF7u); + factory.Clients[0].ParentWords["MAIN.Flags"].ShouldBe(new byte[] { 0xF7, 0xFF }); } /// RMW works on a DWORD parent (bit 20 set above the 16-bit boundary). @@ -335,12 +335,31 @@ public sealed class TwinCATReadWriteTests { var (drv, factory) = NewDriver( new TwinCATTagDefinition("Hi", "ads://5.23.91.23.1.1:851", "GVL.Status.20", TwinCATDataType.Bool)); - factory.Customise = () => new FakeTwinCATClient { Values = { ["GVL.Status"] = 0u } }; + factory.Customise = () => new FakeTwinCATClient { ParentWords = { ["GVL.Status"] = [0, 0, 0, 0] } }; await drv.InitializeAsync("{}", CancellationToken.None); await drv.WriteAsync([new WriteRequest("Hi", true)], CancellationToken.None); - factory.Clients[0].Values["GVL.Status"].ShouldBe(1u << 20); + BitConverter.ToUInt32(factory.Clients[0].ParentWords["GVL.Status"]).ShouldBe(1u << 20); + } + + /// + /// A bit beyond a 16-bit parent is refused BadOutOfRange and nothing is written — before, the hardcoded + /// 32-bit RMW wrote into the neighbouring bytes. No health write: a refusal is not a comms fault. + /// + [Fact] + public async Task Bit_beyond_a_16_bit_parent_is_refused_without_writing() + { + var (drv, factory) = NewDriver( + new TwinCATTagDefinition("Hi", "ads://5.23.91.23.1.1:851", "MAIN.Word.20", TwinCATDataType.Bool)); + factory.Customise = () => new FakeTwinCATClient { ParentWords = { ["MAIN.Word"] = [0, 0] } }; + await drv.InitializeAsync("{}", CancellationToken.None); + + var results = await drv.WriteAsync([new WriteRequest("Hi", true)], CancellationToken.None); + + results.Single().StatusCode.ShouldBe(TwinCATStatusMapper.BadOutOfRange); + factory.Clients[0].ParentWords["MAIN.Word"].ShouldBe(new byte[] { 0, 0 }); + drv.GetHealth().State.ShouldBe(DriverState.Healthy); } /// A failed parent read short-circuits the RMW and surfaces the read status. @@ -351,6 +370,7 @@ public sealed class TwinCATReadWriteTests new TwinCATTagDefinition("Flag", "ads://5.23.91.23.1.1:851", "MAIN.Flags.3", TwinCATDataType.Bool)); factory.Customise = () => new FakeTwinCATClient { + ParentWords = { ["MAIN.Flags"] = [0xAA, 0] }, ReadStatuses = { ["MAIN.Flags"] = TwinCATStatusMapper.BadNodeIdUnknown }, }; await drv.InitializeAsync("{}", CancellationToken.None); @@ -358,25 +378,24 @@ public sealed class TwinCATReadWriteTests var results = await drv.WriteAsync([new WriteRequest("Flag", true)], CancellationToken.None); results.Single().StatusCode.ShouldBe(TwinCATStatusMapper.BadNodeIdUnknown); - factory.Clients[0].WriteLog.ShouldBeEmpty(); + factory.Clients[0].ParentWords["MAIN.Flags"].ShouldBe(new byte[] { 0xAA, 0 }); } - /// A Good parent read that yields a null value must NOT zero the word — it surfaces - /// a Bad status and writes nothing (treating null as 0 would clear every set bit). + /// A Good parent read that yields no bytes must NOT zero the word — it surfaces a Bad status + /// and writes nothing (treating it as 0 would clear every set bit). [Fact] - public async Task Bit_write_with_null_parent_value_does_not_zero_the_word() + public async Task Bit_write_with_no_parent_bytes_does_not_zero_the_word() { var (drv, factory) = NewDriver( new TwinCATTagDefinition("Flag", "ads://5.23.91.23.1.1:851", "MAIN.Flags.3", TwinCATDataType.Bool)); - // Do NOT seed Values["MAIN.Flags"] — the fake returns (null, Good) for the parent read. + // Do NOT seed ParentWords["MAIN.Flags"] — the fake answers the parent read Good with no bytes. factory.Customise = () => new FakeTwinCATClient(); await drv.InitializeAsync("{}", CancellationToken.None); var results = await drv.WriteAsync([new WriteRequest("Flag", true)], CancellationToken.None); results.Single().StatusCode.ShouldBe(TwinCATStatusMapper.BadCommunicationError); - // No parent write attempted — the word is left untouched on the device. - factory.Clients[0].WriteLog.ShouldBeEmpty(); + factory.Clients[0].ParentWords.ShouldNotContainKey("MAIN.Flags"); } /// Concurrent bit writes to the same word compose correctly (per-parent lock). @@ -387,12 +406,12 @@ public sealed class TwinCATReadWriteTests .Select(b => new TwinCATTagDefinition($"Bit{b}", "ads://5.23.91.23.1.1:851", $"MAIN.Flags.{b}", TwinCATDataType.Bool)) .ToArray(); var (drv, factory) = NewDriver(tags); - factory.Customise = () => new FakeTwinCATClient { Values = { ["MAIN.Flags"] = 0u } }; + factory.Customise = () => new FakeTwinCATClient { YieldInBitWrite = true, ParentWords = { ["MAIN.Flags"] = [0, 0] } }; await drv.InitializeAsync("{}", CancellationToken.None); await Task.WhenAll(Enumerable.Range(0, 8).Select(b => drv.WriteAsync([new WriteRequest($"Bit{b}", true)], CancellationToken.None))); - Convert.ToUInt32(factory.Clients[0].Values["MAIN.Flags"]).ShouldBe(0xFFu); + factory.Clients[0].ParentWords["MAIN.Flags"].ShouldBe(new byte[] { 0xFF, 0 }); } }