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

XPT2046 Touch driver #350

Merged
merged 1 commit into from
Dec 14, 2021
Merged

XPT2046 Touch driver #350

merged 1 commit into from
Dec 14, 2021

Conversation

spearson78
Copy link
Contributor

No description provided.

@sago35
Copy link
Member

sago35 commented Dec 11, 2021

Thank you for your contribution.
Please add Examples.

@sago35
Copy link
Member

sago35 commented Dec 11, 2021

I would like you to implement the following interface.

type Pointer interface {
ReadTouchPoint() Point
}

@spearson78
Copy link
Contributor Author

I've added an example and migrated to the ReadTouchPoint interface.

I removed the internal scaling to display size to be consistent with the fourwire implementation however I currently return 12 bit values. Should I scale them to 16 bit to match fourwire?

Copy link
Member

@sago35 sago35 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update.
For the most part I think it is very nice.

Please add it to README.md.
Also, please add smoketest to the Makefile.

xpt2046/xpt2046.go Outdated Show resolved Hide resolved
xpt2046/xpt2046.go Show resolved Hide resolved
xpt2046/xpt2046.go Outdated Show resolved Hide resolved
xpt2046/xpt2046.go Show resolved Hide resolved
@spearson78
Copy link
Contributor Author

I've added a default value of 10 for precision if the user sets it to 0 and I now scale the values to 16 bits
I moved the precision to a new Config struct. Should I move the entire pin config to the Configure method?
I updated the Readme and make smoke-test targets.

I'd like to say thank you for guiding me through the requirements for contributions to this project. This is my first commit to tinygo and its been very enjoyable so far :)

@sago35
Copy link
Member

sago35 commented Dec 14, 2021

It seems that there are few cases where the config struct contains pin information.
I think the current source code is fine.

Copy link
Member

@sago35 sago35 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

I'm a little concerned because I think tz is probably 24-bit-range.
However, this is probably not a problem for z.

@sago35
Copy link
Member

sago35 commented Dec 14, 2021

Thank you for your contribution.
Now marging.

@sago35 sago35 merged commit 43099c5 into tinygo-org:dev Dec 14, 2021
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.

2 participants