Closed cbacott closed 5 years ago
mazunki
commented
5 years ago I also need to add the --device 0 option. I'm not being allowed to use fan mode 0 or 1. My fans were at 500rpm, now I got them up to 1000, unintentionally. Can't seem to lower them again.
cbacott
commented
5 years ago Yeah, something changed for me. I'm now not able to run it with --device=0. I was looking at the code and decided to start fresh, so I rm'd the whole thing and re-cloned the git. Now, it won't do anything, if I set device=0 it says it's out of range. Not sure what changed here. I'm poking around the code.
EDIT: Dammit, nvm. I wasn't running it with sudo while debugging, and forgot that I needed to. It's working still with sudo. Still messing around in the code.
EDIT2: Temporary fix for me, you can try this and recompile. Open main.c, change line 67 from -1 to 0. That's setting the initial value of device_number to 0 rather than -1. I don't know what the author was trying to do by making that -1, so it's a hack at this point. But, it seems to work for me.
Basically, device_number is initialized to -1 normally, the scan happens and the scan returns the number of devices found. After that, it checks if device_number is >= 0, though I think they meant to check scanlist_count? Doing that causes a seg-fault, so I'm only guessing at their intentions. Anyway, since device_number is still -1 from initialization, it never goes into that block to actually do anything. That's why it worked for me to set device=0 manually. I don't see what harm there is in initializing that value to 0 rather than -1. The next bit after checking that device_number >= 0 compares it to scanlist_count, and since a normal user couldn't open any of the devs, the value is 0 and it just does nothing, with an error message that might need tweaking. Then, it exits. However, if one runs it with sudo, scanlist_count is 1, thus the code continues and it all works.
I'm not sure what issue you're having exactly, mazunki, but this quick hack has made this app work for me.
EDIT3: Final edit. I figured they really did mean to check scanlist_count, just needed to have device_number >= 0 afterwards, as the seg fault comes from trying to set an array to -1. I switched back to -1 initialization, but tweaked the whole if block starting at line 90 to properly handle detecting no detected devices, and if devices detected then properly set it. Again, not sure what the author was attempting to do so I can't comment on how this would handle multiple devices. I'm setting device_number = scanlist_count-1 which I know is only going to by default access the last device found. Maybe there was supposed to be some other logic in there to loop through them?
Anyway, until author checks on this, if anyone is interested in that quick hack patch here it is.
--- main.c.orig 2019-08-11 11:14:42.255704603 -0500
+++ main.c 2019-08-11 11:36:30.379459920 -0500
@@ -87,7 +87,7 @@
msg_debug( "DEBUG: scan done, start routines\n" );
msg_debug( "DEBUG: selected device_number = %d\n", device_number );
- if ( device_number >= 0 )
+ if ( scanlist_count > 0 )
{
if ( device_number >= scanlist_count )
{
@@ -97,6 +97,7 @@
}
else
{
+ device_number = scanlist_count-1;
if ( scanlist[device_number].device->driver == &corsairlink_driver_rmi )
{
psu_settings( scanlist[device_number], flags, settings );
@@ -119,6 +121,10 @@
}
}
}
+ else
+ {
+ msg_info("No devices detected. You may not have permissions to access the device.\n");
+ }
exit:
corsairlink_close( context );
audiohacked
commented
5 years ago device_number is set to -1 so that it forces the user to select the intended device. What this hack does is always select the last detected device which breaks usability for myself (since I have several devices that uses this utility).
cbacott
commented
5 years ago That is a bit embarrassing, I didn't consider that. This patch will allow selecting of another device, while also providing a default selection like before, and also spits out a good error message to notify the user that they need permission to access the devices.
--- main.c.orig 2019-08-11 11:14:42.255704603 -0500
+++ main.c 2019-08-13 21:30:08.147805494 -0500
@@ -64,7 +64,7 @@
{
int rr; // result from libusb functions
- int8_t device_number = -1;
+ int8_t device_number = 0;
int scanlist_count = 0;
@@ -87,7 +87,7 @@
msg_debug( "DEBUG: scan done, start routines\n" );
msg_debug( "DEBUG: selected device_number = %d\n", device_number );
- if ( device_number >= 0 )
+ if ( scanlist_count > 0 )
{
if ( device_number >= scanlist_count )
{
@@ -119,6 +119,10 @@
}
}
}
+ else
+ {
+ msg_info("No devices detected. You may not have permissions to access the device.\n");
+ }
exit:
corsairlink_close( context );@cbacott
My error is different. I made an issue for it at #195 .
mazunki
commented
5 years ago Changing
- int8_t device_number = -1;
+ int8_t device_number = 0;followed by sudo make install
does the trick for me, having only one device to care about.
I still need to use channel=0, though, followed by the same command with channel=1.
Describe the bug Since upgrading to a Ryzen 3700 and Prime X370-pro, cannot control my Corsair H110i. Command says it detects the device, and dmesg shows the scan successfully, however pump and fan speed do not change. Dump is showing something weird at the end:
The following command used to set the first fan into full bore loud mode, it doesn't seem to do anything anymore.
Desktop (please complete the following information):
OS: Ubuntu 18.04 (this is the same OS this used to work under)
Compiled this from git, not showing any newer version available via git pull. I'm willing to dig into the code itself and adding debug statements if need be, just wondering if this is some kind of known problem before I start spending time on it. It looks like, from the dump command, that it's detecting it as device 0 correctly, but after that it's setting the device to -1. I just discovered that you can specify the device manually, and when I used --device 0, it correctly works. It used to just operate on the device correctly from the autoscan.