[tylerkron]

Not merging — for review.

The violation

MobileShellViewModel.StreamToggle — a view-model — hand-wrote the SCPI command
"SYSTem:POWer:STATe 1\r\n" and pushed it through AbstractStreamingDevice.Write(string), a
raw-bytes escape hatch on the device's public surface. The UI owned the command mnemonic and
the wire terminator, and said so in eleven lines of comment:

// NOTE: Write() sends RAW bytes with no terminator (unlike the producer
// path, which appends "\r\n"). The firmware SCPI parser is line-based, so
// an unterminated command merges with the next write and BOTH are dropped
// — include the terminator explicitly. On WiFi this Write is a caught
// no-op; on USB it actually transmits (adversarial audit).
try { device.Write("SYSTem:POWer:STATe 1\r\n"); }
catch { /* best-effort; InitializeStreaming still gates on IsStreaming */ }

That is the app's rule broken in one line: the UI talks to Core's public surface and to the
app's own device layer, and does not know about SCPI or transports.

Write existed only to serve that line. It had exactly one caller in this repo — that one —
and zero in upstream daqifi-desktop, which carries the same abstract member. Every
transport still implemented it: serial wrote ASCII straight at _transport.Stream with a
SerialPort fallback, USB did the same, and WiFi threw NotSupportedException — which is what
the bare catch is for. On the mobile shell's own primary transport the power-on step did
nothing at all.

The fix

AbstractStreamingDevice.TurnDeviceOn() — one concrete method on the base class, sending Core's
ScpiMessageProducer.TurnDeviceOn through the SendMessage path that already carries every
other command this class issues (Reboot, AddChannel, RemoveAllChannels, the SD interface
switch). The view-model calls device.TurnDeviceOn() and knows nothing else.

Then Write goes: the abstract member, its three transport implementations, and its three
test-double stubs.

Second, smaller one in the same family: the connect dialog's USB tile bound
{Binding Port.PortName}, walking from the view into the System.IO.Ports.SerialPort object
the device owns. It binds the device's own PortName projection now.

What got flatter

git diff --numstat origin/main...HEAD, production files only:

File	+	−
Device/AbstractStreamingDevice.cs	20	3
Device/SerialDevice/SerialStreamingDevice.cs	1	31
Device/UsbDevice/UsbStreamingDevice.cs	0	24
Device/WiFiDevice/DaqifiStreamingDevice.cs	0	11
View/ConnectionDialog.axaml	1	1
Views/MobileShellViewModel.cs	6	12
production	28	82
tests (2 new files, 3 doubles simplified)	133	14
total, 11 files	161	96

One abstract member and four implementations of it become one concrete method. No new interface,
no new service, no new indirection — SendMessage was already there and already correct.

Wire behaviour

ScpiMessage.GetBytes() is Encoding.ASCII.GetBytes(Data + "\r\n") and
ScpiMessageProducer.TurnDeviceOn is "SYSTem:POWer:STATe 1" (the literal is present in the
pinned Daqifi.Core 1.7.0 assembly), so:

• Serial and USB: byte-identical. Same command, same terminator, same ASCII encoding — the
  producer supplies the framing the view-model used to append by hand. Three tests assert the
  exact byte array.
• WiFi: this is a behaviour change, and it is disclosed rather than hidden. Before, the call
  threw NotSupportedException and was swallowed, so nothing was sent. Now the command goes out.
  That is what the code's own comment says the step is for ("a device that associated on WiFi but
  never powered its ADC front-end streams NO data"), and it is the standard connect handshake —
  Core sends the same command itself during a connect with InitializeDevice=true, and the app
  reaches for it on the serial side in SerialStreamingDevice.PowerOnWifiModule. It is
  idempotent. Not bench-verified over WiFi: the bench board is USB, so this path was reasoned
  and unit-tested, not measured.
• {Binding PortName} vs {Binding Port.PortName}: identical whenever Port is set, which
  it is for every device in that list (both constructors assign it). When Port is null the new
  binding falls back to Name where the old one rendered blank. Port's setter now raises
  PropertyChanged for PortName as well, so the binding refreshes exactly as the old path did.

