Skip to content

Conversation

norro
Copy link

@norro norro commented Nov 17, 2020

This PR introduces package whitelist resp. blacklist, see #284.

If a file ${CMAKE_SOURCE_DIR}/../../../BridgePackageInclude.List" is found and non-empty (one ROS package name per line), only those ROS packages are mapped by the bridge.
Otherwise, if a file ${CMAKE_SOURCE_DIR}/../../../BridgePackageIgnore.List" is found and non-empty (one ROS package name per line), those ROS packages are ignored by the bridge.

This PR is very much work-in-progress with feedback appreciated, especially regarding the following points:

  • User interface: Alternatives to looking for lists in the ROS workspace?
  • Testing: No clever idea how to test this feature (needs to test different cmake setups?)
  • General implementation

I consider message blacklist/whitelist as a next (and additional) step, after implementing this package blacklist/whitelist.

norro added 2 commits November 17, 2020 21:21
* Adds cmake file that parses package include resp. exclude list
* Filtering cmake dependencies based on include/exclude list

ros2#284

Signed-off-by: norro <[email protected]>
* CMake passes package include/ignore lists to python scripts generating
the bridge c++ code
* Generation of factories and mappings restricted to packages filtered
by include/ignore lists

ros2#284

Signed-off-by: norro <[email protected]>
@norro norro marked this pull request as draft November 17, 2020 20:39
@norro
Copy link
Author

norro commented Nov 17, 2020

@clalancette I would very much appreciate your feedback.

@sloretz
Copy link
Contributor

sloretz commented Dec 1, 2020

Is the goal a feature that ignores ROS packages at compile time? If so, I would recommend a user interface of a CMake variable specified on the command line. When using colcon this would look like

colcon build --cmake-args -DROS1_BRIDGE_IGNORE_PKGS=std_msgs;std_srvs --packages-select ros1_bridge

@norro
Copy link
Author

norro commented Dec 2, 2020

@sloretz Yes, that's right.
I like your idea of using cmake variables and will adapt the PR accordingly soon.

Thanks for your feedback!

@clalancette clalancette added the more-information-needed Further information is required label Dec 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
more-information-needed Further information is required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants