Skip to content

nrf: rework of pin files; add new boards; add pin claiming #1158

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 7 commits into from
Sep 5, 2018

Conversation

dhalbert
Copy link
Collaborator

@dhalbert dhalbert commented Aug 31, 2018

  • Redo pin definitions in nrf.
    • Remove use of pins.csv files: Use file and structures similar to atmel-samd port, so that pin definitions can be commented and formatted.
    • Store pin numbers as combination port/relative-pin-number, as in atmel-samd, instead of storing port separately. Simplifies some code and allows them to be passed around more easily.
  • Add pin claiming, so that two peripherals cannot be created with overlapping pins. Need to store pin numbers in some peripheral objects, as in atmel-samd. Add calls to claim_pin() and pin_reset_number() in peripheral classes.
  • Add PCA10059 dongle board definition.
  • Add Arduino/Metro style pin names to PCA10056 board: D0, SCL, etc.
  • Rename pin_reset() to pin_reset_number() in common-hal/microcontroller/Pins.c to avoid confusion between routines that take pin objects and those that take pin numbers. Made this change in atmel-samd also for consistency.
  • Add stub for neopixel_write.
  • Change printf() call in main.c to mp_printf(): was causing DEBUG builds to fail.
  • Fix atmel-samd: IncrementalEncoder and PulseIn don't call claim_pin() #1157: missing pin claiming in some atmel-samd devices.

More work to do in peripherals, including allow multiple instances in some cases, implementing not-yet-implemented. Those are covered in other issues.

@tannewt will merge, but @arturo182 take a look if you wish, because there are changes of interest in your device code.

Copy link
Collaborator

@arturo182 arturo182 left a comment

Choose a reason for hiding this comment

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

Looks really good! Few minor issues.

Copy link
Collaborator

@arturo182 arturo182 left a comment

Choose a reason for hiding this comment

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

Just a few more questions.

arturo182
arturo182 previously approved these changes Sep 4, 2018
Copy link
Collaborator

@arturo182 arturo182 left a comment

Choose a reason for hiding this comment

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

LGTM

tannewt
tannewt previously requested changes Sep 4, 2018
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Overall looks great! Two minor comments and then ok by me.

@tannewt
Copy link
Member

tannewt commented Sep 5, 2018

Will Approve after this is updated to fix the conflict. Thanks!

tannewt
tannewt previously approved these changes Sep 5, 2018
Copy link
Member

@tannewt tannewt 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! Thanks!

@dhalbert
Copy link
Collaborator Author

dhalbert commented Sep 5, 2018

@tannewt had to redo after merge due to pin struct changes; I forgot to try a recompile. Ready for another mini-review and travis is happy.

Tested with a NeoPixel ring.

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Thank you!

@tannewt tannewt merged commit 23b23dd into adafruit:master Sep 5, 2018
@dhalbert dhalbert deleted the nrf-pin-claiming branch September 12, 2018 16:54
@dhalbert
Copy link
Collaborator Author

Fixes #1089

@hansmbakker
Copy link

hansmbakker commented Nov 4, 2018

@dhalbert I see PCA10059 was added by this PR, but I don't see it among the files released for circuitpython 3.1.1.

Should we compile it ourselves to get started for the PCA10059 board? Is there a "getting started" page for that?

I don't see the board mentioned on https://github.com/adafruit/circuitpython/tree/master/ports/nrf

@tannewt
Copy link
Member

tannewt commented Nov 4, 2018

@hansmbakker We've done a lot more work on 4.x. Either try those releases or test builds from here.

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

Successfully merging this pull request may close these issues.

4 participants