cnlohr / rv003usb

CH32V003 RISC-V Pure Software USB Controller
MIT License
436 stars 43 forks source link

bootloader not enumerating on Surface Pro 6/7 #24

Closed xsrf closed 8 months ago

xsrf commented 11 months ago

So after messing around in https://github.com/cnlohr/rv003usb/issues/22 I found that the bootloader won't enumerate on my Microsoft Surface Pro 6 and 7 devices with Windows 10 (with or without dock and with or without USB Hub in between). Windows 10 will complain about Invalid Configuration Descriptor. The demo_composite_hid works fine though! Three other Windows 10 devices I tried (2 Dell, 1 Fujitsu) had no issues with the bootloader enumerating. Any ideas?

cnlohr commented 11 months ago

I do not have time to address this for a bit, but, I am curious if anyone finds a solution. It is possible I could have bespoke code in the boot loader for configuring ports, because I tried making it as compressed as possible.

cnlohr commented 11 months ago

I regret that without actual hardware myself I can't repro such issues. I would apprecate any further robustness you can add to this project, but for now, I will close this issue.

xsrf commented 10 months ago

@cnlohr I found the issue. I changed the size in https://github.com/cnlohr/rv003usb/blob/a75e557b558e9407c05fe6e08f8f463201bfa095/bootloader/usb_config.h#L88 from 0x40 to 0x04 just because it looked right by comparing with other configs. I really don't know what size this refers to since I didn't dig deeper into how USB descriptors work. However, with this changed, it now also enumerates fine on my Surface 6 and 7. I won't do a PR since I have no clue what I changed. I may if you confirm this is actually the cause ;)

xsrf commented 10 months ago

It can't upload code though...

../../ch32v003fun/../minichlink/minichlink -w blink.bin flash -b
Found B003Fun Bootloader
Halting Boot Countdown
Warning: Issue with hid_send_feature_report. Retrying
Warning: Issue with hid_send_feature_report. Retrying
Warning: Issue with hid_send_feature_report. Retrying
Warning: Issue with hid_send_feature_report. Retrying
Warning: Issue with hid_send_feature_report. Retrying
Warning: Issue with hid_send_feature_report. Retrying
Warning: Issue with hid_send_feature_report. Retrying
Warning: Issue with hid_send_feature_report. Retrying
Warning: Issue with hid_send_feature_report. Retrying
Warning: Issue with hid_send_feature_report. Retrying
Warning: Issue with hid_send_feature_report. Retrying
Warning: Issue with hid_send_feature_report. Retrying
Could not setup interface.
make: *** [../../ch32v003fun/ch32v003fun.mk:66: cv_flash] Error -33
Nable80 commented 10 months ago

@xsrf I think you should try to use 0x08 instead. 0x40 (64) looks right for full-speed devices but it may be a problem for low-speed ones, AFAIR they are limited to 8-byte transfers.

xsrf commented 10 months ago

@xsrf I think you should try to use 0x08 instead. 0x40 (64) looks right for full-speed devices but it may be a problem for low-speed ones, AFAIR they are limited to 8-byte transfers.

Could you explain what this size refers to? Is it something like the max packet size? Can you explain why my surface 6 and 7 would report "Invalid Configuration Descriptor" for 0x40 but not for 0x04?

Nable80 commented 10 months ago

Could you explain what this size refers to? Is it something like the max packet size?

Yes, exactly, it corresponds to a wMaxPacketSize field: https://www.beyondlogic.org/usbnutshell/usb5.shtml#:~:text=of%20the%20interface.-,Endpoint%20Descriptors,-Endpoint%20descriptors%20are

Can you explain why my surface 6 and 7 would report "Invalid Configuration Descriptor" for 0x40 but not for 0x04?

https://www.beyondlogic.org/usbnutshell/usb4.shtml#Interrupt:~:text=The%20maximum%20data%20payload%20size%20for%20low%2Dspeed%20devices%20is%208%20bytes. The maximum data payload size for low-speed devices is 8 bytes, full-speed devices are limited to 64 bytes. This is mandated by the specification but I definitely remember full-speed devices that announced larger values and both host controller and OS were OK with it. I guess the 8-byte limit for low-speed devices isn't too strict on common hardware too. Maybe MS Surface host controller hardware (or drivers) are too strict and refuse to work with this violation.

I'm sorry for being that lazy, I should have provided some explanatory links from the very beginning.

Nable80 commented 10 months ago

The demo_composite_hid works fine though!

FYI: this demo specifies wMaxPacketSize=4 for mouse endpoint and 8 for keyboard: https://github.com/cnlohr/rv003usb/blob/master/demo_composite_hid/usb_config.h#L156 https://github.com/cnlohr/rv003usb/blob/master/demo_composite_hid/usb_config.h#L183

xsrf commented 10 months ago

@Nable80 I've now tried different things:

wMaxPacketSize 0x04 / 0x08 enumerate on Dell and MS Surface. wMaxPacketSize 0x40 / 0x0A enumerate on Dell but NOT on MS Surface.

minichlink on Surface with wMaxPacketSize 0x04 / 0x08: Warning: Issue with hid_send_feature_report. Retrying

minichlink on Dell with wMaxPacketSize 0x04 / 0x08: Warning: Issue with hid_send_feature_report. Retrying minichlink on Dell with wMaxPacketSize 0x40 (default): Error: Could not initialize any supported programmers 😕

xsrf commented 10 months ago

I've now set-up a third PC (Fujitsu). Same result as on Dell. The original bootloader, completely unmodified on @cnlohr s CH32V003FUN board, and on my own board with pins modified for PORTC, enumerates fine on Dell and Fujitsu, but minichlink won't even recognize it. On the other hand, my bootloader with modified wMaxPacketSize gets recognized by minichlink on all PCs, but with the error above...

xsrf commented 10 months ago

Here are my binaries, with the default pin mapping of this repository / for the CH32V003FUN board. bootloader.zip

Nable80 commented 10 months ago

I guess minichlink and/or bootloader was designed with the assumption that we can always send X bytes in one packet, where 8 < X ≤ 64, unfortunately I'm not familiar enough with this code (I don't even have CH32 MCUs yet…) to quickly find how to fix this. Let's hope @cnlohr suggests something after looking through these updates.

Nable80 commented 10 months ago

@xsrf I think I should also suggest you to rebuild minichlink with enabled debugging (uncomment #define DEBUG_B003 and anything similar) and maybe with additional printf calls somewhere in and around the hid_send_feature_report function. It'll help us to understand what transfer sizes are used and what data is sent/received/expected. I don't know whether you find it difficult or acceptable. Unfortunately I can't provide the exact patch now (there are more urgent things to do), but it's really interesting to dig into this issue.

xsrf commented 10 months ago

So, I've recompiled minichlink.exe with debugging and this is what I get now with the bootloader set to 0x08 packet size:

\ch32v003fun\minichlink>minichlink.exe -a -w ws2812bdemo.bin flash -B
Found B003Fun Bootloader
Halting Boot Countdown
Commit TX: 128 bytes
aa 00 00 00 81 46 94 c1 fd 56 14 c1 82 80 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 cd ab 34 12

hid_send_feature_report = -1
Warning: Issue with hid_send_feature_report. Retrying
hid_send_feature_report = -1
Warning: Issue with hid_send_feature_report. Retrying
hid_send_feature_report = -1
Warning: Issue with hid_send_feature_report. Retrying
hid_send_feature_report = -1
Warning: Issue with hid_send_feature_report. Retrying
hid_send_feature_report = -1
Warning: Issue with hid_send_feature_report. Retrying
hid_send_feature_report = -1
Warning: Issue with hid_send_feature_report. Retrying
hid_send_feature_report = -1
Warning: Issue with hid_send_feature_report. Retrying
hid_send_feature_report = -1
Warning: Issue with hid_send_feature_report. Retrying
hid_send_feature_report = -1
Warning: Issue with hid_send_feature_report. Retrying
hid_send_feature_report = -1
Warning: Issue with hid_send_feature_report. Retrying
hid_send_feature_report = -1
Warning: Issue with hid_send_feature_report. Retrying
hid_send_feature_report = -1
Warning: Issue with hid_send_feature_report. Retrying
Could not setup interface.

The wireshark capture from the USB interface is also attached. I've filtered all devices that transmitted data when the bootloader was not connected, so it should be just the bootloader and probably my USB Hub in between. minichlink_B003Fun_0x08_hid_send_feature_report_20231024_1519.zip

I've changed the bootloader to be active for 15s for the capture.

xsrf commented 10 months ago

So, https://github.com/cnlohr/ch32v003fun/blob/85863e31ef2effd6c310b5fdfaeea01aa4960401/minichlink/pgm-b003fun.c#L156 always returns -1. I tried sending just 4 bytes instead of all 128 - same result. I also changed the first 0xaa to 0x00 because docs for hid_send_feature_report say that's required for devices without numbered feature reports. Still same.

Nable80 commented 10 months ago

Thank you for collecting detailed debug data. Do you have any PC where minichlink actually works with the default value (0x40)? You can collect a wireshark dump there and compare these two dumps.

Another option is to use a USB-UART dongle, it's possible to use it for debug output to find out what actually happens (does it receive anything from the host at all) on the bootloader side.

xsrf commented 10 months ago

Do you have any PC where minichlink actually works with the default value (0x40)?

No. I have two PCs where the bootloader enumerates fine (I use Nirsoft USBLogView btw.) but minichlink won't even recognize it. It just fails with

minichlink>minichlink.exe -a -w ws2812bdemo.bin flash -B
Error: Could not initialize any supported programmers
Error: Could not initialize any supported programmers

which is like no programmer is connected at all. It must already fail here: https://github.com/cnlohr/ch32v003fun/blob/85863e31ef2effd6c310b5fdfaeea01aa4960401/minichlink/minichlink.c#L62C19-L62C19 Which implies it could already fail here: https://github.com/cnlohr/ch32v003fun/blob/85863e31ef2effd6c310b5fdfaeea01aa4960401/minichlink/pgm-b003fun.c#L546 I'll debug that...

I could also connect my logic analyzer and capture the raw data on the wire ...

xsrf commented 10 months ago

Yep, confirmed. With 0x40 in the bootloader it already fails at hid_open() despite a HID device with the correct PID and VID being present in device manager and Nirsoft USBLogView. So I guess Windows in this case may somehow block access to the device, like the USB Host controller did on my Microsoft Surface 6/7?

Nable80 commented 10 months ago

Can you can step into the hid_open_path() while debugging, please? This function consists of several steps (system library calls, each of which could fail), determining exact step and error code could help you to find out what should be done in order to make Windows working. I also noticed there are at least 2 WinAPI-specific implementations of HID functions inside the minichlink code: the first one uses hid.dll, another one is enabled by uncommenting define HIDAPI_USE_DDK. I wonder if there are any differences in their limitations.

xsrf commented 10 months ago

@Nable80 I guess I could, but do you see any benefits? By now I'm pretty sure that wMaxPacketSize > 8 is the reason why all of my 5 Windows systems I tested refused to cummunicate with the device. I'd prefer to go on with wMaxPacketSize = 8 and try to get that work. Or do you have a good reason to investigate the issue with 0x40 further?

xsrf commented 10 months ago

Proceeding with a bootloader set to wMaxPacketSize = 8 I found that HidP_GetCaps() in hid_open_path() returns OutputReportByteLength = InputReportByteLength = 0. After that, hid_write() or hid_send_feature_report() fail when trying to write to the device - probably because windows won't send anything if OutputReportByteLength = 0 🤔 WriteFile() returns 0 and GetLastError() is not ERROR_IO_PENDING, so hid_write() fails. This is also reflected by the Wireshark captures. It's not sending anything.

Nable80 commented 10 months ago

Or do you have a good reason to investigate the issue with 0x40 further?

Nope, I definitely meant it makes sense to debug the wMaxPacketSize = 8 case further and to make it working (it's weird that device is definitely detected by OS but minichlink says otherwise). I re-read my previous post and now I see it wasn't clear enough, sorry.

I wonder what data was read by HidD_GetPreparsedData into the buffer that is pointed by pp_data, was it truncated due to 8-byte limit (and some loop should be added in order to read it completely) or there's another reason for HidP_GetCaps to put bogus values into caps, e.g. the bootloader's USB descriptors must be corrected somehow.

xsrf commented 10 months ago

That's what USBview actually gives me 🤔

[Port2]  :  USB-Eingabegerät

Is Port User Connectable:         yes
Is Port Debug Capable:            no
Companion Port Number:            0
Companion Hub Symbolic Link Name: 
Protocols Supported:
 USB 1.1:                         yes
 USB 2.0:                         yes
 USB 3.0:                         no

Device Power State:               PowerDeviceD3

       ---===>Device Information<===---
English product name: "rv003usb"

ConnectionStatus:                  
Current Config Value:              0x01  -> Device Bus Speed: Low
Device Address:                    0x17
Open Pipes:                           1

          ===>Device Descriptor<===
bLength:                           0x12
bDescriptorType:                   0x01
bcdUSB:                          0x0110
bDeviceClass:                      0x00  -> This is an Interface Class Defined Device
bDeviceSubClass:                   0x00
bDeviceProtocol:                   0x00
bMaxPacketSize0:                   0x08 = (8) Bytes
idVendor:                        0x1209 = Vendor ID not listed with USB.org
idProduct:                       0xB003
bcdDevice:                       0x0002
iManufacturer:                     0x01
     English (United States)  "cnlohr"
iProduct:                          0x02
     English (United States)  "rv003usb"
iSerialNumber:                     0x03
     English (United States)  "NBTT"
bNumConfigurations:                0x01

          ---===>Open Pipes<===---

          ===>Endpoint Descriptor<===
bLength:                           0x07
bDescriptorType:                   0x05
bEndpointAddress:                  0x81  -> Direction: IN - EndpointID: 1
bmAttributes:                      0x03  -> Interrupt Transfer Type
wMaxPacketSize:                  0x0008
bInterval:                         0xFF

       ---===>Full Configuration Descriptor<===---

          ===>Configuration Descriptor<===
bLength:                           0x09
bDescriptorType:                   0x02
wTotalLength:                    0x0022  -> Validated
bNumInterfaces:                    0x01
bConfigurationValue:               0x01
iConfiguration:                    0x00
bmAttributes:                      0x80  -> Bus Powered
MaxPower:                          0x64 = 200 mA

          ===>Interface Descriptor<===
bLength:                           0x09
bDescriptorType:                   0x04
bInterfaceNumber:                  0x00
bAlternateSetting:                 0x00
bNumEndpoints:                     0x01
bInterfaceClass:                   0x03  -> HID Interface Class
bInterfaceSubClass:                0xFF
bInterfaceProtocol:                0xFF
iInterface:                        0x00

          ===>HID Descriptor<===
bLength:                           0x09
bDescriptorType:                   0x21
bcdHID:                          0x0110
bCountryCode:                      0x00
bNumDescriptors:                   0x01
bDescriptorType:                   0x22 (Report Descriptor)
wDescriptorLength:               0x000F

          ===>Endpoint Descriptor<===
bLength:                           0x07
bDescriptorType:                   0x05
bEndpointAddress:                  0x81  -> Direction: IN - EndpointID: 1
bmAttributes:                      0x03  -> Interrupt Transfer Type
wMaxPacketSize:                  0x0008
bInterval:                         0xFF
Nable80 commented 10 months ago

It matches the bootloader code but it doesn't show what was actually fetched by HidD_GetPreparsedData in minichlink, you can place a breakpoint in debugger and see the contents of local variables at the moment between HidD_GetPreparsedData and HidP_GetCaps calls, and after the HidP_GetCaps call too. Maybe there's something wrong with the way how minichlink calls WinAPI functions, maybe something is missing in descriptors but I don't even have a Windows PC with development tools in order to debug in on my side. If I had such setup, I'd definitely debug it myself. Unfortunately I can only give some debugging tips for now.

xsrf commented 10 months ago

I don't have a proper debugger set up, but here are a few dumps:

// code
    dev->output_report_length = caps.OutputReportByteLength;
    dev->input_report_length = caps.InputReportByteLength;
    //dev->output_report_length = 8;
    int i;
    printf( "pp_data: %lu bytes\n", 128  );
    for( i = 0; i < 128 ; i++ )
    {
        printf( "%02x ", ((uint8_t*)&pp_data)[i] );
        if( ( i & 0xf ) == 0xf ) printf( "\n" );
    }
    if( ( i & 0xf ) != 0xf ) printf( "\n" );
// outputs
pp_data: 128 bytes
d0 de 55 00 00 00 00 00 ff 00 01 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 01 00 00 00 00 00 00 00 00 00 00 00
00 00 01 00 00 00 01 00 20 1d ac 00 00 00 00 00
c0 fb 14 00 00 00 00 00 fb a1 40 00 00 00 00 00
80 74 ac 00 00 00 00 00 03 b0 00 00 00 00 00 00
00 00 00 00 00 00 00 00 01 00 00 00 00 00 00 00

pp_data: 128 bytes
30 d3 53 00 00 00 00 00 ff 00 01 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 01 00 00 00 00 00 00 00 00 00 00 00
00 00 01 00 00 00 01 00 20 1d a2 00 00 00 00 00
c0 fb 14 00 00 00 00 00 fb a1 40 00 00 00 00 00
80 74 a2 00 00 00 00 00 03 b0 00 00 00 00 00 00
00 00 00 00 00 00 00 00 01 00 00 00 00 00 00 00

pp_data: 128 bytes
40 dd 63 00 00 00 00 00 ff 00 01 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 01 00 00 00 00 00 00 00 00 00 00 00
00 00 01 00 00 00 01 00 20 1d 1e 00 00 00 00 00
c0 fb 14 00 00 00 00 00 fb a1 40 00 00 00 00 00
80 74 1e 00 00 00 00 00 03 b0 00 00 00 00 00 00
00 00 00 00 00 00 00 00 01 00 00 00 00 00 00 00

I don't think this is relevant though. I can just hardcode output_report_length = 8 and it won't help. It's actually just used in https://github.com/cnlohr/ch32v003fun/blob/85863e31ef2effd6c310b5fdfaeea01aa4960401/minichlink/hidapi.c#L720 but since

if (length >= dev->output_report_length) {
        /* The user passed the right number of bytes. Use the buffer as-is. */
        buf = (unsigned char *) data;
    } else { ...

always returns true for output_report_length = 0 it's not relevant. The actual WriteFile() call that fails doesn't know about output_report_length

Nable80 commented 10 months ago

I guess 128 != sizeof(HIDP_PREPARSED_DATA) but I hope it would be enough to cover the whole structure. There's another problem: ppdata is already a pointer, I think you didn't need to add & before it.

I can just hardcode output_report_length = 8 and it won't help.

Yes, of course. But it's not what I meant. I think there's something wrong with the the descriptors and Win32 kernel doesn't like it and prohibits any further interaction. Some checks aren't passing. HidP_GetCaps returned bogus values, so at first I thought some of these checks could be found there. Upd: no, they aren't there, it just copies input/output/feature report lengths (and other data) from preparsed data to a HIDP_CAPS structure. HidD_GetPreparsedData is another copying wrapper too, it just returns what was read by IOCTL_HID_GET_COLLECTION_INFORMATION and IOCTL_HID_GET_COLLECTION_DESCRIPTOR and these ioctl's are another wrappers… But I think I'm starting to understand where it all goes, we have some bogus/missing values here: https://github.com/cnlohr/rv003usb/blob/master/bootloader/usb_config.h#L38-L47

I don't see HID_REPORT_SIZE (and HID_REPORT_COUNT is a different parameter), 0xAA/0xFF values look suspicious, etc.

Here's a HID descriptor from some real device (a mouse?) that I found in the Internet (here, to be exactly): 05 01 09 02 A1 01 09 01 A1 00 05 09 19 01 29 08 15 00 25 01 95 08 75 01 81 02 95 00 81 03 06 00 FF 09 40 95 02 75 08 15 81 25 7F 81 02 05 01 09 38 15 81 25 7F 75 08 95 01 81 06 09 30 09 31 16 01 F8 26 FF 07 75 0C 95 02 81 06 C0 C0

You can decode it using https://eleccelerator.com/usbdescreqparser/ or any other suitable tool:

0x05, 0x01,        // Usage Page (Generic Desktop Ctrls)
0x09, 0x02,        // Usage (Mouse)
0xA1, 0x01,        // Collection (Application)
0x09, 0x01,        //   Usage (Pointer)
0xA1, 0x00,        //   Collection (Physical)
0x05, 0x09,        //     Usage Page (Button)
0x19, 0x01,        //     Usage Minimum (0x01)
0x29, 0x08,        //     Usage Maximum (0x08)
0x15, 0x00,        //     Logical Minimum (0)
0x25, 0x01,        //     Logical Maximum (1)
0x95, 0x08,        //     Report Count (8)
0x75, 0x01,        //     Report Size (1)
0x81, 0x02,        //     Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position)
0x95, 0x00,        //     Report Count (0)
0x81, 0x03,        //     Input (Const,Var,Abs,No Wrap,Linear,Preferred State,No Null Position)
0x06, 0x00, 0xFF,  //     Usage Page (Vendor Defined 0xFF00)
0x09, 0x40,        //     Usage (0x40)
0x95, 0x02,        //     Report Count (2)
0x75, 0x08,        //     Report Size (8)
0x15, 0x81,        //     Logical Minimum (-127)
0x25, 0x7F,        //     Logical Maximum (127)
0x81, 0x02,        //     Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position)
0x05, 0x01,        //     Usage Page (Generic Desktop Ctrls)
0x09, 0x38,        //     Usage (Wheel)
0x15, 0x81,        //     Logical Minimum (-127)
0x25, 0x7F,        //     Logical Maximum (127)
0x75, 0x08,        //     Report Size (8)
0x95, 0x01,        //     Report Count (1)
0x81, 0x06,        //     Input (Data,Var,Rel,No Wrap,Linear,Preferred State,No Null Position)
0x09, 0x30,        //     Usage (X)
0x09, 0x31,        //     Usage (Y)
0x16, 0x01, 0xF8,  //     Logical Minimum (-2047)
0x26, 0xFF, 0x07,  //     Logical Maximum (2047)
0x75, 0x0C,        //     Report Size (12)
0x95, 0x02,        //     Report Count (2)
0x81, 0x06,        //     Input (Data,Var,Rel,No Wrap,Linear,Preferred State,No Null Position)
0xC0,              //   End Collection
0xC0,              // End Collection
// 77 bytes

Report Size = 8 looks promising (maybe it's in bits, though), yet I don't understand what HID_REPORT_COUNT (Report Count) means, it's time to read the USB HID spec… I'll try to study other examples from https://github.com/tmk/tmk_keyboard/wiki/USB:-HID-Report-Descriptor later, they look promising.

xsrf commented 10 months ago

@Nable80 is the HID report descriptor even relevant? We defined endpoint 1 as an IN endpoint (0x81 // bEndpointAddress (IN/D2H)), it can only send data from the device to the host. We're having trouble sending to the host, though. Where are we actually sending to? The control endpoint 0? The control endpoint is the only endpoint that can send and receive data if I got the spec right. Why isn't the control endpoint enough?

Nable80 commented 10 months ago

IMHO It's definitely relevant, USB HID allows you to use devices without installing additional kernel-mode drivers (which have to be signed) and also without admin privileges. So-called driverless devices work this way. But OS definitely won't allow random applications to send arbitrary control packets, so proper HID report descriptors should be defined.

I just checked a USB keyboard I have and it doesn't define any OUT endpoints except EP0 but it definitely receives and handles writes from host: when I press Caps/Num/Scroll lock on my laptop's keyboard, lights are toggled on the external USB one. Wireshark traces (well, I could find it from the spec but it would be a longer way) show that key-presses are read using the IN interrupt endpoint, while lights are set using writes to control EP0. I know, this explanation is incomplete (I'll find more details about HID reports later, when I read the relevant code and specification parts) but I didn't dig deeply into the USB HID area before, I used different MCUs and userspace drivers (this approach is extremely easy when you don't have to support Windows users).

Nable80 commented 10 months ago

I should also say I agree with your doubts about missing interrupt OUT endpoints. Such endpoints are allowed by the USB HID specification and I clearly see their usage in this generic example: https://infocenter.nordicsemi.com/pdf/nan_22.pdf It'll take some time to understand the exact details (how to adapt descriptors from their example to rv003usb code) but I definitely like their explanation.

cnlohr commented 10 months ago

A lot has gone on here. Do you guys have any firm suggestions of which way we could go with this one?

Nable80 commented 10 months ago

Hi, I'm happy to see you in this thread!

Do you guys have any firm suggestions of which way we could go with this one?

I did some studies about HID reports for sending data to device but I'm somehow busy now to build my own test setup and provide exact suggestions (I'm sure the HID descriptors should be fixed but I need some free time to build and test the right ones). If you have any time for this issue, could you give some additional information, please? I.e. did you test this setup (bootloader + minichlink) under MS Windows (which version it was?) and can you provide a wireshark dump with a working example of device enumeration + minichlink detection?

xsrf commented 10 months ago

@cnlohr thx for opening this again. My plan was to

1) test if I can get any other application to talk to the bootloader - e.g. Chrome Devtools for WebUSB ( chrome://usb-internals/ ) or WebUSB in general. I use Wireshark to check if communication is happening.

2) Try to get minichlink to talk to a different USB HID device, e.g. one of your regular HID examples, since the demo_composite_hid kinda works (It types b but it doesn't move the mouse on my PCs which might also be a hint?)

I'm also curios if someone really had this all working on windows before ;)

xsrf commented 9 months ago

btw. I don't make any progress... I tried WebHID in Chrome to send a feature report to the rv003usb and to a simple USB mouse, but it also fails with "Failed to write the feature report." in both cases:

[device] = await HID.requestDevice({ filters: [{ 'vendorId': 0x04D9 , 'productId': 0x1133 }]}); // Fujitsu USB Mouse
await device.open();
await device.sendFeatureReport(0, Uint8Array.from([42, 0])); // Uncaught DOMException: Failed to write the feature report.
await device.close();

[device] = await HID.requestDevice({ filters: [{ 'vendorId': 0x1209 , 'productId': 0xb003 }]}); // rv003usb
await device.open();
await device.sendFeatureReport(0, Uint8Array.from([42, 0])); // Uncaught DOMException: Failed to write the feature report.
await device.close();

Also in both cases Wireshark shows no USB communication with the device.

Same with https://github.com/todbot/hidapitester btw. (wrote -1 Bytes):

> hidapitester.exe --vidpid 1209:B003 --open --get-report-descriptor -l 8 --send-feature 0,255 
Opening device, vid/pid: 0x1209/0xB003
Report Descriptor:
 05 01 09 FF A1 01 85 AA 09 FF 15 00 25 01 75 00 95 00 B1 00 C0
Writing 8-byte feature report...wrote -1 bytes:
 00 FF 00 00 00 00 00 00
Closing device

> hidapitester.exe --vidpid 04D9:1133 --open --get-report-descriptor -l 8 --send-feature 0,255 
Opening device, vid/pid: 0x04D9/0x1133
Report Descriptor:
 05 01 09 02 A1 01 09 01 A1 00 05 09 19 01 29 03 15 00 25 01 75 01 95 03 81 02 75 05 95 01 81 03
 05 01 09 30 09 31 09 38 15 81 25 7F 75 08 95 03 81 06 C0 C0
Writing 8-byte feature report...wrote -1 bytes:
 00 FF 00 00 00 00 00 00
Closing device
xsrf commented 9 months ago

Btw. I can't even write feature reports to the hidapitester reference HID device on all my Windows PCs I've tested https://github.com/todbot/hidapitester/issues/27. I'm not sure why this all is wrong on so many layers but now I'm almost convinced this will never work with feature reports 😞

xsrf commented 9 months ago

@cnlohr you know anyone who had success with this on Windows? To me it looks like it's not possible to send USB feature reports on Windows - period. Regardless what hardware or software I try, it doesn't work. I'm stuck...

cnlohr commented 8 months ago

@xsrf I just validated today the demo_hidapi works on Windows on multiple PCs. I can't seem to find a configuration that fails.

I updated the demo and explanations. I tried using hidapitester, but, it wants to use 64 bytes for some reason, so I added a new endpoint, 0xAB (171) . That works with hidapitester.

cnlohr commented 8 months ago

While this isn't the bootloader, is this sufficient for your purposes?

xsrf commented 8 months ago

@cnlohr thx, I will try to reproduce this and report it here. Did you use hidapitester with option -l 8 like I quoted above? It only tried to send 8 Bytes.

xsrf commented 8 months ago

@cnlohr okay, so I flashed demo_hidapi and that actually works, I can write to the device and read the data back:

# Feature 171

> hidapitester --vidpid 1209/D003 --open --get-report-descriptor
Opening device, vid/pid: 0x1209/0xD003
Report Descriptor:
 05 FF 09 00 A1 02 85 AA 09 01 15 00 25 00 75 08 95 FE B1 02 85 AB 09 01 15 00 25 00 75 08 95 3F
 B1 02 C0
Closing device

> hidapitester --vidpid 1209/D003 --open --send-feature 171,255,254,1
Opening device, vid/pid: 0x1209/0xD003
Writing 64-byte feature report...wrote 64 bytes:
 AB FF FE 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Closing device

> hidapitester --vidpid 1209/D003 --open -l 8 --send-feature 171,255,254,1
Opening device, vid/pid: 0x1209/0xD003
Writing 8-byte feature report...wrote 8 bytes:
 AB FF FE 01 00 00 00 00
Closing device

> hidapitester --vidpid 1209/D003 --open --read-feature 171
Opening device, vid/pid: 0x1209/0xD003
Reading 64-byte feature report, report_id 171...read 64 bytes:
 AB FF FE 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Closing device

> hidapitester --vidpid 1209/D003 --open -l 8 --read-feature 171
Opening device, vid/pid: 0x1209/0xD003
Reading 8-byte feature report, report_id 171...read -1 bytes:   <-- fails!
 AB 00 00 00 00 00 00 00 
Closing device

# Feature 170

> hidapitester --vidpid 1209/D003 --open --send-feature 170,42,42
Opening device, vid/pid: 0x1209/0xD003
Writing 64-byte feature report...wrote 64 bytes:
 AA 2A 2A 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Closing device

> hidapitester --vidpid 1209/D003 --open --read-feature 170
Opening device, vid/pid: 0x1209/0xD003
Reading 64-byte feature report, report_id 170...read -1 bytes:   <-- fails!
 AA 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Closing device

> hidapitester --vidpid 1209/D003 -l 255 --open --read-feature 170
Opening device, vid/pid: 0x1209/0xD003
Reading 255-byte feature report, report_id 170...read 255 bytes:
 AA 2A 2A 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Closing device

Looks like hidapitester defaults to 64 bytes, but you can use -l to specify the length for reads and writes. So it actually works with feature 170 / 0xAA. Let's see if I can do some progress now...

xsrf commented 8 months ago

Okay so the HID descriptor for the bootloader looks pretty broken actually... with

static const uint8_t special_hid_desc[] = { 
  HID_USAGE_PAGE ( HID_USAGE_PAGE_DESKTOP      )                 ,
  HID_USAGE      ( 0xff  )                 , // Needed?
  HID_COLLECTION ( HID_COLLECTION_APPLICATION )                 ,
    HID_REPORT_ID    ( 0xaa                                   )
    HID_USAGE        ( 0xff              ) ,
    HID_FEATURE      ( HID_DATA | HID_ARRAY | HID_ABSOLUTE    ) ,
    HID_REPORT_COUNT ( 128 ) ,
  HID_COLLECTION_END
};

it results in

0x05, 0x01,        // Usage Page (Generic Desktop Ctrls)
0x09, 0xFF,        // Usage (0xFF)
0xA1, 0x01,        // Collection (Application)
0x85, 0xAA,        //   Report ID (-86)
0x09, 0xFF,        //   Usage (0xFF)
0x15, 0x00,        //   Logical Minimum (0)
0x25, 0x01,        //   Logical Maximum (1)
0x75, 0x00,        //   Report Size (0)
0x95, 0x00,        //   Report Count (0)
0xB1, 0x00,        //   Feature (Data,Array,Abs,No Wrap,Linear,Preferred State,No Null Position,Non-volatile)
0xC0,              // End Collection

which actually has a Report Size and Report Count of 0, so it can't transfer any data.

I changed that to

static const uint8_t special_hid_desc[] = { 
  HID_USAGE_PAGE ( HID_USAGE_PAGE_DESKTOP      )                 ,
  HID_USAGE      ( 0xff  )                 , // Needed?
  HID_COLLECTION ( HID_COLLECTION_APPLICATION )                 ,
    HID_REPORT_SIZE ( 8 ),
    HID_REPORT_COUNT ( 128 ) ,
    HID_REPORT_ID    ( 0xaa                                   )
    HID_USAGE        ( 0xff              ) ,
    HID_FEATURE      ( HID_DATA | HID_ARRAY | HID_ABSOLUTE    ) ,
  HID_COLLECTION_END
};

and now I can actually send data to the bootloader and read it back:

> hidapitester --vidpid 1209:B003 --open -l 129 --send-feature 170,255,255
Opening device, vid/pid: 0x1209/0xB003
Writing 129-byte feature report...wrote 129 bytes:
 AA FF FF 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00
Closing device

> hidapitester --vidpid 1209:B003 --open -l 129 --read-feature 170
Opening device, vid/pid: 0x1209/0xB003
Reading 129-byte feature report, report_id 170...read 128 bytes:
 AA FF FF 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00
Closing device
xsrf commented 8 months ago

Another issue: minichlink still failed because it is trying to send 128 Bytes (127 Bytes data + Report ID), but the descriptor says the Report Count is 128 (so 129 bytes data including Report Id). I then changed the Report Count in the descriptor to 127 and now minichlink finally sends data to the bootloader. However, it loops on confirming the first transaction and times out:

> minichlink.exe -a -w blink.bin flash -B
-- hid_open(614) --
-- hid_open(614) --
-- hid_open_path(651) --
pp_data: 128 bytes
70 bf 63 00 00 00 00 00 ff 00 01 00 00 00 00 00
80 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 01 00 00 00 00 00 00 00 00 00 00 00
00 00 01 00 00 00 01 00 20 1d 97 00 00 00 00 00
c0 fb 14 00 00 00 00 00 fb a1 40 00 00 00 00 00
a0 74 97 00 00 00 00 00 03 b0 00 00 00 00 00 00
00 00 00 00 00 00 00 00 01 00 00 00 00 00 00 00

dev->output_report_length 0
dev->input_report_length 0
TryInit_B003Fun() #0
TryInit_B003Fun() #1
Found B003Fun Bootloader
Halting Boot Countdown
Commit TX: 128 bytes
aa 00 00 00 81 46 94 c1 fd 56 14 c1 82 80 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 cd ab 34 12

hid_send_feature_report(448, 9926000, 128)
hid_send_feature_reporthid_send_feature_report = 128
Commit RX: 129 bytes
aa 00 00 00 81 46 94 c1 fd 56 14 c1 82 80 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 cd ab 34 12
0e
Commit RX: 129 bytes
aa 00 00 00 81 46 94 c1 fd 56 14 c1 82 80 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 cd ab 34 12
0e
Commit RX: 129 bytes
aa 00 00 00 81 46 94 c1 fd 56 14 c1 82 80 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 cd ab 34 12
0e

.....
Error: Timed out waiting for stub to complete
Could not setup interface.

Guess I now have to check all the debug code I added to minichlink weeks ago ;) But I'd call this huge progress 🎉

xsrf commented 8 months ago

So I checked the minichlink code and also wireshark capture. Minichlink actually receives 128 Bytes of data back from the bootloader according to the capture. The first byte being the Report ID 0xAA followed by 127 Bytes. The debug code actually reads 129 Bytes because hid_get_feature_report() returns "the number of bytes read plus one for the report ID" (not 100% sure if I got this correct). So it prints one additonal byte (0x0e) which is actually outside the buffer.

However, it repeats endlessly because it expects the 2nd Byte in the response (the one after the report ID) to be 0xFF, which it is not. https://github.com/cnlohr/ch32v003fun/blob/f1820f2995e146ce8be0e3a94cb34a53d8238061/minichlink/pgm-b003fun.c#L201

@cnlohr What would set the response from the bootloader to 0xFF?

xsrf commented 8 months ago

So I just replaced the if( eps->respbuffer[1] == 0xff ) break; with break; as I don't get why reading back the programmed data would result in 0xFF in the first byte. After that the programming actually completes. It goes through hundreds TX/RX iterations and ends with Image written. However, the programmed code won't run... Actually, the previously flashed code still runs 🤔

../ch32v003fun/minichlink/minichlink -w demo_hidapi.bin flash -b
-- hid_open(614) -- 

-- hid_open(614) -- 

-- hid_open_path(651) -- 

pp_data: 128 bytes
f0 bc 55 00 00 00 00 00 ff 00 01 00 00 00 00 00 
80 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 01 00 00 00 00 00 00 00 00 00 00 00 
00 00 01 00 00 00 01 00 30 1d 53 00 00 00 00 00 
c0 fb 14 00 00 00 00 00 fb a1 40 00 00 00 00 00 
20 75 53 00 00 00 00 00 03 b0 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 01 00 00 00 00 00 00 00 

dev->output_report_length 0 

dev->input_report_length 0 

TryInit_B003Fun() #0
TryInit_B003Fun() #1
Halting Boot Countdown
Commit TX: 128 bytes
aa 00 00 00 81 46 94 c1 fd 56 14 c1 82 80 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 cd ab 34 12 

hid_send_feature_report(472, 5469648, 128)

hid_send_feature_reporthid_send_feature_report = 128
Commit RX: 129 bytes
aa 00 00 00 81 46 94 c1 fd 56 14 c1 82 80 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 cd ab 34 12 
0e 
Interface Setup
Read Binary Blob: 4 bytes from 40022010
Commit TX: 128 bytes
aa 00 00 00 23 a0 05 00 13 07 45 03 0c 43 50 43 
2e 96 21 07 94 41 14 c3 91 05 11 07 e3 cc c5 fe 
93 06 f0 ff 14 c1 82 80 00 00 00 00 00 00 00 00 
00 00 00 00 10 20 02 40 04 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 cd ab 34 12 

hid_send_feature_report(472, 5469648, 128)

hid_send_feature_reporthid_send_feature_report = 128
Commit RX: 129 bytes
aa 00 00 00 23 a0 05 00 13 07 45 03 0c 43 50 43 
2e 96 21 07 94 41 14 c3 91 05 11 07 e3 cc c5 fe 
93 06 f0 ff 14 c1 82 80 00 00 00 00 00 00 00 00 
00 00 00 00 10 20 02 40 04 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 cd ab 34 12 
3c 
Commit TX: 128 bytes
aa 00 00 00 23 a0 05 00 13 07 45 03 0c 43 50 43 
2e 96 21 07 14 43 94 c1 91 05 11 07 e3 cc c5 fe 
93 06 f0 ff 14 c1 82 80 00 00 00 00 00 00 00 00 
00 00 00 00 10 20 02 40 04 00 00 00 00 00 02 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 cd ab 34 12 

hid_send_feature_report(472, 5469648, 128)

~~~~~~~~~~ 7000 lines ~~~~~~~~~~

Read Binary Blob: 4 bytes from 4002200c
Commit TX: 128 bytes
aa 00 00 00 23 a0 05 00 13 07 45 03 0c 43 50 43 
2e 96 21 07 94 41 14 c3 91 05 11 07 e3 cc c5 fe 
93 06 f0 ff 14 c1 82 80 00 00 00 00 00 00 00 00 
00 00 00 00 0c 20 02 40 04 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 cd ab 34 12 

hid_send_feature_report(472, 5469648, 128)

hid_send_feature_reporthid_send_feature_report = 128
Commit RX: 129 bytes
aa 00 00 00 23 a0 05 00 13 07 45 03 0c 43 50 43 
2e 96 21 07 94 41 14 c3 91 05 11 07 e3 cc c5 fe 
93 06 f0 ff 14 c1 82 80 00 00 00 00 00 00 00 00 
00 00 00 00 0c 20 02 40 04 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 cd ab 34 12 
3c 
Image written.
Booting
Commit TX: 128 bytes
aa 00 00 00 37 07 67 45 b7 27 02 40 13 07 37 12 
98 d7 37 97 ef cd 13 07 b7 9a 98 d7 23 a6 07 00 
13 07 00 08 98 cb b7 f7 00 e0 37 07 00 80 23 a8 
e7 d0 82 80 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 cd ab 34 12 

hid_send_feature_report(472, 5469648, 128)

hid_send_feature_reporthid_send_feature_report = 128
Commit RX: 129 bytes
aa 00 00 00 37 07 67 45 b7 27 02 40 13 07 37 12 
98 d7 37 97 ef cd 13 07 b7 9a 98 d7 23 a6 07 00 
13 07 00 08 98 cb b7 f7 00 e0 37 07 00 80 23 a8 
e7 d0 82 80 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 cd ab 34 12 
34 
xsrf commented 8 months ago

I tried writing a bunch of 0xAAs to flash (minichlink.exe -a -w AA.bin flash -B) and reading them back (minichlink.exe -a -r read.bin flash 500 -B) with the bootloader, but it reads back all 0x00. Reading the flash back with WCH-LinkE shows that the previous code is still there. So while USB communication and reading/writing the scratchpad now seems to work, writing to flash does not.

xsrf commented 8 months ago

Okay, so I guess the data is received correctly, but the scratchpad is not executed. https://github.com/cnlohr/rv003usb/blob/4f1b55a42ea71b0026842e04551bf9318aefcc94/bootloader/bootloader.c#L270C1-L270C1 would set the last 4 bytes of the scratchpad to 0, but they still read back 0x1234abcd, as you can see above.

xsrf commented 8 months ago

Okay... https://github.com/cnlohr/rv003usb/blob/4f1b55a42ea71b0026842e04551bf9318aefcc94/bootloader/bootloader.c#L264 e->count never came near SCRATCHPAD_SIZE. I changed it to if( start + length >= scratchpad + SCRATCHPAD_SIZE ) instead to detect if the current USB packet was writing to the end of the scratchpad. And now it finally works! 🎉 The first 4 Bytes of the response now also read back FF FF FF FF so I can put the retry check back to minichlink. I'll create a pull request soon. Somebody needs to check if this breaks Linux ;)

cnlohr commented 8 months ago

Sorry, I am seeing all of this right now.

I really would appreciate a PR for minichlink. I can totally fix up any Linux issues.

Let me know what you find, or want to tweak!

SIDE-NOTE: Can you include an example of talking to the hidapi stuff using WebHID?

xsrf commented 8 months ago

@cnlohr in the end I didn't do any changes to minichlink. All I've changes was just for debugging. The WebHID/WebUSB was just one of my desperate but failed tries to talk to any USB device via feature requests, which didn't work - for reasons I didn't know back then. Now I could try it again, and I actually will because I have a project that should receive text messages via USB, preferably from the browser ;)