Add infinite loop after wdt_enable call

[kasbah]

Currently when using Leonardo and derivatives reseting the watchdog in the main sketch code will interfere with the bootloader reset. This solution was proposed by pjanco in the arduino.cc forums.

[NicoHood]

Please see https://github.com/arduino/Arduino/issues/6033

[MLXXXp]

This is a situation I didn't consider when working on issue #6033 and PR #6055. My PR is has changes to restore the watchdog timer for a sketch that's using it, if the auto-boot is aborted by bringing the port back up within the watchdog window. It doesn't account for this problem with the sketch resetting the watchdog.

Running an infinite loop after enabling the watchdog is not an acceptable fix, though. We're in the middle of an ISR at this point, with interrupts disabled. Looping here would mean the ISR would never exit, so additional interrupt handling to clean up for, or abort, the auto-reset sequence could not occur.

I think using an infinite loop to prevent the sketch code from resetting or disabling the watchdog is probably the only acceptable solution, but it would have to be executed in the foreground, not in an ISR.

Perhaps the ISR could just set a flag which could be tested in the infinite for loop that calls loop() in file main.cpp. The flag would be set in CDC.cpp after starting the auto-reset watchdog (where the loop is in this PR). The flag would be cleared where the watchdog is stopped and restored if the port comes back up.

Declare the flag somewhere:

volatile bool _autoBootWatchdogRunning = false;

In main.cpp:

--- org/main.cpp	2016-02-19 10:57:26.000000000 -0500
+++ new/main.cpp	2017-03-12 17:47:08.483680084 -0400
@@ -43,6 +43,9 @@
 	setup();
     
 	for (;;) {
+#if defined(USBCON)
+		while (_autoBootWatchdogRunning) { }
+#endif
 		loop();
 		if (serialEventRun) serialEventRun();
 	}

Note that this fix will not prevent the case where the sketch is resetting the watchdog within an ISR that it has created. I don't think there is an acceptable solution for this case. You would have to disable all interrupts (or shut down their source in some other way), other than those used by USB, during the watchdog timeout window. If the auto-reset was aborted, you would then have to re-enable those interrupts that were previously enabled. This would likely end up being quite messy.

[MLXXXp]

I've done some testing of my proposal above and it seems to work quite well. I can create a pull request for it but it would be best to decide the fate of PR #6055 first, and then add it as an additional commit to #6055, or make a separate PR against its changes afterwards.

[joshgoebel]

I think using an infinite loop to prevent the sketch code from resetting or disabling the watchdog is probably the only acceptable solution, but it would have to be executed in the foreground, not in an ISR.

Will that still allow the USB code to run incase the baud rate is toggled again, cancelling the reset - or is that functionality that no one uses anyways? Is any USB stuff is processed by serialEventRun then that would need to still be running while you skip loop().

Basically turning it into:

if (!_autoBootWatchdogRunning) { 
  loop();
}

[joshgoebel]

Note that this fix will not prevent the case where the sketch is resetting the watchdog within an ISR that it has created. I don't think there is an acceptable solution for this case.

Yeah I think that's an unsolvable edge case. Though most sketches shouldn't be mucking with the WDT.

[MLXXXp]

@yyyc514,
The USB code for cancelling the reset will still run properly. It's all interrupt driven.

I believe serialEventRun is strictly for the benefit of the serialEvent() function made available to sketches, so it's better not to run it while waiting on the watchdog. Also, I think it's only for physical UARTs not the USB CDC virtual one.

[joshgoebel]

I believe serialEventRun is strictly for the benefit of the serialEvent() function made available to sketches, so it's better not to run it while waiting on the watchdog. Also, I think it's only for physical UARTs not the USB CDC virtual one.

Ok, great. I just saw that and didn't look at it super closely.

[facchinm]

Closing in in favour of #6055