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

Bit-in-word RMW pass 1 � Modbus + FOCAS #128

Merged
dohertj2 merged 1 commits from rmw-modbus-focas into v2 2026-04-19 20:27:19 -04:00
Conversation 0 Commits 1 Files Changed 6 +409 -17
6 changed files with 409 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

81
src/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FwlibFocasClient.cs
View File

@@ -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<string, SemaphoreSlim> _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<uint> WriteAsync(
public async Task<uint> 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,
};
}
/// <summary>
/// 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.
/// </summary>
private async Task<uint> 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<bool> 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;

62
src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs
View File

@@ -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<ushort, SemaphoreSlim> _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
}
}
/// <summary>
/// Read-modify-write one bit in a holding register. FC03 → bit-swap → FC06. Serialised
/// against other bit writes targeting the same register via <see cref="GetRmwLock"/>.
/// </summary>
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<ISubscriptionHandle> 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}");
}

123
tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasPmcBitRmwTests.cs Normal file
View File

@@ -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
{
/// <summary>
/// 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.
/// </summary>
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<uint> 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);
}
}

12
tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FwlibNativeHelperTests.cs
View File

@@ -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<NotSupportedException>(() =>
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]

141
tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusBitRmwTests.cs Normal file
View File

@@ -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
{
/// <summary>Fake transport capturing each PDU so tests can assert on the read + write sequence.</summary>
private sealed class RmwTransport : IModbusTransport
{
public readonly ushort[] HoldingRegisters = new ushort[256];
public readonly List<byte[]> Pdus = new();
public Task ConnectAsync(CancellationToken ct) => Task.CompletedTask;
public Task<byte[]> 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<byte[]>(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)));
}
}

7
tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusDataTypeTests.cs
View File

@@ -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<InvalidOperationException>(() => ModbusDriver.EncodeRegister(true, tag))
.Message.ShouldContain("read-modify-write");
.Message.ShouldContain("WriteBitInRegisterAsync");
}
// --- String ---