Skip to content

created scope launcher + signal selector #435

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
aaryapatnaik wants to merge 1 commit into
base: master
Choose a base branch
from
Open

created scope launcher + signal selector #435

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions daqapp/src/action.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ impl AppAction {
"Spawn CAN List",
widget_constructor::WidgetConstructor::ViewerList,
),
(
"Spawn Scope",
widget_constructor::WidgetConstructor::ScopeEmpty,
),
(
"Spawn Bootloader",
widget_constructor::WidgetConstructor::Bootloader,
Expand Down
157 changes: 140 additions & 17 deletions daqapp/src/ui/scope.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
use crate::messages;
use crate::{app, messages, util};
use eframe::egui;
use egui_plot::{Line, Plot, PlotPoints};
use std::collections::VecDeque;

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

@LelsersLasers LelsersLasers Aug 16, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


pub struct Scope {
pub title: String,
msg_id: u32,
msg_name: String,
signal_name: String,
msg_id: Option<u32>,

@LelsersLasers LelsersLasers Aug 16, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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")

@LelsersLasers LelsersLasers Aug 16, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 }

selected_msg: Option<can_dbc::Message>,
window: VecDeque<(f64, f64)>, // (time, value)
window_duration_seconds: f64,
decimation_factor: u64,
Expand All @@ -17,13 +21,35 @@ pub struct Scope {
}

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);

@LelsersLasers LelsersLasers Aug 16, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Self {
title,
msg_id,
msg_name,
signal_name,
msg_id: Some(msg_id),
msg_name: Some(msg_name),
signal_name: Some(signal_name),
msg_picker: DbcMsgPickerState::default(),
selected_msg: None,
window: VecDeque::new(),
window_duration_seconds: 10.0, // Default 10 seconds
decimation_factor: 0,
decimation_counter: 0,
reference_time: None,
is_paused: false,
}
}

