Skip to content

Conversation

@lulf
Copy link
Collaborator

@lulf lulf commented Sep 1, 2021

This decouples the TX and RX buffer more clearly in the radio API:

  • The TX buffer is now passed as a [u8] slice rather than using the
    associated radio type. This makes sense because the radio is only
    given access to this buffer in TxRequest event, in which case it is
    now handed a &[u8] rather than a custom type. It also now only has
    read access to the TX buffer.
  • The RX buffer is accessed by lorawan-device after an RxDone event, in
    which case it really only needs a &mut [u8] to parse the packet, and
    -device doesn't really change at all.
  • Introduce a RadioBuffer type thats used internally to wrap the
    provided TX buffer slice, with a way to conveniently append to the
    buffer.

There are lots of lifetime arguments being added, in order to allow the
TX buffer to be provided externally. I believe this will be useful in order to fix get_random to use
proper traits as well. There are some important performance benefits from these changes:

  • TX buffer allocation can be done externally to lorawan-device,
    which is crucial for keeping async future stack usage down because the
    size of the buffer will not impact the stack anymore.
  • Radio implementations are now free to decide how the RX buffer is
    managed as well, for the same reasons as above.

Although I would ideally like TX and RX to share buffer, I think that would better come in a later PR.

This decouples the TX and RX buffer more clearly:

* The TX buffer is now passed as a [u8] slice rather than using the
  associated radio type. This makes sense because the radio is only
  given access to this buffer in TxRequest event, in which case it is
  now handed a &[u8] rather than a custom type. It also now only has
  read access to the TX buffer.
* The RX buffer is accessed by lorawan-device after an RxDone event, in
  which case it really only needs a &mut [u8] to parse the packet, and
  -device doesn't really change at all.
* Introduce a RadioBuffer type thats used internally to wrap the
  provided TX buffer slice, with a way to conveniently append to the
  buffer.

There are lots of lifetime arguments being added, in order to allow the
TX buffer to be provided externally. There are some important
performance benefits from these changes:

* TX buffer allocation can be done externally to lorawan-device,
  which is crucial for keeping async future stack usage down because the
  size of the buffer will not impact the stack anymore.
* Radio implementations are now free to decide how the RX buffer is
  managed as well, for the same reasons as above.
Copy link
Collaborator

@lthiery lthiery left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a great change 👍

@ivajloip
Copy link
Collaborator

ivajloip commented Sep 7, 2021

Yes, that seems very nice! Thank you so much! 🙂

@ivajloip ivajloip merged commit d62faae into lora-rs:master Sep 7, 2021
@lulf lulf deleted the remove-buffer-type branch September 7, 2021 20:21
Dirbaio pushed a commit to Dirbaio/lora-rs that referenced this pull request Nov 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants