cpu/nrf52/radio/nrf802154: fix radio after soft ACK#21786
Conversation
github-actions
Bot
added
Platform: ARM
|
This does appear to fix the issue on my NRF. I did not test the 802154 stack directly but I can see that the side-effect (my app not working at all the cpu is stuck in a busy loop in the 802154 stack) is gone and I can use riot as expected. |
This comment was marked as duplicate.
This comment was marked as duplicate.
mguetschow
left a comment
mguetschow
left a comment
There was a problem hiding this comment.
Thanks for your proposed fix! Could you elaborate a bit on what the problem was and why it can be fixed that way? I'm not familiar with the nrf52 radio driver, but the changes look a bit like black magic to me.
I did not notice that there are 2 timers in the driver. One previously for sending an ACK and the other for IFS. I removed both. |
crasbe
added
Type: bug
Sounds plausible, although I would rather have expected an assertion failure in that case. |
|
I tested an |
|
There is also for example no duplicate when the nrf does not do CSMA for the ACK: |
|
Wait is this only a work around when no CSMA is supported by the driver? I dont know why to wait for a random backoff when sending an ACK |
|
When transmitting an ACK we have to wait |
|
I would propose to fix the ACK very simple like in the commit above to not delay the release. |
That's properly the right way to go, fine with me. But could you please add a todo to the respective part in the code that will need to be adapted in the follow-up PR so we don't loose track of it? As said before, I'd appreciate some git history cleaning separating left-over fixes from the actual fix with the timers. And this is not supposed to be a draft anymore, right? I've tested ICMP pings between |
fabian18
force-pushed
the
pr/fix_nrf52_after_soft_ack
branch
from
a83212c to
2a32d82
Compare
|
I do now see duplicate responses to a multicast ping with This is with two instances of For the (optional) 
Looks fine on |
|
I opened the two But what is the command I have to use for |
|
Hm what could it be ... I tried to reproduce with border router and gnrc networking as in the original soft ACK PR. |
|
Do we have to simulate air time? Would the transmission delay else be almost 0? |
You can just run
That might be it! |
|
Without the simulated airtime there is not DUP anymore as expected. So can I just drop it or do I have to create and opt-in pseudomodule? |
|
Do you mean remove the simulated airtime for ACKs on socket_zep? |
|
Only for ACK? I intended in general. |
|
Hm do you think it would be better to add this to |
|
I don`t understand what the difference would be when the artificial delay is moved to the dispatch program. |
|
The Problem seems to be that |
|
The TX state would be 3, but I only see the switch to IDLE from 0 1 2. I get 2 duplicates but no assert trigger on either side.. |
|
How about --- i/cpu/native/socket_zep/socket_zep.c
+++ w/cpu/native/socket_zep/socket_zep.c
@@ -516,9 +516,16 @@ static int _request_transmit(ieee802154_dev_t *dev)
dev->cb(dev, IEEE802154_RADIO_INDICATION_TX_START);
- /* delay transmission to simulate airtime */
- zepdev->ack_timer.callback = _send_frame;
- ztimer_set(ZTIMER_USEC, &zepdev->ack_timer, time_tx);
+ /* native overhead prevents short timers from triggering in time, send directly if delay is less than 200 µs */
+ if (time_tx <= 200) {
+ _send_frame(zepdev->ack_timer.arg);
+ } else {
+ time_tx -= 200;
+
+ /* delay transmission to simulate airtime */
+ zepdev->ack_timer.callback = _send_frame;
+ ztimer_set(ZTIMER_USEC, &zepdev->ack_timer, time_tx);
+ }
return 0;
}It's hacky, but |
github-actions
Bot
added
the
Platform: native
6 participants
Contribution description
Fix
nrf52driver after submac software ACK.Testing procedure
gnrc_networkingwith anynrf52board and any other 802.15.4 transceiver.Issues/PRs references
Bug introduced with #21533
fixes #21782