// New constructor, used by sidebar, opens into picker ui
pub fn new_empty(instance_num: usize) -> Self {
let title = format!("Scope #{}", instance_num);
Self {
title,
msg_id: None,
msg_name: None,
signal_name: None,
msg_picker: DbcMsgPickerState::default(),
selected_msg: None,
window: VecDeque::new(),
window_duration_seconds: 10.0, // Default 10 seconds
decimation_factor: 0,
Expand All @@ -33,6 +59,26 @@ impl Scope {
}
}

// Clears signal assignment and buffered data and goes back to picker ui so new signal can b chosen
fn reset_to_picker(&mut self) {
self.msg_id = None;
self.msg_name = None;
self.signal_name = None;
self.selected_msg = None;
self.window.clear();
self.reference_time = None;
}

// Called once the user has picked a message and signal, assigns scopes target and resets plot buffer
fn assign_signal(&mut self, msg_id: u32, msg_name: String, signal_name: String) {
self.msg_id = Some(msg_id);
self.msg_name = Some(msg_name);
self.signal_name = Some(signal_name);
self.selected_msg = None;
self.window.clear();
self.reference_time = None;
}

pub fn add_point(&mut self, timestamp: chrono::DateTime<chrono::Local>, value: f64) {
if self.is_paused {
return;
Expand Down Expand Up @@ -85,11 +131,83 @@ impl Scope {
}
}

pub fn show(&mut self, ui: &mut egui::Ui) -> egui_tiles::UiResponse {
ui.heading(format!(
"📊 {}: {} - {}",
self.title, self.msg_name, self.signal_name
));
// Renders picker ui when there's no specific signal selected
fn show_picker(&mut self, ui: &mut egui::Ui, parser: &app::ParserInfo) {
if let Some(msg) = self
.msg_picker
.show(ui, &parser.parser, self.selected_msg.is_none())
{
self.selected_msg = Some(msg);
}

let Some(selected_msg) = self.selected_msg.clone() else {
return;
};

ui.separator();
ui.label(
egui::RichText::new(format!(
"Selected Message: {} (0x{:03X}) — pick a signal:",
selected_msg.name,
util::can::can_dbc_to_u32_without_extid_flag(&selected_msg.id)
))
.strong(),
);

let msg_id = util::can::can_dbc_to_u32_without_extid_flag(&selected_msg.id);
for sig in &selected_msg.signals {
if ui.button(&sig.name).clicked() {
self.assign_signal(msg_id, selected_msg.name.clone(), sig.name.clone());
break;
}
}

if ui.button("← Back to message search").clicked() {
self.selected_msg = None;
}
}

pub fn show(
&mut self,
ui: &mut egui::Ui,
parser: Option<&app::ParserInfo>,
) -> egui_tiles::UiResponse {
// Since no signal is assigned ask the user to pick a signal
if self.msg_id.is_none() || self.msg_name.is_none() || self.signal_name.is_none() {
ui.heading(format!("📊 {}: No signal selected", self.title));
ui.separator();

let Some(parser) = parser else {
no_dbc_placeholder(ui);
return egui_tiles::UiResponse::None;
};

self.show_picker(ui, parser);
return egui_tiles::UiResponse::None;
}

let msg_name = self.msg_name.clone().unwrap();

@LelsersLasers LelsersLasers Aug 16, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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() {
Comment on lines +189 to +203

@gitar-bot gitar-bot Bot Aug 16, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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 👍 / 👎

let Some(parser) = parser else {
no_dbc_placeholder(ui);
return egui_tiles::UiResponse::None;
};
self.show_picker(ui, parser);
return egui_tiles::UiResponse::None;
}

// Horizontal container
ui.horizontal(|ui| {
Expand Down Expand Up @@ -142,7 +260,7 @@ impl Scope {
.view_aspect(2.0)
.auto_bounds(egui::Vec2b::TRUE)
.x_axis_label("Time (seconds)")
.y_axis_label(&self.signal_name)
.y_axis_label(&signal_name)
.show(ui, |plot_ui| {
if self.window.is_empty() {
return;
Expand All @@ -154,7 +272,7 @@ impl Scope {
.map(|(time, value)| [*time, *value])
.collect();

let line = Line::new(&self.signal_name, points)
let line = Line::new(&signal_name, points)
.color(egui::Color32::from_rgb(100, 200, 100))
.stroke(egui::Stroke::new(
2.0,
Expand All @@ -168,16 +286,21 @@ impl Scope {
}

pub fn handle_can_message(&mut self, msg: &messages::MsgFromCan) {
// If no signal is assigned, there's nothing to plot
let (Some(target_msg_id), Some(signal_name)) = (self.msg_id, &self.signal_name) else {
return;
};

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;

@LelsersLasers LelsersLasers Aug 16, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}

let Some(signal) = parsed_msg.decoded.signals.get(&self.signal_name) else {
let Some(signal) = parsed_msg.decoded.signals.get(signal_name) else {
return;
};

self.add_point(parsed_msg.timestamp, signal.value.physical);
}
}
}
}
6 changes: 6 additions & 0 deletions daqapp/src/ui/sidebar.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,12 @@ pub fn show(app: &mut app::DAQApp, ctx: &egui::Context) {
));
}

if ui.button("Add Scope").clicked() {
app.action_queue.push(action::AppAction::SpawnWidget(
widget_constructor::WidgetConstructor::ScopeEmpty,
));
}

if ui.button("Add Bootloader").clicked() {
app.action_queue.push(action::AppAction::SpawnWidget(
widget_constructor::WidgetConstructor::Bootloader,
Expand Down
4 changes: 4 additions & 0 deletions daqapp/src/widget_constructor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ pub enum WidgetConstructor {
msg_name: String,
signal_name: String,
},
ScopeEmpty,
LogParser,
SendUi,
BusLoad,
Expand Down Expand Up @@ -50,6 +51,9 @@ impl WidgetConstructor {
msg_name,
signal_name,
} => widgets::Widget::Scope(ui::scope::Scope::new(id, msg_id, msg_name, signal_name)),
WidgetConstructor::ScopeEmpty => {
widgets::Widget::Scope(ui::scope::Scope::new_empty(id))
}
WidgetConstructor::LogParser => {
widgets::Widget::LogParser(ui::log_parser::LogParser::new(id))
}
Expand Down
11 changes: 10 additions & 1 deletion daqapp/src/widget_ids.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,16 @@ impl WidgetIds {
}

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

@LelsersLasers LelsersLasers Aug 16, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 {

let disc = std::mem::discriminant(&kind);
let disc = if matches!(kind, widget_constructor::WidgetConstructor::ScopeEmpty) {

@LelsersLasers LelsersLasers Aug 16, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

std::mem::discriminant(&widget_constructor::WidgetConstructor::Scope {
msg_id: 0,
msg_name: String::new(),
signal_name: String::new(),
})
} else {
std::mem::discriminant(&kind)
};

let counter = self.counters.entry(disc).or_insert(1);
let id = *counter;
*counter += 1;
Expand Down
2 changes: 1 addition & 1 deletion daqapp/src/widgets.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ impl Widget {
Widget::ViewerTable(w) => w.show(ui, action_queue, formatter, parser),
Widget::ViewerList(w) => w.show(ui, formatter, parser),
Widget::Bootloader(w) => w.show(ui),
Widget::Scope(w) => w.show(ui),
Widget::Scope(w) => w.show(ui, parser),
Widget::LogParser(w) => w.show(ui, parser),
Widget::SendUi(w) => w.show(ui, parser, formatter),
Widget::BusLoad(w) => w.show(ui),
Expand Down
Loading