Take initial parameters from parameters yaml files and constructor arguments. #225

nuclearsandwich · 2018-08-17T00:05:19Z

With this pull request initial parameters can be passed via yaml parameters file with the __params:= and as a list of parameters in the initial_parameters kwarg of the Node constructor.

Parameters in the constructor will override parameters from a parameters file.

If at least one reviewer could keep a close eye on my use of Py_DECREF and make sure that I have employed everywhere I should and nowhere I shouldn't have I'd be grateful.. I'm fairly confident in my usage but it's also my first time in Python's C API.

To test this PR I've been using the params.yaml in this gist https://gist.github.com/nuclearsandwich/8753121711671bfa8d9cb5f718ce09bc and the talker node from demo_nodes_py as well as the parameter services behavior just merged from #214

Connects to #202

sloretz

There are a few places where error checking is needed. Most cpython APIs can set an exception and return NULL.

sloretz · 2018-08-17T00:18:49Z

rclpy/src/rclpy/_rclpy.c

+        PyErr_Format(PyExc_RuntimeError, "Failed to parse yaml params file: %s", param_files[i]);
+        return false;
+      }
+      allocator.deallocate(param_files[i], allocator.state);


I think this needs a call to allocator.deallocate(param_files, allocator.state) after the array members have been deallocated.

sloretz · 2018-08-17T00:23:15Z

rclpy/src/rclpy/_rclpy.c

+  PyObject * value;
+  if (variant->bool_value) {
+    type_enum_value = 1;
+    value = variant->bool_value ? Py_True : Py_False;


Need to Py_INCREF(Py_True) or Py_INCREF(Py_False).

sloretz · 2018-08-17T00:24:07Z

rclpy/src/rclpy/_rclpy.c

+    value = PyUnicode_FromString(variant->string_value);
+  } else if (variant->byte_array_value) {
+    type_enum_value = 5;
+    value = PyList_New(variant->byte_array_value->size);


Need to check for NULL return here

Same comment in a couple places below

NULL check is now just below.

Looks like this returns a list of bytes() instances. It might be more convenient for users if this was one bytes() object with all the bytes in it. If that sounds reasonable I think PyBytes_FromStringAndSize could convert byte_array_value in one go.

It might be more convenient for users if this was one bytes() object with all the bytes in it.

It would be more convenient but it would violate the interface specification which states that a byte should be represented as a bytes of length one and that any of the array types should be represented by a Python list of the scalar type. http://design.ros2.org/articles/generated_interfaces_python.html

@mikaelarguedas and I talked about this and they may have a link to further discussion material.

sloretz · 2018-08-17T00:25:19Z

rclpy/src/rclpy/_rclpy.c

+    type_enum_value = 5;
+    value = PyList_New(variant->byte_array_value->size);
+    for (size_t i = 0; i < variant->byte_array_value->size; i++) {
+      PyList_SetItem(value, i, PyBytes_FromFormat("%u", variant->byte_array_value->values[i]));


Since list is new you can use PyList_SET_ITEM here

The docs don't mention it but it looks like PyBytes_FromFormat can error and return NULL.

Same comments about PyList_SET_ITEM below

sloretz · 2018-08-17T00:30:17Z

rclpy/src/rclpy/_rclpy.c

+    type_enum_value = 6;
+    value = PyList_New(variant->bool_array_value->size);
+    for (size_t i = 0; i < variant->bool_array_value->size; i++) {
+      PyList_SetItem(value, i, variant->bool_array_value->values[i] ? Py_True : Py_False);


Same comment about needing to incref True and false. PyList_SetItem and PyList_SET_ITEM steal a reference

sloretz · 2018-08-17T00:38:19Z

rclpy/src/rclpy/_rclpy.c

+    }
+  }
+
+  PyObject * type = PyObject_CallObject(parameter_type_cls, Py_BuildValue("(i)", type_enum_value));


Need to check Py_BuildValue didn't return NULL, as well as PyObject_CallObject.

sloretz · 2018-08-17T00:40:06Z

rclpy/src/rclpy/_rclpy.c

+  }
+
+  rcl_node_t * node = (rcl_node_t *)PyCapsule_GetPointer(node_capsule, "rcl_node_t");
+  const rcl_node_options_t * node_options = rcl_node_get_options(node);


