Allow parameter changes while keeping defaults

[amalnanavati]

Description

This PR allows us to change non-read-only parameters for create_action_server, have them persist across runs of ada_feeding_launch.xml, but still maintain access to the original parameters (e.g., in case we have to revert them). In practice, only the tree_kwargs are non-read-only.

It achieves this through the following:

1. Parameters for create_action_server are not defined in two files: (a) ada_feeding_action_servers_default.yaml and (b) ada_feeding_action_servers_current.yaml.
   1. All parameters but the ones for the WatchdogListener are placed under the default namespace in ada_feeding_action_servers_default.yaml.
   2. ada_feeding_action_servers_current.yaml contains parameters in the current namespace: (a) current.overridden_parameters which is a list of the parameters that are overridden, and (b) the values for each of those parameters.
2. On the node side, all parameters in the default' namespace are declared as read-only. However, for the ones that the node wants to make over-writeable, it declares a read-write version of that parameter in the current` namespace.
3. When the node receives a set parameter request for a parameter in the current namespace, it does the following:
   1. Checks (e.g., that the parameter type is correct)
   2. Update the parameter. In particularly, since the only non-read-only parameters are tree_kwargs, it re-creates the tree with the new kwargs.
   3. Write the updated overridden parameters to ada_feeding_action_servers_current.yaml in the share folder.

Testing procedure

This was tested in sim.

• Setup
  • ros2 run ada_feeding dummy_ft_sensor.py
  • ros2 launch feeding_web_app_ros2_test feeding_web_app_dummy_nodes_launch.xml run_web_bridge:=false run_motion:=false
  • (Update path based on your current directory) ros2 run ada_feeding_perception republisher --ros-args --params-file src/ada_feeding/ada_feeding_perception/config/republisher.yaml
  • ros2 launch ada_feeding ada_feeding_launch.xml use_estop:=false
• Testing without MoveIt
  • Run ros2 param list. For the /ada_feeding_action_servers node, verify that all the tree_kwargs in default are also in current.
  • Get the value of a param in current and verify it is unset: e.g., ros2 service call /ada_feeding_action_servers/get_parameters rcl_interfaces/srv/GetParameters "{names: ['current.MoveAbovePlate.tree_kwargs.f_mag']}"
  • Get the value of the corresponding param in default and verify that it is set as expected (should be 4.0): e.g., ros2 service call /ada_feeding_action_servers/get_parameters rcl_interfaces/srv/GetParameters "{names: ['default.MoveAbovePlate.tree_kwargs.f_mag']}"
  • Attempt to change the default parameter, and verify that it fails: e.g., ros2 service call /ada_feeding_action_servers/set_parameters rcl_interfaces/srv/SetParameters "{parameters: [{name: 'default.MoveAbovePlate.tree_kwargs.f_mag', value: {type: 3, double_value: 1.0}}]}"
  • Attempt to change the current parameter to an incorrect type, and verify that it failes: e.g., ros2 service call /ada_feeding_action_servers/set_parameters rcl_interfaces/srv/SetParameters "{parameters: [{name: 'current.MoveAbovePlate.tree_kwargs.f_mag', value: {type: 4, string_value: 'ASDFGHJKL'}}]}"
    • Verify that in the share directory (within the install folder of the workspace), ada_feeding_action_servers_current.yaml is unchanged.
  • Attempt to change the current parameter to a correct type, and verify that it works: e.g., ros2 service call /ada_feeding_action_servers/set_parameters rcl_interfaces/srv/SetParameters "{parameters: [{name: 'current.MoveAbovePlate.tree_kwargs.f_mag', value: {type: 3, double_value: 1.0}}]}"
    • Verify that in the share directory (within the install folder of the workspace), ada_feeding_action_servers_current.yaml has changed to reflect the overridden parameter.
    • Verify that in the output of the node, you see some info logs indicating that the MoveAbovePlate tree was re-created.
  • Get the value of the param in default and verify that it is unchanged (should be 4.0): e.g., ros2 service call /ada_feeding_action_servers/get_parameters rcl_interfaces/srv/GetParameters "{names: ['default.MoveAbovePlate.tree_kwargs.f_mag']}"
  • Get the current value of the param and verify it is set correctly (should be 1.0): e.g., ros2 service call /ada_feeding_action_servers/get_parameters rcl_interfaces/srv/GetParameters "{names: ['current.MoveAbovePlate.tree_kwargs.f_mag']}"
• Testing with MoveIt
  • Launch MoveIt: ros2 launch ada_moveit demo.launch.py sim:=mock
  • Move the robot to the user's mouth, and verify that it stops 2.5cm away from the mouth.
    • ros2 action send_goal /MoveToStagingConfiguration ada_feeding_msgs/action/MoveTo "{}" --feedback
    • ros2 action send_goal /MoveToMouth ada_feeding_msgs/action/MoveToMouth "{}" --feedback
  • Move back to the staging configuration: ros2 action send_goal /MoveFromMouthToStagingConfiguration ada_feeding_msgs/action/MoveTo "{}" --feedback
  • Change the distance to mouth to be 10cm, and verify that it succeeds: ros2 service call /ada_feeding_action_servers/set_parameters rcl_interfaces/srv/SetParameters "{parameters: [{name: 'current.MoveToMouth.tree_kwargs.plan_distance_from_mouth', value: {type: 8, double_array_value: [0.10, 0.0, -0.01]}}]}"
  • Move to the mouth, and verify that it stops 10cm away: ros2 action send_goal /MoveToMouth ada_feeding_msgs/action/MoveToMouth "{}" --feedback
  • Verify that the parameter change persists across runs of the code:
    • Terminate the ada_feeding_launch.xml code and the ada_moveit code.
    • Re-run both
    • Move the robot to the user's mouth, and verify that it stops 10cm away from the mouth.
      • ros2 action send_goal /MoveToStagingConfiguration ada_feeding_msgs/action/MoveTo "{}" --feedback
      • ros2 action send_goal /MoveToMouth ada_feeding_msgs/action/MoveToMouth "{}" --feedback

Before opening a pull request

• Format your code using black formatter python3 -m black .
• Run your code through pylint and address all warnings/errors. The only warnings that are acceptable to not address is TODOs that should be addressed in a future PR. From the top-level ada_feeding directory, run: pylint --recursive=y --rcfile=.pylintrc ..

Before Merging

• Squash & Merge

[egordon]

Generally fine given the test cases. Just one nit and 2 non-fatal concerns.
This is a pretty elegant solution for saving parameters!

[egordon]

Doe we expect this to ever not be 3D? When?

If not, we should keep it a Tuple to enforce the size.

[amalnanavati]

We always expect it to be length 3, but the issue is that ROS2 params cannot be of type tuple, only type list. And Python's typing interface doesn't currently allow us to specify a length for Lists.

However, I found this discussion in the mypy library, which is a static type checker for Python. So I'll change it to Annotated[Sequence[float], 3] so that future Python type checkers can verify it.

[egordon]

Nit: you can use .values

[egordon]

Now that this is in a commit and is updated by code, it should be added to .gitignore.

[amalnanavati]

I initially put it in the .gitignore, but then realized that .gitignore doesn't do anything for files that are already in the repository. It seems like (based on this thread) the only thing we can do is running git update-index --assume-unchanged ada_feeding/config/ada_feeding_action_servers_current.yaml in our local copy, but we can't do any global configurations to ignore changes to this file.

I'd say we just have to be vigilant in PRs to verify changes to this file are not pushed.

[amalnanavati]

Actually, I think update-index --assume-unchanged can mess up some git checks; like it thought I had local changes in the current.yaml file even though I didn't, so prevented me from changing branches. I'd say we should not use that command, and instead just be vigilant on the PR side to verify changes to this file don't get passed in.