search

AprilRobotics / apriltag_ros

A ROS wrapper of the AprilTag 3 visual fiducial detector
Other
373 stars 339 forks source link

ROS2 support #76

Closed christian-rauch closed 1 year ago

christian-rauch commented 4 years ago

I created an AprilTag ROS2 node from scratch: https://github.com/christianrauch/apriltag_ros. Would you be interested in hosting the repo in the AprilRobotics group/organisation?

If we do so, it would be useful to align the message definitions and topics. The ROS2 node depends on a dedicated apriltag_msgs repo. I split the message definitions from the node so that they can be checked out independently from the node and library, e.g. for replaying logs or third party nodes. I extended the AprilTagDetection.msg with more information about the detected tag, so that it can be used by another AprilTag visualisation node.

rgreid commented 4 years ago

Hi @christian-rauch thanks for your contribution!  While I'm not part of the APRIL Robotics Lab, we do use the ROS1 apriltag_ros package in production and will move to ROS2 soon. 

Can I nudge this discussion by asking: 

  1. Have you had any community involvement/testing of this code?  (looks like all the commits are yours?)
  2. Does it provide the same features as the ROS1 apriltag_ros package?
  3. Does it adhere to ROS2 best practices etc?

For the apriltag_ros team @wxmerkt @dmalyuta:

  1. Can I suggest it's not a matter of 'if' but 'when' to support ROS2?
  2. What metric should be used to decide whether to adopt this completely new code base, vs. upgrade the existing code to ROS2?  e.g. ticking 1-3 above? 
  3. Might this be merged into a foxy branch in this repo?
  4. Thoughts about updating the existing ROS1 package to use @christian-rauch's apriltag_msgs repo? I'm leaning away from this as it's a breaking change.
christian-rauch commented 4 years ago

Have you had any community involvement/testing of this code? (looks like all the commits are yours?)

Nope :-) I rewrote the node from scratch. But I hope that will change with the move to the AprilRobotics group.

Does it provide the same features as the ROS1 apriltag_ros package?

Yes, as far as it concerns that basic features of tag detection and pose estimation. I does not include the visualisation, which is part of another node.

Does it adhere to ROS2 best practices etc?

I am not sure if there is an official guide on "best practices". The node is implemented as a "component" that can be loaded together with other nodes into the same process (in the same way was ROS1 nodelets are). This is the recommended approach.

What metric should be used to decide whether to adopt this completely new code base, vs. upgrade the existing code to ROS2? e.g. ticking 1-3 above? Might this be merged into a foxy branch in this repo?

My opinion about this is that both versions (ROS1, ROS2) should co-exist in two repos apriltag_ros1 and apriltag_ros2, using the same C library and message definitions. The code bases are too different to consider one as the fork the other.

goekce commented 2 years ago

Friendly reminder: I am using apriltags in ROS2 and would like to see it released as a package. Is there an update?

christian-rauch commented 2 years ago

I would like to see it released as a package. Is there an update?

This depends on if/when this eventually gets moved to the AprilRobotics group as a new apriltag_ros2 repo. After it has been decided where the upstream repo will live and there is sufficient interest in a bloom release, I will look into this.

lorepieri8 commented 2 years ago

Any update? As of today, and as a ROS2 user, what is the suggested way to use apriltags?
@christian-rauch

christian-rauch commented 2 years ago

Well, I suggest using https://github.com/christianrauch/apriltag_ros :-)

lorepieri8 commented 2 years ago

Thanks, I've tested it and looks great!

wep21 commented 2 years ago

I also created a PR from ros1 codebase to make it easy to merge changes which someone creates into ros1 branch. I cannot decide which one is preferable to be released as apriltag_ros (related issue), so I would like to ask the community users about it. @christian-rauch How do you think about it? cc @wxmerkt

christian-rauch commented 2 years ago

I am obviously biased in this discussion. I created the ROS2 node many years ago with the intention to have it hosted in the @AprilRobotics group, released and officially supported. From my point of view, creating a second ROS2 AprilTag node afterwards just increases the maintenance costs without benefits, and also causes confusion for users. I would have preferred if missing features would have been proposed as a PR instead of creating yet another node doing the same.

Regarding the ros2 branch... this seems to be common, but I don't think it is useful. All the functionality of the node is in the apriltag library and the node is supposed to be just a thin wrapper. Hence the node code is quite ROS1 or ROS2 (or your framework of choice) specific and there is not much that can be merged between both branches.

wep21 commented 1 year ago

@maximilianwulf I'm sorry for my long leave. I give up merging my PR to respect @christian-rauch 's initial work. Is this fine with you?