search

ros-perception / vision_msgs

Algorithm-agnostic computer vision message types for ROS.
Apache License 2.0
155 stars 74 forks source link

Sync ros2 with master #32

Closed sloretz closed 4 years ago

sloretz commented 4 years ago

This brings the message definitions on the ros2 branch up to date with the kinetic-devel branch.

Kukanani commented 4 years ago

Hi Shane, thanks for the PR. This is probably all good, but just for reference - the current kinetic branch represents breaking changes compared to previous kinetic branches because of new message fields. I need to move those breaking changes to a new master branch.

If the goal is to simply update the ROS2 branch to the latest and greatest, this is the correct set of changes to pull in. But if you are trying to make the ROS2 branch sync the actual (i.e. released) kinetic functionality, then we need to be more careful.

Can you confirm which of these two is the PR's intent?

I'll see if I can get to testing this (and doing the above branch edits) over the weekend.

sloretz commented 4 years ago

I think my intent is to get the latest and greatest. I'd like to use the version of ObjectHypothesis where id is a string instead of an int.

Kukanani commented 4 years ago

Sounds good.

I reverted the breaking changes to kinetic-devel, putting them in master instead (#33), and renamed to this issue to be "sync with master" since that's now the correct description (and what you want). It seems to have broken this PR in the process, but honestly I'm not sure how.

Sorry for the extra headache, I should have done this sooner.

Kukanani commented 4 years ago

I've verified that this branch builds on a clean Dashing install w/colcon.

sloretz commented 4 years ago

It seems to have broken this PR in the process, but honestly I'm not sure how.

Should be fixed now. Tthe PR is a merge of master into ros2 plus the commit 7928732 which makes the test work in ROS 2.

Kukanani commented 4 years ago

LGTM