Use parameter yaml on startup for configuring greenwave monitor + rename to greenwave diagnostics

[bmchalenv]

Add support for ROS parameter YAML on startup and rename message diagnostics to greenwave diagnostics to match parameter yaml group name. Look at the example.yaml if you want to see how it is integrated.

Fixes a few bugs/makes the system robust also. Specifically #16 .

[greptile-apps]

Greptile Summary

This PR adds YAML parameter configuration support for greenwave_monitor and renames the diagnostics system from message_diagnostics to greenwave_diagnostics. It also fixes issue #16 by adding proper destructors that cancel timers and clean up resources before node destruction.

Key Changes:

• Added YAML parameter loading via gw_frequency_monitored_topics and gw_monitored_topics parameters
• Implemented deferred initialization (100ms delay) to allow ROS graph to settle before topic discovery
• Added destructors to GreenwaveMonitor and MinimalPublisher that properly cancel timers and reset diagnostics, fixing the exit code -11 crash
• Renamed message_diagnostics namespace/classes to greenwave_diagnostics throughout the codebase
• Updated find_topic_type_with_retry to find_topic_type using get_publishers_info_by_topic() instead of get_topic_names_and_types()
• Added retry logic with configurable wait times for topic discovery
• Enhanced UI adaptor to handle expected frequency and tolerance from diagnostics with proper error handling

Issues Already Noted in Previous Threads:

• Test files create temporary YAML files without cleanup (resource leak)
• Several comments about topics with expected_frequency: 0.0 behavior have been discussed

Testing:

• Comprehensive test coverage added for YAML parameter loading in both test_greenwave_monitor.py and test_topic_monitoring_integration.py
• Tests verify proper handling of valid parameters, invalid/negative parameters, integer parameters, and nonexistent topics

Confidence Score: 4/5

• This PR is safe to merge with the understanding that previously identified issues (temporary file cleanup) remain unaddressed
• The PR successfully addresses issue Greenwave monitor sometimes exits with return code -11, causes flaky test. #16 with proper destructors and adds a valuable YAML configuration feature. The implementation is well-tested and the core logic is sound. Score is 4/5 because: (1) temporary file cleanup issues noted in previous threads remain unaddressed, and (2) the deferred initialization approach with a fixed 100ms delay could be more robust, though it's adequate for most use cases
• Test files (test_greenwave_monitor.py, test_topic_monitoring_integration.py) have resource leaks from uncleaned temporary files as noted in previous review threads

Important Files Changed

Filename	Overview
greenwave_monitor/src/greenwave_monitor.cpp	Added YAML parameter loading support and deferred initialization; includes proper destructor cleanup that addresses exit code -11 issue
greenwave_monitor/include/greenwave_monitor.hpp	Added destructor with proper timer cancellation and resource cleanup to fix issue #16 (exit code -11)
greenwave_monitor/include/minimal_publisher_node.hpp	Added destructor that properly cancels timer and resets diagnostics, directly fixes issue #16
greenwave_monitor/test/test_greenwave_monitor.py	Added comprehensive YAML parameter loading tests; temporary files created but not cleaned up (resource leak)
greenwave_monitor/test/test_topic_monitoring_integration.py	Added integration tests for YAML parameter loading; same temporary file cleanup issue as test_greenwave_monitor.py

Sequence Diagram

sequenceDiagram
    participant User
    participant LaunchFile
    participant GreenwaveMonitor
    participant InitTimer
    participant ROS2Graph
    participant MinimalPublisher
    participant Diagnostics

    User->>LaunchFile: Launch with YAML config
    LaunchFile->>GreenwaveMonitor: Create node with parameters
    GreenwaveMonitor->>GreenwaveMonitor: allow_undeclared_parameters(true)
    GreenwaveMonitor->>GreenwaveMonitor: automatically_declare_parameters_from_overrides(true)
    GreenwaveMonitor->>InitTimer: create_wall_timer(100ms)
    Note over GreenwaveMonitor: Defer topic discovery to allow ROS graph to settle
    
    User->>LaunchFile: Launch publisher nodes
    LaunchFile->>MinimalPublisher: Create publisher node
    MinimalPublisher->>ROS2Graph: Register publishers
    
    InitTimer-->>GreenwaveMonitor: Timer fires after 100ms
    GreenwaveMonitor->>InitTimer: cancel()
    GreenwaveMonitor->>GreenwaveMonitor: deferred_init()
    GreenwaveMonitor->>GreenwaveMonitor: add_topics_from_parameters()
    GreenwaveMonitor->>GreenwaveMonitor: Parse gw_frequency_monitored_topics
    GreenwaveMonitor->>GreenwaveMonitor: Parse gw_monitored_topics
    
    loop For each topic in YAML
        GreenwaveMonitor->>ROS2Graph: get_publishers_info_by_topic()
        ROS2Graph-->>GreenwaveMonitor: Publisher info
        GreenwaveMonitor->>GreenwaveMonitor: add_topic(topic, retries=5)
        GreenwaveMonitor->>Diagnostics: Create GreenwaveDiagnostics
        alt Has expected_frequency > 0
            GreenwaveMonitor->>Diagnostics: setExpectedDt(freq, tolerance)
        else frequency <= 0
            Note over GreenwaveMonitor: Monitor without frequency settings
        end
    end
    
    GreenwaveMonitor->>GreenwaveMonitor: Create service servers
    
    MinimalPublisher->>Diagnostics: Publish messages
    Diagnostics->>Diagnostics: updateDiagnostics()
    Diagnostics->>ROS2Graph: Publish to /diagnostics
    
    User->>GreenwaveMonitor: SIGINT (shutdown)
    GreenwaveMonitor->>InitTimer: cancel() in destructor
    GreenwaveMonitor->>GreenwaveMonitor: Clear diagnostics & subscriptions
    MinimalPublisher->>MinimalPublisher: cancel timer in destructor
    MinimalPublisher->>MinimalPublisher: reset greenwave_diagnostics_
    Note over GreenwaveMonitor,MinimalPublisher: Proper cleanup prevents exit code -11

Loading

[greptile-apps]

_{5 files reviewed, 5 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

_{5 files reviewed, 5 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

_{5 files reviewed, 5 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

_{1 file reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

_{1 file reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[sgillen]

This also closes #19

[sgillen]

Let's elaborate - ignored and we use the default "report topic frequency but don't check it" behavior

[sgillen]

I think we should include the string topic here as well? Also with expected_frequency = 0?

[sgillen]

Nit: We're missing the copyright header here.

[bmchalenv]

I left out the string topic on purpose so that it gets picked up by the topics:= parameter. I still think that parameter is useful for just starting the monitor without frequency and tolerance set. Wanted to make sure there was still an example.

[bmchalenv]

I moved topics:= into the YAML so that users can reference it.

[greptile-apps]

_{15 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

_{15 files reviewed, 4 comments}

_{Edit Code Review Agent Settings | Greptile}