Add support for CST716/CST718S touch sensor used in P8 #1130
Conversation
StarGate01
force-pushed
the
p8-touch
branch
5 times, most recently
from
43e5b5a to
dff2747
Compare
StarGate01
force-pushed
the
p8-touch
branch
5 times, most recently
from
ccbf78c to
a3f1b93
Compare
src/touchhandler/TouchHandler.cpp
Outdated
| void TouchHandler::UpdateLvglTouchPoint() { | ||
| if (info.touching) { | ||
| #if defined(DRIVER_TOUCH_GESTURE) | ||
| // GESTURE config only generates a single event / state change | ||
| // so the LVGL wrapper is used to generate a successive release state update | ||
| lvgl.SetNewTap(info.x, info.y); | ||
| #else | ||
| if (!isCancelled) { | ||
| lvgl.SetNewTouchPoint(info.x, info.y, true); | ||
| } | ||
| #endif |
There was a problem hiding this comment.
While I don't have these devices to test, I think this function may be able to be simplified further. On the PineTime, a swipe immediately switches the screen and isCancelled is used to ignore the rest of the touch until it has been released. On the other devices the swipe event only occurs on release if I understand correctly, so parts of this could be removed.
Having preprocessor in the middle of a function isn't very nice, so depending on how different the drivers end up being, we can see if it is worth creating separate files for them or create new functions that are only included in certain builds.
There was a problem hiding this comment.
depending on how different the drivers end up being,
This PR covers all three behaviors of the CST71X chips. The extent of differences between the behaviors wont change (as long as the hardware does not), so the drivers won't diverge further than they did in this PR.
I decided against splitting the drivers into multiple files / classes, because that would have lead to a high amount of code duplication. The only pre-processor macros added are in TouchHandler.cpp, at https://github.com/InfiniTimeOrg/InfiniTime/blob/a3f1b93df25d80031baeb2d094497c2e915ad5d6/src/touchhandler/TouchHandler.cpp#L73= to select how events should be interpreted, and on https://github.com/InfiniTimeOrg/InfiniTime/blob/a3f1b93df25d80031baeb2d094497c2e915ad5d6/src/touchhandler/TouchHandler.cpp#L110= which you already commented on.
I think you are correct in that GESTURE drivers don't use isCanceled at all, since an event is only dispatched at gesture release. I am not quite sure how we could improve the code though.
There was a problem hiding this comment.
I'm not sure it would lead to a lot of code duplication. Firstly ConvertGesture() could be moved to Cst816s.cpp for example. Then there's not that much code left in the first place and while there are only two pre-processor macros, they end up touching a large portion of the code. It would also be easier to find optimizations if they were separated and freely let them diverge. For example removing isCancelled from GESTURE and perhaps REPORT if possible.
If the end of UpdateLvglTouchPoints(), which is responsible for the release event, doesn't do anything on GESTURE, the entire function could be replaced with a simpler one for better visibility.
There was a problem hiding this comment.
TouchHandler received some changes recently. TouchHandler should probably be a part of the driver, since it's just processing the touch screen data into an InfiniTime format, but that's not necessarily in the scope of this PR. This PR should be rebased, all unnecessary changes should be removed, and preprocessor shouldn't modify the behavior of functions, but can be used to pick which function to call. Then we may be able to merge this.
This function sends a tap down and tap up event to LVGL, despite being called only once.
The P8 smartwatches use the CST716 or CST816S chips in various modes. The device ID of these chips cannot be used for runtime detection, because it does not give the hardware revision / firmware configuration.
|
Build checks have not completed. Possible reasons for this are:
|
|
hmm, not ready yet (?)
|
This PR has been broken out of #1050, and depends on / includes #1128 .
DRIVER_TOUCH.