Improved robot localization and trajectory following#3716
Conversation
| @@ -0,0 +1,149 @@ | |||
| #include "imu.h" | |||
There was a problem hiding this comment.
Separate into IMU PR per #3703 for thorough implementation
| bool PrimitiveExecutor::isLinearTrajectoryNew( | ||
| const std::optional<TrajectoryPath>& new_trajectory) const | ||
| { | ||
| if (new_trajectory.has_value() != trajectory_path_.has_value()) | ||
| { | ||
| // Either we don't have a trajectory right now, and this one does, or we have one | ||
| // and this trajectory doesn't. | ||
| return true; | ||
| } | ||
|
|
||
| if (!new_trajectory.has_value()) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| return distance(new_trajectory->getDestination(), | ||
| trajectory_path_->getDestination()) > | ||
| LINEAR_DESTINATION_THRESHOLD_METERS; | ||
| } | ||
|
|
||
| bool PrimitiveExecutor::isAngularTrajectoryNew( | ||
| const BangBangTrajectory1DAngular& new_trajectory) const | ||
| { | ||
| if (!angular_trajectory_.has_value()) | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| return new_trajectory.getDestination() | ||
| .minDiff(angular_trajectory_->getDestination()) | ||
| .toDegrees() > ANGULAR_DESTINATION_THRESHOLD_DEGREES; | ||
| } |
There was a problem hiding this comment.
Create a TODO PR to extract this into a delegate class. SRP code smell for prim executor
| /** | ||
| * Check if the given trajectory is "new". That is, if its destination differs | ||
| * meaningfully from our current trajectory. | ||
| * | ||
| * @param new_trajectory The new trajectory requested by the AI. | ||
| * @return True if the new trajectory requested is meaningfully different from the | ||
| * current trajectory. That is, if the destinations are new. | ||
| */ | ||
| bool isLinearTrajectoryNew(const std::optional<TrajectoryPath>& new_trajectory) const; | ||
|
|
||
| /** | ||
| * Check if the given trajectory is "new". That is, if its destination differs | ||
| * meaningfully from our current trajectory. | ||
| * | ||
| * @param new_trajectory The new trajectory requested by the AI. | ||
| * @return True if the new trajectory requested is meaningfully different from the | ||
| * current trajectory. That is, if the destinations are new. | ||
| */ | ||
| bool isAngularTrajectoryNew(const BangBangTrajectory1DAngular& new_trajectory) const; |
There was a problem hiding this comment.
TODO: Move to trajectory planner as static methods?
| static constexpr double ANGULAR_STALL_ERROR_MAX_DEGREES = 13; | ||
|
|
||
| static constexpr double LINEAR_PURE_PID_THRESHOLD_METERS = 0.5; | ||
| static constexpr double ANGULAR_PURE_PID_THRESHOLD_DEGREES = 25; |
There was a problem hiding this comment.
TODO: Keep Primitive Executor lightweight, move the tuning logic to lower level. Reference with interface e.g.
VelocityController
There was a problem hiding this comment.
Template type with linear or angular variants? VelocityController<BangBangTrajectory1DAngular>?
| /** | ||
| * Update the robot's position and orientation from data reported by vision. | ||
| * | ||
| * @param position Vision reading of the robot's position in world space | ||
| * @param orientation Vision reading of the robot's orientation in world space | ||
| * @param age_seconds Age in seconds of the vision snapshot (time since it was taken) | ||
| */ | ||
| void updateVision(const Point& position, const Angle& orientation, | ||
| double age_seconds); | ||
|
|
||
| /** | ||
| * Update the robot's velocity from data reported by motor sensors | ||
| * (i.e. encoders or Hall sensors). | ||
| * | ||
| * @param velocity Motor sensor reading of the robot's velocity in local space | ||
| * @param angular_velocity Motor sensor reading of the robot's angular velocity | ||
| */ | ||
| void updateMotorSensors(const Vector& velocity, | ||
| const AngularVelocity& angular_velocity); | ||
|
|
||
| /** | ||
| * Update the angular velocity from IMU. | ||
| * | ||
| * @param angular_velocity angular velocity of the robot | ||
| */ | ||
| void updateImu(const AngularVelocity& angular_velocity); |
There was a problem hiding this comment.
Code smell. Should be 1 unified update method. Consider using either method overloading or template
There was a problem hiding this comment.
I disagree here, we need different implementation for different sensors no? And sensors can provide the same types of measurements so method overloading does not work here
There was a problem hiding this comment.
To be clear, our kalman filter differentiates between measurements from different sensors
There was a problem hiding this comment.
I agree with William that the methods should stay separate. However, the implementations do look very similar, so there is probably an abstraction via a helper function that could be created
| std::unique_ptr<MotorService> motor_service_; | ||
| std::unique_ptr<NetworkService> network_service_; | ||
| std::unique_ptr<PowerService> power_service_; | ||
| std::unique_ptr<ImuService> imu_service_; |
| EuclideanToWheel::EuclideanToWheel(const robot_constants::RobotConstants& robot_constants) | ||
| : robot_constants_(robot_constants) | ||
| { | ||
| // Angles [rads] | ||
| auto p = robot_constants_.front_wheel_angle_deg * M_PI / 180.; | ||
| auto t = robot_constants_.back_wheel_angle_deg * M_PI / 180.; | ||
|
|
||
| auto cos_p = std::cos(p); | ||
| auto sin_p = std::sin(p); | ||
| auto cos_t = std::cos(t); | ||
| auto sin_t = std::sin(t); | ||
|
|
||
| // 1. Physical wheel coordinates in meters | ||
| // Mapped to the robot frame: +X = Right, +Y = Forward | ||
| double fr_x = 0.06632, fr_y = 0.03485; | ||
| double br_x = 0.05592, br_y = -0.04985; | ||
|
|
||
| // 2. Unit drive vectors (extracted directly from the matrix's linear columns) | ||
| double fr_dx = -sin_p, fr_dy = cos_p; | ||
| double br_dx = sin_t, br_dy = cos_t; | ||
|
|
||
| // 3. Dynamically calculate the rotational lever arms using the 2D cross product | ||
| // Lever Arm = | X * Dy - Y * Dx | | ||
| double rot_f = std::abs((fr_x * fr_dy) - (fr_y * fr_dx)); | ||
| double rot_b = std::abs((br_x * br_dy) - (br_y * br_dx)); | ||
|
|
||
| // clang-format off | ||
| euclidean_to_wheel_velocity_D_ << | ||
| -sin_p, cos_p, rot_f, | ||
| -sin_p, -cos_p, rot_f, | ||
| sin_t, -cos_t, rot_b, | ||
| sin_t, cos_t, rot_b; | ||
| // clang-format on | ||
|
|
||
| // Calculate Pseudo-inverse dynamically | ||
| wheel_to_euclidean_velocity_D_inverse_ = | ||
| (euclidean_to_wheel_velocity_D_.transpose() * euclidean_to_wheel_velocity_D_) | ||
| .inverse() * | ||
| euclidean_to_wheel_velocity_D_.transpose(); | ||
| } | ||
|
|
||
| #else |
| @@ -0,0 +1,151 @@ | |||
| #pragma once | |||
There was a problem hiding this comment.
TODO: Merge with localizer or IMU impl, or on its own
| std::map<RobotId, std::tuple<Point, Angle, Vector, AngularVelocity>> | ||
| ErForceSimulator::getRobotIdToStateMap( |
There was a problem hiding this comment.
Should create struct that contains the state instead of using a tuple. Could be reused in many places
There was a problem hiding this comment.
Or just use the RobotState class that already exists
|
Separated implementation of RobotLocalizer to other PR #3725 |
Description
Warning
This PR will be broken down into several smaller work items. Do not merge.
Testing Done
Resolved Issues
Foundations for #3703, #3704, #3705, #3706
Length Justification and Key Files to Review
Review Checklist
It is the reviewers responsibility to also make sure every item here has been covered
.hfile) should have a javadoc style comment at the start of them. For examples, see the functions defined inthunderbots/software/geom. Similarly, all classes should have an associated Javadoc comment explaining the purpose of the class.TODO(or similar) statements should either be completed or associated with a github issue