Skip to content

make sensor component msg qos configurable from robot #366

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

Merged
merged 2 commits into from
May 15, 2025
Merged

Conversation

yuokamoto
Copy link
Contributor

No description provided.

@yuokamoto yuokamoto requested review from arturocuma and Copilot May 8, 2025 13:38
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR makes QoS for sensor component messages configurable from the robot by removing the QoS parameter from several InitalizeWithROS2 function signatures and instead using an internal QoS property.

  • Removes the QoS parameter from InitalizeWithROS2 in overlap and pose sensor components.
  • Introduces and uses a QoS property in base sensor components and odom publisher for consistent configuration.
  • Updates the odom publisher and robot interface to apply the new QoS configuration.

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
Public/Sensors/RRROS2OverlapSensorComponent.h Removed the QoS parameter from the InitalizeWithROS2 signature.
Public/Sensors/RRROS2BaseSensorComponent.h, .cpp Added an overload that sets the QoS property and updated initialization calls to use the internal QoS value.
Public/Sensors/RRPoseSensorManager.h, .cpp Removed the QoS parameter from InitalizeWithROS2 to align with the new configuration mechanism.
Private/Tools/RRROS2OdomPublisher.cpp Changed the default QoS from KeepLast to SensorData.
Private/Robots/RRRobotROS2Interface.cpp Updated odom component configuration to use the new QoS property.
Comments suppressed due to low confidence (2)

Source/RapyutaSimulationPlugins/Public/Sensors/RRROS2BaseSensorComponent.h:210

  • It is advisable to document the change in behavior for InitalizeWithROS2 overloads, specifically noting that the QoS property now drives publisher initialization rather than a method parameter.
UPROPERTY(EditAnywhere, BlueprintReadWrite) UROS2QoS QoS = UROS2QoS::SensorData;

Source/RapyutaSimulationPlugins/Private/Tools/RRROS2OdomPublisher.cpp:12

  • The QoS default change from KeepLast to SensorData may affect odometry message delivery; please verify that this change meets the intended behavior and expected reliability requirements.
QoS = UROS2QoS::SensorData;

@yuokamoto yuokamoto requested a review from Copilot May 12, 2025 14:01
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR makes sensor component message QoS configurable from the robot by removing the explicit InQoS parameter from sensor initialization functions and instead relying on a dedicated QoS property.

  • Removed the InQoS parameter from several InitalizeWithROS2 functions
  • Updated default QoS assignments to UROS2QoS::SensorData across sensor and odom components
  • Propagated robot-level QoS (OdomQoS) to odom publishers

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Show a summary per file
File Description
RRROS2OverlapSensorComponent.h Removed the InQoS parameter from the InitalizeWithROS2 declaration.
RRROS2BaseSensorComponent.h Adjusted initializer overloads to use the class-level QoS property.
RRPoseSensorManager.h Removed the InQoS parameter from the override of InitalizeWithROS2.
RRRobotROS2Interface.h Added a QoS property (OdomQoS) for configuring odom messages.
RRROS2OdomPublisher.cpp Changed the default QoS initialization from UROS2QoS::KeepLast to SensorData.
RRROS2OverlapSensorComponent.cpp Updated the function call to the base class by removing the InQoS parameter.
RRROS2BaseSensorComponent.cpp Propagated QoS using the member variable and provided an overload accepting InQoS.
RRPoseSensorManager.cpp Updated the parent initializer call by removing the InQoS parameter.
RRBaseOdomComponent.cpp Initialized the QoS property with UROS2QoS::SensorData.
RRRobotROS2Interface.cpp Assigned the robot’s OdomQoS to the odom component’s QoS.

@yuokamoto yuokamoto merged commit 680a125 into devel May 15, 2025
@yuokamoto yuokamoto deleted the odom_qos branch May 15, 2025 12:40
github-actions bot pushed a commit that referenced this pull request May 15, 2025
* make sensor component msg qos configurable from robot

* Update RRROS2OverlapSensorComponent.h

update documentation
github-actions bot pushed a commit that referenced this pull request May 15, 2025
* make sensor component msg qos configurable from robot

* Update RRROS2OverlapSensorComponent.h

update documentation
github-actions bot pushed a commit that referenced this pull request May 15, 2025
* make sensor component msg qos configurable from robot

* Update RRROS2OverlapSensorComponent.h

update documentation
github-actions bot pushed a commit that referenced this pull request May 15, 2025
* make sensor component msg qos configurable from robot

* Update RRROS2OverlapSensorComponent.h

update documentation
github-actions bot pushed a commit that referenced this pull request May 15, 2025
* make sensor component msg qos configurable from robot

* Update RRROS2OverlapSensorComponent.h

update documentation
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