RIOT
RIOT copied to clipboard
gnrc_netif: remove NETDEV_MSG_TYPE_EVENT processing from thread
Contribution description
This PR is the first one for removing the netdev
dependency in gnrc_netif
.
It moves the NETDEV_EVENT_MSG_TYPE_EVENT handling to each netif->ops->msg_handler
. That way an implementation of gnrc_netif_ops_t
can use other mechanisms for handling device ISRs.
With some follow ups it will be possible to use gnrc_netif
without netdev
, so it's possible to directly hook drivers or other interfaces (radio HALs, etc).
Testing procedure
Besides Murdock compile tests, this should be tested to ensure all gnrc_netif_ops_t
implementation still work (gnrc_netif_ethernet, gnrc_netif_lorawan, etc).
Issues/PRs references
None so far
@jia200x please address or answer my comments. Otherwise, I would be fine with merging. @miri64 do you agree?
Yepp, AFAIR I did not test this yet, though.
Here is what I've done already. I'd wait with further tests until the code is in a "final" state
Is this PR waiting for something (besides more testing)?
Is this PR waiting for something (besides more testing)?
At least now for a rebase :-/.
some radio drivers are missing
I can confirm that ACK timeouts still work on at86rf215
which (ab)uses NETDEV_MSG_TYPE_EVENT
for ACK timeouts.
I'm happy to jettison this code when the new sub-Mac is merged :smile:
I think I addressed all comments
I'm happy to jettison this code when the new sub-Mac is merged :smile:
/me wonders if @benpicco knows that RIOT supports more MAC layers than the 802.15.4 one :thinking: :stuck_out_tongue_winking_eye:
Well I was hoping to get #14950 merged before any new sub-MACs pop up :wink:
esp_wifi
and atwinc15x0
are handled by gnrc_netif_ethernet.c
I assume?
Feel free to squash.
esp_wifi
andatwinc15x0
are handled bygnrc_netif_ethernet.c
I assume?
Sorry, after a couple of months with other tasks I am no longer up to date, but I guess you are right. atwinc15x0
is just a netdev
driver and esp_wifi
is using gnrc_netif_ethernet
.
What's the state of this?
It's still alive. I will reactivate it after next Friday
It's still alive. I will reactivate it after next Friday
I completely forgot about it. I just rebased it to current master.
We can work on this as well I suppose.
it's already rebased. It just needs reviewing
As is this is pretty uncontroversial: instead of directly handling the NETDEV_MSG_TYPE_EVENT
case in _gnrc_netif_thread
it's now handled by the default
case which then calls netif->ops->msg_handler
which then handles NETDEV_MSG_TYPE_EVENT
.
The only thing I don't understand is what we gain from this, other than slightly more overhead. The netdev
dependency is still there, just moved to gnrc_netif_msg_handler_netdev()
.
The only thing I don't understand is what we gain from this, other than slightly more overhead. The netdev dependency is still there, just moved to gnrc_netif_msg_handler_netdev().
It's just one of the steps required in order to remove the netdev dependency from gnrc_netif
.
Also, we can avoid compiling that part at all if we are using other mechanisms such as gnrc_netif_events
or thread flags.
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.
It looks like we were close to merging this - do you still want this in?
It looks like we were close to merging this - do you still want this in?
Yes! I just amended @maribu's comments.
This doesn't look scary to me, so I would insist on full testing.
I think there a "not" missing :)
CI is unhappy
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.
What's missing here?
hmmmm this became obsolete, as we already removed this msg mechanism via https://github.com/RIOT-OS/RIOT/pull/16748. Closing.