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 support for FRI ver2.5 #85

Closed
wants to merge 2 commits into from
Closed

Add support for FRI ver2.5 #85

wants to merge 2 commits into from

Conversation

cmower
Copy link
Member

@cmower cmower commented May 7, 2023

This PR adds support for ver2.5. @mhubii I'm not sure how you want to handle this? Currently this overwrites the v1.15. So the PR is not ready to merge. Probably good to add a switch in the build somewhere so the user can specify which version they use.

I would suggest creating a macro that specifies the FRI version, then use something like

#if FRI_VERSION == 115
// v1.15 implementation
#elif FRI_VERSION == 25
// v2.5 implementation
#endif

Also, I am getting the error when building:

--- stderr: lbr_fri_ros2                                                              
CMake Error at CMakeLists.txt:19 (add_subdirectory):
  add_subdirectory not given a binary directory but the given source
  directory "/home/cm22/balgrist_ws/src/fri/fri" is not a subdirectory of
  "/home/cm22/balgrist_ws/src/lbr_fri_ros2_stack/lbr_fri_ros2".  When
  specifying an out-of-tree source a binary directory must be explicitly
  specified.


CMake Error at CMakeLists.txt:41 (target_link_libraries):
  The plain signature for target_link_libraries has already been used with
  the target "lbr_fri_ros2".  All uses of target_link_libraries with a target
  must be either all-keyword or all-plain.

  The uses of the plain signature are here:

   * /opt/ros/foxy/share/ament_cmake_target_dependencies/cmake/ament_target_dependencies.cmake:145 (target_link_libraries)



---

I think the issue is that I made the FRI package non-ament_cmake, so linking the library is not the same. Perhaps this is something you know better than me how to fix?

If you could let me know if you have any ideas re this issue above then I'll fix, and can do the macro support switch thing i mention above. However, for this I will also need to know how you want to handle the switch? Ie. where you think it should be placed?

@cmower cmower requested a review from mhubii May 7, 2023 12:18
@mhubii
Copy link
Member

mhubii commented May 9, 2023

this PR needs some re-design. Below is the (approximate) new layout

fri_repo drawio

@mhubii
Copy link
Member

mhubii commented May 19, 2023

@mhubii mhubii mentioned this pull request May 19, 2023
5 tasks
@mhubii mhubii mentioned this pull request Aug 8, 2023
@mhubii
Copy link
Member

mhubii commented Jan 29, 2024

hi @cmower , is this something we can revive? Or are you too occupied? Asking because we will be going to Zurich in April

I am only a little afraid of re-basing this PR to humble branch, not sure your contribution is tracked in that scenario. Maybe I can commit under your name?

Also refers to lbr-stack/fri#8

@mhubii mhubii mentioned this pull request Feb 23, 2024
3 tasks
@mhubii
Copy link
Member

mhubii commented Feb 23, 2024

Closing in favor of #158

@mhubii mhubii closed this Feb 23, 2024
@mhubii mhubii deleted the dev-foxy-fri-v2.5 branch May 13, 2024 17:47
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