Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Use urdf/model.hpp for rolling #1476

Merged

Conversation

nakul-py
Copy link
Contributor

@nakul-py nakul-py commented Jan 8, 2025

Description

  • Replaced deprecated urdf/model.h with conditional includes using urdf/model.hpp for Rolling.
  • Maintained compatibility for Humble and Jazzy using rclcpp/version.h.

Solve #1473 (review)

Copy link
Member

@saikishor saikishor left a comment

Choose a reason for hiding this comment

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

Hello @nakul-py

Please also remove these redundant lines from here :

#include "rclcpp/version.h"
#if RCLCPP_VERSION_GTE(29, 0, 0)
#include "urdf/model.hpp"
#else
#include "urdf/model.h"
#endif

Copy link

codecov bot commented Jan 8, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.81%. Comparing base (7ddaf83) to head (c602975).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1476      +/-   ##
==========================================
- Coverage   83.83%   83.81%   -0.02%     
==========================================
  Files         122      122              
  Lines       11120    11120              
  Branches      944      943       -1     
==========================================
- Hits         9322     9320       -2     
- Misses       1489     1491       +2     
  Partials      309      309              
Flag Coverage Δ
unittests 83.81% <ø> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
..._state_broadcaster/src/joint_state_broadcaster.cpp 89.24% <ø> (+0.56%) ⬆️

... and 3 files with indirect coverage changes

@nakul-py nakul-py requested a review from saikishor January 8, 2025 12:41
@nakul-py
Copy link
Contributor Author

nakul-py commented Jan 8, 2025

Hey @saikishor will you also review this pull request #1475

saikishor
saikishor previously approved these changes Jan 8, 2025
Copy link
Member

@saikishor saikishor left a comment

Choose a reason for hiding this comment

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

LGTM

@saikishor
Copy link
Member

Hey @saikishor will you also review this pull request #1475

Sure, when I get some time. I'll do it.

@saikishor saikishor changed the title Use urdf/model.hpp for rolling and urdf/model.h for jazzy or humble. Use urdf/model.hpp for rolling Jan 8, 2025
@saikishor saikishor dismissed their stale review January 9, 2025 08:43

Missed the urdf/model.h include while reviewing

Co-authored-by: Bence Magyar <bence.magyar.robotics@gmail.com>
Copy link
Contributor

@christophfroehlich christophfroehlich left a comment

Choose a reason for hiding this comment

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

Thanks for the follow-up

saikishor
saikishor previously approved these changes Jan 9, 2025
Copy link
Member

@saikishor saikishor left a comment

Choose a reason for hiding this comment

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

LGTM

@christophfroehlich christophfroehlich dismissed stale reviews from saikishor and themself via c602975 January 9, 2025 09:43
@christophfroehlich christophfroehlich merged commit 6299094 into ros-controls:master Jan 9, 2025
24 checks passed
mamueluth pushed a commit to StoglRobotics-forks/ros2_controllers that referenced this pull request Mar 14, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants