rename listen methods

[2bndy5]

Since I'm writing the API from scratch here, I think it is a good opportunity to consider changes that would be considered backward incompatible in C++. Namely, the startListening() and stopListening() methods.

We have had more than a few complaints/requests to adapt the RF24 API functions into something that makes more sense (in human terms of spoken English language). Although that C++ API is rather tied to what maniacBug originally wrote, this lib's API isn't carved in stone (yet).

Proposal

Rename the following methods

• start_listening() -> as_rx() (fallible -- meaning needs SPI transaction)
• stop_listening() -> as_tx() (fallible -- meaning needs SPI transaction)
• is_listening() -> is_rx() (infallible -- meaning data is returned from cached config register's value; no SPI transaction)

The idea is to make the API names as obvious as possible. Our experience, despite all efforts, has been that people would rather not read docs. And if a function's name needs additional explanation (especially for usage in examples), then the name was poorly chosen.

Additional context

I'm not exposing the CE pin for the instantiated radio object. The only case this might be useful is when using write() which is similar to C++ RF24::writeFast(). It would be nice to have a way to explicitly exit active TX mode by setting the CE pin LOW after checking transmission's outcome (using get_status_flags()). Currently, the only way to do that is to call stop_listening() while already in TX mode, but this seems non-intuitive.

[TMRh20]

Following along on your progress here...

This is a good idea, has me thinking about the RF24 library.

You could possibly even take it further and eliminate the need for as_rx() as_tx() by calling them based on is_listening() whenever respective available()or write() functions are called I think? Not sure if that's a good idea but thought I'd throw it out there.

[2bndy5]

Hmm, making as_tx() implicit kinda makes sense, but users would definitely need a way to explicitly enter RX mode. Remember, available() is also used to check for ACK payloads.

I did this sort of intuition in the Circuitpython lib, but I found the first write() would always take a lot longer to execute.

---

I also thought about having an enum and setting the role of the radio using the enum. But that API seems a bit too verbose.

pub enum Role {
    Rx,
    Tx,
}

// ... 

pub fn set_role(&mut self, role: Role) -> Result<(), Self::Error>;

pub fn get_role(&self) -> Role;

Where user code would look like:

radio.set_role(Role::Rx)?;

// or

if radio.get_role() == Role::Rx {
    radio.set_role(Role::Tx)?;
}

Furthermore, this whole concept revolves around the fact that the radio's role is a binary state, disregarding other nuances like changing roles or power consumption (standby-I/II modes). I don't think an enum solution is the best fit.

[2bndy5]

eliminate the need for as_rx() as_tx() by calling them based on is_listening() whenever respective available()or write() functions are called

I'm erring away from this.

1. My experience with this intuition in the CirPy lib is that the first transmission seems to take longer if radio is not already in TX mode. While this is ensuring proper usage of the radio, it may be perceived as unexpected behavior to users that are not familiar with the source code.
2. Calling as_tx() is practically required once the addresses are set to the RX/TX pipes. Currently, is_rx() only checks the PTX flag in the cached STATUS byte; it does not account for when a RX address is set to pipe 0; IIRC the CirPy lib's write() does check that as well.
3. Just to reiterate, I can't think of an adequate way to make entering RX mode intuitive because available() can be used regardless of the radio's current role.

I'm not ruling out this idea for added intuition. But I don't think it fits within idiomatic rust convention, where everything is explicit and error checking is always at the forefront.

With all of that said, I added a check in send() (which is similar to C++ RF24::write() without the added timeout) to check is_rx() to prevent an infinite loop when waiting for a change in the status flags (MAX_RT or TX_DS). I should probably make it return an Err(anyhow!("radio not in TX mode")) instead of an Ok(false).

rf24-rs/library/src/radio/rf24/radio.rs

Lines 138 to 155 in 9fc47fc

	fn send(&mut self, buf: &[u8], ask_no_ack: bool) -> Result<bool, Self::RadioErrorType> {
	if self.is_rx() {
	// check if in RX mode to prevent an infinite below
	return Ok(false);
	}
	self._ce_pin.set_low().map_err(Nrf24Error::Gpo)?;
	// this function only handles 1 payload at a time
	self.flush_tx()?; // flush the TX FIFO to ensure we are sending the given buf
	if !self.write(buf, ask_no_ack, true)? {
	return Ok(false);
	}
	self._delay_impl.delay_ns(10000);
	// now block until we get a tx_ds or tx_df event
	while self._status & 0x30 == 0 {
	self.spi_read(0, commands::NOP)?;
	}
	Ok(self._status & mnemonics::MASK_TX_DS == mnemonics::MASK_TX_DS)
	}

In rust, a Result::Err is elevated as a panic! (or an exception in the bindings) if not handled programmatically.