Skip to content

Fix <ignition_frame_id> not working for GpuLidarSensor#218

Merged
ahcorde merged 3 commits into
gazebosim:ign-sensors6from
doisyg:Fix_GpuLidarSensor_frame
Apr 19, 2022
Merged

Fix <ignition_frame_id> not working for GpuLidarSensor#218
ahcorde merged 3 commits into
gazebosim:ign-sensors6from
doisyg:Fix_GpuLidarSensor_frame

Conversation

@doisyg
Copy link
Copy Markdown
Contributor

@doisyg doisyg commented Apr 17, 2022

🦟 Bug fix

Fixes #217
Additionally it seems that frame_id was not also applied for ForceTorqueSensor and SegmentationCameraSensor, so fixed too.

Summary

Checklist

  • Signed all commits for DCO
  • Added tests
  • 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.

Guillaume Doisy added 2 commits April 17, 2022 15:12
Signed-off-by: Guillaume Doisy <guillaume.doisy@wyca.fr>
Signed-off-by: Guillaume Doisy <guillaume.doisy@wyca.fr>
Comment thread src/GpuLidarSensor.cc
msgs::Convert(_now);
// Set frame_id
for (auto i = 0; i < this->dataPtr->pointMsg.mutable_header()->data_size(); ++i) {
if (this->dataPtr->pointMsg.mutable_header()->data(i).key() == "frame_id"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Style nits.

The linter is complaining here:

  /github/workspace/src/GpuLidarSensor.cc:260:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
  /github/workspace/src/GpuLidarSensor.cc:264:  Lines should be <= 80 characters long  [whitespace/line_length] [2]

Also, please put the { in a new line

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

lint is now happy but not so sure about the formating

Signed-off-by: Guillaume Doisy <guillaume.doisy@wyca.fr>
@doisyg doisyg force-pushed the Fix_GpuLidarSensor_frame branch from 6bf97e8 to ad8c09d Compare April 18, 2022 17:01
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 18, 2022

Codecov Report

Merging #218 (6bf97e8) into ign-sensors6 (cb24d95) will increase coverage by 7.44%.
The diff coverage is n/a.

❗ Current head 6bf97e8 differs from pull request most recent head ad8c09d. Consider uploading reports for the commit ad8c09d to get more accurate results

@@               Coverage Diff                @@
##           ign-sensors6     #218      +/-   ##
================================================
+ Coverage         72.55%   80.00%   +7.44%     
================================================
  Files                33        1      -32     
  Lines              3094       15    -3079     
================================================
- Hits               2245       12    -2233     
+ Misses              849        3     -846     
Impacted Files Coverage Δ
src/ForceTorqueSensor.cc
src/GpuLidarSensor.cc
src/SegmentationCameraSensor.cc
src/Manager.cc
src/AirPressureSensor.cc
src/RenderingSensor.cc
src/NavSatSensor.cc
src/PointCloudUtil.cc
src/Lidar.cc
include/ignition/sensors/ForceTorqueSensor.hh
... and 24 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 cb24d95...ad8c09d. Read the comment docs.

@iche033
Copy link
Copy Markdown
Contributor

iche033 commented Apr 18, 2022

changes look good to me. Not sure why some rendering tests on Jammy CI is failing but they do not look related.

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.

<ignition_frame_id> not working for GpuLidarSensor

6 participants