[USB] Generic implementation providing HID composite (Mouse and Keyboard) and CDC in FS/HS #388

fpistm · 2018-12-12T16:05:57Z

This PR includes HID composite (Keyboard and Mouse) and CDC and will replace #91 and #344.
This new PR will be more generic, no more needs to have usb/ directory in the variant.
USB pins are in PeripheralPins.c, usbd_desc and usbd_conf are common and handle USB base and USB OTG FS/HS (also FS in HS).
This means all MCU with USB capabilities could be handled.
PR include your remarks and improvements (I hope).
Several define can be redefined in variant.h or thanks the build_opt.h including timer which by default is set to TIM6 as well the TIMx_IRQn (CDC_TIM and CDC_TIM_IRQn)
LPM is currently not managed as well the BOS descriptor.

Several tests will be required and also some other optimisation (ex: @edogaldo comments #344 (comment) )
USBSerial::write(const uint8_t *buf, size_t len) has been added but this have to be rewrote properly.
readBytes() could probably be added.

Anyway first tests are promising, see below.

First CDC tests performed (using pjrc USB benchmark):

Before Speed improvement from @hasenbanck and @ktand:

Disco F746
- FS : Average bytes per second = 31983 (Linux)
- HS: Average bytes per second = 254233 (Linux)

After Speed improvement from @hasenbanck and @ktand:

Disco F746
- FS: Average bytes per second = 786305 (Windows) / 959592 (Linux)
- HS: Average bytes per second = 3205058 (Windows) / 2972262 (Linux)
Disco F407
- FS: Average bytes per second = 790577 (Windows) / 845907 (Linux)

Note: Compare to previous PR, USB is not provided as a built-in library.
So, I did not add the reenumerate() as the usbd init is not the same. Any comments are welcome.
Updated implementation of reenumerate() could be something like that (don't know how manage for USB HS):

void USBSerial::reenumerate() {
  /* Re-enumerate the USB */
  volatile unsigned int i;
#ifdef USE_USB_HS_IN_FS
  uint32_t pinDP = pinNametoDigitalPin(USB_OTG_HS_DP);
#elif defined(USB_OTG_FS)
  uint32_t pinDP = pinNametoDigitalPin(USB_OTG_FS_DP);
#else /* USB */
  uint32_t pinDP = pinNametoDigitalPin(USB_DP);
#endif

  pinMode(pinDP, OUTPUT);
  digitalWrite(pinDP, LOW);
  for (i = 0; i < 1512; i++) {
  };
  pinMode(pinDP, INPUT);
  for (i = 0; i < 512; i++) {
  };
}

hasenbanck · 2018-12-13T07:30:31Z

Good work. But sadly I can't test the branch right now, since my only working STM32 prototype with USB support is right now at a beta testers place.

Could you maybe provide an example in the example project on how to use the features provided in thid PR?

fpistm · 2018-12-13T08:06:05Z

There is no special example.
Thanks the menu select the desired use case. HID (FS or HS) or CDC (FS or HS).
Then for HID you can try USB keyboard or mouse from Arduino sketches.
For CDC use SerialUSB instead of Serial (I will update later to have Serial linked to SerialUSB)

hasenbanck · 2018-12-13T08:10:21Z

Ah okay, I got confused them by your comment about the void USBSerial::reenumerate() method. If you can just use the Menu inside Arduino, that's even easier. PlatfromIO seem to get support for this core this week, so I'm interested to see how use this feature there.

fpistm · 2018-12-13T08:13:02Z

oh no. My concerns was about the reenumerate() provided by @ktand. I did not merge this part of his improvement as the usb init is not the same than you PR. (in fact USB is not moved as a builtin library).
So any comment on this part of code are welcome.

AnHardt · 2018-12-18T15:29:51Z

Compiling with;
Generic STN32F103 - BluePillF104C8 - 128kB - Enabled with generic Serial - CDC Full Speed
results in;

In file included from C:\Users\andre\AppData\Local\Arduino15\packages\STM32\hardware\stm32\1.4.0\cores\arduino\stm32\usb\cdc\usbd_cdc_if.c:24:0:

C:\Users\andre\AppData\Local\Arduino15\packages\STM32\hardware\stm32\1.4.0\cores\arduino\stm32\usb\cdc\usbd_cdc_if.c: In function 'CDC_disable_TIM_Interrupt':

C:\Users\andre\AppData\Local\Arduino15\packages\STM32\hardware\stm32\1.4.0\cores\arduino\stm32\usb\cdc\usbd_cdc_if.h:46:22: error: 'TIM6_IRQn' undeclared (first use in this function)

 #define CDC_TIM_IRQn TIM6_IRQn

.
Has only Timers up to TIM4.

fpistm · 2018-12-18T16:24:07Z

@AnHardt

Several define can be redefined in variant.h or thanks the build_opt.h including timer which by default is set to TIM6 as well the TIMx_IRQn (CDC_TIM and CDC_TIM_IRQn)

So use the build_opt.h and -DCDC_TIM=TIMx -DCDC_TIM_IRQn=TIMx_IRQn wit x the number of the timer you want

BennehBoy · 2018-12-19T13:54:21Z

How is CDC enabled within a sketch? Tried a few things but still get an unrecognized USB device in Windows.

fpistm · 2018-12-19T14:00:25Z

You enable it thanks the USB interface menu. Then it depends of your board. HS or FS.

BennehBoy · 2018-12-19T14:07:50Z

Ha, OK so I need to hack boards.txt for the BLACK407VET, thanks.

BennehBoy · 2018-12-19T14:30:07Z

So compiled with USB CDC FS option I can see the board as CDC FS & a Serial USB COM port in USBDeview but it is 'purple' and I cannot select the COM port in the IDE Serial monitor, is there some internal glue that needs to happen to get Serial talking via USB? (I noticed the re-enumerate comments above).

fpistm · 2018-12-19T15:57:27Z

Ha, OK so I need to hack boards.txt for the BLACK407VET, thanks.

If you get the full PR, the menu should be available.

So compiled with USB CDC FS option I can see the board as CDC FS & a Serial USB COM port in USBDeview but it is 'purple' and I cannot select the COM port in the IDE Serial monitor, is there some internal glue that needs to happen to get Serial talking via USB? (I noticed the re-enumerate comments above).

Nice it is recognized, anyway you should be able to select it.
To use it in Arduino sketch simply use SerialUSB instead of Serial or Serialx
Could you tell me which host OS version you used (I guess windows) ?
I will make a try as I have the black F407.
As said I need to perform mote tests before merge it so any help are welcome ;) Thanks

Note: purple on USBDeview means :

The device is connected. You must disconnect the device from USBDeview or from Windows "Safely Remove Hardware" option before you physically unplug it

BennehBoy · 2018-12-19T16:48:27Z

Host OS is Windows 10.

I have applied the full PR, there are no USB options for GenF4 in the included boards.txt, I just copied the appropriate lines.

Is there anyway to overload Serial to SerialUSB as in Roger's core?

BennehBoy · 2018-12-19T16:56:53Z

Windows 10

Still cannot use the COM port, simple sketch of:

void setup() {

  // put your setup code here, to run once:
SerialUSB.begin(57600);
}

void loop() {
  // put your main code here, to run repeatedly:
SerialUSB.println("Hello World!");
}

Tried also on WIndows 8.1 - OS fails to install drivers for the devices.

BennehBoy · 2018-12-19T17:08:00Z

Have also tried Ubuntu 16.04 LTS, I can See the CDC FS device but not the Serial device via 'sudo lsusb -v'

fpistm · 2018-12-19T20:18:44Z

@BennehBoy
I've reproduced your issue.
1/ I've forgot to add the USB menu to GENF4. I will add it.
2/ By default, same issue than you. USB recognized but COM port not available.
In fact this is due to the VID/PID. VID is set to 0xleaf in the boards.txt.
As I've hardcoded the PID in the core, it is not well recognized. As I guess for the 0xleaf PID, the PID for VCP is not the same.

Set PID in board.txt for GENF4 to 0x0483 solve this issue. 0x483 is for STMicroelectronics

Then VCP works fine.

I've made the benchmark:

ignoring startup buffering...
ignoring startup buffering...
ignoring startup buffering...
ignoring startup buffering...
ignoring startup buffering...
Bytes per second = 960077
Bytes per second = 960015
Bytes per second = 960015
Bytes per second = 960031
Bytes per second = 960031
Bytes per second = 960046
Bytes per second = 960031
Bytes per second = 960077
Bytes per second = 960000
Bytes per second = 768049
Bytes per second = 959985
Bytes per second = 960046
Bytes per second = 960031
Bytes per second = 960077
Bytes per second = 959985
Average bytes per second = 947233

So this seems to be a good results for USB FS.

BennehBoy · 2018-12-19T20:38:45Z

Made this change but I still cannot open the COM port :(

I will try on another Win10 machine and report back

BennehBoy · 2018-12-19T20:47:32Z

Here's what device manager says:

CDC in FS Mode

The drivers for this device are not installed. (Code 28)

There are no compatible drivers for this device.


To find a driver for this device, click Update Driver.

USB Serial Device

This device cannot start. (Code 10)

A device which does not exist was specified.

Same issue on 2 machines

fpistm · 2018-12-19T21:14:12Z

In this case it seems to be a driver issue.
Do you have this driver installed?
https://www.st.com/en/development-tools/stsw-stm32102.html
If yes then try to uninstall it as generic driver should be ok.

BennehBoy · 2018-12-19T22:52:34Z

OK, it must be driver related. The only driver packages I can see in programs & features are related to ST-Link, removing them does not help.

I can now see the COM port on a Windows 8.1 system which has never been used for Dev. I'll try again on a clean Win 10 machine tomorrow.

BennehBoy · 2018-12-19T23:30:00Z

I replaced the driver using Zadig and can now use the comport, however two devices are shown 1 of which is still inoperable. Getting there 👍

fpistm · 2018-12-20T07:05:56Z

I replaced the driver using Zadig and can now use the comport, however two devices are shown 1 of which is still inoperable. Getting there +1

Fine.
When you told you see 2 devices, is it under USBDeview ? Sometime some clean is required for USBDeview.

BennehBoy · 2018-12-20T08:00:39Z

It's actually 3, a composite, a Communication Device, and a CDC Data device. Don't get this on a Windows 8.1 machine. I'll try on a couple of Windows 10 machines later in case I've goosed this one.

Ubuntu 116.04 LTS Server I see a USB device (interface 0), but no tty gets mapped.

I have a spare machine that I'll boot from Another distro later to see how that copes.

ktand · 2018-12-20T18:36:07Z

@fpistm Just tried the PR and it works just fine in FS mode on the STM32F407 based Armed 3d printer board. Shows up as a "STMicroelectronics Virtual COM port" device in Windows 10, as expected.

@ktand

Added fixes as advised by @ktand * If the USB packet to be sent is equal to the USB buffer size (64 bytes), a Zero Length Packet must be sent. Otherwise the USB CDC connection will fail. * Add check for linestate in CDC_Flush and in CDC_TIM_PeriodElapsedCallback to ignore transmission if the device is disconnected.

Add USB menu for Generic F4 Change PID to 0x0483 Signed-off-by: Frederic.Pillon <[email protected]>

Signed-off-by: Frederic.Pillon <[email protected]>

See https://www.usb.org/defined-class-codes Well documented for Win10: https://docs.microsoft.com/en-us/windows-hardware/drivers/usbcon/usb-driver-installation-based-on-compatible-ids * If you want to load Usbser.sys automatically, set the class code to 02 and subclass code to 02 in the Device Descriptor. For more information, see USB communications device class (or USB CDC) Specification found on the USB DWG website. With this approach, you are not required to distribute INF files for your device because the system uses Usbser.inf. * If your device specifies class code 02 but a subclass code value other than 02, Usbser.sys does not load automatically. Pnp Manager tries to find a driver. If a suitable driver is not found, the device might not have a driver loaded. In this case, you might have to load your own driver or write an INF that references another in-box driver. * If your device specifies class and subclass codes to 02, and you want to load another driver instead of Usbser.sys, you have to write an INF that specifies the hardware ID of the device and the driver to install. For examples, look through the INF files included with sample drivers and find devices similar to your device. For information about INF sections, see Overview of INF Files. Signed-off-by: Frederic.Pillon <[email protected]>

@makarenya

Issue raised by @makarenya (see stm32duino#388) USB Specification EP0 should never STALL. Issue reproduced when using USB 3.0 port. Device is not recognized if STALL present. Signed-off-by: Frederic.Pillon <[email protected]>

BennehBoy · 2019-01-15T19:52:06Z

Not sure if this is an issue/optimisation opportunity for future PR's but compiling with CDC enabled results in ~10K more SRAM usage than without. That's a big hit for smaller boards...

fpistm · 2019-01-16T08:14:12Z

Right, this is mainly due to the buffer, hereafter .map extraction.

Source code	Name	Size dec	Size hex	Section	Address
core.a(usbd_cdc_if.c.o)	StackTxBuffer	2048	0x800	.bss	0x20000464
core.a(usbd_cdc_if.c.o)	UserTxBuffer	2048	0x800	.bss	0x20000c64
core.a(usbd_cdc_if.c.o)	UserRxBuffer	2048	0x800	.bss	0x20001464
core.a(usbd_conf.c.o)	g_hpcd	1008	0x3f0	.bss	0x20001ca4
core.a(usbd_interface.c.o)	hUSBD_Device_CDC	668	0x29c	.bss	0x20002194
core.a(usbd_desc.c.o)	USBD_StrDesc	256	0x100	.bss	0x20002094

This could probably optimized

BennehBoy · 2019-01-16T09:37:25Z

Ok I'll do a quick compare with Roger's core later to see what buffer size it uses.

…

On Wed, Jan 16, 2019 at 8:14 AM Frederic Pillon ***@***.***> wrote: Right, this is mainly due to the buffer, hereafter .map extraction. Source code Name Size dec Size hex Section Address core.a(usbd_cdc_if.c.o) StackTxBuffer 2048 0x800 .bss 0x20000464 core.a(usbd_cdc_if.c.o) UserTxBuffer 2048 0x800 .bss 0x20000c64 core.a(usbd_cdc_if.c.o) UserRxBuffer 2048 0x800 .bss 0x20001464 core.a(usbd_conf.c.o) g_hpcd 1008 0x3f0 .bss 0x20001ca4 core.a(usbd_interface.c.o) hUSBD_Device_CDC 668 0x29c .bss 0x20002194 core.a(usbd_desc.c.o) USBD_StrDesc 256 0x100 .bss 0x20002094 This could probably optimized — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#388 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AX0eXlLZSWLBUb5IytlGz-C57xQo-TpNks5vDt9WgaJpZM4ZPxzT> .

BennehBoy · 2019-01-16T10:47:08Z

Not sure if I'm barking up the wrong tree and these are sub buffers, but Serial TX & RX buffers for CDC are 256 bytes in rogers core (on F1 at least):

https://github.com/rogerclarkmelbourne/Arduino_STM32/blob/6a9c7956538892fa6fbe4a9ffc90d3c4bfab9441/STM32F1/cores/maple/libmaple/usb/stm32f1/usb_cdcacm.c#L263

makarenya · 2019-01-16T11:01:29Z

Pardon, but what about my PR (into this PR). I use smaller buffers (only 64b for transmitting and 128b for sending, when this PR uses 2048b buffer for receiving and TWO 2048b buffers for sending). My PR not used timers to determine the moment, when it can continue sending data. Instead, it uses a USB interrupt to detect this moment (which is somewhat more effective)

BennehBoy · 2019-01-16T11:03:52Z

I think your PR has conflicts - perhaps rebase now that this one is merged, and just include the changes - would be v welcome from me, or split into two? One for timer removal, one for buffer size reduction...

makarenya · 2019-01-16T11:14:56Z

Buffer sizes can be reduced in the current PR. They are simply chosen thoughtlessly large. Maximum size of USB packet in full speed mode can't be greater when 64b. So 128b is enough. For the high speed devices, maximum size of the buffer is 512b. so you need only 1k, not 2k.

Just write this define:

#if USE_USB_HS
#define CDC_BUFFER_SIZE (USB_HS_MAX_PACKET_SIZE * 2)
#else
#define CDC_BUFFER_SIZE (USB_FS_MAX_PACKET_SIZE * 2)
#endif

And you will have extra small and compact buffers for small devices and two time smaller buffers in devices with HS support (but they have more RAM, and it's not will a problem for it)

Timer reduction and extreme memory optimization PR i make in few days.

BennehBoy · 2019-01-16T11:20:34Z

I think you just need to rebase your existing PR against master - 388 has been merged already.

makarenya · 2019-01-16T11:23:51Z

Yes, give me few days, please!

BennehBoy · 2019-01-16T11:24:53Z

I'm not pressuring you 😃

fpistm · 2019-01-16T11:28:31Z

@makarenya don't worry I always planned to remove Timer as you suggested (and also @edogaldo)

roc2 · 2019-03-05T09:31:58Z

@fpistm It is not working when add Keyboard and Mouse in one sketch, but it work well when using Keyboard or Mouse standard-alone. They can't work together?? I test this with version 1.5.0, the testing code:

#include <Mouse.h>
#include <Keyboard.h>

void setup()
{
  Keyboard.begin();
  
  Mouse.begin();
}

void loop()
{
  Keyboard.write('w');

  Mouse.move(100,100);
  delay(1000);
  Mouse.move(200,300);
  
  delay(2000);
  
}

fpistm · 2019-03-05T09:45:21Z

@roc2 normally yes. I've tested with both on the same sketch.
Which host OS. Which board?...

roc2 · 2019-03-05T09:51:01Z

@fpistm My board is Black STM32F407VE, on win10 OS.

fpistm · 2019-03-05T13:40:46Z

Tested with 1.5.0 on Win10 it works.
Anyway it fails on repo master. Linked to F4 HAL update.
Edit: Some check has been added in HAL PCD. As init is called twice this lead to error.
This will be fixed to avoid multiple call to HID_Composite_Init.

@makarenya

Issue raised by @makarenya (see stm32duino#388) USB Specification EP0 should never STALL. Issue reproduced when using USB 3.0 port. Device is not recognized if STALL present. Signed-off-by: Frederic.Pillon <[email protected]>

[USB] Generic implementation providing HID composite (Mouse and Keyboard) and CDC in FS/HS

fpistm added the enhancement New feature or request label Dec 12, 2018

fpistm added this to the 1.5.0 milestone Dec 12, 2018

fpistm self-assigned this Dec 12, 2018

This was referenced Dec 12, 2018

CDC implementation #344

Closed

USB features #222

Closed

fpistm force-pushed the USB branch from ae6ea52 to 445a06c Compare December 13, 2018 08:35

fpistm force-pushed the USB branch from 445a06c to 274375d Compare December 20, 2018 09:18

hasenbanck and others added 6 commits January 11, 2019 09:47

[USB CDC] Add proposed speed improvements

a3eeb41

[USB] Update Generic F4

74982c5

Add USB menu for Generic F4 Change PID to 0x0483 Signed-off-by: Frederic.Pillon <[email protected]>

[USB] Update TIM definition

b417ed0

Signed-off-by: Frederic.Pillon <[email protected]>

[USB] Fix EPO STALL issue in STM32 USB Device library 2.5.1

eabb1b9

Issue raised by @makarenya (see stm32duino#388) USB Specification EP0 should never STALL. Issue reproduced when using USB 3.0 port. Device is not recognized if STALL present. Signed-off-by: Frederic.Pillon <[email protected]>

fpistm force-pushed the USB branch from 5c3d321 to eabb1b9 Compare January 11, 2019 08:47

fpistm merged commit b83969b into stm32duino:master Jan 11, 2019

fpistm removed the on going Currently work on this label Jan 11, 2019

fpistm deleted the USB branch January 12, 2019 18:57

makarenya mentioned this pull request Jan 21, 2019

Timers removal, rebased to master. Also fixed PMA memory distribution #410

Closed

BennehBoy mentioned this pull request Jan 26, 2019

USB enhancement #419

Merged

5 tasks

fpistm removed the help wanted 🙏 Extra attention is needed label Mar 5, 2019

benwaffle pushed a commit to benwaffle/Arduino_Core_STM32 that referenced this pull request Apr 10, 2019

Merge pull request stm32duino#388 from fpistm/USB

25f30a8

[USB] Generic implementation providing HID composite (Mouse and Keyboard) and CDC in FS/HS

[USB] Generic implementation providing HID composite (Mouse and Keyboard) and CDC in FS/HS #388

[USB] Generic implementation providing HID composite (Mouse and Keyboard) and CDC in FS/HS #388

Conversation

fpistm commented Dec 12, 2018 • edited Loading

First CDC tests performed (using pjrc USB benchmark):

Before Speed improvement from @hasenbanck and @ktand:

After Speed improvement from @hasenbanck and @ktand:

hasenbanck commented Dec 13, 2018

fpistm commented Dec 13, 2018 • edited Loading

hasenbanck commented Dec 13, 2018

fpistm commented Dec 13, 2018

AnHardt commented Dec 18, 2018

fpistm commented Dec 18, 2018

BennehBoy commented Dec 19, 2018

fpistm commented Dec 19, 2018

BennehBoy commented Dec 19, 2018

BennehBoy commented Dec 19, 2018

fpistm commented Dec 19, 2018 • edited Loading

BennehBoy commented Dec 19, 2018

BennehBoy commented Dec 19, 2018

BennehBoy commented Dec 19, 2018

fpistm commented Dec 19, 2018

BennehBoy commented Dec 19, 2018

BennehBoy commented Dec 19, 2018 • edited Loading

fpistm commented Dec 19, 2018

BennehBoy commented Dec 19, 2018 • edited Loading

BennehBoy commented Dec 19, 2018

fpistm commented Dec 20, 2018 • edited Loading

BennehBoy commented Dec 20, 2018

ktand commented Dec 20, 2018

BennehBoy commented Jan 15, 2019

fpistm commented Jan 16, 2019

BennehBoy commented Jan 16, 2019 via email

BennehBoy commented Jan 16, 2019

makarenya commented Jan 16, 2019 • edited Loading

BennehBoy commented Jan 16, 2019

makarenya commented Jan 16, 2019 • edited Loading

BennehBoy commented Jan 16, 2019

makarenya commented Jan 16, 2019

BennehBoy commented Jan 16, 2019

fpistm commented Jan 16, 2019

roc2 commented Mar 5, 2019 • edited Loading

fpistm commented Mar 5, 2019

roc2 commented Mar 5, 2019

fpistm commented Mar 5, 2019 • edited Loading

fpistm commented Dec 12, 2018 •

edited

Loading

fpistm commented Dec 13, 2018 •

edited

Loading

fpistm commented Dec 19, 2018 •

edited

Loading

BennehBoy commented Dec 19, 2018 •

edited

Loading

BennehBoy commented Dec 19, 2018 •

edited

Loading

fpistm commented Dec 20, 2018 •

edited

Loading

makarenya commented Jan 16, 2019 •

edited

Loading

makarenya commented Jan 16, 2019 •

edited

Loading

roc2 commented Mar 5, 2019 •

edited

Loading

fpistm commented Mar 5, 2019 •

edited

Loading