1

I'm writing code for stm32l011 MCU and it have to do one basic task for now - wait for a specific byte(predefined by me) from the UART and following "\r\n" or "\n\r" sequence. If it receives that, it set's a null pointer to the address of the predefined char, which is read and handled in the main loop.

    volatile uint8_t uart_buff[UART_BUFF_LEN] = {'\0'};
    volatile uint8_t *uart_data = NULL;
    volatile uint8_t *uart_stack = uart_buff+2;

    void USART2_IRQHandler(void){
        if(LL_USART_IsActiveFlag_RXNE(USART2)){
            *uart_stack = LL_USART_ReceiveData8(USART2);
            user_uart_send_byte(*uart_stack);

            //same as if(!(strcmp((uart_stack-1), "\r\n") || strcmp((uart_stack-1), "\n\r")))
            if((*uart_stack == '\r' || *uart_stack == '\n') && (*(uart_stack-1) == '\r' || *(uart_stack-1) == '\n'))
            {
                uart_data = uart_stack - 2;
                uart_stack = uart_buff+2;
            }

            uart_stack++;
        }
    }

and main:

    extern volatile uint32_t systick_counter_ms;
    extern volatile uint8_t *uart_data;

    int main(void){

        init();

        while(1)
        {
            LL_GPIO_TogglePin(GPIOB, LL_GPIO_PIN_3);
            //delay_ms(1); //without that it doesn't want to work
            if(uart_data)
            {
                adc_transmit_reading(*uart_data);
                uart_data = NULL;
            }
        }
    }

I wrote the C program above and it didn't seem to work. But when I introduced delay in the main loop it just magically sprang to life. Why is that? Key things to note: There's echo from the UART so I'm sure that it handles the interrupt properly in both cases, also I tried to remove optimizations of the compiler(which optimizes for size(Os)) by using volatile variables. Also the delay is made by while loop, which waits interrupt which increments variable every millisecond.

My layman's guess is that because I'm only looping around that uart_data pointer, somehow the synchronization between the interrupt and main loop isn't happening.

Thanks in advance :)

Hristo Mitrev
  • 17
  • 7
  • 1
    There are physical things happening that are slower than your run-time clock speed. The delay provides enough time for the value of `uart_data` to change before testing it. – ryyker Feb 27 '19 at 14:12
  • You have race conditions here. You need to ensure that those variables cannot be accessed from two places at once. The delay makes the bugs surface far less frequently. See [this](https://electronics.stackexchange.com/a/329961/6102) for advise of how to write interrupts proper. – Lundin Feb 27 '19 at 14:21
  • I don't understand. Why it doesn't change in the interrupt in the first place and also I sample it in a loop - if it didn't change the first time, why not the next time I take a sample? Also I didn't note that my clock is derived from MSI and is slowed down to around 4MHz to optimize consumption. – Hristo Mitrev Feb 27 '19 at 14:21
  • When "it does not work" you can drop a breakpoint as `adc_transmit_reading` and see what manner of crap you got in `uart_data`. Make sure that interrupts freeze during debug break. – Lundin Feb 27 '19 at 14:26
  • So I just need to write get() function where the ISR is implemented and use that function instead of reading directly the extern global variable? It makes sense, but it seems a little bit less efficient unfortunately. In this case it doesn't matter, but is there any way to make my implementation work? – Hristo Mitrev Feb 27 '19 at 14:28
  • You need to protect against race conditions and its best to keep that code inside the UART driver where it belongs, rather than to "spaghetti outsource" it to some unrelated part of the code. Your MCU is probably some 100 times faster than the UART, too, so the bottleneck is clocking in the data. Unrelated, you most likely have the option to DMA the incoming data, which is always a better solution than interrupts, even if it comes with other kinds of complexity. – Lundin Feb 27 '19 at 14:32
  • Oh - I also didn't mention that when it doesn't work it didn't even go into the if() statement – Hristo Mitrev Feb 27 '19 at 14:32
  • When you disassemble it, does it generate code for the if statement at all? Otherwise you got the bug pointed out in the answer by Louis. – Lundin Feb 27 '19 at 14:34
  • I also thought about DMA, but since I have one task and one task only and every three bytes can be a potential command, I decide to check on every incoming byte and figured out that it wouldn't be more efficient with DMA. Am I wrong? – Hristo Mitrev Feb 27 '19 at 14:41
  • Depends on baudrate and how much work your CPU has to do overall. If it has nothing better to do, then sure you can have byte-wise UART interrupts. Interrupts are kind of the root of all evil though... but sometimes a necessary evil. – Lundin Feb 27 '19 at 14:54

1 Answers1

3

The problem is that you have indicated that your uart_data points to volatile data. But in your loop, you are testing the pointer and not the data.

So you must indicate that your pointer is volatile as well:

extern volatile uint8_t * volatile uart_data;

You can find more information in this post: Why is a point-to-volatile pointer, like "volatile int * p", useful?

Louis Caron
  • 1,043
  • 1
  • 11
  • 17
  • This might be one of the problems indeed. – Lundin Feb 27 '19 at 14:34
  • The code has, indeed, a couple of other issues, with the uart_stack handling but this is not the scope of the question! ;-) – Louis Caron Feb 27 '19 at 14:46
  • Thank you a lot sir, you were right. Also If you have the time, I'll be really glad to know about the other issues of my code :) I know it's not pretty what I've done with uart_stack and uart_buff, but it seemed that it will not cause undefined behaviour or smt nasty – Hristo Mitrev Feb 27 '19 at 14:56
  • Maybe I have to memset the first two bytes of uart_buff to 0 or smt to prevent error if I receive "\r\n" at the very beginning? – Hristo Mitrev Feb 27 '19 at 15:28