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

add ros parameters file to node context #60

Merged
merged 7 commits into from
Mar 3, 2021
Merged

add ros parameters file to node context #60

merged 7 commits into from
Mar 3, 2021

Conversation

Karsten1987
Copy link
Contributor

fixes #59

With this PR, the actual parameters are being set:

 ➭ ros2 param list
...
/controller_manager:
  joint_state_controller.type
  joint_trajectory_controller.type
  update_rate
  use_sim_time

While implementing this, I found actually a second bug in which all specified controllers in the loaded yaml file would be loaded. It didn't show in the example, however this would necessarily lead to resource conflicts when more than one controller use the same command interface.

Signed-off-by: Karsten Knese <Karsten1987@users.noreply.github.com>
Signed-off-by: Karsten Knese <Karsten1987@users.noreply.github.com>
Copy link
Collaborator

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

@Karsten1987 I was testing your contribution but I was not able to run the example or get the /joint_states. You didn't launch the controllers.

@Karsten1987
Copy link
Contributor Author

You didn't launch the controllers.

That's on purpose. As mentioned above, not all controllers can be loaded/configured at the same time due to resource conflicts. Also I believe it's not good practice to launch them within the gazebo plugin. I'd rather use either the spawner functionality (once available) or load them explicitly via services through a launch file.

@ahcorde
Copy link
Collaborator

ahcorde commented Feb 17, 2021

I will have a look to the PR ros-controls/ros2_control#310

anyhow the conditional to get the udpate_rate is required.

@Karsten1987
Copy link
Contributor Author

Karsten1987 commented Feb 17, 2021

you don't necessarily need the spawner PR. You should be able to use ros2 control load_start_controller in the launch file to start the controllers.

Not sure we need to manually parse the parameters to get the update_rate. We should be able to extract that parameter via a get_parameter call to controller manager.

Signed-off-by: Karsten Knese <Karsten1987@users.noreply.github.com>
@Karsten1987
Copy link
Contributor Author

anyhow the conditional to get the udpate_rate is required.

done in 1047c22

@ahcorde
Copy link
Collaborator

ahcorde commented Feb 18, 2021

@Karsten1987

I was testing the ros2 control commands. I was able to start the joint_state_controller

ros2 control load_start_controller joint_trajectory_controller` 

But I'm not able to load the joint_trajectory_controller. Digging about what it's happening here. The /joint_trajectory_controller parameters are not set.

ros2 param get /joint_trajectory_controller  joints
String values are: []

You should set up the parameters for the controller in the code.

@Karsten1987 Karsten1987 changed the title use upstream yaml parser add ros parameters file to node context Feb 24, 2021
Signed-off-by: Karsten Knese <Karsten1987@users.noreply.github.com>
@Karsten1987
Copy link
Contributor Author

There's currently no clear way to set parameters to a gazebo plugin's node. So I had to do this manually by parsing the ros parameter's file and add the arguments to the node's context. That's certainly not a recommended way, but the only feasible at the moment.

/cc @chapulina @jacobperron

Signed-off-by: Karsten Knese <Karsten1987@users.noreply.github.com>
@joshnewans
Copy link

I just wanted to add my 2c and say that as someone fairly new to all this, I found it quite confusing that the plugin loaded my controllers on its own as I expected a flow similar to my real robot where I used ros2 control load_start_controller xxx. The new behaviour resulting from this PR seemed more intuitive to me.

This PR resolved an issue I was having where the controller_manager and its child controllers did not obey the use_sim_time parameter I specified in the yaml file (and as I understand it there is currently no way to globally enable use_sim_time).

@Karsten1987
Copy link
Contributor Author

@ahcorde friendly ping.

Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
Copy link
Collaborator

ahcorde commented Mar 2, 2021

I created a PR which builds on top of this PR load the controllers in the launch files: #64

Load controllers in the example launch files
@ahcorde ahcorde merged commit c23f70a into master Mar 3, 2021
@ahcorde ahcorde deleted the fix_params branch March 3, 2021 06:49
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.

Custom parameter parsing not working
3 participants