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

[twincat] TwinCAT — Bit-indexed BOOL writes (RMW) #343

Merged
dohertj2 merged 1 commits from auto/twincat/1.3 into auto/driver-gaps 2026-04-25 17:25:23 -04:00
Conversation 0 Commits 1 Files Changed 3 +195 -4
3 changed files with 195 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

74
src/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/AdsTwinCATClient.cs
View File

@@ -24,6 +24,11 @@ internal sealed class AdsTwinCATClient : ITwinCATClient
private readonly AdsClient _client = new();
private readonly ConcurrentDictionary<uint, NotificationRegistration> _notifications = new();
// Per-parent-symbol RMW locks. Keys are bounded by the writable-bit-tag cardinality
// and are intentionally never removed — a leaking-but-bounded dictionary is simpler
// than tracking liveness, matching the AbCip / Modbus / FOCAS pattern from #181.
private readonly ConcurrentDictionary<string, SemaphoreSlim> _bitWriteLocks = new();
public AdsTwinCATClient()
{
_client.AdsNotificationEx += OnAdsNotificationEx;
@@ -75,9 +80,9 @@ 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.");
if (bitIndex is int bit && type == TwinCATDataType.Bool)
return await WriteBitInWordAsync(symbolPath, bit, value, cancellationToken)
.ConfigureAwait(false);
try
{
@@ -94,6 +99,69 @@ internal sealed class AdsTwinCATClient : ITwinCATClient
}
}
/// <summary>
/// Read-modify-write a single bit within an integer parent word. <paramref name="symbolPath"/>
/// is the bit-selector path (e.g. <c>Flags.3</c>); the parent is the same path with the
/// <c>.N</c> suffix stripped and is read/written as a UDINT — TwinCAT handles narrower
/// parents (BYTE/WORD) implicitly through the UDINT projection.
/// </summary>
/// <remarks>
/// Concurrent bit writers against the same parent are serialised through a per-parent
/// <see cref="SemaphoreSlim"/> to prevent torn reads/writes. Mirrors the AbCip / Modbus /
/// FOCAS bit-RMW pattern.
/// </remarks>
private async Task<uint> WriteBitInWordAsync(
string symbolPath, int bit, object? value, CancellationToken cancellationToken)
{
var parentPath = TryGetParentSymbolPath(symbolPath);
if (parentPath is null) return TwinCATStatusMapper.BadNotSupported;
var setBit = Convert.ToBoolean(value);
var rmwLock = _bitWriteLocks.GetOrAdd(parentPath, _ => new SemaphoreSlim(1, 1));
await rmwLock.WaitAsync(cancellationToken).ConfigureAwait(false);
try
{
var read = await _client.ReadValueAsync(parentPath, typeof(uint), cancellationToken)
.ConfigureAwait(false);
if (read.ErrorCode != AdsErrorCode.NoError)
return TwinCATStatusMapper.MapAdsError((uint)read.ErrorCode);
var current = Convert.ToUInt32(read.Value ?? 0u);
var updated = ApplyBit(current, bit, setBit);
var write = await _client.WriteValueAsync(parentPath, updated, cancellationToken)
.ConfigureAwait(false);
return write.ErrorCode == AdsErrorCode.NoError
? TwinCATStatusMapper.Good
: TwinCATStatusMapper.MapAdsError((uint)write.ErrorCode);
}
catch (AdsErrorException ex)
{
return TwinCATStatusMapper.MapAdsError((uint)ex.ErrorCode);
}
finally
{
rmwLock.Release();
}
}
/// <summary>
/// Strip the trailing <c>.N</c> bit selector from a TwinCAT symbol path. Returns
/// <c>null</c> when the path has no parent (single segment / leading dot).
/// </summary>
internal static string? TryGetParentSymbolPath(string symbolPath)
{
var dot = symbolPath.LastIndexOf('.');
return dot <= 0 ? null : symbolPath.Substring(0, dot);
}
/// <summary>Set or clear bit <paramref name="bit"/> in <paramref name="word"/>.</summary>
internal static uint ApplyBit(uint word, int bit, bool setBit)
{
var mask = 1u << bit;
return setBit ? (word | mask) : (word & ~mask);
}
public async Task<bool> ProbeAsync(CancellationToken cancellationToken)
{
try

20
tests/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/FakeTwinCATClient.cs
View File

@@ -41,7 +41,25 @@ internal class FakeTwinCATClient : ITwinCATClient
{
if (ThrowOnWrite) throw Exception ?? new InvalidOperationException();
WriteLog.Add((symbolPath, type, bitIndex, value));
Values[symbolPath] = value;
// Model the parent-word RMW path the production AdsTwinCATClient performs for
// bit-indexed BOOL writes so driver-level tests can assert the resulting parent state.
if (bitIndex is int bit && type == TwinCATDataType.Bool)
{
var parentPath = AdsTwinCATClient.TryGetParentSymbolPath(symbolPath);
if (parentPath is not null)
{
var current = Values.TryGetValue(parentPath, out var p) && p is not null
? Convert.ToUInt32(p) : 0u;
Values[parentPath] = AdsTwinCATClient.ApplyBit(
current, bit, Convert.ToBoolean(value));
}
}
else
{
Values[symbolPath] = value;
}
var status = WriteStatuses.TryGetValue(symbolPath, out var s) ? s : TwinCATStatusMapper.Good;
return Task.FromResult(status);
}

105
tests/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATBitWriteTests.cs Normal file
View File

@@ -0,0 +1,105 @@
using Shouldly;
using Xunit;
using ZB.MOM.WW.OtOpcUa.Driver.TwinCAT;
namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests;
[Trait("Category", "Unit")]
public sealed class TwinCATBitWriteTests
{
// ---- Helper unit tests ----
[Theory]
[InlineData("Flags.3", "Flags")]
[InlineData("MAIN.Status.7", "MAIN.Status")]
[InlineData("GVL.Motors[0].Status.5", "GVL.Motors[0].Status")]
public void TryGetParentSymbolPath_strips_trailing_bit_selector(string input, string expected)
{
AdsTwinCATClient.TryGetParentSymbolPath(input).ShouldBe(expected);
}
[Theory]
[InlineData("Counter")] // single segment — no parent
[InlineData(".Bad")] // leading dot
public void TryGetParentSymbolPath_returns_null_for_pathless(string input)
{
AdsTwinCATClient.TryGetParentSymbolPath(input).ShouldBeNull();
}
[Theory]
[InlineData(0u, 0, true, 0x0000_0001u)]
[InlineData(0u, 31, true, 0x8000_0000u)]
[InlineData(0xFFFF_FFFFu, 0, false, 0xFFFF_FFFEu)]
[InlineData(0xFFFF_FFFFu, 31, false, 0x7FFF_FFFFu)]
[InlineData(0x0000_0008u, 3, true, 0x0000_0008u)] // already set — idempotent
[InlineData(0x0000_0000u, 3, false, 0x0000_0000u)] // already clear — idempotent
public void ApplyBit_sets_or_clears_bit(uint word, int bit, bool setBit, uint expected)
{
AdsTwinCATClient.ApplyBit(word, bit, setBit).ShouldBe(expected);
}
// ---- Driver-level round-trip ----
private static (TwinCATDriver drv, FakeTwinCATClientFactory factory) NewDriver(params TwinCATTagDefinition[] tags)
{
var factory = new FakeTwinCATClientFactory();
var drv = new TwinCATDriver(new TwinCATDriverOptions
{
Devices = [new TwinCATDeviceOptions("ads://5.23.91.23.1.1:851")],
Tags = tags,
Probe = new TwinCATProbeOptions { Enabled = false },
}, "drv-1", factory);
return (drv, factory);
}
[Fact]
public async Task Bit_write_sets_bit_in_parent_word()
{
var (drv, factory) = NewDriver(
new TwinCATTagDefinition("FlagBit3", "ads://5.23.91.23.1.1:851", "Flags.3", TwinCATDataType.Bool));
await drv.InitializeAsync("{}", CancellationToken.None);
// Parent word starts at 0.
factory.Customise = () => new FakeTwinCATClient { Values = { ["Flags"] = 0u } };
var results = await drv.WriteAsync(
[new Core.Abstractions.WriteRequest("FlagBit3", true)], CancellationToken.None);
results.Single().StatusCode.ShouldBe(TwinCATStatusMapper.Good);
// Parent should now have bit 3 set.
Convert.ToUInt32(factory.Clients[0].Values["Flags"]!).ShouldBe(0x0000_0008u);
}
[Fact]
public async Task Bit_write_clears_bit_without_disturbing_neighbours()
{
var (drv, factory) = NewDriver(
new TwinCATTagDefinition("FlagBit3", "ads://5.23.91.23.1.1:851", "Flags.3", TwinCATDataType.Bool));
await drv.InitializeAsync("{}", CancellationToken.None);
// Parent word has bits 0, 3, 7 set initially.
factory.Customise = () => new FakeTwinCATClient { Values = { ["Flags"] = 0x0000_0089u } };
var results = await drv.WriteAsync(
[new Core.Abstractions.WriteRequest("FlagBit3", false)], CancellationToken.None);
results.Single().StatusCode.ShouldBe(TwinCATStatusMapper.Good);
// Bit 3 cleared; bits 0 and 7 untouched.
Convert.ToUInt32(factory.Clients[0].Values["Flags"]!).ShouldBe(0x0000_0081u);
}
[Fact]
public async Task Bit_write_does_not_throw_NotSupported()
{
// Regression: AdsTwinCATClient previously threw NotSupportedException for bit writes.
var (drv, factory) = NewDriver(
new TwinCATTagDefinition("Bit", "ads://5.23.91.23.1.1:851", "GVL.Word.0", TwinCATDataType.Bool));
await drv.InitializeAsync("{}", CancellationToken.None);
factory.Customise = () => new FakeTwinCATClient { Values = { ["GVL.Word"] = 0u } };
var results = await drv.WriteAsync(
[new Core.Abstractions.WriteRequest("Bit", true)], CancellationToken.None);
results.Single().StatusCode.ShouldBe(TwinCATStatusMapper.Good);
// Status should be Good, not BadNotSupported (which is what the catch block produced).
results.Single().StatusCode.ShouldNotBe(TwinCATStatusMapper.BadNotSupported);
}
}