Skip to content

Conversation

@Rongrrz
Copy link
Contributor

@Rongrrz Rongrrz commented Feb 14, 2025

Why are we doing this?

Vision wants some cool curves (minimize driving time and potentially award-worthy?)

What's changing?

A new command that builds off of the SwerveSimpleTrajectoryCommand logic and does currrrrvesss

Questions/notes for reviewers

Vision wants this thing to be HEAVILY REVIEWED for ANY potential optimizations. Logic written by AI has been pointed out in the JavaDocs and we can only pray that it IS the best way written.

How this was tested

  • unit tests added
  • tested on robot

@Rongrrz Rongrrz requested a review from a team as a code owner February 14, 2025 06:38
for (int i = 1; i <= steps; i++) {
double lerpFraction = i / (double) steps;
XbotSwervePoint point = new XbotSwervePoint(
deCasteljauIterative(allPoints, lerpFraction),
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Both videos are worth watching in their entirety, but the first one has a section about deCasteljau early on

* @param endPoint of our command
* @param steps to split our Bézier curve into
*/
public void setBezierConfiguration(List<Translation2d> controlPoints, Pose2d endPoint, int steps) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we know the n of the highest order bezier curve the vision coprocessor will be sending us?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't expect too-too much, probably 2-3 control points, no more than 5. I don't think we need a bajillion control points to traverse through an obstacle.

List<Translation2d> temp = new ArrayList<>(points);

// Compute the position using de Casteljau's algorithm (all we know is that it works)
for (int level = 1; level < n; level++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You could do some profiling on the Rio, for example, using higher and higher order bezier curves and see how long this function takes to calculate. This would help us set an upper bound for how many points the Rio is willing to accept

Copy link
Contributor

Choose a reason for hiding this comment

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

agreed, level^2 gets scary fast!

@JohnGilb
Copy link
Contributor

High level question for @Kobeeeef and @Rongrrz - do we want to be sending n-order bezier curves from vision->core? Sending a series of connected cubic bezier or connected cubic hermite would allow for more complex paths while making computation much easier (probably on both sides)

@Kobeeeef
Copy link
Contributor

High level question for @Kobeeeef and @Rongrrz - do we want to be sending n-order bezier curves from vision->core? Sending a series of connected cubic bezier or connected cubic hermite would allow for more complex paths while making computation much easier (probably on both sides)

yes I believe having multiple curves is a lot better, as there will be less data needing to be transferred & computed and also allowing for much easier sharp turns.

List<Translation2d> temp = new ArrayList<>(points);

// Compute the position using de Casteljau's algorithm (all we know is that it works)
for (int level = 1; level < n; level++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

agreed, level^2 gets scary fast!

* @param lerpFraction is our completion percentage
* @return our position during lerpFraction (progress of operation completion)
*/
private Translation2d deCasteljauIterative(List<Translation2d> points, double lerpFraction) {
Copy link
Contributor

Choose a reason for hiding this comment

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

⭐ I wonder if these should be static to make testing easier?

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.

5 participants