From 8c309aebf39dc54fe842d1201dc2dc5c0d566f03 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sun, 19 Apr 2026 20:25:27 -0400 Subject: [PATCH] =?UTF-8?q?RMW=20pass=201=20=E2=80=94=20Modbus=20BitInRegi?= =?UTF-8?q?ster=20+=20FOCAS=20PMC=20Bit=20write=20paths.=20First=20half=20?= =?UTF-8?q?of=20task=20#181=20=E2=80=94=20the=20two=20drivers=20where=20re?= =?UTF-8?q?ad-modify-write=20is=20a=20clean=20protocol-level=20insertion?= =?UTF-8?q?=20(Modbus=20FC03/FC06=20round-trip=20+=20FOCAS=20pmc=5Frdpmcrn?= =?UTF-8?q?g=20/=20pmc=5Fwrpmcrng=20round-trip).=20Per-driver=20SemaphoreS?= =?UTF-8?q?lim=20registry=20keyed=20on=20the=20parent=20word=20address=20s?= =?UTF-8?q?erialises=20concurrent=20bit=20writes=20so=20two=20writers=20ta?= =?UTF-8?q?rgeting=20different=20bits=20in=20the=20same=20word=20don't=20l?= =?UTF-8?q?ose=20one=20another's=20update.=20Modbus=20=E2=80=94=20ModbusDr?= =?UTF-8?q?iver=20gains=20WriteBitInRegisterAsync=20+=20=5FrmwLocks=20Conc?= =?UTF-8?q?urrentDictionary.=20WriteOneAsync=20routes=20BitInRegister=20(H?= =?UTF-8?q?oldingRegisters=20region=20only)=20through=20RMW=20ahead=20of?= =?UTF-8?q?=20the=20normal=20encode=20path.=20Read=20uses=20FC03=20Read=20?= =?UTF-8?q?Holding=20Registers=20for=201=20register=20at=20tag.Address,=20?= =?UTF-8?q?bit-op=20on=20the=20returned=20ushort=20via=20(current=20|=201 --- .../FwlibFocasClient.cs | 81 ++++++++-- .../ModbusDriver.cs | 62 +++++++- .../FocasPmcBitRmwTests.cs | 123 +++++++++++++++ .../FwlibNativeHelperTests.cs | 12 +- .../ModbusBitRmwTests.cs | 141 ++++++++++++++++++ .../ModbusDataTypeTests.cs | 7 +- 6 files changed, 409 insertions(+), 17 deletions(-) create mode 100644 tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasPmcBitRmwTests.cs create mode 100644 tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusBitRmwTests.cs diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FwlibFocasClient.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FwlibFocasClient.cs index 5fb65e6..63a55fd 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FwlibFocasClient.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FwlibFocasClient.cs @@ -1,4 +1,5 @@ using System.Buffers.Binary; +using System.Collections.Concurrent; namespace ZB.MOM.WW.OtOpcUa.Driver.FOCAS; @@ -24,6 +25,13 @@ internal sealed class FwlibFocasClient : IFocasClient private ushort _handle; private bool _connected; + // Per-PMC-byte RMW lock registry. Bit writes to the same byte get serialised so two + // concurrent bit updates don't lose one another's modification. Key = "{addrType}:{byteAddr}". + private readonly ConcurrentDictionary _rmwLocks = new(); + + private SemaphoreSlim GetRmwLock(short addrType, int byteAddr) => + _rmwLocks.GetOrAdd($"{addrType}:{byteAddr}", _ => new SemaphoreSlim(1, 1)); + public bool IsConnected => _connected; public Task ConnectAsync(FocasHostAddress address, TimeSpan timeout, CancellationToken cancellationToken) @@ -55,21 +63,72 @@ internal sealed class FwlibFocasClient : IFocasClient }; } - public Task WriteAsync( + public async Task WriteAsync( FocasAddress address, FocasDataType type, object? value, CancellationToken cancellationToken) { - if (!_connected) return Task.FromResult(FocasStatusMapper.BadCommunicationError); + if (!_connected) return FocasStatusMapper.BadCommunicationError; cancellationToken.ThrowIfCancellationRequested(); return address.Kind switch { - FocasAreaKind.Pmc => Task.FromResult(WritePmc(address, type, value)), - FocasAreaKind.Parameter => Task.FromResult(WriteParameter(address, type, value)), - FocasAreaKind.Macro => Task.FromResult(WriteMacro(address, value)), - _ => Task.FromResult(FocasStatusMapper.BadNotSupported), + FocasAreaKind.Pmc when type == FocasDataType.Bit && address.BitIndex is int => + await WritePmcBitAsync(address, Convert.ToBoolean(value), cancellationToken).ConfigureAwait(false), + FocasAreaKind.Pmc => WritePmc(address, type, value), + FocasAreaKind.Parameter => WriteParameter(address, type, value), + FocasAreaKind.Macro => WriteMacro(address, value), + _ => FocasStatusMapper.BadNotSupported, }; } + /// + /// Read-modify-write one bit within a PMC byte. Acquires a per-byte semaphore so + /// concurrent bit writes against the same byte serialise and neither loses its update. + /// + private async Task WritePmcBitAsync( + FocasAddress address, bool newValue, CancellationToken cancellationToken) + { + var addrType = FocasPmcAddrType.FromLetter(address.PmcLetter ?? "") ?? (short)0; + var bit = address.BitIndex ?? 0; + if (bit is < 0 or > 7) + throw new InvalidOperationException( + $"PMC bit index {bit} out of range (0-7) for {address.Canonical}."); + + var rmwLock = GetRmwLock(addrType, address.Number); + await rmwLock.WaitAsync(cancellationToken).ConfigureAwait(false); + try + { + // Read the parent byte. + var readBuf = new FwlibNative.IODBPMC { Data = new byte[40] }; + var readRet = FwlibNative.PmcRdPmcRng( + _handle, addrType, FocasPmcDataType.Byte, + (ushort)address.Number, (ushort)address.Number, 8 + 1, ref readBuf); + if (readRet != 0) return FocasStatusMapper.MapFocasReturn(readRet); + + var current = readBuf.Data[0]; + var updated = newValue + ? (byte)(current | (1 << bit)) + : (byte)(current & ~(1 << bit)); + + // Write the updated byte. + var writeBuf = new FwlibNative.IODBPMC + { + TypeA = addrType, + TypeD = FocasPmcDataType.Byte, + DatanoS = (ushort)address.Number, + DatanoE = (ushort)address.Number, + Data = new byte[40], + }; + writeBuf.Data[0] = updated; + + var writeRet = FwlibNative.PmcWrPmcRng(_handle, 8 + 1, ref writeBuf); + return writeRet == 0 ? FocasStatusMapper.Good : FocasStatusMapper.MapFocasReturn(writeRet); + } + finally + { + rmwLock.Release(); + } + } + public Task ProbeAsync(CancellationToken cancellationToken) { if (!_connected) return Task.FromResult(false); @@ -216,11 +275,11 @@ internal sealed class FwlibFocasClient : IFocasClient switch (type) { case FocasDataType.Bit: - // Bit-in-byte write is a read-modify-write at the PMC level — the underlying - // pmc_wrpmcrng takes a byte payload, so caller must set the bit on a byte they - // just read. This path is flagged for the follow-up RMW work in task #181. - throw new NotSupportedException( - "FOCAS Bit writes require read-modify-write; tracked in task #181."); + // PMC Bit writes with a non-null bitIndex go through WritePmcBitAsync's RMW path + // upstream. This branch only fires when a caller passes Bit with no bitIndex — + // treat the value as a whole-byte boolean (non-zero / zero). + data[0] = Convert.ToBoolean(value) ? (byte)1 : (byte)0; + break; case FocasDataType.Byte: data[0] = (byte)(sbyte)Convert.ToSByte(value); break; diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs index 66f0ed2..2b17486 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs @@ -264,8 +264,27 @@ public sealed class ModbusDriver return results; } + // BitInRegister writes need a read-modify-write against the full holding register. A + // per-register lock keeps concurrent bit-write callers from stomping on each other — + // Write bit 0 and Write bit 5 targeting the same register can arrive on separate + // subscriber threads, and without serialising the RMW the second-to-commit value wins + // + the first bit update is lost. + private readonly System.Collections.Concurrent.ConcurrentDictionary _rmwLocks = new(); + + private SemaphoreSlim GetRmwLock(ushort address) => + _rmwLocks.GetOrAdd(address, _ => new SemaphoreSlim(1, 1)); + private async Task WriteOneAsync(IModbusTransport transport, ModbusTagDefinition tag, object? value, CancellationToken ct) { + // BitInRegister → RMW dispatch ahead of the normal encode path so the lock + read-modify- + // write sequence doesn't hit EncodeRegister's defensive throw. + if (tag.DataType == ModbusDataType.BitInRegister && + tag.Region is ModbusRegion.HoldingRegisters) + { + await WriteBitInRegisterAsync(transport, tag, value, ct).ConfigureAwait(false); + return; + } + switch (tag.Region) { case ModbusRegion.Coils: @@ -309,6 +328,44 @@ public sealed class ModbusDriver } } + /// + /// Read-modify-write one bit in a holding register. FC03 → bit-swap → FC06. Serialised + /// against other bit writes targeting the same register via . + /// + private async Task WriteBitInRegisterAsync( + IModbusTransport transport, ModbusTagDefinition tag, object? value, CancellationToken ct) + { + var bit = tag.BitIndex; + if (bit > 15) + throw new InvalidOperationException( + $"BitInRegister bit index {bit} out of range (0-15) for tag {tag.Name}."); + var on = Convert.ToBoolean(value); + + var rmwLock = GetRmwLock(tag.Address); + await rmwLock.WaitAsync(ct).ConfigureAwait(false); + try + { + // FC03 read 1 holding register at tag.Address. + var readPdu = new byte[] { 0x03, (byte)(tag.Address >> 8), (byte)(tag.Address & 0xFF), 0x00, 0x01 }; + var readResp = await transport.SendAsync(_options.UnitId, readPdu, ct).ConfigureAwait(false); + // resp = [fc][byte-count=2][hi][lo] + var current = (ushort)((readResp[2] << 8) | readResp[3]); + + var updated = on + ? (ushort)(current | (1 << bit)) + : (ushort)(current & ~(1 << bit)); + + // FC06 write single holding register. + var writePdu = new byte[] { 0x06, (byte)(tag.Address >> 8), (byte)(tag.Address & 0xFF), + (byte)(updated >> 8), (byte)(updated & 0xFF) }; + await transport.SendAsync(_options.UnitId, writePdu, ct).ConfigureAwait(false); + } + finally + { + rmwLock.Release(); + } + } + // ---- ISubscribable (polling overlay via shared engine) ---- public Task SubscribeAsync( @@ -575,8 +632,11 @@ public sealed class ModbusDriver return b; } case ModbusDataType.BitInRegister: + // Reached only if BitInRegister is somehow passed outside the HoldingRegisters + // path. Normal BitInRegister writes dispatch through WriteBitInRegisterAsync via + // the RMW shortcut in WriteOneAsync. throw new InvalidOperationException( - "BitInRegister writes require a read-modify-write; not supported in PR 24 (separate follow-up)."); + "BitInRegister writes must go through WriteBitInRegisterAsync (HoldingRegisters region only)."); default: throw new InvalidOperationException($"Non-register data type {tag.DataType}"); } diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasPmcBitRmwTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasPmcBitRmwTests.cs new file mode 100644 index 0000000..21909b5 --- /dev/null +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasPmcBitRmwTests.cs @@ -0,0 +1,123 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.FOCAS; + +namespace ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests; + +[Trait("Category", "Unit")] +public sealed class FocasPmcBitRmwTests +{ + /// + /// Fake client simulating PMC byte storage + exposing it as a sbyte so RMW callers can + /// observe the read-modify-write round-trip. ReadAsync for a Bit with bitIndex surfaces + /// the current bit; WriteAsync stores the full byte the driver issues. + /// + private sealed class PmcRmwFake : FakeFocasClient + { + public byte[] PmcBytes { get; } = new byte[1024]; + + public override Task<(object? value, uint status)> ReadAsync( + FocasAddress address, FocasDataType type, CancellationToken ct) + { + if (address.Kind == FocasAreaKind.Pmc && type == FocasDataType.Byte) + return Task.FromResult(((object?)(sbyte)PmcBytes[address.Number], FocasStatusMapper.Good)); + if (address.Kind == FocasAreaKind.Pmc && type == FocasDataType.Bit && address.BitIndex is int bit) + return Task.FromResult(((object?)((PmcBytes[address.Number] & (1 << bit)) != 0), FocasStatusMapper.Good)); + return base.ReadAsync(address, type, ct); + } + + public override Task WriteAsync( + FocasAddress address, FocasDataType type, object? value, CancellationToken ct) + { + // Driver writes the full byte after RMW (type==Byte with full byte value), OR a raw + // bit write (type==Bit, bitIndex non-null) — depending on how the driver routes it. + if (address.Kind == FocasAreaKind.Pmc && type == FocasDataType.Byte) + { + PmcBytes[address.Number] = (byte)Convert.ToSByte(value); + return Task.FromResult(FocasStatusMapper.Good); + } + if (address.Kind == FocasAreaKind.Pmc && type == FocasDataType.Bit && address.BitIndex is int bit) + { + var current = PmcBytes[address.Number]; + PmcBytes[address.Number] = Convert.ToBoolean(value) + ? (byte)(current | (1 << bit)) + : (byte)(current & ~(1 << bit)); + return Task.FromResult(FocasStatusMapper.Good); + } + return base.WriteAsync(address, type, value, ct); + } + } + + private static (FocasDriver drv, PmcRmwFake fake) NewDriver(params FocasTagDefinition[] tags) + { + var fake = new PmcRmwFake(); + var factory = new FakeFocasClientFactory { Customise = () => fake }; + var drv = new FocasDriver(new FocasDriverOptions + { + Devices = [new FocasDeviceOptions("focas://10.0.0.5:8193")], + Tags = tags, + Probe = new FocasProbeOptions { Enabled = false }, + }, "drv-1", factory); + return (drv, fake); + } + + [Fact] + public async Task Bit_set_surfaces_as_Good_status_and_flips_bit() + { + var (drv, fake) = NewDriver( + new FocasTagDefinition("Run", "focas://10.0.0.5:8193", "R100.3", FocasDataType.Bit)); + await drv.InitializeAsync("{}", CancellationToken.None); + fake.PmcBytes[100] = 0b0000_0001; + + var results = await drv.WriteAsync([new WriteRequest("Run", true)], CancellationToken.None); + + results.Single().StatusCode.ShouldBe(FocasStatusMapper.Good); + fake.PmcBytes[100].ShouldBe((byte)0b0000_1001); + } + + [Fact] + public async Task Bit_clear_preserves_other_bits() + { + var (drv, fake) = NewDriver( + new FocasTagDefinition("Flag", "focas://10.0.0.5:8193", "R100.3", FocasDataType.Bit)); + await drv.InitializeAsync("{}", CancellationToken.None); + fake.PmcBytes[100] = 0xFF; + + await drv.WriteAsync([new WriteRequest("Flag", false)], CancellationToken.None); + + fake.PmcBytes[100].ShouldBe((byte)0b1111_0111); + } + + [Fact] + public async Task Subsequent_bit_sets_in_same_byte_compose_correctly() + { + var tags = Enumerable.Range(0, 8) + .Select(b => new FocasTagDefinition($"Bit{b}", "focas://10.0.0.5:8193", $"R100.{b}", FocasDataType.Bit)) + .ToArray(); + var (drv, fake) = NewDriver(tags); + await drv.InitializeAsync("{}", CancellationToken.None); + fake.PmcBytes[100] = 0; + + for (var b = 0; b < 8; b++) + await drv.WriteAsync([new WriteRequest($"Bit{b}", true)], CancellationToken.None); + + fake.PmcBytes[100].ShouldBe((byte)0xFF); + } + + [Fact] + public async Task Bit_write_to_different_bytes_does_not_contend() + { + var tags = Enumerable.Range(0, 4) + .Select(i => new FocasTagDefinition($"Bit{i}", "focas://10.0.0.5:8193", $"R{50 + i}.0", FocasDataType.Bit)) + .ToArray(); + var (drv, fake) = NewDriver(tags); + await drv.InitializeAsync("{}", CancellationToken.None); + + await Task.WhenAll(Enumerable.Range(0, 4).Select(i => + drv.WriteAsync([new WriteRequest($"Bit{i}", true)], CancellationToken.None))); + + for (var i = 0; i < 4; i++) + fake.PmcBytes[50 + i].ShouldBe((byte)0x01); + } +} diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FwlibNativeHelperTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FwlibNativeHelperTests.cs index 3593d9b..31136b5 100644 --- a/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FwlibNativeHelperTests.cs +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FwlibNativeHelperTests.cs @@ -80,11 +80,17 @@ public sealed class FwlibNativeHelperTests } [Fact] - public void EncodePmcValue_Bit_throws_NotSupported_for_RMW_gap() + public void EncodePmcValue_Bit_without_bit_index_writes_byte_boolean() { + // Task #181 closed the Bit-write gap — PMC Bit with a bitIndex now routes through + // WritePmcBitAsync's RMW path upstream, and raw EncodePmcValue only gets the + // no-bit-index case (treated as a whole-byte boolean). var buf = new byte[40]; - Should.Throw(() => - FwlibFocasClient.EncodePmcValue(buf, FocasDataType.Bit, true, bitIndex: 3)); + FwlibFocasClient.EncodePmcValue(buf, FocasDataType.Bit, true, bitIndex: null); + buf[0].ShouldBe((byte)1); + + FwlibFocasClient.EncodePmcValue(buf, FocasDataType.Bit, false, bitIndex: null); + buf[0].ShouldBe((byte)0); } [Fact] diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusBitRmwTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusBitRmwTests.cs new file mode 100644 index 0000000..27fce2a --- /dev/null +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusBitRmwTests.cs @@ -0,0 +1,141 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.Modbus; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests; + +[Trait("Category", "Unit")] +public sealed class ModbusBitRmwTests +{ + /// Fake transport capturing each PDU so tests can assert on the read + write sequence. + private sealed class RmwTransport : IModbusTransport + { + public readonly ushort[] HoldingRegisters = new ushort[256]; + public readonly List Pdus = new(); + + public Task ConnectAsync(CancellationToken ct) => Task.CompletedTask; + + public Task SendAsync(byte unitId, byte[] pdu, CancellationToken ct) + { + Pdus.Add(pdu); + if (pdu[0] == 0x03) + { + // FC03 Read Holding Registers. + var addr = (ushort)((pdu[1] << 8) | pdu[2]); + var qty = (ushort)((pdu[3] << 8) | pdu[4]); + var resp = new byte[2 + qty * 2]; + resp[0] = 0x03; + resp[1] = (byte)(qty * 2); + for (var i = 0; i < qty; i++) + { + resp[2 + i * 2] = (byte)(HoldingRegisters[addr + i] >> 8); + resp[3 + i * 2] = (byte)(HoldingRegisters[addr + i] & 0xFF); + } + return Task.FromResult(resp); + } + if (pdu[0] == 0x06) + { + // FC06 Write Single Register. + var addr = (ushort)((pdu[1] << 8) | pdu[2]); + var v = (ushort)((pdu[3] << 8) | pdu[4]); + HoldingRegisters[addr] = v; + return Task.FromResult(new byte[] { 0x06, pdu[1], pdu[2], pdu[3], pdu[4] }); + } + return Task.FromException(new NotSupportedException($"FC 0x{pdu[0]:X2} not supported by fake")); + } + + public ValueTask DisposeAsync() => ValueTask.CompletedTask; + } + + private static (ModbusDriver drv, RmwTransport fake) NewDriver(params ModbusTagDefinition[] tags) + { + var fake = new RmwTransport(); + var opts = new ModbusDriverOptions + { + Host = "fake", + Tags = tags, + Probe = new ModbusProbeOptions { Enabled = false }, + }; + return (new ModbusDriver(opts, "modbus-1", _ => fake), fake); + } + + [Fact] + public async Task Bit_set_reads_current_register_ORs_bit_writes_back() + { + var (drv, fake) = NewDriver( + new ModbusTagDefinition("Flag3", ModbusRegion.HoldingRegisters, 10, ModbusDataType.BitInRegister, BitIndex: 3)); + await drv.InitializeAsync("{}", CancellationToken.None); + fake.HoldingRegisters[10] = 0b0000_0001; // bit 0 already set + + var results = await drv.WriteAsync([new WriteRequest("Flag3", true)], CancellationToken.None); + + results.Single().StatusCode.ShouldBe(0u); + fake.HoldingRegisters[10].ShouldBe((ushort)0b0000_1001); // bit 3 now set, bit 0 preserved + // Two PDUs: FC03 read then FC06 write. + fake.Pdus.Count.ShouldBe(2); + fake.Pdus[0][0].ShouldBe((byte)0x03); + fake.Pdus[1][0].ShouldBe((byte)0x06); + } + + [Fact] + public async Task Bit_clear_reads_current_register_ANDs_bit_off_writes_back() + { + var (drv, fake) = NewDriver( + new ModbusTagDefinition("Flag3", ModbusRegion.HoldingRegisters, 10, ModbusDataType.BitInRegister, BitIndex: 3)); + await drv.InitializeAsync("{}", CancellationToken.None); + fake.HoldingRegisters[10] = 0xFFFF; // all bits set + + await drv.WriteAsync([new WriteRequest("Flag3", false)], CancellationToken.None); + + fake.HoldingRegisters[10].ShouldBe((ushort)0b1111_1111_1111_0111); // bit 3 cleared, rest preserved + } + + [Fact] + public async Task Concurrent_bit_writes_to_same_register_preserve_all_updates() + { + // Serialization test — 8 writers target different bits in register 20. Without the RMW + // lock, concurrent reads interleave + last-to-commit wins so some bits get lost. + var tags = Enumerable.Range(0, 8) + .Select(b => new ModbusTagDefinition($"Bit{b}", ModbusRegion.HoldingRegisters, 20, ModbusDataType.BitInRegister, BitIndex: (byte)b)) + .ToArray(); + var (drv, fake) = NewDriver(tags); + await drv.InitializeAsync("{}", CancellationToken.None); + fake.HoldingRegisters[20] = 0; + + await Task.WhenAll(Enumerable.Range(0, 8).Select(b => + drv.WriteAsync([new WriteRequest($"Bit{b}", true)], CancellationToken.None))); + + fake.HoldingRegisters[20].ShouldBe((ushort)0xFF); // all 8 bits set + } + + [Fact] + public async Task Bit_write_on_different_registers_proceeds_in_parallel_without_contention() + { + var tags = Enumerable.Range(0, 4) + .Select(i => new ModbusTagDefinition($"Bit{i}", ModbusRegion.HoldingRegisters, (ushort)(50 + i), ModbusDataType.BitInRegister, BitIndex: 0)) + .ToArray(); + var (drv, fake) = NewDriver(tags); + await drv.InitializeAsync("{}", CancellationToken.None); + + await Task.WhenAll(Enumerable.Range(0, 4).Select(i => + drv.WriteAsync([new WriteRequest($"Bit{i}", true)], CancellationToken.None))); + + for (var i = 0; i < 4; i++) + fake.HoldingRegisters[50 + i].ShouldBe((ushort)0x01); + } + + [Fact] + public async Task Bit_write_preserves_other_bits_in_the_same_register() + { + var (drv, fake) = NewDriver( + new ModbusTagDefinition("BitA", ModbusRegion.HoldingRegisters, 30, ModbusDataType.BitInRegister, BitIndex: 5), + new ModbusTagDefinition("BitB", ModbusRegion.HoldingRegisters, 30, ModbusDataType.BitInRegister, BitIndex: 10)); + await drv.InitializeAsync("{}", CancellationToken.None); + + await drv.WriteAsync([new WriteRequest("BitA", true)], CancellationToken.None); + await drv.WriteAsync([new WriteRequest("BitB", true)], CancellationToken.None); + + fake.HoldingRegisters[30].ShouldBe((ushort)((1 << 5) | (1 << 10))); + } +} diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusDataTypeTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusDataTypeTests.cs index 9b99764..b85b55f 100644 --- a/tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusDataTypeTests.cs +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusDataTypeTests.cs @@ -132,12 +132,15 @@ public sealed class ModbusDataTypeTests } [Fact] - public void BitInRegister_write_is_not_supported_in_PR24() + public void BitInRegister_EncodeRegister_still_rejects_direct_calls() { + // BitInRegister writes now go through WriteBitInRegisterAsync's RMW path (task #181). + // EncodeRegister should never be reached for this type — if it is, throwing keeps an + // unintended caller loud rather than silently clobbering the register. var tag = new ModbusTagDefinition("T", ModbusRegion.HoldingRegisters, 0, ModbusDataType.BitInRegister, BitIndex: 5); Should.Throw(() => ModbusDriver.EncodeRegister(true, tag)) - .Message.ShouldContain("read-modify-write"); + .Message.ShouldContain("WriteBitInRegisterAsync"); } // --- String ---