refactor: migrate to stopListening(txAddress) from openWritingPipe() (#1030)

follow up to #1029

`stopListening()` now writes the TX address to pipe 0 (for reading and writing). This deprecates the need for `openWritingPipe()` by offering an overloaded `stopListening(txAddress)`.

All examples (and the python wrapper) have been updated to use the new public `stopListening(txAddress)` accordingly.

The main take away is no duplicate SPI transactions when switching to TX mode and (re)setting the TX address. This aims to fix the performance regression in #1029.

---------

Co-authored-by: TMRh20 <tmrh20@gmail.com>

Author: 2bndy5

.github/workflows/doxygen.yml

@@ -50,5 +50,5 @@ jobs:
     uses: nRF24/.github/.github/workflows/build_docs.yaml@main
     with:
       deploy-gh-pages: ${{ github.event_name == 'release' || (github.event_name == 'workflow_dispatch' && github.ref == 'refs/heads/master') }}
-      doxygen-version: '1.12.0'
+      doxygen-version: '1.13.2'
     secrets: inherit

.readthedocs.yaml

@@ -15,7 +15,7 @@ build:
   commands:
     # Install doxygen from source distributions (conda forge does not keep up-to-date doxygen releases)
     - >
-      DOXYGEN_VERSION="1.12.0" &&
+      DOXYGEN_VERSION="1.13.2" &&
       mkdir .doxygen && cd .doxygen &&
       echo $(pwd) &&
       echo "https://sourceforge.net/projects/doxygen/files/rel-$DOXYGEN_VERSION/doxygen-$DOXYGEN_VERSION.linux.bin.tar.gz" &&

RF24.cpp

@@ -603,7 +603,6 @@ void RF24::_init_obj()
     _spi = &SPI;
 #endif // defined (RF24_SPI_PTR)
 
-    pipe0_reading_address[0] = 0;
     if (spi_speed <= 35000) { //Handle old BCM2835 speed constants, default to RF24_SPI_SPEED
         spi_speed = RF24_SPI_SPEED;
     }
@@ -1192,6 +1191,24 @@ void RF24::stopListening(void)
 
 /****************************************************************************/
 
+void RF24::stopListening(const uint64_t txAddress)
+{
+    memcpy(pipe0_writing_address, &txAddress, addr_width);
+    stopListening();
+    write_register(TX_ADDR, pipe0_writing_address, addr_width);
+}
+
+/****************************************************************************/
+
+void RF24::stopListening(const uint8_t* txAddress)
+{
+    memcpy(pipe0_writing_address, txAddress, addr_width);
+    stopListening();
+    write_register(TX_ADDR, pipe0_writing_address, addr_width);
+}
+
+/****************************************************************************/
+
 void RF24::powerDown(void)
 {
     ce(LOW); // Guarantee CE is low on powerDown
@@ -1626,12 +1643,14 @@ void RF24::openReadingPipe(uint8_t child, uint64_t address)
 
     if (child <= 5) {
         // For pipes 2-5, only write the LSB
-        if (child < 2) {
-            write_register(pgm_read_byte(&child_pipe[child]), reinterpret_cast<const uint8_t*>(&address), addr_width);
-        }
-        else {
+        if (child > 1) {
             write_register(pgm_read_byte(&child_pipe[child]), reinterpret_cast<const uint8_t*>(&address), 1);
         }
+        // avoid overwriting the TX address on pipe 0 while still in TX mode.
+        // NOTE, the cached RX address on pipe 0 is written when startListening() is called.
+        else if (static_cast<bool>(config_reg & _BV(PRIM_RX)) || child != 0) {
+            write_register(pgm_read_byte(&child_pipe[child]), reinterpret_cast<const uint8_t*>(&address), addr_width);
+        }
 
         // Note it would be more efficient to set all of the bits for all open
         // pipes at once.  However, I thought it would make the calling code
@@ -1668,12 +1687,14 @@ void RF24::openReadingPipe(uint8_t child, const uint8_t* address)
     }
     if (child <= 5) {
         // For pipes 2-5, only write the LSB
-        if (child < 2) {
-            write_register(pgm_read_byte(&child_pipe[child]), address, addr_width);
-        }
-        else {
+        if (child > 1) {
             write_register(pgm_read_byte(&child_pipe[child]), address, 1);
         }
+        // avoid overwriting the TX address on pipe 0 while still in TX mode.
+        // NOTE, the cached RX address on pipe 0 is written when startListening() is called.
+        else if (static_cast<bool>(config_reg & _BV(PRIM_RX)) || child != 0) {
+            write_register(pgm_read_byte(&child_pipe[child]), address, addr_width);
+        }
 
         // Note it would be more efficient to set all of the bits for all open
         // pipes at once.  However, I thought it would make the calling code
@@ -2037,6 +2058,11 @@ void RF24::stopConstCarrier()
     powerDown(); // per datasheet recommendation (just to be safe)
     write_register(RF_SETUP, static_cast<uint8_t>(read_register(RF_SETUP) & ~_BV(CONT_WAVE) & ~_BV(PLL_LOCK)));
     ce(LOW);
+    flush_tx();
+    if (isPVariant()) {
+        // restore the cached TX address
+        write_register(TX_ADDR, pipe0_writing_address, addr_width);
+    }
 }
 
 /****************************************************************************/

RF24.h

@@ -379,13 +379,16 @@ class RF24
      * @warning When the ACK payloads feature is enabled, the TX FIFO buffers are
      * flushed when calling this function. This is meant to discard any ACK
      * payloads that were not appended to acknowledgment packets.
-     *
-     * @note For auto-ack purposes, the TX address passed to openWritingPipe() will be restored to
-     * RX pipe 0. This still means that `stopListening()` shall be called before
-     * calling openWritingPipe() because the TX address is cached in openWritingPipe().
      */
     void stopListening(void);
 
+    /**
+     * @brief Similar to startListening(void) but changes the TX address.
+     * @param txAddress The new TX address.
+     * This value will be cached for auto-ack purposes.
+     */
+    void stopListening(const uint8_t* txAddress);
+
     /**
      * Check whether there are bytes available to be read
      * @code
@@ -523,6 +526,8 @@ class RF24
      * New: Open a pipe for writing via byte array. Old addressing format retained
      * for compatibility.
      *
+     * @deprecated Use `RF24::stopListening(uint8_t*)` instead.
+     *
      * Only one writing pipe can be opened at once, but this function changes
      * the address that is used to transmit (ACK payloads/packets do not apply
      * here). Be sure to call stopListening() prior to calling this function.
@@ -2022,6 +2027,17 @@ class RF24
      */
     void whatHappened(bool& tx_ok, bool& tx_fail, bool& rx_ready);
 
+    /**
+     * Similar to startListening(void) but changes the TX address.
+     *
+     * @deprecated Use stopListening(const uint8_t*) instead.
+     * See our [migration guide](migration.md) to understand what you should update in your code.
+     *
+     * @param txAddress The new TX address.
+     * This value will be cached for auto-ack purposes.
+     */
+    void stopListening(const uint64_t txAddress);
+
 private:
     /**@}*/
     /**

docs/migration.md

@@ -30,22 +30,34 @@ if radio.available() { /* .. */ }
 
 </td></tr></table>
 
-## openReadingPipe(uint8_t, uint64_t) and openWritingPipe(uint64_t)
+## 64-bit integer addresses
 
-> **Deprecated since v1.3.11**
+Any function that accept an address in the form of `uint64_t` is discouraged. This includes
 
-These functions' address parameter used a 64-bit unsigned integer (`uint64_t`).
+- `RF24::openReadingPipe(uint8_t, uint64_t)`
+   > **Deprecated since v1.3.11**
+- `RF24::openWritingPipe(uint64_t)`
+   > **Deprecated since v1.3.11**
+- `RF24::stopListening(const uint64_t)`
+   > **Deprecated since v1.5**
+
+These functions' address parameter use a 64-bit unsigned integer (`uint64_t`).
 The nRF24L01 can only use up to 40 bit addresses.
-Thus, there was an unused 24 bits being allocated for addresses using this function.
+Thus, there is an unused 24 bits being allocated for addresses using these functions.
 
 There are overloaded functions that use a buffer instead:
 
 - `RF24::openReadingPipe(uint8_t, const uint8_t*)`
 - `RF24::openWritingPipe(const uint8_t*)`
+- `RF24::stopListening(const uint8_t*)`
 
 These eliminate the unnecessary 24 bits by only using the length of the buffer (`uint8_t*`)
 specified by `RF24::setAddressWidth()`.
 
+@see The `RF24::openWritingPipe(const uint8_t*)` is now deprecated in favor of the
+overloaded `RF24::stopListening(const uint8_t*)` function.
+See the section below for more detail.
+
 > [!CAUTION]
 > The endianness (byte order) of a buffer is reversed compared to a 64-bit integer.
 > ```c
@@ -177,6 +189,7 @@ The aptly named `RF24::clearStatusFlags()` is designed to be a replacement for `
 Like `RF24::clearStatusFlags()`, `RF24::setStatusFlags()` takes 1 parameter whose value is defined by
 the `rf24_irq_flags_e` enumerated constants. These constant values specify individual flags;
 they can also be OR'd together to specify multiple flags.
+
 Additionally, `RF24::clearStatusFlags()` returns the STATUS byte containing the flags that
 caused the IRQ pin to go active LOW.
 This allows the user code to allocate less memory when diagnosing the IRQ pin's meaning.
@@ -211,3 +224,64 @@ radio.clearStatusFlags(RF24_RX_DR);
 ```
 
 </td></tr></table>
+
+## openWritingPipe(const uint8_t*)
+
+> Deprecated since v1.5
+
+Originally, `RF24::openWritingPipe(const uint8_t*)` was just a compliment to
+`RF24::openReadingPipe()`.
+It changes the address on pipe 0 because that is the only pipe that can be
+used for transmitting.
+
+Unfortunately, there was a bug that prevented the given TX address from being
+persistent on pipe 0 if the user code also set an RX address to pipe 0.
+This bug would surface when switching between RX mode and TX mode (via
+`RF24::startListening()` and `RF24::stopListening()` respectively) or after
+`RF24::stopConstCarrier()` (if `RF24::isPVariant()` returns `true`).
+
+The solution is to cache the TX address on the `RF24` instance.
+Consequently, this solution did not fit well with the traditional order of
+functions used to set up the radio's TX or RX mode.
+
+By overloading `RF24::stopListening(const uint8_t*)`, we are able to ensure proper radio
+setup without requiring certain functions are called in a certain order.
+
+- Use `RF24::stopListening(const uint8_t*)` to set the TX address and enter inactive TX mode.
+  `RF24::openReadingPipe()` can now (as of v1.5) be used in TX mode without consequence.
+- Use `RF24::stopListening()` to enter inactive TX mode without changing the TX address.
+
+> [!warning]
+> Avoid using pipe 0 for RX operations to improve performance and reliability.
+>
+> For implementation detail, see the source for `RF24::openReadingPipe()` and
+> `RF24::stopListening()`. Ultimately, the datasheet's Appendix A has a detailed
+> example outlining the order of a proper radio setup.
+
+<table><tr>
+<th>Old</th>
+<th>New (supported)</th>
+</tr><tr><td>
+
+```cpp
+// set TX address (pipe 0)
+radio.openWritingPipe(tx_address);
+
+// set RX address (pipe 1)
+radio.openReadingPipe(1, rx_address);
+
+// idle radio using inactive TX mode
+radio.stopListening();
+```
+
+</td><td>
+
+```cpp
+// set TX address (pipe 0)
+radio.stopListening(tx_address); // enters inactive TX mode
+
+// set RX address (pipe 1)
+radio.openReadingPipe(1, rx_address);
+```
+
+</td></tr></table>

examples/AcknowledgementPayloads/AcknowledgementPayloads.ino

@@ -84,18 +84,16 @@ void setup() {
   // this feature for all nodes (TX & RX) to use ACK payloads.
   radio.enableAckPayload();
 
-  // set the TX address of the RX node into the TX pipe
-  radio.openWritingPipe(address[radioNumber]);  // always uses pipe 0
+  // set the TX address of the RX node for use on the TX pipe (pipe 0)
+  radio.stopListening(address[radioNumber]);  // put radio in TX mode
 
   // set the RX address of the TX node into a RX pipe
   radio.openReadingPipe(1, address[!radioNumber]);  // using pipe 1
 
   // additional setup specific to the node's role
   if (role) {
     // setup the TX payload
-
     memcpy(payload.message, "Hello ", 6);  // set the payload message
-    radio.stopListening();                 // put radio in TX mode
   } else {
     // setup the ACK payload & load the first response into the FIFO
 

examples/GettingStarted/GettingStarted.ino

@@ -74,16 +74,14 @@ void setup() {
   // number of bytes we need to transmit a float
   radio.setPayloadSize(sizeof(payload));  // float datatype occupies 4 bytes
 
-  // set the TX address of the RX node into the TX pipe
-  radio.openWritingPipe(address[radioNumber]);  // always uses pipe 0
+  // set the TX address of the RX node for use on the TX pipe (pipe 0)
+  radio.stopListening(address[radioNumber]);  // put radio in TX mode
 
   // set the RX address of the TX node into a RX pipe
   radio.openReadingPipe(1, address[!radioNumber]);  // using pipe 1
 
-  // additional setup specific to the node's role
-  if (role) {
-    radio.stopListening();  // put radio in TX mode
-  } else {
+  // additional setup specific to the node's RX role
+  if (!role) {
     radio.startListening();  // put radio in RX mode
   }
 

examples/InterruptConfigure/InterruptConfigure.ino

@@ -102,20 +102,15 @@ void setup() {
   // Acknowledgement packets have no payloads by default. We need to enable
   // this feature for all nodes (TX & RX) to use ACK payloads.
   radio.enableAckPayload();
-  // Fot this example, we use the same address to send data back and forth
 
-  // set the TX address of the RX node into the TX pipe
-  radio.openWritingPipe(address[radioNumber]);  // always uses pipe 0
+  // set the TX address of the RX node for use on the TX pipe (pipe 0)
+  radio.stopListening(address[radioNumber]);  // put radio in TX mode
 
   // set the RX address of the TX node into a RX pipe
   radio.openReadingPipe(1, address[!radioNumber]);  // using pipe 1
 
-  // additional setup specific to the node's role
-  if (role) {
-    // setup for TX mode
-    radio.stopListening();  // put radio in TX mode
-
-  } else {
+  // additional setup specific to the node's RX role
+  if (!role) {
     // setup for RX mode
 
     // let IRQ pin only trigger on "data_ready" event in RX mode

examples/ManualAcknowledgements/ManualAcknowledgements.ino

@@ -88,8 +88,8 @@ void setup() {
   // number of bytes we need to transmit a float
   radio.setPayloadSize(sizeof(payload));  // char[7] & uint8_t datatypes occupy 8 bytes
 
-  // set the TX address of the RX node into the TX pipe
-  radio.openWritingPipe(address[radioNumber]);  // always uses pipe 0
+  // set the TX address of the RX node for use on the TX pipe (pipe 0)
+  radio.stopListening(address[radioNumber]);  // put radio in TX mode
 
   // set the RX address of the TX node into a RX pipe
   radio.openReadingPipe(1, address[!radioNumber]);  // using pipe 1
@@ -98,7 +98,6 @@ void setup() {
     // setup the TX node
 
     memcpy(payload.message, "Hello ", 6);  // set the outgoing message
-    radio.stopListening();                 // put radio in TX mode
   } else {
     // setup the RX node
 

examples/MulticeiverDemo/MulticeiverDemo.ino

@@ -27,13 +27,13 @@ RF24 radio(CE_PIN, CSN_PIN);
 // an identifying device destination
 // Notice that the last byte is the only byte that changes in the last 5
 // addresses. This is a limitation of the nRF24L01 transceiver for pipes 2-5
-// because they use the same first 4 bytes from pipe 1.
-uint64_t address[6] = { 0x7878787878LL,
-                        0xB3B4B5B6F1LL,
-                        0xB3B4B5B6CDLL,
-                        0xB3B4B5B6A3LL,
-                        0xB3B4B5B60FLL,
-                        0xB3B4B5B605LL };
+// because they use the same first 4 MSBytes from pipe 1.
+uint8_t address[6][5] = { { 0x78, 0x78, 0x78, 0x78, 0x78 },
+                          { 0xF1, 0xB6, 0xB5, 0xB4, 0xB3 },
+                          { 0xCD, 0xB6, 0xB5, 0xB4, 0xB3 },
+                          { 0xA3, 0xB6, 0xB5, 0xB4, 0xB3 },
+                          { 0x0F, 0xB6, 0xB5, 0xB4, 0xB3 },
+                          { 0x05, 0xB6, 0xB5, 0xB4, 0xB3 } };
 
 // role variable is used to control whether this node is sending or receiving
 char role = 'R';  // integers 0-5 = TX node; character 'R' or integer 82 = RX node
@@ -183,9 +183,8 @@ void setRole() {
     payload.nodeID = role;
     payload.payloadID = 0;
 
-    // Set the address on pipe 0 to the RX node.
-    radio.stopListening();  // put radio in TX mode
-    radio.openWritingPipe(address[role]);
+    // set the TX address of the RX node for use on the TX pipe (pipe 0)
+    radio.stopListening(address[role]);  // put radio in TX mode
 
     // According to the datasheet, the auto-retry features's delay value should
     // be "skewed" to allow the RX node to receive 1 transmission at a time.