Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

touchhandler: custom gesture detection #2003

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

KaffeinatedKat
Copy link
Contributor

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

Copy link

github-actions bot commented Feb 4, 2024

Build size and comparison to main:

Section Size Difference
text 373224B 116B
data 940B 0B
bss 63532B 16B

@mark9064
Copy link
Contributor

mark9064 commented Feb 9, 2024

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
Copy link
Contributor Author

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.

@KaffeinatedKat KaffeinatedKat marked this pull request as draft February 9, 2024 21:07
@mark9064
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Collaborator

JF002 commented Feb 11, 2024

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
Copy link
Contributor Author

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
Copy link
Collaborator

JF002 commented Feb 11, 2024

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

@KaffeinatedKat
Copy link
Contributor Author

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 (#2000 (comment)). The long duration makes it impractical for this, as it's not much faster than just manually dismissing them all

@pipe01
Copy link
Contributor

pipe01 commented Apr 23, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants