3

I'm new here and I'm trying to solve my projects problem. I'm trying to send send data over UART from Raspberry Pi B+ to my STM32F407 and process that data in real-time. My problem is: How to actually synchronize main() thred and UART interruption handler not to stop for a long time my UART interruption. Data sent from Raspberry looks like (where n is number): nwnx Code:

void UART4_IRQHandler(void)
{
    if(USART_GetITStatus(UART4, USART_IT_RXNE))
    {
        if(!flag)
        {
            processing = true;
            widthTemp[j] = USART_ReceiveData(UART4);
            USART_ClearITPendingBit(UART4, USART_IT_RXNE);

            if(widthTemp[j] != 'w')
            {
                j++;
            }
            else
            {
                j = 0;
                flaga = true;
                processing = false;
                //__disable_irq();
                NVIC_DisableIRQ(UART4_IRQn);
            }
        }else if(flag)
        {
            processing = true;
            heightTemp[j] = USART_ReceiveData(UART4);
            USART_ClearITPendingBit(UART4, USART_IT_RXNE);

            if(heightTemp[j] != 'x')
            {
                j++;
            }
            else
            {
                j++;
                heightTemp[j] = '\n';
                j = 0;
                processing = false;
                flag = false;
                NVIC_DisableIRQ(UART4_IRQn);
            }
        }
    }
}

and now main code:

while (1)
{
    if(!NVIC_GetPendingIRQ(UART4_IRQn))
    {
        //if()
        if(flag == true && processing == false)
        {
            sscanf(heightTemp, "%d", &height );
            USART_puts(UART4, heightTemp); // sending back data to raspberry (temporary)
            for(i = 0; i< sizeof(widthTemp);i++)
            {
                heightTemp[i] = '\0';
            }
            NVIC_EnableIRQ(UART4_IRQn);
        }
        if(flag == false && processing == false)
        {
            sscanf(widthTemp, "%d", &width );
            USART_puts(UART4, widthTemp); // sending back data to raspberry (temporary)
            for(i = 0; i< sizeof(widthTemp);i++)
            {
                widthTemp[i] = '\0';
            }
            NVIC_EnableIRQ(UART4_IRQn);
        }
    }
}

My problem is that at some point rasbperry is starting to have huge delays in recieved data.

Questions: 1. Can I disable interruptions in interruption handler like I did in my code to let my main() know that it can proceed data ? And if yes, do I check good register for this task ? 2. Is there a better way to send buffer (widthTemp f.e.) to some variable ? Maybe I don't need to complicate my code like this. 3. Can I use maybe threding in my code ? Will it help ? 4. This is my first post so any information about how to format questions, advices about the code etc. will be nice.

Clifford
  • 88,407
  • 13
  • 85
  • 165
P. Kudła
  • 105
  • 2
  • 8
  • 1
    1. Yes you can there is a global interrupt enable bit which you are supposed to disable whenever you go into an interrupt handle. 2. YES, you should NOT process the data in your interrupt handler this is a very bad practice. You should instead put it in a buffer with a flag saying hey main process the new data. 3. You don't need threads. 4. Your post is decently formatted. – arduic Jan 05 '16 at 12:44
  • @arduic : There is a global interrupt enable/disable, but you are not *"supposed"* to disable it in an interrupt handler. Cortex-M has a priority based preemptive interrupt controller and nested interrupts are supported - disabling interrupts globally defeats that function. Beyond that you have posted what looks like an answer as a comment. Your other points 2 and 3 would be better expressed as an answer. – Clifford Jan 05 '16 at 12:50
  • @Clifford right sorry used to more attiny kinda stuff. Well either way your interrupt handler should be much smaller. Typically people prefer if your handler is something of the pseudo code. Report handling of interrupt, Put data interrupt is passing into some kind of buffer, Mark data for processing, exit interrupt. Then in main you have a while(1) loop checking if the data mark for processing ever shows up. By putting all your code inside the interrupt you risk taking to long to process the interrupt before the next comes in. – arduic Jan 05 '16 at 12:53
  • 1) Don't use that bloat STlib. It just adds complexity without solving anything or making anything more clear. 2) Normally, there is no need to manually clear the pending bits. 3) You should read more about the Cortex-M4 interrupt system in general and that of the STM32F specifically before fiddling at this level. – too honest for this site Jan 05 '16 at 12:54
  • @arduic: The Cortex-M interrupt system is no way comparable to that of the AVR8. And the STM32F adds additional complexity (and complication). Actually most imlementations with Cortex-M do. And prioritisation allows a different approach when handling interrupt data. Depending on system-design, you can very well process the data in the handler - provided you can guarantee timing constraints, of course. – too honest for this site Jan 05 '16 at 12:56
  • Ok, thank you all guys for answering my questions in practically no time. I will change my code and see how it will work then. – P. Kudła Jan 05 '16 at 13:02
  • You should not process UART data in both: interrupt and thread mode. Use either: interrupt or thread mode/polling. Use proper communication mechanisms (semaphores/queus/etc.) between these modes. – too honest for this site Jan 05 '16 at 13:06
  • @arduic : Regarding your advice; yes I agreed with that part, but it is an answer not a comment. Post an answer. – Clifford Jan 05 '16 at 13:09
  • @Clifford I made it a comment instead of an answer because I was hoping he would post some kind of response as to why he wasn't doing that or why he thought that might not be the problem he had. It seems strange to me that someone knows how to use ISR but doesn't know the quote on quote golden rule of KISS (keep it simple stupid) that ISRs demand. – arduic Jan 05 '16 at 13:59
  • @arduic : The actual question is not that implied by the title. He asks (amongst other things) "Is there a better way...?", so you are free ot answer in any way you wish. – Clifford Jan 05 '16 at 14:47
  • Have you considered that your *primary* issue may be that you could be filling up the serial pipe by trying to send more data than the baud rate (or receiving program) can support? With no chance to catch up the buffering would mean it just gets further and further behind. You may need to encode the data more efficiently, send it less frequently, or use a higher baud rate. – Chris Stratton Jan 06 '16 at 01:29
  • Actually, my baud rate is 115200 and I give STM 0.05 s to process data. I think that is even too much time to handle this tasks. – P. Kudła Jan 06 '16 at 13:10
  • this loop: `for(i = 0; i< sizeof(widthTemp);i++) { heightTemp[i] = '\0'; }` is probably incorrect as it is using the size of a different field from the field that is being set to all '\0' – user3629249 Jan 07 '16 at 00:38
  • This two fields were actually of the same size. I changed my code and handle task in main(). It is now working :) – P. Kudła Jan 08 '16 at 08:11

2 Answers2

2

In general, your UART ISR should do little more than:

  • For a receive interrupt, place character from RXD in a ring buffer
  • for a transmit interrupt, get character from a ring buffer and place it in TXD. On STM32, if the ring buffer is empty, you must explicitly switch off the transmitter.
  • clear interrupt flags (handling of error flags optional).

Then your main thread (or some other thread/task if using an thread scheduler or RTOS), simply reads from the Rx ring buffer (or writes to the Tx ring buffer).

Note that on STM32 if the transmitter is not running, the first character should be placed directly in TXD, and the transmitter started, subsequent characters are then places in the Tx ring buffer.

If using an RTOS or other OS, you might use a queue or mailbox rather then implementing your own ring buffer. Some care is required to ensure consistent buffer control when more than one context is accessing the control data.

Note that the STM32 is capable of using DMA transfers to UART operation which may offer an alternative solution, especially if the data rate is very fast, since the STM32 UART has no FIFO and will overrun the RXD if each character is not removed from RXD in the time taken to receive another.

Clifford
  • 88,407
  • 13
  • 85
  • 165
  • using a ring-buffer adds additional copying in thread mode. This might be undesirable for some applications. Actually the prioritised interrupt system of the CM4 allows more flexible interrupt handlers. Adhering to timing contraints, of course. – too honest for this site Jan 05 '16 at 13:09
  • Using DMA is a good idea in general, but the DMA on the STM is a bit tricky, as it does not inter-operate well with intermediate CPU transfers, e.g. reading the length by CPU first, then staring DMA. Basically DMA requests get lost if the DMA-enable is set after the event occured. That gave me some headache (and forced major changes) in a project. – too honest for this site Jan 05 '16 at 13:13
  • @Olaf : Yes and Yes. The use of buffering was intended as a general solution. Implemented correctly (and there are any number of ways of getting it wrong) it is a simple way of implementing lock free inter-context communication. Beyond the title, the question itself is rather wide open; i.e. "is there a better way", to which I am saying, "yes, many; here's one". – Clifford Jan 05 '16 at 13:29
  • If you have an RTOS, I would argue that it is *usually* better to have the TX interrupt just bump a semaphore / flag / whatever your RTOS calls it and write a transmit function that waits for the TX register to before writing it. Unless dropping data is acceptable, you have to implement a blocking mechanism anyway. Blocking on the TX register availability rather than a ring buffer 1) is cleaner code, and 2) is more deterministic (unless multiple threads are transmitting), 3) eliminates the `memcpy`. – Brian McFarland Jan 05 '16 at 16:01
  • @Olaf - Using DMA is a good idea if: 1) Your CPU has something better to do while the I/O completes, 2) Your data comes in "large" bursts, 3) you care about performance more than developer time. Often one or more of these statements are false, making DMA the wrong choice if you're being paid for your time. – Brian McFarland Jan 05 '16 at 16:08
  • @BrianMcFarland: Things are often not that binary as your comment implies. And 3) actually is questionable If you know your hardware, there is not much difference in adding DMA. In fact, it can make things even easier - especially if you have a lot of (hardware) concurrency in your system. However, I see no actual relation to my comment. – too honest for this site Jan 05 '16 at 16:19
  • @Olaf, all I'm saying is DMA often adds more code complexity than its worth and is not "a good idea in general". Its a valuable tool for helping you get the most out of an MCU when you need it but applying when unnecessary is premature optimization. Just based on your own experience you described: if the project didn't really *require* the extra performance from DMA then it was probably the wrong choice as is evident by the bugs and rework. – Brian McFarland Jan 05 '16 at 17:44
  • @BrianMcFarland: YMMV. Mine is different. And I'm paid very well for my time. About premature optimisations: I strongly disagree. Simple reason is it is a fundamental design-decission and changing it requires major rewrite of driver code and changes timing pretty much. Surely for a 9600b/s line it is not much worth a thought, but already for 100k+ it should be seriously considered. And a proper design from the start is not more error-prone than a non-DMA approach. All that strongly depends on further details, of course and a general rule cannot be given - neither pro nor con. – too honest for this site Jan 05 '16 at 17:50
  • @BrianMcFarland : Your suggestion requires a synchronous per-character context switch to a thread in addition ot the necessary synchronous per-character context switch to the ISR. The resulting CPU load will be greater than buffering the data and handling the buffer asynchronously. You also talk about Tx when in the question only data *receive* is mentioned. However because both share a single IRQ, you have to handle both by at least clearing the pending bits. There is no `memcpy()` - one character in, one character out, but a memcpy would still be less CPU load than a context switch. – Clifford Jan 07 '16 at 09:07
  • @Clifford - *Your answer* brought up TX. How do you have a transmit *ring buffer* (recommended by you) without an additional copy? Say you implement a function `uart_tx(const void * data, size_t len)`. If you call `uart_tx( "somedata", 8)`, having a *ring* buffer implies that each byte is *copied* from `somedata` to a ring buffer (or 1st byte to TXD register). If the ring buffer has room, task returns after filling the buffer. If the ring buffer is full, you block on a semaphore or similar. And unless you get really clever, you then have per-byte ISR and thread context switches anyway. – Brian McFarland Jan 07 '16 at 16:22
  • Ring buffers and DMA either one, when properly implemented, will (in some cases) reduce your CPU load & improve performance. At the cost of code complexity that can lead to subtle bugs. I'm just arguing that having dead simple, easy to maintain code is often *better* than performance. If you're writing a reusable driver and have time to thoroughly test, then by all means, optimize. But if your goal is to get some bytes out on a wire and the easy version is "good enough", why change it? – Brian McFarland Jan 07 '16 at 16:33
  • @BrianMcFarland : Why change it. For re-usability of higher level code it is useful to have a general-purpose communications architecture for all platforms. – Clifford Jan 07 '16 at 18:20
-1

As said in @arduic comment, you should not process data in the interrupt. You should keep interrupt service routine as short (and quick) as possible. Therefore, you will not need to disable them, preventing for losing data in the case of UART RX interrupt.

dudu721
  • 342
  • 1
  • 3
  • 13