This post is older than 2 years and might not be relevant anymore
More Info: Consider searching for newer posts

Possible bug in the ESB implementation (nrf_esb.c) up to SDK 13.1?

Hi,

I am using ESB in one of our current projects. I have two devices: one PTX (master) and one PRX (slave). The slave may want to send some data to the master from time to time, but it can only do so if the master sends him a beacon. In the latter case, the data to be sent is packed onto the ACK package via piggybacking. In practice, the PRX device enqueues the data via nrf_esb_write_payload() and waits for the opportunity to send them.

My NRF_ESB_TX_FIFO_SIZE is set to 8. I noticed that if the PRX actually completely fills up its TX FIFO, i.e., it enqueues payloads 8 times between beacons, then the master will receive piggybacked data 9 instead of 8 times. Here's a diagram (time flows to the bottom) of what happens:

PTX ->   beacon   -> PRX (nothing enqueued)

                     PRX enqueues "PIG1"
                     PRX enqueues "PIG2"
                     PRX enqueues "PIG3"
                     PRX enqueues "PIG4"
                     PRX enqueues "PIG5"
                     PRX enqueues "PIG6"
                     PRX enqueues "PIG7"
                     PRX enqueues "PIG8"

PTX ->   beacon   -> PRX
PTX <- ack+"PIG1" <- PRX

PTX ->   beacon   -> PRX
PTX <- ack+"PIG2" <- PRX

PTX ->   beacon   -> PRX
PTX <- ack+"PIG3" <- PRX

PTX ->   beacon   -> PRX
PTX <- ack+"PIG4" <- PRX

PTX ->   beacon   -> PRX
PTX <- ack+"PIG5" <- PRX

PTX ->   beacon   -> PRX
PTX <- ack+"PIG6" <- PRX

PTX ->   beacon   -> PRX
PTX <- ack+"PIG7" <- PRX

PTX ->   beacon   -> PRX
PTX <- ack+"PIG8" <- PRX

PTX ->   beacon   -> PRX
PTX <- ack+"PIG1" <- PRX

After this point, every time PRX enqueues something, PTX will get the new data and one of the old payloads upon two consecutive beacons.

I tracked down the problem in nrf_esb.c and found a workaround for this in on_radio_disabled_rx(). Below an excerpt from the original file from SDK 11:

case NRF_ESB_PROTOCOL_ESB_DPL:
    {
        if (m_tx_fifo.count > 0 &&
            (m_tx_fifo.p_payload[m_tx_fifo.exit_point]->pipe == NRF_RADIO->RXMATCH))
        {
            // Pipe stays in ACK with payload until TX fifo is empty
            // Do not report TX success on first ack payload or retransmit
            if (p_pipe_info->m_ack_payload != 0 && !retransmit_payload)
            {
                if(++m_tx_fifo.exit_point >= NRF_ESB_TX_FIFO_SIZE)
                {
                    m_tx_fifo.exit_point = 0;
                }

                m_tx_fifo.count--;

                // ACK payloads also require TX_DS
                // (page 40 of the 'nRF24LE1_Product_Specification_rev1_6.pdf').
                m_interrupt_flags |= NRF_ESB_INT_TX_SUCCESS_MSK;
            }

              p_pipe_info->m_ack_payload = 1;

              mp_current_payload = m_tx_fifo.p_payload[m_tx_fifo.exit_point];

              update_rf_payload_format(mp_current_payload->length);
              m_tx_payload_buffer[0] = mp_current_payload->length;
              memcpy(&m_tx_payload_buffer[2],
                     mp_current_payload->data,
                     mp_current_payload->length);
        }
        else
        {
            p_pipe_info->m_ack_payload = 0;
            update_rf_payload_format(0);
            m_tx_payload_buffer[0] = 0;
        }

        m_tx_payload_buffer[1] = m_rx_payload_buffer[1];
    }
    break;

And here the changed version; new lines are marked with an asterisk (*):

case NRF_ESB_PROTOCOL_ESB_DPL:
    {
        if (m_tx_fifo.count > 0 &&
            (m_tx_fifo.p_payload[m_tx_fifo.exit_point]->pipe == NRF_RADIO->RXMATCH))
        {
            // Pipe stays in ACK with payload until TX fifo is empty
            // Do not report TX success on first ack payload or retransmit
            if (p_pipe_info->m_ack_payload != 0 && !retransmit_payload)
            {
                if(++m_tx_fifo.exit_point >= NRF_ESB_TX_FIFO_SIZE)
                {
                    m_tx_fifo.exit_point = 0;
                }

                m_tx_fifo.count--;

                // ACK payloads also require TX_DS
                // (page 40 of the 'nRF24LE1_Product_Specification_rev1_6.pdf').
                m_interrupt_flags |= NRF_ESB_INT_TX_SUCCESS_MSK;
            }

            if (m_tx_fifo.count > 0)                                                // *
            {                                                                       // *
              p_pipe_info->m_ack_payload = 1;

              mp_current_payload = m_tx_fifo.p_payload[m_tx_fifo.exit_point];

              update_rf_payload_format(mp_current_payload->length);
              m_tx_payload_buffer[0] = mp_current_payload->length;
              memcpy(&m_tx_payload_buffer[2],
                     mp_current_payload->data,
                     mp_current_payload->length);
            }                                                                       // *
            else                                                                    // *
            {                                                                       // *
              p_pipe_info->m_ack_payload = 0;                                       // *
              update_rf_payload_format(0);                                          // * 
              m_tx_payload_buffer[0] = 0;                                           // *
            }                                                                       // *
        }
        else
        {
            p_pipe_info->m_ack_payload = 0;
            update_rf_payload_format(0);
            m_tx_payload_buffer[0] = 0;
        }

        m_tx_payload_buffer[1] = m_rx_payload_buffer[1];
    }
    break;

With the above changes the code behaves as expected (and works for our purpose), but I am not sure if it still meets specifications.

The same problem is present in SDK 12 through 13.1, too.

Please let me know your thoughts on the issue. Thanks, Tamas

Related