802.15.4 HIL updates; nRF52840 IEEE 802.15.4 Driver Updates #3995
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
nrf
tyler-potyondy
requested changes
May 22, 2024
| 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.
Suggested change
| // Not enough room for CRC | |
| // Provided buffer does not have adequate space. |
There was a problem hiding this comment.
This seems less clear to me.
There was a problem hiding this comment.
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.
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>
bradjc
force-pushed
the
nrf52840-ieee802154-updates
branch
from
f722b2d
to
9479df2
Compare
|
Is this waiting on me? |
tyler-potyondy
approved these changes
Jun 5, 2024
ppannuto
approved these changes
Jun 5, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
OFFerror if try to TX when radio is off.This required some changes to the constants in the HIL.
PHR_SIZEandPHR_OFFSET.MIN_MHR_SIZEas 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.