Conversation
| for (auto loaded_controller : rt_controller_list) | ||
| { | ||
| // TODO(v-lopez) we could cache this information | ||
| // https://github.com/ros-controls/ros2_control/issues/153 |
There was a problem hiding this comment.
This issue was closed
| while (rclcpp::ok()) | ||
| { | ||
| rclcpp::Time begin_last = begin; | ||
| begin = cm->now(); |
There was a problem hiding this comment.
Start the timer right at the beginning of the new iteration
jackcenter
left a comment
jackcenter
left a comment
There was a problem hiding this comment.
I think this is an improvement, but I had one comment on the loop implementation that I'd be interested to know your thoughts on.
| cm->write(); | ||
| rclcpp::Time end = cm->now(); | ||
| end = cm->now(); | ||
| std::this_thread::sleep_for(std::max( |
There was a problem hiding this comment.
This looks like it would cause the time to drift each loop since it's sleeping for a fixed amount of time instead of until the end of the period. Would ros::rate work here? Or setting a hard end time at the beginning of each loop based on the original start? I can test that out if it seems like a reasonable approach.
There was a problem hiding this comment.
If you look at L35, there's a comment about why rclcpp::Rate::sleep() isn't used.
What's here seems fine to me but please feel free to test out your other idea. 👍 This still isn't working as well as I'd hope for. (It's not clear if this bit of code is the cause, though)
| while (rclcpp::ok()) | ||
| { | ||
| rclcpp::Time begin_last = begin; | ||
| begin = cm->now(); |
There was a problem hiding this comment.
This does seem like a better order.
|
I added some testing data for this PR to PR #647. |
|
Replaced by #647 that seems to work better. |
| begin = cm->now(); | ||
| cm->read(); |
There was a problem hiding this comment.
I think we should rather use something like this:
| begin = cm->now(); | |
| cm->read(); | |
| cm->read(); | |
| begin = cm->now(); |
In this case, only update method is not counted in the period. This is not 100% exact when having multiple controller but closer to the real value.
3 participants
Move
begin_last = begin;inside the period measurement. This should make the control loop period measurement more accurate because all of the work in the control loop will now be accounted for.Partial fix to #644 but it is not a complete fix.