search

ImpulseAdventure / GUIslice

GUIslice drag & drop embedded GUI in C for touchscreen TFT on Arduino, Raspberry Pi, ARM, ESP8266 / ESP32 / M5stack using Adafruit-GFX / TFT_eSPI / UTFT / SDL
https://www.impulseadventure.com/elec/guislice-gui.html
MIT License
1.19k stars 211 forks source link

MCU restarts using spinner control #459

Open rvxfahim opened 2 years ago

rvxfahim commented 2 years ago

Describe the bug

MCU crashes if i press the up or down arrows of a spinner control, and nothing else on the screen and no program logic. Crashes with latest guislice release.

I tested toggle button control, that works fine.

Device hardware

Version 0.16 does not have this issue

Pconti31 commented 2 years ago

@rvxfahim Sorry its taken so long for anyone to answer you. I've been busy working on a major update to the Builder. So I confirmed what you said, the spinner is broken in release 0.17 and onward and does work in 0.16.

Since I made the last major change to XSpinner I decided to make sure I didn't ask for a pull with bad code. I grabbed the commit version of my pull request to allow users to substitute up/down character committed on Mar 3, 2020 1b1553c2c15cb5786f18a137b4e8ef93bec80b07 and Yea! spinner works. So I'm not an idiot and I didn't summit bad code.

So where did things go wrong. I did what amounts to a binary search of commits to track down what pull request did break the spinner.

Versions where Spinner works

Versions where Spinner is broken

So July 4 everything is OK and on July 10 the XSpinner causes Arduino Mega to reset. Hopefully Calvin @ImpulseAdventure will take a look at why.

Paul--

rvxfahim commented 2 years ago

is this a mega specific issue or are all mcus affected? I do not have any other board to test this.

Pconti31 commented 2 years ago

@rvxfahim @ImpulseAdventure Good question. I just tried a ESP32 and the same result.

INFO:   Add elem to collection: ElemRef=10/10, ElemRAM=10/10, ElemId=8 (RAM)
Guru Meditation Error: Core  1 panic'ed (Unhandled debug exception)
Debug exception reason: Stack canary watchpoint triggered (loopTask)
Core 1 register dump:
PC      : 0x400d1995  PS      : 0x00060b36  A0      : 0x800d276c  A1      : 0x3ffb0060
A2      : 0x3ffc0ff0  A3      : 0x006400c8  A4      : 0x00140044  A5      : 0x00000006
A6      : 0x0000006d  A7      : 0x3ffc0370  A8      : 0x3ffc10f0  A9      : 0x00000001
A10     : 0x00000001  A11     : 0x00000001  A12     : 0x00000006  A13     : 0x3ffc06b4
A14     : 0x00000006  A15     : 0x3ffc10f0  SAR     : 0x0000001e  EXCCAUSE: 0x00000001
EXCVADDR: 0x00000000  LBEG    : 0x4000164d  LEND    : 0x40001667  LCOUNT  : 0xfffffffe

Backtrace: 0x400d1995:0x3ffb0060 0x400d2769:0x3ffb0090 0x400d2d59:0x3ffb00b0 0x400d330b:0x3ffb00d0 0x400d341b:0x3ffb0100 0x400d40bb:0x3ffb0130 0x400d2a8e:0x3ffb0150 0x400d2ad6:0x3ffb0180 0x400d3339:0x3ffb01a0 0x400d341b:0x3ffb01d0 0x400d40bb:0x3ffb0200 0x400d2a8e:0x3▒

Seems to be caught in a loop and then we run out of stack space. My guess is that when Calvin updated his code for Input Buttons he forgot to update this control with whatever is now required to fit his input model.

If you really need this control you might be better off staying with Release 16 until Calvin is not longer MIA. The Builder should still work with 16 as long as you are not using any hardware input buttons.

Once I finish off my next release of the Builder (either 17b12 or 18.0) I'll take a deeper look at this if Calvin still hasn't answered. Paul--

Pconti31 commented 2 years ago

@rvxfahim @ImpulseAdventure I had some free time this morning so I took a look at this bug. When I first wrote this routine I used a feature of GUIslice API call compound elements (I also used this creating the first Keypad). This feature lets you define a new control based upon existing controls that you package together to make the new super control.

For the spinner its simply two text buttons and one text field. Over time these compound elements have been re-written to use a technique I call Virtual elements. Complex Elements that Users can see with multiple parts but as far as GUIslice is concerned is only one control.

Anyway, The XSpinner is the last control using compound elements and it seems this feature is broken in the latest GUIslice API. The good news is quite a bit of the API code can be removed if this feature is dropped.

Rather then me going nuts trying to debug this feature I simply rewrote the XSpinner to use virtual elements and it works for me using Arduino mega and ESP32 boards I own. Attached is my new version. Post if it works for you using GUIslice 17 and if so I'll post a pull request. I can't say when or if Calvin would process or accept it since I still have an outstanding pull request from 23 days ago. XSpinner.zip

Paul--

rvxfahim commented 2 years ago

This is working on top of the 0.17 Guislice version. For now I consider this as a hotfix :v:

Pconti31 commented 2 years ago

@rvxfahim @ImpulseAdventure I made the pull request #461 to replace the XSpinner. If you are happy with this fix, please add a star to my GUIsliceBuilder repository. Paul--

rvxfahim commented 2 years ago

@rvxfahim @ImpulseAdventure I made the pull request #461 to replace the XSpinner. If you are happy with this fix, please add a star to my GUIsliceBuilder repository. Paul--

I just put a star in this repo https://github.com/ImpulseAdventure/GUIslice-Builder. Is this your repo ?

Pconti31 commented 2 years ago

Yes, Thanks!

ImpulseAdventure commented 2 years ago

Thank you, Paul!!

Yes, I have been very tied up at work and still not "moved in" with access to my dev computer & electronics (due to a prolonged renovation). I expect to have access to my dev setup soon as the reno wraps up. Again, apologies for the delays.

Thank you very much for the "commit bisect" to identify the spinner issue! Yes, it is certainly likely that compound elements have triggered a boundary condition that is unhandled with the new control flow. As you're aware, the compound elements introduce a fair amount of complexity and may be (to my knowledge) relatively unused. For some time I have been inclined to to remove support for them (as they add a lot of overhead/complexity), but have not done so yet.

I really like your updated XSpinner as a demonstration of the virtual elements. If possible, I think it may be a good idea to promote the XSpinnerDrawVirtualTxt() and XSpinnerDrawVirtualBtn() calls to be GUIslice native functions. What do you think? If so, I can look at making a generic version that we can use in XSpinner and XKeyPad (at least the VirtualBtn).

@rvxfahim - Thanks for the initial testing and also confirming that Paul's PR works. I plan to merge those changes in.

Pconti31 commented 2 years ago

@ImpulseAdventure Yes, I think you can now remove support for compound elements. No other shipping controls use it. Coming up with common Virtual Element button and text gets my vote. Paul--