From cb7acda4eec96f63891671b5921f446b0681a02e Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Wed, 5 Aug 2026 16:27:51 -0600 Subject: [PATCH 1/4] refactor(device): extract device administration into a collaborator (part of #344) Moves the reboot, ADC-calibration, voltage-precision and friendly-name commands out of DaqifiStreamingDevice into DeviceAdministrationOperations. This is the "device admin" block named in #344's body that #422 left behind while extracting SD card, diagnostics, network config and LAN chip info. DaqifiStreamingDevice: 1,385 -> 1,271 lines against the issue's ~800 target. The IDeviceOperationHost seam gains exactly two members, both forwarding to device members that already existed: Metadata (the device's own object, since the friendly-name write updates it optimistically) and Disconnect (reboot has to tear the local link down after the device drops its link). Public API is unchanged: all eleven members stay on the device as one-line delegations, so every command still passes through the device's own virtual Send and any subclass override of it. Verified as a move mechanically rather than by eye: a normalized statement multiset diff of what the device lost against what the collaborator gained leaves zero residue on the device side. No existing test changed. The +16 new cases test the collaborator directly and cover only what a direct test can see - the ordering and the total set of calls back into the host - with a fake host that throws on every seam member outside this block's remit. Co-Authored-By: Claude Opus 5 --- SESSION_LOG.md | 14 + .../DeviceAdministrationOperationsTests.cs | 247 ++++++++++++++++++ .../Internal/StreamFrameDecoderTests.cs | 2 + .../Device/DaqifiStreamingDevice.cs | 152 ++--------- .../DeviceAdministrationOperations.cs | 196 ++++++++++++++ .../Device/Internal/IDeviceOperationHost.cs | 17 ++ 6 files changed, 495 insertions(+), 133 deletions(-) create mode 100644 src/Daqifi.Core.Tests/Device/Internal/DeviceAdministrationOperationsTests.cs create mode 100644 src/Daqifi.Core/Device/Internal/DeviceAdministrationOperations.cs diff --git a/SESSION_LOG.md b/SESSION_LOG.md index 8c105f8..c3e047c 100644 --- a/SESSION_LOG.md +++ b/SESSION_LOG.md @@ -40,3 +40,17 @@ - BENCH (real Nq1, fw 3.7.2, USB, non-destructive) via a scratch harness, because the example CLI surfaces none of these counters. **Session 1 (ch 0,1,2 @200 Hz, 3 s):** `rawFrames=475` and `decodedCh0=475` — every delivered frame reached BOTH consumer paths exactly once (the re-raise multiplicity contract, on hardware); `discarded=1` with `PartialAnalogFrame[an=1/en=3]` — the firmware's malformed leading frame caught by the moved guard and withheld from raw consumers; all 3 channels decoded 475 samples each in ascending order; `decodeFailures=0`, `gaps=0`; every event's `sender` asserted to be the device. **Session 2 (ch 0 only @100 Hz, same instance):** `discarded=0`/`failures=0` (BeginSession reset both, no leftover tripped the gate); `rawFrames=238` vs `decodedCh0=237` — one post-stop frame re-raised but NOT decoded, exactly the `if (!IsStreaming)` branch on hardware; `ch1=ch2=0` decoded, so the disable reached the device and the snapshot the decode maps against is still right. Only channel enable/disable + stream start/stop; no NVM write, no reboot, no SD. - Bench-rig note: the example CLI's `--channels` takes a **bitmask** (`7` = ch 0,1,2), not a comma list, and `--format` accepts only `text|csv|jsonl` (no `json`). - Result: PR #435 opened (base main, part of #344, "not merging — for review"), /agentic_review requested. Now **3 loop PRs awaiting review (#433, #434, #435) — at the concurrency cap**, so the next fire should shepherd only, not start a new ticket. + +## 2026-08-05 — Fire: implemented #344 device-administration extraction → PR #436 +- State at start (re-derived from `gh`, not this log): #433 and #435 both MERGED, so only **1** open loop PR (#434) — back **under** the 3-PR cap, and priority 4 is live again. Priority 1: #434 has 0 unresolved `qodo-code-review` threads (GraphQL `reviewThreads`), Qodo Code Review clean (Bugs 0 / Rule violations 0 / Requirement gaps 0), CI `build` green, ready-note already posted → nothing to shepherd. Priorities 2-3 empty. +- Backlog is still 5 open issues. Re-checked #269 before defaulting to #344: its remaining items are 1 (port-release wait — **already implemented**, `PostLanDisconnectPortReleaseDelay` defaults to 1.5 s), 5 (chip-info retry — already implemented), and 2/4 (the no-op-adapter handoff and the LAN prep/recovery sequences), which can only be validated by an actual destructive WINC flash. So #344 remained the only eligible ticket. #333 (breaking API), #271 (destructive WINC flash) and #183 (unshipped firmware dep + new NuGet package) unchanged as skips. +- Candidates weighed inside #344, and why the small one won. `DaqifiDevice` (4,193 lines) is still the largest file, but its remaining blocks are the reconnect loop (~510 lines, entangled with `_sessionEpoch` / `ConnectAsync` / the protected virtual snapshot hooks), the text-exchange core (~400 lines, consumer swap + two locks + disposal state) and the lifecycle/deferral gates (~750 lines of concurrency primitives). Every one of those is a place where a compile-clean move can deadlock. The discipline that has actually worked on this issue (#419/#422/#432/#433/#435) is "pure translation: no lock, no event, no device state", so I took the block that fits it rather than the block with the biggest line count. +- PICKED the **device-administration** block out of `DaqifiStreamingDevice` — reboot, the six ADC-calibration bank commands, the two voltage-precision commands, `SetAdcCalibrationSlope`/`Offset`, and the friendly-name write. This is the "device admin" block named in the issue **body** that #422 left behind while extracting the other four. `DaqifiStreamingDevice` 1,385 → **1,271** against the issue's ~800 target. +- New `DeviceAdministrationOperations` (internal, `Device/Internal/`). Seam gained exactly two members: `Metadata` (the device's own object, not a copy — the friendly-name write is optimistic because the firmware never echoes the name back) and `Disconnect()` (reboot has to tear the local link down, routed through the device so the full disconnect path runs). Public API unchanged; all eleven members stay on the device as one-line delegations. +- Verified as a move mechanically, not by eye: normalized statement multiset diff of what the device lost vs what the collaborator gained leaves **zero** residue on the device side — every removed statement appears in the collaborator. The collaborator-only residues are class/ctor scaffolding plus three signature lines that git treated as unchanged context because they are textually identical either side of the move. +- Tests: zero edits to existing tests. `DaqifiStreamingDeviceTests` (command text), `DaqifiStreamingDeviceFriendlyNameTests` and `DeviceNotConnectedExceptionTests` (all ten disconnected-guard sites) still drive the same behavior through the device — that is the extraction evidence, so they are deliberately untouched and not duplicated. +16 new cases against the collaborator directly, covering only what a direct test can see: **call ordering and the total set of host interactions**. The fake host throws on every seam member outside this block's remit, so a future change that grabs the channels lock or stops a stream fails loudly. +- Mutation-verified the two ordering assertions rather than trusting green: swapping `Reboot` to disconnect-then-send fails `Reboot_SendsTheRebootCommandBeforeTearingTheConnectionDown` (disconnecting first closes the transport the reboot command still has to travel over), and hoisting the metadata write above the two sends fails `SetFriendlyNameAsync_WhenTheSaveSendFails_LeavesMetadataUnchanged`. +- FULL suite green net9 + net10 (2,583 passed, 2 skipped each; +16) + Daqifi.Mcp.Tests 23. Release solution build 0 warnings both TFMs. +- BENCH (real Nq1, fw 3.7.2, USB, non-destructive) — and run as a true **A/B against `origin/main`**, which is the only thing that actually proves a refactor changed nothing on hardware. Built the same harness twice, once against this branch's Core and once against a throwaway `origin/main` worktree, and ran both on the same board: **byte-for-byte identical output**. Only RAM-load commands were issued (`CONFigure:ADC:LOADcal`, `CONFigure:VOLTage:LOAD` — NVM read, RAM write); no NVM write, no bank selection, no friendly-name write, no reboot. AI0 mean 0.0024 V before and after the reload (delta 0.0001 V), so nothing was perturbed. Argument guards fired against the live device (`SetAdcCalibrationSlope(-1)`, `UseAdcCalibration(2)` → `ArgumentOutOfRangeException`, connection still up, error queue clean), and both `SaveAdcCalibration` and `Reboot` refused a disconnected device — so neither an NVM write nor a reboot was ever issued. +- Observation, NOT filed as an issue (device/firmware state, not a Core defect, and identical on main): this bench unit answers `CONFigure:ADC:LOADcal` with `-200,"Execution error"`. `-200` rather than `-113` means the firmware **recognized** the header and failed to execute it — consistent with this unit having no saved user ADC-calibration bank. `CONFigure:VOLTage:LOAD` is accepted cleanly on the same unit. +- Result: PR #436 opened (base main, part of #344, "not merging — for review"), `/agentic_review` requested. Touches no file #434 touches. Now 2 loop PRs awaiting review (#434, #436) — still under the cap. diff --git a/src/Daqifi.Core.Tests/Device/Internal/DeviceAdministrationOperationsTests.cs b/src/Daqifi.Core.Tests/Device/Internal/DeviceAdministrationOperationsTests.cs new file mode 100644 index 0000000..8233585 --- /dev/null +++ b/src/Daqifi.Core.Tests/Device/Internal/DeviceAdministrationOperationsTests.cs @@ -0,0 +1,247 @@ +using Daqifi.Core.Channel; +using Daqifi.Core.Communication.Messages; +using Daqifi.Core.Device; +using Daqifi.Core.Device.Internal; +using Daqifi.Core.Device.SdCard; +using System; +using System.Collections.Generic; +using System.IO; +using System.Threading; +using System.Threading.Tasks; +using Xunit; + +namespace Daqifi.Core.Tests.Device.Internal; + +/// +/// Unit tests for , the reboot / ADC-calibration / +/// voltage-precision / friendly-name block extracted from (#344). +/// +/// +/// +/// What each command puts on the wire, and that each one refuses a disconnected device, is already +/// pinned through the device by DaqifiStreamingDeviceTests, +/// DaqifiStreamingDeviceFriendlyNameTests and DeviceNotConnectedExceptionTests. Those +/// are deliberately untouched — they are the evidence that the extraction changed nothing, so they +/// are not repeated here. +/// +/// +/// These add the part only a direct test can see: the ordering and the total set of calls back +/// into the host. The fake below throws on every member outside this block's remit, so a future +/// change that reaches for the channels lock, stops a stream, or performs device I/O beyond the one +/// command fails loudly rather than passing quietly. +/// +/// +public class DeviceAdministrationOperationsTests +{ + [Fact] + public void Constructor_NullHost_Throws() + { + Assert.Throws(() => new DeviceAdministrationOperations(null!)); + } + + #region Reboot + + [Fact] + public void Reboot_SendsTheRebootCommandBeforeTearingTheConnectionDown() + { + var host = new FakeHost { IsConnected = true }; + + new DeviceAdministrationOperations(host).Reboot(); + + // Order is the whole point: disconnecting first would close the transport the reboot + // command still has to travel over, so the device would never be told to restart. + Assert.Equal(new[] { "send:SYSTem:REboot", "disconnect" }, host.Calls); + } + + [Fact] + public void Reboot_WhenNotConnected_ThrowsAndLeavesTheConnectionAlone() + { + var host = new FakeHost { IsConnected = false }; + + Assert.Throws(() => new DeviceAdministrationOperations(host).Reboot()); + + // The guard runs before anything else, so a refused reboot neither sends nor disconnects. + Assert.Empty(host.Calls); + } + + #endregion + + #region One command, and nothing else + + public static IEnumerable SingleCommandOperations() + { + yield return new object[] { "SaveAdcCalibration", "CONFigure:ADC:SAVEcal" }; + yield return new object[] { "LoadAdcCalibration", "CONFigure:ADC:LOADcal" }; + yield return new object[] { "SaveFactoryAdcCalibration", "CONFigure:ADC:SAVEFcal" }; + yield return new object[] { "LoadFactoryAdcCalibration", "CONFigure:ADC:LOADFcal" }; + yield return new object[] { "SaveVoltagePrecision", "CONFigure:VOLTage:SAVE" }; + yield return new object[] { "LoadVoltagePrecision", "CONFigure:VOLTage:LOAD" }; + yield return new object[] { "UseAdcCalibration(0)", "CONFigure:ADC:USECal 0" }; + yield return new object[] { "UseAdcCalibration(1)", "CONFigure:ADC:USECal 1" }; + } + + /// + /// Each of these is a single fire-and-forget command. The assertion is not only that the right + /// text goes out but that the whole interaction with the device is that one send — no stream + /// stop, no channels lock, no metadata write, no disconnect. Every one of those would throw + /// from , and a second send would fail the equality below. + /// + [Theory] + [MemberData(nameof(SingleCommandOperations))] + public void SingleCommandOperation_SendsExactlyThatCommandAndTouchesNothingElse( + string operation, + string expectedCommand) + { + var host = new FakeHost { IsConnected = true }; + var administration = new DeviceAdministrationOperations(host); + + // Dispatched by name rather than by a delegate parameter: the collaborator is internal, so + // an Action cannot appear on a public test method. + switch (operation) + { + case "SaveAdcCalibration": administration.SaveAdcCalibration(); break; + case "LoadAdcCalibration": administration.LoadAdcCalibration(); break; + case "SaveFactoryAdcCalibration": administration.SaveFactoryAdcCalibration(); break; + case "LoadFactoryAdcCalibration": administration.LoadFactoryAdcCalibration(); break; + case "SaveVoltagePrecision": administration.SaveVoltagePrecision(); break; + case "LoadVoltagePrecision": administration.LoadVoltagePrecision(); break; + case "UseAdcCalibration(0)": administration.UseAdcCalibration(0); break; + case "UseAdcCalibration(1)": administration.UseAdcCalibration(1); break; + default: throw new ArgumentOutOfRangeException(nameof(operation), operation, "Unmapped operation."); + } + + Assert.Equal(new[] { "send:" + expectedCommand }, host.Calls); + } + + [Fact] + public void SetAdcCalibrationSlope_SendsExactlyOneCommand() + { + var host = new FakeHost { IsConnected = true }; + + new DeviceAdministrationOperations(host).SetAdcCalibrationSlope(2, 1.0025); + + Assert.Single(host.Calls); + Assert.StartsWith("send:CONFigure:ADC:chanCALM ", host.Calls[0], StringComparison.Ordinal); + } + + [Fact] + public void SetAdcCalibrationOffset_SendsExactlyOneCommand() + { + var host = new FakeHost { IsConnected = true }; + + new DeviceAdministrationOperations(host).SetAdcCalibrationOffset(3, -0.0031); + + Assert.Single(host.Calls); + Assert.StartsWith("send:CONFigure:ADC:chanCALB ", host.Calls[0], StringComparison.Ordinal); + } + + #endregion + + #region Friendly name + + [Fact] + public async Task SetFriendlyNameAsync_SendsSetThenSaveAndThenWritesMetadata() + { + var host = new FakeHost { IsConnected = true }; + + await new DeviceAdministrationOperations(host).SetFriendlyNameAsync("Bench01"); + + Assert.Equal(2, host.Calls.Count); + Assert.StartsWith("send:SYSTem:DEVice:NAME ", host.Calls[0], StringComparison.Ordinal); + Assert.Equal("send:SYSTem:DEVice:NAME:SAVE", host.Calls[1]); + Assert.Equal("Bench01", host.Metadata.FriendlyName); + } + + /// + /// The metadata write is optimistic because the firmware never echoes the name back — but only + /// once both commands have actually gone out. A send that throws means the device was never + /// told, so the local name must not claim otherwise. + /// + [Fact] + public async Task SetFriendlyNameAsync_WhenTheSaveSendFails_LeavesMetadataUnchanged() + { + var host = new FakeHost { IsConnected = true, FailSendAt = 2 }; + host.Metadata.FriendlyName = "Original"; + + await Assert.ThrowsAsync( + () => new DeviceAdministrationOperations(host).SetFriendlyNameAsync("Bench01")); + + Assert.Equal("Original", host.Metadata.FriendlyName); + } + + [Fact] + public async Task SetFriendlyNameAsync_AlreadyCancelled_SendsNothingAndLeavesMetadataUnchanged() + { + var host = new FakeHost { IsConnected = true }; + host.Metadata.FriendlyName = "Original"; + using var cts = new CancellationTokenSource(); + cts.Cancel(); + + await Assert.ThrowsAnyAsync( + () => new DeviceAdministrationOperations(host).SetFriendlyNameAsync("Bench01", cts.Token)); + + Assert.Empty(host.Calls); + Assert.Equal("Original", host.Metadata.FriendlyName); + } + + #endregion + + /// + /// An that records, in order, the only two things this block + /// is allowed to do to a device: send a command, and (for reboot) disconnect. Everything else + /// throws. + /// + private sealed class FakeHost : IDeviceOperationHost + { + private int _sendCount; + + public List Calls { get; } = new(); + + public bool IsConnected { get; set; } + + public DeviceMetadata Metadata { get; } = new(); + + /// 1-based index of the send that should throw, or 0 for none. + public int FailSendAt { get; set; } + + public void Send(IOutboundMessage message) + { + if (++_sendCount == FailSendAt) + { + throw new InvalidOperationException("transport refused the command"); + } + + Calls.Add("send:" + message.Data); + } + + public void Disconnect() => Calls.Add("disconnect"); + + // Outside this block's remit — reaching for any of these is a regression, not a refinement. + public bool IsUsbConnection => throw new NotSupportedException(); + public bool IsStreaming { get => throw new NotSupportedException(); set => throw new NotSupportedException(); } + public int StreamingFrequency => throw new NotSupportedException(); + public TimeSpan SdCardDownloadTimeout => throw new NotSupportedException(); + public TimeSpan SdCardTransferIdleTimeout => throw new NotSupportedException(); + public void StopStreaming() => throw new NotSupportedException(); + public IReadOnlyList SnapshotChannels() => throw new NotSupportedException(); + public void WithChannelsLock(Action action) => throw new NotSupportedException(); + public Task> ExecuteTextCommandAsync( + Action setupAction, + int responseTimeoutMs = 1000, + int completionTimeoutMs = 250, + CancellationToken cancellationToken = default, + Func? prepareAsync = null, + Func? finalizeAsync = null) => throw new NotSupportedException(); + public Task ExecuteRawCaptureAsync( + Func rawAction, + CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public void EnsureSupported(DeviceFeature feature) => throw new NotSupportedException(); + public FeatureNotSupportedException CreateFeatureNotSupportedException(DeviceFeature feature) + => throw new NotSupportedException(); + public void RaiseLowSdSpaceWarning(LowSdSpaceWarningEventArgs e) => throw new NotSupportedException(); + public void RaiseStreamFrameDiscarded(StreamFrameDiscardedEventArgs e) => throw new NotSupportedException(); + public void RaiseGapDetected(TimestampGapEventArgs e) => throw new NotSupportedException(); + public void RaiseRawStreamFrame(DaqifiOutMessage message) => throw new NotSupportedException(); + public void RaiseStreamDecodeFailure(Exception error) => throw new NotSupportedException(); + } +} diff --git a/src/Daqifi.Core.Tests/Device/Internal/StreamFrameDecoderTests.cs b/src/Daqifi.Core.Tests/Device/Internal/StreamFrameDecoderTests.cs index d643ea7..6e1625a 100644 --- a/src/Daqifi.Core.Tests/Device/Internal/StreamFrameDecoderTests.cs +++ b/src/Daqifi.Core.Tests/Device/Internal/StreamFrameDecoderTests.cs @@ -435,9 +435,11 @@ public void RaiseStreamDecodeFailure(Exception error) public bool IsConnected => throw new NotSupportedException(); public bool IsUsbConnection => throw new NotSupportedException(); public int StreamingFrequency => throw new NotSupportedException(); + public DeviceMetadata Metadata => throw new NotSupportedException(); public TimeSpan SdCardDownloadTimeout => throw new NotSupportedException(); public TimeSpan SdCardTransferIdleTimeout => throw new NotSupportedException(); public void StopStreaming() => throw new NotSupportedException(); + public void Disconnect() => throw new NotSupportedException(); public void Send(IOutboundMessage message) => throw new NotSupportedException(); public void WithChannelsLock(Action action) => throw new NotSupportedException(); public Task> ExecuteTextCommandAsync( diff --git a/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs b/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs index 5766b3d..4d77a4b 100644 --- a/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs +++ b/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs @@ -200,6 +200,7 @@ private void InitializeStreamingDevice() // method, so they are always in place before the device is handed to a caller. _frameDecoder = new StreamFrameDecoder(this); _channelControl = new ChannelControlOperations(this); + _administration = new DeviceAdministrationOperations(this); _networkOperations = new NetworkConfigurationOperations(this); _sdCardOperations = new SdCardOperations(this); _lanChipInfoOperations = new LanChipInfoOperations(this); @@ -963,165 +964,43 @@ public void SetPwmDutyCycle(IChannel channel, int dutyCyclePercent) /// Thrown when the device is not connected. /// Thrown when the operation is cancelled. public Task SetFriendlyNameAsync(string name, CancellationToken cancellationToken = default) - { - if (name is null) - { - throw new ArgumentNullException(nameof(name)); - } - - if (!ScpiMessageProducer.IsFriendlyNameValid(name)) - { - throw new ArgumentException( - $"Device name must be 1-{ScpiMessageProducer.MaxFriendlyNameLength} printable ASCII characters and cannot contain '\"' or '\\'.", - nameof(name)); - } - - if (!IsConnected) - { - throw new DeviceNotConnectedException(); - } - - cancellationToken.ThrowIfCancellationRequested(); - - Send(ScpiMessageProducer.SetDeviceName(name)); - Send(ScpiMessageProducer.SaveDeviceName); - Metadata.FriendlyName = name; - - return Task.CompletedTask; - } + => _administration.SetFriendlyNameAsync(name, cancellationToken); /// public void SetAnalogOutput(int channelNumber, double voltage) => _channelControl.SetAnalogOutput(channelNumber, voltage); /// - public void Reboot() - { - if (!IsConnected) - { - throw new DeviceNotConnectedException(); - } - - Send(ScpiMessageProducer.RebootDevice); - - // The device drops its link while restarting, so tear down the local - // connection rather than leaving a stale one that reports Connected. - Disconnect(); - } + public void Reboot() => _administration.Reboot(); /// - public void SaveAdcCalibration() - { - if (!IsConnected) - { - throw new DeviceNotConnectedException(); - } - - Send(ScpiMessageProducer.SaveAdcCalibration); - } + public void SaveAdcCalibration() => _administration.SaveAdcCalibration(); /// - public void LoadAdcCalibration() - { - if (!IsConnected) - { - throw new DeviceNotConnectedException(); - } - - Send(ScpiMessageProducer.LoadAdcCalibration); - } + public void LoadAdcCalibration() => _administration.LoadAdcCalibration(); /// public void SetAdcCalibrationSlope(int channelNumber, double calM) - { - if (channelNumber < 0) - { - throw new ArgumentOutOfRangeException(nameof(channelNumber), channelNumber, "Channel number cannot be negative."); - } - - if (!IsConnected) - { - throw new DeviceNotConnectedException(); - } - - Send(ScpiMessageProducer.SetAdcCalibrationSlope(channelNumber, calM)); - } + => _administration.SetAdcCalibrationSlope(channelNumber, calM); /// public void SetAdcCalibrationOffset(int channelNumber, double calB) - { - if (channelNumber < 0) - { - throw new ArgumentOutOfRangeException(nameof(channelNumber), channelNumber, "Channel number cannot be negative."); - } - - if (!IsConnected) - { - throw new DeviceNotConnectedException(); - } - - Send(ScpiMessageProducer.SetAdcCalibrationOffset(channelNumber, calB)); - } + => _administration.SetAdcCalibrationOffset(channelNumber, calB); /// - public void SaveFactoryAdcCalibration() - { - if (!IsConnected) - { - throw new DeviceNotConnectedException(); - } - - Send(ScpiMessageProducer.SaveFactoryAdcCalibration); - } + public void SaveFactoryAdcCalibration() => _administration.SaveFactoryAdcCalibration(); /// - public void LoadFactoryAdcCalibration() - { - if (!IsConnected) - { - throw new DeviceNotConnectedException(); - } - - Send(ScpiMessageProducer.LoadFactoryAdcCalibration); - } + public void LoadFactoryAdcCalibration() => _administration.LoadFactoryAdcCalibration(); /// - public void UseAdcCalibration(int bank) - { - if (bank is < 0 or > 1) - { - throw new ArgumentOutOfRangeException(nameof(bank), bank, "Calibration bank must be 0 (factory) or 1 (user)."); - } - - if (!IsConnected) - { - throw new DeviceNotConnectedException(); - } - - Send(ScpiMessageProducer.UseAdcCalibration(bank)); - } + public void UseAdcCalibration(int bank) => _administration.UseAdcCalibration(bank); /// - public void SaveVoltagePrecision() - { - if (!IsConnected) - { - throw new DeviceNotConnectedException(); - } - - Send(ScpiMessageProducer.SaveVoltagePrecision); - } + public void SaveVoltagePrecision() => _administration.SaveVoltagePrecision(); /// - public void LoadVoltagePrecision() - { - if (!IsConnected) - { - throw new DeviceNotConnectedException(); - } - - Send(ScpiMessageProducer.LoadVoltagePrecision); - } + public void LoadVoltagePrecision() => _administration.LoadVoltagePrecision(); // ----------------------------------------------------------------- @@ -1141,6 +1020,9 @@ public void LoadVoltagePrecision() /// Channel enable/disable, DIO, PWM and analog output (). private ChannelControlOperations _channelControl = null!; + /// Reboot, ADC calibration banks, voltage precision and the friendly-name write. + private DeviceAdministrationOperations _administration = null!; + /// WiFi/LAN configuration (). private NetworkConfigurationOperations _networkOperations = null!; @@ -1334,6 +1216,10 @@ bool IDeviceOperationHost.IsStreaming void IDeviceOperationHost.Send(IOutboundMessage message) => Send(message); + DeviceMetadata IDeviceOperationHost.Metadata => Metadata; + + void IDeviceOperationHost.Disconnect() => Disconnect(); + IReadOnlyList IDeviceOperationHost.SnapshotChannels() => SnapshotChannels(); void IDeviceOperationHost.WithChannelsLock(Action action) => WithChannelsLock(action); diff --git a/src/Daqifi.Core/Device/Internal/DeviceAdministrationOperations.cs b/src/Daqifi.Core/Device/Internal/DeviceAdministrationOperations.cs new file mode 100644 index 0000000..bc1eb03 --- /dev/null +++ b/src/Daqifi.Core/Device/Internal/DeviceAdministrationOperations.cs @@ -0,0 +1,196 @@ +using Daqifi.Core.Communication.Producers; +using System; +using System.Threading; +using System.Threading.Tasks; + +#nullable enable + +namespace Daqifi.Core.Device.Internal +{ + /// + /// The device-administration half of — reboot, the ADC + /// calibration banks, voltage-precision persistence, and the friendly-name write — extracted + /// from (#344) so the device delegates rather than hosts it. + /// + /// + /// + /// These are fire-and-forget SCPI commands with no reply to parse: each validates its arguments, + /// checks the connection, and sends. They are grouped here because they share that shape and + /// because none of them touches the channel collection, the streaming session, or any device + /// state — the two exceptions being 's local teardown and + /// 's optimistic metadata write, both of which go back through + /// the host rather than being done here. + /// + /// + /// Everything reaches the device through , so each command + /// still passes through the device's own virtual Send and any subclass override of it. + /// + /// + internal sealed class DeviceAdministrationOperations + { + private readonly IDeviceOperationHost _host; + + internal DeviceAdministrationOperations(IDeviceOperationHost host) + { + _host = host ?? throw new ArgumentNullException(nameof(host)); + } + + /// + internal Task SetFriendlyNameAsync(string name, CancellationToken cancellationToken = default) + { + if (name is null) + { + throw new ArgumentNullException(nameof(name)); + } + + if (!ScpiMessageProducer.IsFriendlyNameValid(name)) + { + throw new ArgumentException( + $"Device name must be 1-{ScpiMessageProducer.MaxFriendlyNameLength} printable ASCII characters and cannot contain '\"' or '\\'.", + nameof(name)); + } + + if (!_host.IsConnected) + { + throw new DeviceNotConnectedException(); + } + + cancellationToken.ThrowIfCancellationRequested(); + + _host.Send(ScpiMessageProducer.SetDeviceName(name)); + _host.Send(ScpiMessageProducer.SaveDeviceName); + _host.Metadata.FriendlyName = name; + + return Task.CompletedTask; + } + + /// + internal void Reboot() + { + if (!_host.IsConnected) + { + throw new DeviceNotConnectedException(); + } + + _host.Send(ScpiMessageProducer.RebootDevice); + + // The device drops its link while restarting, so tear down the local + // connection rather than leaving a stale one that reports Connected. + _host.Disconnect(); + } + + /// + internal void SaveAdcCalibration() + { + if (!_host.IsConnected) + { + throw new DeviceNotConnectedException(); + } + + _host.Send(ScpiMessageProducer.SaveAdcCalibration); + } + + /// + internal void LoadAdcCalibration() + { + if (!_host.IsConnected) + { + throw new DeviceNotConnectedException(); + } + + _host.Send(ScpiMessageProducer.LoadAdcCalibration); + } + + /// + internal void SetAdcCalibrationSlope(int channelNumber, double calM) + { + if (channelNumber < 0) + { + throw new ArgumentOutOfRangeException(nameof(channelNumber), channelNumber, "Channel number cannot be negative."); + } + + if (!_host.IsConnected) + { + throw new DeviceNotConnectedException(); + } + + _host.Send(ScpiMessageProducer.SetAdcCalibrationSlope(channelNumber, calM)); + } + + /// + internal void SetAdcCalibrationOffset(int channelNumber, double calB) + { + if (channelNumber < 0) + { + throw new ArgumentOutOfRangeException(nameof(channelNumber), channelNumber, "Channel number cannot be negative."); + } + + if (!_host.IsConnected) + { + throw new DeviceNotConnectedException(); + } + + _host.Send(ScpiMessageProducer.SetAdcCalibrationOffset(channelNumber, calB)); + } + + /// + internal void SaveFactoryAdcCalibration() + { + if (!_host.IsConnected) + { + throw new DeviceNotConnectedException(); + } + + _host.Send(ScpiMessageProducer.SaveFactoryAdcCalibration); + } + + /// + internal void LoadFactoryAdcCalibration() + { + if (!_host.IsConnected) + { + throw new DeviceNotConnectedException(); + } + + _host.Send(ScpiMessageProducer.LoadFactoryAdcCalibration); + } + + /// + internal void UseAdcCalibration(int bank) + { + if (bank is < 0 or > 1) + { + throw new ArgumentOutOfRangeException(nameof(bank), bank, "Calibration bank must be 0 (factory) or 1 (user)."); + } + + if (!_host.IsConnected) + { + throw new DeviceNotConnectedException(); + } + + _host.Send(ScpiMessageProducer.UseAdcCalibration(bank)); + } + + /// + internal void SaveVoltagePrecision() + { + if (!_host.IsConnected) + { + throw new DeviceNotConnectedException(); + } + + _host.Send(ScpiMessageProducer.SaveVoltagePrecision); + } + + /// + internal void LoadVoltagePrecision() + { + if (!_host.IsConnected) + { + throw new DeviceNotConnectedException(); + } + + _host.Send(ScpiMessageProducer.LoadVoltagePrecision); + } + } +} diff --git a/src/Daqifi.Core/Device/Internal/IDeviceOperationHost.cs b/src/Daqifi.Core/Device/Internal/IDeviceOperationHost.cs index 114891f..01ce732 100644 --- a/src/Daqifi.Core/Device/Internal/IDeviceOperationHost.cs +++ b/src/Daqifi.Core/Device/Internal/IDeviceOperationHost.cs @@ -56,6 +56,23 @@ internal interface IDeviceOperationHost /// void Send(IOutboundMessage message); + /// + /// + /// The device's own metadata object, not a copy: the friendly-name write updates + /// optimistically on it, because the firmware does + /// not echo the new name back and may not stream another status frame for a while. + /// + DeviceMetadata Metadata { get; } + + /// + /// + /// Needed by the reboot command, which has to tear the local connection down after the + /// device drops its link. Routed through the device so the whole disconnect path — lifecycle + /// lock, message pumps, status event — runs exactly as it does for a caller-issued + /// . + /// + void Disconnect(); + /// IReadOnlyList SnapshotChannels(); From 18e9670cf43f0f2d128c868c29d0bd273f5cd833 Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Wed, 5 Aug 2026 17:52:40 -0600 Subject: [PATCH 2/4] docs: log the fire that unblocked #434/#436 from their SESSION_LOG merge conflicts --- SESSION_LOG.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/SESSION_LOG.md b/SESSION_LOG.md index 387cff8..31b31f5 100644 --- a/SESSION_LOG.md +++ b/SESSION_LOG.md @@ -92,3 +92,16 @@ - Pushing the log entry drew a Qodo finding of its own ("Hardware identifiers committed", `SESSION_LOG.md`): the bench note pinned the unit's **serial number** into a public repo. **Valid and taken** — the serial carried no analytical weight (the point was "the device came back populated", not *which* device), so it is now described rather than quoted. Kept the `/dev/cu.usbmodem*` shape, which `README.md` already documents as the ordinary macOS port example and is not an identifier. The same redaction was applied to the PR comment. **Convention for future fires: bench notes state what the device reported, never its serial.** - CI flake, NOT a regression: the docs-only push failed `DaqifiDeviceOperationSerializationTests.TextExchange_CancelledWhileTheOutboundQueueDrains_DoesNotResubscribeTheConsumer` on **net10 only** — a commit that touched one markdown file cannot regress it. The prior push (`ff2e30b`) failed a *different* net10 timing test (`StreamMessageConsumerStallingReaderTests.Start_WhenStoppedReaderExitsWithinGrace_WaitsAndRestartsSameInstance`). Two different timing tests failing on the same TFM across two runs is runner slowness, not one broken test; both pass locally on net9 + net10. Worth watching: **net10 CI appears to be the flaky lane on this repo**, and if it keeps costing reruns it deserves its own issue rather than a rerun each time. - Result: no source change this fire. #437 ready-note + bench comment posted; log appended to the branch it shepherded, per the convention set by the previous shepherd fire. Still **3 loop PRs awaiting review (#434, #436, #437) — at the cap**; the next fire shepherds only until the user merges one. + +## 2026-08-05 — Fire: unblocked BOTH open PRs (#434, #436) — each had gone `CONFLICTING` against main +- State at start (re-derived from `gh`, not this log — the log's closing line claimed "at the cap with #434/#436/#437", stale on arrival): **#437 MERGED** at 23:40, so live open loop PRs were **#434 and #436 → 2, under the 3-PR cap**. +- Priority 1: GraphQL `reviewThreads` shows **0 unresolved `qodo-code-review` threads on both**. Priority 2: both already carry ready-notes. Priority 3: CI `build` green on both head SHAs. So 1-3 looked empty — except `gh pr view` reported **`mergeable: CONFLICTING` on BOTH PRs**, i.e. neither was merge-ready and the user could not have merged either one. Fixed that instead of starting a new ticket: an unmergeable PR awaiting review is worth more than a fourth PR behind it. +- **Diagnosed before touching anything.** `git merge-tree --write-tree origin/main` on each: the *only* conflicting path on either PR is **`SESSION_LOG.md`** — this file, an append collision. Zero source overlap in both cases (#436 touches `Device/DaqifiStreamingDevice.cs` + `Device/Internal/*`; #434 touches `Firmware/*`; #437 landed `Device/DaqifiDevice.cs` + `Device/Internal/LifecycleGate.cs`). +- **This is a self-inflicted, recurring process defect and it should be named as such.** Every fire appends its entry to the end of this file *on its PR branch*, so the moment any loop PR merges, **every other open loop PR goes `CONFLICTING`** — and the repo ruleset requires branches be up to date, so each one then needs a manual resolve before the user can merge. It has now cost two separate fires (the 23:37 `gh pr update-branch` fire on #434, and this one). It will recur: whichever of #434/#436 the user merges first, the other conflicts on this file again, because both branches now carry log entries the other lacks. **The fix is structural — stop appending a shared journal file on feature branches** (one file per fire under a directory, or keep the journal off the PR branches entirely). Flagged rather than changed unilaterally, since it is the loop's own convention. +- Resolutions were **chronological, not "take one side"**, and both entry blocks were kept in full. #436's own entry (fire ended 22:32) sorts *before* main's three #437-era entries; #434's own entry (fire ended 23:37, after the last #437 note at 23:28) sorts *after* them. Verified lossless mechanically rather than by eye: for each merged file, **every non-blank line of both parents is still present** (0 missing against each parent, both PRs). +- Verified each merged tree is the clean union: merged-vs-branch shows exactly main's `LifecycleGate` delta and nothing else; merged-vs-main shows exactly that PR's own files and nothing else. No source file was touched by either resolve. +- Tests, on trees that had **never been built** (this is the point of re-running rather than trusting the pre-merge greens): **#436 merged — 2,608 passed, 2 skipped** on net9 + net10; **#434 merged — 2,606 passed, 2 skipped** on net9 + net10. Plus `Daqifi.Mcp.Tests` 23 on each. Release solution build **0 warnings** both TFMs, both trees. +- BENCH (real Nq1, fw 3.7.2, USB CDC, non-destructive — connect → status → stream → disconnect only; no NVM write, no reboot, no SD, no calibration write). Example CLI rebuilt against each merged core in turn (`-p:DaqifiCoreProjectPath=…`, 0 warnings), because **neither combination had ever run on hardware**: #436's device-administration extraction had never run alongside main's `LifecycleGate`, and #434's merged tree had never been built at all. **#436 merged: 3 cycles, all exit 0**, each `analogIn=16 digital=16 fw=3.7.2` with a stable serial, 30 CSV rows at 20 Hz × 2 s. **#434 merged: 2 cycles, all exit 0**, byte-comparable output — 30 rows each, same reported capabilities. Identical run to run, no wedge across repeated open/close of the port. +- Per the convention set two fires ago, the bench notes state what the device reported and never quote its serial number. +- This entry is appended to **#436 only**, deliberately: putting the same text on both branches would guarantee they conflict with *each other* on top of the existing problem, and buys nothing. +- Result: no source change. Both PRs merged up to date with `main` and pushed; superseding ready-notes posted. Still **2 loop PRs awaiting review (#434, #436) — under the cap**, so the next fire can take a new ticket. Not merging. From 662e2566b4efb847da9e19a8f4d6451b52398cf3 Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Wed, 5 Aug 2026 18:02:39 -0600 Subject: [PATCH 3/4] docs: record the Qodo shared-log-conflict exchange and the migration window --- SESSION_LOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/SESSION_LOG.md b/SESSION_LOG.md index 31b31f5..91910ca 100644 --- a/SESSION_LOG.md +++ b/SESSION_LOG.md @@ -104,4 +104,6 @@ - BENCH (real Nq1, fw 3.7.2, USB CDC, non-destructive — connect → status → stream → disconnect only; no NVM write, no reboot, no SD, no calibration write). Example CLI rebuilt against each merged core in turn (`-p:DaqifiCoreProjectPath=…`, 0 warnings), because **neither combination had ever run on hardware**: #436's device-administration extraction had never run alongside main's `LifecycleGate`, and #434's merged tree had never been built at all. **#436 merged: 3 cycles, all exit 0**, each `analogIn=16 digital=16 fw=3.7.2` with a stable serial, 30 CSV rows at 20 Hz × 2 s. **#434 merged: 2 cycles, all exit 0**, byte-comparable output — 30 rows each, same reported capabilities. Identical run to run, no wedge across repeated open/close of the port. - Per the convention set two fires ago, the bench notes state what the device reported and never quote its serial number. - This entry is appended to **#436 only**, deliberately: putting the same text on both branches would guarantee they conflict with *each other* on top of the existing problem, and buys nothing. -- Result: no source change. Both PRs merged up to date with `main` and pushed; superseding ready-notes posted. Still **2 loop PRs awaiting review (#434, #436) — under the cap**, so the next fire can take a new ticket. Not merging. +- **Qodo independently raised the same defect** on the re-review of #436 ("Shared log merge conflicts", Review recommended) and asked this PR to stop appending here. Substance accepted — it is the second independent read of the same problem — but the remediation was declined *in that PR*, and the reasoning is recorded because a future fire will hit this thread again. (a) Removing the newest block reduces the conflict surface by **exactly zero**: the branch already carried a `SESSION_LOG.md` delta from the fire that created it plus the merge resolution, so #434 conflicts the moment #436 merges either way — the latest append is not the marginal cause. (b) The restructure means deleting/moving this file, and #434 currently *modifies* it, so doing it inside #436 would upgrade a trivial content conflict into a **delete/modify** conflict on the other open PR — the exact failure the change is meant to prevent. Replied and resolved. +- **Scheduling note for whoever takes the structural fix:** it has to land when **no** open PR has a `SESSION_LOG.md` delta, or the migration conflicts with everything in flight. Right now both #434 and #436 do, so the window is after they merge and before the next fire appends. Not filed as an issue unilaterally — it is the loop's own convention, so it is the user's call. +- Result: no source change to any product file. Both PRs merged up to date with `main` and pushed, both **`MERGEABLE`** (state `BLOCKED` = merge-ready pending approval), CI `build` green on both new heads, Qodo clean on #434 and resolved on #436. Still **2 loop PRs awaiting review (#434, #436) — under the cap**, so the next fire can take a new ticket. Not merging. From 1d9ada69e46144d4efbb55571542bff9b9e7ab9f Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Wed, 5 Aug 2026 18:05:45 -0600 Subject: [PATCH 4/4] chore: drop the loop scratch journal from this branch SESSION_LOG.md is an agent scratch journal, not a project artifact. Every branch that appends to it conflicts every other open PR. Removing the delta here; a follow-up untracks it and adds it to .gitignore. Co-Authored-By: Claude Opus 5 --- SESSION_LOG.md | 29 ----------------------------- 1 file changed, 29 deletions(-) diff --git a/SESSION_LOG.md b/SESSION_LOG.md index 91910ca..d653612 100644 --- a/SESSION_LOG.md +++ b/SESSION_LOG.md @@ -41,20 +41,6 @@ - Bench-rig note: the example CLI's `--channels` takes a **bitmask** (`7` = ch 0,1,2), not a comma list, and `--format` accepts only `text|csv|jsonl` (no `json`). - Result: PR #435 opened (base main, part of #344, "not merging — for review"), /agentic_review requested. Now **3 loop PRs awaiting review (#433, #434, #435) — at the concurrency cap**, so the next fire should shepherd only, not start a new ticket. -## 2026-08-05 — Fire: implemented #344 device-administration extraction → PR #436 -- State at start (re-derived from `gh`, not this log): #433 and #435 both MERGED, so only **1** open loop PR (#434) — back **under** the 3-PR cap, and priority 4 is live again. Priority 1: #434 has 0 unresolved `qodo-code-review` threads (GraphQL `reviewThreads`), Qodo Code Review clean (Bugs 0 / Rule violations 0 / Requirement gaps 0), CI `build` green, ready-note already posted → nothing to shepherd. Priorities 2-3 empty. -- Backlog is still 5 open issues. Re-checked #269 before defaulting to #344: its remaining items are 1 (port-release wait — **already implemented**, `PostLanDisconnectPortReleaseDelay` defaults to 1.5 s), 5 (chip-info retry — already implemented), and 2/4 (the no-op-adapter handoff and the LAN prep/recovery sequences), which can only be validated by an actual destructive WINC flash. So #344 remained the only eligible ticket. #333 (breaking API), #271 (destructive WINC flash) and #183 (unshipped firmware dep + new NuGet package) unchanged as skips. -- Candidates weighed inside #344, and why the small one won. `DaqifiDevice` (4,193 lines) is still the largest file, but its remaining blocks are the reconnect loop (~510 lines, entangled with `_sessionEpoch` / `ConnectAsync` / the protected virtual snapshot hooks), the text-exchange core (~400 lines, consumer swap + two locks + disposal state) and the lifecycle/deferral gates (~750 lines of concurrency primitives). Every one of those is a place where a compile-clean move can deadlock. The discipline that has actually worked on this issue (#419/#422/#432/#433/#435) is "pure translation: no lock, no event, no device state", so I took the block that fits it rather than the block with the biggest line count. -- PICKED the **device-administration** block out of `DaqifiStreamingDevice` — reboot, the six ADC-calibration bank commands, the two voltage-precision commands, `SetAdcCalibrationSlope`/`Offset`, and the friendly-name write. This is the "device admin" block named in the issue **body** that #422 left behind while extracting the other four. `DaqifiStreamingDevice` 1,385 → **1,271** against the issue's ~800 target. -- New `DeviceAdministrationOperations` (internal, `Device/Internal/`). Seam gained exactly two members: `Metadata` (the device's own object, not a copy — the friendly-name write is optimistic because the firmware never echoes the name back) and `Disconnect()` (reboot has to tear the local link down, routed through the device so the full disconnect path runs). Public API unchanged; all eleven members stay on the device as one-line delegations. -- Verified as a move mechanically, not by eye: normalized statement multiset diff of what the device lost vs what the collaborator gained leaves **zero** residue on the device side — every removed statement appears in the collaborator. The collaborator-only residues are class/ctor scaffolding plus three signature lines that git treated as unchanged context because they are textually identical either side of the move. -- Tests: zero edits to existing tests. `DaqifiStreamingDeviceTests` (command text), `DaqifiStreamingDeviceFriendlyNameTests` and `DeviceNotConnectedExceptionTests` (all ten disconnected-guard sites) still drive the same behavior through the device — that is the extraction evidence, so they are deliberately untouched and not duplicated. +16 new cases against the collaborator directly, covering only what a direct test can see: **call ordering and the total set of host interactions**. The fake host throws on every seam member outside this block's remit, so a future change that grabs the channels lock or stops a stream fails loudly. -- Mutation-verified the two ordering assertions rather than trusting green: swapping `Reboot` to disconnect-then-send fails `Reboot_SendsTheRebootCommandBeforeTearingTheConnectionDown` (disconnecting first closes the transport the reboot command still has to travel over), and hoisting the metadata write above the two sends fails `SetFriendlyNameAsync_WhenTheSaveSendFails_LeavesMetadataUnchanged`. -- FULL suite green net9 + net10 (2,583 passed, 2 skipped each; +16) + Daqifi.Mcp.Tests 23. Release solution build 0 warnings both TFMs. -- BENCH (real Nq1, fw 3.7.2, USB, non-destructive) — and run as a true **A/B against `origin/main`**, which is the only thing that actually proves a refactor changed nothing on hardware. Built the same harness twice, once against this branch's Core and once against a throwaway `origin/main` worktree, and ran both on the same board: **byte-for-byte identical output**. Only RAM-load commands were issued (`CONFigure:ADC:LOADcal`, `CONFigure:VOLTage:LOAD` — NVM read, RAM write); no NVM write, no bank selection, no friendly-name write, no reboot. AI0 mean 0.0024 V before and after the reload (delta 0.0001 V), so nothing was perturbed. Argument guards fired against the live device (`SetAdcCalibrationSlope(-1)`, `UseAdcCalibration(2)` → `ArgumentOutOfRangeException`, connection still up, error queue clean), and both `SaveAdcCalibration` and `Reboot` refused a disconnected device — so neither an NVM write nor a reboot was ever issued. -- Observation, NOT filed as an issue (device/firmware state, not a Core defect, and identical on main): this bench unit answers `CONFigure:ADC:LOADcal` with `-200,"Execution error"`. `-200` rather than `-113` means the firmware **recognized** the header and failed to execute it — consistent with this unit having no saved user ADC-calibration bank. `CONFigure:VOLTage:LOAD` is accepted cleanly on the same unit. -- Result: PR #436 opened (base main, part of #344, "not merging — for review"), `/agentic_review` requested. Touches no file #434 touches. Now 2 loop PRs awaiting review (#434, #436) — still under the cap. - ## 2026-08-05 — Fire: PR #436 ready-note + implemented #344 (lifecycle-gate extraction) → PR #437 - State at start (re-derived from `gh`, NOT this log — the log's closing line said "at the cap with #433/#434/#435", which was stale on arrival): #433 and #435 have MERGED, and a prior fire opened **#436** without writing an entry. Live open loop PRs were **#434 and #436 → 2, under the 3-PR cap**, so priority 4 was in scope. - Priority 1: zero unresolved `qodo-code-review` threads on both (GraphQL `reviewThreads`, not the summary comment). Priority 2: #436 was Qodo-clean (0 bugs / 0 rule violations / 0 requirement gaps) + CI `build` green with **no** ready-note → posted one. #434 already had its note. Priority 3: no red CI. @@ -92,18 +78,3 @@ - Pushing the log entry drew a Qodo finding of its own ("Hardware identifiers committed", `SESSION_LOG.md`): the bench note pinned the unit's **serial number** into a public repo. **Valid and taken** — the serial carried no analytical weight (the point was "the device came back populated", not *which* device), so it is now described rather than quoted. Kept the `/dev/cu.usbmodem*` shape, which `README.md` already documents as the ordinary macOS port example and is not an identifier. The same redaction was applied to the PR comment. **Convention for future fires: bench notes state what the device reported, never its serial.** - CI flake, NOT a regression: the docs-only push failed `DaqifiDeviceOperationSerializationTests.TextExchange_CancelledWhileTheOutboundQueueDrains_DoesNotResubscribeTheConsumer` on **net10 only** — a commit that touched one markdown file cannot regress it. The prior push (`ff2e30b`) failed a *different* net10 timing test (`StreamMessageConsumerStallingReaderTests.Start_WhenStoppedReaderExitsWithinGrace_WaitsAndRestartsSameInstance`). Two different timing tests failing on the same TFM across two runs is runner slowness, not one broken test; both pass locally on net9 + net10. Worth watching: **net10 CI appears to be the flaky lane on this repo**, and if it keeps costing reruns it deserves its own issue rather than a rerun each time. - Result: no source change this fire. #437 ready-note + bench comment posted; log appended to the branch it shepherded, per the convention set by the previous shepherd fire. Still **3 loop PRs awaiting review (#434, #436, #437) — at the cap**; the next fire shepherds only until the user merges one. - -## 2026-08-05 — Fire: unblocked BOTH open PRs (#434, #436) — each had gone `CONFLICTING` against main -- State at start (re-derived from `gh`, not this log — the log's closing line claimed "at the cap with #434/#436/#437", stale on arrival): **#437 MERGED** at 23:40, so live open loop PRs were **#434 and #436 → 2, under the 3-PR cap**. -- Priority 1: GraphQL `reviewThreads` shows **0 unresolved `qodo-code-review` threads on both**. Priority 2: both already carry ready-notes. Priority 3: CI `build` green on both head SHAs. So 1-3 looked empty — except `gh pr view` reported **`mergeable: CONFLICTING` on BOTH PRs**, i.e. neither was merge-ready and the user could not have merged either one. Fixed that instead of starting a new ticket: an unmergeable PR awaiting review is worth more than a fourth PR behind it. -- **Diagnosed before touching anything.** `git merge-tree --write-tree origin/main` on each: the *only* conflicting path on either PR is **`SESSION_LOG.md`** — this file, an append collision. Zero source overlap in both cases (#436 touches `Device/DaqifiStreamingDevice.cs` + `Device/Internal/*`; #434 touches `Firmware/*`; #437 landed `Device/DaqifiDevice.cs` + `Device/Internal/LifecycleGate.cs`). -- **This is a self-inflicted, recurring process defect and it should be named as such.** Every fire appends its entry to the end of this file *on its PR branch*, so the moment any loop PR merges, **every other open loop PR goes `CONFLICTING`** — and the repo ruleset requires branches be up to date, so each one then needs a manual resolve before the user can merge. It has now cost two separate fires (the 23:37 `gh pr update-branch` fire on #434, and this one). It will recur: whichever of #434/#436 the user merges first, the other conflicts on this file again, because both branches now carry log entries the other lacks. **The fix is structural — stop appending a shared journal file on feature branches** (one file per fire under a directory, or keep the journal off the PR branches entirely). Flagged rather than changed unilaterally, since it is the loop's own convention. -- Resolutions were **chronological, not "take one side"**, and both entry blocks were kept in full. #436's own entry (fire ended 22:32) sorts *before* main's three #437-era entries; #434's own entry (fire ended 23:37, after the last #437 note at 23:28) sorts *after* them. Verified lossless mechanically rather than by eye: for each merged file, **every non-blank line of both parents is still present** (0 missing against each parent, both PRs). -- Verified each merged tree is the clean union: merged-vs-branch shows exactly main's `LifecycleGate` delta and nothing else; merged-vs-main shows exactly that PR's own files and nothing else. No source file was touched by either resolve. -- Tests, on trees that had **never been built** (this is the point of re-running rather than trusting the pre-merge greens): **#436 merged — 2,608 passed, 2 skipped** on net9 + net10; **#434 merged — 2,606 passed, 2 skipped** on net9 + net10. Plus `Daqifi.Mcp.Tests` 23 on each. Release solution build **0 warnings** both TFMs, both trees. -- BENCH (real Nq1, fw 3.7.2, USB CDC, non-destructive — connect → status → stream → disconnect only; no NVM write, no reboot, no SD, no calibration write). Example CLI rebuilt against each merged core in turn (`-p:DaqifiCoreProjectPath=…`, 0 warnings), because **neither combination had ever run on hardware**: #436's device-administration extraction had never run alongside main's `LifecycleGate`, and #434's merged tree had never been built at all. **#436 merged: 3 cycles, all exit 0**, each `analogIn=16 digital=16 fw=3.7.2` with a stable serial, 30 CSV rows at 20 Hz × 2 s. **#434 merged: 2 cycles, all exit 0**, byte-comparable output — 30 rows each, same reported capabilities. Identical run to run, no wedge across repeated open/close of the port. -- Per the convention set two fires ago, the bench notes state what the device reported and never quote its serial number. -- This entry is appended to **#436 only**, deliberately: putting the same text on both branches would guarantee they conflict with *each other* on top of the existing problem, and buys nothing. -- **Qodo independently raised the same defect** on the re-review of #436 ("Shared log merge conflicts", Review recommended) and asked this PR to stop appending here. Substance accepted — it is the second independent read of the same problem — but the remediation was declined *in that PR*, and the reasoning is recorded because a future fire will hit this thread again. (a) Removing the newest block reduces the conflict surface by **exactly zero**: the branch already carried a `SESSION_LOG.md` delta from the fire that created it plus the merge resolution, so #434 conflicts the moment #436 merges either way — the latest append is not the marginal cause. (b) The restructure means deleting/moving this file, and #434 currently *modifies* it, so doing it inside #436 would upgrade a trivial content conflict into a **delete/modify** conflict on the other open PR — the exact failure the change is meant to prevent. Replied and resolved. -- **Scheduling note for whoever takes the structural fix:** it has to land when **no** open PR has a `SESSION_LOG.md` delta, or the migration conflicts with everything in flight. Right now both #434 and #436 do, so the window is after they merge and before the next fire appends. Not filed as an issue unilaterally — it is the loop's own convention, so it is the user's call. -- Result: no source change to any product file. Both PRs merged up to date with `main` and pushed, both **`MERGEABLE`** (state `BLOCKED` = merge-ready pending approval), CI `build` green on both new heads, Qodo clean on #434 and resolved on #436. Still **2 loop PRs awaiting review (#434, #436) — under the cap**, so the next fire can take a new ticket. Not merging.