[commands] Extract common trigger binding logic (#7550)

This makes the logic clearer in the actual binding methods and will hopefully make it less annoying to make changes such as allowing control over initial edges. Also changes Java to use previous and current to match C++.
2026-06-19 00:41:43 +00:00 · 2024-12-14 23:13:41 -08:00
parent 564c1f2de2
commit 70f36cce7e
3 changed files with 128 additions and 209 deletions
--- a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/button/Trigger.java
+++ b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/button/Trigger.java
@@ -24,6 +24,18 @@ import java.util.function.BooleanSupplier;
 * <p>This class is provided by the NewCommands VendorDep
 */
 public class Trigger implements BooleanSupplier {
+  /** Functional interface for the body of a trigger binding. */
+  @FunctionalInterface
+  private interface BindingBody {
+    /**
+     * Executes the body of the binding.
+     *
+     * @param previous The previous state of the condition.
+     * @param current The current state of the condition.
+     */
+    void run(boolean previous, boolean current);
+  }
+
  private final BooleanSupplier m_condition;
  private final EventLoop m_loop;

@@ -49,6 +61,27 @@ public class Trigger implements BooleanSupplier {
    this(CommandScheduler.getInstance().getDefaultButtonLoop(), condition);
  }

+  /**
+   * Adds a binding to the EventLoop.
+   *
+   * @param body The body of the binding to add.
+   */
+  private void addBinding(BindingBody body) {
+    m_loop.bind(
+        new Runnable() {
+          private boolean m_previous = m_condition.getAsBoolean();
+
+          @Override
+          public void run() {
+            boolean current = m_condition.getAsBoolean();
+
+            body.run(m_previous, current);
+
+            m_previous = current;
+          }
+        });
+  }
+
  /**
   * Starts the command when the condition changes.
   *
@@ -57,19 +90,10 @@ public class Trigger implements BooleanSupplier {
   */
  public Trigger onChange(Command command) {
    requireNonNullParam(command, "command", "onChange");
-    m_loop.bind(
-        new Runnable() {
-          private boolean m_pressedLast = m_condition.getAsBoolean();
-
-          @Override
-          public void run() {
-            boolean pressed = m_condition.getAsBoolean();
-
-            if (m_pressedLast != pressed) {
-              command.schedule();
-            }
-
-            m_pressedLast = pressed;
+    addBinding(
+        (previous, current) -> {
+          if (previous != current) {
+            command.schedule();
          }
        });
    return this;
@@ -83,19 +107,10 @@ public class Trigger implements BooleanSupplier {
   */
  public Trigger onTrue(Command command) {
    requireNonNullParam(command, "command", "onTrue");
-    m_loop.bind(
-        new Runnable() {
-          private boolean m_pressedLast = m_condition.getAsBoolean();
-
-          @Override
-          public void run() {
-            boolean pressed = m_condition.getAsBoolean();
-
-            if (!m_pressedLast && pressed) {
-              command.schedule();
-            }
-
-            m_pressedLast = pressed;
+    addBinding(
+        (previous, current) -> {
+          if (!previous && current) {
+            command.schedule();
          }
        });
    return this;
@@ -109,19 +124,10 @@ public class Trigger implements BooleanSupplier {
   */
  public Trigger onFalse(Command command) {
    requireNonNullParam(command, "command", "onFalse");
-    m_loop.bind(
-        new Runnable() {
-          private boolean m_pressedLast = m_condition.getAsBoolean();
-
-          @Override
-          public void run() {
-            boolean pressed = m_condition.getAsBoolean();
-
-            if (m_pressedLast && !pressed) {
-              command.schedule();
-            }
-
-            m_pressedLast = pressed;
+    addBinding(
+        (previous, current) -> {
+          if (previous && !current) {
+            command.schedule();
          }
        });
    return this;
@@ -139,21 +145,12 @@ public class Trigger implements BooleanSupplier {
   */
  public Trigger whileTrue(Command command) {
    requireNonNullParam(command, "command", "whileTrue");
-    m_loop.bind(
-        new Runnable() {
-          private boolean m_pressedLast = m_condition.getAsBoolean();
-
-          @Override
-          public void run() {
-            boolean pressed = m_condition.getAsBoolean();
-
-            if (!m_pressedLast && pressed) {
-              command.schedule();
-            } else if (m_pressedLast && !pressed) {
-              command.cancel();
-            }
-
-            m_pressedLast = pressed;
+    addBinding(
+        (previous, current) -> {
+          if (!previous && current) {
+            command.schedule();
+          } else if (previous && !current) {
+            command.cancel();
          }
        });
    return this;
@@ -171,21 +168,12 @@ public class Trigger implements BooleanSupplier {
   */
  public Trigger whileFalse(Command command) {
    requireNonNullParam(command, "command", "whileFalse");
-    m_loop.bind(
-        new Runnable() {
-          private boolean m_pressedLast = m_condition.getAsBoolean();
-
-          @Override
-          public void run() {
-            boolean pressed = m_condition.getAsBoolean();
-
-            if (m_pressedLast && !pressed) {
-              command.schedule();
-            } else if (!m_pressedLast && pressed) {
-              command.cancel();
-            }
-
-            m_pressedLast = pressed;
+    addBinding(
+        (previous, current) -> {
+          if (previous && !current) {
+            command.schedule();
+          } else if (!previous && current) {
+            command.cancel();
          }
        });
    return this;
@@ -199,23 +187,14 @@ public class Trigger implements BooleanSupplier {
   */
  public Trigger toggleOnTrue(Command command) {
    requireNonNullParam(command, "command", "toggleOnTrue");
-    m_loop.bind(
-        new Runnable() {
-          private boolean m_pressedLast = m_condition.getAsBoolean();
-
-          @Override
-          public void run() {
-            boolean pressed = m_condition.getAsBoolean();
-
-            if (!m_pressedLast && pressed) {
-              if (command.isScheduled()) {
-                command.cancel();
-              } else {
-                command.schedule();
-              }
+    addBinding(
+        (previous, current) -> {
+          if (!previous && current) {
+            if (command.isScheduled()) {
+              command.cancel();
+            } else {
+              command.schedule();
            }
-
-            m_pressedLast = pressed;
          }
        });
    return this;
@@ -229,23 +208,14 @@ public class Trigger implements BooleanSupplier {
   */
  public Trigger toggleOnFalse(Command command) {
    requireNonNullParam(command, "command", "toggleOnFalse");
-    m_loop.bind(
-        new Runnable() {
-          private boolean m_pressedLast = m_condition.getAsBoolean();
-
-          @Override
-          public void run() {
-            boolean pressed = m_condition.getAsBoolean();
-
-            if (m_pressedLast && !pressed) {
-              if (command.isScheduled()) {
-                command.cancel();
-              } else {
-                command.schedule();
-              }
+    addBinding(
+        (previous, current) -> {
+          if (previous && !current) {
+            if (command.isScheduled()) {
+              command.cancel();
+            } else {
+              command.schedule();
            }
-
-            m_pressedLast = pressed;
          }
        });
    return this;
--- a/wpilibNewCommands/src/main/native/cpp/frc2/command/button/Trigger.cpp
+++ b/wpilibNewCommands/src/main/native/cpp/frc2/command/button/Trigger.cpp
@@ -15,159 +15,117 @@ using namespace frc2;

 Trigger::Trigger(const Trigger& other) = default;

+void Trigger::AddBinding(wpi::unique_function<void(bool, bool)>&& body) {
+  m_loop->Bind([condition = m_condition, previous = m_condition(),
+                body = std::move(body)]() mutable {
+    bool current = condition();
+
+    body(previous, current);
+
+    previous = current;
+  });
+}
+
 Trigger Trigger::OnChange(Command* command) {
-  m_loop->Bind(
-      [condition = m_condition, previous = m_condition(), command]() mutable {
-        bool current = condition();
-
-        if (previous != current) {
-          command->Schedule();
-        }
-
-        previous = current;
-      });
+  AddBinding([command](bool previous, bool current) {
+    if (previous != current) {
+      command->Schedule();
+    }
+  });
  return *this;
 }

 Trigger Trigger::OnChange(CommandPtr&& command) {
-  m_loop->Bind([condition = m_condition, previous = m_condition(),
-                command = std::move(command)]() mutable {
-    bool current = condition();
-
+  AddBinding([command = std::move(command)](bool previous, bool current) {
    if (previous != current) {
      command.Schedule();
    }
-
-    previous = current;
  });
  return *this;
 }

 Trigger Trigger::OnTrue(Command* command) {
-  m_loop->Bind(
-      [condition = m_condition, previous = m_condition(), command]() mutable {
-        bool current = condition();
-
-        if (!previous && current) {
-          command->Schedule();
-        }
-
-        previous = current;
-      });
+  AddBinding([command](bool previous, bool current) {
+    if (!previous && current) {
+      command->Schedule();
+    }
+  });
  return *this;
 }

 Trigger Trigger::OnTrue(CommandPtr&& command) {
-  m_loop->Bind([condition = m_condition, previous = m_condition(),
-                command = std::move(command)]() mutable {
-    bool current = condition();
-
+  AddBinding([command = std::move(command)](bool previous, bool current) {
    if (!previous && current) {
      command.Schedule();
    }
-
-    previous = current;
  });
  return *this;
 }

 Trigger Trigger::OnFalse(Command* command) {
-  m_loop->Bind(
-      [condition = m_condition, previous = m_condition(), command]() mutable {
-        bool current = condition();
-
-        if (previous && !current) {
-          command->Schedule();
-        }
-
-        previous = current;
-      });
+  AddBinding([command](bool previous, bool current) {
+    if (previous && !current) {
+      command->Schedule();
+    }
+  });
  return *this;
 }

 Trigger Trigger::OnFalse(CommandPtr&& command) {
-  m_loop->Bind([condition = m_condition, previous = m_condition(),
-                command = std::move(command)]() mutable {
-    bool current = condition();
-
+  AddBinding([command = std::move(command)](bool previous, bool current) {
    if (previous && !current) {
      command.Schedule();
    }
-
-    previous = current;
  });
  return *this;
 }

 Trigger Trigger::WhileTrue(Command* command) {
-  m_loop->Bind(
-      [condition = m_condition, previous = m_condition(), command]() mutable {
-        bool current = condition();
-
-        if (!previous && current) {
-          command->Schedule();
-        } else if (previous && !current) {
-          command->Cancel();
-        }
-
-        previous = current;
-      });
+  AddBinding([command](bool previous, bool current) {
+    if (!previous && current) {
+      command->Schedule();
+    } else if (previous && !current) {
+      command->Cancel();
+    }
+  });
  return *this;
 }

 Trigger Trigger::WhileTrue(CommandPtr&& command) {
-  m_loop->Bind([condition = m_condition, previous = m_condition(),
-                command = std::move(command)]() mutable {
-    bool current = condition();
-
+  AddBinding([command = std::move(command)](bool previous, bool current) {
    if (!previous && current) {
      command.Schedule();
    } else if (previous && !current) {
      command.Cancel();
    }
-
-    previous = current;
  });
  return *this;
 }

 Trigger Trigger::WhileFalse(Command* command) {
-  m_loop->Bind(
-      [condition = m_condition, previous = m_condition(), command]() mutable {
-        bool current = condition();
-
-        if (previous && !current) {
-          command->Schedule();
-        } else if (!previous && current) {
-          command->Cancel();
-        }
-
-        previous = current;
-      });
+  AddBinding([command](bool previous, bool current) {
+    if (previous && !current) {
+      command->Schedule();
+    } else if (!previous && current) {
+      command->Cancel();
+    }
+  });
  return *this;
 }

 Trigger Trigger::WhileFalse(CommandPtr&& command) {
-  m_loop->Bind([condition = m_condition, previous = m_condition(),
-                command = std::move(command)]() mutable {
-    bool current = condition();
-
+  AddBinding([command = std::move(command)](bool previous, bool current) {
    if (!previous && current) {
      command.Schedule();
    } else if (previous && !current) {
      command.Cancel();
    }
-
-    previous = current;
  });
  return *this;
 }

 Trigger Trigger::ToggleOnTrue(Command* command) {
-  m_loop->Bind([condition = m_condition, previous = m_condition(),
-                command = command]() mutable {
-    bool current = condition();
-
+  AddBinding([command](bool previous, bool current) {
    if (!previous && current) {
      if (command->IsScheduled()) {
        command->Cancel();
@@ -175,17 +133,12 @@ Trigger Trigger::ToggleOnTrue(Command* command) {
        command->Schedule();
      }
    }
-
-    previous = current;
  });
  return *this;
 }

 Trigger Trigger::ToggleOnTrue(CommandPtr&& command) {
-  m_loop->Bind([condition = m_condition, previous = m_condition(),
-                command = std::move(command)]() mutable {
-    bool current = condition();
-
+  AddBinding([command = std::move(command)](bool previous, bool current) {
    if (!previous && current) {
      if (command.IsScheduled()) {
        command.Cancel();
@@ -193,17 +146,12 @@ Trigger Trigger::ToggleOnTrue(CommandPtr&& command) {
        command.Schedule();
      }
    }
-
-    previous = current;
  });
  return *this;
 }

 Trigger Trigger::ToggleOnFalse(Command* command) {
-  m_loop->Bind([condition = m_condition, previous = m_condition(),
-                command = command]() mutable {
-    bool current = condition();
-
+  AddBinding([command](bool previous, bool current) {
    if (previous && !current) {
      if (command->IsScheduled()) {
        command->Cancel();
@@ -211,17 +159,12 @@ Trigger Trigger::ToggleOnFalse(Command* command) {
        command->Schedule();
      }
    }
-
-    previous = current;
  });
  return *this;
 }

 Trigger Trigger::ToggleOnFalse(CommandPtr&& command) {
-  m_loop->Bind([condition = m_condition, previous = m_condition(),
-                command = std::move(command)]() mutable {
-    bool current = condition();
-
+  AddBinding([command = std::move(command)](bool previous, bool current) {
    if (previous && !current) {
      if (command.IsScheduled()) {
        command.Cancel();
@@ -229,8 +172,6 @@ Trigger Trigger::ToggleOnFalse(CommandPtr&& command) {
        command.Schedule();
      }
    }
-
-    previous = current;
  });
  return *this;
 }
--- a/wpilibNewCommands/src/main/native/include/frc2/command/button/Trigger.h
+++ b/wpilibNewCommands/src/main/native/include/frc2/command/button/Trigger.h
@@ -11,6 +11,7 @@
 #include <frc/event/EventLoop.h>
 #include <frc/filter/Debouncer.h>
 #include <units/time.h>
+#include <wpi/FunctionExtras.h>

 #include "frc2/command/Command.h"
 #include "frc2/command/CommandScheduler.h"
@@ -291,6 +292,13 @@ class Trigger {
  bool Get() const;

 private:
+  /**
+   * Adds a binding to the EventLoop.
+   *
+   * @param body The body of the binding to add.
+   */
+  void AddBinding(wpi::unique_function<void(bool, bool)>&& body);
+
  frc::EventLoop* m_loop;
  std::function<bool()> m_condition;
 };