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

Add Nano 33 BLE Board #1909

Merged
merged 9 commits into from Jun 24, 2020
Merged

Add Nano 33 BLE Board #1909

merged 9 commits into from Jun 24, 2020

Conversation

bradjc
Copy link
Contributor

@bradjc bradjc commented Jun 4, 2020

Pull Request Overview

This pull request adds a board main.rs for the long-discussed (see #994) Nano 33 BLE board. Since it is based on the nrf52840, this is pretty straightforward.

The bootloader situation is not the best at the moment, but that is a work in progress. Also right now the UART requires attaching an FTDI chip to the UART pins. However, when the CDC USB stack is merged we can switch to using the USB header.

Testing Strategy

This pull request was tested by running the hello loop and blink apps on the board.

TODO or Help Wanted

n/a

Documentation Updated

  • Updated the relevant files in /docs, or no updates are required.

Formatting

  • Ran make prepush.

hudson-ayers
hudson-ayers previously approved these changes Jun 4, 2020
Copy link
Contributor

@hudson-ayers hudson-ayers left a comment

Choose a reason for hiding this comment

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

I don't have a board to test this on but overall it looks good!

boards/nano33ble/Makefile Outdated Show resolved Hide resolved
boards/nano33ble/src/main.rs Outdated Show resolved Hide resolved
boards/nano33ble/src/main.rs Outdated Show resolved Hide resolved
hudson-ayers
hudson-ayers previously approved these changes Jun 4, 2020
hudson-ayers
hudson-ayers previously approved these changes Jun 9, 2020
boards/nano33ble/README.md Show resolved Hide resolved
boards/nano33ble/README.md Outdated Show resolved Hide resolved
@brghena brghena added the blocked Waiting on something, like a different PR or a dependency. label Jun 13, 2020
@ppannuto ppannuto requested a review from brghena June 19, 2020 19:31
@brghena brghena removed the blocked Waiting on something, like a different PR or a dependency. label Jun 19, 2020
Copy link
Contributor

@brghena brghena left a comment

Choose a reason for hiding this comment

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

Two comments for you, but I think this is good to go. I tested it on hardware (in the nano33_3 branch) and found it to work great.

boards/nano33ble/README.md Outdated Show resolved Hide resolved
Comment on lines +28 to +46
fn write(&mut self, buf: &[u8]) {
let uart = unsafe { &mut nrf52840::uart::UARTE0 };
if !self.initialized {
self.initialized = true;
uart.configure(uart::Parameters {
baud_rate: 115200,
stop_bits: uart::StopBits::One,
parity: uart::Parity::None,
hw_flow_control: false,
width: uart::Width::Eight,
});
}
for &c in buf {
unsafe {
uart.send_byte(c);
}
while !uart.tx_ready() {}
}
}
Copy link
Contributor

@brghena brghena Jun 19, 2020

Choose a reason for hiding this comment

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

How hard would it be to have the panic dump print out over USB instead of UART (or in addition)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Really hard, maybe impossible? I don't know how to do it without interrupts.

It might be worth thinking about handling panics in apps differently so that the kernel is still running while the panic message is printed.

Copy link
Contributor

Choose a reason for hiding this comment

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

I won't block this PR on it, but we're going to have to think of some way to at least be able to debug crashing apps over USB serial if we want this to be a first-class board.

Copy link
Contributor

Choose a reason for hiding this comment

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

brghena
brghena previously approved these changes Jun 22, 2020
@brghena
Copy link
Contributor

brghena commented Jun 24, 2020

bors r+

@bors bors bot merged commit 188a8c1 into master Jun 24, 2020
@bors bors bot deleted the nano33_2 branch June 24, 2020 18:17
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.

None yet

4 participants