TWIM and the proper utilization of the event handler

Question

I have been working on a C++ library for the BME280 sensor to be used within the Nordic SDK and am so far only focused on using TWIM, Within this library I intend to use TWIM in a non-blocking manner by utilizing an event handler. The code I will be including below is functional so the purpose of this post is not so much "how do I implement this" but more "am I implementing this correctly".

I have searched through devzone and found slightly different implementations of using the twi event handler, most of which seem to defeat the purpose by using an empty while loop. I have taken a look at the twi sensor example included with the SDK as well as a number of the external drivers included with the SDK, namely the mcp4725 driver. Here we find examples that repeatedly call a sensor read function in the main loop then wait for the event handler to set a transfer flag. The twi_sensor example seems to only care about the RX event being raised.

Fullscreen

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
// twi_sensor.c
int main
{
    while (true)
    {
        nrf_delay_ms(500);
        do
        {
            __WFE();
        }while (m_xfer_done == false);
        read_sensor_data();
        NRF_LOG_FLUSH();
    }
}
XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX

// twi_sensor.c
int main
{
    while (true)
    {
        nrf_delay_ms(500);

        do
        {
            __WFE();
        }while (m_xfer_done == false);

        read_sensor_data();
        NRF_LOG_FLUSH();
    }
}

If we look into mcp4725.c we find an example of an event handler that tracks both XFER_RX and XFER_TX event types but waits for the flags to be set in an empty wait loop on line 34:

Fullscreen

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
static void twi_handler(nrf_drv_twi_evt_t const * p_event, void * p_context)
{
    switch (p_event->type)
    {
        case NRF_DRV_TWI_EVT_DONE:
            if (p_event->xfer_desc.type == NRF_DRV_TWI_XFER_TX)
            {
                m_xfer_done = true;
            }
            if (p_event->xfer_desc.type == NRF_DRV_TWI_XFER_RX)
            {
                m_read_done = true;
            }
            break;
        default:
            break;
    }
}
ret_code_t mcp4725_set_voltage(uint16_t val, bool write_eeprom)
{
XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX

static void twi_handler(nrf_drv_twi_evt_t const * p_event, void * p_context)
{
    switch (p_event->type)
    {
        case NRF_DRV_TWI_EVT_DONE:
            if (p_event->xfer_desc.type == NRF_DRV_TWI_XFER_TX)
            {
                m_xfer_done = true;
            }
            if (p_event->xfer_desc.type == NRF_DRV_TWI_XFER_RX)
            {
                m_read_done = true;
            }
            break;
        default:
            break;
    }
}

ret_code_t mcp4725_set_voltage(uint16_t val, bool write_eeprom)
{
    /* Shift parameter val to get 2 8-bits values. */
    uint8_t reg[3] = {write_eeprom ? MCP4725_EEPROM_ADDRESS : MCP4725_DAC_ADDRESS,
                      (val>>4), (val<<4)};

    m_xfer_done = false;

    ret_code_t err_code = nrf_drv_twi_tx(&m_twi, MCP4725_BASE_ADDRESS, reg, sizeof(reg), false);
    if (err_code != NRF_SUCCESS)
    {
        return err_code;
    }

    while (m_xfer_done == false);

    return NRF_SUCCESS;
}

My first question is, is there any benefit to tracking m_xfer_done and m_read_done separately other than to make clear which transfer type finished?

As I currently have my event handler and readRegister functions implemented, I follow the lead of the mcp4725 driver above and keep track of both RX and TX event types. This works but I am wondering if it couldn't be equally effective if consolidated to a generic m_xfer_done variable.

Fullscreen

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
void BME280::twim_handler(const nrfx_twim_evt_t *p_event, void * p_context)
{
    switch (p_event->type)
    {
    case NRFX_TWIM_EVT_DONE:
        if (p_event->xfer_desc.type == NRFX_TWIM_XFER_TX)
        {
            BME280::m_tx_done = true;
        }
        if (p_event->xfer_desc.type == NRFX_TWIM_XFER_RX)
        {
            BME280::m_rx_done = true;
        }
        break;
    case NRFX_TWIM_EVT_ADDRESS_NACK:
        break;
    case NRFX_TWIM_EVT_DATA_NACK:
        break;
    default:
        break;
    }
XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX

void BME280::twim_handler(const nrfx_twim_evt_t *p_event, void * p_context)
{
    switch (p_event->type)
    {
    case NRFX_TWIM_EVT_DONE:
        if (p_event->xfer_desc.type == NRFX_TWIM_XFER_TX)
        {
            BME280::m_tx_done = true;
        }
        if (p_event->xfer_desc.type == NRFX_TWIM_XFER_RX)
        {
            BME280::m_rx_done = true;
        }
        break;
    case NRFX_TWIM_EVT_ADDRESS_NACK:
        break;
    case NRFX_TWIM_EVT_DATA_NACK:
        break;
    default:
        break;
    }
}

ret_code_t BME280::readRegister(uint8_t reg_addr, uint8_t *p_reg_data, uint16_t len)
{
    ret_code_t err_code;

    err_code = nrfx_twim_tx(&_twim, _dev_addr, &reg_addr, sizeof(reg_addr), true);
    if (err_code == NRF_SUCCESS)
    {
        do
        {
            __WFE();
        } while (!BME280::m_tx_done);
        BME280::m_tx_done = false;
    }

    err_code = nrfx_twim_rx(&_twim, _dev_addr, p_reg_data, len);
    if (err_code == NRF_SUCCESS)
    {
        do
        {
            __WFE();
        } while (!BME280::m_rx_done);
        BME280::m_rx_done = false;
    }

    return NRF_SUCCESS;
}

My second question is, would there be any benefit to using nrfx_twim_xfer() with a TXRX transfer description? I assume that would allow me to eliminate one wait loop from my readRegister function and only require handling the appropriate transfer type in the event handler. I am also assuming I would only need a single m_xfer_done flag to wait on.

Lastly, is the do { __WFE(); } while(!BME280::m_tx_done); all that is needed to keep these functions non-blocking? The key part being __WFE().

Susheel Nuguru · Accepted Answer

My first question is, is there any benefit to tracking m_xfer_done and m_read_done separately other than to make clear which transfer type finished? 
 I do not think that there is any extra benefit apart from keeping them separate. These TX and a new RX (Not TXRX) operations are asynchronous/non-blocking. What would happen if you did both TX and then and RX and then doing something else or waiting for this operations to be done. How would your application know if one has failed, then which one did fail? I do not see any big disadvantage in keeping this two separate. 
 
 My second question is, would there be any benefit to using nrfx_twim_xfer() with a TXRX transfer description? I assume that would allow me to eliminate one wait loop from my readRegister function and only require handling the appropriate transfer type in the event handler. I am also assuming I would only need a single m_xfer_done flag to wait on. 
 In this particular scenario for TXTX,yes your assumptions seems very much valid. 
 
 Lastly, is the do { __WFE(); } while(!BME280::m_tx_done); all that is needed to keep these functions non-blocking? The key part being __WFE(). 
 The example did not have anything else to do even after the non-blocking call. Some applications would like to do something else instead of blocking wait of the transfer. If your application has some other tasks that could be done which are not depending on this transfer, then you can eliminate the while loop and finish your task. But as soon as your application becomes dependable for this transfer to be complete to move on, then it needs to wait like this wrapping the __WFE on the condition of the transfer flag being set.

TWIM and the proper utilization of the event handler

Top Replies