Skip to content

Conform to ros format for header field frame_id of sensor msgs#195

Merged
ahcorde merged 6 commits into
ign-sensors3from
ahcorde/3/lidar_frame_id
Mar 1, 2022
Merged

Conform to ros format for header field frame_id of sensor msgs#195
ahcorde merged 6 commits into
ign-sensors3from
ahcorde/3/lidar_frame_id

Conversation

@ahcorde
Copy link
Copy Markdown
Contributor

@ahcorde ahcorde commented Feb 18, 2022

🎉 New feature

Summary

While I was giving support for the turtlebot3 in Ignition , I found that the frame_id is generated with this format <model_name>::<link_name>::<sensor_bane>, then when we use the bridge to republish the data in the ROS network, the frame_id is modified here and it will look like <model_name>/<link_name>/<sensor_bane>, but this format is still not valid, it not matching with any frame_id in tf.

This PR overwrite the frame_id value and set it to another value which allows to make it compatible with ROS.

I don't know where is the right place to document this.

Test it

Following the steps in the other PR you can test this.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde requested a review from iche033 as a code owner February 18, 2022 15:21
@ahcorde ahcorde self-assigned this Feb 18, 2022
@github-actions github-actions Bot added the 🏯 fortress Ignition Fortress label Feb 18, 2022
@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 18, 2022

Codecov Report

Merging #195 (b2ad054) into ign-sensors3 (5d24d67) will increase coverage by 0.09%.
The diff coverage is 100.00%.

Impacted file tree graph

@@               Coverage Diff                @@
##           ign-sensors3     #195      +/-   ##
================================================
+ Coverage         77.48%   77.57%   +0.09%     
================================================
  Files                23       23              
  Lines              2367     2377      +10     
================================================
+ Hits               1834     1844      +10     
  Misses              533      533              
Impacted Files Coverage Δ
src/AirPressureSensor.cc 86.30% <100.00%> (ø)
src/AltimeterSensor.cc 88.50% <100.00%> (ø)
src/CameraSensor.cc 75.37% <100.00%> (ø)
src/DepthCameraSensor.cc 74.48% <100.00%> (ø)
src/ImuSensor.cc 90.38% <100.00%> (ø)
src/Lidar.cc 73.15% <100.00%> (ø)
src/LogicalCameraSensor.cc 90.14% <100.00%> (ø)
src/MagnetometerSensor.cc 88.63% <100.00%> (ø)
src/RgbdCameraSensor.cc 75.21% <100.00%> (ø)
src/Sensor.cc 89.78% <100.00%> (+0.80%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5d24d67...b2ad054. Read the comment docs.

@ahcorde ahcorde changed the base branch from ign-sensors6 to ign-sensors3 February 18, 2022 15:41
@ahcorde ahcorde force-pushed the ahcorde/3/lidar_frame_id branch from f6fa433 to 858c394 Compare February 18, 2022 15:43
Comment thread src/Lidar.cc Outdated
Signed-off-by: ahcorde <ahcorde@gmail.com>
…ame_id

Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde changed the title Added frame_id parameter to lidar sensor Conform to ros format for header field frame_id of sensor msgs Feb 28, 2022
Signed-off-by: ahcorde <ahcorde@gmail.com>
Comment thread include/ignition/sensors/Sensor.hh Outdated
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde requested a review from iche033 March 1, 2022 09:45
Copy link
Copy Markdown
Contributor

@iche033 iche033 left a comment

Choose a reason for hiding this comment

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

looks good to me. Not sure what's wrong with the sensors windows CI build. I just retriggered a new build

Copy link
Copy Markdown
Contributor

@iche033 iche033 left a comment

Choose a reason for hiding this comment

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

CI is green now

@ahcorde ahcorde merged commit 1eb8a63 into ign-sensors3 Mar 1, 2022
@ahcorde ahcorde deleted the ahcorde/3/lidar_frame_id branch March 1, 2022 21:33
halodluc pushed a commit to halodluc/ign-sensors that referenced this pull request Mar 3, 2022
@osrf-triage
Copy link
Copy Markdown

This pull request has been mentioned on Gazebo Community. There might be relevant details there:

https://community.gazebosim.org/t/new-ignition-releases-2022-03-25-fortress-edifice-citadel/1343/1

@doisyg
Copy link
Copy Markdown
Contributor

doisyg commented Mar 26, 2022

Is it possible to trigger a release of libignition-sensors6 to benefit from this PR in Fortress bin?

@chapulina
Copy link
Copy Markdown
Contributor

@doisyg , see #207

@doisyg
Copy link
Copy Markdown
Contributor

doisyg commented Apr 1, 2022

Hello,
Testing this PR since it is now merged and released. It works for laserscan msgs but not for pointclouds.
I believe the type ignition.msgs.PointCloudPacked is missing the frame field.

@osrf-triage
Copy link
Copy Markdown

This pull request has been mentioned on Gazebo Community. There might be relevant details there:

https://community.gazebosim.org/t/new-ignition-releases-2022-04-13-fortress-edifice/1367/1

@doisyg
Copy link
Copy Markdown
Contributor

doisyg commented Apr 17, 2022

This PR is not working for GpuLidarSensor published as ignition.msgs.PointCloudPacked
#217 and fixed here: #218

@ashBabu
Copy link
Copy Markdown

ashBabu commented Sep 14, 2023

Saw this now only. I tried <optical_frame_id>, <ignition_frame_id>, <ign_frame_id> etc but nothing worked for me in gazebo fortress, ros2 humble. #384. ignition sensors6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏯 fortress Ignition Fortress

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants