Clean up general parameter settings in robot.yaml and base.yaml

[mjstn]

We need to finalize changes set in motion over a year ago where we want to avoid use of apt upgrade from wiping out many customer specific settings made in the base.yaml file which can be updated on the apt upgrade.

The goals of this issue are two fold: 1) Document and understand parameter setup as used in shipping Magni images from mid 2020 2) Document and understand how we plan to have many parameters in robot.yaml 3) After a review by product experts of above points we decide our timeline to get all this stuff merged OR modified as required

History: We decided to move many motor controller and other key operating mode parameters from base.yaml to robot.yaml. A lot of work was done to do this by Rohan but the issue has been only creaping along and needs closure so that it will finally be safe to use magni_robot repository along with ubiquity_motor and our main codeline will then be more supportable again.

This issue spans the magni_robot and ubiquity_motor repositories in that the parameters of interest are mostly used in ubiquity_motor but the mechanisms to manage them are mostly in magni_robot launch related files for magni-base service.

How to proceed is first to generate document to explain this mess: Step one of this task will be to form a spreadsheet that shows names of each parameter and which will move to robot.yaml.

Key File Locations to help focus on the task this will be important to inspect and modify:

• base.yaml on image this is in /opt/ros/kinetic/share/magni_bringup/param/base.yaml If the magni_robot repository is under catkin_ws/src then it is in magni_bringup/param/base.yaml in that folder
  • robot.yaml This is always in /etc/ubiquity/robot.yaml ROS workspace will overwrite this (that is the point of this issue)
  • A key launch file: /opt/ros/kinetic/share/magni_bringup/launch/core.launch (also can be overlayed by magni_robot)
  • A key python script that loads robot.yaml is: /opt/ros/kinetic/lib/magni_bringup/launch_core.py

A line for each parameter we pull in from any .yaml file will be in a spreadsheet.

• Show were it comes from in currently shipping product which is most always base.yaml.
• Show our plan which 'should' exist mostly in magni_robot for which parameters will move to robot.yaml
• Show if our launch file(s) setup and put into place defaults in the case the param is not in either .yaml file

Then as notes in this spreadsheet (so we keep this all together) we want to explain the plan for what is in magni_robot repository today for launch files that will use robot.yaml much more for more ubiquity_motor things like PID, wheel characteristics and so on.

We want to document which parameters override others to explain these possibilities and more:

• If a parameter is not present in any .yaml file what will it default to in launch file(s)
• If a parameter is in base.yaml but not in robot.yaml is it ignored or used as a parameter for ubiquity_motor
• If a parameter is only in robot.yaml will it be used to over-rule the default in the launch file(s)
• If a parameter is in both base.yaml and robot.yaml which one 'wins' and is used to over-rule the default in the launch file(s)

[JanJericevic]

(Moved correspondence from Slack for history)

This is the working document describing my work so far, made observations and questions I have. At the end, it describes the next assignment I will work on within the project. I will update this file as I finish individual assignments.

Feedback from @mjstn:

Your specific questions:

• robot.yaml only had raspicam and sonars. Yes that is correct for right now BUT the main point of ALL of this is to move many into robot.yaml which persists through an apt upgrade.
• pid_velocity : We want this to be 0 and not 15000 as part of what I discuss below.
• controller_board_version is sort of old and should be taken out of the files. This was only required a very long time ago in early production.
• I will post separate info on boot sequence to not clutter this post too much.
• Lets discuss the radius multipliers after 'the dust settles' put that question on a 'to be dealt with but not right now list'.

Not in any particular order but here is feedback:

