search

clubcapra / markhor

🐐 Capra-Markhor is a ROS-based solution for managing and operating Club Capra's second rescue robot. 🐐
9 stars 1 forks source link

Remove markhor_estop and replace with capra_estop. #24

Closed myriamlacroix closed 2 years ago

myriamlacroix commented 2 years ago
saxtot commented 2 years ago

Looks good to me. @lvanasse did you expect more changes here?

lvanasse commented 2 years ago

The submodule for capra_estop is missing. Look in the estop branch, that Zackary did.

myriamlacroix commented 2 years ago

The submodule for capra_estop is missing. Look in the estop branch, that Zackary did.

When testing, capraestop was one level higher, directly in the workspace, with other capra... modules. Is it better to put it in markhor directory?

lvanasse commented 2 years ago

Humm no you're right, if we want to follow the "package" philosophy, we should keep capra_estop outside. Although this is dangerous, since markhor will not work unless there's the package in the above directory.

So what do you think about at least adding documentation concerning this dependency in the README, and somehow adding an error message when the package is not found when trying to run roslaunch ? Maybe test with find_pkg ? http://wiki.ros.org/roslaunch/XML

What do you think ?

myriamlacroix commented 2 years ago

image I tested removing the package and this message appears. I think it's clear enough? I will add the documentation in README.

lvanasse commented 2 years ago

Awesome and sounds good to me! Thanks, Myriam!