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.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Porting tf2_sensor_msgs in C++ (#2) #75

Merged
merged 2 commits into from
Oct 26, 2018
Merged

Porting tf2_sensor_msgs in C++ (#2) #75

merged 2 commits into from
Oct 26, 2018

Conversation

SteveMacenski
Copy link
Contributor

@SteveMacenski SteveMacenski commented Oct 25, 2018

@clalancette Lets try this again 馃挴

@tfoote tfoote added the in review Waiting for review (Kanban column) label Oct 25, 2018
@clalancette
Copy link
Contributor

Cool, here is another CI run:

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

@SteveMacenski
Copy link
Contributor Author

Mmh. I don't actually see where tf2_sensor_msgs is being built in that output. The issues appear to me to be upstream of this. Thoughts?

@clalancette
Copy link
Contributor

Mmh. I don't actually see where tf2_sensor_msgs is being built in that output. The issues appear to me to be upstream of this. Thoughts?

It's here: https://ci.ros2.org/job/ci_linux/5353/consoleFull#console-section-378 . The reason you don't see "much" being done is because this looks like a header-only library in C++, so all it is doing is installing the header file and surrounding infrastructure. The problems that are causing this to be yellow are indeed elsewhere in the codebase, so we can ignore those. Assuming the other builds finish with the same set of errors, I'll be happy to approve.

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.

Yellow CI is because of other problems in the codebase unrelated to this PR.

@SteveMacenski
Copy link
Contributor Author

Awesome, merge as you see fit

@clalancette clalancette merged commit af186c8 into ros2:ros2 Oct 26, 2018
@clalancette clalancette removed the in review Waiting for review (Kanban column) label Oct 26, 2018
@clalancette
Copy link
Contributor

Done, thanks for the contribution.

<run_depend>tf2</run_depend>
<run_depend>python_orocos_kdl</run_depend>
<depend>cmake_modules</depend>
<depend>Eigen3</depend>
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure capital Eigen3 is defined in rosdistro.

ERROR: the following packages/stacks could not have their rosdep keys resolved
to system dependencies:
tf2_sensor_msgs: Cannot locate rosdep definition for [Eigen3]

@clalancette , was this migration intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ruffsl I think you're right, I looked at rosdistro and it looks like eigen leads to eigen3 debians

Copy link
Member

Choose a reason for hiding this comment

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

See #76

Copy link
Contributor

Choose a reason for hiding this comment

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

I actually don't remember at all, sorry. However, this package is released into Bouncy, so I'm pretty sure what is currently in there works one way or another.

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.

None yet

4 participants