Parse parameter descriptors in yaml #533

bpwilcox · 2019-10-25T23:50:59Z

This PR addresses the feature request in #481, namely parsing parameter descriptor keys in the yaml file. This feature will enable parameter descriptors to be added into nodes at run-time.

Overview of additions are:

New struct types to contain parameter descriptor information (in alignment with the ParameterDescriptor.msg)
- rcl_param_descriptor_t, rcl_node_params_descriptor_t
- descriptor keys are compared for assignment into struct
Added map level for MAP_PARAM_DESCRIPTORS_LVL to parse descriptors
- new map level is triggered when the 'parameter__descriptors' key is parsed by the yaml event
- some added API to handle descriptor keys/values
Independent allocation/access to parameters and parameter descriptors in the params_t struct
- each node will have its list of parameters as well as descriptors
- parameter descriptors may be added in yaml file without corresponding parameter or mixed

The proposed syntax for this PR is:

  node1:
    ros__parameters:
      foo: 42
      bar: 25.0
      param1: 10
      parameter__descriptors:
        param1:
          type: 2
          min_value: 5
          max_value: 100
          step: 5
          read_only: false
          description: "int parameter"
          additional_constraints: "only multiples of 5"
        param2:
          min_value: 1.0
          max_value: 10.0
          description: "double parameter"

I've made accompanying changes in rclcpp to extract the parameter descriptors struct into a ParameterDescriptor.msg which will override when calling declare_parameter (or if the overrides are automatically declared). I will submit that PR to the rclcpp repo soon, though this is a pre-requisite.

hidmic

First pass!

hidmic · 2019-11-21T14:09:10Z

rcl_yaml_param_parser/include/rcl_yaml_param_parser/types.h

+  int64_t * min_value_int;
+  int64_t * max_value_int;
+  int64_t * step_int;
+} rcl_param_descriptor_t;


@wjwwood meta: not a comment for you @bpwilcox, but replicating rcl_interfaces/msg/ParameterDescriptor structure is not ideal. The same goes with rcl_interfaces/msg/Parameter, so I wonder if we should be re-using the message C representation instead (like we partially do in rcl_action already). Thoughts?

In a related note, I understand pointers are used here to have indication of absence. At the same time, ROS 2 interfaces have no optional fields, so something, somewhere, will choose defaults. If we had all of that here, we could get rid of many heap allocations and simplify the code.

rcl_yaml_param_parser/include/rcl_yaml_param_parser/types.h

hidmic · 2019-11-21T14:10:30Z

rcl_yaml_param_parser/src/parser.c

 } namespace_tracker_t;

 #define PARAMS_KEY "ros__parameters"
+#define PARAMS_DESCRIPTORS_KEY "parameter__descriptors"


@bpwilcox meta: why not ros__parameter_descriptors for consistency?

Given it is already scoped under ros__parameters level it seemed redundant, but I'm not opposed.

This does bring up my original approach to have the parameter__descriptors be at the same level as ros__parameters (i.e. no longer scoped), so the two would be uncoupled. It seemed like much more work at the time with not too significant benefits, but I'm open to discussing it.

hidmic · 2019-11-21T14:17:15Z

rcl_yaml_param_parser/src/parser.c

@@ -826,6 +1136,77 @@ void rcl_yaml_node_struct_print(
      }
    }
  }
+  printf("\n Node Name\t\t\t\tParameter Descriptors\n");


@bpwilcox nit: I guess it's fine to begin with, as it precludes the need to match parameter values with parameter descriptors, but it'd be nice to collapse both printed tables into one.

Right, I didn't want to have to match values and descriptors.

hidmic · 2019-11-21T14:18:16Z

rcl_yaml_param_parser/src/parser.c

+        parameter_idx++)
+      {
+        if (
+          (NULL != node_descriptors_st->parameter_names))


@bpwilcox nit: can't this line be collapsed with the one above?

hidmic · 2019-11-21T14:28:26Z

rcl_yaml_param_parser/src/parser.c

+          break;
+        }
+
+        /// Add a parameter name into the node parameters


@bpwilcox node parameter descriptors maybe?

hidmic · 2019-11-21T14:29:32Z

rcl_yaml_param_parser/src/parser.c

+  } else if (0 == strncmp("type", ns_tracker->descriptor_key_ns, strlen("type"))) {
+    if (val_type == DATA_TYPE_INT64) {
+      param_descriptor->type = (uint8_t *)ret_val;
+      if (NULL == param_descriptor->type) {


@bpwilcox here and everywhere else within this function, why do you check for ret_val nullity again? And why does the error message set reads as a memory allocation error?? Bad copy-paste?

I'm not checking ret_val for nullity directly here, that is already done once when it is first assigned. Here I am following the same pattern as in the parse_value, where it checks the allocated descriptor struct field after being cast from ret_val. Am I misunderstanding the original code on that pattern?

Hmm I'm not following you, I don't see such pattern. But even if there was, you're not allocating memory here when you cast (as the log message below suggests). It's a C-like cast, it won't result in NULL if it isn't NULL already. Or I completely missed the point of this code.

I realized my mistake, you're correct. The check was required when allocating the arrays in parse_value, which I am not doing here.

hidmic · 2019-11-21T14:36:22Z

rcl_yaml_param_parser/src/parser.c

@@ -38,6 +38,7 @@ typedef enum yaml_map_lvl_e
  MAP_UNINIT_LVL = 0U,
  MAP_NODE_NAME_LVL = 1U,
  MAP_PARAMS_LVL = 2U,
+  MAP_PARAMS_DESCRIPTORS_LVL = 3U


@bpwilcox hmm, putting parameter descriptors within the ros__parameters and relying on users not using the parameter__descriptors key for their own purposes under the assumption that it's unlikely enough is not ideal. Consider either making this map a sibling of ros__parameters or supporting further nesting e.g.:

my_node: ros__parameters: param1: 1.0 param2: value: foo descriptor: read_only: true

I would think the further nesting could be more confusing to the user, just my opinion (it seems to conflate the yaml syntax rules for when it is a parameter vs a descriptor). I mentioned this in a another comment, but yes I can see parameter__descriptors as a sibling to ros__parameters,but as I did attempt that first, I think it is a bit more messy to write the code for doing so than simply using the key to trigger a new map level. If per your suggestion, we change the key to ros__parameter_descriptors, do you think it is unique enough for that assumption?

I'm wary about the word enough there. I'd rather just preclude name collisions. But maybe others don't think alike. @ivanpauno @jacobperron @wjwwood thoughts?

I don't see a reason to nest parameter_descriptors under ros__parameters, and I would rather implement it as a sibling of ros__parameters, as @hidmic suggested. ros__parameter_descriptors is a good name for that sibling.
I also agree that the alternative of further nesting is confusing.

I prefer the extra nesting since it keeps the descriptors physically close to the parameters they are describing. As a sibling to ros__parameters, I think it requires more mental effort. My two cents.

Well, actually that option isn't bad either. But both the value and the descriptor should be optional (and at least one has to be specified).

I don't have a strong opinion. I see merit in both keeping it separate and having descriptors near the overrides.

In regards to the extra nesting, I am unsure what would be a preferable syntax. The one @hidmic presented as an example implies that there are two different ways to define an override, the original way, and the nested version, where the descriptor is attached and there's 2 new key words, value and descriptor. In my opinion, I think it obfuscates the parameter namespacing syntax by adding even more key words to check. For that reason, I'm still in favor of a separate map level (either sibling or scoped under ros__parameters) for the ros__parameter_descriptors where the rules for parsing are cleanly separated. In addition, I wouldn't want to require that both a parameter description and override have to exist, but a user can set either.

The one @hidmic presented as an example implies that there are two different ways to define an override, the original way, and the nested version,

That's correct.

In my opinion, I think it obfuscates the parameter namespacing syntax by adding even more key words to check.

That sounds a bit like an implementation detail i.e. something a user wouldn't (have to) care about.

In any case, I don't have a strong opinion in favor or against a sibling descriptor mapping or nested descriptors (and neither do others, or at least there's no general consensus), but I do think it should be one of those two collision free approaches and not the current one.

hidmic · 2019-11-21T14:37:38Z

rcl_yaml_param_parser/src/parser.c

+
+  if (NULL == ns_tracker->parameter_ns) {
+    RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
+      "Parameter assignment at line %d unallowed in parameter__descriptors", line_num);


@bpwilcox here and elsewhere, isn't it the point of having a PARAMS_DESCRIPTORS_KEY macro to not use the string literal everywhere?

You're referring to why I am using the ns_tracker->descriptor_key_ns in this function?

You're referring to why I am using the ns_tracker->descriptor_key_ns in this function?

Not exactly, that I understand. What I meant to say is, let's try use PARAMS_DESCRIPTORS_KEY instead of parameter_descriptors where possible. For this particular line:

RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( "Parameter assignment at line %d unallowed in " PARAMS_DESCRIPTORS_KEY, line_num);

Sorry for not being clear enough.

I understand now, thanks for clarifying

hidmic · 2019-11-21T14:41:07Z

rcl_yaml_param_parser/include/rcl_yaml_param_parser/types.h

+/// \brief node_params_descriptors_t stores all the parameter descriptors of a single node
+typedef struct rcl_node_params_descriptors_s
+{
+  char ** parameter_names;  ///< Array of parameter names (keys)


@bpwilcox I don't follow why there's an array of parameter names here and a name on each descriptor.

Parallelism? I think I had intended to parallel the structure of rcl_node_params and rcl_node_params_descriptors. It made the code more convenient to write, I believe, though I suppose we could remove one or the other.

Hmm I'm not entirely convinced it's worth the duplication. You can get away with just an rcl_param_descriptor_t array.

This comment still applies i.e. no need to have the parameter name in three (3) places.

I removed the name field in rcl_param_descriptor_t instead since it made more sense following the parallel logic of the code with the parameter values.

hidmic · 2020-01-07T15:57:07Z

@bpwilcox are you planning to circle back on this?

bpwilcox · 2020-01-08T01:59:31Z

@hidmic Yes, I'd started some local changes before the new year but had been preoccupied since. Most notably, as suggested I'm intending to move the ros__parameters and ros__parameter_descriptors to be at sibling levels. With regards to removing parameter names from the descriptor type, I'm not sure if that is worthwhile as it breaks some of the intended code similarity I'd added intentionally before. I'll try to get an update in next week or so.

hidmic · 2021-02-12T21:36:14Z

@bpwilcox do you still plan on submitting changes? Or shall I close this patch?

bpwilcox · 2021-02-19T21:47:03Z

@bpwilcox do you still plan on submitting changes? Or shall I close this patch?

Yes, I believe I should have some more cycles to work on this in the near future.

bpwilcox · 2021-03-23T04:12:30Z

@hidmic I've re-done this work and addressed feedback on a separate branch. It was easier and cleaner than trying to resolve all of the merge conflicts considering the many changes since I've last touched this PR. Would it be better to close this PR and open a new one, or to force push/reset that branch onto this one?

bpwilcox · 2021-03-29T23:37:18Z

@mjeronimo I reset this branch with commits from my revised branch. This way the previous PR comments can be referenced. Maybe we can get some re-reviews of this work while I push ahead in this next month on the rclcpp layer in ros2/rclcpp#909

Signed-off-by: Brian Wilcox <[email protected]>

…d parse key for descriptor map level Signed-off-by: Brian Wilcox <[email protected]>

Signed-off-by: Brian Wilcox <[email protected]>

… descriptor struct API Signed-off-by: Brian Wilcox <[email protected]>

Signed-off-by: Brian Wilcox <[email protected]>

…ameters and descriptors under node name in yaml Signed-off-by: Brian Wilcox <[email protected]>

Signed-off-by: Brian Wilcox <[email protected]>

… outputs Signed-off-by: Brian Wilcox <[email protected]>

Signed-off-by: Brian Wilcox <[email protected]>

bpwilcox · 2021-06-29T16:58:14Z

rcl_yaml_param_parser/src/parse.c

@@ -786,13 +786,6 @@ rcutils_ret_t parse_key(
      {
        /// Till we get PARAMS_KEY or PARAMS_DESCRIPTORS_KEY, keep adding to node namespace
        if (0 == strncmp(PARAMS_KEY, value, strlen(PARAMS_KEY))) {
-          if (0U == ns_tracker->num_node_ns) {


In the current unit tests, it was only testing the case of a node with a namespace. With no namespace, this error occurs when a yaml file includes both parameters (PARAMS_KEY) and descriptors (PARAM_DESCRIPTORS_KEY) under the same scope.

For example, this yaml would cause the error:

parameters_node: ros__parameters: param1: 30 param3: "hello world" ros__parameter_descriptors: param1: type: 2 min_value: 5 max_value: 100 step: 5 read_only: false description: "int parameter" additional_constraints: "only multiples of 5" param2: min_value: 1.0 max_value: 10.0 description: "double parameter"

If these lines are removed, then this yaml becomes acceptable and the unit tests still pass. There will still be an error if trying to put a key without a node name (which is expected), but it is not caught here. I'm still trying to determine what should be the proper solution if anyone has some thoughts to add.

hidmic

First pass, mostly high-level comments.

Thanks for picking this up again @bpwilcox !

hidmic · 2021-08-09T21:36:21Z

rcl_yaml_param_parser/include/rcl_yaml_param_parser/parser.h

@@ -119,6 +119,17 @@ rcl_variant_t * rcl_yaml_node_struct_get(
  const char * param_name,
  rcl_params_t * params_st);

+/// \brief Get the descriptor struct for a given parameter
+/// \param[in] node_name is the name of the node to which the parameter belongs
+/// \param[in] param_name is the name of the parameter whose value is to be retrieved


@bpwilcox this needs an update:

Suggested change

/// \param[in] param_name is the name of the parameter whose value is to be retrieved

/// \param[in] param_name is the name of the parameter whose descriptor is to be retrieved

hidmic · 2021-08-09T21:36:38Z

rcl_yaml_param_parser/include/rcl_yaml_param_parser/parser.h

+/// \param[in] node_name is the name of the node to which the parameter belongs
+/// \param[in] param_name is the name of the parameter whose value is to be retrieved
+/// \param[inout] params_st points to the populated (or to be populated) parameter struct
+/// \return parameter descriptor struct on success and NULL on failure


@bpwilcox what if no descriptor was specified? what will this function do?

If the parameter name or node name does not exist for the descriptor, then it will still return a descriptor (not NULL). I found that this is the same behavior as what happens with rcl_yaml_node_struct_get.

@bpwilcox it'd be best to document what the expected behavior is.

hidmic · 2021-08-09T21:38:30Z

rcl_yaml_param_parser/include/rcl_yaml_param_parser/types.h

+/// \brief node_params_descriptors_t stores all the parameter descriptors of a single node
+typedef struct rcl_node_params_descriptors_s
+{
+  char ** parameter_names;  ///< Array of parameter names (keys)


This comment still applies i.e. no need to have the parameter name in three (3) places.

hidmic · 2021-08-09T21:40:44Z

rcl_yaml_param_parser/include/rcl_yaml_param_parser/types.h

+{
+  char ** parameter_names;  ///< Array of parameter names (keys)
+  rcl_param_descriptor_t * parameter_descriptors;  ///< Array of corresponding parameter descriptors
+  size_t num_params;  ///< Number of parameters in the node


@bpwilcox did you mean num_descriptors?

hidmic · 2021-08-09T21:42:47Z

rcl_yaml_param_parser/src/impl/yaml_descriptor.h

+/// Finalize an rcl_param_descriptor_t
+///
+RCL_YAML_PARAM_PARSER_PUBLIC
+void rcl_yaml_descriptor_fini(


@bpwilcox did you mean rcl_param_descriptor_fini? Same elsewhere. YAML is implied.

This parallels the function naming in yaml_variant.h so I just kept it. If we change the name to your suggestion, I'd recommend we do the same for the variant version. I don't know if this is a big concern, however.

@bpwilcox Hmm, I see. Though if we were to match rcl_variant_t APIs naming pattern it should be rcl_yaml_param_descriptor_fini().

hidmic · 2021-08-09T21:51:38Z

rcl_yaml_param_parser/include/rcl_yaml_param_parser/types.h

+  rcl_param_descriptor_t * parameter_descriptors;  ///< Array of corresponding parameter descriptors
+  size_t num_params;  ///< Number of parameters in the node
+  size_t capacity_descriptors;  ///< Capacity of parameters in the node
+} rcl_node_params_descriptors_t;


@bpwilcox wouldn't adding an rcl_param_descriptor_t ** parameter_descriptors; field to the rcl_node_params_s struct simplify the code a bit? Using NULL to indicate when a descriptor is not available, precluding parameter descriptors without an associated parameter value.

I was initially going to put the together as you recommend, however I do not think we would want the parameter descriptors to require association with a parameter value. In my view, it should be perfectly acceptable to have a yaml file with descriptors only and declared parameters in code may comply with these descriptions.

@bpwilcox hmm, that's a strange use case. I'd expect the parameter file to specify both, or the descriptor to be specified in code. In what scenario would you want to externally change the parameter descriptor of a parameter declared in code?

hidmic · 2021-08-09T21:59:11Z

rcl_yaml_param_parser/include/rcl_yaml_param_parser/types.h

+  int64_t * min_value_int;
+  int64_t * max_value_int;
+  int64_t * step_int;
+} rcl_param_descriptor_t;


In a related note, I understand pointers are used here to have indication of absence. At the same time, ROS 2 interfaces have no optional fields, so something, somewhere, will choose defaults. If we had all of that here, we could get rid of many heap allocations and simplify the code.

hidmic · 2021-09-28T22:51:03Z

@bpwilcox friendly ping

bpwilcox · 2021-10-14T16:28:21Z

Thanks for your feedback @hidmic I will be addressing your feedback soon!

…m/descriptor keys under node name Signed-off-by: Brian Wilcox <[email protected]> Signed-off-by: Brian <[email protected]>

Signed-off-by: Brian <[email protected]>

bpwilcox · 2022-01-31T20:39:39Z

Hey @hidmic Could you take another pass through this review since my latest commits?

hidmic

Alright, another pass @bpwilcox. Sorry for the delay. Too much to do, and this PR's a lengthy one.

hidmic · 2022-02-17T15:02:03Z

rcl_yaml_param_parser/include/rcl_yaml_param_parser/parser.h

+/// \param[in] node_name is the name of the node to which the parameter belongs
+/// \param[in] param_name is the name of the parameter whose value is to be retrieved
+/// \param[inout] params_st points to the populated (or to be populated) parameter struct
+/// \return parameter descriptor struct on success and NULL on failure


@bpwilcox it'd be best to document what the expected behavior is.

hidmic · 2022-02-17T15:05:51Z

rcl_yaml_param_parser/include/rcl_yaml_param_parser/types.h

+  rcl_param_descriptor_t * parameter_descriptors;  ///< Array of corresponding parameter descriptors
+  size_t num_params;  ///< Number of parameters in the node
+  size_t capacity_descriptors;  ///< Capacity of parameters in the node
+} rcl_node_params_descriptors_t;


@bpwilcox hmm, that's a strange use case. I'd expect the parameter file to specify both, or the descriptor to be specified in code. In what scenario would you want to externally change the parameter descriptor of a parameter declared in code?

hidmic · 2022-02-17T15:07:29Z

rcl_yaml_param_parser/src/impl/node_params_descriptors.h

+
+///
+/// Create rcl_node_params_descriptors_t structure
+///


@bpwilcox here and elsewhere in this file, can we document return values and parameters?

hidmic · 2022-02-17T15:10:11Z

rcl_yaml_param_parser/src/impl/yaml_descriptor.h

+/// Finalize an rcl_param_descriptor_t
+///
+RCL_YAML_PARAM_PARSER_PUBLIC
+void rcl_yaml_descriptor_fini(


@bpwilcox Hmm, I see. Though if we were to match rcl_variant_t APIs naming pattern it should be rcl_yaml_param_descriptor_fini().

hidmic · 2022-02-17T15:19:14Z

rcl_yaml_param_parser/test/test_node_params_descriptors.cpp

@@ -0,0 +1,84 @@
+// Copyright 2020 Open Source Robotics Foundation, Inc.


@bpwilcox nit: here and elsewhere, dates probably need an update?

hidmic · 2022-02-17T17:07:18Z

rcl_yaml_param_parser/test/test_yaml_descriptor.cpp

+    rcl_param_descriptor_t src_descriptor{}; \
+    rcl_param_descriptor_t dest_descriptor{}; \
+    rcutils_allocator_t allocator = rcutils_get_default_allocator(); \
+    src_descriptor.field = &tmp_field; \


@bpwilcox nit: you may also do:

Suggested change

src_descriptor.field = &tmp_field; \

auto tmp_field = field_value;

src_descriptor.field = &tmp_field; \

to not have to add a temporary lvalue for each literal rvalue you want to use for testing.

hidmic · 2022-02-17T17:11:06Z

rcl_yaml_param_parser/test/test_yaml_descriptor.cpp

+  rcl_param_descriptor_t dest_descriptor{};
+  rcutils_allocator_t allocator = rcutils_get_default_allocator();
+
+  char * tmp_description = rcutils_strdup("param description", allocator);


@bpwilcox nit: following above's suggestion, you may be able to merge this test into the one before by directly passing rcutils_strdup() and setting up a scope exit block to finalize src_descriptor.

hidmic · 2022-02-17T17:13:40Z

rcl_yaml_param_parser/src/parse.c

+    return RCUTILS_RET_ERROR;
+  }
+
+  char * dt = "dynamic_typing";


@bpwilcox nit: why make this one a variable, and a non const one of all things?

hidmic · 2022-02-17T17:15:07Z

rcl_yaml_param_parser/src/parse.c

+                "Internal error adding node namespace at line %d", line_num);
+              break;
+            }
+          }


@bpwilcox nit: can we reduce code duplication? map_level bumps seem to be the only difference between the last two clauses.

hidmic · 2022-02-17T17:19:17Z

rcl_yaml_param_parser/src/parse.c

@@ -801,13 +1083,19 @@ rcutils_ret_t parse_file_events(
              yaml_event_delete(&event);
              return RCUTILS_RET_ERROR;
            }
-            if (0U == params_st->params[node_idx].num_params) {
+            if (0U == params_st->params[node_idx].num_params && map_level != 3U) {


@bpwilcox meta: since ros__parameters and ros__parameter_descriptors are siblings, perhaps we should have another flag to signal which one we're processing as opposed to repurposing map_level

Shubham2S · 2022-05-28T14:02:00Z

Hi, I'm looking forward to use parameter descriptors in the YAML file. What is the status of this PR?

fujitatomoya · 2022-05-31T21:03:41Z

@bpwilcox are you planning to work on this?

bpwilcox force-pushed the parse_parameter_descriptors branch 2 times, most recently from feb60ac to f28d0cc Compare October 28, 2019 23:18

bpwilcox mentioned this pull request Oct 30, 2019

Add ParameterDescriptor override from yaml ros2/rclcpp#909

Open

wjwwood added the enhancement New feature or request label Nov 14, 2019

wjwwood requested a review from hidmic November 15, 2019 23:08

wjwwood assigned hidmic Nov 15, 2019

hidmic requested changes Nov 21, 2019

View reviewed changes

bpwilcox force-pushed the parse_parameter_descriptors branch from fb190e6 to b61a63c Compare November 22, 2019 23:24

bpwilcox force-pushed the parse_parameter_descriptors branch 2 times, most recently from ada671e to bf425d8 Compare March 29, 2021 23:31

bpwilcox added 14 commits June 25, 2021 00:15

add param_descriptor type

2185043

Signed-off-by: Brian Wilcox <[email protected]>

add parameter descriptors map level and namespace key

d5712fd

Signed-off-by: Brian Wilcox <[email protected]>

add param descriptor copy and deallocation

2ab8f09

Signed-off-by: Brian Wilcox <[email protected]>

add node_params_descriptors init and re/deallocation

f337a6a

Signed-off-by: Brian Wilcox <[email protected]>

add find_descriptor, parse_desriptor, and modify parse file events an…

702c448

…d parse key for descriptor map level Signed-off-by: Brian Wilcox <[email protected]>

modify replace ns with parameter descriptor level

817bce1

Signed-off-by: Brian Wilcox <[email protected]>

modify parser implementation for parameter descriptors, add get param…

2640d09

… descriptor struct API Signed-off-by: Brian Wilcox <[email protected]>

add new files to CMakeLists

b299ff2

Signed-off-by: Brian Wilcox <[email protected]>

check for new map at MAP_NODE_NAME_LVL: fixes issue to allow both par…

ce49272

…ameters and descriptors under node name in yaml Signed-off-by: Brian Wilcox <[email protected]>

fix test now that more calloc calls with param descriptors

7836801

Signed-off-by: Brian Wilcox <[email protected]>

add parameter descriptor yaml tests

9da5590

Signed-off-by: Brian Wilcox <[email protected]>

add parameter to test yaml for extra coverage and verify parsed param…

0b8a5d7

… outputs Signed-off-by: Brian Wilcox <[email protected]>

add unit tests for param descriptors

c06b47a

Signed-off-by: Brian Wilcox <[email protected]>

fix cpplint

dbfbe87

Signed-off-by: Brian Wilcox <[email protected]>

bpwilcox added 2 commits June 25, 2021 00:15

fix uncrustify issues

f5338cc

Signed-off-by: Brian Wilcox <[email protected]>

change param descriptor key name to ros__parameter_descriptors

a5939aa

Signed-off-by: Brian Wilcox <[email protected]>

bpwilcox force-pushed the parse_parameter_descriptors branch 2 times, most recently from ea2051a to 27674f6 Compare June 29, 2021 16:02

bpwilcox commented Jun 29, 2021

View reviewed changes

hidmic self-requested a review August 9, 2021 13:17

hidmic reviewed Aug 9, 2021

View reviewed changes

bpwilcox force-pushed the parse_parameter_descriptors branch from 02e1906 to f7c5ba8 Compare December 7, 2021 14:31

bpwilcox added 2 commits December 7, 2021 14:39

remove error msg for node name before key to allow for both keys para…

e6cf65d

…m/descriptor keys under node name Signed-off-by: Brian Wilcox <[email protected]> Signed-off-by: Brian <[email protected]>

address feedback, remove descriptor name, change field naming

1d1eb27

Signed-off-by: Brian <[email protected]>

bpwilcox force-pushed the parse_parameter_descriptors branch from f7c5ba8 to 1d1eb27 Compare December 7, 2021 14:40

add dynamic_typing field

a66f54d

Signed-off-by: Brian <[email protected]>

hidmic self-requested a review January 31, 2022 23:24

hidmic reviewed Feb 17, 2022

View reviewed changes

audrow changed the base branch from master to rolling June 28, 2022 14:20

Shubham2S mentioned this pull request Sep 10, 2022

No support to add parameter descriptions in YAML file. HBRS-SDP/ss22-ros2-perception#39

Open

	/// \param[in] param_name is the name of the parameter whose value is to be retrieved
	/// \param[in] param_name is the name of the parameter whose descriptor is to be retrieved

		@@ -0,0 +1,84 @@
		// Copyright 2020 Open Source Robotics Foundation, Inc.

	src_descriptor.field = &tmp_field; \
	auto tmp_field = field_value;
	src_descriptor.field = &tmp_field; \

Parse parameter descriptors in yaml #533

Are you sure you want to change the base?

Parse parameter descriptors in yaml #533

Conversation

bpwilcox commented Oct 25, 2019

hidmic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bpwilcox Nov 22, 2019 • edited Loading

Choose a reason for hiding this comment

hidmic Nov 21, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bpwilcox Dec 4, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hidmic commented Jan 7, 2020

bpwilcox commented Jan 8, 2020

hidmic commented Feb 12, 2021

bpwilcox commented Feb 19, 2021

bpwilcox commented Mar 23, 2021

bpwilcox commented Mar 29, 2021

Choose a reason for hiding this comment

hidmic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hidmic commented Sep 28, 2021

bpwilcox commented Oct 14, 2021

bpwilcox commented Jan 31, 2022

hidmic left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Shubham2S commented May 28, 2022

fujitatomoya commented May 31, 2022

bpwilcox Nov 22, 2019 •

edited

Loading

hidmic Nov 21, 2019 •

edited

Loading

bpwilcox Dec 4, 2019 •

edited

Loading

hidmic left a comment •

edited

Loading