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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix SolidPrimitive.msg to contain a single Polygon #189

Merged
merged 1 commit into from
May 18, 2022

Conversation

xmfcx
Copy link
Contributor

@xmfcx xmfcx commented May 18, 2022

Follow up from #167

In my previous PR I was supposed to add a single geometry_msgs::msg::Polygon to the message but I've added an array of it by mistake 🤦‍♂️

It's even a part of humble now :(

This PR fixes it.

Signed-off-by: M. Fatih Cırıt <mfc@leodrive.ai>
@xmfcx
Copy link
Contributor Author

xmfcx commented May 18, 2022

@clalancette the ABI won't be compatible but is it possible to include this fix in humble since it came out relatively new?

The worst case scenario I will add a comment line on the old message as a back-port clarification. Something like // Only add a single polygon here.

@clalancette
Copy link
Contributor

@clalancette the ABI won't be compatible but is it possible to include this fix in humble since it came out relatively new?

Today is the last possible moment to do this. I'm very much scared of doing this, but we'll discuss it internally and see what we want to do here.

@xmfcx
Copy link
Contributor Author

xmfcx commented May 18, 2022

Today is the last possible moment to do this. I'm very much scared of doing this, but we'll discuss it internally and see what we want to do here.

I see, I am so sorry 🙇‍♂️ for making this mistake and realizing it too late. Thank you so much for helping.

@clalancette
Copy link
Contributor

CI:

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

@clalancette
Copy link
Contributor

Besides the CI, I also went through all of the packages released into Humble to see which ones could potentially be broken by this. It's actually a fairly short list of packages that use the SolidPrimitives message, the list looks to be:

  • geometric_shapes
  • moveit_core
  • moveit_ros
  • moveit_commander
  • ompl
  • moveit_msgs
  • moveit_visual_tools
  • rc_reason_clients
  • rc_reason_msgs
  • rosbridge_suite

I went through and looked at all of the uses here, and none of them seem to use the polygon or PRISM type. I also went ahead and built all of them locally with this patch applied, so I think we are OK to go forward with this. Still waiting on CI to finish, then we'll backport and release.

@audrow audrow merged commit dfb9991 into ros2:master May 18, 2022
@audrow
Copy link
Member

audrow commented May 18, 2022

@Mergifyio backport humble

mergify bot pushed a commit that referenced this pull request May 18, 2022
Signed-off-by: M. Fatih Cırıt <mfc@leodrive.ai>
(cherry picked from commit dfb9991)
@mergify
Copy link

mergify bot commented May 18, 2022

backport humble

✅ Backports have been created

clalancette pushed a commit that referenced this pull request May 19, 2022
Signed-off-by: M. Fatih Cırıt <mfc@leodrive.ai>
(cherry picked from commit dfb9991)

Co-authored-by: M. Fatih Cırıt <xmfcx@users.noreply.github.com>
@xmfcx xmfcx deleted the fix-solid-primitive-msg branch May 25, 2022 14:03
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

3 participants