-
Notifications
You must be signed in to change notification settings - Fork 94
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 before end markers #126
base: master
Are you sure you want to change the base?
Add before end markers #126
Conversation
Is there a reason that |
They don’t allow negative values. Also it would work since it would involve sleep actions with negative times. |
Ah I forgot I disallowed that after all. I'd rather open that up in the API though than add another method.
I would propose clamping the sleep duration to zero / eliding the sleep if the duration would be negative. |
I think you're misunderstanding the point of these methods. The point is that the new methods offset from the end of the current / next trajectory segment. It is not the same as the start of the current / next trajectory segment. It is useful for when you're not sure how long a trajectory segment will take. |
Understood. Concretely, my counterproposal is to replace .beforeEndTime(2.0) { ... }
.forward(20.0) with .forward(20.0)
.afterTime(-2.0) { ... } |
I misunderstood you the first time because negative values would need extra processing. All done now. The new code is not tested on a real robot, but unit tests show that it produces identical trajectory actions to the old implementation. One question: negative values get treated as 0 in |
Adds the
beforeEndTime
andbeforeEndDisp
markers, which executes the given action a givendt
ords
before the end of the current / next trajectory segment. I believe this is more correct and functional compared to #123, but I did have to change the signature ofMarkerFactory#make
. This code has been tested in production on my team's auto.