XAML in this repo is not compile-checked — ConnectionDialog.axaml has no x:DataType, so that
binding resolves by reflection and a wrong name fails silently at runtime with a blank cell.
PortName is a public instance property on SerialStreamingDevice (the element type of
AvailableSerialDevices), it is the only Port. binding path in any .axaml in the repo, and
SerialStreamingDevicePortNameTests pins it so a future rename fails a test instead of blanking
the tile. No member reachable from XAML was renamed or removed by this PR.

Tests

139 passed, 0 failed (main is at 133; this adds 6).

• AbstractStreamingDeviceTurnDeviceOnTests (3) — one message sent, the right mnemonic, and the
  exact framed byte array. The last one is the regression guard for the terminator the view-model
  used to own.
• SerialStreamingDevicePortNameTests (3) — PortName reports the port even once discovery has
  renamed the device, falls back to Name, and notifies when Port is replaced.

Gate

• Desktop: 0 errors. iOS Release / iossimulator-arm64: 0 errors.
• dotnet test: 139 / 139.
• Warnings: zero delta. Both sides cold-built in the same worktree (stash → build → pop) with
  third_party/oxyplot-avalonia/obj cleared each time, third_party/ excluded and (line,col)
  normalised out: 180 distinct first-party warnings on the merge base, 180 on the branch,
  diff-identical. Raw: desktop 500 lines, iOS 502, both sides.
• packages.lock.json unchanged; iOS restore run without -r.
• Android head not built here (workload absent on this Mac) — CI built it green on this head
  (3m39s), so that gap is closed rather than assumed. Every deleted name was grepped across
  Daqifi.Avalonia.Android/, .iOS/, .Desktop/ and .Tests/: no Write call site outside the
  one removed.
• The app was not launched: serial discovery opens ports and other agents were live on this
  machine. The one XAML change was verified statically and pinned with a test instead — see above.

[tylerkron]

/agentic_review

[qodo-code-review]

PR Summary by Qodo

Route device power-on through the messaging abstraction

🐞 Bug fix ✨ Enhancement 🧪 Tests 🕐 20-40 Minutes

AI Description

• Route stream power-on through the device layer and Core SCPI framing.
• Remove raw transport writes across serial, USB, WiFi, and test doubles.
• Bind USB connection tiles to the observable port-name projection.

Diagram

graph TD
  Mobile["Mobile shell"] -->|"TurnDeviceOn"| Device["Device abstraction"] -->|"SCPI message"| Core["Core messaging"] -->|"Framed bytes"| Transport["Device transport"] --> Firmware["DAQiFi firmware"]
  Dialog["Connect dialog"] -->|"PortName binding"| Serial["Serial device"] -->|"Extends"| Device

Loading

High-Level Assessment

The chosen approach is appropriate: a concrete operation on AbstractStreamingDevice preserves the UI/device boundary while reusing Core’s canonical SCPI message and framing path for every transport. Calling Core directly from the view-model would retain layering leakage, while introducing a separate command service would add unnecessary indirection for an operation already owned by the device abstraction.

Files changed (11) +161 / -96

Bug fix (5) +28 / -58

AbstractStreamingDevice.csCentralize device power-on messaging +20/-3

Centralize device power-on messaging

• Removes the raw Write escape hatch and adds TurnDeviceOn using Core’s ScpiMessageProducer through SendMessage. This keeps command syntax and line framing inside the device layer.

Daqifi.Avalonia/Daqifi.Desktop/Device/AbstractStreamingDevice.cs

SerialStreamingDevice.csRemove serial raw writes and publish PortName changes +1/-31

Remove serial raw writes and publish PortName changes

• Deletes direct ASCII writes to the serial transport and legacy SerialPort fallback. Port replacement now raises PropertyChanged for PortName so the connection-dialog binding refreshes.

Daqifi.Avalonia/Daqifi.Desktop/Device/SerialDevice/SerialStreamingDevice.cs

DaqifiStreamingDevice.csRemove unsupported WiFi raw-write API +0/-11

Remove unsupported WiFi raw-write API

• Deletes the Write implementation that always threw NotSupportedException, allowing power-on requests to use the transport-independent Core messaging path instead.

Daqifi.Avalonia/Daqifi.Desktop/Device/WiFiDevice/DaqifiStreamingDevice.cs

ConnectionDialog.axamlBind USB tiles to the device port projection +1/-1

Bind USB tiles to the device port projection

• Changes the connection tile from traversing Port.PortName to binding directly to the device’s PortName property.

Daqifi.Avalonia/Daqifi.Desktop/View/ConnectionDialog.axaml

MobileShellViewModel.csReplace hand-written SCPI power-on bytes +6/-12

Replace hand-written SCPI power-on bytes

• Calls TurnDeviceOn before streaming instead of constructing and writing a terminated SCPI command from the view-model. This preserves best-effort startup behavior while removing transport knowledge from the UI layer.

Daqifi.Avalonia/Views/MobileShellViewModel.cs

Refactor (1) +0 / -24

UsbStreamingDevice.csRemove direct USB transport writes +0/-24

Remove direct USB transport writes

• Deletes the raw ASCII Write implementation so device commands consistently use the Core message path.

Daqifi.Avalonia/Daqifi.Desktop/Device/UsbDevice/UsbStreamingDevice.cs

Tests (5) +133 / -14

AbstractStreamingDeviceChannelAdoptionTests.csRemove obsolete raw-write test stub +2/-4

Remove obsolete raw-write test stub

• Updates the minimal test device after removal of the abstract Write member and revises its documentation to describe the remaining required overrides.

Daqifi.Avalonia.Tests/Device/AbstractStreamingDeviceChannelAdoptionTests.cs

AbstractStreamingDeviceTurnDeviceOnTests.csVerify power-on command content and framing +67/-0

Verify power-on command content and framing

• Adds a recording device double and tests that TurnDeviceOn sends exactly one canonical SCPI message. The tests pin both its command text and ASCII CRLF wire representation.

Daqifi.Avalonia.Tests/Device/AbstractStreamingDeviceTurnDeviceOnTests.cs

AbstractStreamingDeviceWincGateTests.csRemove obsolete write override from WINC tests +4/-6

Remove obsolete write override from WINC tests

• Simplifies the test device now that raw Write is no longer part of AbstractStreamingDevice and updates the accompanying documentation.

Daqifi.Avalonia.Tests/Device/AbstractStreamingDeviceWincGateTests.cs

DroppedDeviceTestDoubles.csAlign dropped-device double with the reduced abstraction +2/-4

Align dropped-device double with the reduced abstraction

• Removes the unused Write stub and documents that dropped-device tests only require connection type and message-sending members.

Daqifi.Avalonia.Tests/Device/DroppedDeviceTestDoubles.cs

SerialStreamingDevicePortNameTests.csCover the observable serial port-name projection +58/-0

Cover the observable serial port-name projection

• Adds tests for reporting the active COM port, falling back to the device name, and notifying PortName bindings when the SerialPort changes.

Daqifi.Avalonia.Tests/Device/SerialStreamingDevicePortNameTests.cs

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Tip of the day

💡 Did you know, you can turn on the rule miner and Qodo learns your standards from review history

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous reviews

Review updated until commit ae5bcaa ⚖️ Balanced

Results up to commit 8c656bb ⚖️ Balanced

No changes from previous review

[tylerkron]

Qodo-clean, CI green — ready for review

Swept at head 8c656bb. All 7 checks green on that exact SHA (the "Claimed bumps are actually applied" check skips correctly on a non-Dependabot PR); MERGEABLE / CLEAN against main 4a8c338; zero unresolved qodo-code-review threads on either surface — no inline threads at all, and the Code Review by Qodo summary reads Bugs (0) / Rule violations (0) / Requirement gaps (0) with updated_at equal to created_at, so it has not been silently re-edited since it landed on this head. The branch sits directly on the current main tip, so CI ran with the workflows from 8977e80.

Three extra checks, because this touches a device path:

Serial/USB byte equivalence — established by reading, not only by test. ScpiMessageProducer.TurnDeviceOn is new ScpiMessage("SYSTem:POWer:STATe 1") and ScpiMessage.GetBytes() is Encoding.ASCII.GetBytes(Data + "\r\n"), so the framed bytes are identical to the "SYSTem:POWer:STATe 1\r\n" the view-model used to hand-write. The destination is the same stream, too: SendMessage → CoreDevice.Send → MessageProducer.WriteMessageToStream, which does _stream.Write(message.GetBytes()), and that producer is constructed over _transport.Stream — exactly what the old Write wrote to. No double terminator, none missing. Two mechanical differences, both in the safe direction: the write is now flushed, and it goes through the single producer queue instead of racing that queue with a direct stream write, so power-on is now strictly ordered ahead of the channel-enable and start commands that follow it.

The WiFi behaviour change is real, is an improvement, and is UNVERIFIED ON HARDWARE. The app's WiFi device connects with InitializeDevice = false, so Core's own connect-time TurnDeviceOn was skipped, and the old Write threw NotSupportedException straight into a bare catch — the power-on step did nothing at all on WiFi. Nothing depended on that throw: the bool return was ignored, WiFi's SendMessage swallows and logs on its own, and the only downstream is InitializeStreaming, which gates on IsStreaming. It is the same command Core itself sends over the same TCP path during an initializing connect, and Core's SessionCommandInterpreter does not track it, so it moves no session state. But nobody has driven it over WiFi against a board. Reviewers should treat "WiFi power-on now actually transmits" as the one claim on this PR that reasoning and unit tests cannot finish.

XAML binding audit. Bindings here are not compile-checked — ConnectionDialog.axaml has no x:DataType and no CompileBindings, so this is the gap the build gate structurally cannot cover. Across all 32 .axaml files on this branch: zero references to the removed Write, and zero remaining Port. walks. The one changed binding, {Binding PortName}, resolves against SerialStreamingDevice — the element type of AvailableSerialDevices is ObservableCollection<SerialStreamingDevice> — where PortName => Port?.PortName ?? Name is a public instance property. The only other PortName hit is the unrelated {Binding ManualPortName} on the manual-entry tab. IStreamingDevice never declared Write, and there are no reflective (nameof(Write) / GetMethod("Write")) references anywhere, so the compile gate covers the remainder of the removal.

Verification only — not merged, nothing changed.

[tylerkron]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit ae5bcaa

[tylerkron]

Ready — the merge-queue failure is fixed

What ejected it. This PR deletes AbstractStreamingDevice.Write(string) and updated every test double that overrode it — every one that existed at 4a8c338, its branch point. #178 landed on main afterwards and added a fourth: ConnectionDialogFirmwarePauseTests.TestDevice. Merged, its

public override bool Write(string command) => throw new NotSupportedException();

had nothing left to override — CS0115, in the Test step of "Library + Desktop head + capture harness" (run 33671611557).

Worth naming, because it will happen again: git was right to call this MERGEABLE / CLEAN. The two changes touch disjoint files and only fail together at compile time, so nothing before the merge queue ever compiled them in the same tree. A green PR check and a clean mergeability status are both blind to this; only the queue's own build sees it.

What changed. Merged current origin/main (5a36aee) in — merge rather than rebase, so the four rounds of Qodo history stay anchored — and deleted the stale override, plus the doc comment that still promised the double could write. It was a placeholder giving ConnectionManager.DeviceBeingUpdated a non-null instance in six tests; none of them writes, sends or connects, and AbstractStreamingDevice now has exactly two abstract members (ConnectionType, SendMessage), both still implemented. #178's subject — discovery staying paused across a manual HID flash — is untouched.

Swept the whole tree, not just the file the compiler named (compilation stops early, so the first error is not the list):

• override bool Write( post-merge: exactly one, the file above. Zero now.
• Callers of .Write( on a streaming device: zero. Every remaining .Write( is Volatile.Write, Stream.Write, or SerialPort.Write on a port FirmwareUpdateCoordinator opens raw itself.

Gate, on the merged result: desktop head clean; iOS restore + Release iossimulator-arm64 clean; packages.lock.json untouched. 284 tests pass, up from 278 on main — the +6 are this PR's own AbstractStreamingDeviceTurnDeviceOnTests and SerialStreamingDevicePortNameTests. Qodo re-reviewed at ae5bcaa: 0 bugs, 0 rule violations, 0 requirement gaps, no inline threads.

The rest of the queue is clear. Every other open PR (#195, #196, #198, #199, #200, #202) was test-merged onto 5a36aee and built, not diffed — a diff and a mergeable status both missed this one. All six compile and pass. None of them removes or renames a public member, and none deletes or renames a file, so none has this shape. The whole batch merged together (main + this + all six) also builds clean, 336 tests passing.