This should be moved below the if (!node) check

sloretz · 2018-08-17T00:40:54Z

rclpy/src/rclpy/_rclpy.c

+  const rcl_node_options_t * node_options = rcl_node_get_options(node);
+  const rcl_allocator_t allocator = node_options->allocator;
+  if (!node) {
+    PyErr_Format(PyExc_RuntimeError, "Failed to retrieve rcl node from capsule");


PyCapsule_GetPointer would have already set an exception here. Returning NULL is enough.

sloretz · 2018-08-17T00:42:41Z

rclpy/src/rclpy/_rclpy.c

+    return PyList_New(0);
+  }
+
+  PyObject * parameter_type_cls = PyObject_GetAttrString(parameter_cls, "Type");


Need to check for NULL

nuclearsandwich · 2018-08-17T20:39:39Z

Linux
Linux-aarch64
macOS
Windows

sloretz · 2018-08-17T20:48:34Z

rclpy/rclpy/node.py

@@ -60,7 +60,7 @@ class Node:

    def __init__(
        self, node_name, *, cli_args=None, namespace=None, use_global_arguments=True,
-        start_parameter_services=True
+        start_parameter_services=True, initial_parameters=[]


The same list instance is used for every Node object initialized with the default initial_parameters. In this case it should be fine since no code modifies the list. I recommend using either None or tuple() as the default to be defensive against that though.

Of course. 😁 I think we were even talking about this together last week.

sloretz · 2018-08-17T20:54:07Z

rclpy/src/rclpy/_rclpy.c

+ *         false when there was an error during parsing and a Python exception was raised.
+ *
+ */
+static bool _parse_param_files(


nit, the rest of the file has return type on its own line.

sloretz · 2018-08-17T21:19:24Z

rclpy/src/rclpy/_rclpy.c

+    for (size_t i = 0; i < variant->byte_array_value->size; i++) {
+
+      member_value = PyBytes_FromFormat("%u", variant->byte_array_value->values[i]);
+      if (NULL == member_value) {


The list still has its reference count incremented. Need to Py_DECREF(value) before returning.

sloretz · 2018-08-17T21:27:14Z

rclpy/src/rclpy/_rclpy.c

+    value = PyUnicode_FromString(variant->string_value);
+  } else if (variant->byte_array_value) {
+    type_enum_value = 5;
+    value = PyList_New(variant->byte_array_value->size);


Looks like this returns a list of bytes() instances. It might be more convenient for users if this was one bytes() object with all the bytes in it. If that sounds reasonable I think PyBytes_FromStringAndSize could convert byte_array_value in one go.

sloretz · 2018-08-17T21:37:12Z

rclpy/src/rclpy/_rclpy.c

+      return NULL;
+    }
+    for (size_t i = 0; i < variant->integer_array_value->size; i++) {
+      PyList_SET_ITEM(value, i, PyLong_FromLong(variant->integer_array_value->values[i]));


PyLong_FromLong can return NULL if it fails to create a new python object.

sloretz · 2018-08-17T21:58:33Z

rclpy/src/rclpy/_rclpy.c

+  }
+  PyObject * parameter_type_cls = PyObject_GetAttrString(parameter_cls, "Type");
+  if (NULL == parameter_type_cls) {
+    PyErr_Format(PyExc_RuntimeError, "Error getting 'Type' attribute from Parameter class");


I expect PyObject_GetAttrString would have already raised AttributeError

You would appear to be correct. https://github.com/python/cpython/blob/e42b705188271da108de42b55d9344642170aa2b/Objects/object.c#L880

sloretz · 2018-08-17T21:59:43Z

rclpy/src/rclpy/_rclpy.c

+
+  if (node_options->use_global_arguments) {
+    if (!_parse_param_files(rcl_get_global_arguments(), allocator, params)) {
+      return NULL;


Need to free params

I thought so too but rcl_parse_yaml_file will actually fini the passed in struct on error https://github.com/ros2/rcl/blob/adc0190259a4eb725480e7b32a90b902bc5279b0/rcl_yaml_param_parser/src/parser.c#L1382.

c08b236 makes _parse_param_files fini the params struct in both error situations so that the caller does not need to.

sloretz · 2018-08-17T21:59:54Z

rclpy/src/rclpy/_rclpy.c

+  }
+
+  if (!_parse_param_files(&(node_options->arguments), allocator, params)) {
+    return NULL;


same about freeing params here

sloretz · 2018-08-17T22:00:30Z

rclpy/src/rclpy/_rclpy.c

+  }
+
+  if (!PyObject_HasAttrString(parameter_cls, "Type")) {
+    PyErr_Format(PyExc_RuntimeError, "Parameter class is missing 'Type' attribute");


need to fini params here and a few places below

sloretz · 2018-08-17T22:01:34Z

rclpy/src/rclpy/_rclpy.c

+  rcl_node_params_t node_params = params->params[node_index];
+  PyObject * parameter_list = PyList_New(node_params.num_params);
+  if (NULL == parameter_list) {
+    return NULL;


Py_DECREF(parameter_type_cls) and in another place below

sloretz · 2018-08-17T23:03:29Z

rclpy/src/rclpy/_rclpy.c

+    if (RCL_RET_OK != ret) {
+      PyErr_Format(PyExc_RuntimeError, "Failed to get initial parameters: %s",
+        rcl_get_error_string_safe());
+      return false;


Replying to https://github.com/ros2/rclpy/pull/225/files#r211049548 , since the calling code doesn't fini params if this errors then I think params needs to be fini'd here.

With da7da1a the implementation no longer takes an rcl_params_t as input and instead creates and uses the data structure internally only if parameter arguments are found without error so the params object no longer exists at this point.

nuclearsandwich · 2018-08-17T23:38:38Z

Linux
Linux-aarch64
macOS
Windows

Next CI after fixing lint and some warnings. I couldn't track the MSBuild warnings so I'll get those next wave.

nuclearsandwich · 2018-08-20T21:07:26Z

rclpy/src/rclpy/_rclpy.c

+    return NULL;
+  }
+
+  int node_index = -1;


MSBuild (rightly) complains that intmight lose information when assigned from a size_t below. But changing to a size_t makes the sigil value -1 invalid. Do we have a pattern we use for this sort of situation like storing a second node_index_found value and setting and checking it along with the index?

nuclearsandwich · 2018-08-21T20:05:57Z

While setting up system tests I discovered that the yaml parameter parser doesn't preserve any parameters currently in the struct as I though. So I'll need to refactor this to extract parameters one file as a time and store them in a Python dict rather than a list.

nuclearsandwich · 2018-08-22T20:49:05Z

refactor this to extract parameters one file at a time and store them in a Python dict rather than a list.

This refactor is complete and the implementation is now working against the existing tests in system_tests/test_cli.

wjwwood · 2018-08-23T22:33:17Z

rclpy/rclpy/node.py

@@ -100,6 +100,13 @@ def __init__(

        self.__executor_weakref = None

+        node_parameters = _rclpy.rclpy_get_node_parameters(Parameter, self.handle)
+        # use the set_parameters API so parameter events are publish.


I tried to read the code to understand this but I'm a bit lost I think. In this case aren't the parameters potentially set twice, once as the "node parameters" and then again using the initial parameters?

And wouldn't that generate two events? Is that the intended behavior? I could see it either way: "I'd like to see the evolution of the values on startup" or "I just care about what it was initialized with not what went into determining the initial value".

I thought I was matching the rclcpp behavior in sending events for parameters changed by subsequent parameter files but I must've been seeing double as ros2 topic echo /parameter_events for a cpp node only sends events for the end state of initial parameters.

wjwwood · 2018-08-23T22:34:07Z

Looks like this comment is still pending right?

#225 (comment)

nuclearsandwich · 2018-08-23T23:26:42Z

Looks like this comment is still pending right? #225 (comment)

It was addressed indirectly. Added an explanation.

nuclearsandwich · 2018-08-23T23:27:49Z

Linux
Linux-aarch64
macOS
Windows

mikaelarguedas · 2018-08-24T00:49:21Z

rclpy/rclpy/node.py

+        elif node_parameters:
+            self.set_parameters(node_parameters)
+        elif initial_parameters:
+            self.set_parameters(initial_parameters)


At first glance, it looks like this could be simplified quite a bit by setting default values for node_parameters and initial_parameters.

(I didnt check the implementation so this is hypothetical)
But if:
initial_parameters defaulted to an empty list and node_parameters to an empty dict.
Could we get rid of the conditional logic and just have the following?

params_by_name = {p.name: p for p in node_parameters} params_by_name.update({p.name: p for p in initial_parameters}) self.set_parameters(params_by_name.values())

I wouldnt expect a big performance hit if these are empty.

mikaelarguedas

looks like a few comments were not addressed, added a few comments inline as well

mikaelarguedas · 2018-08-24T00:55:09Z

rclpy/src/rclpy/_rclpy.c

@@ -3369,6 +3370,385 @@ rclpy_clock_set_ros_time_override(PyObject * Py_UNUSED(self), PyObject * args)
  Py_RETURN_NONE;
 }

+/// Create an rclpy.parameter.Parameter from an rcl_variant_t
+/**
+ * On failure a Python exception is raised and false is returned if:


NULL is returned on failure (which is the right thing to return 👍 we should just update the docblock).

~~not sure what the if: is referring to ? are we missing a sentence here?~~ Matches the rest of the file, pretty surprising sentence though but out of scope of this PR

mikaelarguedas · 2018-08-24T00:55:42Z

rclpy/src/rclpy/_rclpy.c

+ * \param[in] parameter_type_cls The PythonObject for the Parameter.Type class.
+ *
+ * Returns a pointer to an rclpy.parameter.Parameter with the name, type, and value from
+ * the variant or NULL when raising a python exception.


Nit: python -> Python same multiple times below

mikaelarguedas · 2018-08-24T00:59:48Z

rclpy/src/rclpy/_rclpy.c

+  PyObject * value;
+  PyObject * member_value;
+  if (variant->bool_value) {
+    type_enum_value = 1;


are these type_enum_values matching the types defined in https://github.com/ros2/rcl_interfaces/blob/db27f0e8619460848d80c1442f7fec0c56ee63e5/rcl_interfaces/msg/ParameterType.msg ?
If yes, any reason for not reusing these enums?

mikaelarguedas · 2018-08-24T01:15:09Z

rclpy/src/rclpy/_rclpy.c

+  int param_files_count = rcl_arguments_get_param_files_count(args);
+  rcl_ret_t ret;
+  bool successful = true;
+  if (param_files_count > 0) {


we could reduce nesting by just returning false if param_files_count <= 0

mikaelarguedas · 2018-08-24T01:18:04Z

rclpy/src/rclpy/_rclpy.c

+  rcl_ret_t ret;
+  bool successful = true;
+  if (param_files_count > 0) {
+    ret = rcl_arguments_get_param_files(args, allocator, &param_files);


as ret is not used anywhere else we could just check the return value of rcl_arguments_get_param_files and return on failure

mikaelarguedas · 2018-08-24T01:27:33Z

rclpy/src/rclpy/_rclpy.c

+  }
+  PyObject * param = PyObject_CallObject(parameter_cls, args);
+  Py_DECREF(args);
+  Py_DECREF(type);


mikaelarguedas · 2018-08-24T01:30:38Z

rclpy/src/rclpy/_rclpy.c

+ * Raises RuntimeError if the parameters file fails to parse
+ *
+ * \param[in] parameter_cls The rclpy.parameter.Parameter class object.
+ * \param[in] node_handle Capsule pointing to the node handle


Nit: node_handle -> node_capsule

mikaelarguedas · 2018-08-24T01:33:30Z

rclpy/src/rclpy/_rclpy.c

+  }
+
+  rcl_node_t * node = (rcl_node_t *)PyCapsule_GetPointer(node_capsule, "rcl_node_t");
+  if (!node) {


Nit: if (NULL == node)

mikaelarguedas · 2018-08-24T01:34:32Z

rclpy/src/rclpy/_rclpy.c

+  }
+  PyObject * parameter_type_cls = PyObject_GetAttrString(parameter_cls, "Type");
+  if (NULL == parameter_type_cls) {
+    /* PyObject_GetAttrString raises AttributeError on failure. */


Nit: please use // for single line comments

mikaelarguedas · 2018-08-24T01:49:36Z

rclpy/src/rclpy/_rclpy.c

+  }
+  Py_DECREF(parameter_type_cls);
+
+  const char * node_namespace = rcl_node_get_namespace(node);


is this deallocated anywhere?

No. Since this is a const "borrowed" reference to the internal node string. It's lifetime is bound to the node it is from.

nuclearsandwich · 2018-08-24T20:54:23Z

rclpy/src/rclpy/_rclpy.c

@@ -3613,7 +3994,7 @@ static PyMethodDef rclpy_methods[] = {

  {
    "rclpy_convert_from_py_qos_policy", rclpy_convert_from_py_qos_policy, METH_VARARGS,
-    "Convert a QoSPolicy python object into a rmw_qos_profile_t."
+    "Convert a QoSPolicy Python object into a rmw_qos_profile_t."


@mikaelarguedas had me turn all my python references to Python so I brought this one along for the ride. Has nothing else to do with this PR.

Uses rcl_yaml_param_parser to find and parse parameter yaml files and supply initial parameters to rclpy nodes.

rclpy nodes can now take a list of initial parameters via their constructor and will check rcl arguments for parameter files returning any parameters specific to that node.

This also fixes an issue where early return would prevent deallocation of later array members.

The previous implementation relied on the (later debunked) assumption that calling rcl_parse_yaml_file repeatedly with the same rcl_params_t would yield a struct that contains the cumulative parameters from those files. That isn't actually the case so we need to create an intermediate storage which can be used instead. In rclcpp a std::map is used but we have no such datatype here. Instead we'll use a PyDict which maps node names to a nested dict of rclpy.parameter.Parameters by parameter name and then query that struct for parameters to return to the node.

This was reincorporated unintentionally during merge conflict resolution.

nuclearsandwich · 2018-08-27T14:10:41Z

FYI for any reviewers who have pulled locally I've just rebased and pushed in order to resolve merge conflicts.

nuclearsandwich · 2018-08-28T18:27:54Z

Linux
Linux-aarch64
macOS
Windows

sloretz · 2018-08-28T22:50:38Z

rclpy/src/rclpy/_rclpy.c

+      return false;
+    }
+    PyObject * parameter_dict;
+    if (!PyDict_Contains(node_params_dict, py_node_name)) {


the docs say this can set an exception and return -1 , though I'm not sure under what circumstances

I did a bit of empirical testing and it can return -1 when the item you pass in as a dict isn't. Since we create the dict that doesn't seem like a case worth checking at runtime.

sloretz · 2018-08-28T22:53:24Z

rclpy/src/rclpy/_rclpy.c

+      }
+      if (-1 == PyDict_SetItem(node_params_dict, py_node_name, parameter_dict)) {
+        Py_DECREF(py_node_name);
+        return false;


Py_DECREF(parameter_dict) too

sloretz · 2018-08-28T22:56:04Z

rclpy/src/rclpy/_rclpy.c

+      parameter_dict = PyDict_GetItem(node_params_dict, py_node_name);
+      if (NULL == parameter_dict) {
+        Py_DECREF(py_node_name);
+        return false;


Confusingly, PyDict_GetItem does not set an exception. In the rest of the places this function returns false a python exception is already set before returning.

sloretz · 2018-08-28T22:58:39Z

rclpy/src/rclpy/_rclpy.c

+      Py_INCREF(parameter_dict);
+    }
+    rcl_node_params_t node_params = params->params[i];
+    for (size_t ii = 0; ii < node_params.num_params; ii++) {


Nit, use prefix increment ++ii, though I'm sure the compiler is smart enough to not make a copy here anyways.

sloretz · 2018-08-28T23:09:22Z

rclpy/src/rclpy/_rclpy.c

+ * \param[in] args The arguments to parse for parameter files
+ * \param[in] allocator Allocator to use for allocating and deallocating within the function.
+ * \param[out] params_by_node_name A Python dict object to place parsed parameters into.
+ *


nit, missing documentation of parameter_cls, and parameter_type_cls

sloretz · 2018-08-28T23:10:27Z

rclpy/src/rclpy/_rclpy.c

+      rcl_get_error_string_safe());
+    return false;
+  }
+  for (int i = 0; i < param_files_count; i++) {


sloretz · 2018-08-28T23:13:01Z

rclpy/src/rclpy/_rclpy.c

+            params, allocator, parameter_cls, parameter_type_cls, params_by_node_name))
+        {
+          PyErr_Format(PyExc_RuntimeError,
+            "Failed to fill params dict from file: %s", param_files[i]);


Except as noted about PyDict_GetItem, an exception is already set here.

sloretz · 2018-08-28T23:15:48Z

rclpy/src/rclpy/_rclpy.c

+          PyErr_Format(PyExc_RuntimeError,
+            "Failed to fill params dict from file: %s", param_files[i]);
+          rcl_yaml_node_struct_fini(params);
+          return false;


I think param_files needs to be deallocated before returning.

sloretz · 2018-08-28T23:23:20Z

rclpy/src/rclpy/_rclpy.c

+    Py_DECREF(params_by_node_name);
+    return NULL;
+  }
+  allocator.deallocate(node_name_with_namespace, allocator.state);


Nit, could deduplicate this line by putting it before the NULL check above.

sloretz · 2018-08-28T23:25:16Z

rclpy/src/rclpy/_rclpy.c

+  // PyDict_GetItem is a borrowed reference. INCREF so we can return a new one.
+  Py_INCREF(node_params);
+
+  Py_DECREF(parameter_type_cls);


I think this was already decref'd

Need to Py_DECREF(py_node_name_with_namespace);

sloretz

LGTM with green ci

nuclearsandwich · 2018-08-29T18:46:11Z

Linux
Linux-aarch64
macOS
Windows

nuclearsandwich · 2018-08-29T19:29:38Z

Thank you @sloretz and @mikaelarguedas. This PR wouldn't have been realized without the time you spent providing clear feedback and meticulous review.

nuclearsandwich added the in review Waiting for review (Kanban column) label Aug 17, 2018

nuclearsandwich self-assigned this Aug 17, 2018

nuclearsandwich requested a review from mikaelarguedas August 17, 2018 00:05

dirk-thomas mentioned this pull request Aug 17, 2018

Support for parameters #202

Closed

6 tasks

sloretz reviewed Aug 17, 2018

View reviewed changes

nuclearsandwich commented Aug 20, 2018

View reviewed changes

nuclearsandwich added in progress Actively being worked on (Kanban column) in review Waiting for review (Kanban column) and removed in review Waiting for review (Kanban column) in progress Actively being worked on (Kanban column) labels Aug 21, 2018

wjwwood reviewed Aug 23, 2018

View reviewed changes

mikaelarguedas reviewed Aug 24, 2018

View reviewed changes

nuclearsandwich commented Aug 24, 2018

View reviewed changes

nuclearsandwich added 7 commits August 24, 2018 13:55

Add dependency on rcl_yaml_param_parser.

93b3daf

Add rclpy_get_node_parameters function.

9e31f51

Uses rcl_yaml_param_parser to find and parse parameter yaml files and supply initial parameters to rclpy nodes.

Add initial parameters to Node constructor.

799e432

rclpy nodes can now take a list of initial parameters via their constructor and will check rcl arguments for parameter files returning any parameters specific to that node.

Deallocate array after members are deallocated.

a490fef

This also fixes an issue where early return would prevent deallocation of later array members.

Increment references of boolean values.

595f8e3

Add much more null checking.

e80c255

Fix unsafe statement order and don't raise a new exception.

9c5d206

nuclearsandwich and others added 15 commits August 24, 2018 13:57

Dereference bool_value.

950f1f2

Add initial_parameters kwarg to rclpy.create_node.

10d15fb

typo

9dd57d8

fixup: Linters.

14a23d1

Merge parameters from cli and constructor before setting.

edad250

python -> Python per feedback.

bb68363

Reduce nesting with early return.

43db3c2

Return a dict of parameter name: parameter instead of a list.

26243f9

Fix null check and comment styles.

eb092ea

Simplify setting initial params from a dict.

090dcbf

Fix docblock (false - >NULL)

57a9984

More DECREF.

655b6cc

Remove earlier version of parameter setting.

04bfe36

This was reincorporated unintentionally during merge conflict resolution.

Use parameter message type enums instead of hard-coding values.

2a60867

nuclearsandwich force-pushed the initial-parameters branch from b5fd549 to 2a60867 Compare August 27, 2018 14:08

sloretz reviewed Aug 28, 2018

View reviewed changes

nuclearsandwich added 2 commits August 29, 2018 06:09

Address feedback from Shane.

584d547

Don't set new exception.

4726384

sloretz approved these changes Aug 29, 2018

View reviewed changes

nuclearsandwich mentioned this pull request Aug 29, 2018

Test parameter behavior for rclpy nodes ros2/system_tests#293

Merged

Merge remote-tracking branch 'ros2/master' into initial-parameters

6369aac

nuclearsandwich merged commit 92bb336 into master Aug 29, 2018

nuclearsandwich removed the in review Waiting for review (Kanban column) label Aug 29, 2018

nuclearsandwich deleted the initial-parameters branch August 29, 2018 19:31

nuclearsandwich mentioned this pull request Aug 30, 2018

Fix warning when parameter value is uninitialized. #234

Merged

Take initial parameters from parameters yaml files and constructor arguments. #225

Take initial parameters from parameters yaml files and constructor arguments. #225

Conversation

nuclearsandwich commented Aug 17, 2018

sloretz 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nuclearsandwich commented Aug 17, 2018

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

Choose a reason for hiding this comment

nuclearsandwich commented Aug 17, 2018

Choose a reason for hiding this comment

nuclearsandwich commented Aug 21, 2018

nuclearsandwich commented Aug 22, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wjwwood commented Aug 23, 2018

nuclearsandwich commented Aug 23, 2018

nuclearsandwich commented Aug 23, 2018 • edited Loading

Choose a reason for hiding this comment

mikaelarguedas 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

nuclearsandwich commented Aug 27, 2018

nuclearsandwich commented Aug 28, 2018

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

sloretz left a comment

Choose a reason for hiding this comment

nuclearsandwich commented Aug 29, 2018 • edited Loading

nuclearsandwich commented Aug 29, 2018

nuclearsandwich commented Aug 23, 2018 •

edited

Loading

nuclearsandwich commented Aug 29, 2018 •

edited

Loading