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

Make nrf52 15.4 driver standards compliant; create generic 15.4 component #1878

Merged
merged 2 commits into from May 28, 2020

Conversation

hudson-ayers
Copy link
Contributor

Pull Request Overview

This pull request fixes #1806 by making several adjustments to the nrf52 ieee802.15.4 driver so that it sends packets which are valid according to the standard. It also creates a generic 15.4 component, and uses it to replace the chip-specific components used for the same purpose by Imix and nrf52dk_base.

The primary issue was that the existing 15.4 code is too specific to the rf233, including an assumption that frame buffers passed to the radio driver will begin the PSDU 2 bytes after the beginning of the buffer, because the first byte of the buffer is assumed to be used to hold a SPI command (an rf233 specific implementation detail). The existing driver avoided this problem by appending an extra header in hardware -- the s0 header -- which is not intended to be included when the multiprotocol radio is used for iee802.15.4. The s0 header made the on-air frames invalid, and caused correctly configured radios to drop the frames in hardware. The fix in this PR is to offset the DMA pointer given to the hardware by 1 rather than including an extra header that does not belong. A better fix would involve replacing all instances of radio::PSDU_OFFSET with a chip-specific reference to an associated constant, but that would be a much more involved change.

I made several other changes to the 15.4 driver to fix obvious issues or to make the driver more closely match the driver used by RiotOS.

The generic component still has the issue that it is impossible to initialize a MuxMac without a 15.4 userspace driver, but I believe resolving that in a generic way will require some HIL changes so I am leaving it for another time.

Testing Strategy

This pull request was tested using the libtock-c apps examples/tests/ieee802154/radio_tx and examples/tests/ieee802154/radio_rx and verifying that packets can be sent from Imix->Imix, Imix->nrf52840dk, nrf52840dk->Imix, and nrf52840dk->nrf52840dk.

I also used a 15.4 packet sniffer to sniff packets sent by both an Imix and an nrf52840dk and verified that the packets appear identical. (before this PR the nrf packets would not be detected by a sniffer at all!)

TODO or Help Wanted

N/A

Documentation Updated

  • No updates are required.

Formatting

  • Ran make format.
  • Fixed errors surfaced by make clippy.

Copy link
Contributor

@bradjc bradjc left a comment

Choose a reason for hiding this comment

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

Very cool.

Copy link
Member

@ppannuto ppannuto left a comment

Choose a reason for hiding this comment

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

bors d+

I'll confess to a more cursory review of this, but working radio code > non-working IMO

@bors
Copy link
Contributor

bors bot commented May 27, 2020

✌️ hudson-ayers can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@bradjc bradjc added last-call Final review period for a pull request. P-Upkeep This a relatively minor change, or one that is limited in scope, and requires less scrutiny. labels May 27, 2020
@bradjc
Copy link
Contributor

bradjc commented May 28, 2020

bors r+

@bors bors bot merged commit 252bd9b into tock:master May 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
last-call Final review period for a pull request. P-Upkeep This a relatively minor change, or one that is limited in scope, and requires less scrutiny.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

802.15.4 between RF233 and nrf52840 radios fails
3 participants