wpilibsuite / allwpilib

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

Sysid crashes when all data is filtered out during analysis #6438

Closed willtoth closed 2 weeks ago

willtoth commented 7 months ago

When trying to characterize a flywheel, opening the exported WPILog from AdvantageScope and selecting the correct fields, once the 'Load' button is pressed, the application crashes.

After exporting only the single Spark Flex that and the test state, the application no longer crashes, and is able to run correctly.

To Reproduce Steps to reproduce the behavior:

  1. Run sysid routine on robot
  2. Gather wpilog from robot, import to AdvantageScope, export with all fields to generate URCL data
  3. Load into sysid, using the fields (any of SparkFlex-14,15,16,17):
    • sysid-test-state-Launcher
    • NT:URCL/SparkFlex-17/AppliedOutputVoltage
    • NT:URCL/SparkFlex-17/MotorVelocityRPM
    • NT:URCL/SparkFlex-17/MotorPositionRotations
  4. Press 'Load'

To correct it

  1. Load the same WPILog into AdvantageScope
  2. Export, but set the prefix output to sysid-test-state-Launcher,NT:URCL/SparkFlex-17
  3. Run with the same settings as before
  4. Success

Desktop (please complete the following information):

Additional context Three logs provided here

calcmogul commented 7 months ago

I can't reproduce the crash on either 2024.3.1 or main (host is Linux x86_64). Screenshot_20240313_113228

willtoth commented 7 months ago

Okay I'm seeing that now too. Not sure what changed, as I definitely tried all 4 of the referenced SparkFlex's.

Try again with SparkFlex-15 from the same log file. That just crashed for me when loading.

calcmogul commented 7 months ago

I reproduced the crash with SparkFlex-15.

Thread 1 "sysid" received signal SIGSEGV, Segmentation fault.
0x0000555555aef770 in units::linear_scale<double>::operator() (this=0xffffffffffffffc0)
    at /home/tav/frc/wpilib/allwpilib/wpimath/src/main/native/include/units/base.h:2498
2498                    inline constexpr T operator()() const noexcept { return m_value; }                  ///< returns value.
(gdb) bt
#0  0x0000555555aef770 in units::linear_scale<double>::operator() (this=0xffffffffffffffc0)
    at /home/tav/frc/wpilib/allwpilib/wpimath/src/main/native/include/units/base.h:2498
#1  0x0000555555aefd97 in units::operator-<units::unit_t<units::unit<std::ratio<1l, 1l>, units::base_unit<std::ratio<0l, 1l>, std::ratio<0l, 1l>, std::ratio<1l, 1l>, std::ratio<0l, 1l>, std::ratio<0l, 1l>, std::ratio<0l, 1l>, std::ratio<0l, 1l>, std::ratio<0l, 1l>, std::ratio<0l, 1l> >, std::ratio<0l, 1l>, std::ratio<0l, 1l> >, double, units::linear_scale>, units::unit_t<units::unit<std::ratio<1l, 1l>, units::base_unit<std::ratio<0l, 1l>, std::ratio<0l, 1l>, std::ratio<1l, 1l>, std::ratio<0l, 1l>, std::ratio<0l, 1l>, std::ratio<0l, 1l>, std::ratio<0l, 1l>, std::ratio<0l, 1l>, std::ratio<0l, 1l> >, std::ratio<0l, 1l>, std::ratio<0l, 1l> >, double, units::linear_scale>, 0> (lhs=..., rhs=...)
    at /home/tav/frc/wpilib/allwpilib/wpimath/src/main/native/include/units/base.h:2568
#2  0x0000555555cb6776 in GetMaxStepTime (data=...)
    at /home/tav/frc/wpilib/allwpilib/sysid/src/main/native/cpp/analysis/FilteringUtils.cpp:326
#3  0x0000555555cb687f in sysid::InitialTrimAndFilter (data=0x7fffffffd410, settings=0x555556019178, 
    positionDelays=std::vector of length 0, capacity 0, velocityDelays=std::vector of length 0, capacity 0, 
    minStepTime=..., maxStepTime=..., unit="Meters")
    at /home/tav/frc/wpilib/allwpilib/sysid/src/main/native/cpp/analysis/FilteringUtils.cpp:344
#4  0x0000555555ae97d6 in sysid::AnalysisManager::PrepareGeneralData (this=0x555557128e60)
    at /home/tav/frc/wpilib/allwpilib/sysid/src/main/native/cpp/analysis/AnalysisManager.cpp:140
#5  0x0000555555aea15e in sysid::AnalysisManager::PrepareData (this=0x555557128e60)
    at /home/tav/frc/wpilib/allwpilib/sysid/src/main/native/cpp/analysis/AnalysisManager.cpp:182
#6  0x0000555555d2239a in sysid::Analyzer::PrepareData (this=0x5555560190f0)
    at /home/tav/frc/wpilib/allwpilib/sysid/src/main/native/cpp/view/Analyzer.cpp:272
#7  0x0000555555d22f79 in sysid::Analyzer::AnalyzeData (this=0x5555560190f0)
    at /home/tav/frc/wpilib/allwpilib/sysid/src/main/native/cpp/view/Analyzer.cpp:402
#8  0x0000555555ada417 in operator()<sysid::TestData> (__closure=0x5555560149f8, data=...)
    at /home/tav/frc/wpilib/allwpilib/sysid/src/main/native/cpp/App.cpp:112

The data filtering doesn't properly handle when the sample vector is empty.

willtoth commented 7 months ago

Huh, this is a really good point, I did not realize only one of the four motors was spinning at the time. Sorry about that, but hopefully results in a better warning for the next team to do that :)