drivers/at86rf2xx: Provide confirm_send() #16273
Conversation
- make use of `netdev_driver_t::send_confirm` when not `NULL`
- assume when `send_confirm` is `NULL` that `send()` blocks until completion
- otherwise, `gnrc_netif will block via `core_thread_flags` until the TX
completed event is issued, while also handling any IRQs raised during TX
- use `core_thread_flags` instead of `core_msg` to pass on events to the netif
thread
- this makes sure IRQs are no longer lost, making `gnrc_netif_events`
obsolete. Consequently, it is removed
- this allows any type of event being issues for IRQ context, which is
beneficial for peripheral drivers
fabian18
requested review from
cgundogan,
daniel-k,
haukepetersen,
jia200x,
miri64,
PeterKietzmann and
smlng
as code owners
fabian18
changed the title
drivers/include/at86rf2xx.h
Outdated
| @@ -251,6 +251,7 @@ typedef struct { | |||
| uint16_t flags; /**< Device specific flags */ | |||
| uint8_t state; /**< current state of the radio */ | |||
| uint8_t tx_frame_len; /**< length of the current TX frame */ | |||
| uint8_t status; /**< Tx/Rx status (TRAC_STATUS) */ | |||
There was a problem hiding this comment.
Why is this value kept in the device descriptor?
The TRAC_STATUS only reports the status of TX with automatic retransmissions. This can be read directly on TX_DONE (as it was before)
There was a problem hiding this comment.
In confirm_send() the status of the last transmission is evaluated, and in the _isr() the status is saved.
If TRAC_STATUS was not needed in _isr(), then I guess I could also read the register in confirm_send() and not save it.
But looking at the code of openthread_netdev.c :: _event_cb(), the event is passed to send_pkt() which distinguishes between cca failure, no ACK and no error. So that´s why I kept the #ifdef MODULE_OPENTHREAD block inside _isr().
There was a problem hiding this comment.
We could do the same in _isr when #ifdef MODULE_OPENTHREAD. As mentioned earlier, this status only tells what was the result of TX_ARET (it's not even read in BASIC_MODE).
Besides that, we could drop these lines if we do a minor adoption to the event callbacks of netdev. At least GNRC is able to differentiate TX_COMPLETE from TX_COMPLETE_DATA_PENDING.
There was a problem hiding this comment.
Note that the new API deprecates events such as TX_COMPLETE_DATA_PENDING and only confirm_send() is used to pass over additional info/flags via the auxiliary info field.
Maybe it makes sense to let this PR also depend on an update of openthread?
There was a problem hiding this comment.
Or I could add a commit on top.
If the return value of confirm_send() is passed to the ot send_pkt() function and is properly evaluated, it should be the same behavior.
There was a problem hiding this comment.
If the change is not too large, that would be indeed better. Without an updated driver, it is not possible to test the new feature anyway.
| bool at86rf2xx_is_busy(const at86rf2xx_t *dev) | ||
| { | ||
| uint8_t s; | ||
| while ((s = at86rf2xx_get_status(dev)) == AT86RF2XX_STATE_IN_PROGRESS) {} |
There was a problem hiding this comment.
here you could treat "STATE_IN_PROGRESS" as busy. In that case you could simply add this case to the if below
There was a problem hiding this comment.
Alright, @maribu suggested the same.
jeandudey
added
Area: drivers
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions. |
stale
bot
added
the
State: stale
stale
bot
removed
the
State: stale
github-actions
bot
added
Area: build system
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions. |
stale
bot
added
the
State: stale
Contribution description
sys/net/gnrc/netif/ieee802154to allow IEEE 802154 drivers to provide the new netdev APIdrivers/at86rf2xxto provideconfirm_send()Requires
USEMODULE += gnrc_netif_pktq.Testing procedure
examples/gnrc_networkingping6
Issues/PRs references
Depends on: