Skip to content

Comments

[wpimath] Replace Trajectory API with new one#8172

Draft
zachwaffle4 wants to merge 299 commits intowpilibsuite:2027from
zachwaffle4:trajectory-api
Draft

[wpimath] Replace Trajectory API with new one#8172
zachwaffle4 wants to merge 299 commits intowpilibsuite:2027from
zachwaffle4:trajectory-api

Conversation

@zachwaffle4
Copy link

@zachwaffle4 zachwaffle4 commented Aug 12, 2025

The new API is based on this document.

Java

  • Sample classes
  • Sample interpolation
  • Trajectory classes
  • JSON serialization
  • Protobuf serialization
  • Generation/deserialization API
  • Unit tests: interpolation
  • Unit tests: serialization

C++

  • Sample classes
  • Sample interpolation
  • Trajectory classes
  • JSON serialization
  • Protobuf serialization
  • Generation/deserialization API
  • Unit tests: interpolation
  • Unit tests: serialization

Notes

This PR does not remove the old trajectory generators, even though they aren't really used. The TrajectoryGenerator class was refactored to produce Trajectory<SplineSample>, where SplineSample replaces the old Trajectory.State class. I did that because many tests rely on the old Trajectory class (such as kinematics and Kalman filters), so they have all been updated to use Trajectory<SplineSample instead; in another PR I will move SplineSample and the old TrajectoryGenerator/Config/etc classes into the test source set.

@github-actions
Copy link
Contributor

This PR modifies commands. Please open a corresponding PR in Python Commands and include a link to this PR.

@github-actions github-actions bot added component: wpilibj WPILib Java component: command-based WPILib Command Based Library component: wpimath Math library component: examples 2027 2027 target labels Aug 12, 2025
… new api

Signed-off-by: Zach Harel <zach@zharel.me>
Signed-off-by: Zach Harel <zach@zharel.me>
# Conflicts:
#	wpilibjExamples/src/main/java/edu/wpi/first/wpilibj/examples/simpledifferentialdrivesimulation/Robot.java
#	wpimath/src/main/java/edu/wpi/first/math/controller/LTVDifferentialDriveController.java
#	wpimath/src/main/java/edu/wpi/first/math/controller/LTVUnicycleController.java
#	wpimath/src/main/java/edu/wpi/first/math/trajectory/DifferentialSample.java
#	wpimath/src/main/java/edu/wpi/first/math/trajectory/Trajectory.java
#	wpimath/src/main/java/edu/wpi/first/math/trajectory/TrajectoryGenerator.java
#	wpimath/src/main/java/edu/wpi/first/math/trajectory/TrajectoryParameterizer.java
#	wpimath/src/main/java/edu/wpi/first/math/trajectory/TrajectorySample.java
#	wpimath/src/test/java/edu/wpi/first/math/controller/LTVDifferentialDriveControllerTest.java
#	wpimath/src/test/java/edu/wpi/first/math/controller/LTVUnicycleControllerTest.java
#	wpimath/src/test/java/edu/wpi/first/math/estimator/DifferentialDrivePoseEstimator3dTest.java
#	wpimath/src/test/java/edu/wpi/first/math/estimator/DifferentialDrivePoseEstimatorTest.java
#	wpimath/src/test/java/edu/wpi/first/math/estimator/ExtendedKalmanFilterTest.java
#	wpimath/src/test/java/edu/wpi/first/math/estimator/KalmanFilterTest.java
#	wpimath/src/test/java/edu/wpi/first/math/estimator/MecanumDrivePoseEstimator3dTest.java
#	wpimath/src/test/java/edu/wpi/first/math/estimator/MecanumDrivePoseEstimatorTest.java
#	wpimath/src/test/java/edu/wpi/first/math/estimator/MerweUKFTest.java
#	wpimath/src/test/java/edu/wpi/first/math/estimator/S3UKFTest.java
#	wpimath/src/test/java/edu/wpi/first/math/estimator/SwerveDrivePoseEstimator3dTest.java
#	wpimath/src/test/java/edu/wpi/first/math/estimator/SwerveDrivePoseEstimatorTest.java
#	wpimath/src/test/java/edu/wpi/first/math/kinematics/MecanumDriveOdometry3dTest.java
#	wpimath/src/test/java/edu/wpi/first/math/kinematics/MecanumDriveOdometryTest.java
#	wpimath/src/test/java/edu/wpi/first/math/kinematics/SwerveDriveOdometry3dTest.java
#	wpimath/src/test/java/edu/wpi/first/math/kinematics/SwerveDriveOdometryTest.java
#	wpimath/src/test/java/edu/wpi/first/math/trajectory/CentripetalAccelerationConstraintTest.java
#	wpimath/src/test/java/edu/wpi/first/math/trajectory/DifferentialDriveKinematicsConstraintTest.java
#	wpimath/src/test/java/edu/wpi/first/math/trajectory/DifferentialDriveVoltageConstraintTest.java
#	wpimath/src/test/java/edu/wpi/first/math/trajectory/EllipticalRegionConstraintTest.java
#	wpimath/src/test/java/edu/wpi/first/math/trajectory/RectangularRegionConstraintTest.java
#	wpimath/src/test/java/edu/wpi/first/math/trajectory/TrajectoryConcatenateTest.java
#	wpimath/src/test/java/edu/wpi/first/math/trajectory/TrajectoryGeneratorTest.java
#	wpimath/src/test/java/edu/wpi/first/math/trajectory/TrajectoryTransformTest.java
…rential drive kinematics, voltage, and region enforcement and their relevant tests

Signed-off-by: Zach Harel <zach@zharel.me>
@github-actions github-actions bot removed the component: command-based WPILib Command Based Library label Aug 15, 2025
@zachwaffle4
Copy link
Author

i am very sorry that the git history is kinda cursed because of how i fixed the tests/commands

…sentation; implement interpolation methods and kinematics handling

Signed-off-by: Zach Harel <zach@zharel.me>
…assisSpeeds classes

Signed-off-by: Zach Harel <zach@zharel.me>
…jectory data handling

Signed-off-by: Zach Harel <zach@zharel.me>
…ve fromSample method, and make transform abstract

Signed-off-by: Zach Harel <zach@zharel.me>
…nt interpolation for SwerveModuleState

Signed-off-by: Zach Harel <zach@zharel.me>
…r accelerations) and make both classes interpolatable

Signed-off-by: Zach Harel <zach@zharel.me>
Signed-off-by: Zach Harel <zach@zharel.me>
Signed-off-by: Zach Harel <zach@zharel.me>
new ChassisSpeeds(
vel.vx + accel.ax * dts, vel.vy + accel.ay * dts, vel.omega + accel.alpha * dts);

var newPose = pose.exp(new Twist2d(newVel.vx * dts, newVel.vy * dts, newVel.omega * dts));
Copy link
Member

Choose a reason for hiding this comment

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

The x and y velocities are typically in the global frame, so the pose exponential won't give correct results.

Copy link
Author

Choose a reason for hiding this comment

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

The vel and accel properties are (supposed to) be in the robot frame.

public final Pose2d pose;

/** The robot velocity at this sample (in the robot's reference frame). */
public final ChassisSpeeds vel;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public final ChassisSpeeds vel;
public final ChassisSpeeds velocity;

Copy link
Author

Choose a reason for hiding this comment

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

I think the abbreviations here are fine

Copy link
Member

Choose a reason for hiding this comment

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

I disagree. We're not code golfing here. You can spare 5 more letters.

double rightAccel = input.get(1);

double vx = (vl + vr) / 2.0;
double trackwidth = (vr - vl) / omega;
Copy link
Member

Choose a reason for hiding this comment

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

This is numerically ill-conditioned (divide-by-zero) if the robot isn't rotating. In this expression, trackwidth is undefined if omega = 0. If you substitute in trackwidth and simplify, alpha is undefined if vr - vl = 0.

The ill-conditioning can be avoided by getting the trackwidth directly from the trajectory generator, but I don't see a clean way to do that off the top of my head.

Copy link
Author

Choose a reason for hiding this comment

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

I can set alpha to 0 if either (vr-vl) or omega = 0 but other than that I don't know how else to resolve this

Copy link
Member

Choose a reason for hiding this comment

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

If vr - vl = 0, ω = 0, and ar - al ≠ 0, then the trackwidth will be NaN (0/0) while α should be nonzero. The indeterminate value for trackwidth can be resolved with L'Hôpital's rule, but that gives (ar - al)/α, which contains the value we're trying to compute.

Copy link
Member

Choose a reason for hiding this comment

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

Seems like the best fix is to treat alpha as a model input, then have the trajectory generator provide that in the sample since the trajectory generator knows the trackwidth. Including the angular acceleration along with the wheel linear accelerations is analogous to including the angular velocity along with the wheel linear velocities.

I just fixed this in Choreo: SleipnirGroup/Choreo#1301.

Signed-off-by: Zach Harel <zach@zharel.me>
…for clarity

Signed-off-by: Zach Harel <zach@zharel.me>
…djust dynamics methods

Signed-off-by: Zach Harel <zach@zharel.me>
…use it already had the protobuf) and fix toString format

Signed-off-by: Zach Harel <zach@zharel.me>
Signed-off-by: Zach Harel <zach@zharel.me>
Signed-off-by: Zach Harel <zach@zharel.me>
…celerations, and SwerveModuleAccelerations with relevant protobufs/structs

Signed-off-by: Zach Harel <zach@zharel.me>
…anumDriveWheelAccelerations, and SwerveModuleAccelerations with relevant protobufs/structs

Signed-off-by: Zach Harel <zach@zharel.me>
# Conflicts:
#	wpimath/robotpy_pybind_build_info.bzl
#	wpimath/src/main/python/semiwrap/DifferentialSample.yml
#	wpimath/src/main/python/semiwrap/DifferentialTrajectory.yml
#	wpimath/src/main/python/semiwrap/HolonomicTrajectory.yml
#	wpimath/src/main/python/semiwrap/SplineSample.yml
#	wpimath/src/main/python/semiwrap/SplineTrajectory.yml
#	wpimath/src/main/python/semiwrap/TrajectorySample.yml
# Conflicts:
#	wpimath/src/generated/main/java/org/wpilib/math/proto/Trajectory.java
#	wpimath/src/generated/main/native/cpp/wpimath/protobuf/trajectory.npb.cpp
#	wpimath/src/main/java/org/wpilib/math/trajectory/proto/TrajectoryProto.java
#	wpimath/src/main/java/org/wpilib/math/trajectory/proto/TrajectoryStateProto.java
#	wpimath/src/main/proto/trajectory.proto
#	wpimath/src/test/java/org/wpilib/math/controller/LTVDifferentialDriveControllerTest.java
#	wpimath/src/test/java/org/wpilib/math/trajectory/proto/TrajectoryProtoTest.java
#	wpimath/src/test/java/org/wpilib/math/trajectory/proto/TrajectoryStateProtoTest.java
Comment on lines +4 to +6
// Copyright (c) FIRST and other WPILib contributors.
// Open Source Software; you can modify and/or share it under the terms of
// the WPILib BSD license file in the root directory of this project.
Copy link
Member

Choose a reason for hiding this comment

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

There's a lot of duplicate copyright headers.

Copy link
Author

Choose a reason for hiding this comment

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

yeah i think i broke something with the generated quickbuf files

}

void FieldObject2d::SetTrajectory(const wpi::math::Trajectory& trajectory) {
template <typename SampleType>
Copy link
Member

Choose a reason for hiding this comment

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

Template function definitions need to be in the header instead.

Copy link
Author

@zachwaffle4 zachwaffle4 Feb 18, 2026

Choose a reason for hiding this comment

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

oops (for context this was originally not a template function and so i just. updated the function definition in here)

Comment on lines 230 to 240
// v = (v_r + v_l) / 2 (1)
// w = (v_r - v_l) / (2r) (2)
// k = w / v (3)
//
// v_l = v - wr
// v_l = v - (vk)r
// v_l = v(1 - kr)
//
// v_r = v + wr
// v_r = v + (vk)r
// v_r = v(1 + kr)
Copy link
Member

Choose a reason for hiding this comment

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

This derivation is no longer used and can be removed.

public boolean equals(Object o) {
return o == this
|| o instanceof ChassisAccelerations c && ax == c.ax && ay == c.ay && alpha == c.alpha;
|| o instanceof ChassisAccelerations c
Copy link
Member

Choose a reason for hiding this comment

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

This seems unrelated to the PR and should be separate.

Copy link
Author

Choose a reason for hiding this comment

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

This is required such that NaN accelerations compare equal so that the tests pass with the spline trajectory generators.

Comment on lines +61 to +62
* @param leftSpeed The left-wheel speed at this sample.
* @param rightSpeed The right-wheel speed at this sample.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* @param leftSpeed The left-wheel speed at this sample.
* @param rightSpeed The right-wheel speed at this sample.
* @param leftSpeed The left wheel speed at this sample.
* @param rightSpeed The right wheel speed at this sample.

* @param pose The robot pose at this sample (in the field reference frame).
* @param linearVelocity The linear velocity in m/s.
* @param linearAcceleration The linear acceleration in m/s².
* @param curvature The curvature of the path at this sample, in 1/m.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* @param curvature The curvature of the path at this sample, in 1/m.
* @param curvature The curvature of the path at this sample, in rad/m.

WPILIB_DLLEXPORT
void to_json(wpi::util::json& json, const Trajectory::State& state);
protected:
// Helper: make all samples relative to a Pose2d
Copy link
Member

Choose a reason for hiding this comment

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

Need proper docs here. Helper: is redundant.

return out;
}

// Helper: concatenate with another list of samples, offsetting their
Copy link
Member

Choose a reason for hiding this comment

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

Need proper docs here. Helper: is redundant.

wpi::util::Lerp(start.acceleration.ay, end.acceleration.ay, t),
wpi::util::Lerp(start.acceleration.alpha, end.acceleration.alpha, t)};

// v_{k+1} = v_k + a_k * dt
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// v_{k+1} = v_k + a_k * dt
// vₖ₊₁ = vₖ + aₖΔt

start.velocity.vy + start.acceleration.ay * interpT,
start.velocity.omega + start.acceleration.alpha * interpT};

// x_{k+1} = x_k + v_k * dt + 0.5 a (dt)^2
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// x_{k+1} = x_k + v_k * dt + 0.5 a (dt)^2
// xₖ₊₁ = xₖ + vₖΔt + ½aₖ(Δt)²

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants