RMW pass 1 — Modbus BitInRegister + FOCAS PMC Bit write paths. First half of task #181 — the two drivers where read-modify-write is a clean protocol-level insertion (Modbus FC03/FC06 round-trip + FOCAS pmc_rdpmcrng / pmc_wrpmcrng round-trip). Per-driver SemaphoreSlim registry keyed on the parent word address serialises concurrent bit writes so two writers targeting different bits in the same word don't lose one another's update. Modbus — ModbusDriver gains WriteBitInRegisterAsync + _rmwLocks ConcurrentDictionary. WriteOneAsync routes BitInRegister (HoldingRegisters region only) through RMW ahead of the normal encode path. Read uses FC03 Read Holding Registers for 1 register at tag.Address, bit-op on the returned ushort via (current | 1<<bit) for set / (current & ~(1<<bit)) for clear, write back via FC06 Write Single Register. Per-address lock prevents concurrent bit writes to the same register from racing. Rejects out-of-range bits (0-15) with InvalidOperationException. EncodeRegister's BitInRegister branch repurposed as a defensive guard — if a non-RMW caller ever reaches it, throw so an unintended bypass stays loud rather than silently clobbering. FOCAS — FwlibFocasClient gains WritePmcBitAsync + _rmwLocks keyed on {addrType}:{byteAddr}. Driver-layer WriteAsync routes Bit writes with a bitIndex through the new path; other Pmc writes still hit the direct pmc_wrpmcrng path. RMW uses cnc_rdpmcrng + Byte dataType to grab the parent byte, bit-op with (current | 1<<bit) or (current & ~(1<<bit)), cnc_wrpmcrng to write back. Rejects out-of-range bits (0-7, FOCAS PMC bytes are 8-bit) with InvalidOperationException. EncodePmcValue's Bit branch now treats a no-bitIndex case as whole-byte boolean (non-zero / zero); bitIndex-present writes never hit this path because they dispatch to WritePmcBitAsync upstream. Tests — 5 new ModbusBitRmwTests + 4 new FocasPmcBitRmwTests + 1 renamed pre-existing test each covering — bit set preserves other bits, bit clear preserves other bits, concurrent bit writes to same word/byte compose correctly (8-parallel stress), bit writes on different parent words proceed without contention (4-parallel), sequential bit sets compose into 0xFF after all 8. Fake PmcRmwFake in FOCAS tests simulates the PMC byte storage + surfaces it through the IFocasClient contract so the test asserts driver-level behavior without needing Fwlib32.dll. FwlibNativeHelperTests.EncodePmcValue_Bit_throws_NotSupported_for_RMW_gap replaced with EncodePmcValue_Bit_without_bit_index_writes_byte_boolean reflecting the new behavior. ModbusDataTypeTests.BitInRegister_write_is_not_supported_in_PR24 renamed to BitInRegister_EncodeRegister_still_rejects_direct_calls; the message assertion updated to match the new defensive message. Modbus tests now 182/182, FOCAS tests now 119/119; full solution builds 0 errors; AbCip/AbLegacy/TwinCAT untouched (those get their RMW pass in a follow-up since libplctag bit access may need a parallel parent-word handle). Task #181 stays pending until that second pass lands.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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}");
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user