-
Notifications
You must be signed in to change notification settings - Fork 653
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
802.15.4 HIL updates; nRF52840 IEEE 802.15.4 Driver Updates #3995
Conversation
return Err((ErrorCode::BUSY, buf)); | ||
} else if radio::PSDU_OFFSET + frame_len >= buf.len() { | ||
} else if buf.len() < radio::PSDU_OFFSET + frame_len + radio::MFR_SIZE { | ||
// Not enough room for CRC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Not enough room for CRC | |
// Provided buffer does not have adequate space. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems less clear to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only mentioning the CRC here is inaccurate since it may be that there is not enough room for the PHR or that there is not enough room for the CRC. Perhaps updating then to:
// Not enough room for CRC and/or PHR.
@@ -1298,24 +1349,24 @@ impl<'a> kernel::hil::radio::RadioData<'a> for Radio<'a> { | |||
buf: &'static mut [u8], | |||
frame_len: usize, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not entirely sure what the correct name is for this, but once again, frame_len
seems somewhat misleading here since the frame does not contain the CRC here (that is to be added by hardware). There may not be a better name for this.
No address filtering for nrf52840 in 15.4 mode.
Calling `radio_initialize()` in the HIL::init function puts the radio in to RX mode. That means the radio is on unconditionally in any board that includes the 15.4. The HIL::start() function should be used to start the radio and put it in RX mode.
Avoid an issue where the radio is off but the state is not.
This matches the HIL and the rf233
Co-authored-by: Tyler Potyondy <77175673+tyler-potyondy@users.noreply.github.com>
f722b2d
to
9479df2
Compare
Is this waiting on me? |
Pull Request Overview
This pull request is a pass over the nRF52840 15.4 driver. The changes are fairly well encapsulated in commits, but listed here:
OFF
error if try to TX when radio is off.This required some changes to the constants in the HIL.
PHR_SIZE
andPHR_OFFSET
.MIN_MHR_SIZE
as it is incorrect (should be 7, as the minimum PHR is 9, and the MFR is 2, so that leaves 7).Testing Strategy
Need to do this.
TODO or Help Wanted
n/a
Documentation Updated
/docs
, or no updates are required.Formatting
make prepush
.