Skip to content

feat(dio): first-class digital output — correct stream bit mapping, MCP tools, docs#280

Merged
tylerkron merged 2 commits into
mainfrom
claude/mystifying-tereshkova-1c19ba
Jul 3, 2026
Merged

feat(dio): first-class digital output — correct stream bit mapping, MCP tools, docs#280
tylerkron merged 2 commits into
mainfrom
claude/mystifying-tereshkova-1c19ba

Conversation

@tylerkron

Copy link
Copy Markdown
Contributor

Summary

Digital output (SetDioDirection / SetDioValue) existed in Core but was effectively invisible: undocumented, unexposed over MCP, and never verified against hardware. This PR makes digital out a first-class, hardware-validated capability.

Stream decode fix (the load-bearing change)

DecodeDigital mapped stream bits by the channel's position within the enabled-channel list (dense packing, inherited from the desktop reference implementation). The firmware actually streams the whole DIO port as a raw pin-state snapshot — bit N is pin N, regardless of which channels the client enabled, because the wire-level DIO enable (DIO:PORt:ENAble) is global, not per pin (DIO_StreamingTrigger reads with mask 0xFFFF and digital_data is a raw memcpy of the port value; see NanoPB_Encoder.c / DIO.c in daqifi-nyquist-firmware).

With a subset of digital channels enabled, the old decode read the wrong bits — e.g. enabling only DIO 4 read pin 0's state. Decode now indexes by channel number. Channels beyond the payload get no sample (unchanged), and output-direction channels remain unsampled (unchanged, matches desktop semantics).

MCP server

  • configure_digital_channels — enable exactly the given digital channels (mirrors configure_analog_channels)
  • set_digital_directioninput / output
  • set_digital_output — drive high/low; auto-switches an input channel to output so one call drives a pin
  • list_channels now reports OutputValue for digital channels; get_device_status reports EnabledDigitalChannels

Docs

README capability row + digital-output recipe; MCP README tool table.

Hardware validation (Nyquist 1, USB serial)

Bench rig: +5 V into DIO pin 0, loopbacks pin 2→4 and pin 13→15. Two phases, 16/16 checks passed:

  • Raw SCPI (exact command strings Core produces): direction set/readback (confirmed 1 = output on the wire), state set/query through both loopbacks, both levels.
  • Core end-to-end: SetDioDirection/SetDioValue through DaqifiDeviceFactory.ConnectSerialAsync, streaming at 10 Hz with a strict subset of digital channels enabled ({0, 4, 15}) — each channel tracked its own pin (the old positional decode fails this exact scenario), through four drive combinations on two loopbacks.

Also observed while bench-testing (documented here for future reference, not addressed in this PR):

  • The bench unit's firmware does not reliably start a stream when zero analog channels are enabled (DIO-only streaming). Streaming digital inputs alongside at least one analog channel works dependably. Likely a firmware-side issue; worth a daqifi-nyquist-firmware issue if reproducible on current firmware.
  • The Nyquist 1 board terminal labels don't map 1:1 to firmware pin indices on this unit (first bank labeled "DIO 1..8" → firmware pins 0..7; the 13↔15-labeled terminals mapped to firmware pins 13/15). UI layers should surface firmware pin indices consistently.

Tests

  • 2 new decode tests that specifically discriminate channel-number mapping from positional mapping (subset enables with noise bits); all 15 decode tests pass.
  • 5 new MCP agent tests (read-only blocking, direction parsing, unknown-device errors).
  • Full suite: 1301 Core + 20 MCP tests green on net9.0 and net10.0.

🤖 Generated with Claude Code

…CP tools, docs

Digital output (SetDioDirection / SetDioValue) existed in Core but was
unadvertised, unexposed over MCP, and never verified against hardware.
Bench-testing against a real Nyquist 1 surfaced a latent decode bug:

- DecodeDigital mapped stream bits by the channel's position within the
  enabled-channel list (dense packing, inherited from the desktop
  reference implementation). The firmware actually streams the whole
  DIO port as a raw pin-state snapshot — bit N is pin N, regardless of
  which channels the client enabled (the wire-level DIO enable is
  global). Subset enables therefore read the wrong bits. Decode now
  indexes by channel number, per the firmware wire format.

- MCP server: new configure_digital_channels, set_digital_direction,
  and set_digital_output tools; list_channels now reports OutputValue
  for digital channels and get_device_status reports enabled digital
  channels.

- README: digital I/O capability row and a digital-output recipe.

Verified on a Nyquist 1 over USB serial (+5V into DIO pin 0, loopbacks
pin 2->4 and 13->15): direction semantics (1=output), state set/query,
and Core end-to-end streaming with a strict subset of digital channels
enabled ({0, 4, 15}) tracking driven outputs through both loopbacks.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@tylerkron tylerkron requested a review from a team as a code owner July 3, 2026 19:17
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Make digital output first-class: fix stream bit mapping + MCP tools + docs

🐞 Bug fix ✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

Grey Divider

AI Description

• Fix digital stream decoding to map bits by pin/channel number, not enabled-channel position.
• Add MCP tools to configure digital channels, set direction, and drive digital outputs.
• Extend status/channel reporting and add tests + docs for the digital I/O workflow.
Diagram

sequenceDiagram
  participant C as MCP Client
  participant T as DaqifiTools (MCP)
  participant A as DaqifiAgent
  participant S as Daqifi.Core StreamingDevice
  participant D as Nyquist Device

  C->>T: set_digital_output(device_id, ch, high)
  T->>A: SetDigitalOutputAsync(...)
  A->>S: SetDioDirection(ch, Output)
  A->>S: SetDioValue(ch, high/low)
  S->>D: SCPI DIO direction/value
  D-->>S: Stream frame (DigitalData port snapshot)
  S-->>S: DecodeDigital(bitIndex = ChannelNumber)
  A-->>T: DigitalPinResult / ChannelInfo
  T-->>C: Tool response
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Firmware-version/capability gated decode
  • ➕ Preserves compatibility if older firmware ever used dense-packed enabled-channel encoding
  • ➕ Allows explicit behavior selection per device model/firmware
  • ➖ Requires reliable firmware versioning/capability reporting and additional branching complexity
  • ➖ Harder to test without representative legacy devices/fixtures
2. Heuristic auto-detect (dense vs raw snapshot)
  • ➕ Potentially supports both encodings without firmware handshake
  • ➕ Can fall back based on payload size vs enabled channel count
  • ➖ Heuristics can be ambiguous (e.g., payload size coincidentally matches enabled count)
  • ➖ Adds complexity and risk of mis-detection in edge cases

Recommendation: Current approach (bitIndex = ChannelNumber, skip channels beyond payload, don’t sample output-direction pins) is the simplest and best fit for the described firmware wire format and matches the PR’s hardware validation. Consider adding a version-gated or heuristic fallback only if you expect to support firmware variants that densely-pack enabled channels.

Files changed (8) +310 / -24

Enhancement (3) +188 / -6
DaqifiAgent.csAdd digital channel config + direction/output control APIs +122/-0

Add digital channel config + direction/output control APIs

• Implements ConfigureDigitalChannelsAsync (enable exact set), SetDigitalDirectionAsync (input/output parsing), and SetDigitalOutputAsync (auto-switch to output before driving). Adds helpers to list enabled digital channels and validate channel existence.

src/Daqifi.Mcp/DaqifiAgent.cs

Dtos.csExpose enabled digital channels and digital output value in DTOs +40/-6

Expose enabled digital channels and digital output value in DTOs

• Extends DeviceStatus with EnabledDigitalChannels and ChannelInfo with OutputValue for digital channels. Adds new result DTOs for digital configuration (ConfigureDigitalResult) and pin operations (DigitalPinResult).

