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

Change pat #525

Open
wants to merge 4 commits into
base: ros2-devel
Choose a base branch
from
Open

Change pat #525

wants to merge 4 commits into from

Conversation

rafal-gorecki
Copy link
Contributor

@rafal-gorecki rafal-gorecki commented Apr 7, 2025

Description

  • Changes to use internal user PAT

Tests 🧪

Copy link
Contributor

coderabbitai bot commented Apr 7, 2025

Walkthrough

This pull request implements a series of configuration updates and framework transitions. GitHub workflows now use a different secret token for authentication. Repository dependency versions are updated, and documentation clarifies configuration details (e.g., updating plugin names from Ignition to Gazebo). Remapping configurations in launch and URDF files have been modified, and numerous Gazebo-related files have been updated to switch dependencies, include paths, plugin registrations, and environment variable names. A new parameter (stamped_control: true) is added to multiple localization configurations, and minor formatting and QoS updates have been applied in other ROS components.

Changes

File(s) Change Summary
.github/workflows/release-candidate.yaml
.github/workflows/release-project.yaml
Replaced authentication token ${{ secrets.RAFAL_ACCESS_TOKEN }} with ${{ secrets.GH_PAT }} in multiple steps; updated workflow file reference from build.yml to build.yaml for documentation rebuild.
README.md
ROS_API.md
Updated the battery_config_path description in README from Ignition to Gazebo LinearBatteryPlugin; adjusted spacing/formatting in ROS API documentation.
husarion_ugv/hardware_deps.repos
husarion_ugv/simulation_deps.repos
Updated repository version identifiers for behaviortree_ros2, husarion_controllers, ros_components_description (and husarion_gz_worlds in simulation_deps).
husarion_ugv_controller/launch/controller.launch.py
husarion_ugv_description/urdf/common/gazebo.urdf.xacro
Modified remapping of the drive_controller topic from .../cmd_vel_unstamped to .../cmd_vel; updated battery and controller plugin filenames and names from Ignition to Gazebo conventions.
husarion_ugv_gazebo/... (
CMakeLists.txt, config/gz_bridge.yaml, config/teleop_with_estop.config, hooks/setup_envs.sh.in,
include/husarion_ugv_gazebo/estop_system.hpp, include/husarion_ugv_gazebo/gui/e_stop.hpp, package.xml, plugins.xml,
src/estop_system.cpp, src/gui/e_stop.cpp, src/led_strip.cpp )
Transitioned from the Ignition framework to the Gazebo framework by updating dependencies, target libraries, plugin registration macros, include paths, environment variable names, and class inheritance/interfaces.
husarion_ugv_lights/src/lights_driver_node.cpp Replaced rmw_qos_profile_services_default with rclcpp::ServicesQoS() in the service client instantiation.
husarion_ugv_localization/config/enu_localization.yaml
husarion_ugv_localization/config/enu_localization_with_gps.yaml
husarion_ugv_localization/config/relative_localization.yaml
husarion_ugv_localization/config/relative_localization_with_gps.yaml
Added a new parameter stamped_control: true to enable timestamped control inputs.

Possibly related PRs

✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ 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>, please review it.
    • 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 gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @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 generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai plan to trigger planning for file edits and PR creation.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • 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.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

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.

@rafal-gorecki rafal-gorecki changed the base branch from ros2 to ros2-devel April 7, 2025 14:52
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

🔭 Outside diff range comments (1)
husarion_ugv_gazebo/src/led_strip.cpp (1)

98-99: ⚠️ Potential issue

Logging functions not updated from Ignition to Gazebo

Similar to the issue in e_stop.cpp, this file still uses Ignition logging functions (igndbg, ignerr) despite the migration to the Gazebo framework.

Also applies to: 150-152

🧹 Nitpick comments (3)
.github/workflows/release-candidate.yaml (1)

78-78: Consistent Formatting for Access Token:
In the "Create new branch" step, the token is specified as ${{ secrets.GH_PAT}}. For consistency and readability, consider adding a space before the closing curly braces (i.e., ${{ secrets.GH_PAT }}).

-          access_token: ${{ secrets.GH_PAT}}
+          access_token: ${{ secrets.GH_PAT }}
husarion_ugv_localization/config/enu_localization_with_gps.yaml (1)

51-53: Parameter Addition: stamped_control
The new parameter stamped_control: true has been added immediately after use_control: true which is an appropriate placement. This flag should enable timestamped control inputs and appears consistent with the overall configuration design. Please verify that any downstream components that consume this parameter are updated accordingly.

husarion_ugv_localization/config/enu_localization.yaml (1)

41-43: New Flag Integration: stamped_control
The stamped_control: true parameter has been inserted correctly into the control configuration block. This addition, placed directly after use_control: true, maintains consistency with the other localization configuration files. Consider updating the documentation or any relevant comments to reflect the behavioral change introduced by timestamped control.

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2df5588 and 9388a1a.

📒 Files selected for processing (24)
  • .github/workflows/release-candidate.yaml (2 hunks)
  • .github/workflows/release-project.yaml (3 hunks)
  • README.md (1 hunks)
  • ROS_API.md (1 hunks)
  • husarion_ugv/hardware_deps.repos (1 hunks)
  • husarion_ugv/simulation_deps.repos (1 hunks)
  • husarion_ugv_controller/launch/controller.launch.py (1 hunks)
  • husarion_ugv_description/urdf/common/gazebo.urdf.xacro (2 hunks)
  • husarion_ugv_gazebo/CMakeLists.txt (4 hunks)
  • husarion_ugv_gazebo/config/gz_bridge.yaml (1 hunks)
  • husarion_ugv_gazebo/config/teleop_with_estop.config (24 hunks)
  • husarion_ugv_gazebo/hooks/setup_envs.sh.in (1 hunks)
  • husarion_ugv_gazebo/include/husarion_ugv_gazebo/estop_system.hpp (2 hunks)
  • husarion_ugv_gazebo/include/husarion_ugv_gazebo/gui/e_stop.hpp (2 hunks)
  • husarion_ugv_gazebo/package.xml (1 hunks)
  • husarion_ugv_gazebo/plugins.xml (1 hunks)
  • husarion_ugv_gazebo/src/estop_system.cpp (6 hunks)
  • husarion_ugv_gazebo/src/gui/e_stop.cpp (2 hunks)
  • husarion_ugv_gazebo/src/led_strip.cpp (2 hunks)
  • husarion_ugv_lights/src/lights_driver_node.cpp (1 hunks)
  • husarion_ugv_localization/config/enu_localization.yaml (1 hunks)
  • husarion_ugv_localization/config/enu_localization_with_gps.yaml (1 hunks)
  • husarion_ugv_localization/config/relative_localization.yaml (1 hunks)
  • husarion_ugv_localization/config/relative_localization_with_gps.yaml (1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
husarion_ugv_gazebo/src/gui/e_stop.cpp (1)
husarion_ugv_gazebo/include/husarion_ugv_gazebo/gui/e_stop.hpp (1)
  • EStop (35-35)
⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: Run unit tests hardware build type
  • GitHub Check: Run unit tests simulation build type
🔇 Additional comments (59)
README.md (1)

86-86: Update to battery_config_path description is clear and correct.
The new description clearly indicates that the configuration file is for the Gazebo LinearBatteryPlugin and is intended for simulation only. Ensure that similar updates (if any) across the documentation are also consistent with Gazebo usage.

husarion_ugv_gazebo/hooks/setup_envs.sh.in (1)

1-2: Environment Variable Renaming is Correct and Consistent.

The updated environment variable names correctly reflect the transition from Ignition to Gazebo. Both GZ_GUI_PLUGIN_PATH and GZ_GAZEBO_SYSTEM_PLUGIN_PATH are assigned the expected value, preserving the original functionality while aligning with the updated framework naming conventions.

husarion_ugv_lights/src/lights_driver_node.cpp (1)

71-72: Improved QoS specification using modern ROS 2 API

The change from using the lower-level rmw_qos_profile_services_default to the more idiomatic rclcpp::ServicesQoS() is a good modernization. This approach is more type-safe and better expresses intent while maintaining the same QoS characteristics for service communications.

.github/workflows/release-candidate.yaml (2)

27-27: Update PAT token in Docs build step:
The GitHub token is now set to ${{ secrets.GH_PAT }}, which aligns with the new internal PAT requirements. Please verify that this updated secret is correctly configured in the repository settings.


85-85: Update PAT token for OS image build step:
The GitHub token in the "Build OS image" step is now correctly set to ${{ secrets.GH_PAT }}, ensuring that the workflow uses the proper internal PAT.

.github/workflows/release-project.yaml (3)

68-68: PAT token update for repository release trigger:
The token for triggering the release of the husarion-ugv-rpi-os-img repository is now updated to ${{ secrets.GH_PAT }}. This change ensures that authentication now leverages the internal PAT as required.


85-85: PAT token update in the OS image build step:
Here, the token is updated consistently to ${{ secrets.GH_PAT }}. Additionally, verify that the reference branch (${{ env.MAIN_BRANCH }}) is correct for your build process.


104-105: PAT token and workflow file update in documentation rebuild step:
The documentation rebuild step now uses ${{ secrets.GH_PAT }} and references the workflow file build.yaml. Ensure that these changes align with the updated documentation build process and that all dependent configurations are consistent.

husarion_ugv_localization/config/relative_localization_with_gps.yaml (1)

49-53: Consistent Parameter Update: stamped_control
The addition of stamped_control: true under the control configuration section is consistent with similar changes in related configuration files. Ensure that using odom1_relative: true (as seen in this file) is intentional and that the timestamped control behavior is correctly handled downstream.

husarion_ugv_localization/config/relative_localization.yaml (1)

41-43: Parameter Inclusion: stamped_control
The new configuration parameter stamped_control: true is now part of the ekf_filter section. Its placement appears appropriate and mirrors the changes made in other related YAML files. Make sure the system’s components that depend on time-stamped control inputs are verified to work with this new setting.

husarion_ugv_gazebo/config/teleop_with_estop.config (18)

7-8: Main Window Dimension Update
The main window’s dimensions have been updated to 1920×1080. This increase in resolution should enhance the overall GUI display on high‐resolution screens.


16-21: MinimalScene Plugin – GUI Tag and Layout Properties
The plugin’s GUI configuration has been updated from an Ignition-specific tag to <gz-gui>. In addition, the width and available width values have been set to 1501, and the boolean properties for focus (focus and activeFocus) as well as hovered have been explicitly defined. These changes promote consistency with the new Gazebo framework and improved layout control.

Also applies to: 29-30, 38-38, 48-48


90-90: EntityContextMenuPlugin – Theme Consistency
The configuration now uses <gz-gui>, and the toolbar color properties (pluginToolBarColor and pluginToolBarTextColor) have been updated to “#bbdefb” and “#111111” respectively. This update ensures a unified visual appearance across plugins.

Also applies to: 153-154


221-222: GzSceneManager Plugin – Toolbar Color Update
The toolbar color properties have been updated to “#bbdefb” and “#111111”. This simple change aligns the plugin’s appearance with the new visual guidelines.


289-290: Interactive View Control Plugin – GUI Color Theme
The change to <gz-gui> with updated toolbar color settings (“#bbdefb” and “#111111”) maintains a consistent UI theme.


357-358: CameraTracking Plugin – Updated Toolbar Color
The updated toolbar color values have been applied here as well to ensure the consistent look-and-feel across all plugins using <gz-gui>.


425-426: MarkerManager Plugin – Visual Consistency Adjustment
The toolbar color properties are now explicitly set to “#bbdefb” and “#111111”, aligning this plugin with the standardized theme.


493-494: SelectEntities Plugin – Theme Update
The updated <gz-gui> toolbar color settings have been applied, ensuring that the visual presentation conforms to the new UI standards.


629-631: VisualizationCapabilities Plugin – Consistent GUI Styling
The change to <gz-gui> along with the updated toolbar colors (“#bbdefb” and “#111111”) supports a cohesive visual appearance, matching the rest of the plugins.


643-643: WorldControl Plugin – Layout and Available Width Updates
The width property is now set to 1199, and the available width has been updated to match. These modifications should help align the plugin’s layout with the overall UI design. It’s advisable to review the anchoring and placement in case the new dimensions impact interaction.

Also applies to: 660-660


711-711: WorldStats Plugin – GUI and Position Update
The switch to <gz-gui> (line 711) together with the updated x-position (1211 on line 717) and the consistent toolbar color settings (lines 779–781) ensures that this plugin adheres to the unified visual theme.

Also applies to: 717-717, 779-781


788-788: Shapes Plugin – GUI Framework Transition
The Shapes plugin now uses <gz-gui> along with the new toolbar color settings. These changes contribute to a consistent and modernized look throughout the application.

Also applies to: 851-853


856-856: Lights Plugin – Updated GUI Configuration
By transitioning to <gz-gui> (line 856) and updating the toolbar color properties (lines 919–921), the Lights plugin now matches the fresh visual style applied across the project.

Also applies to: 919-921


924-924: TransformControl Plugin – Consistent Theme Application
The update to <gz-gui> along with the updated toolbar color values in TransformControl ensure a unified interface. It is recommended to verify that these dimension settings do not adversely affect control interactions.

Also applies to: 987-989


992-992: Screenshot Plugin – GUI and Theme Enhancements
The Screenshot plugin now uses the updated <gz-gui> configuration and consistently applies the new toolbar colors. This change supports a harmonized user experience across all plugins.

Also applies to: 1055-1057


1060-1060: CopyPaste Plugin – Visual Theme Standardization
The CopyPaste plugin has been updated to use <gz-gui> along with the new toolbar color settings, ensuring that its appearance is in line with the overall design language.

Also applies to: 1123-1125


1129-1129: EStop Plugin – GUI Framework Modernization
The EStop plugin now adopts the <gz-gui> configuration. Please verify that the provided namespace placeholder ({namespace}) is resolved correctly at runtime.


1199-1199: Teleop Plugin – Enhanced Focus and Toolbar Settings
The Teleop plugin has been updated to use <gz-gui>. Notably, the focus properties have been set to true (lines 1210–1211) and the toolbar color settings (lines 1261–1263) have been applied consistently. Ensure that enabling focus in this context does not interfere with other UI elements and that all interactive behaviors are as expected.

Also applies to: 1210-1211, 1229-1229, 1261-1263

ROS_API.md (1)

61-61: Minor formatting improvement looks good

The spacing around the navsat_transform node name has been adjusted for better consistency with other entries in the documentation.

husarion_ugv_gazebo/package.xml (1)

20-20: Dependency correctly updated to use Gazebo framework

The switch from ign_ros2_control to gz_ros2_control aligns with the project's transition from the Ignition robotics framework to the Gazebo framework. This change ensures proper integration with the updated simulation environment.

husarion_ugv_gazebo/include/husarion_ugv_gazebo/estop_system.hpp (3)

21-22: Include paths updated for Gazebo framework

The header includes have been correctly updated to reference the Gazebo framework libraries instead of Ignition.


38-39: Documentation updated to reflect framework change

The class description has been properly updated to reference the new base class from the Gazebo framework.


41-41: Class inheritance updated for Gazebo framework

The EStopSystem class now correctly inherits from gz_ros2_control::GazeboSimSystem instead of the previous Ignition-based class, maintaining the same functionality while using the updated framework.

husarion_ugv/hardware_deps.repos (1)

5-5:

✅ Verification successful

Repository dependency versions updated

The versions for behaviortree_ros2, husarion_controllers, and ros_components_description have been updated to newer commits. This ensures compatibility with the Gazebo framework transition and likely includes bug fixes or improved functionality.

Also applies to: 9-9, 13-13


🏁 Script executed:

#!/bin/bash
# Verify that the same repository versions are used in simulation_deps.repos to maintain consistency
echo "Checking for consistency between hardware_deps.repos and simulation_deps.repos..."
if [ -f "husarion_ugv/simulation_deps.repos" ]; then
  echo "Versions in hardware_deps.repos:"
  grep -A 1 "version:" husarion_ugv/hardware_deps.repos
  
  echo -e "\nVersions in simulation_deps.repos:"
  grep -A 1 "version:" husarion_ugv/simulation_deps.repos
else
  echo "simulation_deps.repos not found"
fi

Length of output: 1013


Repository dependency versions verified and consistent

I confirmed that the updated commits for behaviortree_ros2 (first listed dependency), husarion_controllers, and ros_components_description are identical in both the hardware and simulation dependencies files. This alignment ensures compatibility with the Gazebo framework transition as intended. Note that the simulation file now includes an additional dependency (husarion_gz_worlds) with commit 97275568c34e6fe191ca34fa1c1c2b897faf9610—please confirm that this addition is deliberate.

husarion_ugv/simulation_deps.repos (1)

5-5: Repository dependency versions updated for Gazebo framework transition

These changes update the repository dependencies to specific commits that are compatible with the Gazebo framework transition throughout the project. This is consistent with the broader shift from Ignition to Gazebo seen in other files.

Also applies to: 9-9, 13-13, 17-17

husarion_ugv_gazebo/config/gz_bridge.yaml (2)

6-6: Namespace updated from 'ignition.msgs' to 'gz.msgs'

The message type namespaces have been systematically updated from ignition.msgs.* to gz.msgs.* for several topics, which is consistent with the transition from Ignition to Gazebo framework.

Also applies to: 12-12, 23-23, 29-29


16-17:

❓ Verification inconclusive

Topic cmd_vel now uses TwistStamped instead of Twist

The cmd_vel topic now uses a stamped message type that includes a timestamp header. This change has corresponding updates in the controller launch file remapping and potentially in other components that use this topic.

You may want to verify that all components that publish or subscribe to cmd_vel now handle the stamped message type correctly. This is particularly important for:

  1. Any navigation components
  2. Teleop nodes
  3. Custom controllers

🏁 Script executed:

#!/bin/bash
# Find any code that might be using the old unstamped message type
echo "Searching for potential cmd_vel Twist (unstamped) subscribers or publishers"
rg -A 3 -B 3 "geometry_msgs/.+?Twist[^S]" --glob "*.{cpp,hpp,py}"
echo "Searching for explicit cmd_vel_unstamped references"
rg -A 3 -B 3 "cmd_vel_unstamped" --glob "*.{cpp,hpp,py,xml,launch}"

Length of output: 409


CMD_VEL message type update looks correct—but please verify all subscribers handle stamped messages

  • The configuration update in husarion_ugv_gazebo/config/gz_bridge.yaml now correctly uses geometry_msgs/msg/TwistStamped for the cmd_vel topic.
  • Our initial searches for outdated (unstamped) usage produced no matches, suggesting that references to the old type are not present.
  • Given the low output quality from the search, please manually verify that all components (navigation, teleop nodes, and custom controllers) are updated and correctly handle this stamped message type before finalizing the merge.
husarion_ugv_controller/launch/controller.launch.py (1)

240-240: Updated remapping for drive_controller/cmd_vel

This change correctly updates the remapping to match the transition to using stamped command velocity messages, removing the "_unstamped" suffix from the source topic name. This aligns with the changes in the bridge configuration file where the cmd_vel topic now uses TwistStamped.

husarion_ugv_gazebo/plugins.xml (1)

5-5:

✅ Verification successful

Base class updated to GazeboSimSystemInterface

The plugin base class has been updated from ign_ros2_control::IgnitionSystemInterface to gz_ros2_control::GazeboSimSystemInterface, which is necessary for compatibility with the Gazebo framework. Ensure that any implementation that inherits from this interface has been updated accordingly.


🏁 Script executed:

#!/bin/bash
# Verify that the EStopSystem implementation has been updated to use the Gazebo interface
echo "Checking the EStopSystem implementation for interface changes"
rg -A 5 "class EStopSystem" --glob "*.{cpp,hpp}"

Length of output: 821


Review Update: Base Class Interface Change Verified

  • The plugin file husarion_ugv_gazebo/plugins.xml now correctly specifies the base class as gz_ros2_control::GazeboSimSystemInterface.
  • Verification of the EStopSystem implementation in husarion_ugv_gazebo/include/husarion_ugv_gazebo/estop_system.hpp confirms it has been updated to inherit from the corresponding Gazebo class (gz_ros2_control::GazeboSimSystem), with no remnants of the old ign_ros2_control::IgnitionSystemInterface.
  • The changes are consistent and ensure compatibility with the Gazebo framework.
husarion_ugv_gazebo/include/husarion_ugv_gazebo/gui/e_stop.hpp (2)

20-20: Dependency migration from Ignition to Gazebo framework

The include path has been updated from Ignition to Gazebo (gz) namespace, which is consistent with the framework transition mentioned in the PR objectives.


29-29: Updated class inheritance to match framework change

Class inheritance has been correctly updated to use gz::gui::Plugin instead of the previous Ignition equivalent.

husarion_ugv_gazebo/src/gui/e_stop.cpp (4)

17-19: Dependency migration from Ignition to Gazebo framework

The include paths have been properly updated to their Gazebo (gz) equivalents, maintaining consistency with the framework transition.


24-24: Updated constructor initialization to match framework change

Constructor initialization has been correctly modified to use gz::gui::Plugin() instead of the previous Ignition equivalent.


102-102: Updated plugin registration macro to match framework change

Plugin registration has been correctly updated from IGNITION_ADD_PLUGIN to GZ_ADD_PLUGIN with the appropriate parameter type.


59-61:

❓ Verification inconclusive

Logging functions not updated from Ignition to Gazebo

While the framework has been migrated from Ignition to Gazebo, the logging functions (ignwarn, ignerr, ignmsg) are still using the Ignition prefix. These should be updated to their Gazebo equivalents if available.

Also applies to: 68-72, 89-89


🏁 Script executed:

#!/bin/bash
# Check for logging function definitions in Gazebo
rg --type cpp "namespace gz.*Console" -A 10 -B 10

Length of output: 51


Update Logging Functions to Gazebo Equivalents

The code in husarion_ugv_gazebo/src/gui/e_stop.cpp still uses Ignition logging macros (e.g. ignwarn) on lines 59–61 (and similarly at lines 68–72, 89). Our initial search for Gazebo logging patterns (using a lookup for namespace gz.*Console) produced no output—which is inconclusive. To further verify whether Gazebo now provides replacement logging macros (such as gzwarn, gzerr, or gzmsg), please run additional checks or manually verify if such Gazebo logging functions are available. If they do exist, the Ignition logging calls should be updated accordingly; otherwise, consider either adopting Gazebo’s logging mechanism (if available) or documenting the choice to retain the Ignition macros.

  • Affected File: husarion_ugv_gazebo/src/gui/e_stop.cpp
  • Affected Lines: 59–61, 68–72, 89

Additionally, I recommend running the following shell script to search for any usage of Gazebo logging macros:

Once you’ve confirmed the availability (or absence) of a Gazebo logging replacement, please update the code and documentation accordingly.


🏁 Script executed:

#!/bin/bash
# Search for potential Gazebo logging macros replacements
for macro in gzwarn gzerr gzmsg; do
  echo "Searching for $macro usage:"
  rg --type cpp "$macro" -A 2 -B 2 || echo "No matches found for $macro"
done

Length of output: 702


Manual Verification Required for Logging Update

Based on the verification steps:

  • A search for potential Gazebo logging macros (e.g., gzwarn, gzerr, gzmsg) returned no results, suggesting that no direct replacements exist in the current codebase.
  • The file husarion_ugv_gazebo/src/gui/e_stop.cpp (lines 59–61, 68–72, 89) still uses Ignition logging macros, which may not align with the framework migration if updated Gazebo logging functions are available.

Next Steps:

  • Verify with Official Gazebo Documentation: Confirm whether Gazebo provides updated logging macros (or an alternative logging mechanism) that should replace the Ignition macros.
  • If Updated Macros Exist: Update the logging statements accordingly.
  • If Not: Document the decision to retain the Ignition logging calls or consider integrating a custom mapping to the Gazebo logging system if desired.

Please review and consider these points carefully.

husarion_ugv_gazebo/src/led_strip.cpp (2)

34-34: Updated Model namespace to match framework change

The Model class reference has been correctly updated from gz::gazebo::Model to gz::sim::Model, which is consistent with the framework transition.


280-282: Updated plugin registration macro to match framework change

Plugin registration has been correctly updated from IGNITION_ADD_PLUGIN to GZ_ADD_PLUGIN, maintaining consistent framework usage.

husarion_ugv_description/urdf/common/gazebo.urdf.xacro (3)

13-14: Updated battery plugin filename to match framework change

The plugin filename has been correctly updated from the Ignition to Gazebo naming scheme, while preserving the plugin name which already used the gz namespace.


36-36: Updated controller plugin name and filename to match framework change

Both the plugin filename and plugin name have been correctly updated to use the Gazebo equivalents.


40-46:

❓ Verification inconclusive

Modified remappings for controller

The remapping for command velocity has been updated from cmd_vel_unstamped to cmd_vel, and several e-stop related remappings have been removed. These changes align with the updated framework's parameter handling.


🏁 Script executed:

#!/bin/bash
# Check for other references to cmd_vel_unstamped that might need updating
rg "cmd_vel_unstamped" --type-not md

Length of output: 36


Attention: Confirm Remapping Updates in URDF File

The changes in husarion_ugv_description/urdf/common/gazebo.urdf.xacro correctly update the controller remappings by replacing the outdated cmd_vel_unstamped with cmd_vel and removing the now unnecessary e-stop remappings. An automated search for "cmd_vel_unstamped" produced no results, which aligns with the intended updates. However, given the limited output from the script, please perform a manual review of the entire codebase to ensure no residual references remain.

  • File Affected: husarion_ugv_description/urdf/common/gazebo.urdf.xacro (lines 40–46)
  • Key Update: Command velocity remapping changed from cmd_vel_unstamped to cmd_vel
  • Action Required: Manually verify that no stray references to cmd_vel_unstamped or removed e-stop remappings exist in other parts of the project
husarion_ugv_gazebo/src/estop_system.cpp (8)

24-26: Switch to Gazebo includes looks correct.
These lines properly transition from Ignition to Gazebo includes for IMU messages and components. No issues spotted.


43-44: New fields for joints.
Defining gz::sim::Entity and gz_ros2_control::GazeboSimSystemInterface::ControlMethod within the jointData struct appears consistent with the rest of the Gazebo-based code.


60-65: IMU entity and callback signature updates.
Line 60 initializes sim_imu_sensors_ to gz::sim::kNullEntity, and line 62 changes the parameter type in OnIMU to gz::msgs::IMU &. Implementation in line 65 is straightforward. Ensure that all existing call sites and message bindings are updated to use the new gz::msgs::IMU type.


79-83: Renamed private class dedication.
Declaring class gz_ros2_control::GazeboSimSystemPrivate with default constructor and destructor aligns with the new Gazebo-based naming scheme. No further concerns.


90-92: Pointer to EntityComponentManager.
Storing gz::sim::EntityComponentManager *ecm in this private struct can be acceptable if the pointer is guaranteed to remain valid for the lifetime of the system. Verify that no dangling pointer scenarios arise when the plugin or simulation lifecycle changes.


135-135: Forwarding to base write method.
Calling GazeboSimSystem::write(time, period) when e_stop_active_ is false is correct for bridging custom E-stop logic with the base behavior.


141-149: Topic naming alignment.
Swapping out ~/e_stop, ~/e_stop_reset, and ~/e_stop_trigger for "hardware/e_stop", "hardware/e_stop_reset", and "hardware/e_stop_trigger" is fine so long as any external references are updated accordingly.


182-182: Updated plugin export macro.
Exporting the plugin with gz_ros2_control::GazeboSimSystemInterface is consistent with the rest of the Gazebo migration.

husarion_ugv_gazebo/CMakeLists.txt (1)

11-15:

❓ Verification inconclusive

Migration to Gazebo dependencies.
These changes replace Ignition libraries with the corresponding gz libraries (gz-sim8, gz-msgs10, gz-plugin2, gz-transport13, gz_ros2_control) and update plugin export references. Verify that the specified versions are installed and properly satisfy your environment requirements (e.g., for gz-sim8).

Also applies to: 35-35, 40-40, 50-51, 59-59, 83-83


🏁 Script executed:

#!/bin/bash
# Example commands to verify presence of the required Gazebo libraries at the correct versions:
dpkg -l | grep gz-sim8
dpkg -l | grep gz-msgs10
dpkg -l | grep gz-plugin2
dpkg -l | grep gz-transport13

Length of output: 491


Dependency Migration Verification – Manual Check Required

The migration in husarion_ugv_gazebo/CMakeLists.txt (lines 11–15 and corresponding locations) correctly replaces Ignition libraries with the updated gz libraries. However, the automated dependency check using dpkg -l did not execute because the dpkg command is unavailable in your current environment.

Action Required:

  • Please manually verify that the required Gazebo libraries (gz-sim8, gz-msgs10, gz-plugin2, gz-transport13, and gz_ros2_control) are installed in the expected versions for your setup.
  • If needed, use alternative package management commands (e.g., apt list --installed) or any environment-specific verification method to confirm installation.

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.

1 participant