Skip to content

[rfsim] fix delayed sleep handling and align with radio API - #833

Open
EskoDijk wants to merge 2 commits into
openthread:mainfrom
EskoDijk:pr-rfsim-state-fixes-and-api
Open

[rfsim] fix delayed sleep handling and align with radio API#833
EskoDijk wants to merge 2 commits into
openthread:mainfrom
EskoDijk:pr-rfsim-state-fixes-and-api

Conversation

@EskoDijk

@EskoDijk EskoDijk commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Two commits:

[rfsim] fix delayed sleep handling and align with radio API

The delayed sleep scheduled by `otPlatRadioSleep()` was not always applied.
This commit fixes that by calling `applyRadioDelayedSleep()` from more
code paths. Also, all radio platform functions are aligned to comply with
the clarified `otPlatRadio...()` API contract of OT PR #13580.

Delayed sleep related changes:

 - `platformRadioTxDone()` now applies a pending sleep once the Ack Tx is
   done.
 - the `RX_FRAME_ONGOING` timer path in `platformRadioProcess()` applies a
   pending sleep too. That path is not only a failsafe: the simulator sends
   RxDone just to the frame's addressee, so it is the normal exit for any
   frame that is overheard by a neighboring node.
 - `platformRadioRxDone()` calls `radioReceive()` now before the substate
   transitions, so that `otPlatRadioReceiveDone()` is invoked before the
   radio sleeps, as the API requires. Previously the sleep state was
   applied first, after which `radioReceive()` returned early and the
   received frame was silently dropped.

API contract related changes:

 - `otPlatRadioSleep()` returns `OT_ERROR_BUSY` while transmitting.
 - `otPlatRadioDisable()` accepts a pending transition to sleep and enacts
   it; `OT_ERROR_INVALID_STATE` now means that the radio is neither in Sleep
   state nor transitioning to Sleep state.
 - `otPlatRadioReceive()` returns `OT_ERROR_INVALID_STATE` while
   transmitting, and models the radio ramp-up time when the receive channel
   is changed. An Ack Tx that is already on air is not aborted, which is
   allowed by the API; this is done for practical reasons documented in the
   comments.
 - `otPlatRadioEnergyScan()` cancels a pending transition to sleep.

Radio state machine:

 - `setRadioState(OT_RADIO_STATE_DISABLED)` resets the substate to Ready
   instead of Startup; the startup time is applied by `otPlatRadioEnable()`.
 - the Startup -> Ready transition updates `sOngoingOperationChannel`, so
   that the simulator learns the new listening channel as soon as the
   ramp-up is done. Without this, a node stays registered on its previous
   channel until its next transmission and misses frames sent to it in
   the meantime.

[rfsim] return OT_ERROR_NONE on delayed sleep in otPlatRadioSleep

When `otPlatRadioSleep()` is called while the radio is receiving an
incoming frame, waiting in AIFS, or actively transmitting an auto-ACK,
`ot-rfsim` schedules a delayed transition to sleep (`sDelaySleep = true`)
which takes effect once the ongoing frame/ACK operation completes.

abtink and others added 2 commits September 4, 2026 19:52
When `otPlatRadioSleep()` is called while the radio is receiving an
incoming frame, waiting in AIFS, or actively transmitting an auto-ACK,
`ot-rfsim` schedules a delayed transition to sleep (`sDelaySleep = true`)
which takes effect once the ongoing frame/ACK operation completes.

Previously, `otPlatRadioSleep()` returned `OT_ERROR_BUSY` in these
substates which would be incorrect. The radio driver must accept the
sleep request and transition to sleep after finishing the current
atomic operation without returning an error.

This commit updates `otPlatRadioSleep()` to return `OT_ERROR_NONE`
when scheduling delayed sleep, indicating that the sleep request
has been successfully accepted.
The delayed sleep scheduled by `otPlatRadioSleep()` was not always applied.
This commit fixes that by calling `applyRadioDelayedSleep()` from more
code paths. Also, all radio platform functions are aligned to comply with
the clarified `otPlatRadio...()` API contract of OT PR #13580.

