Skip to content

esb: fix radio staying in RX when no pending send requests #25089

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

esb: fix radio staying in RX when no pending send requests #25089

wants to merge 1 commit into from

Conversation

doki-nordic
Copy link
Contributor

When fast switching is enabled, the radio stays in RXIDLE state even if there are no more packets to send because SW part switches to idle state to early, before radio gets to a DISABLED state.

Ref: NCSDK-35281

@NordicBuilder NordicBuilder added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Oct 16, 2025
@doki-nordic doki-nordic force-pushed the esb-fast-ch-sw-incorrect-spectrum branch from 2e660ca to a9c0692 Compare October 16, 2025 15:21
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Oct 16, 2025 •
edited
Loading

CI Information

To view the history of this post, clich the 'edited' button above
Build number: 6

Inputs:

Sources:

sdk-nrf: PR head: ba4a8c6851657f5929491e75e53eec69345a0bae

more details

sdk-nrf:

PR head: ba4a8c6851657f5929491e75e53eec69345a0bae
merge base: 2cc973169f347c57f36df932972cc3d2c48b8f07
target head (main): 260975db898d4919ba7c687af6c1f0efaf056c43
Diff

Github labels

Enabled Name Description
ci-disabled Disable the ci execution
ci-all-test Run all of ci, no test spec filtering will be done
ci-force-downstream Force execution of downstream even if twister fails
ci-run-twister Force run twister
ci-run-zephyr-twister Force run zephyr twister
List of changed files detected by CI (1) 
subsys
│  ├── esb
│  │  │ esb.c

Outputs:

Toolchain

Version: 46667c6630
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:46667c6630_bba2ea5f2e

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

  • ◻️ Toolchain - Skipped: existing toolchain is used
  • ✅ Build twister
    • sdk-nrf test count: 48
  • ✅ Integration tests
    • ✅ test-fw-nrfconnect-proprietary_esb
Disabled integration tests
    • test-fw-nrfconnect-nrf_lrcs_mosh
    • test-fw-nrfconnect-nrf_lrcs_positioning
    • desktop52_verification
    • test_ble_nrf_config
    • test-fw-nrfconnect-apps
    • test-fw-nrfconnect-ble_mesh
    • test-fw-nrfconnect-ble_samples
    • test-fw-nrfconnect-chip
    • test-fw-nrfconnect-fem
    • test-fw-nrfconnect-nfc
    • test-fw-nrfconnect-nrf-iot_cloud
    • test-fw-nrfconnect-nrf-iot_libmodem-nrf
    • test-fw-nrfconnect-nrf-iot_lwm2m
    • test-fw-nrfconnect-nrf-iot_samples
    • test-fw-nrfconnect-nrf-iot_serial_lte_modem
    • test-fw-nrfconnect-nrf-iot_thingy91
    • test-fw-nrfconnect-nrf-iot_zephyr_lwm2m
    • test-fw-nrfconnect-nrf_crypto
    • test-fw-nrfconnect-ps-main
    • test-fw-nrfconnect-rpc
    • test-fw-nrfconnect-rs
    • test-fw-nrfconnect-tfm
    • test-fw-nrfconnect-thread-main
    • test-low-level
    • test-sdk-audio
    • test-sdk-dfu
    • test-sdk-find-my
    • test-sdk-mcuboot
    • test-sdk-pmic-samples
    • test-sdk-wifi
    • test-secdom-samples-public

Note: This message is automatically posted and updated by the CI

@doki-nordic doki-nordic force-pushed the esb-fast-ch-sw-incorrect-spectrum branch from a9c0692 to 96e98f0 Compare October 16, 2025 16:03
@doki-nordic doki-nordic marked this pull request as ready for review October 16, 2025 16:03
@doki-nordic doki-nordic requested a review from a team as a code owner October 16, 2025 16:03
@doki-nordic doki-nordic force-pushed the esb-fast-ch-sw-incorrect-spectrum branch from 96e98f0 to 5841871 Compare October 17, 2025 10:31
@NordicBuilder NordicBuilder requested a review from a team October 17, 2025 10:31
Szynkaa
Szynkaa reviewed Oct 17, 2025
View reviewed changes
subsys/esb/esb.c Outdated
nrf_radio_packetptr_set(NRF_RADIO, rx_payload_buffer);
if (fast_switching) {
nrf_radio_int_disable(NRF_RADIO, ESB_RADIO_INT_END_MASK);
nrf_radio_event_clear(NRF_RADIO, ESB_RADIO_EVENT_END);
Copy link
Member

Choose a reason for hiding this comment

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

I don't think event clear after disabling interrupt is needed.

Szynkaa
Szynkaa reviewed Oct 17, 2025
View reviewed changes
subsys/esb/esb.c

if (fast_switching) {
nrf_radio_int_disable(NRF_RADIO, ESB_RADIO_INT_END_MASK);
nrf_radio_event_clear(NRF_RADIO, ESB_RADIO_EVENT_END);
Copy link
Member

Choose a reason for hiding this comment

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

I don't think disabling interrupt here is needed as it was already disabled in line 1410. Similarly event clear after disabling interrupt is not needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I copied together int_disable and event_clear just to be sure. At this point interrupt should be already disabled.

Regarding event_clear - it is needed here. Radio has still an old END event active here, since it was never cleared. When we enable interrupt later, it will trigger invalid interrupt immediately based on old END event.

@doki-nordic
esb: fix radio staying in RX when no pending send requests
ba4a8c6 
When fast switching is enabled, the radio stays in RXIDLE state
even if there are no more packets to send because SW part switches
to idle state to early, before radio gets to a DISABLED state.

Ref: NCSDK-35281

Signed-off-by: Dominik Kilian <[email protected]>
@doki-nordic doki-nordic force-pushed the esb-fast-ch-sw-incorrect-spectrum branch from 5841871 to ba4a8c6 Compare October 17, 2025 15:46
@doki-nordic doki-nordic requested a review from Szynkaa October 17, 2025 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@maje-emb maje-emb maje-emb approved these changes

@Szynkaa Szynkaa Awaiting requested review from Szynkaa

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@doki-nordic @NordicBuilder @Szynkaa @maje-emb