created scope launcher + signal selector by aaryapatnaik · Pull Request #435 · PurdueElectricRacing/monorepo

aaryapatnaik · 2026-08-16T00:14:30Z

Signal selector system

Scope widget

gitar-bot · 2026-08-16T00:15:31Z

CI failed: Formatting check failure in the CI build due to unformatted Rust code introduced by the PR changes.

Overview

1 CI job failed across 1 analyzed log due to a code formatting violation in the "Check formatting" check.

Failures

Rust Code Formatting Violation (confidence: high)

Type: tooling
Affected jobs: 95089696099
Related to change: yes
Root cause: The code changes in the PR do not adhere to the formatting rules defined by rustfmt, causing the formatting check step to exit with exit code 1.
Suggested fix: Run cargo fmt locally, review the formatting changes, and commit and push them to the branch.

Summary

Change-related failures: 1 code formatting check failure
Infrastructure/flaky failures: None
Recommended action: Run cargo fmt locally and push the updated formatting changes.

Code Review 👍 Approved with suggestions 0 resolved / 1 findings

Adds a scope launcher and signal selector UI component for visualizing daqapp signals. Consider resolving the stale heading and picker render issue when changing signals.

💡 Quality: Change Signal renders stale heading + picker in same frame

📄 daqapp/src/ui/scope.rs:189-203

After a signal is assigned, show unwraps and draws the heading "📊 {title}: {msg} - {signal}" and the Change Signal button; clicking it calls reset_to_picker() which nulls msg_id in the same frame, so the immediately-following if self.msg_id.is_none() block then draws the picker directly under the stale signal heading for one frame. It works but produces a brief inconsistent UI. Consider checking reset_to_picker was triggered before drawing the assigned-signal heading (e.g. capture the click result and branch before rendering the heading), removing the duplicated picker/no_dbc_placeholder block.

🤖 Prompt for agents

Code Review: Adds a scope launcher and signal selector UI component for visualizing daqapp signals. Consider resolving the stale heading and picker render issue when changing signals.

1. 💡 Quality: Change Signal renders stale heading + picker in same frame
   Files: daqapp/src/ui/scope.rs:189-203

   After a signal is assigned, `show` unwraps and draws the heading "📊 {title}: {msg} - {signal}" and the Change Signal button; clicking it calls `reset_to_picker()` which nulls `msg_id` in the same frame, so the immediately-following `if self.msg_id.is_none()` block then draws the picker directly under the stale signal heading for one frame. It works but produces a brief inconsistent UI. Consider checking `reset_to_picker` was triggered before drawing the assigned-signal heading (e.g. capture the click result and branch before rendering the heading), removing the duplicated picker/no_dbc_placeholder block.

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

`Auto-apply`	`Compact`
`gitar auto-apply:on`	`gitar display:verbose`

_{Was this helpful? React with 👍 / 👎 | Gitar}

gitar-bot · 2026-08-16T00:17:00Z

