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

Refactor and cleanup utils #6

Merged
merged 1 commit into from Apr 5, 2021
Merged

Conversation

luca-della-vedova
Copy link
Member

@luca-della-vedova luca-della-vedova commented Apr 5, 2021

Bug fix

Fixed bug

Utility libraries were not being properly linked and one of them (the one in rmf_building_sim_common) only happened to work because the code was duplicated between header and .cpp file.

Fix applied

This PR removes all the code duplication in the utils.hpp file and links correctly the libraries through target_link_libraries.
The utils.cpp file was actually in the rmf_building_sim_gazebo_plugins namespace so it wasn't used anywhere.

TODO

Figure out if we should move the utils code to a common library, a lot of functions are now duplicated between rmf_robot_sim_common and rmf_building_sim_common

Signed-off-by: Luca Della Vedova <luca@openrobotics.org>
@gbiggs
Copy link
Collaborator

gbiggs commented Apr 5, 2021

Maybe we should consider merging rmf_robot_sim_common and rmf_building_sim_common into a single rmf_sim_common library. How much not-duplicated code is there?

@luca-della-vedova
Copy link
Member Author

Maybe we should consider merging rmf_robot_sim_common and rmf_building_sim_common into a single rmf_sim_common library. How much not-duplicated code is there?

The rmf_building_sim_common utils are completely contained in the rmf_robot_sim_common utils, it's effectively a subset of the utilities

@luca-della-vedova luca-della-vedova merged commit 16542ed into slotcar-move Apr 5, 2021
@luca-della-vedova luca-della-vedova deleted the refactor_utils branch April 5, 2021 04:03
luca-della-vedova added a commit that referenced this pull request Apr 5, 2021
* Move slotcar to rmf_robot_sim_*

Signed-off-by: Geoffrey Biggs <gbiggs@killbots.net>

* Fix namespace usage

Signed-off-by: Geoffrey Biggs <gbiggs@killbots.net>

* Refactor and cleanup utils (#6)

Signed-off-by: Luca Della Vedova <luca@openrobotics.org>

Co-authored-by: Luca Della Vedova <luca@openrobotics.org>
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.

None yet

2 participants