• If a parameter is in robot.yaml it overrides any other location in fact that is the key thing we need to have happen.
• robot.yaml does not get replaced by an apt-upgrade so we want users to be able to save robot.yaml off of the robot and use it for future images (of course being careful to maintain any new added parameters in robot.yaml of the newer image.
• Make sure pid_velocity: is set to 0 in all locations.
• The launch files are supposed to use defaults in case no parameter is in any .yaml at all.
• We must have these parameters be able to be in robot.yaml and as stated override other same name params in launch and other yamls.
• pid_proportional: 4000, pid_integral: 2, pid_derivative: -110, pid_moving_buffer_size: 40, pid_velocity: 0
• There are two missing parameters that need to be in robot.yaml and I don't know how we 'lost' them. Also have in robot.yaml wheel_type: "standard" and fw_max_pwm: 300

[mjstn]

The specific goal here is to move the following parameters so they are in robot.yaml and do not come from base.yaml. The parameters below will exist in robot.yaml and will no longer be present in base.yaml

This might require changes to launch_core.py or just verification that is is working and getting parameters from robot.yaml

You need to understand how the system starts and it is complicated. Here are some basic notes of mine:

• The start of magni-base.service runs magni-base script that does setup but also 'roslaunch magni_demos teleop.launch'
• teleop.launch is simple but does an include to launch magni_bringup/launch/base.launch and the fun begins
• base.launch will run magni_bringup/launch_core.py
• launch_core.py reads in config params in robot.yaml and at the end passes params as it launches core.launch
• core.launch is where base.yaml parameters are pulled in and core.launch sets up all the params and at the end launches the motor node

So core.launch must get the parameters below from robot.yaml and they will no longer be in base.yaml

serial_port: "/dev/ttyAMA0" pid_proportional: 5000 pid_integral: 2 pid_derivative: -100 pid_denominator: 1000 pid_moving_buffer_size: 50 pid_velocity: 0

Wheel separation and radius multipliers

wheel_separation_multiplier: 1.0 # default: 1.0 wheel_radius_multiplier : 1.0 # default: 1.0

Velocity and acceleration limits

Whenever a min* is unspecified, default to -max*

linear: x: has_velocity_limits : false max_velocity : 1.0 # m/s has_acceleration_limits: true max_acceleration : 0.5 # m/s^2 angular: z: has_velocity_limits : false max_velocity : 2.0 # rad/s has_acceleration_limits: true max_acceleration : 5.0 # rad/s^2 (edited)

Also in robot.yaml since it is related to these parameters we want to add OLED display enabled here

oled_display: controller : SH1106

[mjstn]

I will now explain roughly how these parameters get setup: (all in ubiquity_motor repository)

• motor_hardware.cc sets these up as internal parameters for the node but all are set to -1 (undefined)
• motor_parameters.h defines all these as members of FirmwareParams class and each is set to a default value in motor_parameters.h
• Also in class FirmwareParams in its constructor all these parameters AND ALL THE ONES WE ARE PUTTING IN robot.yaml will be then initialized IF THEY WERE PASSED INTO motor node like we did in core.launch

There are MANY parameters that do not show up at all that the motor node supports so they are all defaulted.
For the parameters we do pull in from the base.yaml and robot.yaml files those get passed in as ROS parameters as the motor node is launched in core.launch and then the motor node uses passed in params instead of defaults.

[JanezCim]

Please be advised that robot.yaml is not static - we are adding parameters in other projects (like extra parameter for lidar location and parameter weather or not the imu fusion is enabled). So before changing things here - which is prone to break a lot of stuff - @JanJericevic i suggest we have a call or something to talk about how those new parameters are usually added to projets that require custom setups.

[JanJericevic]

I created a branch in magni_robot repo. All of the changes so far can be viewed there.

[rohbotics]

from Slack

From @JanJericevic today: I'm curious why the robot.yaml file and its parameters are imported this way. Is there a specific reason why the parameters are imported using the py script and not by the tag in a launch file (as is base.yaml)? I'm asking because of the talk that we had the other day @mjstn. If the robot.yaml file could be included in such a way it would, I think, simplify things in regards to importing additional user-made parameters. Right now, if the user wants to add a parameter to robot.yaml, he also has to change launch_core.py so that it passes the parameter on to core.launch and then change core.launch as well so that it sets the parameter.

Here are the reasons it isn't a rosparam file:

• Explicit "overriding" behavior, which is easy to get subtly wrong with rosparam
• Clearer structure and names that we can set, instead of having to use the rosparam paths
• Part of the vision was to support non-rosparam-able configuration settings, like what app to launch by default
• Not everything in there is actually a ROS parameter right now, like force_time_sync or the camera position which is passed directly to xacro
• Allow for string comparison if statements for determining which nodes to launch, ex: "pi_sonar_v1" vs only having bools

If we are really trying to clean this up, it might make more sense for launch_core.py to generate the roslaunch XML based on the parameters. That might reduce the number of places we have to touch to add a new parameter. It would also allow for switching between different nodes at launch easier, for things like picking which lidar we have etc.

Also I would still keep the internal base.yaml for non-user editable "parameters".

[JanJericevic]

The incorporation of base.yaml file was left as is. My question came from talking to @mjstn how the robot.yaml file could eventually include many parameters (ours and user-added), because of which the launch_core.py and core.launch files could become overcluttered. Right now the only thing we did was move some of the parameters to robot.yaml and change the launch_core.py and core.launch so that they fit the moved parameters. The only thing that changed from a functionality perspective was the checking function in launch_core.py that checks if all the "default parameters" are included in robot.yaml.

So how do we proceed? Should I move to exploring the idea of generating the XML?

[rohbotics]

@JanJericevic I think for the purposes of this issue, the XML generation is way out of scope. I have created a new one https://github.com/UbiquityRobotics/magni_robot/issues/159

This issue should be closed as soon as the simple cleanup changes you have done are merged in.

[JanJericevic]

Ok I understand, thank you.