docs(plan): bit-index RMW writes implementation plan + tasks
This commit is contained in:
Joseph Doherty
@@ -0,0 +1,485 @@
|
||||
# Inbound bit-index RMW writes Implementation Plan
|
||||
|
||||
> **For Claude:** REQUIRED SUB-SKILL: Use superpowers-extended-cc:subagent-driven-development to implement this plan task-by-task.
|
||||
|
||||
**Goal:** Close the two `stillpending.md` §2 "bit-within-word write" lines — AbLegacy B/I/O-file bit writes and TwinCAT BOOL-within-word writes — both as parent-word read-modify-write.
|
||||
|
||||
**Architecture:** AbLegacy already has the full RMW machinery (`WriteBitInWordAsync` + per-parent `GetRmwLock`); it merely excludes B/I/O at the dispatch guard — we drop the exclusion. TwinCAT has no write-side RMW (it throws); we add a driver-level RMW in `TwinCATDriver.WriteAsync` (read the parent word as `UDInt`→`uint`, flip the bit, write it back, under a new per-parent `SemaphoreSlim`) and remove the client-side throw. Both edits are driver-internal: no Commons/wire/proto/EF change.
|
||||
|
||||
**Tech Stack:** C# / .NET 10, libplctag (AbLegacy PCCC), Beckhoff TwinCAT.Ads, xUnit + Shouldly. Design: `docs/plans/2026-06-17-stillpending-bit-rmw-writes-design.md` (committed `cf231a78`).
|
||||
|
||||
**Branch:** `feat/stillpending-bit-rmw-writes` off master `67da6d4f` (design committed `cf231a78`).
|
||||
|
||||
**Dependency graph:** `{T1 ∥ T2} → T3 → T4 → T5`. T1 (AbLegacy) and T2 (TwinCAT) touch disjoint projects → dispatch their implementers concurrently.
|
||||
|
||||
**Hard rules (carried from prior phases):** stage by explicit path, never `git add .`; never stage `sql_login.txt` / `src/Server/.../pki/` / `pending.md` / `current.md` / `docker-dev/docker-compose.yml` / `stillpending.md`; never echo/commit secrets; no force-push; no `--no-verify`; **NO EF migration**; **NO Commons wire/proto contract change**; **NO bUnit**; `dangerouslyDisableSandbox` for all build/test/rig commands.
|
||||
|
||||
---
|
||||
|
||||
### Task 1: AbLegacy B/I/O-file bit writes (drop the exclusion)
|
||||
|
||||
**Classification:** standard
|
||||
**Estimated implement time:** ~4 min
|
||||
**Parallelizable with:** Task 2
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs:363-373` (the `WriteBitInWordAsync` dispatch guard + its class-doc comment)
|
||||
- Test: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyBitRmwTests.cs`
|
||||
|
||||
**Context:** `WriteBitInWordAsync` (`:603-649`) already does parent-word read → mask → write-back with a per-parent `GetRmwLock`, and works today for N/L/S/A files. The dispatch guard excludes B/I/O:
|
||||
|
||||
```csharp
|
||||
if (def.DataType == AbLegacyDataType.Bit && parsed?.BitIndex is int bit
|
||||
&& parsed.FileLetter is not "B" and not "I" and not "O")
|
||||
```
|
||||
|
||||
so B/I/O bit writes fall to `runtime.EncodeValue(def.DataType, bitIndex, …)` → the guard throw in `LibplctagLegacyTagRuntime.EncodeValue` (`:138-144`) → `BadNotSupported`. `AbLegacyAddress.TryParse` already accepts `B3:0/0`, `I:0/0`, `O:1/2` (bit range 0..15, `MaxBitIndexFor`), and `(parsed with { BitIndex = null }).ToLibplctagName()` yields `B3:0` / `I:0` / `O:1`. B/I/O are all 16-bit-`Int`-parent files — the existing non-`L` branch handles them with no new code.
|
||||
|
||||
**Step 1: Write the failing tests**
|
||||
|
||||
Add to `AbLegacyBitRmwTests.cs` (mirror the existing `Bit_set_reads_parent_word_ORs_bit_writes_back` / `Bit_clear_preserves_other_bits_in_N_file_word` pattern, which asserts the parent-word value via `factory.Tags["<parent>"]`):
|
||||
|
||||
```csharp
|
||||
/// <summary>B-file bit set reads the parent word, ORs the bit, writes it back (was BadNotSupported).</summary>
|
||||
[Theory]
|
||||
[InlineData("B3:0/3", "B3:0")]
|
||||
[InlineData("I:0/3", "I:0")]
|
||||
[InlineData("O:1/3", "O:1")]
|
||||
public async Task Bit_set_on_B_I_O_file_RMWs_parent_word(string bitAddr, string parentName)
|
||||
{
|
||||
var factory = new FakeAbLegacyTagFactory
|
||||
{
|
||||
Customise = p => new FakeAbLegacyTag(p) { Value = (short)0b0001 },
|
||||
};
|
||||
var drv = new AbLegacyDriver(new AbLegacyDriverOptions
|
||||
{
|
||||
Devices = [new AbLegacyDeviceOptions("ab://10.0.0.5/1,0")],
|
||||
Tags = [new AbLegacyTagDefinition("F", "ab://10.0.0.5/1,0", bitAddr, AbLegacyDataType.Bit)],
|
||||
Probe = new AbLegacyProbeOptions { Enabled = false },
|
||||
}, "drv-1", factory);
|
||||
await drv.InitializeAsync("{}", CancellationToken.None);
|
||||
|
||||
var results = await drv.WriteAsync([new WriteRequest("F", true)], CancellationToken.None);
|
||||
|
||||
results.Single().StatusCode.ShouldBe(AbLegacyStatusMapper.Good);
|
||||
factory.Tags.ShouldContainKey(parentName);
|
||||
Convert.ToInt32(factory.Tags[parentName].Value).ShouldBe(0b1001);
|
||||
}
|
||||
|
||||
/// <summary>B-file bit clear preserves the other bits in the parent word.</summary>
|
||||
[Fact]
|
||||
public async Task Bit_clear_preserves_other_bits_in_B_file_word()
|
||||
{
|
||||
var factory = new FakeAbLegacyTagFactory
|
||||
{
|
||||
Customise = p => new FakeAbLegacyTag(p) { Value = unchecked((short)0xFFFF) },
|
||||
};
|
||||
var drv = new AbLegacyDriver(new AbLegacyDriverOptions
|
||||
{
|
||||
Devices = [new AbLegacyDeviceOptions("ab://10.0.0.5/1,0")],
|
||||
Tags = [new AbLegacyTagDefinition("F", "ab://10.0.0.5/1,0", "B3:0/3", AbLegacyDataType.Bit)],
|
||||
Probe = new AbLegacyProbeOptions { Enabled = false },
|
||||
}, "drv-1", factory);
|
||||
await drv.InitializeAsync("{}", CancellationToken.None);
|
||||
|
||||
await drv.WriteAsync([new WriteRequest("F", false)], CancellationToken.None);
|
||||
|
||||
Convert.ToInt32(factory.Tags["B3:0"].Value).ShouldBe(unchecked((short)0xFFF7));
|
||||
}
|
||||
|
||||
/// <summary>Concurrent bit writes to the same B-file word compose correctly (per-parent lock).</summary>
|
||||
[Fact]
|
||||
public async Task Concurrent_bit_writes_to_same_B_file_word_compose_correctly()
|
||||
{
|
||||
var factory = new FakeAbLegacyTagFactory
|
||||
{
|
||||
Customise = p => new FakeAbLegacyTag(p) { Value = (short)0 },
|
||||
};
|
||||
var tags = Enumerable.Range(0, 8)
|
||||
.Select(b => new AbLegacyTagDefinition($"Bit{b}", "ab://10.0.0.5/1,0", $"B3:0/{b}", AbLegacyDataType.Bit))
|
||||
.ToArray();
|
||||
var drv = new AbLegacyDriver(new AbLegacyDriverOptions
|
||||
{
|
||||
Devices = [new AbLegacyDeviceOptions("ab://10.0.0.5/1,0")],
|
||||
Tags = tags,
|
||||
Probe = new AbLegacyProbeOptions { Enabled = false },
|
||||
}, "drv-1", factory);
|
||||
await drv.InitializeAsync("{}", CancellationToken.None);
|
||||
|
||||
await Task.WhenAll(Enumerable.Range(0, 8).Select(b =>
|
||||
drv.WriteAsync([new WriteRequest($"Bit{b}", true)], CancellationToken.None)));
|
||||
|
||||
Convert.ToInt32(factory.Tags["B3:0"].Value).ShouldBe(0xFF);
|
||||
}
|
||||
```
|
||||
|
||||
(Match the exact constructor/field shapes used by the existing tests in this file — copy from `Bit_set_reads_parent_word_ORs_bit_writes_back`. If `FakeAbLegacyTag.Value` for I/O parents needs a `short` like the N-file tests, keep `(short)`.)
|
||||
|
||||
**Step 2: Run the tests to verify they fail**
|
||||
|
||||
```bash
|
||||
dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests \
|
||||
--filter "FullyQualifiedName~AbLegacyBitRmwTests" -v minimal
|
||||
```
|
||||
Expected: the new B/I/O tests FAIL — the writes return `BadNotSupported` (parent word unchanged) because B/I/O are excluded from `WriteBitInWordAsync`. Run with `dangerouslyDisableSandbox: true`.
|
||||
|
||||
**Step 3: Drop the B/I/O exclusion + fix the comment**
|
||||
|
||||
In `AbLegacyDriver.cs`, replace the dispatch block (`:363-373`):
|
||||
|
||||
```csharp
|
||||
// PCCC bit-within-word writes — RMW against a parallel parent-word runtime (strip the /N
|
||||
// bit suffix). The per-parent-word lock serialises concurrent bit writers. Applies to every
|
||||
// bit-addressable PCCC file: N-file (N7:0/3), B-file (B3:0/0), and the I/O image files
|
||||
// (I:0/0, O:1/2); L-file bits RMW a 32-bit parent, the rest a 16-bit word. T/C/R sub-elements
|
||||
// don't reach this path because they're not Bit typed. NOTE: an Input-image (I) write is sent
|
||||
// to the PLC like any other write — the device drives the input image from physical inputs and
|
||||
// may reject it; we surface that real PCCC status rather than pre-rejecting at the driver.
|
||||
if (def.DataType == AbLegacyDataType.Bit && parsed?.BitIndex is int bit)
|
||||
{
|
||||
results[i] = new WriteResult(
|
||||
await WriteBitInWordAsync(device, parsed, bit, w.Value, cancellationToken).ConfigureAwait(false));
|
||||
continue;
|
||||
}
|
||||
```
|
||||
|
||||
(This removes only the `&& parsed.FileLetter is not "B" and not "I" and not "O"` clause and updates the comment. `WriteBitInWordAsync` is unchanged — its `isLong = FileLetter == "L"` already routes B/I/O to the 16-bit `Int` branch.)
|
||||
|
||||
**Step 4: Run the tests to verify they pass**
|
||||
|
||||
```bash
|
||||
dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests \
|
||||
--filter "FullyQualifiedName~AbLegacyBitRmwTests" -v minimal
|
||||
```
|
||||
Expected: PASS (new B/I/O + existing N/L tests). Then run the whole AbLegacy suite to confirm no regression:
|
||||
```bash
|
||||
dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests -v minimal
|
||||
```
|
||||
Expected: all green. `dangerouslyDisableSandbox: true`.
|
||||
|
||||
**Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs \
|
||||
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyBitRmwTests.cs
|
||||
git commit -m "feat(ablegacy): B/I/O-file bit-within-word writes via existing RMW path"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 2: TwinCAT BOOL-within-word RMW writes (driver-level)
|
||||
|
||||
**Classification:** standard
|
||||
**Estimated implement time:** ~5 min
|
||||
**Parallelizable with:** Task 1
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs` (add a `_bitRmwLocks` field near the other `ConcurrentDictionary` fields ~`:25-28`; add the RMW branch in `WriteAsync` ~`:292-299`)
|
||||
- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/AdsTwinCATClient.cs:182-206` (remove the BOOL-within-word throw; update the `bitIndex` param doc at `:178`)
|
||||
- Test: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATReadWriteTests.cs`
|
||||
|
||||
**Context:** The driver delegates a bit write straight to the client (`client.WriteValueAsync(symbolName, def.DataType, parsed?.BitIndex, w.Value, ct)`), and `AdsTwinCATClient.WriteValueAsync` (`:189-191`) throws `NotSupportedException` for `Bool + bitIndex`. The real `AdsTwinCATClient` news up a concrete `AdsClient` (not injectable) → RMW buried there is **not** fake-testable. So do the RMW **at the driver level** through two `ITwinCATClient` calls (the fake faithfully stores/returns `Values[parentPath]`, so the full RMW is exercised offline). This mirrors the existing bit-*read* convention (`AdsTwinCATClient.ReadValueAsync:120-128` reads the parent as `uint`); `TwinCATDataType.UDInt` maps to `typeof(uint)` (`MapToClrType:487`), and `TwinCATSymbolPath` is a record so `parsed with { BitIndex = null }` yields the parent path. The per-parent `SemaphoreSlim` spans read+write so concurrent bit-writers don't lose updates (it cannot guard against the PLC program writing the word mid-RMW — inherent, documented).
|
||||
|
||||
**Step 1: Write the failing tests**
|
||||
|
||||
Add to `TwinCATReadWriteTests.cs` (use the file's `NewDriver(...)` helper + `factory.Customise = () => new FakeTwinCATClient { Values = { … } }` pattern from `Successful_DInt_read_returns_Good_value`; the fake's `WriteLog` records `(symbol, type, bit, value)` and `Values[path]` holds the written value):
|
||||
|
||||
```csharp
|
||||
// ---- 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);
|
||||
// parent write went out as UDInt with no bit index
|
||||
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(); // no parent write attempted
|
||||
}
|
||||
|
||||
/// <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);
|
||||
}
|
||||
```
|
||||
|
||||
Also **search for an existing test that asserts the old throw** (a Bool+bitIndex write returning `BadNotSupported` / "task #181"):
|
||||
```bash
|
||||
grep -rn "BOOL-within\|task #181\|bitIndex.*BadNotSupported\|181" \
|
||||
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/
|
||||
```
|
||||
If one exists, update it to the new RMW behavior (it must no longer expect `BadNotSupported`).
|
||||
|
||||
**Step 2: Run the tests to verify they fail**
|
||||
|
||||
```bash
|
||||
dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests \
|
||||
--filter "FullyQualifiedName~TwinCATReadWriteTests" -v minimal
|
||||
```
|
||||
Expected: the new RMW tests FAIL — the write currently throws `NotSupportedException` → the driver's `catch (NotSupportedException)` maps to `BadNotSupported` and never RMWs the parent. `dangerouslyDisableSandbox: true`.
|
||||
|
||||
**Step 3a: Add the per-parent lock field + RMW branch in `TwinCATDriver`**
|
||||
|
||||
Near the other `ConcurrentDictionary` fields (~`:25-28`), add:
|
||||
|
||||
```csharp
|
||||
// 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);
|
||||
```
|
||||
|
||||
In `WriteAsync`, inside the `try` (`:293`), replace the existing 4 lines (`var symbolName = …; var status = await client.WriteValueAsync(…); results[i] = new WriteResult(status);`) with:
|
||||
|
||||
```csharp
|
||||
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);
|
||||
results[i] = new WriteResult(status);
|
||||
```
|
||||
|
||||
(Confirm `using System.Threading;` is present for `SemaphoreSlim`; `System.Collections.Concurrent` is already imported for the other dictionaries. `Convert.ToBoolean`/`Convert.ToUInt32` mirror the AbLegacy RMW.)
|
||||
|
||||
**Step 3b: Remove the client-side throw**
|
||||
|
||||
In `AdsTwinCATClient.cs`, delete the guard at `:189-191`:
|
||||
|
||||
```csharp
|
||||
if (bitIndex is int && type == TwinCATDataType.Bool)
|
||||
throw new NotSupportedException(
|
||||
"BOOL-within-word writes require read-modify-write; tracked in task #181.");
|
||||
```
|
||||
|
||||
so `WriteValueAsync` proceeds straight to `ConvertForWrite` + `_client.WriteValueAsync`. Update the `bitIndex` param doc (`:178`) from `"Optional bit index for BOOL values (not supported for writes)."` to `"Optional bit index for BOOL values. BOOL-within-word writes are handled upstream by TwinCATDriver.WriteAsync as a parent-word read-modify-write, so a bit index does not reach this method on the write path."`
|
||||
|
||||
**Step 4: Run the tests to verify they pass**
|
||||
|
||||
```bash
|
||||
dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests \
|
||||
--filter "FullyQualifiedName~TwinCATReadWriteTests" -v minimal
|
||||
dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests -v minimal
|
||||
```
|
||||
Expected: all green (new RMW + existing suites). `dangerouslyDisableSandbox: true`.
|
||||
|
||||
**Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs \
|
||||
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/AdsTwinCATClient.cs \
|
||||
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATReadWriteTests.cs
|
||||
git commit -m "feat(twincat): BOOL-within-word writes via driver-level parent-word RMW"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 3: Docs + plan-record §2 clear
|
||||
|
||||
**Classification:** small
|
||||
**Estimated implement time:** ~3 min
|
||||
**Parallelizable with:** none (after T1+T2)
|
||||
|
||||
**Files:**
|
||||
- Modify: `docs/drivers/AbLegacy.md` (bit-write section — B/I/O now supported + the input-image caveat)
|
||||
- Modify: `docs/drivers/TwinCAT.md` (BOOL-within-word RMW write + the shared RMW-vs-PLC-program limitation)
|
||||
- Modify: `docs/plans/2026-06-17-stillpending-bit-rmw-writes.md.tasks.json` (mark T1–T3 status; do **not** stage `stillpending.md`)
|
||||
|
||||
**Step 1: Update `docs/drivers/AbLegacy.md`**
|
||||
|
||||
Find the write / bit-addressing section. Document that bit-within-word writes (`B3:0/0`, `I:0/0`, `O:1/2`, plus the existing `N7:0/3`, L-file) are supported via parent-word read-modify-write under a per-parent lock; and add the caveat: *an Input-image (`I`) write is sent to the PLC like any other write — the device drives the input image from physical inputs and may reject it, in which case the driver surfaces the device's real PCCC status rather than pre-rejecting.* Note the shared RMW limitation (the lock serialises the driver's bit-writers but not the PLC program writing the word mid-RMW).
|
||||
|
||||
**Step 2: Update `docs/drivers/TwinCAT.md`**
|
||||
|
||||
Document that BOOL-within-word writes (e.g. `MAIN.Flags.3`) are now supported as a driver-level parent-word read-modify-write (read the parent as `UDInt`/`uint`, flip the bit, write it back) under a per-parent lock, mirroring the existing bit-read. Note the same RMW-vs-PLC-program limitation.
|
||||
|
||||
**Step 3: Clear the §2 lines via the plan record (NOT stillpending.md)**
|
||||
|
||||
Update `docs/plans/2026-06-17-stillpending-bit-rmw-writes.md.tasks.json` to reflect both §2 lines closed (AbLegacy B/I/O bit writes; TwinCAT BOOL-within-word #181). Do **not** edit or stage `stillpending.md`.
|
||||
|
||||
**Step 4: Commit**
|
||||
|
||||
```bash
|
||||
git add docs/drivers/AbLegacy.md docs/drivers/TwinCAT.md \
|
||||
docs/plans/2026-06-17-stillpending-bit-rmw-writes.md.tasks.json
|
||||
git commit -m "docs(drivers): B/I/O + BOOL-within-word bit writes (RMW)"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 4: Full build + AbLegacy + TwinCAT tests + final integration review
|
||||
|
||||
**Classification:** standard
|
||||
**Estimated implement time:** ~4 min
|
||||
**Parallelizable with:** none (after T3)
|
||||
|
||||
**Files:** none (verification only)
|
||||
|
||||
**Step 1: Build the solution**
|
||||
|
||||
```bash
|
||||
dotnet build ZB.MOM.WW.OtOpcUa.slnx -v minimal
|
||||
```
|
||||
Expected: build succeeds, no new warnings. `dangerouslyDisableSandbox: true`.
|
||||
|
||||
**Step 2: Run both driver suites**
|
||||
|
||||
```bash
|
||||
dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests -v minimal
|
||||
dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests -v minimal
|
||||
```
|
||||
Expected: all green. Record the counts.
|
||||
|
||||
**Step 3: Final integration review**
|
||||
|
||||
Dispatch a final integration reviewer over `git diff 67da6d4f..HEAD`. Confirm:
|
||||
- Both edits are driver-internal — no `IDriver` / `WriteRequest` / `WriteResult` / `ITwinCATClient` / `IAbLegacyTagRuntime` *signature* change, no Commons/wire/proto/EF change.
|
||||
- AbLegacy: the only behavioral change is B/I/O bit writes now RMW (N/L/S/A unchanged); the input-image caveat is documented.
|
||||
- TwinCAT: the RMW lives at the driver level and is fake-testable; the client throw is gone; no path can still surface the old `BadNotSupported` for a BOOL-within-word write; the per-parent lock spans read+write (no lost-update window inside the driver) and is released on every exit (incl. the read-failure short-circuit).
|
||||
- No `git add .`; none of the never-stage files staged.
|
||||
|
||||
Apply any Critical/Important fixes via a fresh implementer; re-review.
|
||||
|
||||
**Step 4: Commit (if review fixes were applied)**
|
||||
|
||||
```bash
|
||||
git add <only the fixed files by path>
|
||||
git commit -m "fix(bit-rmw): address final integration review"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 5: Live `/run` acceptance (best-effort) + finish branch
|
||||
|
||||
**Classification:** standard
|
||||
**Estimated implement time:** ~5 min
|
||||
**Parallelizable with:** none (after T4)
|
||||
|
||||
**Files:** none (verification + merge)
|
||||
|
||||
**Step 1: Live `/run` — best-effort, fixture-gated**
|
||||
|
||||
There is no local PCCC (libplctag SLC/ML) or ADS (TwinCAT, Windows-only runtime) sim on this Mac, so this slice's primary acceptance gate is **unit-proven** (as 4c did for the non-Modbus drivers).
|
||||
- Best-effort: if a libplctag `ab_server` emulator can host a writable B-file locally (`docs/drivers/AbServer-Test-Fixture.md`), drive an AbLegacy `B3:0/0` bit set+clear round-trip via the AbLegacy CLI / a skip-gated integration test and confirm the parent word composes. If it can't host a writable B-file, record the honest unit-proven gate + the fixture/operator follow-up.
|
||||
- TwinCAT live stays operator-gated (needs a TwinCAT/ADS target); record as unit-proven.
|
||||
|
||||
Do not block the merge on the fixture — unit coverage (T1 + T2) is the gate; live is a bonus.
|
||||
|
||||
**Step 2: Finish the branch — REQUIRED SUB-SKILL: superpowers-extended-cc:finishing-a-development-branch**
|
||||
|
||||
Verify the full driver suites are green, then (per the standing meta-choice) **merge to master + push**:
|
||||
```bash
|
||||
git checkout master
|
||||
git pull --ff-only
|
||||
git merge --ff-only feat/stillpending-bit-rmw-writes
|
||||
git push
|
||||
git branch -d feat/stillpending-bit-rmw-writes
|
||||
```
|
||||
(If `git checkout master` is blocked by uncommitted branch-only `.tasks.json` edits, commit the bookkeeping to the branch first, then ff-merge — as in Phase 4d.)
|
||||
|
||||
**Step 3: Update memory**
|
||||
|
||||
Update `project_stillpending_backlog.md` (add the bit-RMW-writes paragraph + update REMAINING) and `MEMORY.md` pointer.
|
||||
@@ -0,0 +1,17 @@
|
||||
{
|
||||
"planPath": "docs/plans/2026-06-17-stillpending-bit-rmw-writes.md",
|
||||
"designPath": "docs/plans/2026-06-17-stillpending-bit-rmw-writes-design.md",
|
||||
"designCommit": "cf231a78",
|
||||
"baseMaster": "67da6d4f",
|
||||
"branch": "feat/stillpending-bit-rmw-writes",
|
||||
"scope": "Inbound bit-index RMW writes. AbLegacy: drop the B/I/O exclusion so B/I/O-file bit writes route through the existing WriteBitInWordAsync RMW (input-image write is device-gated, faithful status). TwinCAT: driver-level BOOL-within-word RMW in TwinCATDriver.WriteAsync (read parent as UDInt->uint, flip bit, write back, per-parent SemaphoreSlim) + remove the AdsTwinCATClient throw. Closes 2 stillpending.md §2 lines. NO Commons/wire/proto/EF change; NO bUnit. Live = fixture-gated/unit-proven.",
|
||||
"dependencyGraph": "{T1 ∥ T2} → T3 → T4 → T5 (T1 AbLegacy ∥ T2 TwinCAT touch disjoint projects)",
|
||||
"tasks": [
|
||||
{"id": 1, "subject": "AbLegacy B/I/O-file bit writes (drop the WriteBitInWordAsync exclusion) + tests", "classification": "standard", "status": "pending", "parallelizableWith": [2]},
|
||||
{"id": 2, "subject": "TwinCAT BOOL-within-word driver-level RMW (read/flip/write parent as UDInt, per-parent lock) + remove client throw + tests", "classification": "standard", "status": "pending", "parallelizableWith": [1]},
|
||||
{"id": 3, "subject": "Docs (AbLegacy.md + TwinCAT.md) + plan-record §2 clear (do NOT stage stillpending.md)", "classification": "small", "status": "pending", "blockedBy": [1, 2]},
|
||||
{"id": 4, "subject": "Full build + AbLegacy + TwinCAT tests + final integration review", "classification": "standard", "status": "pending", "blockedBy": [3]},
|
||||
{"id": 5, "subject": "Live /run best-effort (fixture-gated) + finish branch (merge to master + push) + memory", "classification": "standard", "status": "pending", "blockedBy": [4]}
|
||||
],
|
||||
"lastUpdated": "2026-06-17"
|
||||
}
|
||||
Reference in New Issue
Block a user