fix: handle i2c device permissions errors gracefully#409
fix: handle i2c device permissions errors gracefully#409danudey wants to merge 2 commits intonewAM:mainfrom
Conversation
|
Coveralls tests seems to be due to adding more lines to a chunk of code which was already untested. That code probably should be tested, but I'm not yet sure the most optimal way to do so. |
There was a problem hiding this comment.
Thanks for the changes! Sorry for the slow review, this got buried at the bottom of my inbox.
Coveralls tests seems to be due to adding more lines to a chunk of code which was already untested. That code probably should be tested, but I'm not yet sure the most optimal way to do so.
That's fine, small decreases are ok by me. I should probably change the threshold.
| except (OSError, VCPIOError): | ||
| pass | ||
| except VCPPermissionError as exc: | ||
| logging.error("Unable to check i2c device: %s", exc) |
There was a problem hiding this comment.
| logging.error("Unable to check i2c device: %s", exc) | |
| logging.exception("Unable to check i2c device") |
This adds native exception info into the logging record.
| else: | ||
| logging.error( | ||
| "Unable to check i2c device %s: no device number found", device | ||
| ) | ||
| continue |
There was a problem hiding this comment.
I am curious, have you seen this occur? pyudev should only return None if the device number is not a unicode string: https://github.com/pyudev/pyudev/blob/7588bc8c84bf1e994b31020452124bc6618625ce/src/pyudev/device/_device.py#L626-L651
| for device in pyudev.Context().list_devices(subsystem="i2c"): | ||
| vcp = LinuxVCP(device.sys_number) | ||
| try: | ||
| if sys_no := device.sys_number: |
There was a problem hiding this comment.
| if sys_no := device.sys_number: | |
| if (sys_no := device.sys_number) is None: |
I think 0 is a valid device.sys_number.
2 participants
This fix gracefully handles i2c devices which we do not have permissions to open; this means that we no longer have to have every i2c device on the system accessible by the current user in order to use the tool.