search

jnweiger / led-name-badge-ls32

Upload tool for an led name tag with USB-HID interface
GNU General Public License v2.0
230 stars 86 forks source link

Fix HID API sending path through pyhidapi #42

Closed dirkhillbrecht closed 9 months ago

dirkhillbrecht commented 1 year ago

Hi,

I found that HID API is not accessed as its API says: The data buffer must contain the report ID as first byte, "0" if there are no distinct report IDs. Furthermore, I only had success sending data to the display if I also chunked them into parts of 64 bytes. The HID API documentation is a bit unclear about this, but I had errors otherwise.

I also added a "-H" option to force the HID API send path. Through USB Core, I could not send more than 128 bytes to the display (Sertronics import from August 2023). This parameter addition might seem a bit clumsy, I am by no means an experienced Python programmer. Actually, I am almost no Python programmer at all and, frankly spoken, I do not like this language at all...

My journey to this solution was: I encountered the problem described in Issue 36, applied that fix. Then I ran in Issue 40 (and - now - also 41). I learned that on any modern Linux (Ubuntu 23.04), pyhidapi is never used as it is either not found or the HID C library lies in another directory as scanned by pyhidapi. I fixed all that so that pyhidapi was actually used, only to find that it also does not work correctly. Finally, I found the discrepancy between pyhidapi's API doc and its usage in the script and fixed that by this patch. Finally, I can write arbitrary long data to the display.

And here we are. I think this patch is correct but generally, it is rather complicated to get this up and running. And I have absolutely no idea why usb.core does not work at all.

Feel free to contact me in case of any questions!

Regards, Dirk

jnweiger commented 10 months ago

@dirkhillbrecht There are conflicts. Did you start on a slightly older version than current master? You may try updating your branch from my master. There are some trivial changes with description and added epilogue of ArgParser. As soon as it is technically mergeable, let's merge.