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

Cherrypick fixes for ACK header IE related bugs #1470

Merged
merged 4 commits into from
Jan 26, 2024

Conversation

maciejbaczmanski
Copy link
Member

@maciejbaczmanski maciejbaczmanski commented Jan 26, 2024

Revert hotfix from PR: #1441

Fix bugs in current implementation of ieee802154 driver:

  • after refactoring set_vendor_ie_header_lm, case when link_metrics_data_len == 0 has been ignored. Fixed by refactoring the code again.
  • Resetting RCP stack by openthread has been handled incorrectly (-ENOTSUP returned). Fixed by implementing otPlatRadioResetCsl in upstream OpenThread and adding clear_ie flag.
  • refactoring of enh ACK header IE resulted with returning -ENOTSUP when ack_ie.header_ie == NULL (set when csl period set to 0 in otPlatRadioEnableCsl), because
    ieee802154_header_ie_get_element_id accessed NULL pointer.
  • struct ieee802154_config config is a struct containing an union. Members of config were accessed incorrectly in otPlatRadioEnableCsl. Fixed by initializing config with 0 and accessing one member at a time.

Cherry-picked from:

@NordicBuilder
Copy link
Contributor

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff
openthread zephyrproject-rtos/openthread@00076af zephyrproject-rtos/openthread@7761b81 (main) zephyrproject-rtos/[email protected]

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@maciejbaczmanski maciejbaczmanski changed the title Cherrypick for fixing ACK header IE related bugs Cherrypick fixes for ACK header IE related bugs Jan 26, 2024
@maciejbaczmanski maciejbaczmanski force-pushed the ot_enh_ack_fix_ncs_pr branch 2 times, most recently from 9822ed9 to 6f4be47 Compare January 26, 2024 10:16
Copy link

@jciupis jciupis left a comment

Choose a reason for hiding this comment

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

It seems that the commit message of the revert refers to maciejbaczmanski@1da00e0 instead of 1da00e0. Please correct it to keep a valid reference to the commit in the future.

Other than that - lovely 👍

@maciejbaczmanski
Copy link
Member Author

It seems that the commit message of the revert refers to maciejbaczmanski@1da00e0 instead of 1da00e0. Please correct it to keep a valid reference to the commit in the future.

Other than that - lovely 👍

that's weird because commit message does not contain any remote references:
image
I'll change the commit message to a full link

- In `set_vendor_ie_header_lm`, case when
`link_metrics_data_len == 0` has been ignored.
This commit fixes that by setting `header_ie->length = 0`
before returning.
- current implementation of enh ACK header IE returns
`-ENOTSUP` when `ack_ie.header_ie == NULL` or
`ack_ie.header_ie->length == 0`. This commit fixes that by
refactoring checks in `nrf5_configure`.

Co-authored-by: Przemyslaw Bida <[email protected]>

Signed-off-by: Maciej Baczmanski <[email protected]>
(cherry picked from commit d76bcd3)
…4_config`

`struct ieee802154_config config` is a struct containing an
union. Members of `config` were accessed incorrectly in
`otPlatRadioEnableCsl`. Fix by initializing `config` with `0`
and accessing one member at a time.

Signed-off-by: Maciej Baczmanski <[email protected]>
(cherry picked from commit 36b7f44)
additionaly, implement `otPlatRadioResetCsl` functionality

Signed-off-by: Maciej Baczmanski <[email protected]>
(cherry picked from commit 0f1747e)
@rlubos rlubos merged commit b3c97b0 into nrfconnect:main Jan 26, 2024
11 checks passed
@maciejbaczmanski maciejbaczmanski deleted the ot_enh_ack_fix_ncs_pr branch January 26, 2024 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants