[commands] Subsystem: Add default command removal method (#5064)

HAL_GetRuntimeType used to be a free call before the roboRIO2 was added. However, nLoadOut::getTargetClass() is not a free call, and it may hit the IPC layer. Cache this value so it is not called every time.

.clang-format

@@ -156,7 +156,7 @@ SpacesInCStyleCastParentheses: false
 SpacesInParentheses: false
 SpacesInSquareBrackets: false
 SpaceBeforeSquareBrackets: false
-Standard:        c++17
+Standard:        c++20
 StatementMacros:
   - Q_UNUSED
   - QT_REQUIRE_VERSION

docs/build.gradle

@@ -64,6 +64,7 @@ doxygen {
             cppIncludeRoots.add(it.absolutePath)
         }
     }
+    cppIncludeRoots << '../ntcore/build/generated/main/native/include/'
 
     if (project.hasProperty('docWarningsAsErrors')) {
         // C++20 shims

hal/src/main/native/athena/HAL.cpp

@@ -253,12 +253,10 @@ const char* HAL_GetErrorMessage(int32_t code) {
   }
 }
 
+static HAL_RuntimeType runtimeType = HAL_Runtime_RoboRIO;
+
 HAL_RuntimeType HAL_GetRuntimeType(void) {
-  nLoadOut::tTargetClass targetClass = nLoadOut::getTargetClass();
-  if (targetClass == nLoadOut::kTargetClass_RoboRIO2) {
-    return HAL_Runtime_RoboRIO2;
-  }
-  return HAL_Runtime_RoboRIO;
+  return runtimeType;
 }
 
 int32_t HAL_GetFPGAVersion(int32_t* status) {
@@ -523,6 +521,13 @@ HAL_Bool HAL_Initialize(int32_t timeout, int32_t mode) {
     return false;
   }
 
+  nLoadOut::tTargetClass targetClass = nLoadOut::getTargetClass();
+  if (targetClass == nLoadOut::kTargetClass_RoboRIO2) {
+    runtimeType = HAL_Runtime_RoboRIO2;
+  } else {
+    runtimeType = HAL_Runtime_RoboRIO;
+  }
+
   InterruptManager::Initialize(global->getSystemInterface());
 
   hal::InitializeDriverStation();

hal/src/main/native/sim/REVPH.cpp

@@ -201,7 +201,7 @@ int32_t HAL_GetREVPHSolenoids(HAL_REVPHHandle handle, int32_t* status) {
 
   std::scoped_lock lock{pcm->lock};
   auto& data = SimREVPHData[pcm->module].solenoidOutput;
-  uint8_t ret = 0;
+  int32_t ret = 0;
   for (int i = 0; i < kNumREVPHChannels; i++) {
     ret |= (data[i] << i);
   }

wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/Subsystem.java

@@ -50,6 +50,14 @@ public interface Subsystem {
     CommandScheduler.getInstance().setDefaultCommand(this, defaultCommand);
   }
 
+  /**
+   * Removes the default command for the subsystem. This will not cancel the default command if it
+   * is currently running.
+   */
+  default void removeDefaultCommand() {
+    CommandScheduler.getInstance().removeDefaultCommand(this);
+  }
+
   /**
    * Gets the default command for this subsystem. Returns null if no default command is currently
    * associated with the subsystem.

wpilibNewCommands/src/main/native/cpp/frc2/command/Subsystem.cpp

@@ -21,6 +21,10 @@ void Subsystem::SetDefaultCommand(CommandPtr&& defaultCommand) {
                                                     std::move(defaultCommand));
 }
 
+void Subsystem::RemoveDefaultCommand() {
+  CommandScheduler::GetInstance().RemoveDefaultCommand(this);
+}
+
 Command* Subsystem::GetDefaultCommand() const {
   return CommandScheduler::GetInstance().GetDefaultCommand(this);
 }

wpilibNewCommands/src/main/native/include/frc2/command/Subsystem.h

@@ -83,6 +83,12 @@ class Subsystem {
    */
   void SetDefaultCommand(CommandPtr&& defaultCommand);
 
+  /**
+   * Removes the default command for the subsystem.  This will not cancel the
+   * default command if it is currently running.
+   */
+  void RemoveDefaultCommand();
+
   /**
    * Gets the default command for this subsystem.  Returns null if no default
    * command is currently associated with the subsystem.

wpilibc/src/main/native/cpp/DataLogManager.cpp

@@ -26,6 +26,7 @@ namespace {
 
 struct Thread final : public wpi::SafeThread {
   Thread(std::string_view dir, std::string_view filename, double period);
+  ~Thread() override;
 
   void Main() final;
 
@@ -94,6 +95,10 @@ Thread::Thread(std::string_view dir, std::string_view filename, double period)
   StartNTLog();
 }
 
+Thread::~Thread() {
+  StopNTLog();
+}
+
 void Thread::Main() {
   // based on free disk space, scan for "old" FRC_*.wpilog files and remove
   {

wpilibc/src/main/native/cpp/PneumaticHub.cpp

@@ -161,9 +161,14 @@ void PneumaticHub::EnableCompressorAnalog(
                         "maxPressure must be between 0 and 120 PSI, got {}",
                         maxPressure);
   }
-  int32_t status = 0;
+
+  // Send the voltage as it would be if the 5V rail was at exactly 5V.
+  // The firmware will compensate for the real 5V rail voltage, which
+  // can fluctuate somewhat over time.
   units::volt_t minAnalogVoltage = PSIToVolts(minPressure, 5_V);
   units::volt_t maxAnalogVoltage = PSIToVolts(maxPressure, 5_V);
+
+  int32_t status = 0;
   HAL_SetREVPHClosedLoopControlAnalog(m_handle, minAnalogVoltage.value(),
                                       maxAnalogVoltage.value(), &status);
   FRC_ReportError(status, "Module {}", m_module);
@@ -186,9 +191,14 @@ void PneumaticHub::EnableCompressorHybrid(
                         "maxPressure must be between 0 and 120 PSI, got {}",
                         maxPressure);
   }
-  int32_t status = 0;
+
+  // Send the voltage as it would be if the 5V rail was at exactly 5V.
+  // The firmware will compensate for the real 5V rail voltage, which
+  // can fluctuate somewhat over time.
   units::volt_t minAnalogVoltage = PSIToVolts(minPressure, 5_V);
   units::volt_t maxAnalogVoltage = PSIToVolts(maxPressure, 5_V);
+
+  int32_t status = 0;
   HAL_SetREVPHClosedLoopControlHybrid(m_handle, minAnalogVoltage.value(),
                                       maxAnalogVoltage.value(), &status);
   FRC_ReportError(status, "Module {}", m_module);

wpilibc/src/main/native/cpp/simulation/DutyCycleEncoderSim.cpp

@@ -9,9 +9,11 @@
 
 using namespace frc::sim;
 
-DutyCycleEncoderSim::DutyCycleEncoderSim(const frc::DutyCycleEncoder& encoder) {
-  frc::sim::SimDeviceSim deviceSim{"DutyCycle:DutyCycleEncoder",
-                                   encoder.GetSourceChannel()};
+DutyCycleEncoderSim::DutyCycleEncoderSim(const frc::DutyCycleEncoder& encoder)
+    : DutyCycleEncoderSim{encoder.GetSourceChannel()} {}
+
+DutyCycleEncoderSim::DutyCycleEncoderSim(int channel) {
+  frc::sim::SimDeviceSim deviceSim{"DutyCycle:DutyCycleEncoder", channel};
   m_simPosition = deviceSim.GetDouble("position");
   m_simDistancePerRotation = deviceSim.GetDouble("distance_per_rot");
 }

wpilibc/src/main/native/include/frc/XboxController.h

@@ -283,9 +283,9 @@ class XboxController : public GenericHID {
   BooleanEvent Y(EventLoop* loop) const;
 
   /**
-   * Whether the Y button was released since the last check.
+   * Read the value of the back button on the controller.
    *
-   * @return Whether the button was released since the last check.
+   * @return The state of the button.
    */
   bool GetBackButton() const;
 

wpilibc/src/main/native/include/frc/simulation/DutyCycleEncoderSim.h

@@ -26,7 +26,14 @@ class DutyCycleEncoderSim {
   explicit DutyCycleEncoderSim(const DutyCycleEncoder& encoder);
 
   /**
-   * Set the position tin turns.
+   * Constructs from a digital input channel.
+   *
+   * @param channel digital input channel
+   */
+  explicit DutyCycleEncoderSim(int channel);
+
+  /**
+   * Set the position in turns.
    *
    * @param turns The position.
    */

wpilibj/src/main/java/edu/wpi/first/wpilibj/PneumaticHub.java

@@ -294,6 +294,10 @@ public class PneumaticHub implements PneumaticsBase {
       throw new IllegalArgumentException(
           "maxPressure must be between 0 and 120 PSI, got " + maxPressure);
     }
+
+    // Send the voltage as it would be if the 5V rail was at exactly 5V.
+    // The firmware will compensate for the real 5V rail voltage, which
+    // can fluctuate somewhat over time.
     double minAnalogVoltage = psiToVolts(minPressure, 5);
     double maxAnalogVoltage = psiToVolts(maxPressure, 5);
     REVPHJNI.setClosedLoopControlAnalog(m_handle, minAnalogVoltage, maxAnalogVoltage);
@@ -339,6 +343,10 @@ public class PneumaticHub implements PneumaticsBase {
       throw new IllegalArgumentException(
           "maxPressure must be between 0 and 120 PSI, got " + maxPressure);
     }
+
+    // Send the voltage as it would be if the 5V rail was at exactly 5V.
+    // The firmware will compensate for the real 5V rail voltage, which
+    // can fluctuate somewhat over time.
     double minAnalogVoltage = psiToVolts(minPressure, 5);
     double maxAnalogVoltage = psiToVolts(maxPressure, 5);
     REVPHJNI.setClosedLoopControlHybrid(m_handle, minAnalogVoltage, maxAnalogVoltage);

wpilibj/src/main/java/edu/wpi/first/wpilibj/simulation/DutyCycleEncoderSim.java

@@ -18,8 +18,16 @@ public class DutyCycleEncoderSim {
    * @param encoder DutyCycleEncoder to simulate
    */
   public DutyCycleEncoderSim(DutyCycleEncoder encoder) {
-    SimDeviceSim wrappedSimDevice =
-        new SimDeviceSim("DutyCycle:DutyCycleEncoder" + "[" + encoder.getSourceChannel() + "]");
+    this(encoder.getSourceChannel());
+  }
+
+  /**
+   * Constructs from a digital input channel.
+   *
+   * @param channel digital input channel.
+   */
+  public DutyCycleEncoderSim(int channel) {
+    SimDeviceSim wrappedSimDevice = new SimDeviceSim("DutyCycle:DutyCycleEncoder", channel);
     m_simPosition = wrappedSimDevice.getDouble("position");
     m_simDistancePerRotation = wrappedSimDevice.getDouble("distance_per_rot");
   }