trezor / cython-hidapi

:snake: Python wrapper for the HIDAPI
Other
283 stars 109 forks source link

Report global errors in device.error() #153

Closed holdersn closed 1 year ago

holdersn commented 1 year ago

device.error() is currently implemented to raise a ValueError if there is no device open. However, the C function being called can actually handle this case. If there is no device, it will report the last global error (which would typically be an error that occurred when trying to open a device).

Users of cython-hidapi should be able to retrieve these global errors as well. This would help a lot when diagnosing problems during open().

The change is easy: We just need to remove the null-check because the C code handles a NULL parameter by reporting the last global error.


For reference, here is the current implementaiton of device.error():

    def error(self):
        """Get error from device.

        :return:
        :rtype: str
        :raises ValueError: If connection is not opened.
        :raises IOError:
        """
        if self._c_hid == NULL:
            raise ValueError('not open')
        return U(<wchar_t*>hid_error(self._c_hid))

And here is the C code for hid_error():

HID_API_EXPORT const wchar_t * HID_API_CALL  hid_error(hid_device *dev)
{
    if (dev) {
        if (dev->last_error_str == NULL)
            return L"Success";
        return dev->last_error_str;
    }

    if (last_global_error_str == NULL)
        return L"Success";
    return last_global_error_str;
}
holdersn commented 1 year ago

I've provided a pull request.

If interested, I could also change try.py to report the global error in its exception handling. Please let me know if I should do that.

prusnak commented 1 year ago

I've provided a pull request.

If interested, I could also change try.py to report the global error in its exception handling. Please let me know if I should do that.

Please do - just add another commit to the PR branch and push. Thanks

holdersn commented 1 year ago

Done.

prusnak commented 1 year ago

Done

Thanks! Merged