-
Notifications
You must be signed in to change notification settings - Fork 181
[Draft]Add structured parameter support #1254
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
Open
Rahul-K-A
wants to merge
19
commits into
ros2:rolling
Choose a base branch
from
Rahul-K-A:add-structured-parameter-support
base: rolling
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
245dfe4
feat: Add `write_structured_parameter_to_string` function and integra…
Rahul-K-A f9a58d2
chore: Adjust `write_structured_parameter` args
Rahul-K-A 9dc0ca6
chore: Add yaml type
Rahul-K-A 5a91f64
chore: Add allocator and allocations to `write_structured_parameter_t…
Rahul-K-A 88fa00f
chore: Clean up `parse.c` a bit. Other refactors still pending (see T…
Rahul-K-A 663eb4e
chore: Add copy and deallocation operations for the YAML field of `rc…
Rahul-K-A 446b443
chore: Remove redundant `memset`
Rahul-K-A 786a711
chore: Refactor TODOs
Rahul-K-A cecd979
Add function to copy and emit events
Rahul-K-A 7d5ca80
chore: Remove unused printf
Rahul-K-A 88612f0
chore: Add new functions to begin and end string
Rahul-K-A b15a3a5
chore: Add new parameters to parser functions
Rahul-K-A 7ca623f
chore: Add emitter and pass it as part of `parser.c`
Rahul-K-A 610e56a
Add structured yaml parameter parsing using counter method
Rahul-K-A e945c48
chore: Move to a depth-based structured yaml detection system
Rahul-K-A 240e2e8
chore: Remove volatile qualifier for written_size
Rahul-K-A 1587c65
chore: Apply `ament_uncrustify`
Rahul-K-A d6aab38
Merge branch 'ros2:rolling' into add-structured-parameter-support
Rahul-K-A 6882ca7
chore: Fix cpplint complaints
Rahul-K-A File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -274,6 +274,8 @@ rcutils_ret_t parse_value( | |||||
| return RCUTILS_RET_ERROR; | ||||||
| } | ||||||
|
|
||||||
| // rahul-k-a: TODO Combine this with the parameter allocation part of | ||||||
| // `write_structured_parameter_to_string` and put everything in a seperate functiong | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this todo solved or still relevant ? |
||||||
| rcutils_ret_t ret = RCUTILS_RET_OK; | ||||||
| switch (val_type) { | ||||||
| case DATA_TYPE_UNKNOWN: | ||||||
|
|
@@ -590,13 +592,88 @@ _validate_name(const char * name, rcutils_allocator_t allocator) | |||||
| return ret; | ||||||
| } | ||||||
|
|
||||||
| /// | ||||||
| /// Makes a copy of an event and writes the copy to the emitter | ||||||
| /// | ||||||
| rcutils_ret_t write_event_to_emitter( | ||||||
| yaml_emitter_t * emitter, | ||||||
| yaml_event_t * event | ||||||
| ) | ||||||
| { | ||||||
| RCUTILS_CHECK_ARGUMENT_FOR_NULL(emitter, RCUTILS_RET_INVALID_ARGUMENT); | ||||||
| RCUTILS_CHECK_ARGUMENT_FOR_NULL(event, RCUTILS_RET_INVALID_ARGUMENT); | ||||||
| rcutils_ret_t ret = RCUTILS_RET_OK; | ||||||
| // The emitter deletes the event after writing | ||||||
| // So we need to make a copy of it and pass the copy to the emitter | ||||||
| yaml_event_t event_copy; | ||||||
| int success; | ||||||
| switch (event->type) { | ||||||
| /** A SCALAR event. */ | ||||||
| case YAML_SCALAR_EVENT: | ||||||
| { | ||||||
| success = yaml_scalar_event_initialize(&event_copy, event->data.scalar.anchor, | ||||||
| event->data.scalar.tag, event->data.scalar.value, event->data.scalar.length, | ||||||
| event->data.scalar.plain_implicit, event->data.scalar.quoted_implicit, | ||||||
| event->data.scalar.style); | ||||||
| break; | ||||||
| } | ||||||
|
|
||||||
| /** A SEQUENCE-START event. */ | ||||||
| case YAML_SEQUENCE_START_EVENT: | ||||||
| { | ||||||
| success = yaml_sequence_start_event_initialize(&event_copy, | ||||||
| event->data.sequence_start.anchor, event->data.sequence_start.tag, | ||||||
| event->data.sequence_start.implicit, event->data.sequence_start.style); | ||||||
| break; | ||||||
| } | ||||||
| /** A SEQUENCE-END event. */ | ||||||
| case YAML_SEQUENCE_END_EVENT: | ||||||
| { | ||||||
| success = yaml_sequence_end_event_initialize(&event_copy); | ||||||
| break; | ||||||
| } | ||||||
|
|
||||||
| /** A MAPPING-START event. */ | ||||||
| case YAML_MAPPING_START_EVENT: | ||||||
| { | ||||||
| success = yaml_mapping_start_event_initialize(&event_copy, event->data.mapping_start.anchor, | ||||||
| event->data.mapping_start.tag, event->data.mapping_start.implicit, | ||||||
| event->data.mapping_start.style); | ||||||
| break; | ||||||
| } | ||||||
| /** A MAPPING-END event. */ | ||||||
| case YAML_MAPPING_END_EVENT: | ||||||
| { | ||||||
| success = yaml_mapping_end_event_initialize(&event_copy); | ||||||
| break; | ||||||
| } | ||||||
| default: | ||||||
| { | ||||||
| RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("Unexpected YAML token of type %d", | ||||||
| (int) event->type); | ||||||
| ret = RCUTILS_RET_ERROR; | ||||||
| break; | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| if (success == 0) { | ||||||
| RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("Unable to duplicate YAML token of type %d", | ||||||
| (int) event->type); | ||||||
| return RCUTILS_RET_ERROR; | ||||||
| } | ||||||
| yaml_emitter_emit(emitter, &event_copy); | ||||||
| return ret; | ||||||
| } | ||||||
|
|
||||||
|
|
||||||
| /// | ||||||
| /// Parse the key part of the <key:value> pair | ||||||
| /// | ||||||
| rcutils_ret_t parse_key( | ||||||
| const yaml_event_t event, | ||||||
| uint32_t * map_level, | ||||||
| bool * is_new_map, | ||||||
| bool * overwrite_previous_key, | ||||||
| size_t * node_idx, | ||||||
| size_t * parameter_idx, | ||||||
| namespace_tracker_t * ns_tracker, | ||||||
|
|
@@ -709,11 +786,16 @@ rcutils_ret_t parse_key( | |||||
| break; | ||||||
| } | ||||||
| } else { | ||||||
| ret = find_parameter(*node_idx, parameter_ns, params_st, parameter_idx); | ||||||
| if (*overwrite_previous_key == false) { | ||||||
| // Handle cases where the code tries to overwrite the yaml parameter name entry | ||||||
| ret = find_parameter(*node_idx, value, params_st, parameter_idx); | ||||||
| *overwrite_previous_key = true; | ||||||
| } else { | ||||||
| ret = find_parameter(*node_idx, parameter_ns, params_st, parameter_idx); | ||||||
| } | ||||||
| if (ret != RCUTILS_RET_OK) { | ||||||
| break; | ||||||
| } | ||||||
|
|
||||||
| const size_t params_ns_len = strlen(parameter_ns); | ||||||
| const size_t param_name_len = strlen(value); | ||||||
| const size_t tot_len = (params_ns_len + param_name_len + 2U); | ||||||
|
|
@@ -746,23 +828,101 @@ rcutils_ret_t parse_key( | |||||
| return ret; | ||||||
| } | ||||||
|
|
||||||
| rcutils_ret_t initialize_emitter_string( | ||||||
| yaml_emitter_t * emitter) | ||||||
| { | ||||||
| rcutils_ret_t ret = RCUTILS_RET_OK; | ||||||
|
|
||||||
| // Set initial events | ||||||
| yaml_event_t event; | ||||||
| yaml_stream_start_event_initialize(&event, YAML_UTF8_ENCODING); | ||||||
| if (!yaml_emitter_emit(emitter, &event)) { | ||||||
| ret = RCUTILS_RET_ERROR; | ||||||
| } | ||||||
|
|
||||||
| yaml_document_start_event_initialize(&event, NULL, NULL, NULL, 0); | ||||||
| if (!yaml_emitter_emit(emitter, &event)) { | ||||||
| ret = RCUTILS_RET_ERROR; | ||||||
| } | ||||||
|
|
||||||
| return ret; | ||||||
| } | ||||||
|
|
||||||
| rcutils_ret_t end_emitter_string( | ||||||
| yaml_emitter_t * emitter) | ||||||
| { | ||||||
| rcutils_ret_t ret = RCUTILS_RET_OK; | ||||||
|
|
||||||
| // Set initial events | ||||||
| yaml_event_t event; | ||||||
| yaml_document_end_event_initialize(&event, 0); | ||||||
| if (!yaml_emitter_emit(emitter, &event)) { | ||||||
| ret = RCUTILS_RET_ERROR; | ||||||
| } | ||||||
|
|
||||||
| yaml_stream_end_event_initialize(&event); | ||||||
| if (!yaml_emitter_emit(emitter, &event)) { | ||||||
| ret = RCUTILS_RET_ERROR; | ||||||
| } | ||||||
|
|
||||||
| return ret; | ||||||
| } | ||||||
|
|
||||||
| rcutils_ret_t write_structured_parameter_to_string( | ||||||
| yaml_parser_t * parser, | ||||||
| char * yaml_string_buffer, | ||||||
| size_t * written_size, | ||||||
| const size_t node_index, | ||||||
| const size_t parameter_index, | ||||||
| rcl_params_t * params_st) | ||||||
| { | ||||||
| rcutils_ret_t ret = RCUTILS_RET_OK; | ||||||
| rcutils_allocator_t allocator = params_st->allocator; | ||||||
|
|
||||||
| // rahul-k-a: TODO combine this with the parameter allocation part of `parse_value` | ||||||
| // and put everything in a seperate function | ||||||
| char * copied_yaml = rcutils_strndup(yaml_string_buffer, *written_size, allocator); | ||||||
| RCUTILS_CHECK_ARGUMENT_FOR_NULL(copied_yaml, RCUTILS_RET_BAD_ALLOC); | ||||||
|
|
||||||
| rcl_variant_t * param_value = &(params_st->params[node_index].parameter_values[parameter_index]); | ||||||
|
|
||||||
| if (param_value->yaml_value != NULL) { | ||||||
| // Overwriting, deallocate original | ||||||
| allocator.deallocate(param_value->yaml_value, allocator.state); | ||||||
| } | ||||||
| param_value->yaml_value = copied_yaml; | ||||||
| return ret; | ||||||
| } | ||||||
|
|
||||||
|
|
||||||
| /// | ||||||
| /// Get events from parsing a parameter YAML file and process them | ||||||
| /// | ||||||
| rcutils_ret_t parse_file_events( | ||||||
| yaml_parser_t * parser, | ||||||
| yaml_emitter_t * emitter, | ||||||
| char * emitter_string_buffer, | ||||||
| size_t * emitter_written_bytes, | ||||||
| namespace_tracker_t * ns_tracker, | ||||||
| rcl_params_t * params_st) | ||||||
| { | ||||||
| int32_t done_parsing = 0; | ||||||
| bool is_key = true; | ||||||
| bool is_key_value_pair_found = true; | ||||||
| bool is_seq = false; | ||||||
| uint32_t line_num = 0; | ||||||
| data_types_t seq_data_type = DATA_TYPE_UNKNOWN; | ||||||
| uint32_t map_level = 1U; | ||||||
| uint32_t map_depth = 0U; | ||||||
| bool is_new_map = false; | ||||||
|
|
||||||
| uint32_t structure_detect_depth = 0; | ||||||
| *emitter_written_bytes = 0; | ||||||
| bool is_writing_structured_yaml = false; | ||||||
| size_t structured_yaml_param_idx = 0; | ||||||
| rcutils_ret_t ret = RCUTILS_RET_OK; | ||||||
| bool overwrite_previous_key = true; | ||||||
|
|
||||||
| RCUTILS_CHECK_ARGUMENT_FOR_NULL(parser, RCUTILS_RET_INVALID_ARGUMENT); | ||||||
| RCUTILS_CHECK_ARGUMENT_FOR_NULL(params_st, RCUTILS_RET_INVALID_ARGUMENT); | ||||||
| rcutils_allocator_t allocator = params_st->allocator; | ||||||
|
|
@@ -772,7 +932,6 @@ rcutils_ret_t parse_file_events( | |||||
| yaml_event_t event; | ||||||
| size_t node_idx = 0; | ||||||
| size_t parameter_idx = 0; | ||||||
| rcutils_ret_t ret = RCUTILS_RET_OK; | ||||||
| while (0 == done_parsing) { | ||||||
| if (RCUTILS_RET_OK != ret) { | ||||||
| break; | ||||||
|
|
@@ -784,7 +943,16 @@ rcutils_ret_t parse_file_events( | |||||
| ret = RCUTILS_RET_ERROR; | ||||||
| break; | ||||||
| } | ||||||
|
|
||||||
| line_num = ((uint32_t)(event.start_mark.line) + 1U); | ||||||
|
|
||||||
| if (is_writing_structured_yaml) { | ||||||
| if (RCUTILS_RET_ERROR == write_event_to_emitter(emitter, &event)) { | ||||||
| ret = RCUTILS_RET_ERROR; | ||||||
| break; | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| switch (event.type) { | ||||||
| case YAML_STREAM_END_EVENT: | ||||||
| done_parsing = 1; | ||||||
|
|
@@ -794,8 +962,23 @@ rcutils_ret_t parse_file_events( | |||||
| { | ||||||
| /// Need to toggle between key and value at params level | ||||||
| if (is_key) { | ||||||
| // If we're at the parameter level, set this flag to denote a | ||||||
| // value for the key has not been found yet | ||||||
| if (map_level == MAP_PARAMS_LVL) { | ||||||
| is_key_value_pair_found = false; | ||||||
| } | ||||||
| // Since the yaml parameter key is also considered as a | ||||||
| // namespace to all indented children, we must | ||||||
| // make sure that the parameter index of the yaml parameter is not overwritten | ||||||
| // By default, if a namespace is detected, then the parameter entry in the param | ||||||
| // table is replaced by its immediate chile | ||||||
| // This is done for optimization (?) - Rahul-K-A | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| if (is_writing_structured_yaml && (map_depth == structure_detect_depth) ) { | ||||||
| overwrite_previous_key = false; | ||||||
| } | ||||||
| ret = parse_key( | ||||||
| event, &map_level, &is_new_map, &node_idx, ¶meter_idx, ns_tracker, params_st); | ||||||
| event, &map_level, &is_new_map, &overwrite_previous_key, &node_idx, ¶meter_idx, | ||||||
| ns_tracker, params_st); | ||||||
| if (RCUTILS_RET_OK != ret) { | ||||||
| break; | ||||||
| } | ||||||
|
|
@@ -821,6 +1004,8 @@ rcutils_ret_t parse_file_events( | |||||
| return RCUTILS_RET_ERROR; | ||||||
| } | ||||||
| ret = parse_value(event, is_seq, node_idx, parameter_idx, &seq_data_type, params_st); | ||||||
| is_key_value_pair_found = true; | ||||||
|
|
||||||
| if (RCUTILS_RET_OK != ret) { | ||||||
| break; | ||||||
| } | ||||||
|
|
@@ -860,6 +1045,21 @@ rcutils_ret_t parse_file_events( | |||||
| { | ||||||
| is_new_map = false; | ||||||
| } | ||||||
| // Parsing nested (structured) YAML parameters | ||||||
| // If we're at the param level inside the YAML | ||||||
| // If a value has not been found for the previous key, and we get a new mapping event, | ||||||
| // In theory, this means we have a nested yaml struct | ||||||
| if (is_key_value_pair_found == false && is_writing_structured_yaml == false) { | ||||||
| is_writing_structured_yaml = true; | ||||||
| structured_yaml_param_idx = parameter_idx; | ||||||
| structure_detect_depth = map_depth; | ||||||
| initialize_emitter_string(emitter); | ||||||
| if (RCUTILS_RET_ERROR == write_event_to_emitter(emitter, &event)) { | ||||||
| ret = RCUTILS_RET_ERROR; | ||||||
| RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( | ||||||
| "Error adding line %d to structured yaml parameter", line_num); | ||||||
| } | ||||||
| } | ||||||
| break; | ||||||
| case YAML_MAPPING_END_EVENT: | ||||||
| if (MAP_PARAMS_LVL == map_level) { | ||||||
|
|
@@ -888,6 +1088,20 @@ rcutils_ret_t parse_file_events( | |||||
| } | ||||||
| } | ||||||
| map_depth--; | ||||||
| // Terminate structured yaml parameter if needed | ||||||
| if (is_writing_structured_yaml) { | ||||||
| if (map_depth < structure_detect_depth) { | ||||||
| end_emitter_string(emitter); | ||||||
| write_structured_parameter_to_string(parser, emitter_string_buffer, | ||||||
| emitter_written_bytes, node_idx, structured_yaml_param_idx, params_st); | ||||||
| is_writing_structured_yaml = false; | ||||||
| is_key_value_pair_found = true; | ||||||
| structure_detect_depth = 0; | ||||||
| structured_yaml_param_idx = 0; | ||||||
| // Reset byte counter so that we can reuse buffer | ||||||
| *emitter_written_bytes = 0; | ||||||
| } | ||||||
| } | ||||||
| break; | ||||||
| case YAML_ALIAS_EVENT: | ||||||
| RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( | ||||||
|
|
||||||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.