JoySubscription BT node

[delihus]

Description

JoySubscription is a BT node for reading buttons state and run dedicated commands.

Example BT tree:

<?xml version="1.0" encoding="UTF-8"?>
<root BTCPP_format="4">
  <BehaviorTree ID="Dock">
    <Sequence>
      <ForceSuccess>
        <Sequence>
          <AreButtonsPressed topic_name="/panther/joy" buttons="0;0;1;0;0;0;1;1" />
          <DockRobotAction action_name="/panther/dock_robot" use_dock_id="True" dock_id="main_dock"
            dock_type="panther_docking_panther_charging_dock"
            navigate_to_staging_pose="False" />
        </Sequence>
      </ForceSuccess>
      <ForceSuccess>
        <Sequence>
          <AreButtonsPressed topic_name="/panther/joy" buttons="0;0;0;1;0;0;1;1" />
          <UndockRobotAction action_name="/panther/undock_robot"
            dock_type="panther_docking_panther_charging_dock" />
        </Sequence>
      </ForceSuccess>

    </Sequence>
  </BehaviorTree>
</root>

Modifications

• Added std::vector<int> convertFromString<std::vector<int>>(StringView str)
• Added JoySubscription
• Added a variable result to the manager's tests.

Summary by CodeRabbit

• New Features

  • Introduced a new behavior node for handling joystick input, facilitating the integration of joystick button states within a behavior tree framework.
  • Added functionality to convert semicolon-separated strings into vectors of integers, enhancing data handling capabilities.

• Bug Fixes

  • Improved error handling in the string conversion function to ensure robust input processing.

• Tests

  • Added a test suite for the JoySubscription plugin to validate functionality with joystick messages.
  • Introduced new unit tests for the convertFromString function to cover various input scenarios and edge cases.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Base branches to auto review (1)

• ros2-devel

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

The changes introduce a new shared library for joy_subscription_bt_node, implementing joystick input handling within a behavior tree framework. A new template specialization for converting strings to vectors of integers is added. Additionally, test files for the JoySubscription class and AreButtonsPressed class are created, alongside updates to existing tests, enhancing the testing framework. The module now depends on the sensor_msgs package for handling joystick messages.

Changes

Files	Change Summary
panther_manager/CMakeLists.txt	Added shared libraries for joy_subscription_bt_node and are_buttons_pressed_bt_node, along with corresponding tests.
panther_manager/include/panther_manager/behavior_tree_utils.hpp	Introduced template specialization for convertFromString to convert strings to std::vector<int>.
panther_manager/include/panther_manager/plugins/action/joy_subscription_node.hpp	Defined JoySubscription class for joystick message handling.
panther_manager/include/panther_manager/plugins/condition/are_buttons_pressed.hpp	Defined AreButtonsPressed class for checking joystick button states.
panther_manager/package.xml	Added dependency on sensor_msgs package.
panther_manager/src/plugins/action/joy_subscription_node.cpp	Implemented JoySubscription::onTick to process joystick messages.
panther_manager/src/plugins/condition/are_buttons_pressed.cpp	Implemented AreButtonsPressed::onTick to evaluate joystick button states.
panther_manager/test/plugins/action/test_joy_subscription_node.cpp	Created a test suite for JoySubscription functionality.
panther_manager/test/plugins/condition/test_are_buttons_pressed.cpp	Created a test suite for AreButtonsPressed functionality.
panther_manager/test/plugins/action/*.cpp	Modified main functions to store RUN_ALL_TESTS() result in a variable for readability.
panther_manager/test/test_behavior_tree_utils.cpp	Added unit tests for the convertFromString function with various scenarios.

---

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

• X
• Mastodon
• Reddit
• LinkedIn

Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai generate interesting stats about this repository and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai anywhere in the PR title to generate the title automatically.
• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[coderabbitai]

Actionable comments posted: 4

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 415a28e and 5cdf1e1.

Files selected for processing (16)

• panther_manager/CMakeLists.txt (2 hunks)
• panther_manager/include/panther_manager/behavior_tree_utils.hpp (1 hunks)
• panther_manager/include/panther_manager/plugins/action/joy_subscription_node.hpp (1 hunks)
• panther_manager/package.xml (1 hunks)
• panther_manager/src/plugins/action/joy_subscription_node.cpp (1 hunks)
• panther_manager/test/plugins/action/test_call_set_bool_service_node.cpp (1 hunks)
• panther_manager/test/plugins/action/test_call_set_led_animation_service_node.cpp (1 hunks)
• panther_manager/test/plugins/action/test_call_trigger_service_node.cpp (1 hunks)
• panther_manager/test/plugins/action/test_joy_subscription_node.cpp (1 hunks)
• panther_manager/test/plugins/action/test_shutdown_hosts_from_file_node.cpp (1 hunks)
• panther_manager/test/plugins/action/test_shutdown_single_host_node.cpp (1 hunks)
• panther_manager/test/plugins/action/test_signal_shutdown_node.cpp (1 hunks)
• panther_manager/test/plugins/decorator/test_tick_after_timeout_node.cpp (1 hunks)
• panther_manager/test/plugins/test_shutdown_host.cpp (1 hunks)
• panther_manager/test/plugins/test_shutdown_hosts_node.cpp (1 hunks)
• panther_manager/test/test_behavior_tree_utils.cpp (1 hunks)

Files skipped from review due to trivial changes (9)

• panther_manager/test/plugins/action/test_call_set_bool_service_node.cpp
• panther_manager/test/plugins/action/test_call_set_led_animation_service_node.cpp
• panther_manager/test/plugins/action/test_call_trigger_service_node.cpp
• panther_manager/test/plugins/action/test_shutdown_hosts_from_file_node.cpp
• panther_manager/test/plugins/action/test_shutdown_single_host_node.cpp
• panther_manager/test/plugins/action/test_signal_shutdown_node.cpp
• panther_manager/test/plugins/decorator/test_tick_after_timeout_node.cpp
• panther_manager/test/plugins/test_shutdown_host.cpp
• panther_manager/test/plugins/test_shutdown_hosts_node.cpp

Additional comments not posted (11)

panther_manager/package.xml (1)

29-29: Dependency on sensor_msgs is appropriate for joystick input handling.

The addition of sensor_msgs as a dependency is necessary for handling joystick messages, which aligns with the PR's objectives to enhance interactive control of robotic actions. This change is well-justified and integrates seamlessly with the existing dependencies.

panther_manager/test/plugins/action/test_joy_subscription_node.cpp (7)

47-52: Review of PublishJoyMessage method.

The method correctly constructs and publishes a sensor_msgs::msg::Joy message. The use of vector for buttons is appropriate and matches the expected message format.

This method is implemented correctly and follows ROS message publishing conventions.

---

54-57: Test case review: Loading the JoySubscription plugin.

This test validates that the JoySubscription node can be loaded correctly using the provided parameters.

The test is well-structured and effectively checks the plugin's loadability without errors.

---

59-66: Test case review: Handling no joystick message.

This test effectively checks the JoySubscription node's behavior when no messages are published. The expected status is FAILURE, which is correctly asserted.

The test is correctly designed to validate the node's behavior in the absence of input, adhering to the expected behavior described in the node's documentation.

---

68-77: Test case review: Handling joystick message with too few buttons.

This test correctly simulates a scenario where the joystick message does not meet the expected button count, leading to a FAILURE status.

The test is well-implemented to check the node's robustness against insufficient input data, aligning with the expected functionality.

---

79-88: Test case review: Correct button count but incorrect state.

This test effectively evaluates the node's response to a joystick message with the correct number of buttons but incorrect states, resulting in a FAILURE.

The test is appropriately designed to assess the node's functionality when the input meets the size criteria but fails state validation.

---

90-99: Test case review: Excess buttons but correct initial states.

This test accurately assesses the JoySubscription node's ability to handle joystick messages with more buttons than necessary, provided the required initial states are correct, resulting in a SUCCESS.

The test is well-designed to verify the node's tolerance for additional buttons beyond the specified configuration, as long as the necessary conditions are met.

---

101-110: Test case review: Correct button count and correct state.

This test effectively evaluates the JoySubscription node's response to a joystick message with the correct number of buttons and states, resulting in a SUCCESS.

The test is correctly designed to validate the node's functionality when the input fully meets the specified requirements.

panther_manager/CMakeLists.txt (2)

54-56: Properly added new library for joy_subscription_bt_node.

The library joy_subscription_bt_node is correctly defined as a shared library and added to the plugin_libs list. This ensures it will be linked and compiled with the appropriate flags and dependencies as defined later in the CMakeLists.txt.

---

143-147: Correct setup for testing joy_subscription_bt_node.

The test for joy_subscription_bt_node is properly set up using ament_add_gtest. This includes both the test file and the source file, ensuring that the node is tested in isolation. It's also correctly appended to the plugin_tests list, which likely is used to manage all test targets.

panther_manager/test/test_behavior_tree_utils.cpp (1)

124-152: Review of unit tests for convertFromString<std::vector<int>>

The unit tests provided for the convertFromString<std::vector<int>> function are comprehensive and cover various edge cases effectively. The tests are well-structured and use the Google Test framework appropriately.

• Test GoodVectorInt: Validates the correct parsing and vector creation from a semicolon-separated string of integers.
• Test EmptyVectorInt: Ensures that an empty input string correctly throws a BT::RuntimeError, which is consistent with the function's error handling strategy.
• Test InvalidVectorIntWithFloat and InvalidVectorIntWithLetter: These tests check the function's robustness against invalid input formats, ensuring that it throws exceptions as expected.

Overall, these tests are well-implemented and should effectively safeguard the functionality of the convertFromString<std::vector<int>> method.

[coderabbitai]

Actionable comments posted: 0

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 5cdf1e1 and b4b9da3.

Files selected for processing (5)

• panther_manager/CMakeLists.txt (2 hunks)
• panther_manager/include/panther_manager/behavior_tree_utils.hpp (1 hunks)
• panther_manager/include/panther_manager/plugins/condition/are_buttons_pressed.hpp (1 hunks)
• panther_manager/src/plugins/condition/are_buttons_pressed.cpp (1 hunks)
• panther_manager/test/plugins/condition/test_are_buttons_pressed.cpp (1 hunks)

Files skipped from review as they are similar to previous changes (2)

• panther_manager/CMakeLists.txt
• panther_manager/include/panther_manager/behavior_tree_utils.hpp

Additional comments not posted (7)

panther_manager/src/plugins/condition/are_buttons_pressed.cpp (1)

22-43: Review the logic and error handling in onTick.

1. Null Pointer Check: The check for last_msg being null is appropriate and prevents dereferencing a null pointer.
2. Button Count Check: The method correctly checks if the received message has fewer buttons than expected, which is good for avoiding out-of-range errors.
3. Button State Comparison: The use of std::equal assumes that last_msg->buttons has at least as many elements as buttons_. This is implicitly guaranteed by the previous size check, but it might be clearer to explicitly mention this assumption in the comments for future maintainability.

Consider adding a comment before the std::equal call to clarify the assumption about vector sizes being adequate due to the earlier check.

The method logic is correct, but consider enhancing the comments for clarity and maintainability.

panther_manager/include/panther_manager/plugins/condition/are_buttons_pressed.hpp (1)

32-51: Review the class design of AreButtonsPressed.

The class is well-structured for its purpose as a Behavior Tree node handling joystick input:

1. Inheritance and Constructor: Properly inherits from BT::RosTopicSubNode and initializes it in the constructor.
2. Method onTick: Appropriately handles the logic for checking button presses.
3. Method providedPorts: Correctly defines the input ports needed for the node to function, which is essential for integrating with the BT framework.

Overall, the class design follows good OOP practices and is consistent with the Behavior Tree framework's requirements.

The class design is approved as it meets the requirements and follows best practices.

panther_manager/test/plugins/condition/test_are_buttons_pressed.cpp (5)

54-57: Approve the test case for loading the plugin.

The test case LoadingAreButtonsPressedPlugin correctly verifies that the plugin can be loaded without throwing exceptions, which is a fundamental check for any plugin.

The test case is approved as it effectively checks the basic functionality.

---

59-66: Approve the test case for handling no message scenario.

The test case NoMessage effectively checks the node's behavior when no joystick message is received, expecting a failure status, which is the correct behavior in such a scenario.

The test case is approved as it correctly tests the node's response to missing input.

---

68-77: Approve the test case for handling messages with too few buttons.

The test case WrongMessageTooFewButtons checks the node's behavior when the received joystick message has fewer buttons than expected, correctly expecting a failure status. This ensures that the node robustly handles input validation.

The test case is approved as it effectively tests for proper input validation.

---

79-88: Approve the test case for handling correct number of buttons with wrong states.

The test case GoodMessageWrongButtonsState effectively evaluates the node's response when the joystick message has the correct number of buttons but the states do not match the expected configuration, correctly expecting a failure status. This test ensures that the node accurately assesses button states.

The test case is approved as it thoroughly tests the node's logic in evaluating button states.

---

90-99: Approve the test case for handling messages with extra buttons but correct states.

The test case GoodMessageWithTooMuchButtonsAndGoodButtonsState checks the node's behavior when the joystick message includes more buttons than expected but contains the correct states for the required buttons, correctly expecting a success status. This test ensures that the node is flexible in handling inputs with additional data as long as the necessary conditions are met.

The test case is approved as it effectively tests the node's flexibility in handling varied input sizes.