Strange behavior with USB CDC as virtual COM port on NUCLEO-L476RG

Benjamin Brammer · ‎2020-01-07

Hello everybody,

I am using the NUCLEO-L476RG to send simple ACII text over UART (RS232 adapter) or USB CDC as a virtual COM port. UART communication is working seamlessly but when I use the USB CDC library functions from STM there is hapenning something strange I cannot explain:

I am sending in regular intervalls ASCII-tesxt in the follwoing format: ABP;***;***;***\n

The * stands for different numbers. normally I would see this kind of text flow on a terminal like hterm:

ABP;***;***;***\n

... and so on

but with the USB CDC usage I see occasionally this:

ABP;***;***;***\n

ABP;ABP;***;***;***\n

ABP;***;***;***\n

ABP;ABP;***;***;***\n

when I halt the debugger to check if my character array is corrupted, it isn't. So my assumption is that there is something not working correctly with the USB CDC device library from STM. This is the code, that gets invoked when I transmit the ASCII-text:

sprintf(Communication.measure,"ABP;%ld;%ld;%ld\n",Patient.Systolic, Patient.Diastolic, Patient.MAP);
if(System.Mode == UART)
{
	LPUART_transmit_message(Communication.measure);
}
else if(System.Mode == USB)
{
	CDC_Transmit_FS(Communication.measure, strlen(Communication.measure));
}

as you can see, i am using the same array preparations for the USB CDC version as for the UART version, except that the UART version functions seamlessly. The big difference between both versions is that I only utilize USB CDC device library functions provided by STM, not my own.

Has anybody experienced something similar? Or has a good hint where to find the problem?

best regards

Benjamin

Benjamin Brammer · ‎2020-02-18

Hey Bob,

sorry for the late reply, but I had other stuff to attend to.

OK the problem seems to be fixed now, although I think the real problem was my Voltage Range 2. I used this one because of optimum power efficiency and lowest power consumption in run mode (24MHc HCLK and Voltage Range 2). But this seems something not working correctly with a MSI RC of 48000 MHz. I even think this is not possible according to the reference manual under 6.2.9 there is a table that list the clock frequencies dependet on the Voltage Range. And it clearly says Range 2 has a maximum of 24MHz MSI RC range. This seems to be a bug in the STMCubeMX clock configurator, since 48MHz should not be possible with Range 2.

I still need to figure out how I will change this when I want to use the STOP0 mode. But this is something not important right now. So in the end it seemed to be the combination a too high clock with the Voltage Range 2 that caused the problem and nothing with a stray pointer or buffer overflow or so. Quite interesting, don't you think?

Anyhow, I am really, really thankful for your help and efforts on that problem!

Thx,

Bejamin

View solution in original post

Bob S · ‎2020-01-07

I am presuming that LPUART_transmit_message() does not return until the entire string has been sent. Is that correct?

After you call CDC_Transmit_FS(), you need to wait until the data has been sent before you call sprintf() to write new data into the string, Though that wouldn't produce the results you show above. Checking the CDC_Transmit_FS() return value is also a good idea. Though, again, having CDC_Transmit_FS() fail because it was still transmitting the previous data shouldn't cause what you are seeing.

How often are you sending that string?

Benjamin Brammer · ‎2020-01-07

Hello Bob,

yes the LPUART_transmit_message() function does not return until the entire string is sent:

void LPUART_transmit_message(char *message)
{
	strcpy(LPUARTBufferTx, message);
 
	if(hlpuart1.gState == HAL_UART_STATE_READY)
	{
		if(HAL_UART_Transmit_IT(&hlpuart1, (uint8_t *)LPUARTBufferTx, strlen(message)) != HAL_OK)
		{
			Error_Handler();
		}
	}
}

I am sending this ASCII- texts at least every 10s mostly it is every 30s so not very offten.

Bob S · ‎2020-01-07

Do you use "Communication.measure" anywhere else in the code? Are the values always 3 digits (which makes your string always 16 bytes long including the newline)? I imagine at least the systolic might be a 2 digit value assuming a healthy person.

I haven't yet replicated this issue on my L476-DISCO, but it has only been running for about 10 minutes so far. And I'm running a modified version of the HAL USB code which supports multiple VCOM devices. My changes shouldn't affect this issue, but you never know. About how often does this issue appear?

What HAL version are you running? And what CubeMX version?

Bob S · ‎2020-01-07

Oh... more thoughts. Is the "Communication" variable/structure (class??? is this C++ code?) local to this function? Or does it always exist? Unlike your LPUART code, the USB code doesn't copy the data from your buffer right away. So if "Communication" is a local variable and this function exits, or Communication.measure gets de-allocated or overwritten, before the USB code actually copies the data, THAT could cause corrupted data.

Can you have whatever program you use on the PC to display that data also show non-printable characters?

Benjamin Brammer · ‎2020-01-07

yes, I use it if the host would issue a command to request the ASCII-text (ABP;***;***;***\n). But this is only for debug purposes. And works fine. The values can have two digits, but the problem occurs independent of this. I tested long intervals with the same values.

I am using STM32CubeIDE (1.1.0) with STM32CubeMX (5.4.0) and firmware package for L4 V1.14.0 .

Benjamin Brammer · ‎2020-01-07

I write in C-code. and the struct is defined in my main.h :

/**
 * @brief	Definition of the sCom struct. This struct contains the communication
 * 			protocol buffers for data to be sent and command to be decoded.
 */
typedef struct
{
	char command[10];			//ToDo: Maybe the array size needs to be more!
	char payload[10];
	char measure[16];
 
}sCom;

I declare and initialize it in main.c . But I have to tell my different .c files with extern that this struct exists. So also in the file where I invoke the transmission via LPUART or USB CDC.

What do you mean with non-printable characters? I use hterm.

Thanks a lot for your help and efforts! =)

Bob S · ‎2020-01-07

(1) Your measure[] buffer is too small!!! You need at least 17 bytes: 16 for the string including the newline, plus one more for the trailing NULL. And NEVER use "sprintf", always use snprintf() to avoid the buffer overflow that you are currently getting. For example:

snprintf(Communication.measure,sizeof(Communication.measure),"ABP;%ld;%ld;%ld\n",Patient.Systolic, Patient.Diastolic, Patient.MAP);

The 2nd parameter to snprintf() specified the maximum number of bytes to write into the buffer (1st parameter) INCLUDING THE TRAILING NULL. It will truncate the output string if necessary to keep room for the trailing NULL if the actual output string is too long.

Now, granted, your buffer will not overflow if systolic is 3 digits, diastolic is 2 digits and MAP is 3 digits. But anything larger than that will overflow your measure buffer.

(2) Just for testing purposes, don't re-use the "measure" buffer. See if that makes any difference.

Benjamin Brammer · ‎2020-01-07

thanks for your answer, I did not know, that there is allways a trailing NULL. I will check this out tomorrow and hopefully confirm your answer as the best one.

Bob S · ‎2020-01-07

"C" strings always have a trailing NULL. That is how, for example, strlen() figures out how long the string is.

One more thing - since you are using "%ld" format specifiers, what happens if for some unforeseen reason one of the values is greater than 999 (say you got bad data, or something corrupted your data in RAM)? You will end up truncating the string to force it to fit into 16 bytes (17 with NULL). The STM32L476 has LOTS of RAM. Make your "measure" buffer much larger so that your message won't ever get truncated. That way the receiving end will always get a well-formatted message, though it will have to realize the data is nonsense. Note that each "%ld" can take up to 11 characters (10 digits plus sign). Don't use "%.3ld" to try and force a maximum of 3 digits, as that will truncate bad data and possibly make it look valid.