Skip to content

[wpilib, commands] Add warnWhenDisconnected() to HIDs #7877

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

KangarooKoala
Copy link
Contributor

@KangarooKoala KangarooKoala commented Mar 20, 2025

Not sure if we'd want this to be a decorator method instead (which modifies the instance and returns the instance), but having a setter was really simple and should work for every reasonable use case. A decorator would also be hard to name- Following the convention of Command.ignoringDisable(), this would be GenericHID.warningWhenDisconnected(), which doesn't work well because "warning" is mostly used as a noun

I don't think we have unit tests for generic HID, but I tested locally with all 9 permutations of (no call to warnWhenDisconnected() | warnWhenDisconnected(true) | warnWhenDisconnected(false)) x (no controller | xbox controller | controller without enough buttons).
Here's the diff I tested with:

diff --git a/developerRobot/src/main/java/frc/robot/Robot.java b/developerRobot/src/main/java/frc/robot/Robot.java
index 6cf3f4738..abcf007b2 100644
--- a/developerRobot/src/main/java/frc/robot/Robot.java
+++ b/developerRobot/src/main/java/frc/robot/Robot.java
@@ -5,13 +5,21 @@
 package frc.robot;
 
 import edu.wpi.first.wpilibj.TimedRobot;
+import edu.wpi.first.wpilibj2.command.Commands;
+import edu.wpi.first.wpilibj2.command.CommandScheduler;
+import edu.wpi.first.wpilibj2.command.button.CommandXboxController;
 
 public class Robot extends TimedRobot {
   /**
    * This function is run when the robot is first started up and should be used for any
    * initialization code.
    */
-  public Robot() {}
+  public Robot() {
+    var controller = new CommandXboxController(0);
+    controller.warnWhenDisconnected(false);
+    controller.leftStick().whileTrue(Commands.none());
+    Commands.print("Beep").andThen(Commands.waitSeconds(1)).repeatedly().schedule();
+  }
 
   /** This function is run once each time the robot enters autonomous mode. */
   @Override
@@ -35,5 +43,7 @@ public class Robot extends TimedRobot {
 
   /** This function is called periodically during all modes. */
   @Override
-  public void robotPeriodic() {}
+  public void robotPeriodic() {
+    CommandScheduler.getInstance().run();
+  }
 }
diff --git a/developerRobot/src/main/native/cpp/Robot.cpp b/developerRobot/src/main/native/cpp/Robot.cpp
index 6e013599e..1ed15c3c4 100644
--- a/developerRobot/src/main/native/cpp/Robot.cpp
+++ b/developerRobot/src/main/native/cpp/Robot.cpp
@@ -2,15 +2,26 @@
 // Open Source Software; you can modify and/or share it under the terms of
 // the WPILib BSD license file in the root directory of this project.
 
+#include <frc/DriverStation.h>
 #include <frc/TimedRobot.h>
+#include <frc2/command/Commands.h>
+#include <frc2/command/CommandScheduler.h>
+#include <frc2/command/button/CommandXboxController.h>
 
 class Robot : public frc::TimedRobot {
  public:
+  frc2::CommandXboxController controller{0};
+  frc2::CommandPtr beeper = frc2::cmd::Print("Beep").AndThen(frc2::cmd::Wait(1_s)).Repeatedly();
+
   /**
    * This function is run when the robot is first started up and should be
    * used for any initialization code.
    */
-  Robot() {}
+  Robot() {
+    controller.WarnWhenDisconnected(false);
+    controller.LeftStick().WhileTrue(frc2::cmd::None());
+    beeper.Schedule();
+  }
 
   /**
    * This function is run once each time the robot enters autonomous mode
@@ -40,7 +51,9 @@ class Robot : public frc::TimedRobot {
   /**
    * This function is called periodically during all modes
    */
-  void RobotPeriodic() override {}
+  void RobotPeriodic() override {
+    frc2::CommandScheduler::GetInstance().Run();
+  }
 };
 
 int main() {

The beep command makes it easier to tell if warnings are still being emitted.

@KangarooKoala KangarooKoala requested review from a team as code owners March 20, 2025 23:52
Copy link
Contributor

This PR modifies commands. Please open a corresponding PR in Python Commands and include a link to this PR.

@github-actions github-actions bot added component: wpilibj WPILib Java component: wpilibc WPILib C++ component: command-based WPILib Command Based Library labels Mar 20, 2025
@KangarooKoala KangarooKoala changed the title Add warnWhenDisconnected() to HIDs [wpilib, commands] Add warnWhenDisconnected() to HIDs Mar 20, 2025
@KangarooKoala
Copy link
Contributor Author

This PR modifies commands. Please open a corresponding PR in Python Commands and include a link to this PR.

I think that'll need to be started after the WPILib PR is merged, since the changes to CommandGenericHID depend on the changes to GenericHID.

@Starlight220
Copy link
Member

This should follow the verbiage used in the other warning suppression methods

@KangarooKoala
Copy link
Contributor Author

This should follow the verbiage used in the other warning suppression methods

Which other warning suppression methods? The doc comment explicitly calls out that this is NOT the same as DriverStation.silenceJoystickConnectionWarning(), and I didn't notice any other warning suppression methods to model this after.

@Starlight220
Copy link
Member

Even if it is not exactly the same, the phrasing of the method name etc should be similar.

That said, the distinctions should be documented (as they are atm)

@PeterJohnson
Copy link
Member

How does this interact with #7840 (using alerts for joystick warnings)? Is this just a temporary fix until that is implemented?

@KangarooKoala
Copy link
Contributor Author

KangarooKoala commented Mar 21, 2025

Even if it is not exactly the same, the phrasing of the method name etc should be similar.

So the name would be silenceConnectionWarning() with an opening line "Allows the user to specify whether they want connection warnings to be printed to the console when this joystick is disconnected"?

How does this interact with #7840 (using alerts for joystick warnings)? Is this just a temporary fix until that is implemented?

The implementation would be different, but I'm guessing we'd want something with similar semantics, since it'll still be good to indicate that there should not be warnings when particular controllers are disconnected. Alerts are way nicer than DriverStation warnings for not obscuring other stuff happening, but it would still be good to have a clean status without those controllers so people don't get used to ignoring alerts.

Thinking more about 2027, maybe this would become setDisconnectionAlertType(AlertType)? (This would potentially support null or an empty optional as a way to indicate no alert, or we could decide that kInfo is sufficiently low priority for everyone's use cases)

@PeterJohnson
Copy link
Member

PeterJohnson commented Mar 21, 2025

What's the reason for not warning/alerting when certain controllers are disconnected? I would think that would always be an undesirable condition if you're using it in the code. Note for 2027, we will have an easy way to set up multiple teleop/test modes, so things like "only using a joystick for testing" should be handled by that feature?

@KangarooKoala
Copy link
Contributor Author

What's the reason for not warning/alerting when certain controllers are disconnected? I would think that would always be an undesirable condition if you're using it in the code. Note for 2027, we will have an easy way to set up multiple teleop/test modes, so things like "only using a joystick for testing" should be handled by that feature?

It probably would be enough to have users only bind the testing controllers in a particular mode (as long as we make sure to use mode.running.and(trigger) instead of trigger.and(mode.running) so short-circuiting prevents the method poll- which is what the plan already is).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: command-based WPILib Command Based Library component: wpilibc WPILib C++ component: wpilibj WPILib Java
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants