I've only done limited testing on this PR so far. I only have access to an F4 board with native USB, which has internal pullups and works well. I did do compile testing for a few other boards.

I've ordered a blue pill and black pill to test, but it would be good to also test on a maple mini board (since that has external pullup) and the MALYAN M200 V1 (since that used to have special pullup handling).@xC0000005 maybe you could do the latter?

(I also did a few force pushes to fix style errors, should be good now).

And I will test on Maple. 😉

Oh, remove the _INPUT, that should come from here:https://github.com/stm32duino/Arduino_Core_STM32/pull/997/files#diff-d707080ac76ca2646a4a2457bcb86ad9R44

Sorry for the short comment earlier, I was just on my way out.

Turns out with my earlier compile-testing, I had forgotten to enable USB (for our custom board we have it enabled by default, didn't realize that the default boards did not). I did that now and found a dozen other preprocessor typos, I messed up the _PIN to _PINNAME rename and some other stuff. I should really have tested this more thoroughly, sorry.

Anyway, I pushed a fixup commit that at least compiles properly for the bluepill, maple mini and M200, so that should make testing easier.

Thanks!

Thanks for testing :-)

I've done another push, to squash the fixup commit I pushed yesterday (only changes history, not the actual code).

Tested with a Maple mini and works as expected.

Thanks for the testing and reviews. I've also tested on a bluepill, which works as expected too.

There is a remaining definition of USB_DISC_PIN in variant: Generic_F401Rx

Ah, I started working on top of an older branch where that variant did not exist yet. I'll convert it.

The new macros also take PinName constants rather than pin numbers. This
allows merging the code that controls an external pullup with the code
that writes directly to the USB DP pin to fake a disabled pullup (this
code only knows the pin name of the DP pin, not the number).

I thought a bit more on this, but I think it might be nicer to specify pin numbers rather than pin names in the variant, which is more consistent and thus also less likely to cause mistakes. In terms of code that just means the PIN constants must be redefined as PINNAME constants in usbd_if.c, which should just be a few more lines but nothing spectacular (and just one extra lookup at runtime, but that's ok).

Ok, I pretty much rewrote the entire thing. In a nutshelll:

• All three hardware configurations (builtin pullups, external fixed pullups and external controllable pullups) are now handled in the same way: Detach, wait, attach.
• A variant-specified control pin is now always push-pull, the "open-collector" treatment is applied only for the D+ output trick.
• Variants can now just configure pin numbers instead of pin names.

I tested this on an F3 with internal pullups and on a bluepill. Testing on the MALYAN printers and a maple mini again would be appreciated.

For some more details about what I did and tried, read on.

Nitpicking: those sanity check should be under switch : #ifndef USBD_REENUM_DISABLED

I now moved all of the code under this check, with just an emptyUSBD_reenumerate() function in the else.

Internal pullups

I found that the internal pullups can be controlled portably using theUSB_DevConnect andUSB_DevDisconnect LL HAL functions, which the code now does.

Looking through the HAL code I found that the SDIS bit is used only by "USB_OTG_FS" and "USB_OTG_HS" peripherals, apparently some "USB" peripherals have an internal pullup controlled by the "USB_BCDR_DPPU" bit, so I adapted the code to check for both. I usedgit grep -A 10 USB_DevConnect.*\)$ to check that no other bits are used for pullup control.

Even more, I found thatall USB_OTG_HS and USB_OTG_FS peripherals have the SDIS bit, so there is really no need to have code to figure out the D+ trick for them (so I removed that code). I confirmed this observation using acheck script I wrote today which compiles a short header file against all known STM32 cpus (see the script for info). I let the script compile this, which worked for all boards:

#include <stm32_def.h>#if (defined(USB_OTG_FS) || defined(USB_OTG_HS)) && !defined(USB_OTG_DCTL_SDIS)#error "Foo"#endif

I have not been able to test the DPPU bit, in particular I'm not sure how it behaves on startup when the USB peripheral is not enabled yet (I think it will just be a no-op, but it might end up enabling the USB pullup earlier than normal, though that should not be a problem either. I see that the STM32F070x6/B has this bit, which includes the MALYAN M200 V2, so that provides a nice opportunity to test this (also, it explains why these V2 boards have no external pullup, since it is included in the CPU now).

I also considered the option of overriding theUSB_DevConnect andUSB_DevDisconnect functions with our custom code to control the external pullup (or apply the D+ trick), since they arealways called from the PCD code (even on chips that do not have internal pullups). However, the dummy definitions of these functions on chips without internal pullups are not defined weak, so we cannot actually override them.

Re-enumerating while connected

The code as it is now, should be usable to re-enumerate while running (instead of just at reset) too. I don't think we should actively advertise this, but it is interesting to know if it would work.

This works well with the F301RE with SDIS support I have, so it seems the USB stack handles external re-attachment fine. There did seem to be a small issue, possibly with buffering while disconnected (I got some duplicate serial output when the port was reconnected), but I did not investigate this.

I also tried on a bluepill, which has the D+ output trick. Switching to OUTPUT and back to INPUT would of course not work, since then the USB peripheral would lose control of the pin. So I tried using the USB pinmap to switch the pin back to USB AF mode.

However, detaching by switching to OUTPUT did not even work on F103C8, since it seems that the USB peripheral (when enabled) forcibly claims the DP pin, regardless of the AF settings. Even when I switch it to output and leave it at that, USB serial keeps working normally. Even more, thepin map in the variant does not even bother to set the pins into AF mode and just leaves them in input (verified using debugger, so there is no bit of code elsewhere thatdoes initialize to AF). I could not find any reference to this in the documentation, andjust one mention of it online, but that thread quickly focuses on getting things to work on hardware that doesnot seem to misbehave in this way. Since I could not test this properly, and the code was a bit fragile anyway (it would put pins in USB mode during startup, way before the USB peripheral was initialized), I reverted this, the pin mode is now reverted back to INPUT and intended to work at reset time only.

I have no board with DPPU support to test with, though. In case anyone wants to test, here is the sketch I used:

void setup() {  // put your setup code here, to run once:  Serial.begin(115200);  while (!Serial);}unsigned i = 0;extern "C" void USBD_reenumerate(void);void loop() {  // put your main code here, to run repeatedly:  Serial.println(i++);  delay(1000);  if (i % 10 == 0) {      USBD_reenumerate();  }}

I tested your sketch with PNUCLEO_WB55RG which supports DPPU , it is working !!

Testing on the MALYAN printers and a maple mini again would be appreciated.

@matthijskooijman I've tested the maple and it works as expected with your sketch.

@xC0000005 did you made the test on Mylyan ?

W00ps, forgot about the typo. Force pushed a version just fixing the typo, should be good to merge now.