fix: correct EnableAdcChannels docs from binary to decimal bitmask

[tylerkron]

Summary

• The firmware's SCPI_ADCChanEnableSet parses the ENAble:VOLTage:DC parameter as a decimal integer via SCPI_ParamInt32, but the EnableAdcChannels XML docs, README, and DEVICE_INTERFACES.md all showed binary strings like "0000000011".
• This mismatch meant callers following the documented examples would send the wrong channel configuration (e.g. firmware parses "11" as decimal 11 = channels 0,1,3 instead of intended channels 0,1).
• Updated all docs and the test to use correct decimal bitmask values (e.g. "3" for channels 0+1, "84" for channels 2,4,6).

Related: daqifi/daqifi-desktop#439

Note: The daqifi-core-example-app also has this issue — its IsValidChannelMask() validates for binary strings and --channels help text likely shows binary format. That's a separate repo and should be fixed separately.

Test plan

• All 818 core tests pass on both net8.0 and net9.0
• Manual test: use example app with --channels 3 to enable channels 0+1

🤖 Generated with Claude Code

[qodo-code-review]

ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan

Review Summary by Qodo

Fix EnableAdcChannels documentation from binary to decimal bitmask

🐞 Bug fix 📝 Documentation

Walkthroughs

Description

• Corrects EnableAdcChannels parameter format from binary to decimal
• Updates XML docs with accurate decimal bitmask examples
• Fixes README and DEVICE_INTERFACES.md code examples
• Updates unit test to use correct decimal format

Diagram

flowchart LR
  A["Binary format<br/>0000000011"] -->|Incorrect| B["Firmware parses<br/>as decimal 11"]
  C["Decimal format<br/>3"] -->|Correct| D["Firmware parses<br/>as decimal 3"]
  B -->|Wrong channels| E["Channels 0,1,3"]
  D -->|Correct channels| F["Channels 0,1"]
  G["Updated docs<br/>& examples"] -->|References| C

Loading

File Changes

1. src/Daqifi.Core.Tests/Communication/Producers/ScpiMessageProducerTests.cs 🧪 Tests +2/-2

Update test to use decimal bitmask format

• Changed EnableAdcChannels test from binary string "0001010100" to decimal "84"
• Updated expected command output to use decimal format

src/Daqifi.Core.Tests/Communication/Producers/ScpiMessageProducerTests.cs

---

2. src/Daqifi.Core/Communication/Producers/ScpiMessageProducer.cs 📝 Documentation +13/-14

Correct EnableAdcChannels XML docs to decimal format

• Updated XML documentation to clarify decimal bitmask format instead of binary
• Changed parameter description from binary string to decimal integer string
• Updated code examples to show decimal values (e.g., "84" for channels 2,4,6 and "3" for channels
 0,1)
• Added explanation of bit-to-channel mapping with decimal values

src/Daqifi.Core/Communication/Producers/ScpiMessageProducer.cs

---

3. src/Daqifi.Core/Device/DaqifiStreamingDevice.cs 📝 Documentation +2/-1

Update StartLoggingAsync docs to decimal bitmask

• Updated channelMask parameter documentation from binary to decimal format
• Changed example from "0000000011" to "3"
• Added clarification that firmware parses as decimal integer

src/Daqifi.Core/Device/DaqifiStreamingDevice.cs

---

View more (3)

4. src/Daqifi.Core/Device/SdCard/ISdCardOperations.cs 📝 Documentation +2/-1

Update ISdCardOperations docs to decimal bitmask

• Updated channelMask parameter documentation from binary to decimal format
• Changed example from "0000000011" to "3"
• Added clarification that firmware parses as decimal integer

src/Daqifi.Core/Device/SdCard/ISdCardOperations.cs

---

5. README.md 📝 Documentation +1/-1

Update README example to decimal format

• Updated EnableAdcChannels example from binary "0000000011" to decimal "3"
• Added inline comment explaining bitmask calculation (0b11 = 3)

README.md

---

6. docs/DEVICE_INTERFACES.md 📝 Documentation +4/-4

Update DEVICE_INTERFACES.md examples to decimal format

• Updated three code examples to use decimal bitmask format
• Changed "0000000011" to "3" for channels 0 and 1
• Changed "11111111" to "255" for all 8 channels
• Updated comments to clarify decimal bitmask interpretation
• Added inline binary-to-decimal conversion explanations

docs/DEVICE_INTERFACES.md

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX Issues (0)

1. Simulator docs still binary ☑ 🐞 Bug ⚙ Maintainability

