Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ros2 api reorganization #362

Merged
merged 6 commits into from
Jul 9, 2024
Merged

Ros2 api reorganization #362

merged 6 commits into from
Jul 9, 2024

Conversation

pawelirh
Copy link
Contributor

@pawelirh pawelirh commented Jul 5, 2024

Description

This PR aims to enhance the ROS 2 API naming and make it consistent across the project

Modifications

  • Improve ROS2 API names
  • Fix battery tests

Summary by CodeRabbit

  • New Features

    • Renamed battery_node to battery_driver for better clarity and consistency.
    • Updated topics for publishing and subscribing to enhance message organization.
    • Added configuration parameters for ADC device paths and moving average window lengths.
  • Bug Fixes

    • Corrected indentation in .github/workflows/release-repository.yaml.
  • Documentation

    • Updated README files across multiple packages to reflect new node names and configurations.
  • Refactor

    • Renamed various nodes and topics for improved clarity and consistency.
  • Style

    • Adjusted topic and service names for better readability and organization.

Copy link
Contributor

coderabbitai bot commented Jul 5, 2024

Walkthrough

This update encompasses a significant renaming and reorganization of multiple entities across several packages. The changes include rebranding nodes and topics for better clarity and maintaining consistency in naming conventions. It also involves modifying configuration files, launch files, and README documents to align with the new names, alongside minor logic adjustments.

Changes

File Path Change Summary
.github/workflows/release-repository.yaml Minor indentation adjustment in the Grant permissions step.
panther_battery Renaming and reordering of nodes, topics, and parameters for consistency. Adjustments to function signatures and topic names in multiple files.
panther_bringup Renaming ekf_node to ekf_filter in README and configurations for service server topics.
panther_controller Renaming panther_base_controller to drive_controller in README and YAML configuration files.
panther_diagnostics Renaming system_status_node to system_monitor in README, launch files, and source code. Adjusting publishing topic names.
panther_gazebo Updating topic names for battery status and LED lights channels in README and configuration files for better clarity.
panther_hardware_interfaces Modifying topic names associated with motor controllers state in README and source code for alignment with new naming conventions.
panther_lights Renaming ROS topics and services related to LED lights control for consistency. Updating launch files and source code to reflect these changes.
panther_manager Corrections and updates to various topic names in testing files to ensure alignment with new naming conventions.
panther_localization Renaming ekf_node to ekf_filter throughout the README to reflect the node's purpose.

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
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 as 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.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 7

Outside diff range and nitpick comments (2)
panther_manager/config/safety_manager_config.yaml (1)

2-2: Add a description for the new configuration parameters.

To enhance readability and maintainability, consider adding comments that describe the new parameters for battery and cpu temperature window lengths and fan control settings.

  battery:
    temp:
      window_len: 6  # Temperature moving average window length for the battery
  cpu:
    temp:
      window_len: 6  # Temperature moving average window length for the CPU
      fan_on: 75.0  # CPU fan turn on temperature (in Celsius)
      fan_off: 70.0  # CPU fan turn off temperature (in Celsius)
panther_hardware_interfaces/src/panther_system_ros_interface.cpp (1)

[!TIP]
Codebase Verification

Issues Found:

There are multiple instances of the old topic names (motor_controllers_state, io_state, and e_stop) that have not been updated to the new ones. Please update the following occurrences:

  • motor_controllers_state

    • panther_manager/README.md
    • panther_manager/test/test_safety_manager_node.cpp
    • panther_manager/src/safety_manager_node.cpp
    • panther_controller/launch/controller.launch.py
    • panther_hardware_interfaces/README.md
    • panther_hardware_interfaces/test/include/test_constants.hpp
    • panther_battery/README.md
    • panther_battery/src/battery_node.cpp
    • panther_battery/test/include/test_battery_node.hpp
  • io_state

    • panther_manager/README.md
    • panther_manager/test/test_safety_manager_node.cpp
    • panther_manager/test/test_safety_behavior_tree.cpp
    • panther_manager/src/safety_manager_node.cpp
    • panther_manager/include/panther_manager/safety_manager_node.hpp
    • panther_utils/CHANGELOG.rst
    • panther_description/CHANGELOG.rst
    • panther_hardware_interfaces/README.md
    • panther_hardware_interfaces/test/test_gpio_controller.cpp
    • panther_hardware_interfaces/src/panther_system_ros_interface.cpp
    • panther_hardware_interfaces/src/panther_system.cpp
    • panther_hardware_interfaces/src/gpio_controller.cpp
    • panther_hardware_interfaces/include/panther_hardware_interfaces/panther_system_ros_interface.hpp
    • panther_hardware_interfaces/CHANGELOG.rst
    • panther_bringup/CHANGELOG.rst
    • panther_battery/README.md
    • panther_battery/test/test_battery_node_dual_bat.cpp
    • panther_battery/test/test_battery_publisher.cpp
    • panther_battery/include/panther_battery/battery_publisher.hpp
    • panther_battery/test/test_battery_node.cpp
    • panther_controller/launch/controller.launch.py
  • e_stop

    • panther_manager/include/panther_manager/safety_manager_node.hpp
    • panther_manager/README.md
    • panther_manager/include/panther_manager/lights_manager_node.hpp
    • panther_manager/test/test_safety_manager_node.cpp
    • panther_manager/test/test_safety_behavior_tree.cpp
    • panther_manager/test/test_lights_manager_node.cpp
    • panther_manager/test/test_lights_behavior_tree.cpp
    • panther_manager/src/safety_manager_node.cpp
    • panther_manager/src/lights_manager_node.cpp
    • panther_manager/behavior_trees/safety.xml
    • panther_manager/behavior_trees/lights.xml
    • panther_hardware_interfaces/README.md
    • panther_hardware_interfaces/CMakeLists.txt
    • panther_hardware_interfaces/include/panther_hardware_interfaces/panther_system_ros_interface.hpp
    • panther_hardware_interfaces/include/panther_hardware_interfaces/panther_system_e_stop.hpp
    • panther_hardware_interfaces/include/panther_hardware_interfaces/panther_system.hpp
    • panther_hardware_interfaces/src/panther_system_ros_interface.cpp
    • panther_hardware_interfaces/src/panther_system.cpp
    • panther_hardware_interfaces/src/panther_system_e_stop.cpp
    • panther_hardware_interfaces/src/gpio_controller.cpp
    • panther_controller/launch/controller.launch.py

