Skip to content
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.

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

Sentence highlightment #102

Merged
merged 6 commits into from
Nov 26, 2024
Merged

Sentence highlightment #102

merged 6 commits into from
Nov 26, 2024

Conversation

simon0356
Copy link
Contributor

@simon0356 simon0356 commented Oct 6, 2024

Hello there !

Even if I'm I huge fan of minicom I was looking for a serial monitor that allow me to perform sentence highlightment.
I find you plots feature interesting even if i haven't use it so far but i'm sure I will.
I'm still not a rust Guru and this is still work in progress but i wanted to share it with you and get your opinions on it.
I have concern on how much the sentence search using regex will load the CPU.
For sure it will be possible to save previously found sentence and only doing the search process on new lines.
Also I was wondering if

  • it is possible in egui to not perform a full reload of the "console_scroll_area" to preserve CPU
  • to reduce FPS when no new frames was available and to force a repaint_request on RX/TX messages
    Here is some capture i have done :
    serial_monitor_2
    serial_monitor
    ps : For the screenshot I got no "publicly" shareable UART trace so i force the content with same content as your screenshot, that why there is no graph on the screen

Thanks
Simon

@simon0356 simon0356 marked this pull request as draft October 6, 2024 15:26
@hacknus
Copy link
Owner

hacknus commented Oct 6, 2024

Cool, thanks for the input and this idea - looks really cool, I'll look into the code changes! As for your questions:

  • it is possible in egui to not perform a full reload of the "console_scroll_area" to preserve CPU.

I don't think so. It reloads the full GUI also when you move your cursor as far as I know...

  • to reduce FPS when no new frames was available and to force a repaint_request on RX/TX messages.

This would be worth looking into! Could be implemented with a sync-channel that always sends a value when a refresh is requested. We would then replace this line in gui.rs

       std::thread::sleep(Duration::from_millis((1000.0 / MAX_FPS) as u64));

with something like this:

       if let Ok(_) = request_refresh_rx.recv_timeout(Duration::MAX) {
        // just wait until a refresh is requested
       }

could also use request_refresh_rx.recv() without a specified timeout...

Create plot settings tab and highlights settings tab.
- The highlights settings allow user to set until 4 regex sentence to find in content and highlight them with predefined color
- Edits the plot settings to hide the plot view if number of plot equals 0

Signed-off-by: stropee <[email protected]>
@simon0356 simon0356 marked this pull request as ready for review October 29, 2024 08:35
@simon0356
Copy link
Contributor Author

I have rebased your main since you did the optimization for the refresh. Thanks !

@hacknus
Copy link
Owner

hacknus commented Oct 29, 2024

Thanks a lot for the work, this looks great! However, I think the two "tabs" for plot and highlighting are not super obvious. I don't have a clear picture of what would improve it, maybe leaving the plot settings as default and adding a "Advanced Settings" button which opens a settings window with the highlighting settings?

or having the "Plot settings" and the "Highlighting Settings" as collapsible options if that works with egui?

Because right now they are just two buttons that switch between modes, but it is not clear what they do, before one presses the buttons.

What do you think?

@simon0356
Copy link
Contributor Author

I agree that i was myself not convinced by my UI proposal, I will try to work on it as soon as possible.

Thanks for you feedback !

@hacknus
Copy link
Owner

hacknus commented Oct 30, 2024

maybe something like

ui.collapsing("Highlighting Settings", |ui| {
 // add settings
}

could work?

But I think we need to anyway touch up the GUI to make the settings scrollable, if the window/screen is too small

@simon0356
Copy link
Contributor Author

I have created collpasing menu for Highlighting, Plot and Export ( i also create function for each of this settings) with a vertical scroll .
serial_monitor
serial_monitor_2

@hacknus
Copy link
Owner

hacknus commented Nov 5, 2024

Cool, I think this will work nicely with my other PR where the Debug Info will also be a collapsible menu item: #106

Please supply a new entry into the changelog. I'll merge it, once I'm able to test it with hardware!

@simon0356
Copy link
Contributor Author

I have edit the changelog.
I think it could be nice to run an indenter in gui.rs, but I will let you look to.

@hacknus
Copy link
Owner

hacknus commented Nov 26, 2024

Sorry for the delay, thanks a lot for the contribution!!

@hacknus hacknus merged commit 90c0069 into hacknus:main Nov 26, 2024
2 checks passed
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.

2 participants