Adafruit-GFX-Library
Adafruit-GFX-Library copied to clipboard
adafruit
ESP8266 panic due to yield() in writeLine()
Hi,
with commit d81e3351f3c2a8e280dbbd531f8c20d949e719e8 @PaintYourDragon introduced yield() for ESP8266 into writeLine() and drawCircle().
void Adafruit_GFX::writeLine(int16_t x0, int16_t y0, int16_t x1, int16_t y1,
uint16_t color) {
#if defined(ESP8266)
yield();
#endif
Does anyone know the reason for that?
The problem with this is that calling yield() outside loop, e.g. from within code called by a Ticker / ISR will cause the ESP8266 to panic an reset. Thus, some drawing functions cannot be used with Ticker.
Code to reproduce the issue with yield():
#include <Arduino.h>
#include <Ticker.h>
Ticker tick;
ICACHE_RAM_ATTR void tock() {
yield();
}
void setup() {
tick.attach_ms(100, tock);
}
void loop() {
}
Without the yield(), certain time-consuming drawing operations (especially large circles, but others as well) will trigger the watchdog timer and reset the board.
I’d suggest not calling drawing functions directly from a Ticker’ed function. Have the function set a global volatile bool, and elsewhere (e.g. loop() or whatever) periodically poll the variable, call drawing function if it’s set, then clear the flag.
Your advice is right, in general. But for fast / small updates using Ticker should be fine.
Do you have numbers at what scale you run into issues?
fillCircleHelper and fillRect do call writeLine a few times in a loop. But drawCircle actually is never called from somewhere else, so I'm not sure why it would help there if it is only ever called once.
No plans to change it. This is a catch-22…it’s gonna ruin someone’s day one way or the other…and since this is the first time it’s come up in regards to a ticker (whereas the alternate case of no yield() manifested almost immediately with the test sketches), it’s the ticker case that’s gonna lose out.
So, here’s some other opinions:
- Use something other than an ESP8266 for the project. It’s great and cheap and all but that watchdog timer is like a sword of Damocles hanging over anything running on those boards. Maybe they’re just not well-suited to run large bitmapped displays, unless you have lower-level control over the whole application and where time is spent.
- Make a duplicate of the circle-drawing function that leaves out the yield()…either in your own sketch, or in a branch of the GFX library (so it can keep up to date with any other changes).
- If the circle in question is always the same size, or if there’s a fixed set of sizes, consider making a GFXcanvas1 object on startup, sized for the circle. Draw the circle in there, then (in your ticker function) use drawBitmap() to copy that to the screen instead of the circle-drawing function. If the circle doesn’t need to be “clear” (that is, if you can draw the bitmap with both foreground and background colors set) it should be super fast (no yield() required), as it’s just a rectangle blit. (Or use a GFXcanvas16 and drawRGBBitmap()…faster, at the expense of a little RAM.)
Maybe we can leave the yield() behavior customizable to handle edge cases like this. I just wrote a quick prototype of the idea. See draft PR #293 .
Just a thought. Cheers.
@BillyDonahue thx a lot! I would have also suggested some kind of switch.
I'm not familiar with a "weak function", I would have gone with a simple #define ADAFRUIT_SKIP_YIELD. Looks to me that overriding a weak function is more work than a simple define. Are there any benefits with using a weak function?
Btw. the reason why I use ticker is because of the initialization process of my project. When it starts, there are few blocking operations and not all of them are even under my control (like WifiManager waiting for connection). During this, I still have to update the display with some status information. Using a flag would be a PITA because I would have to add the updateDisplay function to several different places. The code I had worked fine since 2017. It took me some time to realize why it would crash with recent libraries...
@BillyDonahue That seems like a reasonable compromise. Then maybe a setter function to enable (default) or disable (@xsrf's case) whether yielding occurs. I’d feel better then about adding other yield checks in other parts of the code where the watchdog hasn’t flared up yet in practice but is probably just waiting (like when drawing lots of text, things like that).
@xsrf :
Looks to me that overriding a weak function is more work than a simple define.
All you do is write a function somewhere in your sketch called void Adafruit_GFX_Yield();. The linker will choose yours because the other one is marked "weak". It's less work than a macro or whatever, and it optimizes better than flags or function pointers.
In an Arduino sketch, the linker is configured to do aggressive garbage collection on a symbol-by-symbol basis. Any variable or function that isn't referenced by the program is omitted. The weak symbol technique was chosen to take advantage of this property. The weak void Adafruit_GFX_Yield() is simply omitted when replaced, saving a little space.
I would have gone with a simple #define ADAFRUIT_SKIP_YIELD
I don't see how that would work. User-provided #defines have no way of affecting a library's C++ files in Arduino IDE, and there's no place in the IDE to set custom -D options for gcc per sketch. So you'd have users modifying the source code of the library or their platform definition, just to tweak a tiny behavioral detail. At that point they could just delete the yield() calls entirely. I think we should avoid requiring that sort of "bad old days" practice and just make the vanilla lib customizable from the user-side.
@PaintYourDragon , I was thinking that since the user-provided Adafruit_GFX_Yield() is arbitrary, users can provide their own setters or enablers, or even do something other than yield() if appropriate, or only yield() when enough ticks have expired or something. The cost of checking for that bool or timestamp would be zero for naive apps. They just keep doing what they've always done, yielding here and there. But sophisticated eggheads could get fancy.
Leaving the callback open-ended solves more problems than a toggle to enable and disable yield() calls. Apps will know better than GFX when they need to serve their background processes. They might need other periodic callbacks to be served besides yield. Like mqttClient.loop() or timer.tick(), so the problem isn't ESP8266 specific.
We could also handle it by adding a void(*_yield)() data member to Adafruit_GFX, initialized to point to a default implementation that does the usual yield on ESP8266. But then the default yield() behavior would still be linked into executables that don't need it. If that's the only call to yield() in the system, it seems to me that this dead code would force the linker to keep yield() and all the scheduler code to support it, which might be a drag? I haven't tested this assertion, just reasoning about it.
PS: Yes, I am surprised there are not yet similar yield() calls sprinkled into the expensive glyph-rasterizing write() functions. I suppose people who find it to be a problem might not complain about it because they can contort their code to print one character at a time and yield() between them, whereas if they have a long-running fillCircle call they're stuck.
@BillyDonahue thx for the explanation!
btw. isn't it drawPixel() that may slow down the execution of large drawing calls? At the end, drawPixel() might be lightning fast if it draws to ram, or incredible slow, if it has to access slow hardware. 🤔
Oh! I get it now. Yeah, doesn’t even need setters, that’s great. (Never dealt with weak functions before. Neat.) @xsrf Yeah, the slowdown on some (not all) drawing primitives is more a function of how many address window settings are performed, which is a fairly SPITFT-centric concept. Maybe it’s something best done in Adafruit_SPITFT::startWrite()? That way it won’t affect RAM-based GFX subclasses…the canvases and such.
Just fyi, found a related issue https://github.com/witnessmenow/WiFi-Tetris-Clock/issues/8
Hi guys, I just stumbled upon this issue. Would replacing yield with wdtFeed solve the case? I cannot test from smartphone, but seems that would reset watchdog and at the same time will not do crashing things that yeild does (maybe, that needs to be checked)
