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

Fix loss of first waypoint in upsample trajectory #416

Merged

Conversation

thettasch
Copy link
Contributor

The current implementation of the upscale trajectory task fails to pass the first waypoint through to the output. This small patch fixes the problem. I have also adjusted the tests to account for the extra point in the outputs. I have tested this successfully with tesseract 0.20.2.

Demonstration of bug

I ran the following dummy trajectory through the UpsampleTrajectoryTask with longest_valid_segment_length=0.5:

Composite Instruction, Description: Tesseract Composite Instruction
{
  Move Instruction, Move Type: 1, State WP: Pos=0 0 0 0 0 0
, Description: Tesseract Move Instruction
  Move Instruction, Move Type: 1, State WP: Pos=1 0 0 0 0 0
, Description: Tesseract Move Instruction
  Move Instruction, Move Type: 1, State WP: Pos=2 0 0 0 0 0
, Description: Tesseract Move Instruction
  Move Instruction, Move Type: 1, State WP: Pos=3 0 0 0 0 0
, Description: Tesseract Move Instruction
}

The current implementation returns the following. Note the loss of the 0 0 0 0 0 0 waypoint.

Composite Instruction, Description: Tesseract Composite Instruction
{
  Move Instruction, Move Type: 1, State WP: Pos=0.333333        0        0        0        0        0
, Description: Tesseract Move Instruction
  Move Instruction, Move Type: 1, State WP: Pos=0.666667        0        0        0        0        0
, Description: Tesseract Move Instruction
  Move Instruction, Move Type: 1, State WP: Pos=1 0 0 0 0 0
, Description: Tesseract Move Instruction
  Move Instruction, Move Type: 1, State WP: Pos=1.33333       0       0       0       0       0
, Description: Tesseract Move Instruction
  Move Instruction, Move Type: 1, State WP: Pos=1.66667       0       0       0       0       0
, Description: Tesseract Move Instruction
  Move Instruction, Move Type: 1, State WP: Pos=2 0 0 0 0 0
, Description: Tesseract Move Instruction
  Move Instruction, Move Type: 1, State WP: Pos=2.33333       0       0       0       0       0
, Description: Tesseract Move Instruction
  Move Instruction, Move Type: 1, State WP: Pos=2.66667       0       0       0       0       0
, Description: Tesseract Move Instruction
  Move Instruction, Move Type: 1, State WP: Pos=3 0 0 0 0 0
, Description: Tesseract Move Instruction
}

After applying my simple fix, the following is returned:

Composite Instruction, Description: Tesseract Composite Instruction
{
  Move Instruction, Move Type: 1, State WP: Pos=0 0 0 0 0 0
, Description: Tesseract Move Instruction
  Move Instruction, Move Type: 1, State WP: Pos=0.333333        0        0        0        0        0
, Description: Tesseract Move Instruction
  Move Instruction, Move Type: 1, State WP: Pos=0.666667        0        0        0        0        0
, Description: Tesseract Move Instruction
  Move Instruction, Move Type: 1, State WP: Pos=1 0 0 0 0 0
, Description: Tesseract Move Instruction
  Move Instruction, Move Type: 1, State WP: Pos=1.33333       0       0       0       0       0
, Description: Tesseract Move Instruction
  Move Instruction, Move Type: 1, State WP: Pos=1.66667       0       0       0       0       0
, Description: Tesseract Move Instruction
  Move Instruction, Move Type: 1, State WP: Pos=2 0 0 0 0 0
, Description: Tesseract Move Instruction
  Move Instruction, Move Type: 1, State WP: Pos=2.33333       0       0       0       0       0
, Description: Tesseract Move Instruction
  Move Instruction, Move Type: 1, State WP: Pos=2.66667       0       0       0       0       0
, Description: Tesseract Move Instruction
  Move Instruction, Move Type: 1, State WP: Pos=3 0 0 0 0 0
, Description: Tesseract Move Instruction
}

Why the bug occurs

When looping through the waypoints of a composite instruction, pairs of waypoints are examined and interpolated between if necessary. To avoid duplication of the endpoints, the loop at line 162 ignores the first point (since it was already added as the last point of the previous interpolation). In doing so, the very first waypoint is lost when continue is called at line 140. The fix is simply to add the very first waypoint to the resulting composite instruction.

@Levi-Armstrong
Copy link
Contributor

Can you rebase on master?

@thettasch thettasch force-pushed the fix/upsample-trajectory-bug branch from 7c454ad to e6c1446 Compare November 17, 2023 08:39
@thettasch
Copy link
Contributor Author

Can you rebase on master?

Done

Copy link
Contributor

@Levi-Armstrong Levi-Armstrong left a comment

Choose a reason for hiding this comment

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

LGTM, pending pipeline

@Levi-Armstrong Levi-Armstrong merged commit a220f30 into tesseract-robotics:master Nov 17, 2023
10 checks passed
@Levi-Armstrong
Copy link
Contributor

Levi-Armstrong commented Nov 17, 2023

Thank you for the fix!

@thettasch
Copy link
Contributor Author

It's a pleasure!

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.

2 participants