frc2609 / Crescendo-2024

Robot code for Crescendo 2024.
Other
1 stars 0 forks source link

Ridiculous Loop Overruns #58

Closed UserC2 closed 6 months ago

UserC2 commented 6 months ago

Bug Description

Steps to Reproduce

  1. Run the code on the robot with all subsystems enabled.
  2. Attempt to drive.
  3. Observe the drive Krakens are blinking orange intermittently (indicating they are disabling themselves) accompanied by a loud clicking sound.

Expected Behavior

Ideas

newjanson commented 6 months ago

Could this be because of the Results results = LimeLightHelpers.getLatestResults("limelight").targetingResults; line in Limelight.java?

UserC2 commented 6 months ago

Issue #5903 in allwpilib specifies it's not a good idea to do e.g. if (controller.leftBumper().getAsBoolean()) ... since every time a call like this is made, a function is added to the EventLoop in CommandScheduler to update the value of the trigger (which is only used once then discarded at the end of the expression)!

We do this in two places:

To fix this, store the trigger in a variable, then call getAsBoolean() on the stored trigger.

After running the robot code for ~5 minutes: image

;)

(This is not the solution, but it certainly isn't helping.)

I lied, the event loop ends up taking ~95% of CPU time eventually (on the laptop) just absolutely full of BooleanSuppliers... this might be significant.

UserC2 commented 6 months ago

Also, VisualVM is helpful in diagnosing loop overruns.

WhyNot180 commented 6 months ago

Could this be because of the Results results = LimeLightHelpers.getLatestResults("limelight").targetingResults; line in Limelight.java?

Changed all json parsing to network tables calls in commit 3a80943. It still needs to be tested how much of an effect this will have.

UserC2 commented 6 months ago

Could this be because of the Results results = LimeLightHelpers.getLatestResults("limelight").targetingResults; line in Limelight.java?

Changed all json parsing to network tables calls in commit 3a80943. It still needs to be tested how much of an effect this will have.

image

@WhyNot180 @newjanson

What do you think this does?

WhyNot180 commented 6 months ago

Could this be because of the Results results = LimeLightHelpers.getLatestResults("limelight").targetingResults; line in Limelight.java?

Changed all json parsing to network tables calls in commit 3a80943. It still needs to be tested how much of an effect this will have.

image

@WhyNot180 @newjanson

What do you think this does?

That is redundant, results.valid is equivalent to getTV (targets valid)

UserC2 commented 6 months ago

Could this be because of the Results results = LimeLightHelpers.getLatestResults("limelight").targetingResults; line in Limelight.java?

Changed all json parsing to network tables calls in commit 3a80943. It still needs to be tested how much of an effect this will have.

image @WhyNot180 @newjanson What do you think this does?

That is redundant, results.valid is equivalent to getTV (targets valid)

That's the fun part, because I've gotten it to crash the code :)

(If the Limelight is poorly tracking the AprilTags (e.g. dark room), then there's a pretty decent chance the code will eventually run into a point where the tags were visible on LimelightHelpers.getTV(...) and then by the next call to NetworkTables (getLatestResults) the Limelight loses track of the targets. Then, the exception is triggered, and goodbye robot code.)

The more useful part of this is that when this happens, odometry becomes NaN, which leads to the shooter angle calculated by AutoSetShooter to be NaN which leads to a fun conversation about why the robot snapped the cable chain again.

UserC2 commented 6 months ago

Closing for now, as #84 fixed some major ones.