I2C Read transfer doesn't happen with ulTaskNotifyTake

[morpho]

I am using nrf_drv_twi_rx to initiate an I2C read transfer but the interrupt isn't fired for the RX transfer when the ulTaskNotifyTake is used, but seems to work without ulTaskNotifyTake() call. When I hooked up to the logic analyzer, it seems the read bytes aren't sent.

void twi_handler(nrf_drv_twi_evt_t const * p_event, void * p_context)	   
{
    Sensor *obj = static_cast<Sensor*>(p_context);
    
    switch (p_event->type)
    {
        case NRF_DRV_TWI_EVT_DONE:
            if (p_event->xfer_desc.type == NRF_DRV_TWI_XFER_RX)
            {
	            vTaskNotifyGiveFromISR(obj->taskHandle, pdFALSE);
	            
            }
    	    else if (p_event->xfer_desc.type == NRF_DRV_TWI_XFER_TX)
    	    {
    	      NRF_LOG_INFO("TX transfer done...\n");
    	    }
            m_xfer_done = true; 
            break;
        default:
            break;
    }
}

void Sensor::initiateI2cRead()
{
    ret_code_t err_code = nrf_drv_twi_rx(&m_twi, ADDR, _buffer, 2);
}

void MCP9808::write(uint8_t reg, uint8_t *buffer, uint8_t size)
{
    ret_code_t err_code;

    err_code = nrf_drv_twi_tx(&m_twi, ADDR, &reg, size, false);
    APP_ERROR_CHECK(err_code);
    while (m_xfer_done == false);
}

void Sensor::xferData(uint8_t size)
{
    write(reg, size);	    
}


void Sensor::mainThread()
{
    xferData(1);  

    while(true)
    {   
        initiateI2cRead();

        uint32_t taskNotify = ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
        
        processData();
    }
}

[Susheel Nuguru]

Sorry for late response morpho.

Normally processData should do nothing if there is no data (it is normally waste of cpu time the first call) but it avoids one race condition when your application state machine depends on process data to complete for preparing for the next transaction.

I would still recommend you to have process data first and call ulTaskNotifyTake . It would only make difference the first call. Then it would be a loop like how you have it now (minus the race condition possibility)

[morpho]

Thanks for the response. 
sorry, I'm still not fully convinced as to why would processData() be still after ulTaskNotifyTake. Isn't the main idea behind ulTaskNotifyTake to block the current task till data is ready to be processed? Perhaps why even go inside processData() if there's nothing to process, plus what are you blocking the test after processData() via ulTaskNotifyTake for?

if this is the flow, and assuming processData() doesn't do anything if there's no data, I assume the following flow:

- interrupt is enabled but data hasn't been read yet

- processData() is invoked but nothing to process 

- block the task till the I2C data is fully read. Unblock it once the data is read but it's not processed

- repeat the loop --> go on to read the next i2c data

If I'm understanding correctly, above is the flow. If that's true, don't you think you're missing out the processing of certain reads in case data wasn't read by the time processData() was invoked?     

while(true)
{
initiateI2cRead();

processData();

uint32_t taskNotify = ulTaskNotifyTake(pdTRUE, portMAX_DELAY);

}

[Susheel Nuguru]

morpho said:

If I'm understanding correctly, above is the flow. If that's true, don't you think you're missing out the processing of certain reads in case data wasn't read by the time processData() was invoked? 

 I can't see the implementation of processData(). Normally if there is no data available, then the implementation of processData should be able to detect that there is no new data and bails out early. 

If you are very confident that you do not need such early bailout mechanism and also you are sure that this function's global data is being protected using some critical sections, then I would not look into this direction. Lets try to focus more on the visible code you provided.

            if (p_event->xfer_desc.type == NRF_DRV_TWI_XFER_RX)
            {
	            vTaskNotifyGiveFromISR(obj->taskHandle, pdFALSE);
	            
            }

try notification from the ISR this way.

    BaseType_t yield_req = pdFALSE;

            case NRF_DRV_TWI_EVT_DONE:
            if (p_event->xfer_desc.type == NRF_DRV_TWI_XFER_RX)
            {
                vTaskNotifyGiveFromISR(obj->taskHandle, &yield_req);

                /* Switch the task if required. */
                portYIELD_FROM_ISR(yield_req);
            }

[morpho]

processData() is mainly just using the raw sensor data to generate an appropriate output value. If you bail out from the function itself, don't you think you'd be most likely to lose the data as opposed to only processing once you know there's something to process?

Also when I put the breakpoints inside the nrf_drv_twi_rx(), and run in the debug mode, I see things running in an expected manner, and if I don't put breakpoints, and I have xTaskNotifyTake(), initiateI2cRead() doesn't get fully executed hence no interrupt generation.

And you're right about using portYIELD_FROM_ISR() inside the ISR but that's not the solution.

[morpho]

I also added a simple GPIO write function call after the read function and the gpio write was successful but the read function still doesn't seem to fully execute i.e don't see read bytes being sent in the logic analyzer. Can you confirm if the nrf_drv_twi_rx() isn't causing any trouble? (i doubt though but still)