[rfsim] fix delayed sleep handling and align with radio API - #833
[rfsim] fix delayed sleep handling and align with radio API#833EskoDijk wants to merge 2 commits into
Conversation
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.
There was a problem hiding this comment.
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.
| 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); | ||
| } |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 :)
|
The failed CI test is |
Two commits:
[rfsim] fix delayed sleep handling and align with radio API
[rfsim] return OT_ERROR_NONE on delayed sleep in
otPlatRadioSleep