Skip to content
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

Unhandled exception from runOnce command called from a Trigger if it creates further Triggers. #7734

Open
chauser opened this issue Jan 26, 2025 · 5 comments
Labels
type: bug Something isn't working.

Comments

@chauser
Copy link
Contributor

chauser commented Jan 26, 2025

Describe the bug
A clear and concise description of what the bug is.

Error at frc.robot.TestBehaviors.bindButtons(TestBehaviors.java:85): Unhandled exception: java.util.ConcurrentModificationException: Cannot bind EventLoop while it is running
        at edu.wpi.first.wpilibj.event.EventLoop.bind(EventLoop.java:28)
        at edu.wpi.first.wpilibj2.command.button.Trigger.addBinding(Trigger.java:70)
        at edu.wpi.first.wpilibj2.command.button.Trigger.onTrue(Trigger.java:110)
        at frc.robot.TestBehaviors.bindButtons(TestBehaviors.java:85)
        at edu.wpi.first.wpilibj2.command.FunctionalCommand.initialize(FunctionalCommand.java:52)
        at edu.wpi.first.wpilibj2.command.WrapperCommand.initialize(WrapperCommand.java:38)
        at edu.wpi.first.wpilibj2.command.CommandScheduler.initCommand(CommandScheduler.java:169)
        at edu.wpi.first.wpilibj2.command.CommandScheduler.schedule(CommandScheduler.java:212)
        at edu.wpi.first.wpilibj2.command.CommandScheduler.schedule(CommandScheduler.java:243)
        at edu.wpi.first.wpilibj2.command.Command.schedule(Command.java:538)
        at edu.wpi.first.wpilibj2.command.button.Trigger.lambda$onTrue$1(Trigger.java:113)
        at edu.wpi.first.wpilibj2.command.button.Trigger$1.run(Trigger.java:78)
        at java.base/java.lang.Iterable.forEach(Iterable.java:75)
        at edu.wpi.first.wpilibj.event.EventLoop.poll(EventLoop.java:38)
        at edu.wpi.first.wpilibj2.command.CommandScheduler.run(CommandScheduler.java:280)
        at frc.robot.Robot.robotPeriodic(Robot.java:24)
        at edu.wpi.first.wpilibj.IterativeRobotBase.loopFunc(IterativeRobotBase.java:400)
        at edu.wpi.first.wpilibj.TimedRobot.startCompetition(TimedRobot.java:133)
        at edu.wpi.first.wpilibj.RobotBase.runRobot(RobotBase.java:419)
        at edu.wpi.first.wpilibj.RobotBase.lambda$startRobot$1(RobotBase.java:490)
        at java.base/java.lang.Thread.run(Thread.java:840)

arising from this code:

      ...
      // var bindCommand(Commands.run(this::bindButtons).until(true));
      var bindCommand = Commands.runOnce(this::bindButtons);
      new Trigger(m_testerController.getHID()::isConnected)
              .onTrue(bindCommand.ignoringDisable(true));
      ...

private void bindButtons() {
        if (m_buttonsBound || !m_testerController.getHID().isConnected()) return;
        m_buttonsBound = true;
        // the following line is  frc.robot.TestBehaviors.bindButtons(TestBehaviors.java:85) in the traceback above
        m_testerController.start().onTrue(Commands.runOnce(m_coralSim::loadCoral).withName("LoadSimCoral"));
        m_testerController.rightBumper().whileTrue(new IntakeCommand(m_intake, m_elevator));
}

when m_testerController is connected causing bindButtons() to be run.

Expected behavior
The bindButtons method binds the buttons without incurring an exception.

Desktop (please complete the following information):

  • OS: Linux 22.04 (simulation). The nature of the behavior suggests that it would be the same on the roboRIO or any other desktop.
  • Project Information:
  • WPILib Information:
    Project Version: 2025.2.1
    VS Code Version: 1.94.2
    WPILib Extension Version: 2025.2.1
    C++ Extension Version: 1.23.2
    Java Extension Version: 1.38.0
    Java Debug Extension Version: 0.58.2024090204
    Java Dependencies Extension Version 0.24.1
    Java Version: 17
    Java Location: /home/hauser/wpilib/2025/jdk
    Vendor Libraries:
    PathplannerLib (2025.2.1)
    CTRE-Phoenix (v5) (5.35.1)
    CTRE-Phoenix (v6) (25.2.1)
    REVLib (2025.0.2)
    ReduxLib (2025.0.1)
    Studica (2025.0.1)
    ThriftyLib (2025.0.1)
    WPILib-New-Commands (1.0.0)
    maplesim (0.3.1)
    photonlib (v2025.1.1)
    YAGSL (2025.2.2)

Additional context
For a couple of seasons I've had this as var bindCommand = Commands.run(this::bindButtons). It has worked fine, but this year I noticed that the command was left running after doing its job -- it should have been runOnce. But on changing it to runOnce it suffered the ConcurrentModificationException as shown above.
I think that the problem is that when a Command is scheduled from a Trigger, the Command's start method is called from the Trigger processing loop; and Commands.runOnce places the method to be run as the start method of the Command that it constructs. Using var bindCommand(Commands.run(this::bindButtons).until(true)); instead, the error does not happen because this::bindButtons is placed as the execute method of the constructed command where it is called from the Scheduler loop.
I'm not sure this needs to be fixed -- at this level of the system small changes may well affect other edge-case behaviors. But at least it is now noted and a work-around provided.

@chauser chauser added the type: bug Something isn't working. label Jan 26, 2025
@KangarooKoala
Copy link
Contributor

Thanks for the detailed bug report! Triggers are only intended to be bound once at startup, so it's unlikely we'll want to fix this behavior. (Glad you found a workaround though!)

On another note, is there a reason why you can't unconditionally bind the controls to m_testerController?

@chauser
Copy link
Contributor Author

chauser commented Jan 27, 2025

Yes -- the idea is to only bind those commands when the test controller is plugged in so that we can have the test code always present in the robot code but not get continuous warning about missing buttons and sticks when the test controller is not plugged in. And we definitely don't want it plugged in all the time because it lets you do things that are not intended for drivers to use in competition.

@KangarooKoala
Copy link
Contributor

KangarooKoala commented Jan 27, 2025

Ah, right, I forgot about the controller disconnected warnings. Since there's a workaround and this isn't a primary intended use of the API, I think this is fine? One of the maintainers should probably confirm before closing this issue.

@chauser
Copy link
Contributor Author

chauser commented Jan 27, 2025

No pressure from me to change it.

@Starlight220
Copy link
Member

You can bind the buttons to a separate EventLoop and poll it depending on whether the controller is connected or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working.
Projects
None yet
Development

No branches or pull requests

3 participants