Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: export tf2 sensor msgs target #536

Merged
merged 2 commits into from
Jul 11, 2022
Merged

feat: export tf2 sensor msgs target #536

merged 2 commits into from
Jul 11, 2022

Conversation

wep21
Copy link
Contributor

@wep21 wep21 commented Jun 25, 2022

  • export tf2 sensor msgs target
  • use modern cmake

@wep21
Copy link
Contributor Author

wep21 commented Jun 25, 2022

@clalancette @ahcorde Could anyone review this PR?

@clalancette clalancette mentioned this pull request Jun 27, 2022
Merged
@audrow audrow changed the base branch from ros2 to rolling June 28, 2022 14:18
@clalancette
feat: export tf2 sensor msgs target
97cf0cf 
Signed-off-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>
clalancette
clalancette approved these changes Jun 30, 2022
View reviewed changes
Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

This looks good to me, thanks for the contribution! I'll run CI on it next.

@clalancette
Copy link
Contributor

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@wep21
Copy link
Contributor Author

wep21 commented Jun 30, 2022

@clalancette eProsima/Fast-DDS#2798 fixes CI?

@clalancette
Copy link
Contributor

@clalancette eProsima/Fast-DDS#2798 fixes CI?

No idea. I've been running CI all day and its been succeeding for me until now. Let's see what happens with that PR.

@wep21
Copy link
Contributor Author

wep21 commented Jun 30, 2022 •
edited
Loading

I guess eProsima/Fast-DDS#2579 (merged 3 hours ago) breaks CI and revert the change which breaks CI in eProsima/Fast-DDS#2798.

@MiguelCompany
Copy link

@wep21 @clalancette eProsima/Fast-DDS#2798 has been merged, and RDev CI has passed

@clalancette
Copy link
Contributor

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@clalancette
Copy link
Contributor

@wep21 So, unfortunately, the Eigen dependency here is causing a problem on Windows (it's always Windows). This also may cause a problem on RHEL and Windows Debug, so we'll also have to run tests there.

I think we need to use some of the workaround that we have in https://github.com/ros2/geometry2/blob/rolling/tf2_eigen_kdl/CMakeLists.txt . Can you look into it?

@wep21
Copy link
Contributor Author

wep21 commented Jun 30, 2022

@clalancette

I think we need to use some of the workaround that we have in https://github.com/ros2/geometry2/blob/rolling/tf2_eigen_kdl/CMakeLists.txt . Can you look into it?

I added the workaround for windows in 8bd1161.

@ahcorde
Copy link
Contributor

ahcorde commented Jul 5, 2022

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

ahcorde
ahcorde requested changes Jul 5, 2022
View reviewed changes
Copy link
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

Windows CI is still failing

add workaround for windows
a9bb596 
Signed-off-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>
@wep21 wep21 requested a review from ahcorde July 9, 2022 10:03
@wep21
Copy link
Contributor Author

wep21 commented Jul 9, 2022

@ahcorde I'm sorry, but I fixed the argument in a9bb596.
Could you run ci again?

@wep21 wep21 requested a review from clalancette July 9, 2022 10:20
@clalancette
Copy link
Contributor

Another run at CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@clalancette
Copy link
Contributor

Windows seems happier with this, which is good progress. Let's see what RHEL and Windows Debug have to say:

  • RHEL Build Status
  • Windows Debug Build Status

@clalancette
Copy link
Contributor

And RHEL and Windows Debug also seem happy. So I'm going to go ahead and merge; thanks @wep21!

@clalancette clalancette merged commit 2687a35 into ros2:rolling Jul 11, 2022
@wep21 wep21 deleted the export-tf2-sensor-msgs branch July 11, 2022 13:21
@wep21
Copy link
Contributor Author

wep21 commented Jul 11, 2022

@clalancette Thank you for merging this PR. Is it possible to backport this change to humble?

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

@clalancette clalancette clalancette approved these changes

@ahcorde ahcorde Awaiting requested review from ahcorde ahcorde is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@wep21 @clalancette @MiguelCompany @ahcorde