gs-usb WinUSB support and timeout=none is forever#2031
gs-usb WinUSB support and timeout=none is forever#2031BenGardiner wants to merge 11 commits intohardbyte:mainfrom
Conversation
|
I'm not even sure these unit tests proposed by the LLM are needed for gs_usb interface. (I also wonder if a removal or deprecation of gs_usb here should be considered because cantact is in tree and is more performant and python-can-candle works is more portable and more performant still) |
Replace GsUsb.scan() and GsUsb.find() calls with local helper functions that call usb.core.find() without specifying a backend, allowing pyusb to auto-detect the best available backend. This enables WinUSB support on Windows in addition to libusbK. Update documentation to reflect WinUSB support and add unit tests. Co-authored-by: BenGardiner <243321+BenGardiner@users.noreply.github.com>
The gs_usb interface directly imports `usb` (pyusb) for USB device discovery, so pyusb must be an explicit dependency rather than relying on it being a transitive dependency of the gs-usb package. Co-authored-by: BenGardiner <243321+BenGardiner@users.noreply.github.com>
pass '0' when timeout=None (as proposed by @zariiii9003 in hardbyte#2026 (comment))
BusABC has a class-level _is_shutdown = True attribute. When __init__ was not called (as in test mocks), GsUsbBus.shutdown() resolved this class attribute and returned early, never calling super().shutdown(). Restructure shutdown() to always call super().shutdown(), using the pre-call _is_shutdown state only to guard interface-specific cleanup. Co-authored-by: BenGardiner <243321+BenGardiner@users.noreply.github.com>
9ef411a to
3e5b1c0
Compare
|
@zariiii9003 thank you. all review comments are addressed. I made this PR to complete the timeout=None behaviour fix in all the drivers . I hope all the extra changes are also helpful. Please note that the python-can gs_usb driver isn't clearly needed anymore for my uses since cantact and candle drivers work with the same devices. |
|
|
||
| # Do not set timeout as None or zero here to avoid blocking | ||
| timeout_ms = round(timeout * 1000) if timeout else 1 | ||
| timeout_ms = round(timeout * 1000) if timeout else 0 |
There was a problem hiding this comment.
This doesn't actually fix the issue you wanted to fix though. Both None and 0 are falsy, so timeout_ms will be 0 for both. According to the comment (i don't know whether it is correct) 0 means blocking.
Otherwise the PR looks much better now, thank you for also adding type annotations. You could remove gs_usb from the mypy exclude list in pyproject.toml now.
Summary of Changes
This changes gs_usb handling of timeout=None to 'forever' by passing 0 to the underlying layer as suggested by @zariiii9003 in #2026 .
But first I needed to make gs_usb use WinUSB instead of libusbK because I didn't want to zadig anymore. I had no clue how to do this so I got copilot claude opus 4.6 to make those base changes. Here's what they said in BenGardiner#1
I have tested here on a 500Kbps medium load bus; I used the same device and setup with all of the cantact, candle and gs_usb python-can drivers and got the same results
Related Issues / Pull Requests
Type of Change
Checklist
tox).Additional Notes
I will add news frag once I know what the PR # is.