Skip to content

Document node parameters and switch to config-file based launch - #3

Merged
mmmarinho merged 2 commits into
jazzyfrom
feature/node-parameters-and-config-launch
Aug 25, 2026
Merged

Document node parameters and switch to config-file based launch#3
mmmarinho merged 2 commits into
jazzyfrom
feature/node-parameters-and-config-launch

Conversation

@mmmarinho

Copy link
Copy Markdown
Contributor

Summary

Applies the standard "ROS 2 Nodes & Parameters" README section and converts the launch file to the config-file-based pattern used by sas_robot_driver_ur.

  • Renames launch/sas_force_sensor_bota_launch.pylaunch/force_sensor_launch.py and converts it to load parameters from config/config.yaml via name + config_file launch arguments.
  • The hardware-specific configuration_file_path (a JSON file) stays a launch argument, now defaulting to config/bota_binary_gen0.json in the package share directory — replacing the previous fragile ../config/ relative-path default.
  • Adds config/config.yaml with the topic_name and sampling_time parameters.
  • Bug fix: the data_files glob in setup.py only installed config/*.json; added a config/*.yaml entry so the new YAML is installed into the share directory.
  • Adds a ROS 2 Nodes & Parameters README section documenting every parameter of the node (type, mandatory/optional, default, purpose).
  • Updates the docker compose files and README to the renamed launch file.

Verification

  • python3 -m py_compile launch/force_sensor_launch.py passes.
  • config/config.yaml parses; setup.py AST-parses.
  • Parameter names cross-checked against force_sensor_bota.py.

Note: this PR was created by an AI agent (OpenHands) on behalf of the user.

- Rename launch/sas_force_sensor_bota_launch.py to
  launch/force_sensor_launch.py and convert it to load parameters from
  config/config.yaml (name + config_file launch arguments). The
  hardware-specific configuration_file_path stays a launch argument,
  with a default pointing at config/bota_binary_gen0.json.
- Add config/config.yaml with the topic_name and sampling_time parameters.
- Fix the dangling data_files glob in setup.py to also install the
  config/*.yaml file.
- Add a 'ROS 2 Nodes & Parameters' section to the README documenting
  every parameter of the node.
- Update the docker compose files and README to the renamed launch file.

Co-authored-by: openhands <openhands@all-hands.dev>

@mmmarinho mmmarinho left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The path to the .json should be embedded in the .yml

Address review feedback: the hardware-specific configuration_file_path is
no longer a launch argument; it is embedded in the YAML configuration files
(config/config.yaml -> bota_binary_gen0.json and the new
config/config_ethercat.yaml -> ethercat_gen0.json).

- force_sensor_launch.py drops the configuration_file_path launch argument
  and loads everything from the config file.
- The node resolves a relative configuration_file_path against the package
  config/ directory (absolute paths still work), so the embedded filenames
  resolve correctly regardless of the install prefix (colcon build in
  docker or system install).
- docker/compose_ethercat.yml now selects the EtherCAT variant via
  config_file:=.../config_ethercat.yaml instead of a JSON path launch arg.
- README updated accordingly.

Co-authored-by: openhands <openhands@all-hands.dev>
@mmmarinho

Copy link
Copy Markdown
Contributor Author

Done — the JSON path is now embedded in the YAML config files (commit 4e510a8):

  • config/config.yaml now contains configuration_file_path: bota_binary_gen0.json and a new config/config_ethercat.yaml contains ethercat_gen0.json, so the hardware variant is selected by the YAML file rather than a launch argument.
  • force_sensor_launch.py no longer declares a configuration_file_path launch argument; all parameters come from the config file.
  • docker/compose_ethercat.yml now selects the variant with config_file:=$$(ros2 pkg prefix sas_force_sensor_bota --share)/config/config_ethercat.yaml.

One deliberate deviation from the UR-style absolute /opt/ros/jazzy/share/... path: bota's docker image removes the apt package and does a colcon build into the dev workspace, so a hardcoded /opt/ros/... path would not resolve inside the container. Instead the YAML stores the JSON filename and the node resolves a non-absolute configuration_file_path against the package config/ directory (absolute paths still work as before, so existing setups are unaffected). This keeps the path embedded in the YAML while remaining install-prefix independent. The README documents the new behavior.

This comment was created by an AI agent (OpenHands) on behalf of the user.

@mmmarinho mmmarinho left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@mmmarinho
mmmarinho marked this pull request as ready for review August 25, 2026 08:27
@mmmarinho
mmmarinho merged commit bf969f2 into jazzy Aug 25, 2026
5 checks passed
@mmmarinho
mmmarinho deleted the feature/node-parameters-and-config-launch branch August 25, 2026 08:27
@mmmarinho

Copy link
Copy Markdown
Contributor Author

Thanks for the review — glad the embedded JSON path approach works for you. Let me know if there is anything else to adjust.

This comment was created by an AI agent (OpenHands) on behalf of the user.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants