search

tuna-f1sh / cyme

List system USB buses and devices; a lib and modern cross-platform lsusb that attempts to maintain compatibility with, but also add new features
GNU General Public License v3.0
141 stars 7 forks source link

Suppress Linux Permission Errors? #9

Closed MS3FGX closed 1 year ago

MS3FGX commented 1 year ago

As mentioned in Known Issues, running cyme without sudo on Linux generates several error messages (on my system, 7) about having insufficient permissions to read all device information before displaying the output. I would propose that showing multiple non-critical error messages each time the program runs is confusing to the user, to say nothing of being unattractive.

I was going to propose a PR that moved theses messages into the debug output, but it appears that the errors pop up there already. So as it stands, when debug output is enabled you actually get multiple permission denied errors per device.

image

I think the first question that should be asked is: are these errors really necessary if they don't (greatly) impact using cyme? Is it enough to simply note in the documentation that you'll lose some info without using sudo? In my opinion, yes.

But if for the sake of argument we are saying it's necessary to show these messages, I still think there is room for improvement. Perhaps the messages could only be shown on the first invocation of cyme? Or maybe the per-device errors could be replaced with a single line that says something like "Device information limited due to user permissions"?

tuna-f1sh commented 1 year ago

Yes I agree with you on this; it's unattractive and too verbose. I think my original thinking was to match lsusb and ensure that the user/a script knows that the data is not complete.

Looking into it again, actually lsusb list and tree both don't use the root_hub descriptors but the usb-ids instead so it does not matter. Where it will print to stderr, is when using --verbose: 'Couldn't open device, some information will be missing' for each device that it could not open as verbose will show string descriptors.

Considering this, I've changed the --lsusb --verbose compatibility mode to match this exactly and removed the error printing to stderr otherwise. It can be returned with a CYME_PRINT_NON_CRITICAL_PROFILER_STDERR env or config key.

Part of changes I'll rebase to a release: https://github.com/tuna-f1sh/cyme/commit/d12917d220885885b574e05a6974596b88ae0239

tuna-f1sh commented 1 year ago

I've pushed this as part of https://github.com/tuna-f1sh/cyme/releases/tag/v1.3.1, I think that closes it.

MS3FGX commented 1 year ago

Just built 1.3.1, all looks good to me. The WARN tag in the debug output is a nice touch as well, makes it more obvious that it's a non-critical error. Overall think it's a great improvement to the software.