-
Notifications
You must be signed in to change notification settings - Fork 433
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
added get_type_name and to_string to ParameterVariant #42
Conversation
switch (get_type()) { | ||
case rclcpp::parameter::ParameterType::PARAMETER_BOOL: | ||
return "bool"; | ||
break; |
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.
These break
statements are unreachable and should be removed.
Same below.
+1 |
c7e930a
to
f8d5f4a
Compare
Ok, I addressed the comments and squashed. |
case rclcpp::parameter::ParameterType::PARAMETER_NOT_SET: | ||
return "not set"; | ||
default: | ||
throw std::runtime_error("Unexpected type from ParameterVariant"); |
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.
Should the error string include the actual numeric value of get_type()
?
Same below.
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.
I'm not sure that's very useful, but I'll include it.
f8d5f4a
to
40aa065
Compare
this was done to enable a more concise example
40aa065
to
e75c1d0
Compare
+1 |
added get_type_name and to_string to ParameterVariant
bool first_byte = true; | ||
bytes << "[" << std::hex; | ||
for (auto & byte : as_bytes()) | ||
{ |
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.
Style error: 3a4f147
Ctests fixes
This was done to enable more concise parameter examples.
I'm open to suggestion on a more elegant way to do the bytes to_string within a switch statement.
Connects to ros2/examples#4