Prevent some leaks & a bit of cleanup #306
Conversation
bugadani
force-pushed
the
leak
branch
3 times, most recently
from
b3ad89b to
a829d22
Compare
| res | ||
| _ => { | ||
| debug!("RX QUEUE FULL"); | ||
| include::ESP_ERR_NO_MEM as esp_err_t |
There was a problem hiding this comment.
we should still do esp_wifi_internal_free_rx_buffer(eb);
There was a problem hiding this comment.
It's done by EspWifiPacketBuffer::drop, or at least that is the intention.
There was a problem hiding this comment.
oh I see .... I should stop reviewing code via GH UI
There was a problem hiding this comment.
And I should probably start describing my changes in more detail so you have the answers before you need to ask 😅
| @@ -1303,6 +1306,8 @@ pub(crate) mod embassy { | |||
| cx: &mut core::task::Context, | |||
| ) -> Option<(Self::RxToken<'_>, Self::TxToken<'_>)> { | |||
| RECEIVE_WAKER.register(cx.waker()); | |||
| TRANSMIT_WAKER.register(cx.waker()); | |||
There was a problem hiding this comment.
Why do we register the TRANSMIT_WAKER here?
There was a problem hiding this comment.
It's technically possible that we return None because we can't create a TxToken. TRANSMIT_WAKER is supposed to signal a change in this state. Since we are registering wakers all over the place regardless of whether they are actually needed, I was thinking "why not".
No description provided.