Delayed sleep related changes:

 - `platformRadioTxDone()` now applies a pending sleep once the Ack Tx is
   done.
 - the `RX_FRAME_ONGOING` timer path in `platformRadioProcess()` applies a
   pending sleep too. That path is not only a failsafe: the simulator sends
   RxDone just to the frame's addressee, so it is the normal exit for any
   frame that is overheard by a neighboring node.
 - `platformRadioRxDone()` calls `radioReceive()` now before the substate
   transitions, so that `otPlatRadioReceiveDone()` is invoked before the
   radio sleeps, as the API requires. Previously the sleep state was
   applied first, after which `radioReceive()` returned early and the
   received frame was silently dropped.

API contract related changes:

 - `otPlatRadioSleep()` returns `OT_ERROR_BUSY` while transmitting.
 - `otPlatRadioDisable()` accepts a pending transition to sleep and enacts
   it; `OT_ERROR_INVALID_STATE` now means that the radio is neither in Sleep
   state nor transitioning to Sleep state.
 - `otPlatRadioReceive()` returns `OT_ERROR_INVALID_STATE` while
   transmitting, and models the radio ramp-up time when the receive channel
   is changed. An Ack Tx that is already on air is not aborted, which is
   allowed by the API; this is done for practical reasons documented in the
   comments.
 - `otPlatRadioEnergyScan()` cancels a pending transition to sleep.

Radio state machine:

 - `setRadioState(OT_RADIO_STATE_DISABLED)` resets the substate to Ready
   instead of Startup; the startup time is applied by `otPlatRadioEnable()`.
 - the Startup -> Ready transition updates `sOngoingOperationChannel`, so
   that the simulator learns the new listening channel as soon as the
   ramp-up is done. Without this, a node stays registered on its previous
   channel until its next transmission and misses frames sent to it in
   the meantime.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the radio state machine and timing logic in ot-rfsim/src/radio.c to improve delayed sleep handling, state transitions, and channel changes. Feedback highlights a critical issue where bypassing the startup transition during an ongoing Ack transmission allows an instantaneous channel change, violating physical timing constraints. It is recommended to check for pending channel changes when exiting the IFS wait state to correctly trigger the startup ramp-up delay.

Comment thread ot-rfsim/src/radio.c
Comment on lines +392 to 399
bool isAckOnAir = (sSubState == RFSIM_RADIO_SUBSTATE_RX_ACK_TX_ONGOING);

if (!isAckOnAir && ((sState == OT_RADIO_STATE_SLEEP && sSubState != RFSIM_RADIO_SUBSTATE_STARTUP) ||
aChannel != sCurrentChannel))
{
// Going from sleep to receive, or a channel change, incurs the ramp-up time.
setRadioSubState(RFSIM_RADIO_SUBSTATE_STARTUP, RFSIM_RAMPUP_TIME_US);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

When isAckOnAir is true, the transition to RFSIM_RADIO_SUBSTATE_STARTUP is bypassed to let the ongoing Ack transmission finish. However, sCurrentChannel is still immediately updated to aChannel. When the Ack transmission completes, the substate machine transitions through RX_TX_TO_RX -> IFS_WAIT -> READY and updates sOngoingOperationChannel to sCurrentChannel without ever incurring the RFSIM_RAMPUP_TIME_US ramp-up delay. This allows an instantaneous channel change, bypassing the physical timing constraints of the radio model.\n\nTo fix this, we should check if a channel change is pending when exiting RFSIM_RADIO_SUBSTATE_IFS_WAIT in platformRadioProcess(). If sCurrentChannel != sOngoingOperationChannel, we should transition to RFSIM_RADIO_SUBSTATE_STARTUP instead of RFSIM_RADIO_SUBSTATE_READY.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The intention of the current code is that the substates RX_TX_TO_RX and IFS_WAIT together are a waiting period in which the radio transitions back to Rx; and the radio does that now on its newly scheduled/selected channel. So physically it's ok per current code: the radio takes 2*sTurnAroundTime and it could do this even faster, in 1 * sTurnAroundTime. The rampup time is equal to sTurnAroundTime.

One option is to document this intended behavior; if so I'll hear it here :)

@EskoDijk

EskoDijk commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

The failed CI test is testSimulationId; which also fails more often in other PRs and on my local machine. (Random failures) - perhaps resolve that in a separate PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants