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.17k stars 209 forks source link

Long Press Event (GSLC_TOUCH_MOVE_IN) Not Happening #455

Open GeorgeIoak opened 2 years ago

GeorgeIoak commented 2 years ago

Describe the bug

When using the FT206 based touch screen driver I'm not seeing this event happening when making a long press

Device hardware

Please confirm whether:

Expected behavior

We should be getting a GSLC_TOUCH_MOVE_IN event but I'm not seeing any with my debug code

Additional info

I am using the esp-tftespi-default-ft6206.h config and doing some testing with button presses. I'm not able to detect a long press event so I did a little debugging.

Here's my button callback

bool CbBtnCommon(void* pvGui,void *pvElemRef,gslc_teTouch eTouch,int16_t nX,int16_t nY)
{
  // Typecast the parameters to match the GUI and element types
  gslc_tsGui*     pGui     = (gslc_tsGui*)(pvGui);
  gslc_tsElemRef* pElemRef = (gslc_tsElemRef*)(pvElemRef);
  gslc_tsElem*    pElem    = gslc_GetElemFromRef(pGui,pElemRef);
  char            acTxt[MAX_STR];

  GSLC_DEBUG_PRINT("TOUCH Event is %u\n",eTouch);

  if (eTouch == GSLC_TOUCH_DOWN_IN) {  // Started press (inside button)
    m_tmLastPress = millis();
  } else if ( eTouch == GSLC_TOUCH_UP_IN ) {  // Released (inside button)
    // From the element's ID we can determine which button was pressed.
    switch (pElem->nId) {
//<Button Enums !Start!>
      case E_BTN_UP:
        m_nH2OTemp++;
        break;
      case E_BTN_DOWN:
        m_nH2OTemp--;
        break;
//<Button Enums !End!>
      default:
        break;
    }
    GSLC_DEBUG_PRINT("Count = %d\n",m_nH2OTemp);
    snprintf(acTxt, MAX_STR, "%u", m_nH2OTemp);
    gslc_ElemSetTxtStr(&m_gui, m_pElemH2OTemp, acTxt);

    } else if ( eTouch == GSLC_TOUCH_MOVE_IN ) {  // Continued press (inside button)
    if ((millis() - m_tmLastPress) > 500) {
    // From the element's ID we can determine which button was pressed.
    switch (pElem->nId) {
//<Button Enums !Start!>
      case E_BTN_UP:
        m_nH2OTemp += 5;
        break;
      case E_BTN_DOWN:
        m_nH2OTemp -=5;
        break;
//<Button Enums !End!>
      default:
        break;
    }
    m_tmLastPress = millis();
    GSLC_DEBUG_PRINT("Long Press Count = %d\n",m_nH2OTemp);
    snprintf(acTxt, MAX_STR, "%u", m_nH2OTemp);
    gslc_ElemSetTxtStr(&m_gui, m_pElemH2OTemp, acTxt);
  }
    }
  return true;
}

Not the most elegant but I wanted to so some quick testing. I added the debug statement to print the value of eTouch and if I'm understanding the definitions correctly from https://github.com/ImpulseAdventure/GUIslice/blob/master/src/GUIslice.h#L347 I should be seeing a value of 8 for a GSLC_TOUCH_MOVE_IN touch event.

The code works for quick press and single count up/down but for a long press I don't see the GSLC_TOUCH_MOVE_IN touch event, I go straight from GSLC_TOUCH_DOWN_IN to GSLC_TOUCH_MOVE_OUT. I'm touching and holding so I don't think it's a matter of my finger moving but I've done this many times and I don't see the GSLC_TOUCH_MOVE_IN touch event happening.

Count = 135
TOUCH Event is 2
TOUCH Event is 9
TOUCH Event is 6
TOUCH Event is 2
TOUCH Event is 5
Count = 136
TOUCH Event is 2
TOUCH Event is 5
Count = 137
TOUCH Event is 2
TOUCH Event is 5
Count = 138

Is this something with the FT206 driver? I confused how you can get a move out without getting a move in event.

Pconti31 commented 2 years ago

@GeorgeIoak First I think you did a fine job without any internals documentation. I myself did a deep dive years ago when i first tried GUIslice and made many notes. At the beginning of Covid I brought them together and even updated them into one document and sent it to Calvin. It was incomplete so no sense in publishing it. Now it's no longer accurate due to many changes Calvin has made since so... Opportunity lost I guess. Anyway, if you ever do a deep dive you will discover many things GUIslice can do that otherwise would seem impossible. For example, i once showed a user how to create a draggable window.

Back to long presses, You are missing the GSLC_TOUCH_UP_IN event.

Now given all of the Button changes for better support of hardware buttons (GSLC_FEATURE_INPUT=1) vs Touch events I really no longer fully understand the flow so I won't try and explain further.

Attached is my example of long touch I provided to another user back in 2020. I just fired it up today on my ESP32 using TFTe_SPI to see if it still works and it does! So I zipped it up for you.

Longpress-Btn.zip

Good luck! Paul--

GeorgeIoak commented 2 years ago

Hi @Pconti31 thanks, I'm trying not to be a pain and only post an issue after I've done as much testing and research and not finding any helpful explanations.

I downloaded your project and ran it on my hardware. Side note, thanks for having everything centered around the middle of the screen. Since the FT6206 CTP driver has a bug your button presses are perfectly mirrored so when I press "increase" the count decreases, and pressing decrease, the count increases!

Your code is close to what I was trying to achieve but it's acting the same in that I never get the count to increase by 10. Also, a minor issue with the reset button is that after you hold for 3 seconds the count does not reset to 0 but it does once you release the button.

If this sketch worked for you what touch panel are you using? If it's not the FT6206 variety I think this is adding to my believe that the capacitive touch screen is not behaving as expected. I know that simple touches work (except for the points in the wrong space) but it seems like the events are not the same. I only have capacitive touch screens so I can't run this on a RTP to test.

Is there anything else I can do to help dig deeper and find the root cause of this?

Pconti31 commented 2 years ago

I tested with HX8357 and STMPE610. I don't own a cap touch so...

Sorry, I have no idea why you are having such problems. In your config file set

define DBG_TOUCH 1

define DBG_LOG 1

and be sure they are not already being set to 0. Then expect to spend some time looking at serial output. also change Serial.begin(115200); or you will go nuts.

Have you read closed issue 360?

Paul--

GeorgeIoak commented 2 years ago

I'm actually a bit surprised that more people aren't using CTP these days. The cost is almost negligible compared to resistive screens.

I did see that issue and I've commented in 425 but those are in reference to the position swapping which still hasn't been addressed from what I can tell.

I'll have to maybe go back to your example and Arduino and set my debug to those values (if they aren't already). I always run at 115200 BAUD, I wish all the examples were at that as these days I don't think there's any reason to go slow..

BTW, do I have to set those defines in the config or can they just be in your sketch? Setting them in your sketch is easier to remember why you added them and for which project.

GeorgeIoak commented 2 years ago

Ah, here's something a bit interesting after adding those defines:

DBG: PreRotate: x=153 y=140
DBG: Touch Press=255 Raw[153,140] Out[140,166]
Trk: (140,166) P=255 : TouchDown

TOUCH Event is 2
Count = 1
DBG: PreRotate: x=153 y=140
DBG: Touch Press=255 Raw[153,140] Out[140,166]
DBG: PreRotate: x=153 y=140
DBG: Touch Press=255 Raw[153,140] Out[140,166]
DBG: PreRotate: x=153 y=140
DBG: Touch Press=255 Raw[153,140] Out[140,166]
DBG: PreRotate: x=153 y=140
DBG: Touch Press=255 Raw[153,140] Out[140,166]
DBG: PreRotate: x=153 y=140
DBG: Touch Press=255 Raw[153,140] Out[140,166]
DBG: PreRotate: x=153 y=140
DBG: Touch Press=255 Raw[153,140] Out[140,166]
DBG: PreRotate: x=153 y=140
DBG: Touch Press=255 Raw[153,140] Out[140,166]
DBG: PreRotate: x=153 y=140

.... A BUNCH more of these as I held my finger down

DBG: Touch Press=255 Raw[153,140] Out[140,166]
DBG: PreRotate: x=153 y=140
DBG: Touch Press=255 Raw[153,140] Out[140,166]
DBG: PreRotate: x=153 y=140
DBG: Touch Press=255 Raw[153,140] Out[140,166]
DBG: PreRotate: x=319 y=479
DBG: Touch Press=255 Raw[319,479] Out[479,0]
Trk: (479,0) P=255 : TouchMove

TOUCH Event is 9
DBG: PreRotate: x=319 y=479
DBG: Touch Press=0 Raw[319,479] Out[479,0]
Trk: (479,0) P=0 : TouchUp

TOUCH Event is 6

So we go in this order

DBG: Touch Press=255 Raw[153,140] Out[140,166]
Trk: (140,166) P=255 : TouchDown
TOUCH Event is 2

DBG: Touch Press=255 Raw[319,479] Out[479,0]
Trk: (479,0) P=255 : TouchMove

TOUCH Event is 9
DBG: PreRotate: x=319 y=479
DBG: Touch Press=0 Raw[319,479] Out[479,0]
Trk: (479,0) P=0 : TouchUp

TOUCH Event is 6

Notice how the TouchUp event has a completely different location than all the other reports.

I'm touching the "decrease" button on the screen which is supposed to be located at 280,130 (with a width a size of 120x40) and when I am pressing the "increase" button has changed state and the counter increases. The "increase button is at 80,130 with the same size.

