Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider abstracting config out of radio instance #5

Open
2bndy5 opened this issue Nov 2, 2024 · 1 comment · May be fixed by #1
Open

Consider abstracting config out of radio instance #5

2bndy5 opened this issue Nov 2, 2024 · 1 comment · May be fixed by #1
Labels
enhancement New feature or request

Comments

@2bndy5
Copy link
Member

2bndy5 commented Nov 2, 2024

This idea will surely consume more memory. The advantage is an easy way to reconfigure the radio using a Config object instead of using) multiple configuration functions on the RF24 (radio) object.

Proposal

I'm probably explaining this wrong, so consider the following pseudo code:

use rf24::radio::Config;
use rf24::PaLevel;

let config = Config::default()
    .auto_retries(3, 15) // delay = 3 * 250 + 250; count = 15
    .pa_level(PaLevel::Low);

let radio = RF24(ce_pin, spi_dev, delay_impl);
radio.init(); // init radio with lib defaults
// re-configure radio using the user config object
radio.with_config(&config); // borrowing not owning config object

This allows the same radio to be easily and quickly reconfigured for different user-defined protocols.

For example, lets say someone wants to switch between different network configurations. Instead of

// network A config
radio.set_data_rate(DataRate::Kbps250);
radio.set_payload_length(16);
radio.open_rx_pipe(1, b"1Node");

// network B config
radio.set_data_rate(DataRate::Mbps1);
radio.set_payload_length(32);
radio.open_rx_pipe(1, [0xC5, 0xC4, 0xC3, 0xC2, 0xBA]);

Users could instead cache a config object for each network config and re-use those objects to switch between the networks:

let config_a = Config::default()
    .set_data_rate(DataRate::Kbps250);
    .set_payload_length(16);
    .open_rx_pipe(1, b"1Node");

let config_b = Config::default()
    .set_data_rate(DataRate::Mbps1);
    .set_payload_length(32);
    .open_rx_pipe(1, [0xC5, 0xC4, 0xC3, 0xC2, 0xBA]);

// change configuration for network A
radio.with_config(&config_a);
// ..

// change configuration for network B
radio.with_config(&config_b);
// ...

Additional Context

I already have been using this approach in the CirPy lib using python's with statement blocks (AKA context managers). See CirPy lib's context example and tutorial on participating in multiple networks with 1 radio.

There's seems to be a convention in rust in which a builder structure is preferred over a multitude of config functions or constructor parameters.

@2bndy5 2bndy5 added the enhancement New feature or request label Nov 2, 2024
@2bndy5 2bndy5 linked a pull request Nov 8, 2024 that will close this issue
8 tasks
@2bndy5
Copy link
Member Author

2bndy5 commented Nov 8, 2024

Conserving memory footprint

In exploring this idea, I found a handy library called bitfield-struct. It is an interesting way to serialize bits of a register into fields of a struct. I ended up using this to replace magic numbers with const offsets/masks. For instance, the STATUS byte that is cached from every SPI transaction is now represented by a 1-byte struct with fields that describe each bit:

// within RF24 struct as private API

// capture and decode the STATUS byte
self._status = StatusFlags::from_bits(self._buf[0]);

// now access each flag as a field
self._status.tx_full() // -> bool
self._status.rx_pipe() // -> u8 ranging [0, 7]
self._status.tx_ds() // -> bool
// ...

// the raw status byte can still be accessed
self._status.into_bits() & 0x70 // isolate IRQ flags

I did the same to the internal cached CONFIG register as well...

This helped a lot to reduce the memory footprint of a EsbConfig struct. By my calculations the nRF24L01's register (excluding addresses) only occupy a maximum 64 bits (including padding). The addresses unavoidably consume 19 bytes.

This bitfield-struct lib was so impressive that I came up with the idea to transform enumerations into masks when writing them to registers:

CrcLength::Bit16.into_bits() // -> 0x0C

Config API

As proposed, I landed on the following API exposed to end-users:

let config = EsbConfig::default() // uses library defaults akin to C++ RF24 lib
    .with_auto_ack(0b11) // enable auto-ack on pipes 0 and 1
    .with_rx_dr(false) // disable RX data ready event on IRQ pin
    .with_ack_payloads(true) // enables dynamic payloads also
    .with_crc_length(CrcLength::Bit8)
    // set address for RX pipe 1 (and opens the pipe)
    .with_rx_address(1, &b"1Node");

// when ready to use the cached config
let mut radio = RF24::new(ce_obj, spi_dev_obj, delay_ns_impl);
radio.init()?;
radio.with_config(&config)?;

The EsbConfig struct also features getters, but I doubt that would be as useful to users; they're useful to implement RF24::with_config(). Also RF24::init() will now

  1. check if is_plus_variant
  2. check for binary corruption on SPI lines
  3. call/return RF24::with_config(&EsbConfig::default())

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant