Skip to content

Use SensorDataQoS for lidar subscriptions#37

Merged
benemer merged 2 commits intoPRBonn:mainfrom
ibrahimhroob:patch-1
Jul 15, 2025
Merged

Use SensorDataQoS for lidar subscriptions#37
benemer merged 2 commits intoPRBonn:mainfrom
ibrahimhroob:patch-1

Conversation

@ibrahimhroob
Copy link
Contributor

Switched from SystemDefaultsQoS to SensorDataQoS for both LaserScan and PointCloud2 subscriptions to ensure compatibility with sensor publishers using BEST_EFFORT reliability. This avoids QoS incompatibility errors when subscribing to topics.

Switched from SystemDefaultsQoS to SensorDataQoS for both LaserScan and PointCloud2 subscriptions
to ensure compatibility with sensor publishers using BEST_EFFORT reliability. This avoids QoS
incompatibility errors when subscribing to topics.
Copy link
Member

@benemer benemer left a comment

Choose a reason for hiding this comment

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

Thanks, Ibrahim!

It would probably also make sense to change that for KISS-ICP, right?

@ibrahimhroob
Copy link
Contributor Author

Hi Benedikt, For KISS-ICP, the sensor subscription QoS is set to the sensor data profile: https://github.com/PRBonn/kiss-icp/blob/75517d5d4a767e93539b0fad16cfc7ed98d1fe4f/ros/src/OdometryServer.cpp#L84C37-L84C50 Actually I got this fix from KISS-ICP

@benemer
Copy link
Member

benemer commented Jul 15, 2025

Ah nice, good catch!

@benemer benemer requested a review from saurabh1002 as a code owner July 15, 2025 08:12
Copy link
Member

@benemer benemer left a comment

Choose a reason for hiding this comment

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

CI passes now, I had to make some changes to the CI :) Looks good and thanks again!

@benemer benemer merged commit 07c2851 into PRBonn:main Jul 15, 2025
9 checks passed
@ibrahimhroob ibrahimhroob deleted the patch-1 branch July 15, 2025 08:51
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.

3 participants