Overall LGTM, but I will say that along with ros2/rclcpp#762, ros2/rclpy#370 was introduced. Whatever changes are proposed have to be applied to both client libraries IMHO.

Whatever changes are proposed have to be applied to both client libraries

I'll follow-up to make sure that this happens.

IMO, it would be nicer if the implementation for wildcard matching was done in rcl, then the client libraries would get it for free.

IMO, it would be nicer if the implementation for wildcard matching was done in rcl, then the client libraries would get it for free.

I think that's possible, it only requires a refactor of the current implementation.

I think that's possible, it only requires a refactor of the current implementation.

Hmm, I think I fiddled with it some time ago. Or @Lobotuerk did. We require extra API, as the wildcard matching cannot happen until we have the fully qualified node name.

Checking ros2/rcl#809, I have some doubts if some things are valid or not:

1. /*
2. /asd/*/bsd
3. /asd/**/bsd
4. /*asd
5. /asd*
6. /a*sd
7. /**asd
8. /asd**
9. /a**sd

4 to 9 should be all invalid IMO, * and ** should always be between slashes.
1 should work, because /asd/* works (and I don't think namespace / should be special).
2, 3 also sound like reasonable things to support, but I'm not sure if they are really useful or not.

@ivanpauno I completely agree with all your examples. IIUC, the logic added to rcl in ros2/rcl#809 does consider partial matching invalid (e.g. examples 4 to 9).

It might be the case that example 1 isn't considered valid in ros2/rcl#809, I'd have to check.

It might be the case that example 1 isn't considered valid in ros2/rcl#809, I'd have to check.

We should follow-up.

@ivanpauno @jacobperron

I didn't notice your comment before. #303 (comment)

ros2/rcl#809 expects items from 4 to 9 to be invalid (EXPECT_FALSE(res) with the rcl_parse_yaml_file).

Am I missing something?

@iuhilnehc-ynos i think everything is covered as expected on #303 (comment).

@jacobperron @ivanpauno i will go ahead to merge this. if anything is missing, let us know!