Declaring a callback function in usart.h, using it in usart.c but defining it in main.c

I see you already have some general answers, so I'm going to pick on a few details.

Unless you're doing something unusual, don't use a callback function. Callback functions are in general complicated and prone to unconsidered details. For example, who's stack is current when the callback function is called? If it gets called in an interrupt routine, then it has to complete "quickly" to avoid more latency in other interrupts. You also don't know what stack and therefore how much stack space is available in the callback routine.

A good basic software interface to a hardware UART is four routines, all in the UART-specific module:

• uart_put

  Called by the application to send one byte out the UART. This routine actually stuffs the byte into the output ring buffer and makes sure the UART transmit interrupt is enabled. If the ring buffer is full, it blocks until it is empty.

• UART transmit interrupt routine.

  Grabs the next byte from the output ring buffer and writes it to the UART. Disables UART transmit interrupt if the ring buffer is now empty.

• UART receive interrupt routine.

  Gets the received byte from the UART and writes it to the input ring buffer. The only trickiness is what to do when the ring buffer is full. The simplest answer is to drop the byte and let the higher level protocol deal with it. If the UART is overrunning the firmware's ability to handle received characters, then something else is already seriously wrong. You might set an error flag too.

  The fastest common and standard baud rate is 115.2 kBaud. With 10 bit times per character, that's 11.52 kbytes/s, which is one byte every 86.8 µs. That's a long time for a modern micro. At 50 MHz instruction rate, for example, that's 4340 instruction cycles.

  If processing the UART input is bursty, you need to make sure the input ring buffer is long enough. To allow for a whole 10 ms of unprocessed input bytes, for example, requires only 115 bytes in the ring buffer.

  If you really can't handle UART data at the maximum for the baud rate, then you need to consider a lower baud rate or flow control in the higher levels of the protocol. For most of the microcontroller projects I've done, 115.2 kbaud with a modest receive buffer is plenty to guarantee no overrun.

• uart_get

  Called by the application to get the next UART input byte. It blocks until there is a byte available, then removes it from the input ring buffer and passes it back to the caller.

Those are the basics that are good enough for quite a few projects. Note that "blocking" doesn't necessarily mean sitting in a busy-loop. Usually, especially when there is asynchronous input like from a UART, there is a multi-tasking system in use. Blocking means calling TASK_YIELD in a loop until it is possible to proceed. The task that is sending or receiving UART data will stall, but the rest of the system will keep running. When tasks are properly designed, there is nothing else for them to do until the UART operation completes.

Added bells and whistles include routines the application can call to find whether UART_PUT and UART_GET can complete immediately, whether all the buffers are empty and the UART completely idle, etc.

In general, it is good to dedicate a separate task for each asynchronous data input stream. A good example is a command processor. It is an infinite loop that gets the next UART input byte as a command opcode and vectors to the command routine for that opcode. Each command routine gets whatever data bytes go with the command, then returns back to the start of the main loop. A new byte not being available stalls this process until the byte is available. As long as the micro with the multi-tasking system can keep up averaged over the maximum input buffer time, everything keeps working.

About ring buffers

No send/receive mutex needed

I see Lundin mentioned disabling interrupts around accesses to the input and output buffers while state may be inconsistent. You can do that, but it is possible to create ring buffers (FIFOs) that don't need that protection.

The trick is to use a PUT and GET offset into the buffer. The PUT offset is for the buffer entry where the next data will be written. GET is the index where the next data will be read from. If you define the buffer to be empty when PUT = GET, then iterrupts do not need to be disabled as long as the indexes are updated in the right order on both reading and writing.

As an example, see my FIFOW_xxx macros in STD.INS.DSPIC in the Embed DSPIC GIT repository. The FIFO for 16 bit data starts on line 4139. There is also a FIFO for 8 bit data, but it is older and less well documented. In particular, here are the two macros that put and get data into and out of the FIFO:

//   Macro FIFOW_PUT name
//
//   Write the word in W0 to the named FIFO.  It is the caller's responsibility
//   to ensure the FIFO has room for the new word.  Invoking this macro with the
//   FIFO full makes a mess.
//
//   Trashes: W1, W2
//
/macro fifow_put
         mov     #fifow_[arg 1]_buf, w1 ;point to start of buffer
         mov     fifow_[arg 1]_put, w2 ;get PUT word index into buffer
         add     w1, w2, w1  ;add byte offset to where to write the word
         add     w1, w2, w1
         mov     w0, [w1]    ;write the word into the buffer

         add     #1, w2      ;make raw new PUT index
         mov     #[v fifow_[arg 1]_bufsz], w1 ;get first invalid buffer index
         cp      w2, w1
         skip_ltu            ;still within the buffer ?
         mov     #0, w2      ;no, wrap back to start of buffer
         mov     w2, fifow_[arg 1]_put ;update the PUT index
  /endmac

//   Macro FIFOW_GET name
//
//   Get the next word from the named FIFO into W0.  It is the caller's
//   responsibility to ensure there is a word in the FIFO to read.  Invoking
//   this macro on a empty FIFO makes a mess.
//
//   Trashes: W1, W2
//
/macro fifow_get
         mov     #fifow_[arg 1]_buf, w1 ;point to start of buffer
         mov     fifow_[arg 1]_get, w2 ;get GET word index
         add     w1, w2, w1  ;add byte offset to where to read the word from
         add     w1, w2, w1
         mov     [w1], w0    ;read the word from the FIFO buffer

         add     #1, w2      ;make raw new GET index
         mov     #[v fifow_[arg 1]_bufsz], w1 ;get first invalid buffer index
         cp      w2, w1
         skip_ltu            ;still within the buffer ?
         mov     #0, w2      ;no, wrap back to start of buffer
         mov     w2, fifow_[arg 1]_get ;update the GET index
  /endmac

Note how the order of checks and operations guarantee there are no race conditions between reading and writing.

This method "wastes" one FIFO entry, since there is always at least one empty entry. However, this type of FIFO only uses two state variables other than the data values themselves. Any other scheme that allows for the FIFO buffer to be completely full would require another state variable. Unless FIFO entries are larger than the state variables, there would be no savings.

Buffer size is a constant

Another point to note is that the size of the buffer is not stored in RAM. It is fixed at build time, so will always be the same value at run time. For example, examine the first parameter to the MOV opcode in the line

         mov     #[v fifow_[arg 1]_bufsz], w1 ;get first invalid buffer index

of the FIFOW_GET macro, above. Expressions in brackets are preprocessor functions. The first parameter is therefore two nested preprocessor functions, preceeded by "#". The "#" indicates a literal value following. In this case, the result of the literal value is loaded into register W1.

The inner preprocessor function is [arg 1]. That expands to the first argument passed to the macro, which is the name of the FIFO. For example, if the FIFO was called "uart_in", then the macro would be invoked

         fifow_get uart_in

and [arg 1] is replaced with uart_in by the preprocessor.

The outer preprocessor function then becomes [v fifow_uart_in_bufsz]. The V function returns the value of a preprocessor constant or variable. In this case it returns the value of the constant fifow_uart_in_bufz. That constant was created and set to the number of slots in the FIFO buffer when the FIFO was created by the FIFOW_DEFINE macro. Here is that macro:

//   Macro FIFOW_DEFINE name, size
//
//   Define a word (16 bit data) FIFO.  NAME will be used to create unique
//   symbols for this FIFO.  All these symbols have the form FIFOW_name_xxx,
//   where XXX refers to particular symbols.  All the interactions with the FIFO
//   via the macros here are only by using NAME.  The various symbols created
//   and the exact details of the FIFO data structure and read/write protocol
//   should be considered private to these macros.  Put another way, a FIFO
//   should only be accessed thru the macros here.
//
//   NAME is the name characters directly, not a string.
//
//   SIZE is the maximum number of words the FIFO must be able to hold.
//
/macro fifow_define
  /const fifow_[arg 1]_bufsz integer = [+ [arg 2] 1] ;buffer size, words

alloc    fifow_[arg 1]_put
alloc    fifow_[arg 1]_get
alloc    fifow_[arg 1]_buf, [* fifow_[arg 1]_bufsz 2]

  /endmac

Note that the FIFO is actually allocated 1 word larger than the requested size (in the /CONST preprocessor command). This is to account for the one unused word mentioned before.

posted 6 days ago

CC BY-SA 4.0

6d ago

Olin Lathrop‭

1407 reputation 1 41 111 17

Indeed this is not a good design since it doesn't use private encapsulation. It is using a global variable which means it is pretty much by definition spaghetti code.

I don't see why you would use callbacks in this case either, since the copying of data is no business of the program outside the driver (unlike lets say a timer driver where it can make perfect sense).

For a normal project you'll probably want to separate everything in 3 separated parts:

• The raw "dumb" reception of data through a MCU-specific hardware peripheral. uart.h + uart.c
• The protocol decoder that knows about the nature of the data - the protocol, but not necessarily anything about the specific hardware peripheral driver. Either it calls the driver to ask for data, or data is passed between the driver and the protocol handler by a middle man ie some application-tier code from main().
• The application that uses the payload of the packet but otherwise doesn't care less about the protocol format, let alone the underlying UART hardware.

For certain smaller projects or hard real-time projects, you might want to merge the driver and protocol handling into one though. In case you want to instantly reject something upon reception, or instantly act on something etc. So in hard real-time projects you might want to decode the protocol on the fly as it is getting received.

As for how to design the driver and ring buffer:

• The interrupt and ISR should be internal to the driver uart.c and how it moves data from hardware buffers to RAM is no business of the rest of the code. That's an entirely internal affair inside the UART driver.

  Depending on system, other parts of the program may need to register the ISR in a vector table etc, but that's about it. ARM systems typically solve this by having default ISRs with "weak linkage", meaning that if you define a ISR with the same name then it takes precedence and that's resolved at link-time. Then everything about the ISR can be encapsulated. You should document that the driver comes with a live ISR though.

• The UART driver can declare the ring buffer type internally or otherwise include a separate ring buffer ADT. It is common that a ring buffer struct knows specifics about the hardware peripheral so that the safe access of data shared with an ISR can be handled from the ring buffer itself.

  For example if you know the baudrate in advance then you know how long a byte transmission takes and there's probably plenty of time to copy that byte until the next one arrives, meaning that you can temporarily disable interrupts while copying in order to protect against race conditions.

  An example of a generic ring buffer from an old project of mine, where the ring buffer has the possibility to set/clear interrupt flags through int_mask in an 8-bit register int_reg:

  typedef struct
  {
    void*              buffer;
    uint16_t           objects_n;
    uint16_t           object_size;
    uint16_t           object_count;
    uint8_t*           begin;
    uint8_t*           end;
    volatile uint8_t*  int_reg;
    uint8_t            int_mask;
    uint8_t*           end_of_buf;
  } rb_buf_t;
  

  This is rather over-engineered though, you might not need to make it this advanced and type-generic - this is from a gateway type of project that handled multiple UART and CAN peripherals all at once.

  But you should probably at least consider storing the size of valid data in the ring buffer struct, so that you can grab the size quickly, and so that you can tell the difference between a completely full and completely empty buffer.

• From inside uart.c you have a place in RAM where the received data is stored. Either it is the ring buffer, or it could be another buffer not connected to the ISR ("double-buffering"), if you for example want to separate the packet getting received right now from the previously complete packet.

  You can chose to expose this internal RAM buffer/ring buffer to the caller, but that should be a read-only access then, since no other part of the program has any business writing to the UART rx buffer.

• If you don't solve race condition bugs by enabling/disabling the specific interrupt, then yeah you can do it with a "poor man's semaphore" ie static volatile bool. Poor as in: in itself it is not guaranteed to be thread-safe/interrupt safe like the higher level concept of semaphores in an OS. No access in C code is otherwise ever atomic unless you actually use C11 atomic types.

  The idea behind this is that the access to the "poor man's semaphore" bool won't be atomic either, it might get interrupted right in the middle of updating the value. But we don't care, the data is either 1 or 0 and so data corruption isn't going to happen. We only care if the data transfer we are trying to protect gets interrupted while updating. The poor man's semaphore is always either on/off and at that point we can access the data. Basically we move the race condition problem away from the data to this bool, where it doesn't cause any harm. volatile variable access is not allowed to be re-ordered either.

  Some more info about volatile and "poor man's semaphore" here: Using volatile in embedded C development (In fact that answer probably answers this whole question in itself?)

• DMA is often far less of a head ache to work with than this "old school" interrupt approach, and it is not nearly as interrupt-intense. So if your MCU supports DMA then give it some serious consideration.

posted 7 days ago

CC BY-SA 4.0

7d ago

Lundin‭

10542 reputation 48 146 1227 200

[ rb_put(UDR0) sounds like an AVR. I'll be using STM32 in some examples, because I'm not familiar with AVRs. You were asking about about STM32 on EE.Codidact recently. ]

But I have a gut feeling that this is wrong. It feels wrong that I handle an intrinsic UART construct UDRE0 in main.c when it feels like it belongs in usart.c .

You can factor out the serial packet handler into a separate .c file. It would sit between main.c and usart.c It may be possible to reuse it [with upgrades over time] across multiple projects.

UART transmission (TX)

Transmission on UART is the easy part.¹ Olin already mentioned that you can iterate through the buffer and call the equivalent of putc(...) for each byte.

STM32 HAL has got a synchronous transmit function HAL_UART_Transmit(...) . There's the DMA transmit function HAL_UART_Transmit_DMA(...). DMA lends itself well to transmission.

UART reception (RX)

Receiving on UART is always harder than transmitting.¹ The inconvenience with synchronous approach is that the main() always needs to poll the UART fast enough. Receiving the UART bytes in the interrupt makes sure that none of them will be missed.

The UART on STM32 can receive variable length packets, and DMA them into memory. It determines the end of a packet by the idle period after it. There's one DMA interrupt at the end of the packet, and the complete buffer would be given to the packet processor.

¹ That's true for a lot of comms. Usually it's easier to transmit well than to receive correctly.

Hard-coded callback

Your proposed way of creating a callback shows up once in a while. I’ll refer to it as hard-coded callback [as opposed to function pointer callback], but I don’t know what this method is called actually. For example, it shows up in the STM32 HAL. void HAL_ADC_ConvCpltCallback(ADC_HandleTypeDef* phAdc)

// @file    stm32g4xx_hal_adc.h
// @brief   Header file of ADC HAL module.

// ADC IRQHandler and Callbacks used in non-blocking modes (Interruption and DMA)
void HAL_ADC_ConvCpltCallback(ADC_HandleTypeDef *hadc);
// ...and there are more prototypes for callbacks there.

The HAL also provides a default implementation (a stub). It’s declared with __weak so it can be overridden in another compilation unit. The __weak keyword is not part of the C standard. The GCC for AVR supports weak linkage via __attribute__((weak)) .

// @file    stm32g4xx_hal_adc.c

__weak void HAL_ADC_ConvCpltCallback(ADC_HandleTypeDef *hadc)
{
  UNUSED(hadc);    /* Prevent unused argument(s) compilation warning */

  /* NOTE : This function should not be modified. When the callback is needed,
            function HAL_ADC_ConvCpltCallback must be implemented in the user file.   */
}

posted 7 days ago

CC BY-NC-SA 4.0

Nick Alexeev‭

111 reputation 1 3 12 4