32F417: can one detect when a USB Host is connected?

[PHolt.1]

Can anyone suggest a way of bypassing this USB OUT function ("OUT" means to Host i.e. the opposite of the correct USB terminology) if no USB cable is plugged in?

This is a fairly common bit of code from the ST Cube USB CDC (VCP) library, with additions from me to make it thread-safe, and a timeout in case a USB Host (with a USB VCP application running) is not connected.

I would like to achieve this without the timeout.

/**
  * @brief  CDC_Transmit_FS
  *         Data to send over USB IN endpoint are sent over CDC interface
  *         through this function. Note that "IN" in USB terminology means
   *         product-> PC.
   *
  * @param  Buf: Buffer of data to be sent
  * @param  Len: Number of data to be sent (in bytes)
  *  *
  * This function had a call rate limit of about 10kHz, and had a packet length limit
  * of about 800 bytes - until the flow control mod below. These limits still apply if
  * flow_control=false.
  *
  * This function is BLOCKING if flow_control=true. That means that if there is no Host
  * application receiving the data (e.g. Teraterm) it will block output.
  * The problem is that if no USB device is connected from startup, the while() loop
  * would block for ever, hence the timeout.
  *
  * Flow control is normally disabled for the debugging output functions.
  *
  * If called before USB thread starts, all output is dumped - for obvious reasons!
  *
  * Mutexed to enable multiple RTOS tasks to output to port 0.
  *
  */
 
#define USB_TX_TIMEOUT 100	// 100ms
 
uint8_t CDC_Transmit_FS(uint8_t* Buf, uint16_t Len, bool flow_control)
{
 
	if ( g_USB_started && (Len>0) )
	{
 
		uint32_t counter = 0;
		bool timeout = false;
 
		osMutexAcquire(g_CDC_transmit_mutex, osWaitForever);
 
		USBD_CDC_HandleTypeDef *hcdc = (USBD_CDC_HandleTypeDef*)hUsbDeviceFS.pClassDataCDC;
 
		// Wait on USB Host accepting the data.
		if ( flow_control )
		{
			// Loop around until USB Host has picked up the last data
			// This rarely holds things up - 20us typ. delay
			while (hcdc->TxState != 0)		// This gets changed by USB thread, hence the g_USB_started test
			{
				osDelay(1);
				counter++;
				if (counter>USB_TX_TIMEOUT)
				{
					timeout=true;
					break;
				}
			}
		}
 
		if (!timeout)
		{
			// Output the data to USB
			USBD_CDC_SetTxBuffer(&hUsbDeviceFS, Buf, Len);
			USBD_CDC_TransmitPacket(&hUsbDeviceFS);
		}
 
		osMutexRelease(g_CDC_transmit_mutex);
 
		g_comms_act[0]=LED_COM_TC;	// indicate data flow on LED 0
 
		// If flow control is selected OFF, we do a crude wait to make CDC output work
		// even with a slow Host. This actually needs to be only a ~100-200us wait, but
		// is host dependent.
		if ( !flow_control )
		{
			osDelay(2);
		}
 
	}
 
	return USBD_OK;
}

[PHolt.1]

Actually if I was going to use TxState I would want it READY when there is no connection, so the data disappears : - )

[PHolt.1]

A pair of functions which can be used to toggle a "CDC active" flag is CDC_Init_FS() and CDC_DeInit_FS().

These, in the ST code, do a malloc() and free() which nicely crashes your target after a few days as windoze goes to sleep periodically - due to heap fragmentation : - ) 

These work fine but give no indication however if there is an application connected on the VCP COM port.

[PHolt.1]

Just an update in case somebody finds this on google 100 years from now...

My last post above works great, together with VBUS (PA9) sensing. The mysterious USB hangs have disappeared.

But remember also to change the init and deinit functions to not use the heap. That is guaranteed to crash your target after days, or weeks, because windoze goes to sleep periodically and eventually the heap gets used up by fragmentation. The deinit function then is just an empty function because you don't need to unalloc a static buffer ; )

[Piranha]

Here is how the connection state of the device is determined for a TinyUSB, which is developed by competent people contrary to ST's "developers". And of course the the situation, when the data is transmitted, but the device is disconnected, is handled internally, and there is no necessity for such a Voodoo dancing with custom half-baked "fixes".

while (hcdc->TxState != 0)

This is still checking the driver state outside of the critical section and therefore is a bug. It must be inside the same critical section, in which the transmission is set up.

if ( g_USB_started && (Len>0) && (HAL_GPIO_ReadPin(GPIOA, GPIO_PIN_9)==1) )

Testing boolean types against non-zero values is a source for potential problems. Normally the test must be against "!= 0" or just leave it out like you've done it for the g_USB_started variable. Think about the consistency. But, the HAL_GPIO_ReadPin() function actually returns a type GPIO_PinState, which means that in this case the proper test is against "!= GPIO_PIN_RESET".

[PHolt.1]

That tinyusb article is incredibly complicated. I can't do anything with that.

The while (hcdc->TxState != 0) flow control method was proposed by yourself, IIRC. Why is it wrong? Isn't the code atomic? The flag TxState is toggled by the USB interrupt code.

The code is

uint8_t CDC_Transmit_FS(uint8_t* Buf, uint16_t Len, bool flow_control)
{
 
	if ( g_USB_started && (Len>0) && (HAL_GPIO_ReadPin(GPIOA, GPIO_PIN_9)==1) && g_CDC_FS_active )
	{
 
		osMutexAcquire(g_CDC_transmit_mutex, osWaitForever);
 
		USBD_CDC_HandleTypeDef *hcdc = (USBD_CDC_HandleTypeDef*)hUsbDeviceFS.pClassDataCDC;
		uint32_t timeout=0;
		bool error=false;
 
		// Wait on USB Host accepting the data.
		// A simple 1 sec timeout prevents a hang.
		if ( flow_control )
		{
			// Loop around until USB Host has picked up the previous data
			while (hcdc->TxState != 0)		// This gets changed by USB interrupt
			{
				osDelay(2);
				timeout++;
				if (timeout>500)
				{
					hcdc->TxState=0;
					error=true;
					break;
				}
			}
		}
 
		if (!error)
		{
			// Output the data to USB.
			// USB interrupts must be disabled around this.
			__disable_irq();
			USBD_CDC_SetTxBuffer(&hUsbDeviceFS, Buf, Len);
			USBD_CDC_TransmitPacket(&hUsbDeviceFS);
			__enable_irq();
		}
 
		osMutexRelease(g_CDC_transmit_mutex);
 
		g_comms_act[0]=LED_COM_TC;	// indicate data flow on LED 0
 
		// If flow control is selected OFF, we do a crude wait to make CDC output work
		// even with a slow Host.
		// In the usage context (outputting debugs to Teraterm) this actually needs to be only
		// a ~100-200us wait, but is host dependent. This gives us a 1-2ms delay between
		// *blocks* (typically whole lines).
		if ( !flow_control )
		{
			asm("nop");		// for breakpoints
			osDelay(2);
		}
 
	}
 
	return USBD_OK;
}

 If I put that TxState text inside the __disable_irq(); section, then interrupts could remain globally disabled for some indeterminate time.

Isn't it the case that once the transmit is open, it will not close unless you transmit some data? It would be really weird if it did that.

That test for VBUS seems to work fine. I am sure there are better tests for an "active host" but I haven't yet found something I can understand.

[Pavel A.]

>  I am sure there are better tests for an "active host" but I haven't yet found something I can understand.

Well, there is a number of known states of USB connection. The usable one is when the host actively polls an IN pipe for data or sends OUT data. One can detect other states (before enumeration, after enumeration but before the host reads the data (for example, before it reads the "serial port") , when the host application is busy or crashed.... when the host suspended the device....) and do something meaningful in these states.

[PHolt.1]

Sure, but that is a simple way to state something much more complicated : - )

BTW Piranha is right but looking at the HAL_GPIO_ReadPin() it cannot possibly return anything other than 0 or 1. It's stupid code, written by somebody who loves complication. It should return an int - 0 or 1.

[Piranha]

Had to look at a similar code based on ST's parody of a USB stack and therefore also took another look at Peter's implementation of CDC_Transmit_FS() function...

1. Even your comment says "USB interrupts", but, instead of disabling just the USB interrupts in NVIC, your code disables all interrupts.
2. Putting the wait loop inside the critical section would indeed be disastrous and, if you would still disable all interrupts, then the interrupt based delay functions would run into a deadlock and freeze the whole system. But the solution is ridiculously simple - instead of putting the wait loop inside the critical section, invert the logic and put the critical section inside the wait loop.
3. Not only the hcdc->TxState, but also the g_USB_started, g_CDC_FS_active and the VBUS pin must be checked and the LED managed inside the same critical section. And, by the way, in other parts of the code those should also be used in a similar critical section or interrupt with the same priority as USB interrupt. But actually this topic shows an easier and less dirtier way of checking the state of the USB device and the CDC port.
4. When the flow_control==false, your code doesn't check the hcdc->TxState at all and calls USB API unconditionally.
5. When the timeout of the wait loop runs out, the code sets hcdc->TxState=0, which is an even more gross violence of the design principles.
6. There is no need to wait for two OS ticks in the wait loop, because there is no need to guarantee a minimum wait time between any two iterations.
7. The additional delay at the end for the flow_control==false case must be inside the mutex protected section, so that it limits all threads, which try transmitting at the same time.
8. There is no need for a NOP instruction, because a breakpoint can be set on the next code line. Also your asm() statement is missing a volatile qualifier, which would not be the case, if you would use the CMSIS defined __NOP() macro.
9. The function always returns the same result code, which makes the return of the result useless.
10. Putting the whole function in a single IF block instead of an early return is a dumb way of structuring code as it makes it harder to read.

So here is my version, which fixes all of the points above:

uint8_t CDC_Transmit_FS(uint8_t* Buf, uint16_t Len, bool flow_control)
{
	if (!Len) {
		return USBD_OK;
	}

	USBD_StatusTypeDef ret = USBD_BUSY;
	USBD_CDC_HandleTypeDef *hcdc = (USBD_CDC_HandleTypeDef *)hUsbDeviceFS.pClassDataCDC;
	uint32_t timeout = flow_control ? 1000 : 1;

	osMutexAcquire(g_CDC_transmit_mutex, osWaitForever);

	do {
		NVIC_DisableIRQ(OTG_FS_IRQHandler);

		if ((hUsbDeviceFS.dev_state == USBD_STATE_CONFIGURED) &&
			(hUsbDeviceFS.ep0_state == USBD_EP0_STATUS_OUT) )
		{
			if (hcdc->TxState == 0) {
				g_comms_act[0] = LED_COM_TC;
				USBD_CDC_SetTxBuffer(&hUsbDeviceFS, Buf, Len);
				ret = USBD_CDC_TransmitPacket(&hUsbDeviceFS);
				timeout = 0;
			} else {
				--timeout;
			}
		} else {
			ret = USBD_FAIL;
			timeout = 0;
		}

		NVIC_EnableIRQ(OTG_FS_IRQHandler);

		if (timeout) {
			osDelay(1);
		}
	} while (timeout);

	if (!flow_control) {
		osDelay(2);
	}

	osMutexRelease(g_CDC_transmit_mutex);
	return ret;
}