Upgrade parameters

[Acuadros95]

Functionality upgrade on micro-ROS parameters:

• Remove parameters: Using API and ros2 param delete

• Add new parameters with ros2 param set, configurable with option allow_undeclared_parameters

• Control of this features and parameter changes on user callback:
  

  rclc/rclc_parameter/include/rclc_parameter/rclc_parameter.h

  Lines 77 to 83 in a79d5b8

  	* Parameter event callback.
  	* This callback will allow the user to allow or reject the following events:
  	* - Parameter value change: Internal and external parameter set on existing parameters.
  	* - Parameter creation: External parameter set on unexisting parameter if `allow_undeclared_parameters` is set.
  	* - Parameter delete: External parameter delete on existing parameter.
  	*
  	* Parameters modifications are disabled while this callback is executed.

• Added ros2 param describe:

  • Parameter description
  • Constrains values and description
  • Read only parameters

• Added low memory mode, reducing the memory allocated with certain constrains:

  • Request size limited to 1 parameter on Set, Get, Get types and Describe services.
  • List parameters request has no prefixes enabled nor depth.
  • Parameter description strings not allowed. (rclc_add_parameter_description)

• Memory benchmark on STM32F4 for 7 parameters with RCLC_PARAMETER_MAX_STRING_LENGTH = 50 and notify_changed_over_dds = true

  • Full mode: 11736 B
  • Low memory mode: 4160 B

• Test suite upgrade with parameterized rclc_parameter_options_t options.

• Update tutorials and documentation:

  • Update parameter server tutorial with rolling micro-ROS/micro-ROS.github.io#366
  • Update rclc parameter demo micro-ROS/micro-ROS-demos#62

[Acuadros95]

What should be the approach with parameter constrains?

• The new API offers a read only flag. This flag blocks parameter changes from external clients, but allows changes on the server side.
• Parameters modification API (Set, get, add, delete, ...) is disabled while the callback is executing.
• Does it make sense to handle numeric constrains internally? The user can handle this constrains in the on_modification callback, leaving the constrains API to publish this information to external parameter clients.

rclc/rclc_parameter/include/rclc_parameter/rclc_parameter.h

Lines 488 to 490 in cb7f5a1

	rclc_add_parameter_constraints_integer(
	rclc_parameter_server_t * parameter_server,
	const char * parameter_name, int64_t from_value, int64_t to_value, uint64_t step);

Related: #273

[pablogs9]

Initial review

[JanStaschulat]

@Acuadros95 please rebase on master to get changed ci.yaml or re-do the changes as in here https://github.com/ros2/rclc/pull/266/files.

Then the CI RCLC Rolling should run.

[Acuadros95]

@JanStaschulat Looking good

[pablogs9]

LGTM, do not merge until documentation in micro-ROS webpage is ready.

@JanStaschulat @ralph-lange would you mind taking a quick look here? We are implementing new functionalities and modifying several things.

[codecov-commenter]

Codecov Report

Merging #274 (d2b2bfe) into master (79a2df2) will increase coverage by 4.36%.
The diff coverage is 83.84%.

@@            Coverage Diff             @@
##           master     #274      +/-   ##
==========================================
+ Coverage   64.77%   69.13%   +4.36%     
==========================================
  Files          16       16              
  Lines        2197     2715     +518     
  Branches      647      765     +118     
==========================================
+ Hits         1423     1877     +454     
+ Misses        459      451       -8     
- Partials      315      387      +72     

Impacted Files	Coverage Δ
...clc_parameter/src/rclc_parameter/parameter_utils.c	68.37% <67.07%> (+12.27%)	⬆️
...lc_parameter/src/rclc_parameter/parameter_server.c	82.41% <86.37%> (+9.20%)	⬆️
rclc/src/rclc/action_goal_handle.c	59.87% <0.00%> (-1.28%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 09de417...d2b2bfe. Read the comment docs.

[pablogs9]

Documentation here

[ralph-lange]

Reviewed the documentation, function declarations for the parameter server, and the example. @JanStaschulat, can you please focus on the parameter server implementation?

[JanStaschulat]

LGTM some minor changes like

• using NULL pointer checks for pointer arguments
• using the |= operator to aggregate state information
• updated documentation.
• one wrong return value
• code style: each parameter of a function should be in a new line

I have not looked at unit tests yet.

[JanStaschulat]

Looks already very great to me.

Could you add a few simple test cases to check for the NULL pointer arguments. rclc_parameter package has already a high coverage about 70%. Which is great. I think with a few simple function calls with fn(...,null, ...) you could improve your code coverage very fast.

https://app.codecov.io/gh/ros2/rclc/compare/274/diff

In general, the current code coverage of all rclc packages is about 65%. Which is not very high. I think we should take same effort to aim for about 80%.

[Acuadros95]

@JanStaschulat Updated coverage to 80%

[JanStaschulat]

LGTM

[pablogs9]

Great job, @JanStaschulat thanks a lot for the thorough review

[Acuadros95]

Updated documentation PR with latest review: micro-ROS/micro-ROS.github.io#366

[ralph-lange]

LGTM

[ros-discourse]

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-humble-hawksbill-released/25729/14