New touch handler #492

Riksu9000 · 2021-07-16T09:34:11Z

Greatly improved responsiveness. Demonstration in Improving touch event handling could improve responsiveness greatly. #471.
TouchModes are no longer needed.
LVGL is now correctly told about the touch positions and times. Dragging, holding and long pressing etc. should now always work correctly. LVGL isn't aware if apps or screens have changed, so CancelTap() is used to stop the current touch from activating buttons.
Cleaned up touchdriver code. Events (0 = Down, 1 = Up, 2 = contact) seem inconsistent, so they're replaced with a simple boolean touching.
For a single touch, only one swipe gesture can be generated. This fixes double gesture in Twos and when swiping up during the animation going from QuickSettings to watch face.

This code probably requires further stress testing to make sure it always works correctly.

Implements #471 and fixes #491.

jonvmey · 2021-07-16T15:29:12Z

src/touchhandler/TouchHandler.cpp

+  auto returnGesture = gesture;
+  gesture = Drivers::Cst816S::Gestures::None;
+  return returnGesture;


This could be simplified as return std::exchange(gesture, Drivers::Cst816S::Gestures::None);

It could be argued whether this is simpler or not. Anyone can read the current code, but I must not be the only one who's never heard of std::exchange() before. I can change it though if this is important to comply with the "Modern C++" requirement.

Up to you ultimately, just a suggestion. I just like to mention things like these because as you said yourself there's lots of useful things in the standard library that many people aren't familiar with. I personally think that std::exchange is nicer/simpler than the equivalent code to do it manually but YMMV.

Riksu9000 · 2021-07-17T23:17:51Z

For some reason the watch freezes if we swipe the screen multiple times while the watch is sleeping.

Riksu9000 · 2021-07-18T09:22:45Z

Removing every twiMaster.Sleep() after Init seems to fix the issue. What should we do about this? It's not great that currently it's woken up and put to sleep in many places, making the real state hard to keep track of.

JF002 · 2021-07-18T19:14:23Z

I haven't look at the code, but it looks like you are improving the touch driver a lot!

Regarding your last comment

Removing every twiMaster.Sleep() after Init seems to fix the issue. What should we do about this? It's not great that currently it's woken up and put to sleep in many places, making the real state hard to keep track of.

Originally, the goal was to handle sleep/wakeup at one place. It was easy when all devices were put to sleep when the watch is going to sleep.
The motion sensor made this a bit more complicated because it's being polled even when the watch is asleep (needed for wake on wrist rotation feature).
Another call to twimaster.Sleep() is needed when a tap is detected on the touch panel : we have to check if it's a single/double tap and wake the watch if it's configured to wake on single/double tap.

Which calls to twimaster.Sleep did you remove? Is it this one https://github.com/JF002/InfiniTime/blob/develop/src/systemtask/SystemTask.cpp#L129 ?

Riksu9000 · 2021-07-18T20:00:54Z

Which calls to twimaster.Sleep did you remove? Is it this one https://github.com/JF002/InfiniTime/blob/develop/src/systemtask/SystemTask.cpp#L129 ?

No. That's the only one i left. I just removed the three other ones in SystemTask.cpp. I don't know if just one of them was causing the problem.

I was thinking maybe twiMaster could just be woken up in the Read() and Write() functions, so there wouldn't be any need to keep track of the state. After my first experiment, I got a bootloop, so I don't really wan't to mess with it any more right now since I don't have a devkit. But does this sound like a good idea?

geekbozu · 2021-07-21T00:45:54Z

This seems to queue the last gesture given.
Turn off screen -> Swipe down -> turn on screen -> Jumps to notifications.
Otherwise much nicer input feels much more natural and consistent

kieranc · 2021-07-23T12:30:18Z

I've been testing this today and it's certainly a lot faster than the old setup, but I've had a number of false inputs, taps interpreted as swipes and the inverse, also the watch locked up and rebooted after a game of 2048. I wonder if it needs some sort of debouncing or filtering, IE if something is interpreted as a swipe but moved only 10px, it's probably meant to be a tap...
Double tap to wake seems less reliable too.

Riksu9000 · 2021-07-23T18:36:47Z

I've been testing this today and it's certainly a lot faster than the old setup, but I've had a number of false inputs, taps interpreted as swipes and the inverse, also the watch locked up and rebooted after a game of 2048. I wonder if it needs some sort of debouncing or filtering, IE if something is interpreted as a swipe but moved only 10px, it's probably meant to be a tap...
Double tap to wake seems less reliable too.

I remember it working quite well for me when I was testing it. I guess I'll have to test it some more.

kieranc · 2021-07-23T20:13:35Z

I remember it working quite well for me when I was testing it. I guess I'll have to test it some more.
I'll test it again tomorrow, it could have been something I did.

kieranc · 2021-07-24T12:46:25Z

I remember it working quite well for me when I was testing it. I guess I'll have to test it some more.
I'll test it again tomorrow, it could have been something I did.

I'm testing it again with the ui/settings update and it seems to be behaving fine. It might have been that my screen was dirty!

kieranc · 2021-07-25T16:37:26Z

I'm testing it again with the ui/settings update and it seems to be behaving fine. It might have been that my screen was dirty!

Further testing makes me think double tap to wake is not working correctly still, I have this PR plus UI/settings updates applied and double tap to wake is very unreliable.

I'm going to test with just this PR, and with a stock build, but could you see if double tap to wake works correctly for you? It seems like it works fine at first but after a few hours it will not wake by double tap.

Riksu9000 · 2021-07-26T08:35:08Z

I was testing the double tap to wake yesterday, and even after leaving the watch on a table overnight, the double tap worked first try in the morning. Maybe you're experiencing something related to the freezing issue, or the queueing issue I haven't fixed yet.

I did find one issue though, which is probably what you were experiencing as well. This is what I wrote in #471:

When we press the screen outside of a button, slide our finger on top of a button and release, a click event is sent. This can even be seen in this online LVGL example.

If we do a very short and quick swipe in the app menu for example, LVGL doesn't care that we moved our finger, it sends a click event anyway where the screen was last touched. CancelTap() is used to combat this, but the swipe gesture arrives too late, and LVGL has already sent the event. I wish there was a way to disable this behaviour in LVGL, but we must work around it for now.

EDIT: This is hard to test, so I'm not sure if this is actually an issue, or if it's technically working correctly. Still this wouldn't happen if this quirk in LVGL didn't exist.

kieranc · 2021-07-26T10:20:19Z

I was testing the double tap to wake yesterday, and even after leaving the watch on a table overnight, the double tap worked first try in the morning. Maybe you're experiencing something related to the freezing issue, or the queueing issue I haven't fixed yet.

I tested overnight with a stock build and double tap worked reliably. I've just flashed a build with only this PR applied, I should know if it's reliable by the end of the day. I can't see how the UI updates PRs could have caused this behaviour, but we will see.

kieranc · 2021-07-26T18:41:37Z

Double tap wake is still being unreliable for me with this PR I'm afraid. I think we need someone else to test it.

JF002 · 2021-08-18T19:00:03Z

Great :+1 Thanks!

So, in the end, if we merge #497, this PR is not WIP anymore, right?

Riksu9000 · 2021-08-18T19:15:12Z

So, in the end, if we merge #497, this PR is not WIP anymore, right?

I just tested it with both PRs and it seems to work correctly as far as I can tell.

JF002 · 2021-08-18T19:31:15Z

Awesome! I added #568, #492 and #497 to 1.4, and I'll test/review/merge them soon :)

kieranc · 2021-08-19T14:12:01Z

Awesome! I added #568, #492 and #497 to 1.4, and I'll test/review/merge them soon :)

DFU with #492 #497 and #568
pinetime-dfu-1.4.0-rc1-492-497-568.zip

DFU with the above plus #458 #480 and #586
pinetime-dfu-1.4.0-rc1-492-497-568-458-480-586.zip

JF002 · 2021-08-28T13:05:53Z

LGTM! 👍

hubmartin · 2021-09-11T16:06:44Z

Hi, I tried to merge this to my P8 and watches react just to the first click/gesture. I'm sure there is different touch controller cst716 in the P8. I'm in the beginings of debugging it. If you have any idea what to check and touch datasheet with registers, that would help me a lot.

it seems like a new configuration is sent to the driver, if I set 0x00 to this register it behaved the same way - only first touch gesture is detected after boot.

  static constexpr uint8_t irqCtl = 0b01110000;
  twiMaster.Write(twiAddress, 0xFA, &irqCtl, 1);

Then the GetTouchInfo is different, I didn't get further yet. However when debugging, every touch is detected in the GetTouchInfo function and I see proper X and Y coordinates. Also not sure if the issue could be around isCancelled.

Thank you.

Riksu9000 · 2021-09-11T16:28:00Z

https://smarterwat.ch/soc-list/nordic-semiconductor/p8.html

Alternatively, swipe behaviour can determine which you have. If you swipe and keep your finger on the display, the CST716 will not register the swipe until after your finger has been removed, whereas the CST816S will register the swipe before your finger has been removed.

Because of the difference in behaviour, info.touching will be false and the gesture won't be registered. The code would need to be tweaked slightly for the different controller.

      if (info.gesture == Pinetime::Drivers::Cst816S::Gestures::SlideDown ||
          info.gesture == Pinetime::Drivers::Cst816S::Gestures::SlideLeft ||
          info.gesture == Pinetime::Drivers::Cst816S::Gestures::SlideUp ||
          info.gesture == Pinetime::Drivers::Cst816S::Gestures::SlideRight ||
          info.gesture == Pinetime::Drivers::Cst816S::Gestures::LongPress) {
        if (info.touching) {
          gesture = info.gesture;
          gestureReleased = false;
        }

To see which version you have on the stock firmware, you can look at the about page. There will be a string that looks like 00 B4 00 02. Ending in 01 means you have the CST816S, and ending in 02 means you have the CST716S.

This value could be used to detect the controller and adjust the touch handler accordingly. I've added reading this value in #489.

hubmartin · 2021-09-11T16:49:28Z

Thanks for tips.
After reboot on the digital watchface I could click and every time the touch point seems to be registered and lvgl.SetNewTouchPoint(info.x, info.y, true); is called and coordinates seems to be right.
But since on digital watchface there is nothing to click, I have to swipe.
After swipe the isCancelled is true and calls to lvgl.SetNewTouchPoint are not passed further because of this condition:

And I'm not sure what this variable exactly does yet.

I also did this hack and it seems that I could swipe many times now. However I still have issues with touch because isCancelled is false,

Riksu9000 · 2021-09-11T17:21:30Z

CancelTap() is also related to this swipe behaviour in Cst816. If we swipe, the screen will switch while we're still touching the screen, and when we finally release, it may activate a button on the new screen. CancelTap() and isCancelled set the touch point to somewhere outside the screen, so when we release, it won't activate anything. So in practice, the touch has been cancelled and won't activate anything in LVGL.

Releasing a touch generates an interrupt because of "EnChange" in irqCtl register, so UpdateLvglTouchPoints() gets run with info.touching being false, which should set isCancelled to false. This is also required for LVGL to detect release.

        case Messages::OnTouchEvent:  //handle release interrupt
          if (touchHandler.GetNewTouchInfo()) {  //info.touching = false
            touchHandler.UpdateLvglTouchPoint();  //isCancelled = false
          }

hubmartin · 2021-09-11T18:11:43Z

Do you have a proper datasheet with registers and bits explained? Thanks

Riksu9000 · 2021-09-11T18:22:21Z

The PineTime wiki has datasheets for the components
https://wiki.pine64.org/index.php/PineTime
https://wiki.pine64.org/images/2/2f/CST816S.zip

This one has the registers.
CST816S寄存器说明-20190508.zh-CN.en.pdf

ildar · 2021-09-28T11:14:56Z

It's a pity P8 users got stuck with 0.13 because of this pull.

This value could be used to detect the controller and adjust the touch handler accordingly. I've added reading this value in #489.

Tried that but it doesn't help much only displaying a nice error message.

The touch driver now automatically detects whether a CST816s (pinetime) or CST716 (some p8 watches) chip is used, and adjusts the decoding logic accordingly. For more information, see the discussion on InfiniTimeOrg#492

StarGate01 · 2022-03-21T14:11:00Z

I added support for the CST716 back in my fork, see StarGate01@6ecb9a1 . For more info, see #62 (comment) . It rarely detects a swipe as a touch, apart from that it works great.

The touch driver now automatically detects whether a CST816s (pinetime) or CST716 (some p8 watches) chip is used, and adjusts the decoding logic accordingly. For more information, see the discussion on InfiniTimeOrg#492

Riksu9000 added 6 commits July 15, 2021 14:11

New touch handler, with issues

fe64176

Quick fix

0d24d2b

Fix most issues

2a3e126

Fix touch wakeup and code cleanup

93ccbf3

Fix gestures

baffa15

Fix remaining known issues

329482f

jonvmey reviewed Jul 16, 2021

View reviewed changes

Update

3e42297

Riksu9000 mentioned this pull request Jul 18, 2021

Bootloop after a manual reset while wearing the watch #317

Closed

Restore Tile.cpp changes

dc2b5f0

Riksu9000 mentioned this pull request Jul 20, 2021

[POC] Feature Favorite Apps on swipe left #487

Closed

Riksu9000 mentioned this pull request Jul 21, 2021

Fit more tasks in SystemInfo #507

Merged

Merge branch 'develop' into new_touch_handler

d666c50

Merge branch 'develop' into new_touch_handler

e4d9636

JF002 mentioned this pull request Jul 25, 2021

Update touchpad driver and automatically detect errors #489

Merged

Merge branch 'develop' into new_touch_handler

a1ba187

Riksu9000 mentioned this pull request Jul 31, 2021

Fix LVGL touch #541

Closed

Merge branch 'develop' into new_touch_handler

85c9979

Riksu9000 mentioned this pull request Aug 26, 2021

Upgrade to LVGL 8 #616

Open

JF002 merged commit 969de9a into InfiniTimeOrg:develop Aug 28, 2021

This was referenced Aug 28, 2021

Improve setting the timer value #390

Closed

Timer v2 #407

Closed

Riksu9000 changed the title ~~WIP New touch handler~~ New touch handler Aug 28, 2021

Riksu9000 deleted the new_touch_handler branch August 28, 2021 15:24

Riksu9000 mentioned this pull request Aug 28, 2021

Swiping left/right in music app skips a bunch of tracks instead of one #519

Closed

This was referenced Sep 12, 2021

Screen Timeout handling issue #398

Closed

Disable sleep while in Paddle app #553

Closed

ildar mentioned this pull request Sep 29, 2021

P8 support #62

Open

Riksu9000 mentioned this pull request Oct 23, 2021

Touch controller error warning on PineTime #763

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New touch handler #492

New touch handler #492

Riksu9000 commented Jul 16, 2021 •

edited

Loading

jonvmey Jul 16, 2021

Riksu9000 Jul 16, 2021

jonvmey Jul 16, 2021

Riksu9000 commented Jul 17, 2021

Riksu9000 commented Jul 18, 2021

JF002 commented Jul 18, 2021

Riksu9000 commented Jul 18, 2021

geekbozu commented Jul 21, 2021

kieranc commented Jul 23, 2021

Riksu9000 commented Jul 23, 2021

kieranc commented Jul 23, 2021

kieranc commented Jul 24, 2021

kieranc commented Jul 25, 2021

Riksu9000 commented Jul 26, 2021 •

edited

Loading

kieranc commented Jul 26, 2021

kieranc commented Jul 26, 2021

JF002 commented Aug 18, 2021

Riksu9000 commented Aug 18, 2021

JF002 commented Aug 18, 2021

kieranc commented Aug 19, 2021

JF002 commented Aug 28, 2021

hubmartin commented Sep 11, 2021 •

edited

Loading

Riksu9000 commented Sep 11, 2021

hubmartin commented Sep 11, 2021

Riksu9000 commented Sep 11, 2021 •

edited

Loading

hubmartin commented Sep 11, 2021 via email •

edited

Loading

Riksu9000 commented Sep 11, 2021

ildar commented Sep 28, 2021 via email •

edited

Loading

StarGate01 commented Mar 21, 2022

New touch handler #492

New touch handler #492

Conversation

Riksu9000 commented Jul 16, 2021 • edited Loading

jonvmey Jul 16, 2021

Choose a reason for hiding this comment

Riksu9000 Jul 16, 2021

Choose a reason for hiding this comment

jonvmey Jul 16, 2021

Choose a reason for hiding this comment

Riksu9000 commented Jul 17, 2021

Riksu9000 commented Jul 18, 2021

JF002 commented Jul 18, 2021

Riksu9000 commented Jul 18, 2021

geekbozu commented Jul 21, 2021

kieranc commented Jul 23, 2021

Riksu9000 commented Jul 23, 2021

kieranc commented Jul 23, 2021

kieranc commented Jul 24, 2021

kieranc commented Jul 25, 2021

Riksu9000 commented Jul 26, 2021 • edited Loading

kieranc commented Jul 26, 2021

kieranc commented Jul 26, 2021

JF002 commented Aug 18, 2021

Riksu9000 commented Aug 18, 2021

JF002 commented Aug 18, 2021

kieranc commented Aug 19, 2021

JF002 commented Aug 28, 2021

hubmartin commented Sep 11, 2021 • edited Loading

Riksu9000 commented Sep 11, 2021

hubmartin commented Sep 11, 2021

Riksu9000 commented Sep 11, 2021 • edited Loading

hubmartin commented Sep 11, 2021 via email • edited Loading

Riksu9000 commented Sep 11, 2021

ildar commented Sep 28, 2021 via email • edited Loading

StarGate01 commented Mar 21, 2022

Riksu9000 commented Jul 16, 2021 •

edited

Loading

Riksu9000 commented Jul 26, 2021 •

edited

Loading

hubmartin commented Sep 11, 2021 •

edited

Loading

Riksu9000 commented Sep 11, 2021 •

edited

Loading

hubmartin commented Sep 11, 2021 via email •

edited

Loading

ildar commented Sep 28, 2021 via email •

edited

Loading