-
Notifications
You must be signed in to change notification settings - Fork 2
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
extending featureset, documentation, framework, and tests #14
base: main
Are you sure you want to change the base?
Conversation
Housekeeping
refactor filee tree
move i2c search into main function
housekeeping, bump go version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some preliminary notes, thank you :)
Why the binary file?
@@ -24,7 +24,11 @@ func init() { | |||
flag.Parse() | |||
|
|||
if *flagDevicePath == "" { | |||
log.Fatal("please specify device path") | |||
path, err := dasung.FindDasungI2CDevicePaths() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how I feel about running ddcutil detect
automatically - it's quite intrusive since it actively probes any i2c buses it can get hold of. I've seen it cause crashes and hangs on rare occasions, and it can interfere with things like the ddcci kernel module. On my machine, it also takes multiple seconds to complete.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so maybe something along the lines of please specify device path (leave blank to autodetect)
, where it doesn't proactively run ddcutil detect
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps invert the logic, so the user has to specify detect
if they want to run ddcutil and it errors on a blank one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about the functionality in the first place - running ddcutil every time is super expensive
Ah my bad -- I wasn't paying close attention to which files I included with my commit |
No description provided.