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

Adafruit 'seesaw' support #612

Merged
merged 3 commits into from
Dec 4, 2023
Merged

Adafruit 'seesaw' support #612

merged 3 commits into from
Dec 4, 2023

Conversation

trichner
Copy link
Contributor

@trichner trichner commented Oct 17, 2023

Here we go again. Simple driver for the "i2c to anything" seesaw chip.

https://learn.adafruit.com/adafruit-seesaw-atsamd09-breakout/overview

This also addresses half of #564.

assertDeepEquals(t, err, nil)
}

func assertDeepEquals[e any](t *testing.T, actual, expected e) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried quicktest again, does not compute for me. I.e. even the existing usages on dev don't work locally.

Copy link
Member

Choose a reason for hiding this comment

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

even the existing usages on dev don't work locally.

That seems to imply something wrong with your local config. no?

Copy link
Contributor Author

@trichner trichner Oct 21, 2023

Choose a reason for hiding this comment

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

Very fair point :)

  1. I now went through re-building tinygo from v0.30.0 and still get the same issue:
~> tinygo test -v ./tester
panic: unimplemented: (reflect.Type).IsVariadic()
FAIL    tinygo.org/x/drivers/tester     1.314s
~> tinygo version
tinygo version 0.30.0 linux/amd64 (using go version go1.20.8 and LLVM version 16.0.1)
  1. then I had a look at the workflow on the repo that runs it successfully, if I do the same with the docker image I still get the error:
> docker run --rm -it -v /<redacted>/tinygo_drivers:/build ghcr.io/tinygo-org/tinygo-dev:latest /bin/bash
root@2632e6b72f70:/go# cd /build
root@2632e6b72f70:/build# tinygo test ./tester
go: downloading github.com/frankban/quicktest v1.10.2
...
go: downloading golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543
panic: unimplemented: (reflect.Type).IsVariadic()
FAIL    tinygo.org/x/drivers/tester     1.366s
  1. furthermore I found that even the GitHub Action on the dev branch seems to succeed with failing tests:
    Screenshot from 2023-10-22 01-16-21

I still don't know what's wrong with my environment, but I found a fix for the CI action not properly failing: #614

Any pointers? At least the docker environment should work, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I figured it out 🤦

Apparently the way to run unit tests is to use go test and not tinygo test.

Out of curiosity, what's the purpose of tinygo test then?

It runs tests fine without the quicktest dependency :D

seesaw/seesaw.go Outdated

// This is needed for the client seesaw device to flush its RX buffer and process the command.
// See seesaw datasheet for timings for specific modules.
time.Sleep(delay)
Copy link
Member

Choose a reason for hiding this comment

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

Was looking over some seesaw experiments I was working on a while back. One thing that I did for the "standardDelay" was to use a functional interface ... I think I did it this way because I wanted flexibility to switch between busy-wait vs sleep (I've noticed sometimes when dealing with microseconds that is time.Sleep is not always super precise on every target, so that flexibility is useful to me).

Not saying that decision was correct/best (the code might not even be working, I see I still have some debugging print statements in there), but here is an example of what I mean in case you are curious about it:

https://github.com/bgould/go-seesaw/blob/main/seesaw.go#L13-L21
https://github.com/bgould/go-seesaw/blob/main/mini-tft-wing.go#L115-L119

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good though. I've actually not considered that.

I've now simplified it further to leave more room open to also add a ReadWithWaiter(...) or similar in the future while simplifying the interface for the most common use cases.
The 'delay' argument felt a bit awkward anyways.

I think as a first start the current implementation will fit most of the applications.

If one must, one can change the dev.ReadDelay = ... before a read call.

@bgould
Copy link
Member

bgould commented Nov 1, 2023

This looks generally good to me, I will look over some more tomorrow. Was trying to find if I have my TFT Featherwing with me to test, but I think I might not have it (also will check more tomorrow).

I can picture that maybe in addition to this low-level package, eventually there could be sub-packages for devices that use it, like /drivers/seesaw/mini-tft-featherwing. A "device" implemented on top of this package could look something like this:

https://github.com/bgould/go-seesaw/blob/main/mini-tft-wing.go

associated example:

https://github.com/bgould/go-seesaw/blob/main/examples/mini-tft-wing/main.go

(like I said I will try to see if I still have that board so that I can give your package a test drive)

@trichner
Copy link
Contributor Author

trichner commented Nov 1, 2023

This looks generally good to me, I will look over some more tomorrow. Was trying to find if I have my TFT Featherwing with me to test, but I think I might not have it (also will check more tomorrow).

I can picture that maybe in addition to this low-level package, eventually there could be sub-packages for devices that use it, like /drivers/seesaw/mini-tft-featherwing. A "device" implemented on top of this package could look something like this:

https://github.com/bgould/go-seesaw/blob/main/mini-tft-wing.go

associated example:

https://github.com/bgould/go-seesaw/blob/main/examples/mini-tft-wing/main.go

(like I said I will try to see if I still have that board so that I can give your package a test drive)

Indeed! And actually that's what I've done here: #564

I added ontop Keypad & NeoPixel and then wrapped it all to make the NeoTrellis work nicely :)

Though in hindsight I think those are not yet polished enough for the tinygo-org/drivers repo ;)

@trichner
Copy link
Contributor Author

Bump? Any chance for a review?

@trichner
Copy link
Contributor Author

trichner commented Dec 2, 2023

What's missing? Any interest?

@bgould
Copy link
Member

bgould commented Dec 4, 2023

Sorry I was trying to find a board with seesaw on it to test, but I could not find. All in all, LGTM, now merging. Thank you for your contribution @trichner

@bgould bgould merged commit 68da68c into tinygo-org:dev Dec 4, 2023
1 check passed
@trichner
Copy link
Contributor Author

trichner commented Dec 4, 2023

Sorry I was trying to find a board with seesaw on it to test, but I could not find. All in all, LGTM, now merging. Thank you for your contribution @trichner

Ahh, that would have been great indeed. Thanks for merging.
I'll give it a go with the Neotrellis, its a more complex component with a seesaw than my soilsensor.

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.

4 participants