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

[jtc] Improve trajectory sampling efficiency #1297

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

RobertWilbrandt
Copy link

This PR addresses the increasing effort required in Trajectory::sample() raised in #1293 by exploiting the monotonically increasing access time for any trajectory. Instead of scanning through the complete trajectory to find the relevant trajectory points for the current sampling time, the Trajectory class keeps track of the last accessed index and only searches from there on. This supersedes the proposed binary search in #1294 and is based on #474.

Based on the requests in #474, i added checks for the new trajectory state to existing tests and added a new test to make sure that the index is correctly reset when calling Trajectory::update(). I verified that this indeed solves the problem by re-running the trajectory from #1293 and again plotting the periods between write() calls in my hardware interface:

jtc_cmp

The upper plot shows the progression of the period before this change, the lower one the behavior wtih this PR applied. The large spikes at the beginning and end are the start and end of the trajectory, the spikes in the middle are probably a result of me testing this on my normal laptop without RT kernel and doing some things on the side.

Copy link
Contributor

@christophfroehlich christophfroehlich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

Copy link

codecov bot commented Sep 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.43%. Comparing base (50036e1) to head (59d15a5).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1297      +/-   ##
==========================================
+ Coverage   80.36%   80.43%   +0.07%     
==========================================
  Files         105      105              
  Lines        9390     9430      +40     
  Branches      828      829       +1     
==========================================
+ Hits         7546     7585      +39     
  Misses       1570     1570              
- Partials      274      275       +1     
Flag Coverage Δ
unittests 80.43% <100.00%> (+0.07%) ⬆️

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

Files with missing lines Coverage Δ
...include/joint_trajectory_controller/trajectory.hpp 75.00% <100.00%> (+1.66%) ⬆️
joint_trajectory_controller/src/trajectory.cpp 91.30% <100.00%> (+0.14%) ⬆️
...int_trajectory_controller/test/test_trajectory.cpp 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

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.

[jtc] Required time for controller update increases over the course of trajectory
2 participants