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

nrf: add initial support for CLOCK and RADIO #463

Closed
wants to merge 29 commits into from

Conversation

hannes-hochreiner
Copy link

@hannes-hochreiner hannes-hochreiner commented Oct 30, 2021

I added support for basic transmission and reception of packets using the RADIO peripheral. In order to use the radio, the high-frequency clock needs to be running using the external oscillator (XO). Therefore, also added support for configuring the low- and high-frequency clocks.

fixes #432

@Dirbaio
Copy link
Member

Dirbaio commented Oct 31, 2021

Thanks for the PR!

Configuring clocks is already supported in embassy_nrf::init here. We shouldn't have two APIs to do the same. The API you added aditionally supports changing the configuration at runtime, which is nice! We can add that to the existing API, however I think it's not strictly required for using the radio so if we do that it'd be a better fit in a separate PR.

About RADIO: As I said in #432, I'm concerned that adding async at such a low layer might prevent implementing protocols because it's too slow or not flexible enough. For example, Nordic's SoftDevice (proprietary BLE stack) makes extensive use of interrupts, peripheral shortcuts, PPI, one RTC and one TIMER.

Do you have any protocol in mind that you want to implement? It would be nice to have some PoC to verify that this approach is viable for implementing higher level protocols on top.

@hannes-hochreiner
Copy link
Author

About the clock configuration: Agreed, there should be only one API to configure the clocks. Using embassy_nrf::init you loose the syntactic sugar of embassy::main, but otherwise it works just fine. I had a quick look at the product specification for the nrf52810, apart from current consumption differences in using timers, there is no need to switch the hf-clock source during runtime. I therefore removed the clock API again.

About the radio: my current use case is sending short messages from battery powered sensors. For this, I am using my proprietary protocol. I have had a look at the protocols you suggested: IEEE 802.15.4 seems like a good protocol for what I am doing, but the only chip I have access to (nrf52810) does not support it; ESB and BLE are supported by the nrf52810. I have yet to investigate, if there is a PoC I can do in a reasonable time.

Do you already have an idea of your preferred architecture for the protocol implementation? I understand that building it up layer by layer might not work due to overhead (see you comment in #432). Do you see the protocol implementation outside of the scope of embassy? Or do you see different APIs (radio_ble, radio_generic, radio_esb, etc) placed behind feature flags as needed?

@jacobrosenthal
Copy link
Contributor

Hey @hannes-hochreiner are you on matrix?
The smoltcp (now with 6lowpan), embassy and rust-iot folks are over there too
https://matrix.to/#/#embassy-rs:matrix.org
https://matrix.to/#/#rust-iot:matrix.org
https://matrix.to/#/#smoltcp:matrix.org

Id love to see everyone talking

@hannes-hochreiner
Copy link
Author

As things stand, I don´t have the bandwidth to implement one of the requested protocols. In the current state of the implementation, the pull request will not be accepted. Hence, I am closing this pull-request.

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.

nrf/radio: implement the radio peripheral
3 participants