+        let msg_name = self.msg_name.clone().unwrap();
+        let signal_name = self.signal_name.clone().unwrap();
+
+        // Horizontal container (heading + new Change Signal button)
+        ui.horizontal(|ui| {
+            ui.heading(format!("📊 {}: {} - {}", self.title, msg_name, signal_name));
+            ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| {
+                if ui.button("🔀 Change Signal").clicked() {
+                    self.reset_to_picker();
+                }
+            });
+        });
+
+        // When change signal is clicked go back to the picker ui
+        if self.msg_id.is_none() {


💡 Quality: Change Signal renders stale heading + picker in same frame

After a signal is assigned, show unwraps and draws the heading "📊 {title}: {msg} - {signal}" and the Change Signal button; clicking it calls reset_to_picker() which nulls msg_id in the same frame, so the immediately-following if self.msg_id.is_none() block then draws the picker directly under the stale signal heading for one frame. It works but produces a brief inconsistent UI. Consider checking reset_to_picker was triggered before drawing the assigned-signal heading (e.g. capture the click result and branch before rendering the heading), removing the duplicated picker/no_dbc_placeholder block.

_{Was this helpful? React with 👍 / 👎}

LelsersLasers

🔥 It seems to work well. I put a few small comments about code style/using Rust types.

Addtionally you should

Rebase on master
run cargo fmt in the daqapp folder

LelsersLasers · 2026-08-16T03:33:09Z

@@ -15,7 +15,16 @@ impl WidgetIds {
    }

    pub fn next(&mut self, kind: widget_constructor::WidgetConstructor) -> usize {


Not exactly your problem as it was left over from a change I made, but I think next() can take WidgetConstructor as a reference instead of an owned value?

Something like this:

pub fn next(&mut self, kind: widget_constructor::WidgetConstructor) -> usize {

LelsersLasers · 2026-08-16T03:38:24Z


    pub fn next(&mut self, kind: widget_constructor::WidgetConstructor) -> usize {
-        let disc = std::mem::discriminant(&kind);
+        let disc = if matches!(kind, widget_constructor::WidgetConstructor::ScopeEmpty) {


I see why this solution works, but I don't think it may be the best for the longer term.

I think something like this could make sense?

Add a new "WidgetType" enum:

#[derive(Eq, PartialEq, Hash, Clone, Copy)] pub enum WidgetKind { ViewerTable, ViewerList, Bootloader, Scope, LogParser, SendUi, BusLoad, BatteryVoltage, BatteryTemps, GgPlot, GpsPlot, Dynamics, Jitter, Hil, }

Then WidgetConstructor can get a method like:

fn kind(&self) -> WidgetKind { match self { Self::ViewerTable => WidgetKind::ViewerTable, Self::ViewerList => WidgetKind::ViewerList, Self::Bootloader => WidgetKind::Bootloader, Self::Scope { .. } | Self::ScopeEmpty => WidgetKind::Scope, Self::LogParser => WidgetKind::LogParser, Self::SendUi => WidgetKind::SendUi, Self::BusLoad => WidgetKind::BusLoad, Self::BatteryVoltage => WidgetKind::BatteryVoltage, Self::BatteryTemps => WidgetKind::BatteryTemps, Self::GgPlot => WidgetKind::GgPlot, Self::GpsPlot => WidgetKind::GpsPlot, Self::Dynamics => WidgetKind::Dynamics, Self::Jitter => WidgetKind::Jitter, Self::Hil => WidgetKind::Hil, } }

And widget ids looks a little more like?

pub fn next(&mut self, constructor: &WidgetConstructor) -> usize { let counter = self.counters.entry(constructor.kind()).or_insert(1); let id = *counter; *counter += 1; id }

I think as a side effect of this, you can also clean up the Hash versus PartialEq implementations?

LelsersLasers · 2026-08-16T03:44:06Z

+            return egui_tiles::UiResponse::None;
+        }
+
+        let msg_name = self.msg_name.clone().unwrap();


Avoid unwrap. I think it's safe because self.msg_name.is_none() has it's code own path, but the Rust type system can help you here to avoid it

LelsersLasers · 2026-08-16T03:48:05Z

-    msg_id: u32,
-    msg_name: String,
-    signal_name: String,
+    msg_id: Option<u32>,


From my understanding, msg_id and msg_name are either both None or both Some at the same time. To get better production, you can make a mini struct and then the Scope just has Option

LelsersLasers · 2026-08-16T03:51:54Z

+    msg_id: Option<u32>,
+    msg_name: Option<String>,
+    signal_name: Option<String>,
+    msg_picker: DbcMsgPickerState, // Picker state, only relevant while msg_id/signal_name are None (or user hit "Change Signal")


You can use a sum type (Rust enum) for this. You could have an enum like (psudeo code):

enum { NoMessagePicked { DbcMsgPickerState} MessagePicked { msg_name, msg_id }, MessageAndSignalPicked { msg_name, msg_id, signal_name }

LelsersLasers · 2026-08-16T03:53:19Z

 impl Scope {
+    // Og constructor, unchanged
    pub fn new(instance_num: usize, msg_id: u32, msg_name: String, signal_name: String) -> Self {
        let title = format!("Scope #{}", instance_num);


Not a problem that you were supposed to solve, but is it possible to make the title include the signal name instead of the instance_num?

LelsersLasers · 2026-08-16T03:56:41Z

 use egui_plot::{Line, Plot, PlotPoints};
 use std::collections::VecDeque;

+use super::dbc_msg_picker::{DbcMsgPickerState, no_dbc_placeholder};


Small: I prefer the full qualified names (ex: add ui to the use crate:: options and then use ui::dbc_msg_picker::DbcMsgPickerState everywhere), but if you like the shorter usages elsewhere, instead of super you can do: still ui to the use crate:: but then do something like use ui::dbc_msg_picker::{DbcMsgPickerState, self} that way we are not relient on the folder structure for the use statement and the free floating function no_dbc_placeholder gets at least 1 level of namespacing.

LelsersLasers · 2026-08-16T04:01:04Z

        if let messages::MsgFromCan::ParsedMessage(parsed_msg) = msg {
-            if parsed_msg.decoded.msg_id != self.msg_id {
+            if parsed_msg.decoded.msg_id != target_msg_id {
                return;


highkey parsed_msg should have .decoded.name and you may be able to drop the reliance on message id

created scope launcher + signal selector

a9e7999

aaryapatnaik requested a review from a team as a code owner August 16, 2026 00:14

github-actions Bot added the daqapp label Aug 16, 2026

gitar-bot Bot reviewed Aug 16, 2026

View reviewed changes

LelsersLasers requested changes Aug 16, 2026

View reviewed changes

LelsersLasers reviewed Aug 16, 2026

View reviewed changes

Navigation Menu

Uh oh!

created scope launcher + signal selector - #435

created scope launcher + signal selector#435
aaryapatnaik wants to merge 1 commit into
masterfrom
aarya/scope-widget

aaryapatnaik commented Aug 16, 2026

Uh oh!

gitar-bot Bot commented Aug 16, 2026 •

edited

Loading

Overview

Failures

Rust Code Formatting Violation (confidence: high)

Summary

Uh oh!

gitar-bot Bot Aug 16, 2026

Uh oh!

LelsersLasers left a comment •

edited by aaryapatnaik

Loading

Uh oh!

LelsersLasers Aug 16, 2026

Uh oh!

LelsersLasers Aug 16, 2026

Uh oh!

LelsersLasers Aug 16, 2026

Uh oh!

LelsersLasers Aug 16, 2026

Uh oh!

LelsersLasers Aug 16, 2026

Uh oh!

LelsersLasers Aug 16, 2026

Uh oh!

LelsersLasers Aug 16, 2026

Uh oh!

LelsersLasers Aug 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -15,7 +15,16 @@ impl WidgetIds {
		}

		pub fn next(&mut self, kind: widget_constructor::WidgetConstructor) -> usize {

Uh oh!

Conversation

aaryapatnaik commented Aug 16, 2026

Uh oh!

gitar-bot Bot commented Aug 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Failures

Rust Code Formatting Violation (confidence: high)

Summary

Uh oh!

gitar-bot Bot Aug 16, 2026

Choose a reason for hiding this comment

Uh oh!

LelsersLasers left a comment • edited by aaryapatnaik Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gitar-bot Bot commented Aug 16, 2026 •

edited

Loading

LelsersLasers left a comment •

edited by aaryapatnaik

Loading