bench: baseline DecodeRawAnalogFrame (AnalogInData)

[tylerkron]

What was wrong

Supported firmware streams raw ADC counts (AnalogInData). The benchmark harness still timed AnalogInDataFloat as the hot path:

• DecodeAnalogFloatFrame was the BenchmarkDotNet baseline, and its summary said the firmware's fast streaming encoder sends calibrated floats. It doesn't: the fast encoder (Nanopb_EncodeStreamingFast in daqifi-nyquist-firmware) writes analog values only as field 2, analog_in_data (packed sint32), and the nanopb path zeroes analog_in_data_float_count.
• DecodeCombinedFrame and SyntheticFrames, which feeds ProtobufFramingBenchmarks and StreamConsumerBenchmarks, built float frames too. So those benchmarks timed a payload no supported firmware sends.
• The README had two protobuf tables. The 2a59fd1 baseline still showed the pre-perf(sdcard): stop the protobuf log parser decoding a whole 64 KB buffer before its first sample #699 first-sample outlier, and a second table showed perf(sdcard): stop the protobuf log parser decoding a whole 64 KB buffer before its first sample #699's after numbers for the same methods.

How it was fixed

• DecodeRawAnalogFrame is [Benchmark(Baseline = true)]. DecodeAnalogFloatFrame stays as the defensive protocol branch: the decoder still accepts pre-scaled floats, but no supported firmware fills that field.
• DecodeCombinedFrame carries raw counts plus DIO.
• Framing and consumer buffers carry AnalogInData raw counts (1_000 + channel), the same shape SdCardParseBenchmarks already uses.
• A raw-count frame is about 40 bytes, not about 71, so ProtobufFramingBenchmarks.FrameCount goes from 50 to 100 (3,959 bytes). The buffer still models one 4 KB read, which is StreamMessageConsumer's default buffer.
• README: one table. The two protobuf SD-card rows are perf(sdcard): stop the protobuf log parser decoding a whole 64 KB buffer before its first sample #699's after figures (ProtobufTimeToFirstSample 9.4 µs / 263 KB, ProtobufDrainAll 2,091.4 µs / 13,170 KB), labelled as copied from perf(sdcard): stop the protobuf log parser decoding a whole 64 KB buffer before its first sample #699 and not re-measured. A small table lists each row's current payload next to the payload its published number was measured on. DecodeCombinedFrame, framing and consumer were measured on floats and have not been re-measured. It also says nothing refreshes the table: the Benchmarks workflow runs on ubuntu-latest and only posts to its run summary. The 2a59fd1 before figures behind the "800× CSV" comparison are kept in the text.

SdCardParseBenchmarks is untouched. No production code, CI, or tests.

Verification

• dotnet build Daqifi.Core.sln -c Release: 0 warnings.
• --job dry run of StreamDecodeBenchmarks, ProtobufFramingBenchmarks and StreamConsumerBenchmarks: all 6 cases run, and DecodeRawAnalogFrame reports Ratio 1.00. That was a smoke test only. The suite was not re-measured, and the README says so.

🤖 Generated with Claude Code

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. Benchmark notes mislabel raw payloads ✓ Resolved 🐞 Bug ⚙ Maintainability ⭐ New

Description

The README says the framing and consumer rows use raw-count buffers, then says those same rows
describe float payloads. Anyone using the documented baseline therefore cannot determine which
payload the published framing and consumer measurements represent.

Code

src/Daqifi.Core.Benchmarks/README.md[R83-87]

+`DecodeRawAnalogFrame` is now the decode baseline, and `DecodeCombinedFrame`, the framing
+buffers and the consumer buffers carry raw counts (`AnalogInData`, what supported firmware
+sends) instead of floats. The framing buffer is also 100 frames instead of 50, so it still
+fills one 4 KB read. The `DecodeCombinedFrame`, framing and consumer rows therefore
+describe float payloads. `DecodeRawAnalogFrame` and `DecodeAnalogFloatFrame` time the same

Relevance

●●● Strong

The sentence directly contradicts the preceding raw-count description; documentation corrections are
consistently accepted.

PR-#754

ⓘ Recommendations generated based on similar findings in past PRs

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The README first states that the framing and consumer buffers carry raw `AnalogInData` counts, but then says the framing and consumer rows describe float payloads. This contradicts the updated `SyntheticFrames` fixture used by those benchmarks and makes the published results ambiguous.

### Fix Focus Areas
- src/Daqifi.Core.Benchmarks/README.md[83-88]

### Recommended Fix
Rewrite the affected sentence so `DecodeCombinedFrame`, framing, and consumer rows are explicitly described as raw-count payloads. Keep `DecodeAnalogFloatFrame` identified as the only float-payload benchmark, if that is the intended scope of the table.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Combined timings still measure unsupported floats ✓ Resolved 🐞 Bug ≡ Correctness

Description

Setup still builds _combined with BuildFrames(analogFloat: true, digital: true), so the
combined benchmark exercises AnalogInDataFloat rather than the raw-count path. This contradicts
the new hot-path claim and makes DecodeCombinedFrame measure a payload that supported firmware
does not produce.

Code

src/Daqifi.Core.Benchmarks/StreamDecodeBenchmarks.cs[R80-81]

+    /// The hot path. Every supported firmware sends raw ADC counts, on every transport, and
+    /// each value is scaled through <see cref="IAnalogChannel.GetScaledValue"/>.

Relevance

●●● Strong

Finding directly contradicts the PR’s stated raw-count benchmark intent; analogous benchmark
correctness fixes were accepted.

PR-#698
PR-#754

ⓘ Recommendations generated based on similar findings in past PRs

---

Context sources

✅ Compliance rules (platform): 13 rules

Review mode: ⏭️ Skipped: This push only revises benchmark README prose and tables, with no runtime, configuration, test, or other behavioral changes.

Tip of the day

💡 Did you know, you can describe a rule in plain language on the Rules page and Qodo drafts it for you

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous reviews

Review updated until commit 6dad3d2

Results up to commit d136df8 🚀 Fast

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Combined timings still measure unsupported floats ✓ Resolved 🐞 Bug ≡ Correctness

Description

Setup still builds _combined with BuildFrames(analogFloat: true, digital: true), so the
combined benchmark exercises AnalogInDataFloat rather than the raw-count path. This contradicts
the new hot-path claim and makes DecodeCombinedFrame measure a payload that supported firmware
does not produce.

Code

src/Daqifi.Core.Benchmarks/StreamDecodeBenchmarks.cs[R80-81]

+    /// The hot path. Every supported firmware sends raw ADC counts, on every transport, and
+    /// each value is scaled through <see cref="IAnalogChannel.GetScaledValue"/>.

Relevance

●●● Strong

Finding directly contradicts the PR’s stated raw-count benchmark intent; analogous benchmark
correctness fixes were accepted.

PR-#698
PR-#754

ⓘ Recommendations generated based on similar findings in past PRs

---

[qodo-code-review]

PR Summary by Qodo

Benchmark raw analog decoding as the supported firmware hot path

🧪 Tests ✨ Enhancement 📝 Documentation 🕐 10-20 Minutes

AI Description

• Makes raw ADC frame decoding the baseline used by supported firmware.
• Feeds framing and consumer benchmarks realistic AnalogInData payloads.
• Consolidates documentation and records post-#699 protobuf SD-card results.

Diagram

graph TD
  RAW["Raw ADC Counts"] --> DECODE["Decode Benchmark"] --> SCALE["Channel Scaling"]
  RAW --> SYNTH["Synthetic Frames"] --> FRAMING["Framing Benchmark"]
  SYNTH --> CONSUMER["Stream Consumer"]
  DECODE --> README["Baseline README"]
  FRAMING --> README
  CONSUMER --> README

Loading

High-Level Assessment

The selected approach is appropriate: it preserves the float decoder benchmark for defensive protocol coverage while making raw counts the realistic baseline everywhere supported firmware is modeled. Removing the float case would reduce coverage, and a full benchmark rerun is better handled by the documented workflow dispatch.

Files changed (3) +22 / -27

Documentation (1) +12 / -20

README.mdConsolidate and clarify benchmark baseline results +12/-20

Consolidate and clarify benchmark baseline results

• Replaces outdated protobuf SD-card figures with post-#699 raw-path measurements and removes the duplicate before/after table. Clarifies which results were copied, which still represent the previous harness, and when they will be refreshed.

src/Daqifi.Core.Benchmarks/README.md

Other (2) +10 / -7

StreamDecodeBenchmarks.csMake raw analog decoding the benchmark baseline +7/-5

Make raw analog decoding the benchmark baseline

• Promotes 'DecodeRawAnalogFrame' to the BenchmarkDotNet baseline because supported firmware emits raw ADC counts. Retains float decoding as a non-baseline defensive protocol case and updates the benchmark documentation accordingly.

src/Daqifi.Core.Benchmarks/StreamDecodeBenchmarks.cs

SyntheticFrames.csGenerate synthetic frames with raw ADC counts +3/-2

Generate synthetic frames with raw ADC counts

• Changes shared framing and consumer benchmark buffers from 'AnalogInDataFloat' values to realistic 'AnalogInData' counts. Updates documentation to identify the firmware-supported payload shape.

src/Daqifi.Core.Benchmarks/SyntheticFrames.cs

[tylerkron]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 67a5738

[tylerkron]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 6dad3d2

[tylerkron]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 6dad3d2

[tylerkron]

Qodo-clean, CI green — ready for review