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.

Sign up for GitHub

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

Jump to bottom

Provide defaults for ParameterValue fields #18

Merged
merged 3 commits into from
Aug 22, 2017
Merged

Provide defaults for ParameterValue fields #18

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
349e96c
Provide defaults for ParameterValue fields
dhood Aug 16, 2017
d99bbe4
Remove unnecessary default for bytes_value
dhood Aug 17, 2017
43081b3
False->false to match design spec
dhood Aug 17, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions rcl_interfaces/msg/ParameterValue.msg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
uint8 type
Copy link
Member

Choose a reason for hiding this comment

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

Actually should we initialize this too?

Copy link
Member

Choose a reason for hiding this comment

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

yeah good point, I guess that PARAMETER_NOT_SET would be a good default. Not sure how we refer to message constants from another msg file though.

Copy link
Member

Choose a reason for hiding this comment

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

Hm, I guess we can't at the moment...

Copy link
Member Author

Choose a reason for hiding this comment

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

My reasoning for not putting a default for this was that, while all-but-one of the _value fields will be unused, the type is not "optional" so the client library should always initialise it itself

Copy link
Member

Choose a reason for hiding this comment

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

Hm, I guess we can't at the moment...

Should we open a ticket for this somewhere (rosidl?) that would be a useful feature

Copy link
Member Author

Choose a reason for hiding this comment

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

I've got two thumbs up on my last comment so will consider this question resolved. (in the future, reactions don't send notifications (at least not to me!) so comments would be appreciated)

Copy link
Member

Choose a reason for hiding this comment

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

👍 sorry forgot about that, yeah it would be great if github allowed to enable notification for "reactions"


# "Variant" style storage of the parameter value.
bool bool_value
int64 integer_value
float64 double_value
string string_value
bool bool_value false
int64 integer_value 0
float64 double_value 0.0
string string_value ""
byte[] bytes_value