[tylerkron]

closes #257

Not merging — for review.

The ticket's premise does not survive the bench

#257 says the comment above the second HydrateDeviceMetadata call in Connect() cites a
22000 Hz ceiling that pinned Daqifi.Core 1.7.0 cannot produce, because "the same sentence says
this board publishes no capability document".

The board publishes one. Read straight off the attached Nq1 (fw 3.7.2, USB,
/dev/cu.usbmodem1101) with the two queries Core itself sends:

=== CONFigure:CAPabilities:APIVersion?
2

=== CONFigure:CAPabilities:JSON?
{"schema_version":2, ... ,"streaming":{ ... "sample_rate_range_hz":{"min":1,"max":22000},
 "conservative_envelope_hz":500,"current_max_rate_hz":7746,
 "rate_model":{"formula":"min(absolute_max_hz, type1_aggregate_max_hz/simultaneous_count, ...)",
 "absolute_max_hz":22000,"type1_aggregate_max_hz":55000,"per_tick_budget_hz":110000,
 "per_tick_overhead":6}, ... }, ... }

Core 1.7.0 accepts schema 1..2 (DaqifiDevice.Minimum/MaximumCapabilityDocumentApiVersion) and the
feature gate is firmware ≥ 3.5.0 (DeviceFeatureRequirements.CapabilityDocumentMinFirmware), so
this document is read and applied. CapabilityDocument.MergeInto assigns
capabilities.MaxSamplingRate = Streaming.MaximumSampleRateHz — that is the
sample_rate_range_hz.max above. 22000 is exactly derivable, and it is this device's own
number.

The sentence the ticket read as "the device has no capability document" is about the wrapper:
without the second hydrate, the wrapper is what has none. Ambiguous, and it cost a ticket — so it
is rewritten here.

What the call actually does, measured

A throwaway console app against the pinned package (DaqifiStreamingDevice over
SerialStreamTransport, snapshotting Metadata at the two points Connect() hydrates at):

--- HYDRATE #1 (first ChannelsPopulated, raise 1)
  PartNumber         : 'Nq1'
  DeviceType         : Nyquist1
  FirmwareVersion    : '3.7.2'
  MaxSamplingRate    : 1000
  CapabilityDocument : null
  Channels           : 32

--- HYDRATE #2 (immediately after InitializeAsync)
  MaxSamplingRate    : 22000
  CapabilityDocument : schema 2, 32 channels, streaming.max=22000, current_max=7746
  Channels           : 32

ChannelsPopulated raises during connect: 1

Rate Core accepts now that init is complete:
    1000 -> accepted (1000)
    1001 -> accepted (1001)
    5000 -> accepted (5000)
   22000 -> accepted (22000)
   22001 -> rejected (ArgumentOutOfRangeException)

Two things follow, and together they are the real justification:

1. MaxSamplingRate does move 1000 → 22000 across that call, and CapabilityDocument null →
   present. So the comment's example was right; the field is evidence for the call after all.
2. ChannelsPopulated is raised exactly once during the whole connect. Core re-raises it after
   applying a document only from SyncAnalogOutputChannelsFromCapabilities, and only when the
   document changes the DAC set — this Nq1's document lists 16 analog inputs and 16 digital I/O and
   no analog outputs, so nothing fires. The post-initialize hydrate is the wrapper's only chance
   to see the document at all.

Nothing later rescues it either: DeviceMetadata.UpdateFromProtobuf re-applies the retained
overlay on every subsequent status message, but the wrapper's copy is only refreshed by a hydrate,
and the only other one runs from that single ChannelsPopulated.

Why #255's bench said 1000

#255 reported 1001 and 30000 clamping to 1000 on this same board. That measurement is of
SampleRateCap/the MCP path, not of Capabilities.MaxSamplingRate after a completed
InitializeAsync — as the run above shows, 1001 is accepted once the document has been applied.
Not chased further here: it is a different layer and a different PR, but it is why the ticket's
"this matches the bench run in #255" reads as corroboration when it is not.

The change

The comment, rewritten to argue from the mechanism instead of a figure: it names the two-stage
ordering, says the document is the only thing in Core that moves Capabilities off the board table,
says why no second ChannelsPopulated rescues it, and ends with the ceiling is the device's to
state — read Capabilities.MaxSamplingRate, so the next reader does not treat any number written
here as a constant. 13 lines replacing 8.

Tests — the part that makes it durable. AbstractStreamingDevice.cs had no coverage of this
call at all, which is exactly why a ticket could propose deleting it and nothing would have
objected. Three cases in a new file, driving the real Connect() template with a Core double whose
InitializeAsync replays the measured ordering (status message → ChannelsPopulated → capability
document):

• The_capability_document_reaches_the_wrapper_only_after_initialization — at ChannelsPopulated
  the wrapper holds 1000 and a null document; when Connect() returns it holds 22000 and the
  document.
• The_ceiling_the_rate_guard_clamps_against_is_the_one_the_document_raised — 5000 is stored, not
  clamped to 1000. This is the user-visible consequence, via fix(streaming-rate): a profile with an out-of-range rate makes the device refuse to record (#248) #255's setter guard.
• A_board_that_publishes_no_document_keeps_the_board_table_ceiling — the control, and the common
  case below firmware 3.5.0: nothing to overlay, ceiling stays at 1000.

Plus two lines in StreamingFrequencyRangeTests noting that its 22000 is this board's measured
figure rather than an invented one.

Nothing was deleted, and no number was swapped. The one-line "22000 → 1000" edit #257 proposed
would have been wrong: it would have replaced a correct figure with the board-table default and
left the call looking unjustified.

Verification

All three new tests pass against unmodified main — the production code is correct, so they
must. The meaningful check is the other direction: with HydrateDeviceMetadata(coreDevice.Metadata)
commented out of Connect(), 2 of the 3 fail —
The_capability_document_reaches_the_wrapper_only_after_initialization on
Assert.NotNull(device.Metadata.CapabilityDocument), and
The_ceiling_the_rate_guard_clamps_against_is_the_one_the_document_raised with
Expected: 5000 / Actual: 1000. The control passes either way, which is what a control is for. The
call was restored and the working tree verified clean before the gate.

Full suite 587/587.

Build gate — desktop and iOS Release/iossimulator-arm64 both green. Warnings measured on my
own merge-base (104d9ae, this branch is rebased onto it), both sides force-rebuilt with
-t:Rebuild: 191 all-project total on each, reconciling as 171 first-party + 20 vendored OxyPlot
warnings on both (13 AVP1002, 4 CS0108, 2 CS0169, 1 CS0618 — the count is 20, not all of
them are AVP1002). Diffing the first-party warning sets gives zero added and zero removed; the one
line-number delta is the same pre-existing CS8625 in this file moving 1785 → 1790, which is the
five extra comment lines above it. No packages.lock.json changed. Android is not buildable on this
Mac (workload absent); CI covers it.

Bench safety. Non-destructive throughout: two SCPI queries plus SYSTem:ECHO 0 on the raw port,
then a normal connect/initialize/disconnect through Core. No firmware or WiFi flash, no SD command,
no reboot or power-cycle. StreamingFrequency is a client-side property on Core — setting it sends
nothing to the device — and it was restored before disconnect anyway. Board lock held for the
duration and released.

Observed, deliberately not fixed here

• The rate sliders still cap at 1000. Minimum="1" Maximum="1000" in the views, while this
  board advertises 22000. The wrapper and Core would both take more; the UI cannot ask for it. A
  real gap, but a UI change with its own argument, not a comment fix.
• current_max_rate_hz is 7746 on this board, not 22000. The wrapper clamps against the
  absolute ceiling only, so a user could pick 20000 and have the device refuse at start. fix(streaming-rate): a profile with an out-of-range rate makes the device refuse to record (#248) #255
  considered and deferred this (Core has EnforceStreamingFrequencyCap for it); nothing here
  changes that trade.
• No screenshot. Internal: the change is a comment and three tests, with no rendered surface.

[tylerkron]

/agentic_review

[qodo-code-review]

PR Summary by Qodo

Pin post-initialize capability metadata hydration

🧪 Tests 📝 Documentation 🕐 20-40 Minutes

AI Description

• Pins post-initialization hydration so device capability documents reach the desktop wrapper.
• Verifies advertised sampling ceilings override board defaults and govern rate clamping.
• Clarifies 22 kHz is device-provided metadata, not an application constant.

Diagram

sequenceDiagram
    actor Caller
    participant Wrapper as Device Wrapper
    participant Core as Core Device
    participant Board as Physical Board
    participant Meta as Wrapper Metadata
    Caller->>Wrapper: Connect
    Wrapper->>Core: Initialize
    Core->>Board: Request status
    Board-->>Core: Board metadata
    Core-->>Wrapper: Channels populated
    Wrapper->>Meta: First hydrate
    Core->>Board: Request capabilities
    Board-->>Core: Device ceiling
    Core-->>Wrapper: Init complete
    Wrapper->>Meta: Final hydrate
    Meta-->>Wrapper: Updated limits

Loading

High-Level Assessment

The targeted connection-template tests are the appropriate approach because they exercise the real event wiring, hydration sequence, and rate guard without requiring physical hardware. Testing hydration in isolation would miss the ordering regression this PR is intended to prevent.

Files changed (3) +189 / -8

Tests (1) +174 / -0

AbstractStreamingDeviceMetadataHydrationTests.csCover two-stage metadata hydration during connection +174/-0

Cover two-stage metadata hydration during connection

• Adds a Core device double that reproduces status-first, capability-document-second initialization. Tests verify the wrapper receives the document after initialization, uses its sampling ceiling, and retains board defaults when no document exists.

Daqifi.Avalonia.Tests/Device/AbstractStreamingDeviceMetadataHydrationTests.cs

Documentation (2) +15 / -8

StreamingFrequencyRangeTests.csDocument the source of the 22 kHz test ceiling +2/-0

Document the source of the 22 kHz test ceiling

• Clarifies that the 22 kHz maximum comes from a measured Nq1 capability document rather than a hypothetical or application-defined limit.

Daqifi.Avalonia.Tests/Device/StreamingFrequencyRangeTests.cs

AbstractStreamingDevice.csClarify why post-initialize metadata hydration is required +13/-8

Clarify why post-initialize metadata hydration is required

• Rewrites the connection comment to explain Core’s two-stage metadata discovery, conditional channel event, and dependency of rate clamping on device-provided capabilities. It also links the hydrate call to its regression tests.

Daqifi.Avalonia/Daqifi.Desktop/Device/AbstractStreamingDevice.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 show, collapse, or hide each part of a finding: code, evidence, and all

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[tylerkron]

Ready for review at c9f03b0f — Qodo clean and settled on this exact SHA (0 findings, 0 review threads, re-checked ~45 min later on both surfaces), CI run 34005243623 green with head_sha matching this SHA, and the reversal independently confirmed.

Verified on hardware (Nq1 fw 3.7.2, /dev/cu.usbmodem1101, read-only — two CONFigure:CAPabilities queries and one connect/initialize/disconnect; no flash, SD, reboot or streaming start):

• the board does publish a schema-2 capability document — sample_rate_range_hz {min 1, max 22000}, current_max_rate_hz 7746, rate_model.absolute_max_hz 22000, 16 analog-input + 16 digital-io + 0 analog-output;
• through the pinned Core 1.7.0 package, MaxSamplingRate goes 1000 → 22000 across InitializeAsync() with CapabilityDocument null → present, and ChannelsPopulated raises exactly once. The zero analog-output channels independently confirm the stated mechanism for why no second raise rescues the wrapper.

So #257's premise is false on the metal and the proposed 22000 → 1000 one-liner would have been wrong.

Verified statically only (not exercised on the board): that Core's schema-version bounds (1..2) and the fw ≥ 3.5.0 gate are what admit this document — read from MergeInto / DeviceFeatureRequirements and cross-checked against the pinned 1.7.0 assembly's own API surface rather than Core main. The merge semantics themselves (MaxSamplingRate = Streaming.MaximumSampleRateHz) are covered behaviourally by the new tests against the pinned package, not merely read.

Also checked: the 2-of-3 failure claim reproduced by actually removing the hydrate call (2 failed with the stated assertions, control passed); +189/−8 and 13-for-8 re-derived; 191 warnings on both force-rebuilt sides with a first-party set diff of 0 added / 0 removed and only the CS8625 1785 → 1790 shift; no packages.lock.json churn. Test-merged onto current main (84380463) and additionally against #267 (3157dc2) — both clean, desktop builds, 591/591 (= 587 + the 4 tests #266 added; the body's 587/587 is correct for its own base). Added type name is unique, and no member is deleted, so nothing can break an unchecked XAML binding.

One free body edit, no commit: the 20 vendored OxyPlot warnings are 13 AVP1002 + 4 CS0108 + 2 CS0169 + 1 CS0618, not 20 AVP1002 — the count and the 191 total were right, only the label was wrong.

Follow-ups filed: #270 for the rate sliders still capping at 1000 while the board advertises 22000 (with the unenforced current_max_rate_hz 7746 folded in as an explicit constraint — it is latent today because the UI cannot offer >1000, and becomes live the moment the cap is raised naively). A correcting comment posted on #255, whose merged body states MaxSamplingRate on this board is 1000; its merged fix is unaffected and still correct.

Not merging.