Robot Localizer#3754
Conversation
…re into Andrewyx/robot_localizer
There was a problem hiding this comment.
@Andrewyx where were these changes from? It seems to completely break the simulated test. When I go watch the replay of the logs there is just nothing
There was a problem hiding this comment.
I think these were hotfixes to try and add ci mode and fix sim tests for merged branch. I haven't verified if they are correct/are working. Feel free to audit and fix them to pass the tests.
StarrryNight
left a comment
There was a problem hiding this comment.
Great work! Added some comments.
| * different | ||
| * @return True if the trajectories are different, false otherwise | ||
| */ | ||
| bool isNew(const Trajectory<double, double, double>& other, |
There was a problem hiding this comment.
isDifferent could be a better representation of what the function does
| * different in degrees. | ||
| * @return True if the trajectories are different, false otherwise | ||
| */ | ||
| bool isNew(const Trajectory<Angle, AngularVelocity, AngularAcceleration>& other, |
| * different | ||
| * @return True if the trajectories are different, false otherwise | ||
| */ | ||
| virtual bool isNew(const Trajectory<P, V, A>& other, double threshold) const = 0; |
| * different | ||
| * @return True if the trajectories are different, false otherwise | ||
| */ | ||
| bool isNew(const Trajectory<Point, Vector, Vector>& other, |
| clock_gettime(CLOCK_MONOTONIC, &last_kicker_fired); | ||
| } | ||
| else if (direct_control_.power_control().chicker().has_chip_distance_meters() || | ||
| direct_control_.power_control() |
There was a problem hiding this comment.
Might want to implement a chicker_status in power_control
| // Robot 1 | ||
| Point(world_ptr->field().centerPoint() + | ||
| Vector(-world_ptr->field().centerCircleRadius(), 0)), | ||
| Vector(-0.95 * world_ptr->field().centerCircleRadius(), 0)), |
There was a problem hiding this comment.
why are we doing this?
| void StSpinMotorController::updateEuclideanVelocity( | ||
| EuclideanSpace_t target_euclidean_velocity) | ||
| { | ||
| const Vector local_velocity(target_euclidean_velocity[1], |
There was a problem hiding this comment.
Why can we just swap these indices? Same for other ones.
|
|
||
| // If we are lagging behind trajectory too much, we have stalled! We need to | ||
| // regenerate trajectory. | ||
| if (trajectory_path_.has_value()) |
There was a problem hiding this comment.
add guard current_primitive_.has_move()
| Angle orientation_to_destination = | ||
| orientation_.minDiff(angular_trajectory_->getDestination()); | ||
| if (orientation_to_destination.toDegrees() < 5) | ||
| if (angular_trajectory_.has_value()) |
Description
Completes the ported changes from #3716, and implements the robot localizer, its various interfaces, and completes refactoring on much of the thunderloop pipeline.
Testing Done
Resolved Issues
Resolves #3725
Length Justification and Key Files to Review
thunderloop.cppprimitive_executor.cpprobot_localizer.cppReview 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