Skip to content

Conversation

@bmchalenv
Copy link
Contributor

Resolves #19 and gives the capability to specify expected frequencies and tolerance through ROS 2 parameters. The topics parameter can now be specified with subfields topics.<topic_name>.(expected_frequency|tolerance).

The original topics parameter with a list still exists and can be used during initialization, but now parameters specified with the new subfields will also add those topics to the monitor node with the expected frequency and tolerance already set. They can be set in a ROS 2 parameter file like:

/monitor_node
    ros__parameters:
        topics:
            /imu:
                expected_frequency: 100
                tolerance: 5

or

/monitor_node:
    ros__parameters:
         topics: ["/image", "/string"]
         "topics./imu.expected_frequency": 100
         "topics./imu.tolerance": 5

@greptile-apps
Copy link

greptile-apps bot commented Jan 2, 2026

Greptile Summary

This PR implements ROS 2 parameter-based configuration for topic monitoring, allowing users to specify expected frequencies and tolerances through YAML files or dynamic parameter changes. The implementation adds a new parameter syntax topics.<topic_name>.(expected_frequency|tolerance) alongside the existing list-based topics parameter.

Key Changes

  • Added parameter parsing infrastructure in greenwave_monitor.cpp to handle nested topic configuration parameters
  • Implemented pending_topic_configs_ map to track incomplete configurations until frequency is set
  • Created parameter callback (on_parameter_change) to handle dynamic parameter updates during runtime
  • Added ui_adaptor.py module providing UI-friendly interface with parameter event tracking
  • Comprehensive test coverage including startup configuration, dynamic changes, YAML loading, and edge cases
  • Updated example launch file demonstrating new dictionary-based parameter syntax

Implementation Quality

  • Proper validation: frequency must be positive, tolerance must be non-negative
  • Graceful handling of partial configurations (e.g., tolerance without frequency)
  • Thread-safe parameter synchronization between service calls and parameter changes
  • Default tolerance (5%) when not specified
  • Backward compatibility maintained with original list-based topics parameter
  • Clean separation between parameter-driven and service-driven configuration paths

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The implementation is well-architected with proper validation, comprehensive test coverage (6 new test files covering startup, dynamic changes, YAML loading, and edge cases), and maintains backward compatibility. The code includes proper error handling, parameter synchronization, and follows ROS 2 best practices. No security issues, logic errors, or breaking changes detected.
  • No files require special attention

Important Files Changed

Filename Overview
greenwave_monitor/src/greenwave_monitor.cpp Adds robust parameter-based topic configuration with proper validation, callbacks, and fallback handling
greenwave_monitor/include/greenwave_monitor.hpp Adds new helper methods and data structures for parameter-based topic configuration
greenwave_monitor/greenwave_monitor/ui_adaptor.py New file providing UI-friendly adaptor with parameter tracking and service clients for monitor interaction
greenwave_monitor/test/parameters/test_param_dynamic.py Tests dynamic parameter changes via ros2 param set command during runtime
greenwave_monitor/test/parameters/test_param_yaml.py Tests loading topic configuration from YAML parameter file

Sequence Diagram

sequenceDiagram
    participant User
    participant Node as GreenwaveMonitor Node
    participant ParamCallback as on_parameter_change
    participant ConfigHandler as apply_topic_config_if_complete
    participant TopicHandler as set_topic_expected_frequency
    participant Diagnostics as MessageDiagnostics

    Note over User,Diagnostics: Startup: Loading Parameters from YAML/Overrides
    User->>Node: Launch with parameter file
    Node->>Node: Constructor: allow_undeclared_parameters(true)
    Node->>Node: Constructor: automatically_declare_parameters_from_overrides(true)
    Node->>Node: load_topic_parameters_from_overrides()
    Node->>Node: Parse all "topics.{topic}.{field}" parameters
    Node->>Node: Store in pending_topic_configs_
    loop For each topic with expected_frequency
        Node->>TopicHandler: set_topic_expected_frequency(add_topic_if_missing=true)
        TopicHandler->>Node: add_topic() if not exists
        TopicHandler->>Diagnostics: setExpectedDt(freq, tolerance)
        TopicHandler->>Node: declare_or_set_parameter() to sync params
    end

    Note over User,Diagnostics: Runtime: Dynamic Parameter Changes
    User->>Node: ros2 param set topics.{topic}.expected_frequency {value}
    Node->>ParamCallback: on_parameter_change([parameters])
    loop For each parameter
        ParamCallback->>ParamCallback: parse_topic_param_name()
        ParamCallback->>ParamCallback: Update pending_topic_configs_[topic]
        ParamCallback->>ConfigHandler: apply_topic_config_if_complete(topic)
        ConfigHandler->>ConfigHandler: Check if frequency is set
        alt Frequency available
            ConfigHandler->>TopicHandler: set_topic_expected_frequency(add_topic_if_missing=true)
            TopicHandler->>Node: add_topic() if needed
            TopicHandler->>Diagnostics: setExpectedDt(freq, tolerance)
            ConfigHandler->>ConfigHandler: erase from pending_topic_configs_
        else No frequency
            ConfigHandler->>ConfigHandler: Wait for frequency parameter
        end
    end

    Note over User,Diagnostics: Service Call: Set Expected Frequency
    User->>Node: Call set_expected_frequency service
    Node->>TopicHandler: set_topic_expected_frequency(update_parameters=true)
    TopicHandler->>Diagnostics: setExpectedDt(freq, tolerance)
    TopicHandler->>Node: declare_or_set_parameter() to sync params
    Node-->>User: Response with success/message
Loading

@bmchalenv bmchalenv marked this pull request as draft January 2, 2026 18:16
… 2 parameters

Signed-off-by: Blake McHale <bmchale@nvidia.com>
…y time

Signed-off-by: Blake McHale <bmchale@nvidia.com>
…cing with parameters

Signed-off-by: Blake McHale <bmchale@nvidia.com>
Signed-off-by: Blake McHale <bmchale@nvidia.com>
Signed-off-by: Blake McHale <bmchale@nvidia.com>
Signed-off-by: Blake McHale <bmchale@nvidia.com>
Signed-off-by: Blake McHale <bmchale@nvidia.com>
Signed-off-by: Blake McHale <bmchale@nvidia.com>
Signed-off-by: Blake McHale <bmchale@nvidia.com>
Signed-off-by: Blake McHale <bmchale@nvidia.com>
Signed-off-by: Blake McHale <bmchale@nvidia.com>
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.

Configuration file for the ncurses dashboard,

1 participant