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

Use binary search in Trajectory::sample() #1294

Conversation

RobertWilbrandt
Copy link
Contributor

This PR addresses #1293 by using binary search instead of the current linear scan in Trajectory::sample(). Apart from timing, this should not change sampling behavior in any case.

I verified this change on my hardware in the scenario described in the issue and could compare the control frequency during trajectory execution of a trajectory with 222.000 points:

jtc_timing_cmp

Copy link

codecov bot commented Sep 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.34%. Comparing base (57c50e5) to head (c0149a6).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1294   +/-   ##
=======================================
  Coverage   80.34%   80.34%           
=======================================
  Files         105      105           
  Lines        9393     9384    -9     
  Branches      831      828    -3     
=======================================
- Hits         7547     7540    -7     
  Misses       1570     1570           
+ Partials      276      274    -2     
Flag Coverage Δ
unittests 80.34% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
joint_trajectory_controller/src/trajectory.cpp 91.86% <100.00%> (+0.70%) ⬆️

@RobertWilbrandt
Copy link
Contributor Author

As there is no need for ever accessing a trajectory at not monotonically increasing sample times, this approach doesn't offer any benefit over the one posted in #474. I'll try to create a PR based on that.

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.

1 participant