wpilibsuite / allwpilib

Official Repository of WPILibJ and WPILibC
https://wpilib.org/
Other
1.07k stars 614 forks source link

Some SysID unit tests have overly large tolerances and incorrect kPs #6434

Open brettle opened 7 months ago

brettle commented 7 months ago

The following 2 lines allow tests to pass with incorrect kPs:

https://github.com/wpilibsuite/allwpilib/blob/11c60df3e02062aa06b8d57711f1a3885229967f/sysid/src/test/native/cpp/analysis/FeedbackAnalysisTest.cpp#L88

https://github.com/wpilibsuite/allwpilib/blob/11c60df3e02062aa06b8d57711f1a3885229967f/sysid/src/test/native/cpp/analysis/FeedbackAnalysisTest.cpp#L103

Note that the tolerances are larger than the expected values, so even some negative kPs would be accepted.

The expected kPs themselves appear to actually be wrong but this went undetected due to the tolerances being too high.

calcmogul commented 7 months ago

We need some models with enough inertia that they have feedback gains above the noise floor.