src/Daqifi.Mcp/Dtos.cs

DaqifiTools.csExport MCP tools for digital channel configuration and output driving +26/-0

Export MCP tools for digital channel configuration and output driving

• Adds MCP tool wrappers for configuring digital channels, setting digital direction, and driving digital outputs, including user-facing descriptions for input schemas and behavior.

src/Daqifi.Mcp/Tools/DaqifiTools.cs

Bug fix (1) +17 / -18
DaqifiStreamingDevice.csFix DecodeDigital to index bits by channel number +17/-18

Fix DecodeDigital to index bits by channel number

• Updates digital stream decoding to interpret DigitalData as a raw port snapshot where bit N corresponds to pin/channel N. Keeps existing semantics: skip output-direction channels and avoid generating samples when payload lacks that bit.

src/Daqifi.Core/Device/DaqifiStreamingDevice.cs

Tests (2) +84 / -0
DaqifiStreamingDeviceDecodeTests.csAdd regression tests for digital bit mapping by channel number +40/-0

Add regression tests for digital bit mapping by channel number

• Introduces tests that enable sparse subsets of digital channels and verify streamed bits are read by channel number (pin index), not by enabled-channel position. Covers noise-bit scenarios to discriminate the two behaviors.

src/Daqifi.Core.Tests/Device/DaqifiStreamingDeviceDecodeTests.cs

DaqifiMcpTests.csTest new digital MCP operations and validation behavior +44/-0

Test new digital MCP operations and validation behavior

• Adds tests ensuring digital configuration/output tools are blocked in read-only mode, direction parsing fails early for invalid strings, and unknown-device errors are actionable.

src/Daqifi.Mcp.Tests/DaqifiMcpTests.cs

Documentation (2) +21 / -0
README.mdDocument digital I/O capability and add digital-output recipe +18/-0

Document digital I/O capability and add digital-output recipe

• Adds Digital I/O to the capabilities table and includes a short C# recipe showing how to switch a DIO channel to output, drive high/low, and return to input.

README.md

README.mdDocument new MCP digital tools +3/-0

Document new MCP digital tools

• Updates the MCP tool table to include configure_digital_channels, set_digital_direction, and set_digital_output.

src/Daqifi.Mcp/README.md

@qodo-code-review

qodo-code-review Bot commented Jul 3, 2026

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Informational

1. Racy README channel lookup ✓ Resolved 🐞 Bug ☼ Reliability
Description
The new README digital-output example enumerates device.Channels directly
(device.Channels.First(...)), but Channels is a live view that can be repopulated concurrently,
which can throw InvalidOperationException during enumeration. The sample should use
device.GetChannelsSnapshot() (or otherwise snapshot under the channels lock) before running LINQ
queries.
Code

README.md[R170-173]

+using Daqifi.Core.Channel;
+
+var dio3 = device.Channels.First(c => c.Type == ChannelType.Digital && c.ChannelNumber == 3);
+
Relevance

⭐⭐⭐ High

Team accepted avoiding live Channels enumeration via snapshotting (PR #277); Core also added locked
channel snapshots (PR #250).

PR-#277
PR-#250

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The README sample uses device.Channels.First(...), while Core documentation states Channels is a
live view and recommends GetChannelsSnapshot() to avoid concurrent-mutation exceptions during
enumeration.

README.md[164-179]
src/Daqifi.Core/Device/DaqifiDevice.cs[46-65]

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 digital-output snippet uses `device.Channels.First(...)`, but `Channels` is a live view over the backing list and can be repopulated concurrently, making LINQ enumeration racy.

### Issue Context
Core provides `GetChannelsSnapshot()` specifically to allow safe enumeration off the consumer thread.

### Fix Focus Areas
- README.md[170-173]

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


Grey Divider

Qodo Logo

Comment thread README.md
Channels is a live view that can be repopulated concurrently; the
snapshot is the safe way to enumerate (Qodo review on #280).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant