Fix usage of visibility macros

[traversaro]

While working on getting the repo to compile on Windows, I noticed some inconsistencies in the use of visibility macros in this repo:

• In some cases, the macro that was defined when building the library was different from the one then.
• In case of tricycle_steering_controller, the wrong export macro was used.

For what regards the first point, note that in general there is no need to define anything, it is possible to use the <libraryName>_EXPORTS macro automatically defined by CMake, see https://cmake.org/cmake/help/latest/prop_tgt/DEFINE_SYMBOL.html . However, this style of manually defining macros is widespread across ROS, so it is out of scope cleaning this.

• Fork the repository.
• Modify the source; please focus on the specific change you are contributing. If you also reformat all the code, it will be hard for us to focus on your change.
• Ensure local tests pass. (colcon test and pre-commit run (requires you to install pre-commit by pip3 install pre-commit)
• Commit to your fork using clear commit messages.
• Send a pull request, answering any default questions in the pull request interface.
• Pay attention to any automated CI failures reported in the pull request, and stay involved in the conversation.

[bmagyar]

Thank you :D

[traversaro]

I see the format problem, I will fix them tomorrow.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9f7e9e9) 47.71% compared to head (f5382c9) 72.41%.
Report is 27 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff             @@
##           master    #1039       +/-   ##
===========================================
+ Coverage   47.71%   72.41%   +24.69%     
===========================================
  Files          41       41               
  Lines        3871     3639      -232     
  Branches     1833     1782       -51     
===========================================
+ Hits         1847     2635      +788     
+ Misses        751      691       -60     
+ Partials     1273      313      -960     

Flag	Coverage Δ
unittests	72.41% <ø> (+24.69%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 34 files with indirect coverage changes

[saikishor]

LGTM

[christophfroehlich]

I'd be happy to hear if you have the framework compiling/running again on Windows. Do you have experience with Windows CI workflows? should be possible now with github. if you want to keep it working for windows a CI workflow would be beneficial..

[traversaro]

I'd be happy to hear if you have the framework compiling/running again on Windows. [..] if you want to keep it working for windows a CI workflow would be beneficial..

Sure! At the moment we are trying to get something to work at RoboStack/ros-humble#137, but I am not sure if we will be able to converge soon.

Do you have experience with Windows CI workflows? should be possible now with github.

At a first glance, the biggest blocker would be to understand if there is any binary distribution for ROS Rolling packages on Windows. For sure we do not have that at RoboStack, but perhaps the official binaries can be used for this, but I do not have a lot of experience with those.

[Tobias-Fischer]

Just to chime in - I am hoping to get a student to work on RoboStack this semester, and one of the items on the list for them would be to distribute rolling in RoboStack ..

[olivier-stasse]

Dear @traversaro thanks a lot for this nice addition.
Will it be possible to use pre-commit on your PR such that the pre-format CI does not comply ?
Thanks a lot in advance.

[bmagyar]

The format stage is at odds with itself locally vs on the CI.
I checked the files and the version of clang-format is pinned... yet when I do the suggested changes locally, and run precommit again, clang-format changes things back to how they are already on the PR... Any guesses?

[saikishor]

The format stage is at odds with itself locally vs on the CI. I checked the files and the version of clang-format is pinned... yet when I do the suggested changes locally, and run precommit again, clang-format changes things back to how they are already on the PR... Any guesses?

I just ran locally the command pre-commit run --all-files and it does suggest me the similar change as in the failed job

[traversaro]

Dear @traversaro thanks a lot for this nice addition. Will it be possible to use pre-commit on your PR such that the pre-format CI does not comply ? Thanks a lot in advance.

Done! I also rebased on top of latest master, let me know if you prefer that I squash the commit or no.

[christophfroehlich]

Dear @traversaro thanks a lot for this nice addition. Will it be possible to use pre-commit on your PR such that the pre-format CI does not comply ? Thanks a lot in advance.

Done! I also rebased on top of latest master, let me know if you prefer that I squash the commit or no.

No need for squash as we do that anyways for merging.

[olivier-stasse]

LGTM