Description

After updating core docs to specify a decimal bitmask for ENAble:VOLTage:DC, the simulator
documentation still describes the parameter as a binary string and shows base-2 parsing. This leaves
contradictory guidance in-repo and can lead to an incorrect simulator implementation that won’t
match firmware behavior.

Code

src/Daqifi.Core/Communication/Producers/ScpiMessageProducer.cs[R318-336]

  /// <summary>
-    /// Creates a command message to enable ADC channels using a binary string.
+    /// Creates a command message to enable ADC channels using a decimal bitmask.
  /// </summary>
-    /// <param name="channelSetString">A binary string where each character represents a channel (0 = disabled, 1 = enabled), right-to-left. For example, "0001010100" enables channels 2, 4, and 6.</param>
+    /// <param name="channelSetString">A decimal integer string representing a bitmask where each bit enables a channel. For example, "84" (0b1010100) enables channels 2, 4, and 6.</param>
  /// <remarks>
-    /// The binary string is read from right to left, where position 0 is the rightmost bit:
-    /// - Position 0: Channel 0
-    /// - Position 1: Channel 1
-    /// - Position 2: Channel 2
+    /// The firmware parses this value as a decimal integer and interprets it as a bitmask:
+    /// - Bit 0 (value 1): Channel 0
+    /// - Bit 1 (value 2): Channel 1
+    /// - Bit 2 (value 4): Channel 2
  /// etc.
-    /// 
-    /// Command: ENAble:VOLTage:DC binaryString
-    /// Example: 
+    ///
+    /// Command: ENAble:VOLTage:DC decimalMask
  /// <code>
-    /// // Enable channels 2, 4, and 6
-    /// messageProducer.Send(ScpiMessageProducer.EnableAdcChannels("0001010100"));
-    /// 
-    /// // Enable channels 0 and 1
-    /// messageProducer.Send(ScpiMessageProducer.EnableAdcChannels("0000000011"));
+    /// // Enable channels 2, 4, and 6 (bitmask = 4 + 16 + 64 = 84)
+    /// device.Send(ScpiMessageProducer.EnableAdcChannels("84"));
+    ///
+    /// // Enable channels 0 and 1 (bitmask = 1 + 2 = 3)
+    /// device.Send(ScpiMessageProducer.EnableAdcChannels("3"));
  /// </code>

Evidence

Core docs now explicitly state the firmware parses the value as a decimal integer bitmask, but
simulator docs still specify `, show examples like 0000000011`, and even show
Convert.ToUInt32(binaryString, 2) parsing—contradicting the updated contract and recreating the
original mismatch in simulator guidance.

src/Daqifi.Core/Communication/Producers/ScpiMessageProducer.cs[318-341]
docs/simulator/SIMULATOR_DESIGN.md[139-160]
docs/simulator/SIMULATOR_DESIGN.md[404-416]
docs/simulator/SIMULATOR_IMPLEMENTATION_PLAN.md[583-590]
docs/simulator/GITHUB_ISSUES.md[258-266]
docs/simulator/GITHUB_ISSUES.md[280-287]

Agent prompt

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

## Issue description
Simulator documentation still describes `ENAble:VOLTage:DC` as taking a binary string and shows base-2 parsing, which contradicts the updated core docs/tests that specify a decimal bitmask string.
### Issue Context
This PR corrects the public API documentation to match firmware behavior (decimal integer parsed as bitmask). Simulator docs should match the same contract to avoid future simulator/test implementations reintroducing the binary/decimal mismatch.
### Fix Focus Areas
- docs/simulator/SIMULATOR_DESIGN.md[139-160]
- docs/simulator/SIMULATOR_DESIGN.md[404-416]
- docs/simulator/SIMULATOR_IMPLEMENTATION_PLAN.md[583-590]
- docs/simulator/GITHUB_ISSUES.md[258-266]
- docs/simulator/GITHUB_ISSUES.md[280-287]
### What to change
- Replace `<binary_string>` with something like `<decimal_mask>` / `<decimal_bitmask>`.
- Update examples from `0000000011` to `3` (and similar).
- Update the simulator parsing example from `Convert.ToUInt32(binaryString, 2)` to decimal parsing (e.g., `Convert.ToUInt32(maskString)`), so it mirrors firmware behavior.
- Update the design doc’s example call `EnableAnalogChannels("0000000011")` to the correct API name and decimal value (e.g., `EnableAdcChannels("3")`), consistent with the rest of the repo.

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

---

ⓘ The new review experience is currently in Beta. Learn more