Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow empty prefix list to get all params; check depth correctly. #230

Merged
merged 2 commits into from
Jun 21, 2016

Conversation

gerkey
Copy link
Member

@gerkey gerkey commented Jun 20, 2016

Connects to #196

@gerkey gerkey added the in progress Actively being worked on (Kanban column) label Jun 20, 2016
@gerkey gerkey added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Jun 20, 2016
@gerkey gerkey self-assigned this Jun 20, 2016
@wjwwood
Copy link
Member

wjwwood commented Jun 20, 2016

lgtm, but that if-statement is getting hairy looking 😄.

@@ -271,17 +271,21 @@ Node::list_parameters(

// TODO(esteve): define parameter separator, use "." for now
for (auto & kv : parameters_) {
if (std::any_of(prefixes.cbegin(), prefixes.cend(), [&kv, &depth](const std::string & prefix) {
if (((prefixes.size() == 0) &&
((depth == 0) ||
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use the constant defined in the service rather then a plain 0 here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dfd3bf8

Matching change in ros2/system_tests@3d1d672

@gerkey gerkey merged commit 3553107 into master Jun 21, 2016
@gerkey gerkey deleted the issue196-again branch June 21, 2016 00:55
@gerkey gerkey removed the in review Waiting for review (Kanban column) label Jun 21, 2016
Karsten1987 pushed a commit that referenced this pull request Dec 7, 2016
* Allow empty prefix list to get all params; check depth correctly.

* use enum instead of constant
nnmm pushed a commit to ApexAI/rclcpp that referenced this pull request Jul 9, 2022
valgrind was complaining about some lost memory here. 
The proposed change should fix it.
DensoADAS pushed a commit to DensoADAS/rclcpp that referenced this pull request Aug 5, 2022
* Add Zstd decompressor
* Add unit test to verify decompression works and add unit test to verify it fails with bad input.
* Add unit test to check that ZstdDecompressor fails if file uri is bad.
* Include example of debug log in comments

Signed-off-by: Zachary Michaels <zmichaels11@gmail.com>

Co-authored-by: Zachary Michaels <zmichaels11@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants