Skip to content

feat: unified Devices pane matching design philosophy#484

Merged
tylerkron merged 4 commits intomainfrom
claude/competent-ride-d1cb54
Apr 18, 2026
Merged

feat: unified Devices pane matching design philosophy#484
tylerkron merged 4 commits intomainfrom
claude/competent-ride-d1cb54

Conversation

@tylerkron
Copy link
Copy Markdown
Contributor

Summary

  • Redesigns the Devices tab as a tile grid with an inline settings drawer, replacing the ListView + MahApps DevicesFlyout. Mirrors the Channels pane pattern from feat: unified Channels pane + design philosophy #479: dark surfaces, USB (cyan) / WiFi (purple) type-coded stripes, and a single direct-manipulation interaction model per the design philosophy.
  • New DevicesPaneViewModel + DeviceTileViewModel own pane state and tile presentation; shell-owned state (firmware path/progress, logging mode, SD format) and commands (firmware upload, network apply, disconnect/reboot) are reached through a Shell passthrough so the drawer binds directly without cross-DataContext hops.
  • Drawer sections: Information, Data Acquisition (segmented Stream/Log toggle + frequency slider + SD format), Network (MODE/SECURITY/SSID/PASSWORD + Apply), Firmware (USB file picker + progress + Update; WiFi shows an amber "requires USB" note), Actions (Reboot + danger Disconnect).
  • Removes the dead IsDeviceSettingsOpen flag and OpenDeviceSettings command from DaqifiViewModel along with the deleted DevicesFlyout.xaml/.cs.
  • Includes a full dark ControlTemplate for the drawer ComboBoxes — the MahApps Light.Blue theme was painting the default toggle button white regardless of the Background setter, which left MODE/SECURITY/DATA FORMAT rendering light against the dark drawer.

Test plan

  • Devices tab shows tiles for each connected device; USB tiles have a cyan stripe/chip, WiFi tiles have a purple stripe/chip
  • Clicking a tile (body or gear) opens the right-side drawer with the correct device's info; clicking the scrim or × closes it
  • INFORMATION panel shows the right CONNECTION / IDENTIFIER / SERIAL / FIRMWARE values
  • LOGGING MODE toggle drives DaqifiViewModel.SelectedLoggingMode; LOG TO DEVICE is disabled for WiFi devices with a tooltip
  • FREQUENCY slider + textbox edit SelectedDevice.StreamingFrequency (500 ms delay on commit)
  • DATA FORMAT combo is visible only in LOG TO DEVICE mode and drives SelectedSdCardLogFormat
  • NETWORK MODE / SECURITY / SSID / PASSWORD bind round-trip to SelectedDevice.NetworkConfiguration; APPLY calls UpdateNetworkConfigurationCommand
  • USB device: firmware BROWSE populates FirmwareFilePath; UPDATE runs the upload and the progress bar updates
  • WiFi device: firmware section shows the amber "Firmware updates require a USB connection." note and no file picker
  • REBOOT and DISCONNECT pills invoke the shell commands; disconnect closes the drawer
  • "+ ADD DEVICE" (status bar and empty state) opens the connection dialog
  • Empty state appears when the last device disconnects; connecting a device brings back the tile grid
  • Tab away from Devices and back — the VM is rebuilt and the tile grid repopulates (matches ChannelsPane Loaded/Unloaded behavior)
  • All drawer ComboBoxes render dark (no white backgrounds) and the popup rows highlight on hover

🤖 Generated with Claude Code

tylerkron and others added 2 commits April 18, 2026 13:01
Redesigns the Devices tab as a tile grid with an inline settings drawer,
replacing the ListView + MahApps DevicesFlyout. Follows the Channels pane
pattern from #479: dark surfaces, USB (cyan) / WiFi (purple) type-coded
stripes, direct tile click opens a right-side drawer with Information /
Data Acquisition / Network / Firmware / Actions sections. Shell-owned
state (firmware path/progress, logging mode, SD format) is exposed via a
Shell passthrough on DevicesPaneViewModel so the drawer binds directly
without cross-VM hops. Removes the now-dead IsDeviceSettingsOpen flag and
OpenDeviceSettings command from DaqifiViewModel.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The MahApps Light.Blue theme paints the default ComboBox toggle button
white regardless of the Background setter, so the NETWORK MODE / SECURITY
(and SD format) combos were rendering light against the dark drawer.
Replaces the ControlTemplate with a full dark version sourced from the
drawer palette, matches the chevron to TextSecondary, routes popup items
through a dark ComboBoxItem style with SurfaceActive hover, and adds
Accent focus + BorderBright hover borders for consistency with the other
drawer inputs.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@tylerkron tylerkron requested a review from a team as a code owner April 18, 2026 19:11
@qodo-code-review
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Unified Devices pane with tile grid and inline settings drawer

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Redesigns Devices tab from ListView to unified tile grid with inline settings drawer
• Mirrors Channels pane design: dark surfaces, USB (cyan) / WiFi (purple) type-coded stripes
• New DevicesPaneViewModel + DeviceTileViewModel own pane and tile state; shell passthrough for
  shared commands
• Drawer sections: Information, Data Acquisition, Network, Firmware, Actions with full dark ComboBox
  template
• Removes dead IsDeviceSettingsOpen flag and OpenDeviceSettings command from DaqifiViewModel
Diagram
flowchart LR
  A["Old ListView + DevicesFlyout"] -->|Replace| B["DevicesPanePrototype"]
  B --> C["DevicesPaneViewModel"]
  C --> D["DeviceTileViewModel Collection"]
  C --> E["Shell Passthrough"]
  E --> F["Firmware/Network/Logging Commands"]
  B --> G["Settings Drawer"]
  G --> H["Information Panel"]
  G --> I["Data Acquisition Panel"]
  G --> J["Network Panel"]
  G --> K["Firmware Panel"]
  G --> L["Actions Panel"]
Loading

Grey Divider

File Changes

1. Daqifi.Desktop/View/Flyouts/DevicesFlyout.xaml.cs Miscellaneous +0/-12

Remove obsolete DevicesFlyout code-behind

• Deleted entire file (12 lines removed)
• Code-behind for old DevicesFlyout is no longer needed

Daqifi.Desktop/View/Flyouts/DevicesFlyout.xaml.cs


2. Daqifi.Desktop/View/Prototype/DevicesPanePrototype.xaml.cs ✨ Enhancement +50/-0

New DevicesPanePrototype code-behind with lifecycle management

• New 50-line code-behind for DevicesPanePrototype UserControl
• Implements Loaded/Unloaded lifecycle to recreate DevicesPaneViewModel when tab switches
• Handles LoggingMode_Click to write segmented toggle selection back to shell
• Disposes ViewModel on unload to clean up ConnectionManager subscription

Daqifi.Desktop/View/Prototype/DevicesPanePrototype.xaml.cs


3. Daqifi.Desktop/ViewModels/DaqifiViewModel.cs 🐞 Bug fix +0/-18

Remove device settings flyout state and command

• Removed _isDeviceSettingsOpen ObservableProperty (no longer needed)
• Deleted OpenDeviceSettings RelayCommand (drawer now owned by DevicesPaneViewModel)
• Removed IsDeviceSettingsOpen = false from CloseFlyouts method

Daqifi.Desktop/ViewModels/DaqifiViewModel.cs


View more (5)
4. Daqifi.Desktop/ViewModels/DeviceTileViewModel.cs ✨ Enhancement +116/-0

New DeviceTileViewModel for tile presentation logic

• New 116-line ViewModel for individual device tiles
• Computes presentation (stripe color, labels, tile colors) from device state
• Subscribes to device PropertyChanged to update tile when device reconnects or firmware updates
• Exposes IsUsb, IsFirmwareOutdated, IsConnected, StripeBrush, TileBackground, TileBorderBrush
 properties
• Implements IDisposable to detach device subscription

Daqifi.Desktop/ViewModels/DeviceTileViewModel.cs


5. Daqifi.Desktop/ViewModels/DevicesPaneViewModel.cs ✨ Enhancement +192/-0

New DevicesPaneViewModel managing tile grid and drawer state

• New 192-line ViewModel backing the unified Devices pane
• Owns device tile collection, drawer open/close state, and selected tile
• Subscribes to ConnectionManager.ConnectedDevices to rebuild tile grid on device connect/disconnect
• Delegates side-effecting commands (connect, disconnect, reboot, firmware, network) to shell
 ViewModel
• Preserves drawer state across rebuild if device still connected; closes drawer if device removed
• Implements IDisposable to clean up subscriptions and dispose tiles

Daqifi.Desktop/ViewModels/DevicesPaneViewModel.cs


6. Daqifi.Desktop/MainWindow.xaml ✨ Enhancement +1/-100

Replace ListView Devices tab with DevicesPanePrototype

• Removed DevicesFlyout from FlyoutsControl (3 lines deleted)
• Replaced old ListView-based Devices tab template with single-line DevicesPanePrototype reference
• Removed 96 lines of ListView item template, empty state trigger, and add-device button

Daqifi.Desktop/MainWindow.xaml


7. Daqifi.Desktop/View/Flyouts/DevicesFlyout.xaml Miscellaneous +0/-534

Remove obsolete DevicesFlyout XAML

• Deleted entire 534-line flyout XAML file
• Contained old tabbed interface (Data Acquisition, Network, Firmware) with MahApps styles
• Replaced by new inline drawer in DevicesPanePrototype with unified dark design

Daqifi.Desktop/View/Flyouts/DevicesFlyout.xaml


8. Daqifi.Desktop/View/Prototype/DevicesPanePrototype.xaml ✨ Enhancement +863/-0

New DevicesPanePrototype with tile grid and settings drawer

• New 863-line UserControl implementing unified Devices pane
• Tile grid layout with device tiles showing name, connection type chip, identifier, serial,
 firmware version, and settings gear
• Right-side settings drawer (420px wide) with scrim overlay containing Information, Data
 Acquisition, Network, Firmware, Actions sections
• Full dark ComboBox template with SurfaceActive hover, Accent focus border, and TextSecondary
 chevron
• Segmented toggle for Logging Mode (Stream to App / Log to Device) with USB-only constraint
• Frequency slider with textbox, Data Format combo (visible only in Log to Device mode)
• Network MODE/SECURITY/SSID/PASSWORD fields with Apply button
• Firmware section: USB shows file picker + progress + Update; WiFi shows amber "requires USB" note
• Actions section with Reboot and Disconnect pills
• Empty state with centered icon and "+ ADD DEVICE" button when no devices connected
• Status bar showing device count and "+ ADD DEVICE" button in connected state

Daqifi.Desktop/View/Prototype/DevicesPanePrototype.xaml


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented Apr 18, 2026

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (2) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Logging mode uses code-behind📘 Rule violation ⌂ Architecture
Description
The new logging-mode UI is wired to a Click handler in code-behind, pushing interaction logic into
the View instead of the ViewModel. This breaks MVVM separation and makes the behavior harder to test
and reuse.
Code

Daqifi.Desktop/View/Prototype/DevicesPanePrototype.xaml[R650-666]

+                                    <RadioButton Grid.Column="0"
+                                                 Content="STREAM TO APP"
+                                                 GroupName="LoggingMode"
+                                                 IsChecked="{Binding Shell.IsLogToDeviceMode,
+                                                             Converter={StaticResource BooleanToInverse}, Mode=OneWay}"
+                                                 Style="{StaticResource SegmentedToggle}"
+                                                 Tag="Stream to App"
+                                                 Click="LoggingMode_Click"/>
+                                    <RadioButton Grid.Column="1"
+                                                 Content="LOG TO DEVICE"
+                                                 GroupName="LoggingMode"
+                                                 IsEnabled="{Binding SelectedTile.IsUsb}"
+                                                 IsChecked="{Binding Shell.IsLogToDeviceMode, Mode=OneWay}"
+                                                 Style="{StaticResource SegmentedToggle}"
+                                                 Tag="Log to Device"
+                                                 Click="LoggingMode_Click"
+                                                 ToolTip="SD card logging requires USB. WiFi devices do not support SD card logging."/>
Evidence
The MVVM rule requires interactive UI elements to bind to ViewModel properties/commands rather than
using code-behind handlers; the RadioButtons are connected via Click="LoggingMode_Click", and the
handler directly mutates shell.SelectedLoggingMode from the View.

Rule 244827: UI changes must update both View and ViewModel in MVVM
Daqifi.Desktop/View/Prototype/DevicesPanePrototype.xaml[650-666]
Daqifi.Desktop/View/Prototype/DevicesPanePrototype.xaml.cs[39-48]

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 logging-mode toggle uses a View code-behind `Click` handler, which violates MVVM requirements.

## Issue Context
The View should bind to a ViewModel command/property for logging mode changes (e.g., `IRelayCommand<string>` with `CommandParameter`), rather than handling clicks in `DevicesPanePrototype.xaml.cs`.

## Fix Focus Areas
- Daqifi.Desktop/View/Prototype/DevicesPanePrototype.xaml[650-666]
- Daqifi.Desktop/View/Prototype/DevicesPanePrototype.xaml.cs[39-48]
- Daqifi.Desktop/ViewModels/DevicesPaneViewModel.cs[44-80]

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


2. Frequency guard bypass🐞 Bug ≡ Correctness
Description
The frequency slider writes directly to SelectedDevice.StreamingFrequency, bypassing the shell’s
SelectedStreamingFrequency setter that blocks changes while logging and shows an error dialog. This
allows the Devices pane to change sampling frequency during an active logging session even though
the rest of the app explicitly disallows it.
Code

Daqifi.Desktop/View/Prototype/DevicesPanePrototype.xaml[R670-693]

+                            <TextBlock Text="FREQUENCY (Hz)" Style="{StaticResource InfoLabel}" Margin="0,16,0,6"/>
+                            <Grid>
+                                <Grid.ColumnDefinitions>
+                                    <ColumnDefinition Width="Auto"/>
+                                    <ColumnDefinition Width="*"/>
+                                </Grid.ColumnDefinitions>
+                                <Border Grid.Column="0"
+                                        Style="{StaticResource DrawerTextField}"
+                                        Width="80"
+                                        Margin="0,0,10,0">
+                                    <TextBox Text="{Binding ElementName=FrequencySlider, Path=Value,
+                                                     UpdateSourceTrigger=PropertyChanged, Mode=TwoWay}"
+                                             Style="{StaticResource DrawerTextBox}"
+                                             TextAlignment="Center"
+                                             FontSize="16"/>
+                                </Border>
+                                <Slider Grid.Column="1"
+                                        x:Name="FrequencySlider"
+                                        Minimum="1" Maximum="1000"
+                                        TickFrequency="1"
+                                        IsSnapToTickEnabled="True"
+                                        VerticalAlignment="Center"
+                                        Value="{Binding SelectedDevice.StreamingFrequency,
+                                                Delay=500, UpdateSourceTrigger=PropertyChanged}"/>
Evidence
The Devices pane binds the slider to the device property directly, while DaqifiViewModel exposes a
guarded SelectedStreamingFrequency property that refuses changes when LoggingManager is active; the
device-level StreamingFrequency property itself has no such guard.

Daqifi.Desktop/View/Prototype/DevicesPanePrototype.xaml[670-693]
Daqifi.Desktop/ViewModels/DaqifiViewModel.cs[256-273]
Daqifi.Desktop/Device/AbstractStreamingDevice.cs[36-43]

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 Devices pane frequency control binds to `SelectedDevice.StreamingFrequency`, which bypasses `DaqifiViewModel.SelectedStreamingFrequency` validation (including blocking changes while logging).

### Issue Context
`DaqifiViewModel.SelectedStreamingFrequency` enforces application rules (e.g., no frequency changes while `LoggingManager.Instance.Active`). The new drawer should route changes through the same guarded property.

### Fix Focus Areas
- Daqifi.Desktop/View/Prototype/DevicesPanePrototype.xaml[670-693]

### Suggested change
- Change the slider (and textbox) binding to use `Shell.SelectedStreamingFrequency` (TwoWay) instead of `SelectedDevice.StreamingFrequency`.
- Keep `DevicesPaneViewModel.OpenSettings` syncing `Shell.SelectedDevice` so the guarded setter continues to apply to the selected device.

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


3. Network apply can throw🐞 Bug ☼ Reliability
Description
The Apply Network Settings button calls Shell.UpdateNetworkConfigurationCommand, but the command
implementation dereferences SelectedDevice without checking for null or connection state. If the
device disconnects while the drawer is open (or SelectedDevice is cleared by other paths), executing
Apply can throw or fail unexpectedly.
Code

Daqifi.Desktop/View/Prototype/DevicesPanePrototype.xaml[R742-746]

+                            <Button Style="{StaticResource AccentPillButton}"
+                                    Content="APPLY NETWORK SETTINGS"
+                                    HorizontalAlignment="Right"
+                                    Margin="0,14,0,0"
+                                    Command="{Binding Shell.UpdateNetworkConfigurationCommand}"/>
Evidence
The Apply button is wired to the shell relay command. That command directly calls
SelectedDevice.UpdateNetworkConfiguration() with no guard. Unit tests verify
UpdateNetworkConfiguration() throws when the device is disconnected, so the command should
defensively prevent execution and/or handle errors.

Daqifi.Desktop/View/Prototype/DevicesPanePrototype.xaml[742-746]
Daqifi.Desktop/ViewModels/DaqifiViewModel.cs[1077-1082]
Daqifi.Desktop.Test/Device/AbstractStreamingDeviceTests.cs[217-231]

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

### Issue description
`UpdateNetworkConfigurationCommand` can execute when `SelectedDevice` is null or disconnected, leading to a null-dereference or an exception from `UpdateNetworkConfiguration()`.

### Issue Context
The Devices drawer binds `APPLY NETWORK SETTINGS` to `Shell.UpdateNetworkConfigurationCommand`. The command method currently does not validate `SelectedDevice`.

### Fix Focus Areas
- Daqifi.Desktop/ViewModels/DaqifiViewModel.cs[1077-1082]
- Daqifi.Desktop/View/Prototype/DevicesPanePrototype.xaml[742-746]

### Suggested change
- Update `DaqifiViewModel.UpdateNetworkConfiguration` to:
 - return early (and/or show an error dialog) if `SelectedDevice == null`.
 - return early (and/or show an error dialog) if `!SelectedDevice.IsConnected`.
 - optionally wrap the await in try/catch to surface failures instead of crashing.
- Preferably add a `CanExecute` predicate to the `[RelayCommand]` so the Apply button disables automatically when no connected device is selected.

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



Remediation recommended

4. DevicesPaneViewModel lacks regions 📘 Rule violation ✧ Quality
Description
The new DevicesPaneViewModel spans ~190 lines and contains many members but does not use #region
directives for logical grouping. This violates the required organization convention for non-trivial
C# types.
Code

Daqifi.Desktop/ViewModels/DevicesPaneViewModel.cs[R16-72]

+public partial class DevicesPaneViewModel : ObservableObject, IDisposable
+{
+    private readonly DaqifiViewModel? _shell;
+    private readonly Dispatcher _dispatcher;
+    private bool _disposed;
+
+    /// <summary>Device tiles in the order ConnectionManager returns them.</summary>
+    public ObservableCollection<DeviceTileViewModel> Devices { get; } = [];
+
+    /// <summary>
+    /// The shell view-model, exposed so the drawer can bind directly to shell-owned
+    /// state (firmware path/progress, logging mode, SD format) and commands
+    /// (firmware upload, network apply) without a second DataContext hop.
+    /// </summary>
+    public DaqifiViewModel? Shell => _shell;
+
+    [ObservableProperty] private bool _hasConnectedDevice;
+    [ObservableProperty] private DeviceTileViewModel? _selectedTile;
+    [ObservableProperty] private bool _isSettingsOpen;
+
+    /// <summary>The underlying device of the tile shown in the settings drawer.</summary>
+    public IStreamingDevice? SelectedDevice => SelectedTile?.Device;
+
+    /// <summary>True when the selected device is USB-connected (firmware update path).</summary>
+    public bool SelectedDeviceSupportsFirmwareUpdate =>
+        SelectedDevice?.ConnectionType == ConnectionType.Usb;
+
+    /// <summary>Opens the inline settings drawer for a device tile.</summary>
+    public IRelayCommand<DeviceTileViewModel> OpenSettingsCommand { get; }
+
+    /// <summary>Closes the inline settings drawer.</summary>
+    public IRelayCommand CloseSettingsCommand { get; }
+
+    /// <summary>Shows the Add-Device dialog via the shell view-model.</summary>
+    public IRelayCommand AddDeviceCommand { get; }
+
+    /// <summary>Disconnects the currently selected device and closes the drawer.</summary>
+    public IRelayCommand DisconnectSelectedCommand { get; }
+
+    /// <summary>Reboots the currently selected device.</summary>
+    public IRelayCommand RebootSelectedCommand { get; }
+
+    /// <summary>Creates the view-model bound to the shell view-model.</summary>
+    public DevicesPaneViewModel(DaqifiViewModel? shell)
+    {
+        _shell = shell;
+        _dispatcher = Dispatcher.CurrentDispatcher;
+
+        OpenSettingsCommand = new RelayCommand<DeviceTileViewModel>(OpenSettings);
+        CloseSettingsCommand = new RelayCommand(CloseSettings);
+        AddDeviceCommand = new RelayCommand(AddDevice);
+        DisconnectSelectedCommand = new RelayCommand(DisconnectSelected, () => SelectedDevice != null);
+        RebootSelectedCommand = new RelayCommand(RebootSelected, () => SelectedDevice != null);
+
+        ConnectionManager.Instance.PropertyChanged += OnConnectionManagerPropertyChanged;
+        Rebuild();
+    }
Evidence
The checklist requires large/non-trivial C# classes to group members using #region/#endregion; the
added class defines many fields/properties/commands without any regions.

Rule 244816: Use #region directives for logical grouping of members
Daqifi.Desktop/ViewModels/DevicesPaneViewModel.cs[16-72]

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

## Issue description
`DevicesPaneViewModel` is a non-trivial class (many members and ~190 lines) but does not use `#region` directives to group members.

## Issue Context
Add regions such as `Fields`, `Properties`, `Commands`, `Event handlers`, `Public methods`, `Private methods`, and `IDisposable` to match the required convention.

## Fix Focus Areas
- Daqifi.Desktop/ViewModels/DevicesPaneViewModel.cs[16-192]

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


5. _selectedTile missing notify-for📘 Rule violation ≡ Correctness
Description
The [ObservableProperty] field _selectedTile drives dependent UI-bound properties
(SelectedDevice, SelectedDeviceSupportsFirmwareUpdate) but is not annotated with
[NotifyPropertyChangedFor]. This violates the required pattern and can lead to missed updates if
the manual partial method logic is refactored or incomplete.
Code

Daqifi.Desktop/ViewModels/DevicesPaneViewModel.cs[R32-42]

+    [ObservableProperty] private bool _hasConnectedDevice;
+    [ObservableProperty] private DeviceTileViewModel? _selectedTile;
+    [ObservableProperty] private bool _isSettingsOpen;
+
+    /// <summary>The underlying device of the tile shown in the settings drawer.</summary>
+    public IStreamingDevice? SelectedDevice => SelectedTile?.Device;
+
+    /// <summary>True when the selected device is USB-connected (firmware update path).</summary>
+    public bool SelectedDeviceSupportsFirmwareUpdate =>
+        SelectedDevice?.ConnectionType == ConnectionType.Usb;
+
Evidence
The rule requires dependent properties of an [ObservableProperty]-generated property to be listed
in [NotifyPropertyChangedFor] on the backing field; _selectedTile has no such attributes even
though SelectedDevice and SelectedDeviceSupportsFirmwareUpdate derive from SelectedTile.

Rule 244798: Annotate dependent UI properties with [NotifyPropertyChangedFor] when using [ObservableProperty]
Daqifi.Desktop/ViewModels/DevicesPaneViewModel.cs[32-42]
Daqifi.Desktop/ViewModels/DevicesPaneViewModel.cs[36-42]

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

## Issue description
`DevicesPaneViewModel` uses `[ObservableProperty]` for `_selectedTile`, but dependent properties (`SelectedDevice`, `SelectedDeviceSupportsFirmwareUpdate`) are not declared via `[NotifyPropertyChangedFor]` on the backing field.

## Issue Context
Even though `OnSelectedTileChanged(...)` currently raises `OnPropertyChanged` manually, the compliance rule requires `[NotifyPropertyChangedFor]` to explicitly declare the dependency.

## Fix Focus Areas
- Daqifi.Desktop/ViewModels/DevicesPaneViewModel.cs[32-80]

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


6. DevicesPaneViewModel uses singleton 📘 Rule violation ⌂ Architecture
Description
The new DevicesPaneViewModel directly depends on ConnectionManager.Instance and captures
Dispatcher.CurrentDispatcher instead of receiving these dependencies via injection. This violates
the dependency-injection requirement and makes the ViewModel harder to test and reuse.
Code

Daqifi.Desktop/ViewModels/DevicesPaneViewModel.cs[R59-72]

+    public DevicesPaneViewModel(DaqifiViewModel? shell)
+    {
+        _shell = shell;
+        _dispatcher = Dispatcher.CurrentDispatcher;
+
+        OpenSettingsCommand = new RelayCommand<DeviceTileViewModel>(OpenSettings);
+        CloseSettingsCommand = new RelayCommand(CloseSettings);
+        AddDeviceCommand = new RelayCommand(AddDevice);
+        DisconnectSelectedCommand = new RelayCommand(DisconnectSelected, () => SelectedDevice != null);
+        RebootSelectedCommand = new RelayCommand(RebootSelected, () => SelectedDevice != null);
+
+        ConnectionManager.Instance.PropertyChanged += OnConnectionManagerPropertyChanged;
+        Rebuild();
+    }
Evidence
The checklist disallows instantiating/obtaining service dependencies inside classes; this ViewModel
reaches for the ConnectionManager singleton and the current Dispatcher in its constructor.

Rule 244829: Inject dependencies instead of instantiating them inside classes
Daqifi.Desktop/ViewModels/DevicesPaneViewModel.cs[59-72]
Daqifi.Desktop/ViewModels/DevicesPaneViewModel.cs[70-71]

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

## Issue description
`DevicesPaneViewModel` pulls dependencies (`ConnectionManager.Instance`, `Dispatcher.CurrentDispatcher`) internally rather than accepting them as injected dependencies.

## Issue Context
For testability and compliance, accept abstractions (e.g., `IConnectionManager`, `Dispatcher`/`IDispatcher`) via constructor parameters (or DI container wiring), and use those fields instead of static/singleton access.

## Fix Focus Areas
- Daqifi.Desktop/ViewModels/DevicesPaneViewModel.cs[59-72]
- Daqifi.Desktop/ViewModels/DevicesPaneViewModel.cs[70-126]

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


View more (2)
7. LoggingMode_Click has underscore📘 Rule violation ✧ Quality
Description
The new method LoggingMode_Click includes an underscore, which violates the required PascalCase
method naming convention. This reduces consistency with the enforced style rules.
Code

Daqifi.Desktop/View/Prototype/DevicesPanePrototype.xaml.cs[R39-49]

+    private void LoggingMode_Click(object sender, RoutedEventArgs e)
+    {
+        // The logging-mode setter on DaqifiViewModel takes the string label and
+        // drives device SwitchMode + manager state; we surface it as a
+        // segmented toggle and write the label back on click.
+        if (sender is RadioButton { Tag: string mode } &&
+            Window.GetWindow(this)?.DataContext is DaqifiViewModel shell)
+        {
+            shell.SelectedLoggingMode = mode;
+        }
+    }
Evidence
The checklist requires PascalCase method names with no underscores; the added handler is named
LoggingMode_Click.

Rule 244817: Enforce PascalCase for method names
Daqifi.Desktop/View/Prototype/DevicesPanePrototype.xaml.cs[39-49]

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

## Issue description
A new method name includes an underscore (`LoggingMode_Click`), violating the PascalCase-only method naming rule.

## Issue Context
If the handler remains, rename it to something like `OnLoggingModeClick` and update the XAML reference. (Preferred overall: remove code-behind handler entirely and bind to a ViewModel command.)

## Fix Focus Areas
- Daqifi.Desktop/View/Prototype/DevicesPanePrototype.xaml.cs[39-49]
- Daqifi.Desktop/View/Prototype/DevicesPanePrototype.xaml[650-666]

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


8. DevicesPanePrototype missing XML docs📘 Rule violation ✧ Quality
Description
The new public DevicesPanePrototype class (and its public constructor) do not have XML
documentation comments with <summary>. This violates the requirement that all public API members
be documented.
Code

Daqifi.Desktop/View/Prototype/DevicesPanePrototype.xaml.cs[R8-15]

+public partial class DevicesPanePrototype : UserControl
+{
+    public DevicesPanePrototype()
+    {
+        InitializeComponent();
+        Loaded += OnLoaded;
+        Unloaded += OnUnloaded;
+    }
Evidence
The checklist requires XML documentation comments for all public members; the added `public partial
class DevicesPanePrototype and public DevicesPanePrototype() are missing /// <summary>`
documentation.

Rule 244813: Require XML documentation comments for all public API members
Daqifi.Desktop/View/Prototype/DevicesPanePrototype.xaml.cs[8-15]

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

## Issue description
A new public class and constructor were added without required XML documentation comments.

## Issue Context
Add `/// <summary>...</summary>` above `DevicesPanePrototype` and its public constructor (and any other public members in this file if introduced later).

## Fix Focus Areas
- Daqifi.Desktop/View/Prototype/DevicesPanePrototype.xaml.cs[8-15]

ⓘ 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

DeviceSettingsViewModel had no remaining references — no bindings, no
instantiations, no tests — and was throwing an MVVMTK0033 warning for
using [ObservableObject] without an ObservableObject base. All of its
state (SelectedDevice, IsLoggingToDevice, CanAccessSdCard, SdCard message)
is provided by the shell view-model and the new DevicesPaneViewModel.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Comment thread Daqifi.Desktop/View/Prototype/DevicesPanePrototype.xaml
Comment thread Daqifi.Desktop/View/Prototype/DevicesPanePrototype.xaml
Comment thread Daqifi.Desktop/View/Prototype/DevicesPanePrototype.xaml
- Replace the LoggingMode_Click code-behind with a RelayCommand<string>
  on DevicesPaneViewModel (SetLoggingModeCommand) and bind the
  RadioButtons to it via CommandParameter. Removes the Click handler
  (which also had a non-PascalCase name) and keeps all interaction in
  the view-model.

- Route the frequency slider through a new DevicesPaneViewModel.FrequencyHz
  proxy that reads from SelectedDevice.StreamingFrequency but writes
  through DaqifiViewModel.SelectedStreamingFrequency. That shell setter
  already blocks changes while LoggingManager.Active and surfaces an
  error dialog — so the drawer can no longer silently bypass the
  mid-session guard.

- Guard DaqifiViewModel.UpdateNetworkConfiguration against null
  SelectedDevice and disconnected devices, and wrap the call in
  try/catch so a transport failure surfaces as an error dialog instead
  of propagating out of the RelayCommand.

- Declare SelectedTile's dependent-property notifications via
  [NotifyPropertyChangedFor] attributes (SelectedDevice,
  SelectedDeviceSupportsFirmwareUpdate, FrequencyHz) and trim the
  partial to only the CanExecute bookkeeping the source generator
  can't emit.

- Add missing XML docs on DevicesPanePrototype and its constructor.

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

📊 Code Coverage Report

Summary

Summary
Generated on: 4/18/2026 - 7:28:00 PM
Coverage date: 4/18/2026 - 7:27:27 PM - 4/18/2026 - 7:27:55 PM
Parser: MultiReport (4x Cobertura)
Assemblies: 3
Classes: 120
Files: 153
Line coverage: 16.9% (1461 of 8622)
Covered lines: 1461
Uncovered lines: 7161
Coverable lines: 8622
Total lines: 24054
Branch coverage: 17.2% (501 of 2901)
Covered branches: 501
Total branches: 2901
Method coverage: Feature is only available for sponsors

Coverage

DAQiFi - 16.7%
Name Line Branch
DAQiFi 16.7% 17.2%
Daqifi.Desktop.App 5.4% 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 42.4% 39.2%
Daqifi.Desktop.Converters.BoolToActiveStatusConverter 0% 0%
Daqifi.Desktop.Converters.BoolToConnectionStatusConverter 0% 0%
Daqifi.Desktop.Converters.BoolToStatusColorConverter 0% 0%
Daqifi.Desktop.Converters.BrushColorMatchConverter 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.9% 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.DiskSpace.DiskSpaceCheckResult 100%
Daqifi.Desktop.DiskSpace.DiskSpaceEventArgs 100%
Daqifi.Desktop.DiskSpace.DiskSpaceMonitor 88.2% 86.6%
Daqifi.Desktop.DuplicateDeviceCheckResult 100%
Daqifi.Desktop.Exporter.OptimizedLoggingSessionExporter 66.5% 62.7%
Daqifi.Desktop.Exporter.SampleData 100%
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.MinMaxDownsampler 98.6% 97.9%
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.DatabaseMigrator 0% 0%
Daqifi.Desktop.Logger.DeviceLegendGroup 100% 100%
Daqifi.Desktop.Logger.LoggedSeriesLegendItem 0% 0%
Daqifi.Desktop.Logger.LoggingContext 100%
Daqifi.Desktop.Logger.LoggingContextDesignTimeFactory 0%
Daqifi.Desktop.Logger.LoggingManager 0% 0%
Daqifi.Desktop.Logger.LoggingSession 16% 5%
Daqifi.Desktop.Logger.PlotLogger 0% 0%
Daqifi.Desktop.Logger.SessionDeviceMetadata 80%
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.AddSamplesSessionTimeIndex 0%
Daqifi.Desktop.Migrations.AddSessionDeviceMetadata 0%
Daqifi.Desktop.Migrations.AddSessionSampleCount 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.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.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.MigrationStatusWindow 0% 0%
Daqifi.Desktop.View.MinimapInteractionController 0% 0%
Daqifi.Desktop.View.Prototype.ChannelsPanePrototype 0% 0%
Daqifi.Desktop.View.Prototype.DevicesPanePrototype 0% 0%
Daqifi.Desktop.View.SettingsDialog 0% 0%
Daqifi.Desktop.View.SuccessDialog 0% 0%
Daqifi.Desktop.ViewModels.AddProfileConfirmationDialogViewModel 0% 0%
Daqifi.Desktop.ViewModels.AddProfileDialogViewModel 0% 0%
Daqifi.Desktop.ViewModels.ChannelsPaneViewModel 0% 0%
Daqifi.Desktop.ViewModels.ChannelTileViewModel 0% 0%
Daqifi.Desktop.ViewModels.ConnectionDialogViewModel 21.7% 19.5%
Daqifi.Desktop.ViewModels.DaqifiViewModel 14.5% 8.2%
Daqifi.Desktop.ViewModels.DeviceLogsViewModel 0% 0%
Daqifi.Desktop.ViewModels.DevicesPaneViewModel 0% 0%
Daqifi.Desktop.ViewModels.DeviceTileViewModel 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.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 - 30.8%
Name Line Branch
Daqifi.Desktop.Common 30.8% 16.6%
Daqifi.Desktop.Common.Loggers.AppLogger 33.7% 16.6%
Daqifi.Desktop.Common.Loggers.NoOpLogger 0%
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
Copy link
Copy Markdown
Contributor Author

Thanks @qodo-code-review — replies to the items in the summary that weren't covered by the inline threads:

#4 LoggingMode_Click typo (_modemode) — Fixed. Resolved as part of #1: the code-behind handler was removed entirely and replaced with an IRelayCommand<string> SetLoggingModeCommand on DevicesPaneViewModel, so the parameter name no longer exists.

#5 [NotifyPropertyChangedFor] on SelectedTile — Fixed in d3cd857. _selectedTile now carries [NotifyPropertyChangedFor(nameof(SelectedDevice))], [NotifyPropertyChangedFor(nameof(SelectedDeviceSupportsFirmwareUpdate))], and [NotifyPropertyChangedFor(nameof(FrequencyHz))]. The OnSelectedTileChanged partial only does the imperative NotifyCanExecuteChanged() work the source generator can't do.

#6 #region directives — Respectfully disagree. The exemplar I was asked to mirror for this redesign is ChannelsPaneViewModel (from PR #479's unified Channels pane), and it does not use #region directives. Adding them asymmetrically to DevicesPaneViewModel would fragment conventions across sibling pane VMs of the same redesign. Happy to apply regions uniformly across both pane VMs in a follow-up if the team prefers that style — but I don't think this PR is the right place to diverge from the exemplar.

#7 XML docs on class and constructor — Fixed in d3cd857. Class-level <summary> describes the pane's role (aggregate tiles, own drawer state, delegate side-effects to the shell); constructor <summary> added. Individual public properties/commands also have doc comments.

#8 DI vs ConnectionManager.Instance — Respectfully disagree. ChannelsPaneViewModel (the redesign exemplar) accesses ConnectionManager.Instance and LoggingManager.Instance directly, and there is no DI container wired for pane VMs in this codebase. The pane VM follows the established pattern; introducing DI only here would create two divergent wiring styles for sibling pane VMs. A broader DI refactor for manager singletons is a reasonable follow-up, but out of scope for this PR.

@tylerkron tylerkron merged commit 7411d73 into main Apr 18, 2026
8 checks passed
@tylerkron tylerkron deleted the claude/competent-ride-d1cb54 branch April 18, 2026 19:31
tylerkron added a commit that referenced this pull request Apr 18, 2026
Replaces the MahApps MetroWindow settings dialog with a right-side
settings drawer at the app-window level, matching the Channels (#479)
and Devices (#484) panes. Preferences are a genuine separate surface
but not a destructive fork, so the drawer pattern fits the design
philosophy better than a modal.

- New IsAppSettingsOpen + AppSettings property on DaqifiViewModel, and
  Toggle/Close commands replacing ShowDAQiFiSettingsDialogCommand
- Drawer appended to MainWindow root Grid at ZIndex=20 so it overlays
  tab content and in-pane drawers; scrim click and X dismiss
- CSV delimiter uses a segmented COMMA/SEMICOLON toggle instead of a
  ComboBox — only two options and direct manipulation reads better
- SettingsViewModel upgraded to ObservableObject with bool selectors
  so the segmented toggle binds two-way
- Deletes SettingsDialog.xaml/.cs and the dialog-service invocation

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
tylerkron added a commit that referenced this pull request Apr 18, 2026
Redesigns the left tab strip to match the Channels (#479) and Devices
(#484) panes: dark surface (#0D0F12), 3px accent stripe on selected,
restrained icon/label pairs, no decorative white borders. Uses the same
color tokens documented in docs/design-philosophy.md.

- Selected: surface lifts to #171A20, stripe fades in over 120ms, text
  and icon brighten to primary
- Hover (unselected only): surface lifts to #13161C, text/icon brighten,
  cursor becomes Hand
- Focus visual: subtle 1px dashed border for keyboard users
- Drops the Label wrapper + IsEnabled=False hack that was greying labels
- Icons 30 -> 24px, labels 11pt

Pane contents for Live Graph / Logged Data / Profiles are unchanged —
they stay on MahApps light chrome until redesigned per Principle 9
(legacy areas are transition states).

Co-Authored-By: Claude Opus 4.7 <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