Skip to content

fix: send decimal channel mask for SD card logging instead of binary#446

Merged
tylerkron merged 3 commits intomainfrom
fix/sd-card-channel-mask-binary-to-decimal
Apr 4, 2026
Merged

fix: send decimal channel mask for SD card logging instead of binary#446
tylerkron merged 3 commits intomainfrom
fix/sd-card-channel-mask-binary-to-decimal

Conversation

@tylerkron
Copy link
Copy Markdown
Contributor

Summary

  • StartSdCardLogging() used Convert.ToString((long)analogChannelMask, 2) which converted the bitmask to a binary string (e.g. "111" for channels 0,1,2). The firmware's SCPI_ParamInt32 parses this as decimal 111 = 0b1101111, enabling channels 0,1,2,3,5,6 instead of the intended 0,1,2.
  • Changed to analogChannelMask.ToString() which sends the correct decimal value (e.g. "7" for channels 0,1,2). This matches AddChannel()/RemoveChannel() which already use decimal format.
  • Updated test to expect decimal "21" instead of binary "10101" for the channels 0,2,4 test case.

Fixes #439

Related core docs fix: daqifi/daqifi-core#160

Test plan

  • Desktop solution builds successfully
  • Manual test: start SD logging with multiple channels enabled, verify correct channels are recorded

🤖 Generated with Claude Code

StartSdCardLogging() used Convert.ToString(analogChannelMask, 2) which
produced a binary string (e.g. "111" for channels 0-2). The firmware's
SCPI_ParamInt32 parses this as decimal 111, enabling the wrong channels.

Changed to analogChannelMask.ToString() to send the correct decimal
bitmask (e.g. "7" for channels 0-2). This matches AddChannel/
RemoveChannel which already use decimal via Convert.ToString() without
a base parameter.

Fixes #439

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@tylerkron tylerkron requested a review from a team as a code owner March 29, 2026 15:55
@tylerkron
Copy link
Copy Markdown
Contributor Author

/agentic_review

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review bot commented Mar 29, 2026

Code Review by Qodo

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

Grey Divider


Action required

1. Locale-sensitive mask string🐞 Bug ☼ Reliability
Description
StartSdCardLogging serializes the channel bitmask using analogChannelMask.ToString() without an
explicit culture, which can produce non-ASCII native digits in some locales and lead to firmware
mis-parse/reject of the SCPI channelMask parameter.
Code

Daqifi.Desktop/Device/AbstractStreamingDevice.cs[476]

+            var channelMaskString = analogChannelMask.ToString();
Evidence
The PR changes StartSdCardLogging to use default-culture ToString() for the mask. Elsewhere in the
same class, numeric values that leave the process are explicitly serialized with
CultureInfo.InvariantCulture (device serial numbers and debug channel mask), indicating
external/protocol strings are expected to be culture-invariant.

Daqifi.Desktop/Device/AbstractStreamingDevice.cs[436-478]
Daqifi.Desktop/Device/AbstractStreamingDevice.cs[250-251]
Daqifi.Desktop/Device/AbstractStreamingDevice.cs[363-372]
Daqifi.Desktop/Device/AbstractStreamingDevice.cs[1123-1131]

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

### Issue description
`StartSdCardLogging()` builds a decimal channel mask string via `analogChannelMask.ToString()` which is culture-sensitive and may emit non-ASCII digits under certain `CurrentCulture` settings. This can break SCPI/device parsing.

### Issue Context
This code is producing a protocol parameter (`channelMask`) for firmware consumption, so formatting must be culture-invariant.

### Fix Focus Areas
- Daqifi.Desktop/Device/AbstractStreamingDevice.cs[436-478]

### Suggested change
Use `analogChannelMask.ToString(CultureInfo.InvariantCulture)` (System.Globalization is already used in this file) to ensure ASCII digits regardless of user locale.

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


Grey Divider

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

Grey Divider

Qodo Logo

tylerkron and others added 2 commits March 30, 2026 09:42
Ensures the decimal channel mask sent to firmware via SCPI is always
formatted with ASCII digits, regardless of the user's locale.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 4, 2026

📊 Code Coverage Report

Summary

Summary
Generated on: 4/4/2026 - 4:47:06 AM
Coverage date: 4/4/2026 - 4:46:35 AM - 4/4/2026 - 4:47:02 AM
Parser: MultiReport (4x Cobertura)
Assemblies: 3
Classes: 107
Files: 138
Line coverage: 19.1% (1132 of 5900)
Covered lines: 1132
Uncovered lines: 4768
Coverable lines: 5900
Total lines: 18119
Branch coverage: 17.2% (372 of 2151)
Covered branches: 372
Total branches: 2151
Method coverage: Feature is only available for sponsors

Coverage

DAQiFi - 18.9%
Name Line Branch
DAQiFi 18.9% 17.2%
Daqifi.Desktop.App 6.3% 0%
Daqifi.Desktop.Channel.AbstractChannel 40.9% 27.7%
Daqifi.Desktop.Channel.AnalogChannel 58.7% 25%
Daqifi.Desktop.Channel.Channel 11.5% 0%
Daqifi.Desktop.Channel.ChannelColorManager 100% 100%
Daqifi.Desktop.Channel.DataSample 91.6%
Daqifi.Desktop.Channel.DigitalChannel 65.2% 25%
Daqifi.Desktop.Commands.CompositeCommand 0% 0%
Daqifi.Desktop.Commands.HostCommands 0%
Daqifi.Desktop.Commands.WeakEventHandlerManager 0% 0%
Daqifi.Desktop.Configuration.FirewallConfiguration 90.6% 66.6%
Daqifi.Desktop.Configuration.WindowsFirewallWrapper 64% 68.4%
Daqifi.Desktop.ConnectionManager 43% 43.1%
Daqifi.Desktop.Converters.BoolToActiveStatusConverter 0% 0%
Daqifi.Desktop.Converters.BoolToConnectionStatusConverter 0% 0%
Daqifi.Desktop.Converters.BoolToStatusColorConverter 0% 0%
Daqifi.Desktop.Converters.ConnectionTypeToColorConverter 0% 0%
Daqifi.Desktop.Converters.ConnectionTypeToUsbConverter 0% 0%
Daqifi.Desktop.Converters.InvertedBoolToVisibilityConverter 0% 0%
Daqifi.Desktop.Converters.ListToStringConverter 0% 0%
Daqifi.Desktop.Converters.NotNullToVisibilityConverter 0% 0%
Daqifi.Desktop.Converters.OxyColorToBrushConverter 0% 0%
Daqifi.Desktop.Converters.StringRightConverter 0% 0%
Daqifi.Desktop.Device.AbstractStreamingDevice 42.8% 38.6%
Daqifi.Desktop.Device.DeviceMessage 0%
Daqifi.Desktop.Device.Firmware.BootloaderSessionStreamingDeviceAdapter 0% 0%
Daqifi.Desktop.Device.Firmware.WifiPromptDelayProcessRunner 0% 0%
Daqifi.Desktop.Device.NativeMethods 100%
Daqifi.Desktop.Device.SerialDevice.SerialStreamingDevice 27.6% 30.8%
Daqifi.Desktop.Device.WiFiDevice.DaqifiStreamingDevice 40.9% 39.4%
Daqifi.Desktop.DialogService.DialogService 0% 0%
Daqifi.Desktop.DialogService.ServiceLocator 0% 0%
Daqifi.Desktop.DuplicateDeviceCheckResult 100%
Daqifi.Desktop.Exporter.OptimizedLoggingSessionExporter 30.2% 35.4%
Daqifi.Desktop.Exporter.SampleData 0%
Daqifi.Desktop.Helpers.BooleanConverter`1 0% 0%
Daqifi.Desktop.Helpers.BooleanToInverseBoolConverter 0% 0%
Daqifi.Desktop.Helpers.BooleanToVisibilityConverter 0%
Daqifi.Desktop.Helpers.EnumDescriptionConverter 100% 100%
Daqifi.Desktop.Helpers.IntToVisibilityConverter 0% 0%
Daqifi.Desktop.Helpers.MyMultiValueConverter 0%
Daqifi.Desktop.Helpers.NaturalSortHelper 100% 100%
Daqifi.Desktop.Helpers.VersionHelper 98.2% 66.2%
Daqifi.Desktop.Logger.DatabaseLogger 0% 0%
Daqifi.Desktop.Logger.LoggedSeriesLegendItem 0% 0%
Daqifi.Desktop.Logger.LoggingContext 0%
Daqifi.Desktop.Logger.LoggingManager 0% 0%
Daqifi.Desktop.Logger.LoggingSession 26.6% 0%
Daqifi.Desktop.Logger.PlotLogger 0% 0%
Daqifi.Desktop.Logger.SummaryLogger 0% 0%
Daqifi.Desktop.Logger.TimestampGapDetector 95% 83.3%
Daqifi.Desktop.Loggers.ImportOptions 0%
Daqifi.Desktop.Loggers.ImportProgress 0% 0%
Daqifi.Desktop.Loggers.SdCardSessionImporter 0% 0%
Daqifi.Desktop.MainWindow 0% 0%
Daqifi.Desktop.Migrations.InitialSQLiteMigration 0%
Daqifi.Desktop.Migrations.LoggingContextModelSnapshot 0%
Daqifi.Desktop.Models.AddProfileModel 0%
Daqifi.Desktop.Models.DaqifiSettings 80.5% 83.3%
Daqifi.Desktop.Models.DebugDataCollection 6.6% 0%
Daqifi.Desktop.Models.DebugDataModel 0% 0%
Daqifi.Desktop.Models.Notifications 0%
Daqifi.Desktop.Models.SdCardFile 0% 0%
Daqifi.Desktop.Services.WindowsPrincipalAdminChecker 0%
Daqifi.Desktop.Services.WpfMessageBoxService 0%
Daqifi.Desktop.UpdateVersion.VersionNotification 0% 0%
Daqifi.Desktop.View.AddChannelDialog 0% 0%
Daqifi.Desktop.View.AddProfileConfirmationDialog 0% 0%
Daqifi.Desktop.View.AddprofileDialog 0% 0%
Daqifi.Desktop.View.ConnectionDialog 0% 0%
Daqifi.Desktop.View.DebugWindow 0% 0%
Daqifi.Desktop.View.DeviceLogsView 0% 0%
Daqifi.Desktop.View.DuplicateDeviceDialog 0% 0%
Daqifi.Desktop.View.ErrorDialog 0% 0%
Daqifi.Desktop.View.ExportDialog 0% 0%
Daqifi.Desktop.View.FirmwareDialog 0% 0%
Daqifi.Desktop.View.Flyouts.ChannelsFlyout 0% 0%
Daqifi.Desktop.View.Flyouts.DevicesFlyout 0% 0%
Daqifi.Desktop.View.Flyouts.FirmwareFlyout 0% 0%
Daqifi.Desktop.View.Flyouts.LiveGraphFlyout 0% 0%
Daqifi.Desktop.View.Flyouts.LoggedSessionFlyout 0% 0%
Daqifi.Desktop.View.Flyouts.NotificationsFlyout 0% 0%
Daqifi.Desktop.View.Flyouts.SummaryFlyout 0% 0%
Daqifi.Desktop.View.Flyouts.UpdateProfileFlyout 0% 0%
Daqifi.Desktop.View.SelectColorDialog 0% 0%
Daqifi.Desktop.View.SettingsDialog 0% 0%
Daqifi.Desktop.View.SuccessDialog 0% 0%
Daqifi.Desktop.ViewModels.AddChannelDialogViewModel 0% 0%
Daqifi.Desktop.ViewModels.AddProfileConfirmationDialogViewModel 0% 0%
Daqifi.Desktop.ViewModels.AddProfileDialogViewModel 0% 0%
Daqifi.Desktop.ViewModels.ConnectionDialogViewModel 21.6% 19.5%
Daqifi.Desktop.ViewModels.DaqifiViewModel 15.2% 9.1%
Daqifi.Desktop.ViewModels.DeviceLogsViewModel 0% 0%
Daqifi.Desktop.ViewModels.DeviceSettingsViewModel 0% 0%
Daqifi.Desktop.ViewModels.DuplicateDeviceDialogViewModel 0%
Daqifi.Desktop.ViewModels.ErrorDialogViewModel 0%
Daqifi.Desktop.ViewModels.ExportDialogViewModel 0% 0%
Daqifi.Desktop.ViewModels.FirmwareDialogViewModel 0% 0%
Daqifi.Desktop.ViewModels.SelectColorDialogViewModel 0% 0%
Daqifi.Desktop.ViewModels.SettingsViewModel 0%
Daqifi.Desktop.ViewModels.SuccessDialogViewModel 85.7%
Daqifi.Desktop.WindowViewModelMapping.IWindowViewModelMappingsContract 0%
Daqifi.Desktop.WindowViewModelMapping.WindowViewModelMappings 0%
Sentry.Generated.BuildPropertyInitializer 100%
Daqifi.Desktop.Common - 33.3%
Name Line Branch
Daqifi.Desktop.Common 33.3% 22.7%
Daqifi.Desktop.Common.Loggers.AppLogger 31.3% 22.7%
Daqifi.Desktop.Common.Loggers.NoOpLogger 60%
Daqifi.Desktop.IO - 100%
Name Line Branch
Daqifi.Desktop.IO 100% ****
Daqifi.Desktop.IO.Messages.MessageEventArgs`1 100%

Coverage report generated by ReportGeneratorView full report in build artifacts

@tylerkron tylerkron merged commit 722fd73 into main Apr 4, 2026
2 checks passed
@tylerkron tylerkron deleted the fix/sd-card-channel-mask-binary-to-decimal branch April 4, 2026 04:48
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.

fix: SD card logging sends binary string to EnableAdcChannels instead of decimal

1 participant