Skip to content

Fix errors overlaying on topics in ncurses frontend#35

Merged
sgillen merged 4 commits intoNVIDIA-ISAAC-ROS:mainfrom
bmchalenv:fix-error-logs-ncurses-dashboard
Feb 27, 2026
Merged

Fix errors overlaying on topics in ncurses frontend#35
sgillen merged 4 commits intoNVIDIA-ISAAC-ROS:mainfrom
bmchalenv:fix-error-logs-ncurses-dashboard

Conversation

@bmchalenv
Copy link
Contributor

@bmchalenv bmchalenv commented Feb 27, 2026

In frontend we should standardize to never use a logger that prints to screen. I've changed our loggers to use the .debug level so that it doesn't cause a mess unless the debug level is set. Potentially we should remove the loggers in the frontend?

It used to look like this:

image

Now it looks like this:

image

@greptile-apps
Copy link

greptile-apps bot commented Feb 27, 2026

Greptile Summary

Fixed ROS log messages from interfering with the ncurses terminal UI by disabling stdout logs and implementing proper error state tracking for colored status messages.

Key Changes:

  • Added --disable-stdout-logs flag to ROS initialization to prevent log output from overlaying the ncurses display
  • Changed toggle_topic_monitoring() to return tuple[bool, str] instead of void, enabling proper error message display in the UI
  • Introduced status_is_error flag to track error states and display error messages in red instead of blue
  • Improved error handling by returning error messages from service calls for UI display
  • Added status clearing when entering frequency input mode to prevent stale messages

Confidence Score: 5/5

  • This PR is safe to merge with no identified risks
  • The changes are well-implemented and focused on fixing UI display issues. The addition of --disable-stdout-logs directly addresses the root cause of log messages interfering with ncurses. Error handling improvements use proper return values and type annotations. No breaking changes or logical errors detected.
  • No files require special attention

Important Files Changed

Filename Overview
greenwave_monitor/greenwave_monitor/ncurses_frontend.py Added ROS stdout log suppression and error state tracking for proper status message coloring in ncurses UI
greenwave_monitor/greenwave_monitor/ui_adaptor.py Changed toggle_topic_monitoring to return success/error tuples for better error handling in UI

Last reviewed commit: fe896e8

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

2 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@sgillen
Copy link
Collaborator

sgillen commented Feb 27, 2026

Instead of not using loggers, is there a way can we instead just redirect stderr to a file (logs default to stderr)? That would be for developers, for users we should use the status bar.

@bmchalenv
Copy link
Contributor Author

Instead of not using loggers, is there a way can we instead just redirect stderr to a file (logs default to stderr)? That would be for developers, for users we should use the status bar.

We can do that. Pretty certain there's a method to configure the ROS logger to go there by default and we can add that in ncurses_dashboard when we run the node. I'll look into it.

Signed-off-by: Blake McHale <bmchale@nvidia.com>
Signed-off-by: Blake McHale <bmchale@nvidia.com>
Signed-off-by: Blake McHale <bmchale@nvidia.com>
@bmchalenv bmchalenv force-pushed the fix-error-logs-ncurses-dashboard branch from b1da55b to 3fa661f Compare February 27, 2026 19:54
Signed-off-by: Blake McHale <bmchale@nvidia.com>
@sgillen sgillen merged commit dbae71f into NVIDIA-ISAAC-ROS:main Feb 27, 2026
17 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