Add USB devices #424
Add USB devices #424
Conversation
christoph-zededa
force-pushed
the
usb
branch
3 times, most recently
from
215b125 to
03aa3e7
Compare
|
Fixed the build errors for Mac OS X and the linting errors about missing error-return-checks. |
christoph-zededa
force-pushed
the
usb
branch
2 times, most recently
from
11861e0 to
67402b7
Compare
|
@christoph-zededa thanks so much for this submission! I'll try to do a full review later today or tomorrow :) |
christoph-zededa
force-pushed
the
usb
branch
from
67402b7 to
1a152f1
Compare
There was a problem hiding this comment.
many thanks for the submission. I think that a usb package will benefit the project.
I'm very wary to add more deps, we should rather head to remove them all.
This is especially true if we can get similar (or identical?) results using golang code and peeking around in /sys.
Other than that, the code looks reasonnable, but I think we can use a bit more tests.
At glance, API additions (user-facing APIs) looks good but I'll leave the final word to @jaypipes
cmd/ghwc/commands/root.go
Outdated
| } | ||
| if err := showAccelerator(cmd, args); err != nil { | ||
| return err | ||
| fmt.Println("------------------------------") |
There was a problem hiding this comment.
minor: we should avoid fixed size separators; either avoid them for now or automatically align to the longest line, this approach will likely require extra code and/or a 3rd party helper.
There was a problem hiding this comment.
I forgot to remove it after debugging. Now it is gone.
| info.Baseboard.String(), | ||
| info.Product.String(), | ||
| info.PCI.String(), | ||
| info.USB.String(), |
There was a problem hiding this comment.
unrelated: this scales pretty poorly and is too hard to extend, I'll file a PR to send an improvement ASAP
pkg/linuxpath/path_linux.go
Outdated
| } | ||
|
|
||
| type Paths struct { | ||
| ChrootSys string |
There was a problem hiding this comment.
this should probably be "SysRoot"
There was a problem hiding this comment.
I renamed it to SysRoot
| fileSpecs := ExpectedCloneStaticContent() | ||
| fileSpecs = append(fileSpecs, ExpectedCloneNetContent()...) | ||
| fileSpecs = append(fileSpecs, ExpectedCloneUSBContent()...) | ||
| fileSpecs = append(fileSpecs, ExpectedClonePCIContent()...) | ||
| fileSpecs = append(fileSpecs, ExpectedCloneGPUContent()...) |
There was a problem hiding this comment.
likewise, this code probably deserves some cleanup, I'll take care in a different PR
| /* | ||
| #cgo pkg-config: libusb-1.0 | ||
| */ |
There was a problem hiding this comment.
Unless this provide critical functionalities we can't replace with our code, I think we should avoid this especially becase it requires C code
There was a problem hiding this comment.
The idea behind this is to use libusb on Mac OS X and use sysfs directly on Linux.
Perhaps it is better that I remove the OS X implementation now and later create another PR so this can be discussed there?
There was a problem hiding this comment.
ok, this is an important clarification which changes the perspective a bit. I would prefer this approach you outline in your last comment, but please don't do any change just yet, let's wait for @jaypipes 's review.
pkg/usb/usb_stub.go
Outdated
| import ( | ||
| "runtime" | ||
|
|
||
| "github.com/pkg/errors" |
There was a problem hiding this comment.
please use the native errors in new code
Updates the third-party dependencies in order for importing projects that have stricter dependency compliance restrictions. For `github.com/jaypipes/pcidb`, uplifts it to the latest 1.1.0 release, which removed the archived `github.com/mitchellh/go-homedir` dependency. For `howett.net/plist`, there hasn't been a tagged release of that repo since December, 2023, so I manually `go get` the latest commit from January 2025 for that repository. `howett.net/plist` is used for MacOS support. For `github.com/StackExchange/wmi`, I changed this to `github.com/yusufpapurcu/wmi` which is the officially-supported fork of the `github.com/StackExchange/wmi` repo. Closes Issue jaypipes#323 Related Issue jaypipes/pcidb#36 Related google/dranet#194 Signed-off-by: Jay Pipes <jaypipes@gmail.com>
Adds a fix for a missing $HOME envvar on Linux. Signed-off-by: Jay Pipes <jaypipes@gmail.com>
instead of checking every return manually, use a for-loop Signed-off-by: Christoph Ostarek <christoph@zededa.com>
basically this code iterates over entries in `/sys/bus/usb/devices/` and reads out information from the `uevent` file and some other files Signed-off-by: Christoph Ostarek <christoph@zededa.com>
not all operating systems expose usb information via sysfs; on the platforms still gousb can be used although it links to native libusb, misses some values and is a lot slower Signed-off-by: Christoph Ostarek <christoph@zededa.com>
3 participants
This PR adds USB devices to the list of devices.
The code basically iterates over
/sys/bus/usb/devices, follows the symlinks and readsuevent,interfaceandproduct.For the snapshot, the symlinks in
/sys/bus/usb/devicesare added as well as the aforementioned files.