Skip to content

Add data validity check and retries in CST816S driver#811

Merged
JF002 merged 2 commits intodevelopfrom
cst816-add-validity-check
Nov 8, 2021
Merged

Add data validity check and retries in CST816S driver#811
JF002 merged 2 commits intodevelopfrom
cst816-add-validity-check

Conversation

@JF002
Copy link
Collaborator

@JF002 JF002 commented Nov 6, 2021

I noticed that I couldn't get 100% reliable readings from the CST816S (touch controller) (see #763 (comment)). I added a few checks in Cst816S::GetTouchInfo to ensure it only returns is data are valid. I also added a retry functionality for the Check ID procedure. It'll hopefully reduce the number of false positive when checking the device IDs.

This PR might fix #763, but I think we still have other issues with I²C (see #763 (comment)).

Anyway, it would be nice if some people could test that PR and ensure it does not show the warning about an incompatible touch controller. @Arsen6331 @geekbozu @kieranc I would be really interested to know if this PR fixes that issue.

@Elara6331
Copy link
Contributor

Elara6331 commented Nov 6, 2021

I just installed this PR. I don't seem to be getting the warning anymore. I have restarted my watch multiple times and haven't gotten it once, so it seems to be fixed so far.

@kieranc
Copy link
Contributor

kieranc commented Nov 7, 2021

I've just done 10+ test cycles with this PR - reboot watch, play a full game of twos, return to menu, reboot, repeat.
No touch controller errors on boot and no crashes. The first 5 cycles I ran with the watch with a back on, taped to the charger, after that I removed the tape and back, put it in a non-conductive stand and only touched the screen. There was one point when it was on the stand when the touch screen seemed to stop responding until I touched the metal bezel, then it went back to working normally, but I couldn't repeat this.

@JF002
Copy link
Collaborator Author

JF002 commented Nov 7, 2021

Thanks for the test @kieranc ! I'm wondering if the presence of the SWD debugger could make the i²c communication less reliable?

EDIT : I've just tested one devkit without SWD connection : no crash anymore 0_o

@JF002 JF002 added this to the 1.7.0 milestone Nov 7, 2021
@kieranc
Copy link
Contributor

kieranc commented Nov 8, 2021

Thanks for the test @kieranc ! I'm wondering if the presence of the SWD debugger could make the i²c communication less reliable?

EDIT : I've just tested one devkit without SWD connection : no crash anymore 0_o

It looks that way but I can't figure out the how/why..
My dev watch (RIP) was intermittently unreliable after 1.4/new touch handler, at one point reseating the touch cable made it work for a few days but then it reverted to previous behaviour (I could swipe but not tap), at other times holding the device or touching the screen with a metallic object seemed to make it work temporarily. It was definitely after I soldered the SWD cables that it started breaking but I kind of assumed I broke something during soldering.
I tried removing the programmer from the soldered cables which had no impact but didn't try removing the cables themselves. It feels to me like some sort of grounding/charge buildup thing but it's difficult to replicate reliably so I don't have a lot more to go on than feelings.
All the references I've seen to this touch controller make reference to it responding via I2C only after a touch event, does our driver implement this? I'm not sure I'm understanding it fully, either it's a case of boot -> init -> no i2c -> touch event -> normal i2c, or, i2c only works correctly immediately after a touch event, and will not work right during a touch event (which is kind of what we're seeing? and kind of aligns with swipe but not tap working?)
Apologies for the wall of text, I'm fairly sure I've been running into this bug for a while, maybe my rambling will spark something useful :p

refs:
https://doc.riot-os.org/group__drivers__cst816s.html
https://wiki.pine64.org/wiki/PineTime#Touch_panel
https://github.com/fbiego/CST816S

@JF002 JF002 merged commit 755ab72 into develop Nov 8, 2021
@JF002 JF002 deleted the cst816-add-validity-check branch November 9, 2021 21:25
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.

Touch controller error warning on PineTime

4 participants