I'm touching close to the center of the button so the RAW report of 153,140 does line up with the increase button (80 + 120/2) but I'm really touching at location 340,130.

So I have 2 questions:

  1. When you do the same do you get the same events or do you have an additional TouchUp event? I know the X/Y locations are off but for this investigation that shouldn't matter
  2. Is the last location reading right when you lift your finger way different than the position when you have your finger pressed?
Pconti31 commented 2 years ago

@GeorgeIoak @ImpulseAdventure While debugging GUIslice API for not showing monochrome images correctly I came across a library I had modified to help out another user back in September, 2021, Paul_V2_Adafruit_FT6206.zip

Turns out Don was having a problem with the standard Adafruit FT6206 library. Anyway, check inside closed issue #405.

It might not help you but could be worth a shot.

I would have mentioned it sooner but I'm an old man and my neurons don't fire as well as they used too. Paul--

GeorgeIoak commented 2 years ago

Hi @Pconti31 Thanks, I remember skimming through that issue but I'll have to re-read it as I too am mentally challenged at times! Longpress-Btn_GSLC.zip

I have been doing some debugging and testing this morning and I think I have a little better handle on what and how the different touch events are happening. GSLC_TOUCH_MOVE_IN only happens when you are touching a button and move inside the button region. From my point of view if you want to detect a long button press you need to follow your code example but in the main program you check if a button has been pressed and that it hasn't been released. If the time elapsed is greater than your threshold then you can react to the long press event. The code I've attached is just a work in progress and just blindly increments by 10 instead of checking what button your finger is on.

I initially was thinking that there was some event that let you know that a long press had happened. Seems like that would be a nice feature to have and could be added as a true event to the touch event handler which I think is gslc_CollectTouch in Guislice.c but I'm not 100% sure. I just know that when I placed debug statements in that function I would get general touch events like 1, 4, 7 (GSLC_TOUCH_DOWN, GSLC_TOUCH_UP, GSLC_TOUCH_MOVE) but in your button callback routine when I check eTouch I get 2, 5, 8 (`GSLC_TOUCH_DOWN_IN, GSLC_TOUCH_UP_IN, GSLC_TOUCH_MOVE_IN) so somewhere in GUIslice it get's the the feedback from the touch driver that an event happens and then that is checked somewhere in the code to see if it relates to a defined element?

I'm not sure how your long button press would work for you unless the resistive touch screen has different event handling or if you moved your finger on the button when you wanted to increment.

With the code I've attached, as long as you hold down your finger the counter will increment by 10 and update immediately. With your code the update would not happen until you lifted your finger.

If you wouldn't mind taking a look at what I've done and see if it makes sense and if it's the proper path to pursue.

Thanks, George cc @ImpulseAdventure

Pconti31 commented 2 years ago

@GeorgeIoak @ImpulseAdventure yes, I think this is question of terminology. The user I wrote the sample for wanted his or her users to actually hold down the button for x seconds then release. A long press would cause one action short another. Seems you want a continuous press to do your increment which as you point out requires some minor mods to my example. I'm responding on my tablet while watching TV so I'll look over your code tomorrow but I expect it's fine. Have fun with your dive into Calvin's code. Paul--

GeorgeIoak commented 2 years ago

Enjoy your evening @Pconti31 and thanks again for following me on this journey. I'm struggling right now so it might be time to take a break.

Yes, For my case it's what I would call a typical button hold case, like on your keyboard or mouse. The code I sent you seemed to work if I ignored which button was held but now that I'm trying to add logic I've messed everything up. I also just noticed that in your code you increment on the touch down but for short taps I would expect this to happen when the button was released. Again, similar to a mouse button in that "it" typically doesn't respond until you release the mouse button. That's a minor issue at this point as quick taps seem to be working just fine for me.

I won't share what I have now because it's full of debug statements and is a mess.

Pconti31 commented 2 years ago

@GeorgeIoak @ImpulseAdventure Well, I don't know about typical but long presses were used a lot with the old 16x2 LCD displays that had only 4 or 5 buttons. If you are taking a break I suggest closing this issue now and open a new ne if you run into any more issues. I'm happy to help out whenever. Also, I would note that my example was meant as a demo that would be easy to follow.

A real application using more than one or two such buttons should consider a better organization of the code. For instance maybe keeping the ENUMs in an array of structs that contain all necessary data and making a subroutine or subroutines that given the cbBtnCommon input data of eTouch and pElem->nId would lookup the button ENUM and given the etouch state start/stop timing and return some indication of what needs doing or maybe even store a function pointer in the struct to call on certain event. Something to think about.

If you ever go that route just be sure and place your new subroutine above the Builder's Button tag and keep empty case statements for your continuous buttons so the builder can still work correctly for your other buttons and allow you to add or delete buttons from the builder.

Paul--

GeorgeIoak commented 2 years ago

@Pconti31 Thanks again for hanging in there. Yes, back in the limited display days you needed long presses to bring up menus but with touch screens and larger displays that allow pages I don't think that's typical anymore but that's besides the point. When I searched for examples and discussions on long button presses I came across that example code that I initially posted. I didn't have much luck finding further descriptions on the events and how they occur so I assumed that GSLC_TOUCH_MOVE_IN was tied to a long press. I believe that GSLC_TOUCH_MOVE_IN events only happen when the touch is inside the element and changes position but stays inside the element. How that example code actually works if you don't move your finger I cannot understand which led to my later question of whether the resistive touch screen events were different that capacitive. I stopped last night because expanding my code to reacting to either increase or decrease led to an error in the count that I just couldn't figure out. The decrease long press worked as planned but the increase would always jump to -76 and after that it would go to -66 but then back to -76. No matter what I changed that always happened. It would correctly enter the main loop and the count would be correct but somehow it skipped any logic block , changed count to -76, and left the main loop and entered the button callback again. There's something going on behind the scenes with the call backs (I assume) that I just couldn't figure out. I probably should close this since my original assumption was incorrect and this would probably be better as a discussion but I'd like to get to the bottom of this as I have to imagine that I'm not the only person who would like a long button press "event".

Pconti31 commented 2 years ago

@GeorgeIoak @ImpulseAdventure With all of the changes Calvin has made to better support hardware buttons the code looks and behaves differently from when I first wrote my Long press example. I haven't really tested it to see if its different or just never worked as I expected. I've been busy working on my own project and answering you in my free time.

So basically you have gone beyond what I can answer without me digging through his code again.

If you are still having a problem with you FT6206 touch driver I would again suggest trying my edits to adafruit ft6205 lib I posted to issue 405 Paul_V2_Adafruit_FT6206.zip. It might not be any better but I think it would be worth trying.

Otherwise no point in my confusing things further. You will either need to dig deeper into his code or wait for him to reply. I would suggest you post a a recap of your questions that you want Calvin to answer and wait for a reply and not post further to this as wading through all of the messages likely will mean your actual issues will get lost. Paul--

GeorgeIoak commented 2 years ago

@Pconti31 I had a chance to review the driver changes you made back in issue 405. I can see how eliminating the multi-touch support would improve the responsiveness on the touch. I looked at your modified ILI9488 but I wasn't sure how I would include that in my project since mine started with TFT-eSPI and I'd have to go back and see if Bodmer had made any changes.

I did solve my -76 odd error. Turns out I had defined a maximum count of 180 but assigned as int8_t. Once I changed that to int16_t everything went back to working as expected and responding to either the increase or decrease button.

Now, as you mentioned above, this really should be turned into a proper handler. Perhaps when Calvin gets back he might have some ideas and/or interest in this.

It's been a bumpy road but at least right now I think I have something that I can do some original work with to move forward and then maybe later get some help with refining the actual code.

Thanks again for your time and patience, it truly is appreciated.

ImpulseAdventure commented 2 years ago

Hey @GeorgeIoak --

Thanks for the patience on this due to delays from my reno, vacation and work.

Back to your original problem statement, your goal appears to be the detection of a long-press event while the press is active. We already have support for long-press events, but these were intended to capture the event after the press has been released. The distinction is important because of the way the events are triggered in the API.

In order to ensure the GUI is responsive and not wasting cycles, touch events are not generated if the touch press does not alter its positional coordinate. In other words, no TOUCH_MOVE events are typically generated for a sustained press. You can see this in the TrackTouch() function. However, the Tick callback is triggered irrespective of any touch movements.

If we want to support long-press detection whilst held down, then I think the easiest way might be to:

I haven't tested the above but anticipate that it may enable someone closer to what you're after.

Note that there could be a secondary issue with how the FT6206 is reporting the coordinates for the touch-up events, but I'd like to test with my setup first to see if your hardware is operating the same way.

thanks!

Pconti31 commented 2 years ago

@GeorgeIoak @ImpulseAdventure In case you didn't understand how to actually implement the tick function I put together a working demonstration of a continuous button press updating a Progress bar and displaying STOPPED or RUNNING text message while you are touching a button.

The key is line 143 inside ContinuePress-Btn_GSLC.h during creation of a Box Element. You don't really need the Box but its the only way to get the Builder to allow you to set a tick callback.

  gslc_ElemSetTickFunc(&m_gui,pElemRef,&CbTickScanner);

ContinuePress-Btn.zip Paul--

GeorgeIoak commented 2 years ago

@Pconti31 Thanks for putting an example together. I got pulled aside on another project so I haven't been able to spend more time on this. Hopefully I'll get back to it soon otherwise I stand a good chance of forgetting everything and starting all over!