Skip to content

Conversation

@dhalbert
Copy link
Collaborator

@dhalbert dhalbert commented Feb 13, 2019

  • Support both UARTE's on nRF52840.
  • Reset UART drivers on restart.
  • Use a table for baud rate selection.
  • Make sure temp buffer doesn't move.
  • Remove nRF52832 (no UART) "not implemented" stuff.

Fixes remaining tasks in #1039.

@dhalbert dhalbert requested a review from tannewt February 13, 2019 03:36
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.

One comment about the allocation. Everything else looks great. Thanks!

if ( !nrfx_is_in_ram(data) ) {
tx_buf = (uint8_t *) gc_alloc(len, false, false);
// TODO: If this is not too big, we could allocate it on the stack.
// Do a long-lived alloc so buffer won't move.
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't be long-lived because it is freed below. That will leave a hole in the long-lived area. It won't move because no other python bytecode is run while it's alive.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK, I was worried that MICROPY_VM_HOOK_LOOP might end up doing something. But it looks like even the displayio stuff will not trigger anything. (Just checking.)

@dhalbert
Copy link
Collaborator Author

Updated to not use long-lived. Travis is happy.

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.

Ok great! Thank you!

@tannewt tannewt merged commit a1a4959 into adafruit:master Feb 14, 2019
@dhalbert dhalbert deleted the nrf-two-uarts branch February 14, 2019 02:10
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