Skip to content

targets: add target circuitplay-bluefruit#819

Merged
deadprogram merged 1 commit into
tinygo-org:devfrom
matloob:dev
Jan 6, 2020
Merged

targets: add target circuitplay-bluefruit#819
deadprogram merged 1 commit into
tinygo-org:devfrom
matloob:dev

Conversation

@matloob

@matloob matloob commented Dec 30, 2019

Copy link
Copy Markdown
Contributor

Add a target for the Adafruit Circuit Playground Bluefruit, which is
based on the nRF52840. Adds the necessary code for the machine
package and the json and linker script files in the targets directory.
The machine package code is based on board_circuitplay_express.go,
with modifications made by consulting the wiring diagram on the
adafruit website here:
https://learn.adafruit.com/adafruit-circuit-playground-bluefruit/downloads

Also adds support to the uf2 conversion packacge to set the familyID
field. The Circuit Playground Bluefruit firmware rejects uf2 files
without the family id set to 0xADA52840 (and without the flag specifying
that the family id is present).

@deadprogram

Copy link
Copy Markdown
Member

Hi @matloob thank you very much for the contribution. I do not have a circuitplay-bluefruit yet, but I tested with my Circuit Playground Express and it did not affect it, as expected. So if it works on your board, works for me.

Could you please also add the new board to the smoketests here: https://github.com/tinygo-org/tinygo/blob/master/Makefile#L149-L253

Lastly, please add the board to the list of supported boards here: https://github.com/tinygo-org/tinygo/blob/master/README.md#supported-boardstargets

Thanks!

@aykevl aykevl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great, thank you for your work on adding this board!
Apart from the things mentioned by @deadprogram, a few nits from me.


// Hardware pins
const (
P0_00 Pin = 0

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These pin names are not board specific, so I think they should be moved into a generic nrf52 file (src/machine/board_nrf52832.go).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment thread targets/circuitplay-bluefruit.ld Outdated

MEMORY
{
FLASH_TEXT (rw) : ORIGIN = 0x00000000+0x26000, LENGTH = 796K /* SoftDevice S140. See https://learn.adafruit.com/introducing-the-adafruit-nrf52840-feather/hathach-memory-map */

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It may be clearer to describe the length with a formula too: LENGTH = 1M - 0x26000. That way it is immediately obvious where that length comes from.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

@bgould

bgould commented Dec 31, 2019

Copy link
Copy Markdown
Member

I have this board, and I can test this out today. Thank you for adding this @matloob !

@bgould

bgould commented Jan 2, 2020

Copy link
Copy Markdown
Member

I get this error when trying to build/flash:

$ tinygo build -target=circuitplay-bluefruit -o /tmp/bluefruit.uf2  src/examples/blinky1/blinky1.go 
error: json: cannot unmarshal string into Go struct field TargetSpec.uf2-family-id of type uint32

@deadprogram

Copy link
Copy Markdown
Member

DId you rebuild tinygo itself using go install? the changes that @matloob made to the uf2 conversion require it.

Comment thread compileopts/target.go Outdated
FlashMethod string `json:"flash-method"`
FlashVolume string `json:"msd-volume-name"`
FlashFilename string `json:"msd-firmware-name"`
UF2FamilyID uint32 `json:"uf2-family-id"`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this property needs to be added to the copyProperties() method below, something like:

  if spec2.UF2FamilyID != 0 {                                                   
    spec.UF2FamilyID = spec2.UF2FamilyID                                                                                                                                  
  }

Otherwise the value of this property is lost in resolveInherits()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done. Now I'm wondering how this ever worked for me...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would suggest making this also a string and then using hex.DecodeString() or whatever to convert to the "real" type.

This just seems like it would make it a lot more readable in the actual JSON files themselves.

What do you think?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sounds good! and this allows us to distinguish between no family id (empty string) and a family id of zero!

"flash-method": "msd",
"msd-volume-name": "CPLAYBTBOOT",
"msd-firmware-name": "firmware.uf2",
"uf2-family-id": "0xADA52840",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I was only able to make this work by specifying the uf2-family-id value as a base 10 integer, ie:

"uf2-family-id": 2913282112,

otherwise encoding/json complains about attempting to unmarshal a string into a uint32

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

@bgould

bgould commented Jan 4, 2020

Copy link
Copy Markdown
Member

The blinky1 and ws2818 examples are working for me using the small tweaks that I've noted above. However I'm not getting any serial device when running programs... should there be one or should I be using a serial converter or something? I tried flash circuitpython onto the board and it creates /dev/ttyACM0 when connected

@deadprogram

Copy link
Copy Markdown
Member

The Circuit Playground Bluefruit is using USB CDC to communicate via serial, similar to how the SAMD21/SAMD51 boards do.

Our options seems to be:

  • implement this ourselves, similar to what we have done with the SAMD21/SAMD51
  • write a CGo wrapper around the aptly named https://github.com/hathach/tinyusb which is what all of the Adafruit boards are using now.

Any thoughts from anyone on this?

@matloob matloob force-pushed the dev branch 2 times, most recently from 365f483 to 9eb3d35 Compare January 4, 2020 13:07
@aykevl

aykevl commented Jan 4, 2020

Copy link
Copy Markdown
Member

I don't like the idea of depending on a C library for such a core feature but it may be the smarter option to reduce the maintenance burden. (It would also be a good use case to prove CGo support). However, I wonder is how well TinyUSB would integrate with TinyGo. For example, what about heap management?

@deadprogram any idea or guess how much work it would be to add support for the nrf52840 for USB? It seems like USB support is not coupled to the SAMD implementation so could theoretically be used from the nrf52840 as well.

In any case, I don't think we should block merging this PR over this issue. Output over USB can be added at a later time. This PR looks fine by me.

Comment thread targets/circuitplay-bluefruit.json Outdated
"flash-method": "msd",
"msd-volume-name": "CPLAYBTBOOT",
"msd-firmware-name": "firmware.uf2",
"uf2-family-id": 2913282112,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

See my previous comment about making this a string then calling hex.DecodeString() to make it more readable.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

@deadprogram

Copy link
Copy Markdown
Member

Regarding the nrf52 USBD support, it is not trivial task of course. The current USB CDC is mostly decoupled I think, but this would be a real test. Also there are as always a number of interesting implementation details that are different.

Regarding using TinyUSB https://github.com/hathach/tinyusb/blob/6ed96cb93b92acf7cce3bd6b84dc5cd8882486c4/docs/getting_started.md#add-tinyusb-to-your-project seems encouraging.

From a bit of a deep dive, https://github.com/hathach/tinyusb/blob/bf76a1e49e6760ef5ac7dd255158ce9475ea8dd1/src/class/cdc/cdc_device.c#L55-L56 seems like TinyUSB allocates its own memory for buffers, but then we can pass our own CGo buffers for transfers: https://github.com/hathach/tinyusb/blob/master/examples/device/cdc_msc/src/main.c#L113-L116

I think it is worth trying a TinyUSB experiment for sure.

@bgould

bgould commented Jan 5, 2020

Copy link
Copy Markdown
Member

I used a USB serial converter connected to the TX/RX pads and confirmed I can get output as expected from various examples. Functionality looks good to me.

@deadprogram

Copy link
Copy Markdown
Member

As soon as @matloob can perhaps address my last feedback https://github.com/tinygo-org/tinygo/pull/819/files#r363045397 then I think we're good.

Add a target for the Adafruit Circuit Playground Bluefruit, which is
based on the nRF52840. Adds the necessary code for the machine
package and the json and linker script files in the targets directory.
The machine package code is based on board_circuitplay_express.go,
with modifications made by consulting the wiring diagram on the
adafruit website here:
https://learn.adafruit.com/adafruit-circuit-playground-bluefruit/downloads

Also adds support to the uf2 conversion packacge to set the familyID
field. The Circuit Playground Bluefruit firmware rejects uf2 files
without the family id set to 0xADA52840 (and without the flag specifying
that the family id is present).
@matloob

matloob commented Jan 5, 2020

Copy link
Copy Markdown
Contributor Author

As soon as @matloob can perhaps address my last feedback https://github.com/tinygo-org/tinygo/pull/819/files#r363045397 then I think we're good.

Done

@deadprogram

Copy link
Copy Markdown
Member

Thank you so very much @matloob for working on this PR and to @bgould for helping test it out.

Now merging.

@deadprogram deadprogram merged commit 1cb9b94 into tinygo-org:dev Jan 6, 2020
@matloob matloob deleted the dev branch January 6, 2020 15:36
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