search

schnoog / Joystick_ESP32S2

Joystick library for ESP32 S2 & S3 devices (native USB) for the Arduino framework.
GNU Lesser General Public License v3.0
34 stars 3 forks source link

Syncronization issue. #6

Closed serg-bloim closed 9 months ago

serg-bloim commented 9 months ago

https://github.com/schnoog/Joystick_ESP32S2/blob/57130d9d50ad34dc7273b836a863812a9ab8f4c0/src/Joystick_ESP32S2.cpp#L691

I just found an issue in the code. USBHID::SendReport method works asynchroniously, i.e. it sends a report and returns immediately, even before the report completes and the method is ready to be called again. If you call it again before it gets ready, there will be an error. The error is not critical, but the next report won't be sent. It doesn't affect absolute attributes like steering or joystick position, but it may affect one-time effects like button clicks and relative changes like mouse movements.

To overcome these issues, there's forth parameter uint32_t timeout_ms, which is used to wait for the report successful sending and returns from the method after that.

I'd recommend to set it to some default value >0 out of the box. And potentially make it configurable for better tuning. I assume most of the use-cases for this library will be using a simpler blocking approach for these calls, which means a timeout is necessary. Also, there are other libs, including standard ones by espressif. They will fail if used together with the current implementation of Joystick_ESP32.

serg-bloim commented 9 months ago

BTW, there's a def value for the timeout https://github.com/espressif/arduino-esp32/blob/6b287db119dab39561572d75599f8171b942352b/libraries/USB/src/USBHID.h#L77C16-L77C16

And std USB libs use the default value: https://github.com/espressif/arduino-esp32/blob/6b287db119dab39561572d75599f8171b942352b/libraries/USB/src/USBHIDMouse.cpp#L60C11-L60C11

schnoog commented 9 months ago

Thanks for your feedback and making me aware of the problem.

I've implemented a default of 2ms and adjusted the Joystick.begin(); method to accept the timeout as second parameter. Joystick.begin(,);