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

[WIP] TimeOut feature #14

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

chhtz
Copy link
Contributor

@chhtz chhtz commented Sep 27, 2024

Added new state NO_POSITION_UPDATES and flag to stop when no input poses arrived for more than transformer_max_latency.
To be compatible with (possibly lagging) simulation time, also added a simulated_time port.

This can be slightly cleaned up, e.g., on timeout the whole trajectory follower logic does not need to be executed.
Also, maybe transformer_max_latency should not be re-used for this feature.

…poses arrived for more than transformer_max_latency.

To be compatible with (possibly lagging) simulation time, also added a `simulated_time` port.
@chhtz
Copy link
Contributor Author

chhtz commented Sep 27, 2024

Another point open for discussion is if stopping on timeout should be the default.
Also, instead of sending zero on timeout, one could only send updates when the robot2map UpdateCallback is called (currently the callback just updates lastPoseUpdate) -- this could safe the added simulated_time input port. This would require the connected motion controller to timeout itself, when no new MotionCommands arrive.

@chhtz
Copy link
Contributor Author

chhtz commented Sep 27, 2024

I did not test yet how this behaves if the input transformation is generated from a slow and a fast task, e.g., a (slow) SLAM component which occasionally updates odometry2map, combined with a fast robot2odometry provider (we usually are ok with ignoring the slow odometry2map update rate).

@planthaber
Copy link

When we are watching the transformer timing out for a timeout feature, I think this is ok. As the max_latency parameter also defines how long the transformer waits until there is an error. so it has to be set to a high value to also get the slow transform.

But this leads me to think about other component behavior, when you have a component that integrates the slow and fast transform, this one also has to stop sending transform, and this moves the high/low frequency issue around in the stack. So the fastest reaction time will always be the max_latency in the whole chain.

@Rauldg
Copy link
Contributor

Rauldg commented Sep 30, 2024

Previously the trajectory follower was sending the last (old) motion commands whenever the poses were not updating.

If I understand correctly, this has been fixed and now, when the poses don't get updated, this is what happens:

  • If send_zero_cmd_on_timeout, stop command will be sent.
  • else no command will be sent.

@Rauldg
Copy link
Contributor

Rauldg commented Sep 30, 2024

Why introducing in this same merge request the simulation time? Seems like a different feature. Maybe better to test one thing at a time? Or do you have tests ready also for this simulation time feature @chhtz?

@Rauldg
Copy link
Contributor

Rauldg commented Sep 30, 2024

I see that there is also another change. Now in the error state the trajectory follower sends stop commands. Not sure if this is really what the component should do. Depending on the application, stopping the motion (maybe suddenly) can be bad. Addressing the next motion command once the trajectory follower has gone into error is maybe a task for a failure management component and not for the failed component itself.

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.

3 participants