Conversation
|
Oh darn, i should have measured this values more carefully... probably calculated wrong LIFS though. Thank you for the PR, i will test this one as well. |
One thing this PR doesn't accomodate for is compensating the timing for the delay caused by starting the radio again. I'll submit a follow-up asap so that you don't have to build a test setup twice :) |
|
@SemjonKerner I've added a commit that disables the hardware IFS handling in the radio. This reduces the overhead of switching the radio to TX mode to around 40 μs (from 130μs) . |
|
The PR is reasonable, the code looks good, ping still works as expected and I can reproduce all timings with and without this PR.
|
|
With the tests from @SemjonKerner as done in #11138 I was still able to trigger incorrect IFS issues with this code. The issue that caused this was that the timer is initialized running. The new fixup commit inserts a @SemjonKerner could you validate the code here with both ping (gnrc_networking) and with txtsnd? It should behave correctly timingwise except for the issue fixed by #11138 |
|
Ping and txtsend with small and large frames still work. Timings look good now. |
d89db8d to
413d3b0
Compare
|
squashed, thanks for the review! |
|
#11138 came between, please rebase. Sorry for the inconvenience. |
ieee802.15.4 specifies 40 symbols as LIFS value and 12 symbols as SIFS value. Furthermore, the 2.4Ghz DSSS mode has a symbol rate of 62.5Ksymbols/s. To have the LIFS and SIFS in the code match the timings from the specification, the TIMER_FREQ must match the symbol rate of 62.5Ksymbol/s such that one tick of the timer equals one symbol in time.
The timer_clear function doesn't clear the hardware timer counter, but is designed to clear the allocation of the channel. The consequence is that the IFS timer here is not set to zero in the callback, but only stopped at the current value. When the timer is started again, it has to count the full timer range until it matches the timeout value again. This commit fixes this issue by using timer_set instead of timer_set_absolute. This way the current timer value (when the timer is stopped) is read and the IFS timeout value is added to the current timer value.
413d3b0 to
658fb06
Compare
Rebased! |
|
Great, thanks for your contribution :) |
Contribution description
This PR fixes two issues with the IFS timing in the nrf802154 radio driver.
I've used the following patch to measure timings of the radio IFS timer:
The GPIO is set before the timer is started and cleared in the interrupt.
Using this at the current master results in the following timing (LIFS value):

measured on the nrf52840 using
examples/gnrc_networkingwithThe payload size of 200B is the most important as it causes the packet to be split into multiple frames.
The correct timing for the LIFS should be:
giving us 40 symbols / 62.5Ksymbol/s = 640μs.
This doesn't match the current measured timeout value.
Fix
The fix here is twofold. The first commit is modifies the
TIMER_FREQto match the symbol rate as specified in the ieee 802.15.4 specification. With this a timer tick and an ieee 802.15.4 symbol are equal in duration. TheSIFSandLIFSdefines then also match their durations in symbols.The second issue is caused by timer_clear function. It doesn't clear the hardware timer counter, but
is designed to clear the allocation of the channel. The consequence is that the IFS timer here is not set to zero in the callback, but only stopped at the current value. When the timer is started again, it has to
count the full timer range until it matches the timeout value again. With the timer never reset to zero, it doesn't count the
LIFS/SIFSduration but the timer range *TIMER_FREQduration (1024 μs in the intial measurement).Measuring again with the same setup as above gives:

Matching the calculated LIFS value from the spec.
Testing procedure
I won't blame anyone for not reproducing my measurement with their own logic analyzer. Testing that ping with large payloads (multiple L2 frames) still work should IMHO be enough. Ping response timings should decrease slightly (121ms vs 117ms average for 1000B payload).
Issues/PRs references
None