feat(twincat): BOOL-within-word writes via driver-level parent-word RMW

This commit is contained in:
Joseph Doherty
2026-06-17 11:55:44 -04:00
parent 5c6b7cd6f9
commit a73e20fb28
3 changed files with 127 additions and 5 deletions
@@ -175,7 +175,9 @@ internal sealed class AdsTwinCATClient : ITwinCATClient
/// </summary>
/// <param name="symbolPath">The ADS symbol path to write to.</param>
/// <param name="type">The TwinCAT data type.</param>
/// <param name="bitIndex">Optional bit index for BOOL values (not supported for writes).</param>
/// <param name="bitIndex">Optional bit index for BOOL values. BOOL-within-word writes are handled
/// upstream by <see cref="TwinCATDriver.WriteAsync"/> as a parent-word read-modify-write, so a
/// bit index does not reach this method on the write path.</param>
/// <param name="value">The value to write.</param>
/// <param name="cancellationToken">The cancellation token.</param>
/// <returns>The OPC UA status code of the write operation.</returns>
@@ -186,10 +188,6 @@ internal sealed class AdsTwinCATClient : ITwinCATClient
object? value,
CancellationToken cancellationToken)
{
if (bitIndex is int && type == TwinCATDataType.Bool)
throw new NotSupportedException(
"BOOL-within-word writes require read-modify-write; tracked in task #181.");
try
{
var converted = ConvertForWrite(type, value);
@@ -28,6 +28,14 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery
private readonly ConcurrentDictionary<string, TwinCATTagDefinition> _tagsByName =
new(StringComparer.OrdinalIgnoreCase);
// Per-parent-word RMW gate for BOOL-within-word writes: a single-bit write is a
// read-modify-write of the parent word (TwinCAT's symbol table doesn't expose "Word.N"),
// so concurrent bit-writers to the same word must serialise or they lose each other's
// updates. Keyed by device + parent symbol. (Cannot guard against the PLC program writing
// the word between our read and write — inherent to RMW.)
private readonly ConcurrentDictionary<string, SemaphoreSlim> _bitRmwLocks =
new(StringComparer.OrdinalIgnoreCase);
// Resolves a read/write/subscribe fullReference to a tag definition, bridging the two
// authoring models: an authored tag-table entry (by name) OR an equipment tag whose
// reference is its raw TagConfig JSON (parsed once via TwinCATEquipmentTagParser, cached).
@@ -293,6 +301,40 @@ 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.
if (def.DataType == TwinCATDataType.Bool && parsed?.BitIndex is int bit)
{
var parentPath = (parsed with { BitIndex = null }).ToAdsSymbolName();
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
{
var word = Convert.ToUInt32(parentValue ?? 0u);
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);
}
}
finally
{
gate.Release();
}
continue;
}
var symbolName = parsed?.ToAdsSymbolName() ?? def.SymbolPath;
var status = await client.WriteValueAsync(
symbolName, def.DataType, parsed?.BitIndex, w.Value, cancellationToken).ConfigureAwait(false);
@@ -266,4 +266,86 @@ public sealed class TwinCATReadWriteTests
factory.Clients[0].DisposeCount.ShouldBe(1);
}
// ---- BOOL-within-word RMW writes ----
/// <summary>A BOOL-within-word set reads the parent word, ORs the bit, writes it back as UDInt.</summary>
[Fact]
public async Task Bit_set_RMWs_parent_word_as_UDInt()
{
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 } };
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);
}
/// <summary>A BOOL-within-word clear preserves the other bits in the parent word.</summary>
[Fact]
public async Task Bit_clear_preserves_other_bits()
{
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 } };
await drv.InitializeAsync("{}", CancellationToken.None);
await drv.WriteAsync([new WriteRequest("Flag", false)], CancellationToken.None);
factory.Clients[0].Values["MAIN.Flags"].ShouldBe(0xFFF7u);
}
/// <summary>RMW works on a DWORD parent (bit 20 set above the 16-bit boundary).</summary>
[Fact]
public async Task Bit_set_on_DWORD_parent_sets_high_bit()
{
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 } };
await drv.InitializeAsync("{}", CancellationToken.None);
await drv.WriteAsync([new WriteRequest("Hi", true)], CancellationToken.None);
factory.Clients[0].Values["GVL.Status"].ShouldBe(1u << 20);
}
/// <summary>A failed parent read short-circuits the RMW and surfaces the read status.</summary>
[Fact]
public async Task Bit_write_surfaces_parent_read_failure()
{
var (drv, factory) = NewDriver(
new TwinCATTagDefinition("Flag", "ads://5.23.91.23.1.1:851", "MAIN.Flags.3", TwinCATDataType.Bool));
factory.Customise = () => new FakeTwinCATClient
{
ReadStatuses = { ["MAIN.Flags"] = TwinCATStatusMapper.BadNodeIdUnknown },
};
await drv.InitializeAsync("{}", CancellationToken.None);
var results = await drv.WriteAsync([new WriteRequest("Flag", true)], CancellationToken.None);
results.Single().StatusCode.ShouldBe(TwinCATStatusMapper.BadNodeIdUnknown);
factory.Clients[0].WriteLog.ShouldBeEmpty();
}
/// <summary>Concurrent bit writes to the same word compose correctly (per-parent lock).</summary>
[Fact]
public async Task Concurrent_bit_writes_to_same_word_compose_correctly()
{
var tags = Enumerable.Range(0, 8)
.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 } };
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);
}
}