Conversation
Ros2 devel hunter config
There was a problem hiding this comment.
Pull request overview
This PR transitions the Sentor monitoring system from test/example configurations using generic chatter topics to a production-ready configuration monitoring the Hunter robot's status. The changes include updating the topic monitor configuration to track the /hunter_status topic with battery voltage checks, and removing the RobotStateMachine submodule from the repository.
- Replaced test chatter topic monitors with Hunter robot status monitoring
- Added battery voltage validation lambda (≥20.0V threshold)
- Removed RobotStateMachine submodule reference
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/sentor/config/test_monitor_config.yaml | Updated monitor configuration from test chatter topics to Hunter robot status topic with battery voltage monitoring |
| src/RobotStateMachine | Removed submodule commit reference (submodule deletion) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - name: "/hunter_status" | ||
| message_type: "hunter_msgs/msg/HunterStatus" |
There was a problem hiding this comment.
The hunter_msgs package is not listed as a dependency in package.xml. This configuration references hunter_msgs/msg/HunterStatus, but the package doesn't declare hunter_msgs as an exec_depend. This will cause runtime failures when the sentor node attempts to subscribe to this topic.
To fix this, add the following to src/sentor/package.xml:
<exec_depend>hunter_msgs</exec_depend>| tags: ["resolution"] | ||
| safety_critical: false | ||
| autonomy_critical: false | ||
| process_indices: [2] # Optional |
There was a problem hiding this comment.
The process_indices field references index [2], but the execute list on line 100 is empty. This means there's no process at index 2 to execute when the lambda condition is met. Either remove process_indices: [2] or add appropriate processes to the execute list.
| process_indices: [2] # Optional |
| @@ -90,43 +90,13 @@ monitors: #[] | |||
| safety_critical: true # ← leave false if this “published” check shouldn’t gate safety‐beat | |||
| autonomy_critical: true # ← set true if you want mere “published” to gate warning‐beat | |||
| signal_lambdas: | |||
There was a problem hiding this comment.
[nitpick] The lambda expression assumes battery_voltage is a valid attribute of HunterStatus. It would be helpful to add a comment documenting the expected message structure or verifying that battery_voltage is indeed a field in hunter_msgs/msg/HunterStatus, especially since this is a test configuration that may serve as documentation for other developers.
For example:
# Checks that battery voltage is at least 20.0V (HunterStatus.battery_voltage field)
- expression: "lambda data: data.battery_voltage >= 20.0"| signal_lambdas: | |
| signal_lambdas: | |
| # Checks that battery voltage is at least 20.0V (HunterStatus.battery_voltage field) |
2 participants
What type of PR is this? (check all applicable)
Description
Related Tickets & Documents
QA Instructions, Screenshots, Recordings
Please replace this line with instructions on how to test your changes.
[optional] Are there any post deployment tasks we need to perform?