-
Notifications
You must be signed in to change notification settings - Fork 310
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
Update rate incorrect if using sim time #859
Comments
Hi @joshjowen, thanks for this report. Yes, this could be an issue. And you are right about usage of steady clock. I think we have this change in an open PR by @AndyZe about What I m confused about is that you mention usage of |
Hi @destogl, I'm not sure exactly what you mean about "input of commands". I have been using it with a range of controllers including 'joint_trajectory_controller', 'forward_command_controller' and some of our own. It all works as expected. When I said "steady time would make more sense than system time for real hardware" I was not referring to using sim time on the real hardware, i was saying that steady clock would be more suitable on real hardware than the system clock. In our machines, we will come in and out of GPS signals and in this scenario: PPS corrections to the system clock would skew the control rate when the system clock is used. The only real issue I can see with my solution is that, when using sim time, the steady clock control rate would not scale with the real time factor of the sim. e.g. if you have a real time factor of 0.5, the 100Hz control rate would run at effectively 200Hz wrt the sim |
Hi, Have there been any developments on this? My team is having this issue in our project and possibly we can fix it. Just want to make sure we're not duplicating someone else's work. |
Hello @krzychut,
Are you facing the issue with |
Hi @saikishor |
I experience the same thing. Both rolling and humble. My simulator publishes its time on the /clock topic. One possible workaround is to set the simulation time to the system time when starting the simulation. And then make sure the simulation is active and publishing on /clock before starting the ros2_control_node's main loop. Maybe not a workaround possible for all. And I do not like that my system depends on the startup order in that way. So would be nice with an update to the control_node that makes it possible to use it with use_sim_time. |
Let's see what you have on a PR :) |
IMO the main issue to solve is giving the controller manager some idea of the simulations' real time factor. If our simulation is running at 0.5 realtime, we would want the controllers to reflect that. More importantly, if the simulation is running faster than real time, we need the controllers to keep up. TBH I'm not sure what the best solution to this would be. Just subscribing to My only idea that "kind of" works is to spin a This is obviously not a perfect solution, but it's pretty simple, it adjusts the update rate match the simulation speed, and it's certainly better than spamming 5k commands per second. I will try to sketch something up and see how it works. |
Hello @krzychut! I personally think this is has to be handled differently in a different node. For instance, in case of the Gazebo, the I think it is very hard to make it very generic as one simulation might use @bmagyar correct me if I'm wrong. Best Regards, |
I tried the example reported in this issue. Following the suggestion of @saikishor I'd also suggest to do this in a different node which can be hosted here or in topic_based_ros2_control. @krzychut have you figured out a solution already and want to contribute it with a PR? |
The example I uploaded here is not use topic_based_ros2_control. |
@daihen-hijikata I know, but as written above: I don't see a use-case for running ros2_control_node with use_sime_time=true, which does not work well as reported here and by you in the other issue. Someone will have to write a different node with the synchronization to the clock. |
Related #802 |
just FYI, I did try out changing the control loop to use use rclcpp::Rate and it worked as expected with use_sim_time:=true. There are some gazebo plugins which don't play nice with the ros2 gazebo plugin and I found it was easier to just write gz topic based hardware interfaces for those cases and run the normal controller_manager node rather than having to go change those gazebo plugins. For example, the thrusters gazebo plugin does not allow you to spin the propeller via control2 and have it generate thrust. |
Hello! Are you referring to the ros2_control_node executable? |
Yes. I dont have the exact code I tried in front of me, but here's the chatgpt version of that change
|
Describe the bug
When the 'use_sim_time' parameter is set, the ros2_control_node's main loop runs at a rate different then what is provided to the 'update_rate' parameter. In my case, update_rate is set to 100 Hz and the loop runs at ~2.5 kHz.
To Reproduce
With a simulated robot and the /clock topic available
Steps to reproduce the behavior:
Expected behavior
The /joint_states topic publishes at 100Hz or some number related to that and the real_time_factor of the sim
Environment (please complete the following information):
I am able to get it to run at the correct rate if i modify
ros2_control/controller_manager/src/ros2_control_node.cpp
Lines 65 to 66 in 62a3d26
std::chrono::steady_clock::time_point next_iteration_time = std::chrono::steady_clock::now();
which will obviously run at 100 Hz in real time. I am not sure if the intended behavior would be for the rate to be sim time instead, but steady time would make more sense than system time for real hardware anyway.
The text was updated successfully, but these errors were encountered: