InfiniTimeOrg / InfiniTime

Firmware for Pinetime smartwatch written in C++ and based on FreeRTOS
GNU General Public License v3.0
2.71k stars 925 forks source link

touchhandler: custom gesture detection #2003

Open KaffeinatedKat opened 7 months ago

KaffeinatedKat commented 7 months ago

This PR implements InfiniTime's own code for detecting gestures within the TouchHandler, instead of using the gesture detection built-in to the Cst816S. The only gesture that hasn't been re-implemented is the DoubleTap because the built in DoubleTap detection works just fine as is.

This PR allows us to easily re-configure the gestures. Swiping works much more consistently now due to a lower distance required to be detected. The LongTap gesture is also greatly improved here, with a much lower hold duration, making it activate much more consistently.

I also plan to implement smooth scrolling in places like the app drawer and the settings screen. Re-implementing the gesture system is required for that, and it will depend on this PR

github-actions[bot] commented 7 months ago
Build size and comparison to main: Section Size Difference
text 373224B 116B
data 940B 0B
bss 63532B 16B
mark9064 commented 7 months ago

Seems to work nicely. Short and fast flicks don't get registered as swipes easily though. Maybe velocity could be taken into account? Example: Try scrolling the settings menus with fast flicks. Most of the time it registers it as a tap and you end up inside a settings page rather than still being on the list

KaffeinatedKat commented 7 months ago

not a bad idea to take velocity into account. I'll continue to play with this to make it easier to use. Going to make this PR a draft until I can implement everything better than what we have right now. Going to also re-implement the DoubleTap gesture and then we can just completely remove the Cst816S gestures from the driver code.

Another benefit of having the TouchHandler do all of the gestures is that it makes it much easier to add more watch support. We won't have worry about the gesture system of the touch controller, and converting those into the TouchHandler events, because the TouchHandler is what is handling the gestures regardless of what the hardware provides.

A touch controller will simply need to provide x and y coordinates of a touch location, and a "isTouching" state (basic touch controller functionality). This will greatly improve the maintainability when InfiniTime eventually supports a wider range of watch models.

mark9064 commented 7 months ago

Oh also another thought. This also breaks tap to wake right? As the gesture events are generated even when the touch panel is in sleep mode, but if we do gestures in software we lose that (and would have to keep the touch panel powered 24/7 like we do right now for double tap to wake)

KaffeinatedKat commented 7 months ago

this is true, I didn't think about that. A simple fix is to keep the Cst816S Tap gesture in the TouchHandler to keep the battery life benefits of the display being asleep

JF002 commented 7 months ago

What would be the added value of re-implementing in software those gestures when the hardware does it automatically for us, in a probably more energy efficient way?

KaffeinatedKat commented 7 months ago

the main reason is that the hardware LongTap is just annoyingly inconsistent, I hate using it. It takes too long to activate and sometimes it just doesn't activate multiple times in a row. I went ahead and re-implemented everything else while I was at it, but other than the LongTap the hardware gestures work fine.

I could always change this PR to just re-implement the long tap, and leave the rest of the gestures as is if that's better

JF002 commented 7 months ago

I wasn't aware we had issues with long taps. Could you give more detail about those inconsistencies?

KaffeinatedKat commented 7 months ago

the LongTap action takes quite a while to activate, and I feel this hold duration is too long. Sometimes the LongTap action just never triggers, despite holding for a long time, and because of how long it takes it takes me a second to realize that it's not going to trigger.

The long duration also makes it not really useful outside of pulling up watchface settings. With a lower duration it could easily be used as an option to clear all the notifications in the notification screen (https://github.com/InfiniTimeOrg/InfiniTime/pull/2000#issuecomment-1925449522). The long duration makes it impractical for this, as it's not much faster than just manually dismissing them all

pipe01 commented 5 months ago

Swiping is quite inconsistent for me right now, so being able to customize it a bit would be great.