UART TX in particular seems to have a race condition with scheduler running

I am noticing that particularly with the transmission of bytes over UART while scheduler is running, either some bytes go missing or the entire set of bytes aren't sent or received. (I'm seeing the UART stuff in the serial port). Though, I can send bytes from the serial port just fine: the ISR is triggered as soon as I type in something for each byte

So for according to the code snippet, I am printing Hello at the start of the app and often times I see Ello being printed and sometimes none of it

I'm wondering if there's a race condition somewhere and any assistance shall be greatly appreciated.

 

void Uart::TxByte(uint8_t byte)
{
        setNrfEvent(NRF_UART_EVENT_TXDRDY, 0);
        pUARTx->TXD = byte;     // triggers the IRQ handler by writing the first byte
}


void Uart::StartTx(uint8_t *buffer, size_t bytesToSend, bool blockingTx)
{
    setNrfEvent(NRF_UART_TASK_STARTTX, 1);
    
    fifoTx.enqueElems(buffer, bytesToSend);
    uint8_t val = fifoTx.deque();
    TxByte(val);

}

void Uart::PrintToTerminal(const char *format, ...)
{
    char buffer[100] = {0};
    va_list args;

    va_start (args, format);
    vsnprintf (buffer, sizeof(buffer), format, args);
    strcat(buffer, "\r\n");
    size_t bytesToSend = strlen(buffer);

    StartTx((uint8_t*) buffer, bytesToSend);
    va_end (args);
}

void UARTE0_UART0_IRQHandler(void)
{
    bool isTxdRdyIrqSet	   = getIrqRegStatus(NRF_UART_INT_MASK_TXDRDY);
    bool isTxdRdyEvntSet   = getNrfEventStatus(NRF_UART_EVENT_TXDRDY);
    
    if (isTxdRdyIrqSet && isTxdRdyEvntSet)
    {   
        setNrfEvent(NRF_UART_EVENT_TXDRDY, 0);	    // clear the TXDRDY bit
        
        // continue to send bytes till the FIFO is empty
        if (!fifoTx.isEmpty())      
        {
    	  uint8_t byte = fifoTx.deque();
    	  TxByte(byte);
        }
        else
        {
    	  // transmission ended
    	  setNrfEvent(NRF_UART_TASK_STOPTX, 1);
        }  
    }
}

bool Uart::getNrfEventStatus(nrf_uart_event_t reg) const
{
    return (bool) *(volatile uint32_t *)((uint8_t *)pUARTx + (uint32_t)reg);	
}

bool Uart::getIrqRegStatus(uint32_t mask)
{
    return (bool) (pUARTx->INTENSET & mask);
}

int main()
{
    uart.PrintToTerminal("Hello");
    // ...
    vTaskStartScheduler();	
}

Parents
  • Hi

    Can you confirm that you are using the nRF52840 device?

    First off you should use the UARTE peripheral rather than the UART peripheral in order to ensure that no bytes are lost. 

    Secondly, you should use the nrfx_uarte driver rather than using the peripheral directly. These drivers have been developed by us over time to configure the peripherals optimally, and have been thoroughly tested to ensure they work well in different applications. 

    Best regards
    Torbjørn

  • Yes, this is nRF62840.

    doesn't UARTE use DMA additionally? why would that ensure no bytes are lost?

    i am actually writing my own driver. do you see whether there's a possibility of a race condition in my snippet?

  • No, _uart.PrintToTermina() is invoked within a thread context and not ISR I believe.
    I am setting TWI_IRQ_PRIORITY as the interrupt priority for the I2C sensor and for UART, I'm passing in  APP_IRQ_PRIORITY_LOWEST as the priority. So perhaps it causes the UART interrupt to be preempted by the I2C's? 

    Although I don't read I2C values until after 3 seconds and the UART transmission shouldn't take that long before the next I2C interrupt occurs no?

    void twi_handler(nrf_drv_twi_evt_t const * p_event, void * p_context)	   
    {
        MCP9808 *obj = static_cast<MCP9808*>(p_context);
        BaseType_t xHigherPriorityTaskWoken = pdFALSE;
    
        switch (p_event->type)
        {
            case NRF_DRV_TWI_EVT_DONE:
                if (p_event->xfer_desc.type == NRF_DRV_TWI_XFER_RX)
                {
                    // unblock the waiting thread (mainThread) once RX XFER is done
                    vTaskNotifyGiveFromISR(obj->taskHandle, &xHigherPriorityTaskWoken); 
                }
                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 MCP9808::mainThread()
    {
        xferData(tmpBuffer, 1);  
        vTaskDelay(pdMS_TO_TICKS(2000));
        uint32_t notifiedValue;
    
        while(true)
        {   
            uint32_t val = i2cRead(); // invokes TWI interrupt
    
            uint32_t taskNotify = ulTaskNotifyTake(pdTRUE, portMAX_DELAY);  // blocks the current thread by decrementing ulNotifiedValue to 0 (taking a semaphore)
            if (taskNotify != 0)
            {
    	        NRF_LOG_WARNING("Transmission ended as expected...\n");
            }
            else
            {
    	        NRF_LOG_WARNING("The call to ulTaskNotifyTake timed out\n");
            }
            
            // if here, ISR is done executing and signalling to unblock this thread
    
            readTempInC();
    
            notify(this);       // calling _uart.PrintToTermina() within notify()
      
            vTaskDelay(pdMS_TO_TICKS(3000));
        }
    }

  • Hi

    Have you checked with the debugger if the UART interrupt is actually called? 

    And if it is called, will it get to the point where the TxByte() function is called to load the next byte?

    If you only see one byte of data every time you try to write a string it seems like the UART interrupt is not working properly. 

    Best regards
    Torbjørn

  • UART interrupt is firing fine and so does the TxByte(). It's not that I don't see anything in terminal, but moreso the rate at which I see the output is waay too slow as you can see in the video shared above. And this only happens for the sensor data that I'm outputting over UART in a thread

  • Hi 

    Sorry for the slow response, I have been out in holiday for a while. 

    Are you still having issues with this? 

    If so, can you please share the current state of your code so I can have a look at it?

    Best regards
    Torbjørn

  • Hi. No worries. I hope you had a great time off. 

    My code hasn't really changed since the last time. Do you want me to attach the whole project?

Reply Children
  • Hi

    I did, thank you Slight smile

    The whole project might not be necessary, but at least the source code for the implementation of the driver, and any files that use the driver. 

    Best regard
    Torbjørn

  • NotificationManager.cpp

    uart.cpp

    it's the following lines in NotificationManager.cpp that's printing to UART being delayed

    snprintf(_notification.msg, 50, "t=%uC", tmpValue);
    _uart.PrintUart(_notification.msg);


    here you go. thanks

  • Hi 

    Is StartTx() used in blocking or non blocking mode in Uart::PrintUart() ?

    I assume the function declaration in the header file has a default value, since you don't specify this parameter?

    How have you implemented the setNrfEvent() function?

    Is it intentional that you are using this to access both events and tasks?

    Have you been able to figure out how many times the irqHandler is called as a result of a single UART transaction?

    (say you try to send 4 bytes, how often will the interrupt happen?)

    Best regards
    Torbjørn

  • Is StartTx() used in blocking or non blocking mode in Uart::PrintUart() ?

    StartTx() does non blocking transfer via interrupts in Uart::PrintUart().

    I assume the function declaration in the header file has a default value, since you don't specify this parameter?

    That's right.

    How have you implemented the setNrfEvent() function?

     void setNrfEvent(T reg, uint32_t value)
        {
            *((volatile uint32_t *)((uint8_t *)pUARTx + (uint32_t)reg)) = value;
            //volatile uint32_t dummy = *((volatile uint32_t *)((uint8_t *)reg + (uint32_t)reg));
            volatile uint32_t dummy = *( (volatile uint32_t *) ((uint8_t *)pUARTx + (uint32_t)reg) );
            (void)dummy;
        }

    Is it intentional that you are using this to access both events and tasks?

    are you saying why am I not using existing nRF UART driver and use my own functions? If so, it's just to be confident in understanding the driver part.

    I really gotta add that it's uber hard to debug on nRF particularly cause as you step into each calls, at some point, it ends up at an Unknown function


    After further debugging, it seemed to me the following was causing an issue. I was creating a local copy of Uart instance inside the class instead of a reference. 

    Though I'm not sure as to how was Uart instance created when it doesn't match the constructor

    class NotificationManager : public Observer
    {
        Subject *subscriptions[maxSubscriptions] = {nullptr};	// store references to Subject
        uint8_t subscriptionIdx = 0;
        Uart _uart;     // *** ISSUE *** a local copy
        BleCustomService &_bleCustSrv;
        Fifo<Notification> _notifications;
    
        public:
        
        // constructor
        template<typename ...Args>
        NotificationManager(BleCustomService &bleCustSrv, Uart &uart, Args ...args) : _bleCustSrv(bleCustSrv), _uart(uart), _notification{0}
        {
            ((subscriptions[subscriptionIdx] = args), ...);
            subscriptions[subscriptionIdx++]->attach(this);
        }

  • Hi 

    My point about the setNrfEvent() function was more on the naming. Events are a specific type of register, and if you are also using this function to access task registers it might have been better to name it setNrfRegister() or something like that. 

    Looking at the definition it seems generic enough to access any register. 

    morpho said:

    After further debugging, it seemed to me the following was causing an issue. I was creating a local copy of Uart instance inside the class instead of a reference. 

    Though I'm not sure as to how was Uart instance created when it doesn't match the constructor

    That might explain the issue, at least it is something you should fix before doing further testing. 

    Possibly the compiler is implementing a default void constructor?

    Best regards
    Torbjørn

Related