nrf-rs / nrf-hal

A Rust HAL for the nRF family of devices
Apache License 2.0
503 stars 139 forks source link

Added non-blocking radio receive #485

Open eivindbergem opened 1 month ago

eivindbergem commented 1 month ago

I've added a simple non-blocking receive. The method returns a struct that takes care of checking status and calling cancel_recv(). Now it calls cancel_recv() for every drop. I can add some state to avoid this if needed.

I didn't put much thought into the names, so please feel free to bikeshed.

BartMassey commented 1 month ago

Thanks much for the contribution!

Sorry about the CI failure, which is new: PR #486 should fix it shortly, after which you'll need to rebase.

Should this be adapted to support the nb crate infrastructure?

eivindbergem commented 4 weeks ago

Should this be adapted to support the nb crate infrastructure?

Yes, that's a good idea :) I'll get on to it.

eivindbergem commented 4 weeks ago

If is_done() is called after having received an Ok() or a CRC error, you would get a WouldBlock for ever:

let recv = radio.recv_non_blocking(&mut packet);
recv.is_done(); // -> WouldBlock
recv.is_done(); // -> Ok(crc)
while recv.is_done() == Err(nb::Error::WouldBlock) {} // Would never leave loop

This is not great. I could add some internal state and another error to indicate that we already have received:

enum RecvError {
    CrcFailure(u16),
    AlreadyReceived,
}

Or, maybe subsequent calls to is_done() should just repeatedly return Ok(crc) or Err(nb::Error::Other(crc)).

BartMassey commented 4 weeks ago

Is this with nb or without? I've lost track.

eivindbergem commented 4 weeks ago

This applies to both, with and without nb.

I'm not sure if it matters that much anyway, I was just thinking out loud. Futures suffer from the same issue, and the documentation makes it clear that futures that have returned Ready should not be polled.

eivindbergem commented 4 weeks ago

I refactored recv() and recv_timeout() to use recv_non_blocking() under the hood. I removed Event::End as it is no longer used. The enum only has one member now, so not sure if it's needed anymore.