Skip to content

[Board] Adafruit Trinket #333

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

Merged
merged 10 commits into from
May 14, 2019
Merged

[Board] Adafruit Trinket #333

merged 10 commits into from
May 14, 2019

Conversation

Munsio
Copy link
Contributor

@Munsio Munsio commented May 7, 2019

Corresponding Issue for discussions: #332

@deadprogram
Copy link
Member

At first glance, this looks great @Munsio thank you very much for the contribution.

@aykevl any comments before I merge it?

@Munsio
Copy link
Contributor Author

Munsio commented May 8, 2019

If you say it looks fine i would like to test it before you merge it. (as stated in the issue i didn't try to run any code on it cause i wasn't sure that it won't brick it)

I get home at aprox. 18:00 GMT and try running the blinky example on it.

@deadprogram
Copy link
Member

One note: I would use the same instructions to put the code on it with tinygo build as the ItsyBitsy. https://tinygo.org/microcontrollers/itsybitsy-m0/#uf2

The UF2 bootloader works quite well and quickly/safely on the various Adafruit boards that support it.

@deadprogram
Copy link
Member

It is also useful to compare your pin mappings to something like https://github.com/adafruit/ArduinoCore-samd/blob/master/variants/trinket_m0/variant.cpp

@Munsio
Copy link
Contributor Author

Munsio commented May 8, 2019

I checked it with the file you mentioned and saw that i didn't provide the DotStart and USB-Host Pins - i am not sure if they should be added? The rest seems okay, there are some placeholder pins described which i also not know if they should be included.

@deadprogram
Copy link
Member

The dotStar requires a bitbanging-style SPI interface, so we do not have that yet.

The USB-host pins you will need, in order to use the serial over USB something like this: https://github.com/tinygo-org/tinygo/blob/master/src/machine/board_itsybitsy-m0.go#L39-L43

Copy link
Member

@aykevl aykevl left a comment

Choose a reason for hiding this comment

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

In general this looks good but testing it on real hardware would be necessary to merge.
It's unlikely you'll really brick the device, certainly when you flash using the bootloader.

@Munsio Munsio changed the title WIP: [Board] Adafruit Trinket [Board] Adafruit Trinket May 8, 2019
@Munsio
Copy link
Contributor Author

Munsio commented May 8, 2019

Blinky is working and Serial is working - if you want you can merge it

I2C0 = I2C{Bus: sam.SERCOM2_I2CM,
SDA: SDA_PIN,
SCL: SCL_PIN,
PinMode: GPIO_SERCOM}
Copy link
Member

Choose a reason for hiding this comment

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

This should be GPIO_SERCOM_ALT here, I think.

@deadprogram
Copy link
Member

I think this PR is about ready to merge, from what I can tell.

Only thing is, it greatly complicates merging my own PR #331 in either case there will be some conflicts to resolve, but I think my changes are fewer, so would better to merge it first, then resolve this one, and merge it.

Any opinions on this?

@Munsio Munsio changed the base branch from master to dev May 9, 2019 08:59
@Munsio
Copy link
Contributor Author

Munsio commented May 9, 2019

For me it is okay if you want to merge #331 first.
The other question is should we wait with this merge-request until i also implemented the I2S to the trinket or should this be a second PR just adding the sound feature.

@deadprogram
Copy link
Member

It will probably require some additions to get it to compile with the I2S changes. Getting it to work with a separate mic is another story. As long as it compiles for now would be enough for me.

Copy link
Member

@aykevl aykevl left a comment

Choose a reason for hiding this comment

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

Quickly reviewed it again and LGTM (assuming it has been tested). Not merging because of #333 (comment).

@deadprogram
Copy link
Member

@Munsio the I2S PR was merged. Can you please rebase dev into this branch, and make sure that your example still builds as you expect? Thank you.

@deadprogram
Copy link
Member

If you rebase dev into your branch, you will discover the following when trying to build your example:

$ tinygo build -size short -o test.elf -target=trinket-m0 examples/blinky1
# machine
src/machine/machine_atsamd21.go:678:16: undeclared name: I2S_SCK_PIN
src/machine/machine_atsamd21.go:679:15: undeclared name: I2S_WS_PIN
src/machine/machine_atsamd21.go:680:15: undeclared name: I2S_SD_PIN

This is what I was referring to in #333 (comment)

@deadprogram
Copy link
Member

Thank you very much for all the great work on this @Munsio now merging. Take a victory lap!

@deadprogram deadprogram merged commit fc2ed2b into tinygo-org:dev May 14, 2019
torntrousers pushed a commit to HarringayMakerSpace/tinygo that referenced this pull request May 18, 2019
* Add support for Adafruit Trinket-M0 board
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.

3 participants