-
Notifications
You must be signed in to change notification settings - Fork 35
Filtering empty names with naive approach #362
Filtering empty names with naive approach #362
Conversation
Finally I've finished merging with current master. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there are empty node names, then the list of node namespaces will be larger than the list of names.
We also need to return a shortened list of node namespaces. Using a similar strategy by keeping a temporary list of node namespaces should work.
@CaptainTrunky do you have plans to resume iterating on this PR? Thanks. |
@CaptainTrunky Friendly ping 🙂 Let us know if you'd like to keep working on this. |
Sorry, I've been busy with my job. I'll try to resolve current issues within next two weeks. |
@CaptainTrunky Another ping |
I've added support for namespaces. Plan to to test it on Wednesday and push final version. |
6feb28e
to
dd4d3e5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CaptainTrunky Thanks for iterating on this!
I've left a few more comments that I think need to be addressed before I trigger CI.
6497482
to
cf92310
Compare
Signed-off-by: captaintrunky <captaintrunky@gmail.com>
* removing <iostream> * removing debug output * removing redundant tmp_namespaces_num * allowing empty namespaces * tmp_names.size() must be equal to tmp_namespaces_num.size() * unifying cleanup procedures Signed-off-by: captaintrunky <captaintrunky@gmail.com>
cf92310
to
70e6973
Compare
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
70e6973
to
71a5c60
Compare
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@CaptainTrunky Thanks for iterating!
Fixes a bug introduced in #362. Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Fixes a bug introduced in #362. Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Connects to ros2/ros2#489
A pull request for #489 basic implementation that allocates a temporary buffer for node names and filters out all empty names. It's not properly tested yet and additional unit tests are required. So next steps are:
add tests (Modifying test for failing on getting an empty node name system_tests#374)
add optimal implementation that does not require additional buffer and copying data
PS
had to reopen PR because rebasing all the 6+ month old changes is way too painful to be productive.
Signed-off-by: captaintrunky captaintrunky@gmail.com