-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve usb.core.Device exception handling and reporting #9672
base: main
Are you sure you want to change the base?
Conversation
shared-module/usb/core/Device.c had several spots where it would raise a MicroPython exception with a NULL argument. That made it very difficult for CircuitPython code to attempt to recover from fault conditions caused by quirky USB device behavior. This adds short error strings to distinguish the different types of faults.
This commit makes sure that functions in usb.core.Device return as soon as they raise a MicroPython exception rather than continuing on with whatever code would normally run. This will hopefully help to avoid things like dereferencing null pointers.
This is the result of a `pre-commit run --all-files` to fix the translation file changes which were causing a CI check fail. I'm not convinced this change is a good idea. I would prefer to set USBError.errno, if possible, but I don't know how to do that.
I added a fix for the failing pre-commit check (translation file needed changes). I'm not convinced that my approach here of adding error strings and updating the translation file is the right way to solve the problem of distinguishing types of USB errors. If there were a way to set USBError.errno, I would prefer to do that. But, I don't know if or how that would be possible. |
@samblenny Did you just do |
I'm new to this. When I looked around for possible ways of invoking the translation pre-commit hook (skipped normally), the best looking documentation option I found was to run |
I tried running |
Probably what you did was equivalent. But when pre-commit complains, just run |
This PR includes a fix for the null pointer dereference from issue #9670 and adds short error message strings to help distinguish between different types of usb.core.USBError exceptions.
I understand that adding unique strings to the core is generally problematic, but I don't know how to avoid it in this case. For writing USB device drivers, it's very helpful to be able to identify the specific causes of the different types of exceptions that get grouped together under USBError. If there were a way to set the value of USBError.errno, that would be ideal. But, my understanding is that errno is generally not supported in MicroPython. Maybe I'm wrong about that?
In my test builds, these changes add about 132 bytes to the used flash firmware space for
BOARD=adafruit_feather_esp32s3_tft
. Shortening the "Pipe error" and "No configuration set" messages saved about 16 bytes. When I experimented with using single character strings vs. 4-5 character strings, it didn't seem to save any additional space.