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

SPI driver for XPT2046 #400

Open
Elara6331 opened this issue Apr 12, 2022 · 5 comments
Open

SPI driver for XPT2046 #400

Elara6331 opened this issue Apr 12, 2022 · 5 comments
Labels
enhancement New feature or request

Comments

@Elara6331
Copy link
Contributor

The XPT2046 touch controller supports SPI. The only driver in TinyGo is without SPI. Both implementations should exist (maybe add a function NewSPI() to use the SPI implementation). I would be happy to do this myself, but I have never written a driver for any component and haven't done very much embedded programming in general, so I don't know how. If someone can to point me in the right direction to do this or make it themselves, I would appreciate it.

@deadprogram deadprogram added the enhancement New feature or request label Apr 12, 2022
@gregoster
Copy link

Please have a look at: https://github.com/gregoster/tinygo-drivers/tree/xpt2046spi/xpt2046
There's still some extra design work needed to make both the GPIO and SPI versions co-exist in the same file (e.g. right now there it little error checking to make sure that SPI bits don't get called in GPIO mode and vice-versa).
I built this driver to get a Waveshare Pico-ResTouch-LCD-2.8 work with both the st7789 and xpt2046 drivers at the same time, as t_clk, t_din, and t_dout end up being 'shared' between the GPIO and SPI, which didn't work for me...
My repo also contains an SPI-only version of the driver (which is what I developed/used for testing). I havn't tested the GPIO-only bits of this driver, so that would also need to be done.
This is my first venture into writing new hardware device drivers, and I must say I learned a fair bit (and had a lot of fun along the way). Feedback/comments definitely welcome.

@conejoninja
Copy link
Member

Hi @gregoster nice work!
It would be great it you could make a PR from it, so we could properly review it and occasionally merge it.

A few general things I saw at first glance:

  • Exported funcs / vars need to be documented. Not required for un-exported, but highly recommended too.
  • Please add an example for each (spi/gpio) in the examples/xpt2046 folder (you could re-use existing one)

@gregoster
Copy link

I found a few cycles this evening to improve things. Cleaned up the code a bunch, and hopefully have removed any 'sharing' issues between GPIO and SPI interfaces. Also added a whole bunch of comments. I also added an example using SPI: https://github.com/gregoster/tinygo-drivers/blob/xpt2046spi/examples/xpt2046/mainspi.go If things are looking reasonable enough I'll next see about getting a PR created. Thanks!

@conejoninja
Copy link
Member

Hello,

Please, put each example inside a folder like

examples/xpt2046/gpio/main.go
examples/xpt2046/spi/main.go

Please, make a PR (to the dev branch), so we can comment and create a proper review of your work, don't worry if you need to make several fixes to your initial PR, that's completely normal.

@gregoster
Copy link

gregoster commented Oct 27, 2022

Examples moved to folders (should have though of that one myself, but I'm still a beginner when it comes to go and file layouts). PR submitted: #477 Thanks for helping walk me through things... lots to learn about this process.

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

No branches or pull requests

4 participants