Faster Lynx E-STOP

[Windwoes]

This patch addresses ftc_app 712 and puts the Lynx modules into an E-STOP state much faster than before if the app was restarted due to a user OpMode refusing to exit.

Here are two videos demonstrating this:

• Before (Notice that the LED strip does not turn off until WELL after the RC app has closed)
• After (Notice that the LED strip turns off immediately after the RC detects that the OpMode is stuck)

Not only does this increase safety, but it can also help teams avoid penalties if, for example, their autonomous OpMode does not stop properly at the end of the 30 seconds.

[cmacfarl]

Hello @FROGbots-4634 This PR is being evaluated for inclusion in the official SDK. However, I see you have it marked a a work in progress? Can you elaborate on that a bit? Thanks.

[Windwoes]

Hi @cmacfarl, so it's marked as WIP for two reasons:

• I haven't been able to test it in a configuration with an RS-485 daisy-chained Hub, because at the moment don't have access to another Expansion Hub. Now, I see no reason why it wouldn't work, but you know how "this ought to work!" goes, hahaha :)

• While I believe the implementation is correct, and it performs as desired during testing, I'm still working to understand how a few edge cases play out.

[Windwoes]

@cmacfarl Quick update: Noah suggested I actually create a couple test cases for this outside of the SDK, which did in fact make checking behavior in edge cases much easier. I did in fact discover an issue, (which is that hangAllFutureAcquisitions is checked without the lock held) which, while it does not impact a normally running OpMode at all, has the potential to render this whole patch ineffective. So, it may be wise to hold off on merging this just yet, in order to let me fix that issue and continue to test a few more edge cases.

[Windwoes]

@cmacfarl Sorry for the delay. I have pushed two more commits. The first updates this branch to be based off v5.1 (no merge conflicts though). The second reworks my previous implementation to an implementation that I feel much more comfortable with - I was able to fix the issues that I found in the old one as well as test to make sure that several edge conditions play out correctly by following Noah's suggestion. However, that being said, given that this is my third try at implementing this correctly, I think it would be wise to have Bob give this a quick lookover before merging since it touches the lowest levels of the Lynx part of the SDK.

[gearsincorg]

@FROGbots-4634 I'm in the process of testing this and it looks good so far.
I've verified that it also works for daisy chained Expansion Hubs.
< 1 Sec shutdown vs 3-4 seconds.

What sort of "edge conditions" were you concerned about..... to help focus my testing.

Phil.

[Windwoes]

@gearsincorg

So one of them was when you have more than one thread spamming the hardware during the E-STOP condition (one of my implementations could get into deadlock in that condition). Another was ensuring that this line was not an issue if the user code was interrupted while a command was in flight and waiting for ACK. I added a comment above it detailing why it's not an issue, but part of me thinks it should be changed to the non-interruptible lock() just to make it more robust against future changes. There were a couple others but those were the most significant (and I'm not even sure I remember what the others were).

[Windwoes]

@gearsincorg also since you're testing, it might be nice if you could just confirm my finding that adding the extra ReentrantLock does not impact loop time / max hardware calls per second in any way.

[Windwoes]

@cmacfarl Any update on this? By the way just to clarify, the edge conditions I mentioned to Mr. Phil were ones that I had tested and played out correctly. Also, even if another weird edge condition exists that would cause something to hang, the worse case scenario is that app restart would be delayed 250ms.

[cmacfarl]

I was waiting for @gearsincorg to circle back around on the issue.

[gearsincorg]

This has been merged, and will be in 5.3 to be released soon.
I did some I/O speed tests, and saw no degradation in performance over 5.2
Any changes in loop times were not observable.