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

Add Polygon Info obtained from zbar #11

Merged
merged 7 commits into from
Apr 20, 2024

Conversation

ijnek
Copy link
Collaborator

@ijnek ijnek commented Feb 15, 2022

Use an interface zbar_ros_interfaces/msg/Symbol.msg to communicate more information about the qr code that is available in zbar.

Deprecated the "barcode" topic in favor of the "symbol" topic (I needed a different name for the topic to avoid msg type conflict). A deprecation warning shows up in the zbar_ros_node when a subscription is detected on the 'barcode' topic.

Before merging,

  • Split off humble branch (which will be used for foxy, galactic and humble)
  • Consider renaming ros2 branch to rolling, to adhere to recommendations from OpenRobotics.

@ijnek ijnek marked this pull request as draft February 15, 2022 02:32
@ijnek ijnek marked this pull request as ready for review August 20, 2022 22:43
@ijnek ijnek force-pushed the rolling-with-polygon-info branch 2 times, most recently from 0e90eda to 6667687 Compare August 20, 2022 22:51
@ijnek
Copy link
Collaborator Author

ijnek commented Aug 20, 2022

CI is failing because of an upstream issue in ament_cmake..
Will have to wait for a release of ament_cmake in all distros, and once that's done we should enable use-ros2-testing for setup-ros so we can use ament_cmake from the testing repo and not wait for a distro synchronization.

@ijnek
Copy link
Collaborator Author

ijnek commented Aug 21, 2022

@paulbovbel would you be able to review the changes in this PR, and if you agree with the changes, do you think there's a better way to deprecate the existing topic?

@paulbovbel
Copy link
Member

@ijnek sorry I missed this. You should consider deprecating the topic for the upcoming Iron release, and then removing it in rolling.

@@ -0,0 +1,36 @@
cmake_minimum_required(VERSION 3.5)
project(zbar_ros_interfaces)
Copy link
Member

Choose a reason for hiding this comment

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

Usually these packages are labeled _msgs rather than _interfaces in the ROS ecosystem.

Copy link
Collaborator Author

@ijnek ijnek May 11, 2023

Choose a reason for hiding this comment

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

My understanding is _msgs is the old ROS 1 convention, and _interfaces is the new ROS 2 convention.

Ref:
https://docs.ros.org/en/rolling/Tutorials/Beginner-Client-Libraries/Custom-ROS2-Interfaces.html
https://roboticsbackend.com/ros2-create-custom-message/ ("End with “_interfaces” so it’s clear the package is an interface package. You’ll often see packages ending with “_msgs”, but this is the old convention.")

RCLCPP_DEBUG(get_logger(), "Barcode detected with data: '%s'", symbol.data.c_str());

RCLCPP_DEBUG(
get_logger(), "Polygon around barcode has %d points", symbol_it->get_location_size());
Copy link
Member

Choose a reason for hiding this comment

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

It's a shame vision_msgs only has 'bounding box' type boundaries rather than the polygons zbar reports.

@ijnek ijnek force-pushed the rolling-with-polygon-info branch from df40d8a to 124e4ba Compare May 15, 2023 01:47
@ijnek
Copy link
Collaborator Author

ijnek commented May 15, 2023

Rebased onto ros2 branch

Signed-off-by: ijnek <kenjibrameld@gmail.com>
Fix print formatting

Signed-off-by: ijnek <kenjibrameld@gmail.com>
Signed-off-by: Kenji Brameld <kenjibrameld@gmail.com>
Signed-off-by: Kenji Brameld <kenjibrameld@gmail.com>
Signed-off-by: Kenji Brameld <kenjibrameld@gmail.com>
Signed-off-by: Kenji Brameld <kenjibrameld@gmail.com>
Signed-off-by: Kenji Brameld <kenjibrameld@gmail.com>
@ijnek ijnek force-pushed the rolling-with-polygon-info branch from 124e4ba to 233697f Compare April 20, 2024 20:31
@ijnek
Copy link
Collaborator Author

ijnek commented Apr 20, 2024

CI failure due to ros-tooling/setup-ros@v0.7 having no support for ubuntu noble, but confirmed that ci passes if using ros-tooling/setup-ros@master

@ijnek ijnek merged commit 2df8b1b into ros-drivers:ros2 Apr 20, 2024
1 check failed
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.

2 participants