[cmd] Decrease default step voltage in SysIdRoutine command

The old default was selected back when DC brushed motors were the norm. Motors are powerful enough now that it can cause wheel slip, which invalidates the test data. Fixes wpilibsuite#7794.
calcmogul · Feb 19, 2025 · 912cdf5 · 912cdf5
1 parent 1362606
commit 912cdf5
Show file tree

Hide file tree

Showing 4 changed files with 7 additions and 7 deletions.
diff --git a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/sysid/SysIdRoutine.java b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/sysid/SysIdRoutine.java
@@ -91,7 +91,7 @@ public Config(
         Time timeout,
         Consumer<State> recordState) {
       m_rampRate = rampRate != null ? rampRate : Volts.of(1).per(Second);
-      m_stepVoltage = stepVoltage != null ? stepVoltage : Volts.of(7);
+      m_stepVoltage = stepVoltage != null ? stepVoltage : Volts.of(4);
       m_timeout = timeout != null ? timeout : Seconds.of(10);
       m_recordState = recordState;
     }

diff --git a/wpilibNewCommands/src/main/native/include/frc2/command/sysid/SysIdRoutine.h b/wpilibNewCommands/src/main/native/include/frc2/command/sysid/SysIdRoutine.h
@@ -27,7 +27,7 @@ class Config {
   ramp_rate_t m_rampRate{1_V / 1_s};
 
   /// The step voltage output used for dynamic test routines.
-  units::volt_t m_stepVoltage{7_V};
+  units::volt_t m_stepVoltage{4_V};
 
   /// Safety timeout for the test routine commands.
   units::second_t m_timeout{10_s};
@@ -53,7 +53,7 @@ class Config {
          std::optional<units::volt_t> stepVoltage,
          std::optional<units::second_t> timeout,
          std::function<void(frc::sysid::State)> recordState)
-      : m_recordState{recordState} {
+      : m_recordState{std::move(recordState)} {
     if (rampRate) {
       m_rampRate = rampRate.value();
     }

diff --git a/wpilibNewCommands/src/test/java/edu/wpi/first/wpilibj2/command/sysid/SysIdRoutineTest.java b/wpilibNewCommands/src/test/java/edu/wpi/first/wpilibj2/command/sysid/SysIdRoutineTest.java
@@ -127,7 +127,7 @@ void testsOutputCorrectVoltage() {
     runCommand(m_dynamicForward);
     orderCheck = inOrder(m_mechanism);
 
-    orderCheck.verify(m_mechanism, atLeastOnce()).drive(Volts.of(7));
+    orderCheck.verify(m_mechanism, atLeastOnce()).drive(Volts.of(4));
     orderCheck.verify(m_mechanism).drive(Volts.of(0));
     orderCheck.verify(m_mechanism, never()).drive(any());
 
@@ -136,7 +136,7 @@ void testsOutputCorrectVoltage() {
     orderCheck = inOrder(m_mechanism);
 
     runCommand(m_dynamicReverse);
-    orderCheck.verify(m_mechanism, atLeastOnce()).drive(Volts.of(-7));
+    orderCheck.verify(m_mechanism, atLeastOnce()).drive(Volts.of(-4));
     orderCheck.verify(m_mechanism).drive(Volts.of(0));
     orderCheck.verify(m_mechanism, never()).drive(any());
   }

diff --git a/wpilibNewCommands/src/test/native/cpp/frc2/command/sysid/SysIdRoutineTest.cpp b/wpilibNewCommands/src/test/native/cpp/frc2/command/sysid/SysIdRoutineTest.cpp
@@ -165,14 +165,14 @@ TEST_F(SysIdRoutineTest, OutputCorrectVoltage) {
   sentVoltages.clear();
 
   RunCommand(std::move(m_dynamicForward));
-  expectedVoltages = std::vector<units::volt_t>{7_V, 0_V};
+  expectedVoltages = std::vector<units::volt_t>{4_V, 0_V};
   EXPECT_NEAR_UNITS(expectedVoltages[0], sentVoltages[0], 1e-6_V);
   EXPECT_NEAR_UNITS(expectedVoltages[1], sentVoltages[1], 1e-6_V);
   currentStateList.clear();
   sentVoltages.clear();
 
   RunCommand(std::move(m_dynamicReverse));
-  expectedVoltages = std::vector<units::volt_t>{-7_V, 0_V};
+  expectedVoltages = std::vector<units::volt_t>{-4_V, 0_V};
   EXPECT_NEAR_UNITS(expectedVoltages[0], sentVoltages[0], 1e-6_V);
   EXPECT_NEAR_UNITS(expectedVoltages[1], sentVoltages[1], 1e-6_V);
   currentStateList.clear();