TinyUSB OUT endpoint uses incorrect endpoint size (IDFGH-8673)

[mitchellcairns]

I've edited this issue to reflect the actual issue I've found and what the bug might be.

Answers checklist.

• I have read the documentation ESP-IDF Programming Guide and the issue is not addressed there.
• I have updated my IDF branch (master or release) to the latest version and checked that the issue is present there.
• I have searched the issue tracker for a similar issue and not found a similar issue.

IDF version.

v5.0-beta1-824-ga8ef7570ca

Operating System used.

Windows

How did you build your project?

VS Code IDE

If you are using Windows, please specify command line type.

PowerShell

Development Kit.

ESP32-S3-WROOM-1 | Custom PCB & Official Devkit

Power Supply used.

USB

What is the expected behavior?

Each time the USB Device is sent an OUT interrupt report, the tud_hid_set_report_cb should be triggered.

What is the actual behavior?

The callback will be triggered once, then never again. (Confirmed other OUT reports are being sent to the device via Wireshark).

Steps to reproduce.

1. Set up project from example: HID Example
2. Use a custom descriptor with an IN and OUT endpoint. I will be using the default VID and PID here.
   static const uint8_t gc_hid_configuration_descriptor[] = { // Configuration number, interface count, string index, total length, attribute, power in mA TUD_CONFIG_DESCRIPTOR(1, 1, 0, 41, TUSB_DESC_CONFIG_ATT_SELF_POWERED, 500), // Interface 9, TUSB_DESC_INTERFACE, 0x00, 0x00, 0x02, TUSB_CLASS_HID, 0x00, 0x00, 0x00, // HID Descriptor 9, HID_DESC_TYPE_HID, U16_TO_U8S_LE(0x0110), 0, 1, HID_DESC_TYPE_REPORT, U16_TO_U8S_LE(61), // Endpoint Descriptor 7, TUSB_DESC_ENDPOINT, 0x81, TUSB_XFER_INTERRUPT, U16_TO_U8S_LE(37), 1, // Endpoint Descriptor 7, TUSB_DESC_ENDPOINT, 0x02, TUSB_XFER_INTERRUPT, U16_TO_U8S_LE(5), 8, };

This descriptor has an OUT endpoint with a size of 5 bytes.

4. Insert functionality in tud_hid_set_report_cb which will be triggered under the conditions in TinyUSB for data received on the OUT endpoint
   if (report_type == HID_REPORT_TYPE_INVALID && report_id == 0) { // Code here to demonstrate the issue. You can change or toggle LED color, as I have been doing, but I'll use a log statement to demonstrate. ESP_LOGI("OUT GOT", "Recieved OUT report: %X", (unsigned int) buffer[0]); }

5. I set up the Espressif driver with a Libusb driver through Zadig (I installed WinUSB) for further testing.

6. I used a python script to send an appropriate chunk of data.
   `import os
   import usb
   from usb.backend import libusb1
   import time
   be = libusb1.get_backend()

brand = 0x303a #Expressif
prod = 0x4004 #ESP32 S3

dev = usb.core.find(idVendor=brand, idProduct=prod, backend=be)

print(dev)

internum = 0
setting = 0
epnum = 0x1

startPayload = [0x11, 0x00, 0x00, 0x00, 0x00]
tmp = dev.write(0x02, startPayload, timeout=1000)`

More Information.

The result is that the write times out.

The fix is to change the OUT size to 6 instead of 5. I believe this is a bug. Even with the report byte being part of the data (which is expected),

[mitchellcairns]

I guess it should be worth noting I'm using a custom HID descriptor here to get the IN and OUT endpoints...

[mitchellcairns]

I did more testing. I found that writing to the OUT endpoint with a single byte works fine. Trying to send multiple bytes in one packet causes a timeout.

[chegewara]

From this description its hard to guess what may be wrong, but why are you trying to catch invalid packets on report_id 0?

report_type == HID_REPORT_TYPE_INVALID && report_id == 0

Another question is, are you using TUD_HID_INOUT_DESCRIPTOR to create descriptor?
I think there was similar issue reported not long ago and solved.

[mitchellcairns]

From this description its hard to guess what may be wrong, but why are you trying to catch invalid packets on report_id 0?

report_type == HID_REPORT_TYPE_INVALID && report_id == 0

Another question is, are you using TUD_HID_INOUT_DESCRIPTOR to create descriptor? I think there was similar issue reported not long ago and solved.

For whatever reason, the tinyUSB api requires you to use HID_REPORT_TYPE_INVALID when receiving data on OUT endpoints. If you read the code comments in the API you'll see what I'm talking about.

[chegewara]

Sorry, but its hard to read unformatted code here.

[mitchellcairns]

Sorry, but its hard to read unformatted code here.

See the link to the code in question. When a transfer is completed from Host to Device, it calls this function and you can see the report type is set to HID_REPORT_TYPE_INVALID. This is how @hathach has decided to implement this. https://github.com/hathach/tinyusb/blob/52a9098b31d993d743ee31efe155b90c7ba5b21e/src/class/hid/hid_device.c#L412

[chegewara]

Its just like you said, buffer length should be sizeof(payload) + 1. extra byte is for reportID. You can see it here, even if its not that obvious:
dev.write(0x02, startPayload, timeout=1000) // 0x02 is reportID, right?
If you still think its a bug, then its tinyUSB bug, not esp-idf.

[mitchellcairns]

As I already said, I'm including the report byte in the size of my payload. Espressif has forked their own branch of TinyUSB. Do they not have any ownership of the bug?

[chegewara]

Sorry, but maybe i dont understand something.
You said this

The fix is to change the OUT size to 6 instead of 5

And my understanding is that you are sending 5 bytes from PC + 1 byte is reportID, which is in total 6 bytes.
So, when you setup OUT endpoint with size of 6 it is working as expected, but in OP you are posting this:

7, TUSB_DESC_ENDPOINT, 0x02, TUSB_XFER_INTERRUPT, U16_TO_U8S_LE(5), 8, };

which is size of 5.

Am i understanding correctly?

[roma-jam]

@mitchellcairns Hi! Thanks for the pointing out the problem. I'll try to do as much as can to help.

May I clarify the device descriptors that you use. I had tried to parse it from the description and that's what I got.

static const uint8_t hid_configuration_descriptor[] = {
    // Configuration number, interface count, string index, total length, attribute, power in mA
    TUD_CONFIG_DESCRIPTOR(1, 1, 0, 41, TUSB_DESC_CONFIG_ATT_SELF_POWERED, 500),
    // Interface
    9, TUSB_DESC_INTERFACE, 0x00, 0x00, 0x02, TUSB_CLASS_HID, 0x00, 0x00, 0x00,
    // HID Descriptor
    9, HID_DESC_TYPE_HID, U16_TO_U8S_LE(0x0110), 0, 1, HID_DESC_TYPE_REPORT, U16_TO_U8S_LE(61),
    // Endpoint Descriptor
    7, TUSB_DESC_ENDPOINT, 0x81, TUSB_XFER_INTERRUPT, U16_TO_U8S_LE(37), 1,
    // Endpoint Descriptor
    7, TUSB_DESC_ENDPOINT, 0x02, TUSB_XFER_INTERRUPT, U16_TO_U8S_LE(5), 8,
};

Does it seems the same, if I may ask?

UPD. If yes, I probably need to look at the 61-length HID report descriptor. If it is possible, I would like to see it also.

Thanks and all the best.

[roma-jam]

@mitchellcairns Hi again.

I have tried to reproduce the bug and I get the following statement:

• The tud_hid_set_report_cb doesn't trigerred when we send output report with length equal to EP size? (e.g. 5 bytes for 5 bytes max EP size, 6 for 6 and so on). The less bytes (4 bytes for 5 EP size, 5 for 6 e.t.c works well and cb is triggered). Also, cb is triggered while we would like to send more than EP max size.

Please, leave a comment to agree/disagree with the statement.

Anyway, I will try to look the cause of this behavior.
Thanks and all the best.

[roma-jam]

@mitchellcairns Hi again.
No worries, I already have everything to proceed.

Small suggestions:

• There is 61-bytes length of report descriptor configured. For correct working we also need to provide 61-bytes length report descriptor in hid_report_descriptor;
• There is some abnormal behavior when configured EP sizes != 64 in tinyUSB stack as we can see from description 37 and 5. Everything should work well when EP descriptors sizes configured as 64.

To make it works with 37, 5 or any other sizes we need a little bit more time and make some changes in the code.

[chegewara]

Im sorry, but i cant agree with this statement.
I am using tinyUSB with internal mouse and keyboard API a,d i am setting lower sizes, like 8 for keyboard (dont remember mouse).
There is no issue.

Even recently i am playing with custom "alphanumeric/bitmaped display" descriptor, and its working with varied size.
Maybe it should be multiply 8, but standard mouse descriptor is 5 byes i think, so im not sure.
I have to admit i am testing with linux, so maybe its issue with windows? And long time ago i discovered that windows has issues with HID descriptors without report ID (it was with BLE HID, but idea and driver on host is the same i guess).

It also may be problem with client app on host. I am using web HID right now and no issue.

[roma-jam]

@chegewara No problem, thanks for the information.

Let me try to repeat again what I said before, maybe it will help.
The problem is present when we try to send output report with the length == EP size, configured in device descriptor. All other data exchange works well.

What does it mean. When we configured EP size for 5 bytes we have the following situation:

• Output report 4 bytes length, tud_hid_set_report_cb() triggered with 4 bytes length.
• Output report 5 bytes length, tud_hid_set_report_cb() not trigerred at all
• Output report 6 bytes length, tud_hid_set_report_cb() trigerred with 11 bytes.

Why? Because when output report size is equal to the EP size (e.g. 5), the USB data transfer divided by 5 bytes to collect whole packet during USB transfer but comparison in the end fulfil with the 64 defined max size. That is not correct as we cannot distinguish two scenarios:

1. the packet is fully received and there is no more data will be
2. the packet is partially received and there will be more

Here is the python code that can help reproduce this behavior.

startPayload1 = [0x11, 0x00, 0x00, 0x00]
tmp = dev.write(0x02, startPayload1, timeout=1000)

startPayload2 = [0x11, 0x00, 0x00, 0x00, 0x00]
tmp = dev.write(0x02, startPayload2, timeout=1000)

startPayload3 = [0x11, 0x00, 0x00, 0x00, 0x00, 0x00]
tmp = dev.write(0x02, startPayload3, timeout=1000)

In my opinion, after running such code we should see the following output:

OUT GOT: Recieved OUT report (4): 11
OUT GOT: Recieved OUT report (5): 11
OUT GOT: Recieved OUT report (6): 11

[chegewara]

I understand that. So i changed my app for this test and here is my descriptor:

Config Number: 1
	Number of Interfaces: 1
	Attributes: a0
	MaxPower Needed: 500mA

	Interface Number: 0
		Name: usbhid
		Alternate Number: 0
		Class: 03(HID  ) 
		Sub Class: 00
		Protocol: 00
		Number of Endpoints: 2

			Endpoint Address: 03
			Direction: out
			Attribute: 3
			Type: Int.
			Max Packet Size: 4
			Interval: 1ms

			Endpoint Address: 83
			Direction: in
			Attribute: 3
			Type: Int.
			Max Packet Size: 4
			Interval: 1ms

as you can size IN and OUT packet size is 4,
and i am sending 4 bytes from web HID app:

tud_hid_set_report_cb => Instance: 0, reportID: 0, report_type: 0, bufsize; 5
buffer => test

I am receiving 5 bytes, where first byte is reportID and 4 bytes its payload.
I dont know, maybe i am using "fixed" tinyUSB version, or maybe the problem is somewhere else.

I have slightly opposite results. When i am sending less bytes than max packet size then i am not receiving data, but the data is somewhere buffered, because when i am sending next packet with 4 bytes payload then i am receiving accumulated payload.
Here is example:

• sending 3 bytes - tes
• sending 3 bytes - tes
• sending 4 bytes - test
• receiving all 13 bytes (10 bytes payload and 3x1 reportID):

SET => Instance: 0, reportID: 0, report_type: 0, bufsize; 13
buffer => testestest

also sending more data than report size is not triggering callback.

PS sorry for my previous answers, looks like i clearly didnt understand and didnt test it properly

[roma-jam]

@chegewara Thank you for more information.
That is interesting, because you do not have the opposite result, you have the same but in different way.

The main idea in your example that you send "test" from web HID app, but in the low level you send [report_id] + [buffer] which is [0x00, 0x74, 0x65, 0x73, 0x74].

This tells us only one thing: when you have sent "tes", actually you have sent 4 bytes (which is equal EP max number in your descriptor).
And this is exactly what I am talking about: the packet is partially received and there will be more

Stack save this data and wait for the next packet. Which is also "tes" (3 bytes from you and 4 bytes in a transport level). And when finally you've send the last one - it is triggered the callback.

This will end when the data overflows with 64 bytes. That means you need to send "tes" for 16 times and one more to make an overflow. Then everything stop working. 😕

[chegewara]

ReportID is not part of payload and is not counted in EP max packet size, at least its how i understand it.
ReportID is sent as a part of control data, together with ep number and other bytes.
I believe that @hathach may answer this as expert.

[roma-jam]

ReportID is exactly the part of the payload and it is the very first byte of it.

UPD: when the ReportID field is present. It can be not present, specification allows that.

Anyway, there are some strange moment which need to be verified.
I'll return when I have verified them.

[mitchellcairns]

With HID, the report ID is part of the payload and is included in the packet size count. EG a packet size of 5 could be

• Report ID
• Byte 0
• Byte 1
• Byte 2
• Byte 3

I work with many HID devices and this is the case in every situation where Report ID is part of the HID report descriptor.

[roma-jam]

Hi everyone!
After checking the HID Specification and making verification of different options, having done more tests we came with confirmation that no further changes in the tinyUSB code according to this issue is necessary.

Specification (Device Class Definition for Human Interface Devices (HID) Firmware Specification—5/27/01 Version 1.11) has a list of report constraints (p.54, §8.4) that tells us the main things:

• Only one report is allowed in a single USB transfer.
• A report might span one or more USB transactions. For example, an application that has 10-byte reports will span at least two USB transactions in a low-speed device.
• All reports except the longest which exceed wMaxPacketSize for the endpoint must terminate with a short packet. The longest report does not require a short packet terminator.

Which means that all the output reports which are more than wMaxPacketSize will be divided into several ones and should be finished with a short report if necessary.

During usage of the python script which is presented in description, this script represents a low-level HID driver and should handle the situation when data packet == wMaxPacketSize which also means that it should add the ending with a short packet. It is the responsibility of the HID driver on the host side according to the DCD for HID specification.

For example, to send short packet without payload in python script is possible with the following lines:

startPayload = [0x11, 0x00, 0x00, 0x00, 0x00]
tmp = dev.write(0x02, startPayload, timeout=1000)

shortPktEnd = []
tmp = dev.write(0x02, shortPktEnd, timeout=1000)

That will trigger report callback as well.

From everything above, tinyUSB stack works correctly and does not require any changes in the code.

Thanks for the detailed descriptions and some extra information which had been brought up during discussion and were very helpful.

[roma-jam]

Hi everyone,

We are closing this issue as there were no more questions based on the issue description.

Feel free to reopen.

[hathach]

@chegewara I am late to the party. As roman-jam already pointed out, though I want to add a bit of explanation with control transfer. HID input/output report can be get/set via BOTH control endpoint and interrupt endpoint. In fact, interrupt endpoint is optional if it is not used often enough e.g in case of keyboard with output for LED indicator.

• Interrupt Endpoint: report id if present is part of the payload (first byte)
• Control endpoint: report id is part of setup packet, payload of control DATA only contains report payload only. (control = setup + data + status)

hope this helps