ros-controls / ros2_control_demos

This repository aims at providing examples to illustrate ros2_control and ros2_controllers
https://control.ros.org
Apache License 2.0
412 stars 187 forks source link

Use own implementation of `stod()` #428

Closed christophfroehlich closed 8 months ago

christophfroehlich commented 9 months ago

Use own implementation of stod(), see https://github.com/ros-controls/ros2_control/pull/1257

christophfroehlich commented 9 months ago

@mergifyio backport humble iron

mergify[bot] commented 9 months ago

backport humble iron

✅ Backports have been created

* [#445 Use own implementation of `stod()` (backport #428)](https://github.com/ros-controls/ros2_control_demos/pull/445) has been created for branch `humble` * [#446 Use own implementation of `stod()` (backport #428)](https://github.com/ros-controls/ros2_control_demos/pull/446) has been created for branch `iron`
olivier-stasse commented 9 months ago

I guess that the best is to make lexical_casts a cpp file like Moveit https://github.com/ros-planning/moveit/blob/3d2105584f414f2494eebff17122b398721d9c2a/moveit_core/utils/src/lexical_casts.cpp

christophfroehlich commented 9 months ago

I saw that also in the CI and was not sure if this is an artifact of old binary packages. Not sure where this comes from, when I understand it I'm happy to change this to a cpp file.

saikishor commented 9 months ago

I saw that also in the CI and was not sure if this is an artifact of old binary packages. Not sure where this comes from, when I understand it I'm happy to change this to a cpp file.

@christophfroehlich it seems like we need to separate them into header and cpp files, if not we will have this linking issue with multiple definitions. Shall I do it for you? or do you want to do it?

christophfroehlich commented 9 months ago

thanks @saikishor, semi-binary builds now. Should we wait for the merge until the next release and sync of ros2_control?

saikishor commented 9 months ago

thanks @saikishor, semi-binary builds now. Should we wait for the merge until the next release and sync of ros2_control?

If this or the prior changes are never released, it is better to wait. However, 90% of people who use ros2_control in rolling probably compile from source.

christophfroehlich commented 9 months ago

However, 90% of people who use ros2_control in rolling probably compile from source.

not sure about that, I'd just wait

saikishor commented 9 months ago

not sure about that, I'd just wait

Yes, let's wait for the release and the sync. I think we should also have waiting_for_sync tags, they are useful in these cases.