Please ensure all instances of the old topic names are updated to the new ones.

Analysis chain

Line range hint 85-93:
LGTM! But verify the usage of new topic names.

The publishers are correctly initialized with appropriate QoS settings and topic names.

However, ensure that all instances of the old topic names are updated to the new ones.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all instances of old topic names have been updated to the new ones.

# Test: Search for the old topic names. Expect: No occurrences of the old topic names.
rg --type cpp $'motor_controllers_state\|io_state\|e_stop'

Length of output: 59


Script:

#!/bin/bash
# Description: Verify all instances of old topic names have been updated to the new ones.

# Test: Search for the old topic names across all file types and include variations.
rg 'motor_controllers_state'
rg 'io_state'
rg 'e_stop'

Length of output: 26916

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 6b5c645 and 9be5a57.

Files selected for processing (55)
  • .github/workflows/release-repository.yaml (1 hunks)
  • panther_battery/README.md (3 hunks)
  • panther_battery/include/panther_battery/battery_node.hpp (1 hunks)
  • panther_battery/launch/battery.launch.py (2 hunks)
  • panther_battery/src/battery_node.cpp (3 hunks)
  • panther_battery/src/dual_battery_publisher.cpp (1 hunks)
  • panther_battery/src/main.cpp (1 hunks)
  • panther_battery/src/single_battery_publisher.cpp (1 hunks)
  • panther_battery/test/include/test_battery_node.hpp (3 hunks)
  • panther_battery/test/test_dual_battery_publisher.cpp (1 hunks)
  • panther_battery/test/test_single_battery_publisher.cpp (1 hunks)
  • panther_bringup/README.md (4 hunks)
  • panther_controller/README.md (5 hunks)
  • panther_controller/config/WH01_controller.yaml (2 hunks)
  • panther_controller/config/WH02_controller.yaml (2 hunks)
  • panther_controller/config/WH04_controller.yaml (2 hunks)
  • panther_controller/launch/controller.launch.py (5 hunks)
  • panther_description/urdf/gazebo.urdf.xacro (1 hunks)
  • panther_diagnostics/README.md (2 hunks)
  • panther_diagnostics/launch/system_status.launch.py (1 hunks)
  • panther_diagnostics/src/main.cpp (1 hunks)
  • panther_gazebo/README.md (1 hunks)
  • panther_gazebo/config/gz_bridge.yaml (2 hunks)
  • panther_gazebo/config/led_strips.yaml (2 hunks)
  • panther_hardware_interfaces/README.md (1 hunks)
  • panther_hardware_interfaces/src/panther_system.cpp (1 hunks)
  • panther_hardware_interfaces/src/panther_system_ros_interface.cpp (1 hunks)
  • panther_hardware_interfaces/test/include/test_constants.hpp (1 hunks)
  • panther_hardware_interfaces/test/test_panther_system_ros_interface.cpp (4 hunks)
  • panther_lights/README.md (3 hunks)
  • panther_lights/launch/lights.launch.py (2 hunks)
  • panther_lights/src/controller_node.cpp (2 hunks)
  • panther_lights/src/controller_node_main.cpp (1 hunks)
  • panther_lights/src/driver_node.cpp (2 hunks)
  • panther_lights/src/driver_node_main.cpp (1 hunks)
  • panther_lights/test/test_controller_node.cpp (1 hunks)
  • panther_lights/test/test_driver_node.cpp (1 hunks)
  • panther_localization/README.md (2 hunks)
  • panther_localization/config/enu_localization.yaml (1 hunks)
  • panther_localization/config/enu_localization_with_gps.yaml (1 hunks)
  • panther_localization/config/relative_localization.yaml (1 hunks)
  • panther_localization/config/relative_localization_with_gps.yaml (1 hunks)
  • panther_localization/launch/localization.launch.py (2 hunks)
  • panther_manager/README.md (5 hunks)
  • panther_manager/behavior_trees/lights.xml (5 hunks)
  • panther_manager/config/lights_manager_config.yaml (1 hunks)
  • panther_manager/config/safety_manager_config.yaml (1 hunks)
  • panther_manager/launch/manager_bt.launch.py (2 hunks)
  • panther_manager/src/lights_manager_node.cpp (1 hunks)
  • panther_manager/src/lights_manager_node_main.cpp (1 hunks)
  • panther_manager/src/safety_manager_node.cpp (1 hunks)
  • panther_manager/src/safety_manager_node_main.cpp (1 hunks)
  • panther_manager/test/test_lights_behavior_tree.cpp (1 hunks)
  • panther_manager/test/test_lights_manager_node.cpp (2 hunks)
  • panther_manager/test/test_safety_behavior_tree.cpp (1 hunks)
Files not processed due to max files limit (1)
  • panther_manager/test/test_safety_manager_node.cpp
Files skipped from review due to trivial changes (15)
  • .github/workflows/release-repository.yaml
  • panther_battery/src/main.cpp
  • panther_diagnostics/src/main.cpp
  • panther_gazebo/README.md
  • panther_gazebo/config/led_strips.yaml
  • panther_lights/README.md
  • panther_lights/src/controller_node_main.cpp
  • panther_lights/src/driver_node_main.cpp
  • panther_localization/config/enu_localization_with_gps.yaml
  • panther_localization/config/relative_localization.yaml
  • panther_localization/config/relative_localization_with_gps.yaml
  • panther_manager/config/lights_manager_config.yaml
  • panther_manager/launch/manager_bt.launch.py
  • panther_manager/src/lights_manager_node_main.cpp
  • panther_manager/src/safety_manager_node_main.cpp
Additional context used
Learnings (2)
panther_hardware_interfaces/src/panther_system_ros_interface.cpp (1)
Learnt from: pawelirh
PR: husarion/panther_ros#343
File: panther_hardware_interfaces/src/panther_system_ros_interface.cpp:229-253
Timestamp: 2024-06-12T16:04:28.211Z
Learning: When creating GitHub issues based on user requests, ensure to include a detailed description that provides context and specifies the missing elements or required changes, as demonstrated in the issue creation for handling all GPIOPin enum cases in UpdateIOStateMsg.
panther_manager/README.md (1)
Learnt from: pawelirh
PR: husarion/panther_ros#301
File: panther_manager/src/lights_manager_node.cpp:77-96
Timestamp: 2024-05-27T07:37:14.213Z
Learning: The parameters for the `panther_manager` module are documented in the README file located at `panther_ros/panther_manager/README.md`.
LanguageTool
panther_diagnostics/README.md

[uncategorized] ~42-~42: You might be missing the article “the” here.
Context: ...lt: build_in_computer]: Frame where computer is located. - ~publish_rate [double...

(AI_EN_LECTOR_MISSING_DETERMINER_THE)

panther_bringup/README.md

[uncategorized] ~31-~31: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...o a YAML file with a description of the user defined animations. - wheel_config_path [*str...

(EN_COMPOUND_ADJECTIVE_INTERNAL)

panther_hardware_interfaces/README.md

[uncategorized] ~38-~38: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ...nther_msgs/DriverState*]: Current motor controllers state and error flags. [//]: # (ROS_AP...

(AI_HYDRA_LEO_APOSTROPHE_S_XS)

Additional comments not posted (80)
panther_gazebo/config/gz_bridge.yaml (3)

Line range hint 9-13:
LGTM!

The updated topic names for battery status follow the new naming conventions and the mappings are correct.


20-23: LGTM!

The updated topic names for lights channel 1 follow the new naming conventions and the mappings are correct.


26-29: LGTM!

The updated topic names for lights channel 2 follow the new naming conventions and the mappings are correct.

panther_diagnostics/launch/system_status.launch.py (1)

34-34: LGTM!

The updated node name system_monitor is consistent with the new naming conventions.

panther_battery/launch/battery.launch.py (2)

33-36: LGTM!

The updated node name battery_driver is consistent with the new naming conventions.


45-45: LGTM!

The launch configuration for battery_driver_node is correct and consistent with the new naming conventions.

panther_diagnostics/README.md (2)

20-20: Node name change approved.

The node name has been updated to system_monitor for better consistency.


33-34: Topic names change approved.

The topic names have been updated for better clarity and consistency.

panther_battery/include/panther_battery/battery_node.hpp (1)

46-46: Function signature change approved.

The function signature of GetADCDevicePath has been updated for better clarity and consistency.

panther_battery/test/test_single_battery_publisher.cpp (2)

59-60: Topic name change approved.

The topic name has been updated to battery/battery_status for better clarity and consistency.


62-63: Topic name change approved.

The topic name has been updated to _battery/battery_1_status_raw for better clarity and consistency.

panther_lights/launch/lights.launch.py (2)

64-64: Node name change approved.

The node name has been updated to lights_driver for better consistency.


75-75: Node name change approved.

The node name has been updated to lights_controller for better consistency.

panther_controller/config/WH01_controller.yaml (2)

8-8: Approved: Addition of drive_controller settings

The addition of drive_controller settings aligns with the PR objectives and seems appropriate for a differential drive controller.


Line range hint 27-63:
Approved: Detailed configuration for drive_controller

The detailed configuration settings for the drive_controller are well-defined and consistent with the objectives of the PR.

panther_controller/config/WH04_controller.yaml (2)

8-8: Approved: Addition of drive_controller settings

The addition of drive_controller settings aligns with the PR objectives and seems appropriate for a differential drive controller.


Line range hint 27-63:
Approved: Detailed configuration for drive_controller

The detailed configuration settings for the drive_controller are well-defined and consistent with the objectives of the PR.

panther_battery/README.md (4)

20-20: Approved: Node name changed to battery_driver

The change of the node name from battery_node to battery_driver enhances clarity and consistency.


33-36: Approved: Updated published topic names

The updated topic names enhance clarity and consistency in the ROS ecosystem.


45-46: Approved: Updated subscribed topic names

The updated subscribed topic names enhance clarity and consistency in the ROS ecosystem.


56-56: Approved: Added new parameter for ADC device path

The addition of the new parameter for ADC device path enhances the configuration options for the battery_driver.

panther_hardware_interfaces/test/include/test_constants.hpp (1)

95-95: Approved: Updated topic name for motor_controllers_state

The updated topic name enhances clarity and consistency in the ROS ecosystem.

panther_localization/README.md (1)

23-23: LGTM! Verify consistency.

The renaming of ekf_node to ekf_filter is consistent with the PR objectives. Ensure this new name is used consistently throughout the file and the codebase.

panther_controller/config/WH02_controller.yaml (1)

8-8: LGTM! Verify consistency.

The renaming of panther_base_controller to drive_controller is consistent with the PR objectives. Ensure this new name is used consistently throughout the file and the codebase.

Verification successful

Renaming verified and consistent.

The renaming of panther_base_controller to drive_controller has been applied consistently across the codebase. No occurrences of the old name were found.

  • panther_controller/README.md
  • panther_description/urdf/gazebo.urdf.xacro
  • panther_controller/package.xml
  • panther_controller/launch/controller.launch.py
  • panther_controller/config/WH01_controller.yaml
  • panther_controller/config/WH04_controller.yaml
  • panther_controller/config/WH02_controller.yaml
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the consistent usage of the new controller name `drive_controller`.

# Test: Search for the old controller name. Expect: No occurrences.
rg --type yaml $'panther_base_controller'

Length of output: 40


Script:

#!/bin/bash
# Description: Verify the consistent usage of the new controller name `drive_controller` and ensure the old name `panther_base_controller` is not present.

# Test: Search for the old controller name. Expect: No occurrences.
rg 'panther_base_controller'

# Test: Search for the new controller name. Expect: Occurrences in relevant files.
rg 'drive_controller'

Length of output: 3108

panther_controller/README.md (6)

18-18: Approved: Node name updated to drive_controller.

The node name change aligns with the PR objective of renaming nodes for better clarity and consistency.


51-51: Approved: Publisher imu/data added.

The addition of this publisher aligns with the documentation style and provides necessary information.


79-80: Approved: Publishers dynamic_joint_states and joint_states added.

The addition of these publishers aligns with the documentation style and provides necessary information.


92-92: Approved: Node name updated to drive_controller.

The node name change aligns with the PR objective of renaming nodes for better clarity and consistency.


108-108: Approved: Subscriber cmd_vel added.

The addition of this subscriber aligns with the documentation style and provides necessary information.


116-117: Approved: Publishers /tf and odometry/wheels added.

The addition of these publishers aligns with the documentation style and provides necessary information.

panther_description/urdf/gazebo.urdf.xacro (2)

46-46: Approved: Topic name updated to cmd_vel.

The topic name change aligns with the PR objective of renaming topics for better clarity and consistency.


47-47: Approved: Topic name updated to odometry/wheels.

The topic name change aligns with the PR objective of renaming topics for better clarity and consistency.

panther_manager/behavior_trees/lights.xml (8)

12-12: Approved: Service name updated to lights/set_animation.

The service name change aligns with the PR objective of renaming services for better clarity and consistency.


22-22: Approved: Service name updated to lights/set_animation.

The service name change aligns with the PR objective of renaming services for better clarity and consistency.


33-33: Approved: Service name updated to lights/set_animation.

The service name change aligns with the PR objective of renaming services for better clarity and consistency.


44-44: Approved: Service name updated to lights/set_animation.

The service name change aligns with the PR objective of renaming services for better clarity and consistency.


51-51: Approved: Service name updated to lights/set_animation.

The service name change aligns with the PR objective of renaming services for better clarity and consistency.


62-62: Approved: Service name updated to lights/set_animation.

The service name change aligns with the PR objective of renaming services for better clarity and consistency.


70-70: Approved: Service name updated to lights/set_animation.

The service name change aligns with the PR objective of renaming services for better clarity and consistency.


78-78: Approved: Service name updated to lights/set_animation.

The service name change aligns with the PR objective of renaming services for better clarity and consistency.

panther_manager/test/test_lights_manager_node.cpp (2)

96-96: Approved: Node name updated to test_lights_manager.

The node name change aligns with the PR objective of renaming nodes for better clarity and consistency.


149-150: Approved: Topic name updated to battery/battery_status.

The topic name change aligns with the PR objective of renaming topics for better clarity and consistency.

panther_lights/test/test_driver_node.cpp (2)

54-54: Initialize the node with the new name.

The node name has been updated to test_lights_driver to reflect the new naming conventions.


56-60: Update topic names for publishers and clients.

The topic names for channel_1_pub_, channel_2_pub_, and set_brightness_client_ have been updated to match the new naming conventions.

panther_battery/test/include/test_battery_node.hpp (3)

50-52: Add constants for ADC device names.

Constants for ADC device names kADCDevice0 and kADCDevice1 have been added.


72-77: Update parameter names in the constructor.

The parameter names for ADC devices and paths have been updated to match the new naming conventions.


107-122: Update node name and topic names in the constructor.

The node name has been updated to battery_driver and topic names for subscriptions and publishers have been updated to match the new naming conventions.

panther_localization/config/enu_localization.yaml (1)

2-2: Update node name to ekf_filter.

The node name has been updated to ekf_filter to reflect the new naming conventions.

panther_battery/src/battery_node.cpp (3)

83-83: Update parameter name for ADC path.

The parameter name for the ADC path has been updated to adc/path to match the new naming conventions.


147-147: Update topic name for driver state subscription.

The topic name for the driver state subscription has been updated to hardware/motor_controllers_state to match the new naming conventions.


169-169: Update parameter name for ADC path.

The parameter name for the ADC path has been updated to adc/path to match the new naming conventions.

panther_lights/src/driver_node.cpp (2)

66-66: Verify topic name change.

Ensure that all instances of the old topic name lights/driver/set/brightness are updated to lights/set_brightness throughout the codebase.

Verification successful

Topic name change verified.

The old topic name lights/driver/set/brightness has been successfully updated to lights/set_brightness throughout the codebase. The new topic name is consistently used in the relevant files.

  • panther_lights/README.md
  • panther_lights/test/test_driver_node.cpp
  • panther_lights/src/driver_node.cpp
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all instances of the old topic name are updated.

# Test: Search for the old topic name. Expect: No occurrences.
rg --type cpp 'lights/driver/set/brightness'

Length of output: 44


Script:

#!/bin/bash
# Description: Verify all instances of the old topic name are updated and the new topic name is used consistently.

# Test 1: Search for the old topic name. Expect: No occurrences.
rg 'lights/driver/set/brightness'

# Test 2: Search for the new topic name. Expect: Matches found.
rg 'lights/set_brightness'

Length of output: 480


80-87: Verify topic name changes.

Ensure that all instances of the old topic names lights/driver/channel_1_frame and lights/driver/channel_2_frame are updated to lights/channel_1_frame and lights/channel_2_frame throughout the codebase.

Verification successful

Verification successful: Topic names updated correctly.

The new topic names lights/channel_1_frame and lights/channel_2_frame are consistently used in the relevant source and test files.

  • panther_lights/src/driver_node.cpp
  • panther_lights/test/test_driver_node.cpp
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all instances of the old topic names are updated.

# Test: Search for the old topic names. Expect: No occurrences.
rg --type cpp 'lights/driver/channel_1_frame'
rg --type cpp 'lights/driver/channel_2_frame'

Length of output: 91


Script:

#!/bin/bash
# Description: Verify all instances of the new topic names are consistently used.

# Test: Search for the new topic names. Expect: Occurrences in relevant files.
rg --type cpp 'lights/channel_1_frame'
rg --type cpp 'lights/channel_2_frame'

Length of output: 605

panther_bringup/README.md (1)

85-85: Verify service server name change.

Ensure that all instances of the old service server name ~/ekf_node/set_pose are updated to localization/set_pose throughout the codebase.

Verification successful

Verification successful.

The service server name has been consistently updated from ~/ekf_node/set_pose to localization/set_pose throughout the codebase.

  • panther_localization/launch/localization.launch.py
  • panther_bringup/README.md
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all instances of the old service server name are updated.

# Test: Search for the old service server name. Expect: No occurrences.
rg --type md '~/ekf_node/set_pose'

Length of output: 36


Script:

#!/bin/bash
# Description: Verify all instances of the old service server name are updated.

# Test: Search for the old service server name. Expect: No occurrences.
rg '~/ekf_node/set_pose'

Length of output: 26


Script:

#!/bin/bash
# Description: Verify all instances of the new service server name are correctly implemented.

# Test: Search for the new service server name. Expect: Some occurrences.
rg 'localization/set_pose'

Length of output: 554

panther_manager/test/test_lights_behavior_tree.cpp (2)

100-101: Verify topic name change.

Ensure that all instances of the old topic name battery are updated to battery/battery_status throughout the codebase.

Verification successful

Verify topic name change.

All instances of the topic name battery/battery_status have been correctly updated throughout the codebase.

  • Instances of the topic name battery/battery_status were found and appear to be correctly updated.
  • Other occurrences of the term battery are related to variables, parameters, or other contexts and do not require changes.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all instances of the old topic name are updated.

# Test: Search for the old topic name. Expect: No occurrences.
rg --type cpp 'battery'

Length of output: 98477


100-103: Verify topic name changes.

Ensure that all instances of the old topic names battery and lights/controller/set/animation are updated to battery/battery_status and lights/set_animation throughout the codebase.

panther_manager/src/lights_manager_node.cpp (2)

66-66: Verify topic name changes.

Ensure that all instances of the old topic names battery and panther_base_controller/e_stop are updated to battery/battery_status and hardware/e_stop throughout the codebase.

Verification successful

Verification successful.

All instances of the old topic names battery and panther_base_controller/e_stop have been correctly updated to battery/battery_status and hardware/e_stop throughout the codebase.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all instances of the old topic names are updated.

# Test: Search for the old topic names. Expect: No occurrences.
rg --type cpp 'battery'
rg --type cpp 'panther_base_controller/e_stop'

Length of output: 98524


Script:

#!/bin/bash
# Description: Verify all instances of the old topic names are updated.

# Test: Search for the old topic names specifically in the context of topic subscriptions and publishers. Expect: No occurrences.
rg --type cpp 'create_subscription<[^>]+>\s*\(\s*"battery"' -l
rg --type cpp 'create_publisher<[^>]+>\s*\(\s*"battery"' -l
rg --type cpp 'create_subscription<[^>]+>\s*\(\s*"panther_base_controller/e_stop"' -l
rg --type cpp 'create_publisher<[^>]+>\s*\(\s*"panther_base_controller/e_stop"' -l

Length of output: 299


66-66: Verify topic name change.

Ensure that all instances of the old topic name battery are updated to battery/battery_status throughout the codebase.

Verification successful

Verification successful.

All instances of the old topic name battery have been correctly updated to battery/battery_status in the context of topic subscriptions.

  • No instances of create_subscription with the old topic name battery were found.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all instances of the old topic name are updated.

# Test: Search for the old topic name. Expect: No occurrences.
rg --type cpp 'battery'

Length of output: 98477


Script:

#!/bin/bash
# Description: Verify all instances of the old topic name in the context of topic subscriptions are updated.

# Test: Search for the old topic name in the context of topic subscriptions. Expect: No occurrences.
rg --type cpp 'create_subscription<.*>("battery",'

Length of output: 161


Script:

#!/bin/bash
# Description: Verify all instances of the old topic name in the context of topic subscriptions are updated.

# Test: Search for the old topic name in the context of topic subscriptions. Expect: No occurrences.
rg --type cpp 'create_subscription<[^>]*>\([^,]*,\s*"battery",'

Length of output: 65

panther_lights/test/test_controller_node.cpp (1)

104-105: LGTM! But verify the usage of new service name.

The service client is correctly initialized with the appropriate service name.

However, ensure that all instances of the old service name are updated to the new one.

Verification successful

Verified: The new service name lights/set_animation is used consistently throughout the codebase.

  • panther_manager/test/test_lights_behavior_tree.cpp
  • panther_lights/src/controller_node.cpp
  • panther_lights/test/test_controller_node.cpp
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all instances of the old service name have been updated to the new one.

# Test: Search for the old service name. Expect: No occurrences of the old service name.
rg --type cpp $'controller/set/animation'

Length of output: 40


Script:

#!/bin/bash
# Description: Verify all instances of the new service name.
# Test: Search for the new service name. Expect: Occurrences of the new service name.

rg --type cpp $'lights/set_animation'

Length of output: 456

panther_hardware_interfaces/test/test_panther_system_ros_interface.cpp (2)

46-46: LGTM!

The PantherSystemRosInterface instance is correctly initialized with the new node name hardware_controller.


64-64: LGTM!

The tests correctly validate the creation and activation of the PantherSystemRosInterface instance with the new node name hardware_controller.

Also applies to: 106-106, 123-123

panther_battery/src/dual_battery_publisher.cpp (1)

42-45: LGTM! But verify the usage of new topic names.

The publishers are correctly initialized with appropriate QoS settings and topic names.

However, ensure that all instances of the old topic names are updated to the new ones.

panther_controller/launch/controller.launch.py (3)

186-186: Ensure Consistency with Renamed Nodes.

The topic drive_controller/odom has been remapped to odometry/wheels. Ensure that this change is consistent with the rest of the codebase and documentation.

Verification successful

Remapping Consistency Verified.

The topic drive_controller/odom has been consistently remapped to odometry/wheels across the codebase. No further references to drive_controller/odom were found without the remapping.

  • panther_controller/launch/controller.launch.py
  • panther_description/urdf/gazebo.urdf.xacro
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify the consistency of remapped topic `odometry/wheels` across the codebase.
rg --type py $'odometry/wheels'

Length of output: 135


Script:

#!/bin/bash
# Verify the consistency of remapped topic `odometry/wheels` across the entire codebase.
rg 'odometry/wheels'

Length of output: 922


Script:

#!/bin/bash
# Verify if the original topic `drive_controller/odom` has been replaced or remapped consistently across the entire codebase.
rg 'drive_controller/odom'

Length of output: 246


185-185: Ensure Consistency with Renamed Nodes.

The topic drive_controller/cmd_vel_unstamped has been remapped to cmd_vel. Ensure that this change is consistent with the rest of the codebase and documentation.

Verification successful

Consistency with Renamed Nodes Verified.

The topic drive_controller/cmd_vel_unstamped has been consistently remapped to cmd_vel across the codebase.

  • panther_description/urdf/gazebo.urdf.xacro
  • panther_controller/launch/controller.launch.py
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify the consistency of remapped topic `cmd_vel` across the codebase.
rg --type py $'cmd_vel'

Length of output: 132


Script:

#!/bin/bash
# Search for the original topic `drive_controller/cmd_vel_unstamped` across the entire codebase to ensure consistency.
rg 'drive_controller/cmd_vel_unstamped'

Length of output: 269


187-187: Ensure Consistency with Renamed Nodes.

The topic drive_controller/transition_event has been remapped to _drive_controller/transition_event. Ensure that this change is consistent with the rest of the codebase and documentation.

Verification successful

Remapping Consistency Verified.

The topic drive_controller/transition_event has been consistently remapped to _drive_controller/transition_event across the codebase.

  • panther_controller/launch/controller.launch.py: Remapping line confirmed.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify the consistency of remapped topic `_drive_controller/transition_event` across the codebase.
rg --type py $'_drive_controller/transition_event'

Length of output: 185


Script:

#!/bin/bash
# Search for the original topic `drive_controller/transition_event` across the codebase to ensure all references have been updated.
rg --type py $'drive_controller/transition_event'

Length of output: 184


Script:

#!/bin/bash
# Search for the original topic `drive_controller/transition_event` across all file types to ensure all references have been updated.
rg $'drive_controller/transition_event'

Length of output: 174

panther_manager/src/safety_manager_node.cpp (2)

87-87: Ensure Consistency with Renamed Nodes.

The topic battery/battery_status is used for the battery state subscription. Ensure that this change is consistent with the rest of the codebase and documentation.

Verification successful

Consistent Usage of battery/battery_status Verified.

The topic battery/battery_status is consistently used across the codebase in both the panther_manager and panther_battery directories.

  • panther_manager/src/safety_manager_node.cpp
  • panther_manager/src/lights_manager_node.cpp
  • panther_battery/src/single_battery_publisher.cpp
  • panther_battery/src/dual_battery_publisher.cpp
  • Various test files
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify the consistency of renamed topic `battery/battery_status` across the codebase.
rg --type cpp $'battery/battery_status'

Length of output: 1154


89-89: Ensure Consistency with Renamed Nodes.

The topic hardware/motor_controllers_state is used for the driver state subscription. Ensure that this change is consistent with the rest of the codebase and documentation.

Verification successful

Consistency Verified with Renamed Nodes.

The topic hardware/motor_controllers_state is consistently used across the codebase, including in the relevant C++ files, documentation, and launch files.

  • panther_manager/src/safety_manager_node.cpp
  • panther_manager/test/test_safety_manager_node.cpp
  • panther_battery/test/include/test_battery_node.hpp
  • panther_battery/src/battery_node.cpp
  • panther_manager/README.md
  • panther_hardware_interfaces/README.md
  • panther_controller/launch/controller.launch.py
  • panther_battery/README.md
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify the consistency of renamed topic `hardware/motor_controllers_state` across the codebase.
rg --type cpp $'hardware/motor_controllers_state'

Length of output: 500


Script:

#!/bin/bash
# Verify the consistency of renamed topic `hardware/motor_controllers_state` across the entire codebase, including documentation.
rg 'hardware/motor_controllers_state'

Length of output: 1103

panther_manager/test/test_safety_behavior_tree.cpp (1)

111-111: Ensure Consistency with Renamed Nodes.

The topic hardware/fan_enable is used for the fan service. Ensure that this change is consistent with the rest of the codebase and documentation.

panther_lights/src/controller_node.cpp (2)

99-100: Ensure Consistency with Renamed Nodes.

The topic lights/channel_<channel>_frame is used for publishing panel frames. Ensure that this change is consistent with the rest of the codebase and documentation.

Verification successful

Ensure Consistency with Renamed Nodes.

The topic lights/channel_<channel>_frame is consistently used across the codebase in the following files:

  • panther_lights/src/driver_node.cpp
  • panther_lights/test/test_driver_node.cpp
  • panther_lights/src/controller_node.cpp

The usages appear to be consistent with the intended renaming.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify the consistency of renamed topic `lights/channel_<channel>_frame` across the codebase.
rg --type cpp $'lights/channel_'

Length of output: 699


74-74: Ensure Consistency with Renamed Nodes.

The service name lights/set_animation is used for setting LED animations. Ensure that this change is consistent with the rest of the codebase and documentation.

Verification successful

Service name lights/set_animation is consistently used across the codebase.

The service name appears consistently in both code and documentation files:

  • panther_manager/README.md
  • panther_manager/test/test_lights_behavior_tree.cpp
  • panther_manager/behavior_trees/lights.xml
  • panther_lights/README.md
  • panther_lights/test/test_controller_node.cpp
  • panther_lights/src/controller_node.cpp
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify the consistency of renamed service `lights/set_animation` across the codebase.
rg --type cpp $'lights/set_animation'

Length of output: 456


Script:

#!/bin/bash
# Verify the consistency of renamed service `lights/set_animation` across the entire codebase, including documentation.
rg 'lights/set_animation'

Length of output: 1825

panther_battery/test/test_dual_battery_publisher.cpp (3)

114-115: Update Topic Names

The topic name has been updated to "/battery/battery_status". Ensure that this change is consistent with the rest of the codebase.


117-118: Update Topic Names

The topic name has been updated to "/_battery/battery_1_status_raw". Ensure that this change is consistent with the rest of the codebase.


120-121: Update Topic Names

The topic name has been updated to "/_battery/battery_2_status_raw". Ensure that this change is consistent with the rest of the codebase.

panther_manager/README.md (6)

20-20: Update Node Name

The node name has been updated to lights_manager. Ensure that this change is consistent with the rest of the codebase.


33-34: Update Topic Names

The topic names have been updated to battery/battery_status and hardware/e_stop. Ensure that these changes are consistent with the rest of the codebase.


42-42: Update Service Client Name

The service client name has been updated to ~/lights/set_animation. Ensure that this change is consistent with the rest of the codebase.


69-69: Update Node Name

The node name has been updated to safety_manager. Ensure that this change is consistent with the rest of the codebase.


82-86: Update Topic Names

The topic names have been updated to battery/battery_status, hardware/e_stop, hardware/io_state, hardware/motor_controllers_state, and system_status. Ensure that these changes are consistent with the rest of the codebase.


166-166: Update Topic Name

The topic name has been updated to battery/battery_status. Ensure that this change is consistent with the rest of the codebase.

panther_hardware_interfaces/src/panther_system.cpp (1)

114-114: Ensure consistency with the new naming convention.

The instantiation of PantherSystemRosInterface with the name "hardware_controller" appears to be part of the renaming for consistency.

Verification successful

Consistent usage of "hardware_controller" confirmed.

The term "hardware_controller" is consistently used across the codebase, including test files and launch configurations, indicating that the renaming has been systematically applied.

  • panther_hardware_interfaces/test/test_panther_system_ros_interface.cpp
  • panther_controller/launch/controller.launch.py
  • panther_hardware_interfaces/test/include/test_constants.hpp
  • panther_hardware_interfaces/src/panther_system.cpp
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of the new name "hardware_controller" in the codebase.

# Test: Search for the usage of "hardware_controller". Expect: Consistent usage across the codebase.
rg --type python --type cpp $'hardware_controller'

Length of output: 80


Script:

#!/bin/bash
# Description: Verify the usage of the new name "hardware_controller" in the codebase.

# Test: Search for the usage of "hardware_controller". Expect: Consistent usage across the codebase.
rg --type cpp --type py 'hardware_controller'

Length of output: 2321

panther_hardware_interfaces/README.md Outdated Show resolved Hide resolved
panther_battery/src/battery_node.cpp Show resolved Hide resolved
panther_localization/README.md Show resolved Hide resolved
panther_bringup/README.md Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

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 9be5a57 and 4cfef26.

Files selected for processing (2)
  • panther_hardware_interfaces/README.md (1 hunks)
  • panther_localization/README.md (2 hunks)
Files skipped from review as they are similar to previous changes (2)
  • panther_hardware_interfaces/README.md
  • panther_localization/README.md

panther_bringup/README.md Outdated Show resolved Hide resolved
panther_localization/README.md Outdated Show resolved Hide resolved
Co-authored-by: Dawid Kmak <73443304+KmakD@users.noreply.github.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 4cfef26 and 386afe3.

Files selected for processing (1)
  • panther_bringup/README.md (4 hunks)
Additional context used
Learnings (1)
panther_bringup/README.md (1)
Learnt from: pawelirh
PR: husarion/panther_ros#362
File: panther_localization/launch/localization.launch.py:81-91
Timestamp: 2024-07-08T06:59:47.857Z
Learning: The executable name for the `ekf_filter` node remains `ekf_node`, and the remapping rule must refer to the old name.
LanguageTool
panther_bringup/README.md

[uncategorized] ~31-~31: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...o a YAML file with a description of the user defined animations. - wheel_config_path [*str...

(EN_COMPOUND_ADJECTIVE_INTERNAL)


[uncategorized] ~34-~34: Possible missing comma found.
Context: ...ls/panther/panther-options). Example of configuration you can find [config/components.yaml](c...

(AI_HYDRA_LEO_MISSING_COMMA)

Additional comments not posted (2)
panther_bringup/README.md (2)

15-16: Update node names in README files.

The old node names battery_node and ekf_node are still present in the README files and need to be updated to battery_driver and ekf_filter respectively.

Ensure that all instances of the old node names are updated throughout the codebase.


50-50: Update node names in README files.

The old node name ekf_node is still present in the README files and needs to be updated to ekf_filter.

Ensure that all instances of the old node names are updated throughout the codebase.

panther_bringup/README.md Show resolved Hide resolved
@pawelirh pawelirh requested a review from KmakD July 8, 2024 13:34
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

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 386afe3 and a749538.

Files selected for processing (1)
  • panther_localization/README.md (2 hunks)
Files skipped from review as they are similar to previous changes (1)
  • panther_localization/README.md

@KmakD KmakD merged commit 8417b4f into ros2-devel Jul 9, 2024
@KmakD KmakD deleted the ros2-api-reorganization branch July 9, 2024 07:07
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