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

ROS2 Port #15

Open
knzo25 opened this issue May 20, 2022 · 9 comments
Open

ROS2 Port #15

knzo25 opened this issue May 20, 2022 · 9 comments

Comments

@knzo25
Copy link

knzo25 commented May 20, 2022

Dear authors,
As part of our sensor calibration toolkit, we in Tier IV are using lidartags for camera-lidar calibration.

Having finished the main part of our development we are wondering if you would be interested in accepting a PR centering in a ROS2 port. However, in addition to the ROS2 migration, we ended up making additional changes to suit our use cases.

The main changes are:

  • ROS2 migration
  • Strong refactoring
  • Focus on detection accuracy rather than speed
  • Improved initial tag detection through modified filters and custom RANSACs.

Thanks or the great project !

@brucejk
Copy link
Member

brucejk commented May 21, 2022

Hi,

Thanks for using the repository and reaching out!

I do appreciate that you guys further developed the package for ROS2!

And definitely YES, I'd be happy to accept a PR as another branch for ROS2.

@knzo25
Copy link
Author

knzo25 commented May 24, 2022

Thanks for the disposition !

Before making the PR we would like to know which parts would you like to accept.

We first just ported the repository to ROS2 to evaluate the work that @sgk-000 was doing as part of his internship (https://github.com/tier4/LiDARTag/commits/galactic)

Afterwards to adapted the system and added some functionalities to make is suitable for our sensors, conditions, and so on, for the task of camera-lidar calibration (https://github.com/tier4/LiDARTag/tree/feature/xx1_vls128).
For this last branch, a brief summary would be:

  • Replaced the 4x line RANSAC for a rectangle estimation and a custom RANSAC.
  • Tweaked filters for improve detection rated in our data (not parameters)
  • Tweaked the system to detect tags when using lidars without intensities.
  • Fixed the case where when the SVD fails to obtain a valid rotation matrix

CC: @aohsato

@knzo25
Copy link
Author

knzo25 commented Feb 8, 2023

Hi, I would like to ask again if you are interested in accepting the changes we implemented.
I understand that you may have probably moved to other projects but it is our policy to, as much as possible, send our or developments upstream :)

@brucejk
Copy link
Member

brucejk commented Feb 13, 2023

Hi,

It has been long. Thanks for the reminder, your patience, and your persistent policy!

Since your PR would be supporting ROS 2, I would prefer you directly pushing to another branch (ROS2), which I have created, if possible.

Please let me know there are other things I could do on my end.

Thanks you all again for using this package and your hard work to push your developments upstream!

@knzo25
Copy link
Author

knzo25 commented Feb 13, 2023

Hi, thank you for your support !
I would like to make the PR later this week since we are in the middle of testing support for a new lidar that may prove to be tricky
🙇

@knzo25
Copy link
Author

knzo25 commented Mar 3, 2023

@brucejk
Sorry for the delay 🙏

I just created the related PR in #19

brucejk pushed a commit that referenced this issue Mar 3, 2023
* Added support for more lidarts and cleaned the launchers a little

Signed-off-by: Kenzo Lobos-Tsunekawa <[email protected]>

* Confirmed support for OS1-128

Signed-off-by: Kenzo Lobos-Tsunekawa <[email protected]>

* Removed duplicated file

Signed-off-by: Kenzo Lobos-Tsunekawa <[email protected]>

---------

Signed-off-by: Kenzo Lobos-Tsunekawa <[email protected]>
@brucejk
Copy link
Member

brucejk commented Mar 3, 2023

Hi,

Thanks for your PR! I just merged.

Could you also provide a simple bagfile and update the readme so users could know how to use it?

@knzo25
Copy link
Author

knzo25 commented Mar 8, 2023

Hi,
Sorry it took a while, we needed to check some privacy concerns and check which bags we are allowed to share.
We uploaded the bags themselves https://drive.google.com/drive/folders/1-9OvHH1tp0VKZ_rXEwz3OrVCgstuj8Ta
and it should be a permalink for the foreseable future.

However, while writing the documentation, I realized that some changes are needed in the msgs package too (https://github.com/UMich-BipedLab/LiDARTag_msgs). Could you create a ros2 branch also ? 🙇

@CLboyrobot
Copy link

Dear authors, As part of our sensor calibration toolkit, we in Tier IV are using lidartags for camera-lidar calibration.

Having finished the main part of our development we are wondering if you would be interested in accepting a PR centering in a ROS2 port. However, in addition to the ROS2 migration, we ended up making additional changes to suit our use cases.

The main changes are:

* ROS2 migration

* Strong refactoring

* Focus on detection accuracy rather than speed

* Improved initial tag detection through modified filters and custom RANSACs.

Thanks or the great project !

hello,could you tell me some details about the code here,(https://github.com/UMich-BipedLab/LiDARTag/blob/release/src/lidartag_decode.cc#L1778) i am very confused the value 12,could you tell me somethingabout the value 12 ,thank you very much

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants