Skip to content

Robot Kalman Filters - #3906

Open
sunghyuneun wants to merge 64 commits into
UBC-Thunderbots:masterfrom
sunghyuneun:sunghyuneun/robot_filter
Open

sunghyuneun wants to merge 64 commits into
UBC-Thunderbots:masterfrom
sunghyuneun:sunghyuneun/robot_filter

Conversation

@sunghyuneun

@sunghyuneun sunghyuneun commented Aug 27, 2026 •

Copy link
Copy Markdown
Contributor

Description

note: i am lazy and i want auto-formatting fixes so i marked it as open but it's not in a final state but it's 95% there i think (excluding physical tests)

Makes changes to robot filter. Notable changes:

  • Rewrote robot filter to utilize linear Kalman filter to get more accurate measurements of each robot's orientation and position
  • As Kalman filter is able to predict the robot position, if the robot temporarily goes missing in the data, it predicts the robot position at that time
  • Removes robot if it is missing for 10 updates, fixing RobotFilter does not remove expired robots if there are no new detections #1717.

Notably a lot of the files changed are part of PR #3900, because I built upon a lot of the work there. Files that should actually be reviewed include:
robot_filter.cpp, robot_filter.h, robot_filter_test.cpp, robot_team_filter.cpp, robot_team_filter_test.cpp

Testing with real robots is incomplete, and some constants should be changed.

Testing Done

Unit tests modified

Resolved Issues

Resolves #3701, fixes #1717.

Length Justification and Key Files to Review

Ball Kalman Filter is +529 -503, therefore this PR is +413, -101.

Review Checklist

It is the reviewers responsibility to also make sure every item here has been covered

  • Function & Class comments: All function definitions (usually in the .h file) should have a javadoc style comment at the start of them. For examples, see the functions defined in thunderbots/software/geom. Similarly, all classes should have an associated Javadoc comment explaining the purpose of the class.
  • Remove all commented out code
  • Remove extra print statements: for example, those just used for testing
  • Resolve all TODO's: All TODO (or similar) statements should either be completed or associated with a github issue

@sunghyuneun sunghyuneun self-assigned this Sep 3, 2026
@sunghyuneun
sunghyuneun marked this pull request as ready for review September 3, 2026 18:12
# Robot always not excessively dribbling
always_validations = [
[BallAlwaysStaysInRegion([tbots_cpp.Circle(ball_location, 0.05)])]
[BallAlwaysStaysInRegion([tbots_cpp.Circle(ball_location, 0.5)])]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this is pretty significant relaxation, is it necessary?

// does not account for: deflections, uneven turf, and the tail of a kick. A kick
// itself is far larger than this, but it is also abrupt enough that the outlier gates
// catch it and reset the filter, so this does not need to cover one.
constexpr double ACCELERATION_NOISE_M_PER_S_SQUARED = 5.0;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

how were these values derived? Will they differ depending on camera? In that case, is anonymizing it here in this namespace the best location to put this?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could these values be adjusted frequently? ie surface, camera, SSL vision quality? In that case, should we consider a dynamically adjustable set of datavalues

* where it ended up. A ball moving at 5 m/s covers over 8 cm between frames at 60 Hz,
* so a test that only asked whether the ball was currently within its own radius of a
* surface would step straight over anything thin, and a goalpost is thin.
*

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

given this desc, maybe a more apt naming scheme is appropriate

Comment on lines +38 to +47
# Disable for kalman filter
#cc_test(
# name = "ball_filter_test",
# srcs = ["ball_filter_test.cpp"],
# deps = [
# ":ball_filter",
# "//shared/test_util:tbots_gtest_main",
# "//software/world:field",
# ],
#)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should this test be updated + enabled?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I started this PR by pulling the Ball Kalman Filter PR so there may be remnants remaining that I haven't removed yet.


void BallFilter::addNewDetectionsToBuffer(std::vector<BallDetection> new_ball_detections,
const Rectangle& filter_area)
std::optional<Ball> BallFilter::estimateBallState(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

how often is this called? is it a concern that we recalc a pseudo inverse on the fly every time?

@sunghyuneun sunghyuneun left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

andrew's comments moved to robot filter stuff where it's relevant.

// catch it and reset the filter, so this does not need to cover one.
// TODO: this does NOT apply to a robot but like whatever man lol
constexpr double ACCELERATION_NOISE_M_PER_S_SQUARED = 4.0;
constexpr double ANG_ACCELERATION_NOISE_RAD_PER_S_SQUARED = 1.0;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Andrewyx comment -

how were these values derived? Will they differ depending on camera? In that case, is anonymizing it here in this namespace the best location to put this?

Could these values be adjusted frequently? ie surface, camera, SSL vision quality? In that case, should we consider a dynamically adjustable set of datavalues


std::optional<Robot> RobotFilter::getFilteredData(
const std::vector<RobotDetection>& new_robot_data, const Timestamp& capture_timestamp,
std::optional<Robot> RobotFilter::estimateRobotState(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

how often is this called? is it a concern that we recalc a pseudo inverse on the fly every time?

@mkhlb
mkhlb self-requested a review October 7, 2026 17:08
@StarrryNight

Copy link
Copy Markdown
Contributor

if you have time can u merge master and resolve the conflicts

@Andrewyx Andrewyx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Some comments. Ran out of time, so handing off to @StarrryNight / @nycrat if possible?

Comment thread .gitignore
.vscode/
.vscode/.*
!.vscode/extensions.json
*.code-workspace

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

intentional?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes!

#include "software/networking/unix/threaded_proto_unix_sender.hpp"
#include "software/simulation/er_force_simulator.h"

// CSV file that the filtered robot state is logged to, alongside the ground truth

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: "Ground truth state" is not informative. Unnecessary jargon and verbosity from AI here I assume. Really confusing what this supposed to actually be, assuming filtered vs real values from sim?

Comment on lines +19 to +24
// Wraps an angle to [-pi, pi] so detected and actual orientations are directly
// comparable (e.g. -3.1 rad and 3.1 rad aren't reported as a ~6.2 rad error)
static double wrapAngle(double radians)
{
return std::atan2(std::sin(radians), std::cos(radians));
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is this not some standard utility we have elsewhere? Seems like a common use case, or why are we "wrapping" in the first place?

Timestamp default_timestamp;
};

// Robot expires when it hasn't received its own data for at least 200 milliseconds.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why 200 ms?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

arbitrary. we should change depending on real robot data.

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.

Investigate Robot Filters RobotFilter does not remove expired robots if there are no new detections

4 participants