Catkin plugin: Add support for ROS tools. #152
Conversation
| LIBRARY DESTINATION ${CATKIN_PACKAGE_LIB_DESTINATION} | ||
| RUNTIME DESTINATION ${CATKIN_PACKAGE_BIN_DESTINATION} | ||
| ) |
kyrofa
force-pushed
the
catkin_support_roslaunch
branch
from
51d3d68
to
7b7ed02
Compare
| plugin = catkin.CatkinPlugin('test-part', self.properties) | ||
|
|
||
| with mock.patch.object(builtins, 'open', | ||
| mock.mock_open(read_data=package_xml)): |
There was a problem hiding this comment.
Could you remove this mock by writting the xml file to the sourcedir?
There was a problem hiding this comment.
Indeed I could, but I feel like actually hitting the filesystem when I don't need to feels a bit brittle and slow.
There was a problem hiding this comment.
In unit tests, I prefer to access the filesystem in temp directories instead of mocking that part.
But your test is correct, if you prefer it this way, I'm ok with that.
There was a problem hiding this comment.
I don't feel strongly about this. I'll update it :) .
kyrofa
force-pushed
the
catkin_support_roslaunch
branch
from
7b7ed02
to
df89d7b
Compare
|
I like this very much. One followup question is; do we need the |
|
@sergiusens I was wondering that myself. Let me experiment, this PR may have enabled the removal of it. |
kyrofa
force-pushed
the
catkin_support_roslaunch
branch
from
df89d7b
to
fb9d6f1
Compare
|
Alright I believe I've resolved all the concerns for this one. I will continue to increase test coverage throughout subsequent PRs, and will remove the roscore plugin once the blocking bug has been resolved. |
|
The example fails to build with: usr/bin/xml2-config |
kyrofa
force-pushed
the
catkin_support_roslaunch
branch
from
fb9d6f1
to
c2e8bfe
Compare
|
@ElOpio oh interesting, I didn't consider the interplay between plugins! I will add that to my manual test procedure. xml2-config isn't a required file, so I have temporarily added the removal of it back to the catkin plugin until we can remove roscore. |
The current Catkin plugin distributes only the built version of a ROS package that would typically go into the devel workspace, but that version doesn't include non-source, non-binary files, such as .launch files. This commit changes the plugin to actually install ROS packages, thereby including any non-source, non-binary files specified by the developer. This commit also updates the wrapper environment to correctly setup the ROS environment, which allows for the use of typical ROS tools within the snapcraft.yaml, such as roslaunch, rosrun, etc., and updates the ROS examples to make use of them. This commit also increases the test coverage of the Catkin plugin twofold. Signed-off-by: Kyle Fazzari <kyle@canonical.com>
kyrofa
force-pushed
the
catkin_support_roslaunch
branch
from
c2e8bfe
to
edd1367
Compare
|
@ElOpio and @kyrofa fwiw, plugins aren't supposed to solve all the conflicts in the world, that is to some extent part of the part author's responsibility. That does no rule out plugins themselves should try to deliver the cleanest environment possible, but not in detriment of its functionality. In the example of |
|
I'm 👍 on this branch btw 😉 |
|
Me too. Merging. |
Catkin plugin: Add support for ROS tools.
Catkin plugin: Add support for ROS tools.
The current Catkin plugin distributes only the built version of a ROS package that would typically go into the devel workspace, but that version doesn't include non-source, non-binary files, such as .launch files. This PR changes the plugin to actually install ROS packages, thereby including any non-source, non-binary files specified by the developer.
This PR also updates the wrapper environment to correctly setup the ROS environment, which allows for the use of typical ROS tools within the snapcraft.yaml, such as roslaunch, rosrun, etc., and updates the ROS examples to make use of them.
Finally, this PR also increases the test coverage of the Catkin plugin twofold.