Skip to content

Added components which can be used by apriltag_detector #287

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

Merged

Conversation

vmehta-humanoid
Copy link
Contributor

Follow up PR to #286 to add some components apriltag_detector uses as composable components.

No issue # to link but if you recommend I can create one.

@traversaro
Copy link
Member

Thanks! On the Windows side we probably need to add a definition of the _USE_MATH_DEFINES macro.

@vmehta-humanoid
Copy link
Contributor Author

vmehta-humanoid commented Apr 16, 2025

Interesting failure %SRC_DIR%\ros-humble-apriltag-mit\src\work\src\TagFamily.cc(107): error C3861: '__builtin_popcountll': identifier not found I don't really need apriltag-mit so I can remove it too if the fix is not straightforward for windows. What do you think @traversaro ?

@wolfv
Copy link
Member

wolfv commented Apr 16, 2025

You could probably patch windows with something like:

https://stackoverflow.com/a/24550632

#ifdef _MSC_VER
#  include <intrin.h>
#  define __builtin_popcount __popcnt
#endif

@wolfv
Copy link
Member

wolfv commented Apr 16, 2025

Although IDK if that works for popcount_ll

@traversaro
Copy link
Member

It seems there is a simple portable implementation of that function in llvm's libcxx, we may think of using it.

@traversaro
Copy link
Member

It seems there is a simple portable implementation of that function in llvm's libcxx, we may think of using it.

https://github.com/llvm/llvm-project/blob/7f4422d99115efbb770e13ccb60cf6bfc190c245/llvm/include/llvm/ADT/bit.h#L372

@traversaro
Copy link
Member

All the apriltag_detector have unconditional add_compile_options that only make sense on GCC and Clang, not on MSVC: https://github.com/search?q=repo%3Aros-misc-utilities%2Fapriltag_detector%20Wextra&type=codem as done in https://github.com/ros2/rosidl/blob/dfdff27ea6231a39333c64a796c15fd3b7c4f839/rosidl_runtime_cpp/CMakeLists.txt#L71-L73, or simply not added on Windows.

@wolfv wolfv merged commit 9967978 into RoboStack:main Apr 17, 2025
5 checks passed
@wolfv
Copy link
Member

wolfv commented Apr 17, 2025

Thanks!

@vmehta-humanoid vmehta-humanoid deleted the add_apriltag_detector_components branch April 17, 2025 16:10
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