UART buffer mostly filled with garbage data (STM32F7)

Question

Basic setup information: I have a Nucleo-F767ZI connected to my PC through the USB cable which is also used for its virtual com port. All the peripherals have been iniitalised to their default mode. I have enabled ADC1 IN5, IN6 with DMA to have a differential voltage measurement (as there is no differential ADC mode in this Nucleo-F767ZI). The timebase is TIM6 because I want to integrate FreeRTOS in the next iteration, but the current iteration has no FreeRTOS code.

After processing my raw ADC data, and sending it over the UART port, for some reason I am getting garbage values in my UART output. Much of the UART output is garbage and the rest is correct. A sample output looks like this:

UART Sample Internal

The ADC array seems completely normal with values close to 0 and 4095 (the adc pins are connected to to 3.3V and GND for testing, thus these values). I should be receiving 1024 voltage values but the end result after the garbage is only about 70-80 lines.

The code in question is being executed inside a DMA conversion complete callback function. I added a function-less while loop to wait, to rule out the possibility that DMA was calling the conversion completre callback function too quickly**. Increasing the value of count_for_no_reason to a value big enough, that printing to the terminal pauses for a couple of seconds, revealed that the garbage is actually in the beginning of the uartOut_volts buffer.

** I realised later that it does not make complete sense, as the UART is anyway in blocking mode. Additionally, I assume even though the DMA could be quicker in filling out the buffer than UART can output the data, there must be an internal check that waits to call the callback function again before this one returns.

The main function is standard except a call to start the DMA.

#define ADC_BUFFER 1024
uint16_t adcIn_raw[ADC_BUFFER * 2] = {0}; // times 2 for the two channels
char uartOut_volts[ADC_BUFFER * 16] = {0}; // times 16 so that there is enough space for the 12 bits in each conversion 
// ...main() and other declarations and auto generated code...
void HAL_ADC_ConvCpltCallback(ADC_HandleTypeDef *hadc)
{
 int uart_len = 0; // keep track of the amount of bits written to the uart buffer
 int fh_idx = 0, sh_idx = 0; // first half (fh) and second half (sh) counter variables 
 int half_adc_buffer = ADC_BUFFER / 2;
 int fh_times2_idx, sh_times2_idx; // for every two indices in adc array we only need to fill one index in voltage array
 uint16_t adc1_ch5_fh, adc1_ch6_fh; // temporary variables to fetch raw adc values
 uint16_t adc1_ch5_sh, adc1_ch6_sh;
 memset(uartOut_volts, 0, sizeof(uartOut_volts));
 if (hadc->Instance == ADC1)
 {
 // go until half array length, and fill both halfs at once
 for (fh_idx = 0; fh_idx < half_adc_buffer; fh_idx++)
 {
 // as fh_idx goes till the half array length sh_idx traverses the second half
 sh_idx = fh_idx + half_adc_buffer;
 // times2 keeps track of the larger raw adc buffer as adc array has two channel values which both need to be combined
 fh_times2_idx = fh_idx * 2;
 sh_times2_idx = sh_idx * 2;
 
 // accessing raw adc values of the two halves
 adc1_ch5_fh = adcIn_raw[fh_times2_idx];
 adc1_ch6_fh = adcIn_raw[fh_times2_idx + 1];
 adc1_ch5_sh = adcIn_raw[sh_times2_idx];
 adc1_ch6_sh = adcIn_raw[sh_times2_idx + 1];
 // the voltage drop is the difference between the two single adc measurements, converted from 12 bit value to volts
 vDrop_volts[fh_idx] = (adc1_ch5_fh - adc1_ch6_fh) * adc12Bits_to_volts;
 vDrop_volts[sh_idx] = (adc1_ch5_sh - adc1_ch6_sh) * adc12Bits_to_volts;
 // load into a uart buffer
 uart_len += snprintf(uartOut_volts + uart_len, 
 sizeof(uartOut_volts) - uart_len, 
 "%f\r\n", vDrop_volts[fh_idx]);
 }
 HAL_UART_Transmit(&huart3, (uint8_t *)uartOut_volts, uart_len, HAL_MAX_DELAY);
 }
 // counting because HAL_Delay in ISR makes the whole code hang
 int count_for_no_reason = 0;
 while (count_for_no_reason < 2e7) count_for_no_reason++;
}

Other code I see on the internet regarding UART and ADC seem to do it more or less like this as well, I am not sure what I have missed. Any direction is greatly appreciated.

EDIT 1: Some edits to answer the comments + expand on details:

• I am using an ST Link reflashed into a Jlink (following the book Hands-On RTOS with Microcontrollers by Brian Amos)
• I understand now that the callback should be as small as possible. But even after moving all UART+snprintf code to the main loop, the problem persisted.

EDIT 2: A debug session made me realise that the UART buffer array was being modified even without actually having gone through a line of code that would modify it. For example, in this image going from line 150 to 157 already modifies data in the UART buffer array.

Debug mode; uart buffer array modifies even before called

The Answer (not sure if I should write an answer or delete the question, given this was a rather "simpler" mistake)

Turns out the problem was indeed in the call to start the DMA. I used sizeof(uartOut_volts) instead of the length of the array. Turns out adcIn_raw and uartOut_volts were next to each other in memory, and giving the DMA permission to write more bytes than what was necessary made the first part of UART buffer being overwritten by DMA.

• Justme: So you are using the ST-Link debug port as the USB-UART interface for the actual UART you are using on the target MCU. You have not stated what baud rate you are using, how large data blocks you are sending at once, and what is the total bandwidth you think you need so that no data is lost or corrupted. But you need to rule out if it is the ST-Link provided VCP implementation by using something else as USB UART. Or at least by communicating slowly, with small data bursts and bandwidth.
• Arsenal: You cannot use HAL_Delay inside an ISR if the ISR has the same or higher (numerically lower) priority than the ISR used for the system timer (usually SysTick). I'd generally refrain from calling long blocking operations inside ISRs. I'm unsure about the HAL implementation by ST, so I cannot say if HAL_UART_Transmit is designed to be called from an ISR. I guess it works as it is using the blocking mode. You are not checking if snprintf is returning an error which could change your uart_len to unexpected values.
• brhans: In addition to not using HAL_Delay() in an interrupt (which is where your HAL_ADC_ConvCpltCallback gets called from) you should not really be trying to make 'blocking' UART calls (HAL_UART_Transmit()) either. You should refactor your code such that you do not try to do so much processing, etc inside your ADC callback and rather just set a flag to indicate to your main loop do do this. This also applies to the snprintf call which can be "dangerous" in an interrupt. Even your simple while loop delay is a bad idea inside an interrupt...
• Lundin: You shouldn't do anything inside a coversion complete DMA callback except shovel data to where it needs to go. In particular, you shouldn't be using stdio.h anywhere in a microcontroller application, because it was designed by dinosaurs in the 1970s and is plain dangerous, but also parts of it like the *printf family of functions are godawful slow.
• Lundin: Send them as raw bytes rather than ASCII? As a bonus you get less overhead that way. But then the sender and receiver must use the same endianness and the same floating point representation.

Answer 1

I think that calling uart_write inside an interrupt service routine may be the root cause of your problem. It has a maximum delay coded in there so it may well be attempting to wait for the USART to become ready.. which will slow down the return from interrupt .

Over the years of using STM32 devices, I have created myself a ring-buffer based serial port driver which means that you can "lock" the buffer, write data into it at an input ring buffer pointer position at almost any time, and then a transmit interrupt handler attached to the USART hardware "locks" the buffer, determines how many bytes it can transfer to the USART and transfers them (basically looping in the ISR until the USART indicates transmit buffer full) , at the same time removing the bytes from the buffer by incrementing the output ring buffer pointer. Some extra code in the "send character" routine checks to see if the USART is currently passively waiting for a character to be sent, and if so directly writes a value into the transmit register to trigger the future generation of "transmit buffer becomes empty" interrupts.

I found that the provided STMCubeMx STM HAL drivers for the USART are annoying in many ways.

Serial library