Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

First hacky implementation of reusing the initial_positions for view_ur #24

Draft
wants to merge 1 commit into
base: ros2
Choose a base branch
from

Conversation

fmauch
Copy link
Collaborator

@fmauch fmauch commented Feb 24, 2022

As we have the config file in place already, anyway, we can also use the parameters.

There are a couple of things I don't like about this implementation:

  • Parsing the yaml file manually seems awkward to me. However, as far as I understood, passing parameter files to nodes only supports yaml files following a certain syntax, which the initial_positions.yaml is not. (See possible workaround 1)
  • I could not manage to make the actual filename configurable as using a LaunchConfiguration object. But that might just be me being stupid.

Possible workaround 1: Include separate file
With defining a separate file with content

joint_state_publisher:
  ros__parameters:
    zeros:
      shoulder_pan_joint: 0.0
      shoulder_lift_joint: -1.57
      elbow_joint: 0.0
      wrist_1_joint: -1.57
      wrist_2_joint: 0.0
      wrist_3_joint: 0.0

we could pass that file to the node directly. However, this would be config duplication.

As I couldn't come up with a better solution, I postet a question on ROS answers: https://answers.ros.org/question/396653/ros2-python-launchfile-load-parametersyaml-into-nodes-parameter/

As we have the config file in place already, anyway, we can also use the parameters.
@fmauch
Copy link
Collaborator Author

fmauch commented Feb 25, 2022

I've actually found that pattern in the industrial-training

print(initial_positions_file)
with open(initial_positions_file, "r") as stream:
try:
initial_positions = yaml.safe_load(stream)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like much this direct dependency on yaml.

Can we do something like in Driver#308

robot_state_publisher_node,
rviz_node,
]
nodes_to_start = [joint_state_publisher_node, robot_state_publisher_node, rviz_node]
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer to keep this "multi-row" because it is easier to read and add nodes if needed. I expect people to copy-paste and extend this file.

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