Fix STM32 flush of TX (used by UBL)

[randellhodges]

Description

With TX_BUFFER_SIZE > 0 there are calls in UBL to SERIAL_FLUSHTX which calls `SERIAL_OUT(flushTX) and the usb serial class does not implement flushTX.

I took @xC0000005 suggestion and created a wrapper class for this just like the existing HAL does for ESP32. serial.h has a #ifdef __STM32F1__ for the F1 boards, but there didn't seem to be a clean way for the general STM32 HAL.

If I overlooked an easier solution, by all means, reject/replace this PR.

Benefits

STM32 HAL will compile when the user has a TX_BUFFER_SIZE > 0 defined along with UBL (and anything else that may or in the future calls SERIAL_FLUSHTX.

Related Issues

#16104

[randellhodges]

Actually, couple of questions.

HAL_ESP32 inlines an empty function, basically a NOOP right? Inside serial.h, for __STM32F1__ it just calls SERIAL_OUT(flush). Which should we do?

I think an easier solution would be to put in a #define __STM32__ inside the HAL.h for HAL_STM32, then just do an #if EITHER(__STM32F1__,__STM32__) inside serial.h.

Edit: Actually, on reflection, the blank class might be better because it would handle if a person wanted do serial over something other than USB. If that something was flushable, then it would work. A define on the HAL level would disable it regardless, so I think my second idea isn't good.

I am still, however, curious if it should be a NOOP/Empty call, or just a call to the regular flush.

[tpruvot]

imo its not the right way to do that, flushtx is just flush on the F1... and all others STM look like...

current code in Marlin/src/core/serial.h

#ifdef __STM32F1__
  #define SERIAL_FLUSHTX()      SERIAL_OUT(flush)
#elif TX_BUFFER_SIZE > 0
  #define SERIAL_FLUSHTX()      SERIAL_OUT(flushTX)
#else
  #define SERIAL_FLUSHTX()
#endif

__STM32F1__ can be replaced by ARDUINO_ARCH_STM32 to include all STM32

[randellhodges]

Thanks for the feedback.

The only thing I can think of that you'd lose doing it that way, is those cases where the user isn't using the USB port to connect, they are using one of the UART ports for a direct connection.

An example is when someone directly connects their PI UART to UART. It isn't generally recommended, but it happens.

Subclassing the USB class would handle both cases (but probably should change the subclass to call the flush and not just noop it?).

Then again, how many people are going to be connecting a non usb serial device that needs to be flushed to an STM32 board? Changing the #ifdef is much simpler for certain.

[randellhodges]

I updated so calls the based flush() method. This more closely minimics the SERIAL_FLUSHTX as defined for the STM32F1 platform.

I noted the reason I followed the ESP32 HAL pattern. Of course, if you prefer to use @tpruvot suggestion, that works too.

For me, I'll always use a USB connection. So either option will work for me.

[tpruvot]

the problem is not only for usb... its for all serial... The class addition is overcomplicated imo, and still incomplete

[randellhodges]

You only recommend changing the serial.h macro define?

I might be making a mountain out of a molehill on this, or my understanding might be flawed, however, it seems that when SERIAL_FLUSHTX is called, the underlying serial, which might be USB/WiFi/MarialSerial/ or a framework serial is called, we would want to call the flushTX if implemented. Maybe there is a better C++ way that I don't know.

A change in the serial.h define, would mean that, for a platform, it would ignore the underlying serial device and never use a buffered flush.

Am I overthinking it?

[tpruvot]

Am I overthinking it?

Yep, as flush RX has no real meaning, a flush is to force a buffer to be wrote.. to avoid a delay like fsync on the filesystem... flush tx seems a command originally used only for one platform... nothing standard nor bugging imo... just use ARDUINO_ARCH_STM32 and will be fine, its only used once in marlin, for UBL and was not the only problem to build it on STM platforms

[randellhodges]

Change made.

I wonder if either it is time to revisit the need for SERIAL_FLUSHTX altogether, or instead of opting out with the define, opt in? That way, other platforms won't have the issue? Not in this PR anyway.

[thinkyhead]

I wonder if either it is time to revisit the need for SERIAL_FLUSHTX altogether…

It may not really be needed. The flushing in UBL is due to the original developer having crashing issues with Pronterface on PC which were solved by adding delays after serial out, or by flushing the output buffer. I've never heard of any other instances of bad host behavior like this, so it may have just been a quirk of that developer's PC setup.