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

Change ROI message type to hri_msgs/NormalizedRegionOfInterest2D #382

Merged

Conversation

LukaJuricic
Copy link
Contributor

@LukaJuricic LukaJuricic commented Jun 27, 2023

This PR proposed to change the type of the ROI from the standard sensor_msgs/RegionOfInterest to hri_msgs/NormalizedRegionOfInterest2D.

The use of a non-standard message is offset by the following advantages:

  • checks on normalized values are image independent and more robust
  • algorithms which depend only on the faces/bodies relative position/size and not on the source image can avoid subscribing to the camera info just to obtain the source image size
  • aligned with hri_msgs/Skeleton2D which makes use of normalized coordinates too
  • NormalizedRegionOfInterest2Dis already part of hri_msgs, let's use it ;)

@severin-lemaignan
Copy link
Contributor

I approve this change.

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.

We looked at this at the ROS 2 meeting, and this seems like a reasonable change. It is also a subset of #362, so that one will have to be rebased after we merge this.

@clalancette clalancette merged commit 9650566 into ros-infrastructure:master Sep 26, 2023
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.

3 participants