Skip to content
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

H110i (and possibly other hid devices) miss features #65

Closed
malakudi opened this issue Dec 2, 2017 · 10 comments
Closed

H110i (and possibly other hid devices) miss features #65

malakudi opened this issue Dec 2, 2017 · 10 comments

Comments

@malakudi
Copy link
Contributor

malakudi commented Dec 2, 2017

I am working with my H110i and current code lucks some features that H110i and possibly other Corsair HID devices support. Features include:

  • Get number of temperature sensors (now code is fixed to 3 temperature sensors even if device does not support 3)
  • Display/set fan mode. Possible options: Fixed Percentage (aka Fixed PWM), Fixed RPM, Quiet, Balanced, Performance, Max, Default and Custom.
  • Display/set pump mode. Current code does nothing for HID, and my H110i requires setting fixed RPM 2350 or 2900 for Quiet/Performance modes.
  • Display Max fan speed recorded since power up.
  • Correctly display firmware version. Now my reported firmware of 0x00 0x20 is printed as 32.0.0.0. It should be 2.0.0 (BCD notation)

I am willing to code the above features if original author agrees. I don't know if there are plans of implementing this on his side, or if specific implementation guidelines are needed. Feel free to comment.

@audiohacked
Copy link
Owner

Feel free to code the features; I do have a HID based CorsairLink, but don't have the time to work on it. I can test using mine. I'd like to keep any function returns for error codes. And keep the code based towards the Asetek Protocol; that means the HID functions should match the Asetek protocol.

@malakudi
Copy link
Contributor Author

malakudi commented Dec 3, 2017

There are functions that I don't know if asetek protocol supports (fan count, temp sensor count etc). I will be implementing these as do-nothing functions for asetek. For example, temp sensor count function will always return 3 for asetek (3 is the value used now in main.c). Is this OK?

@audiohacked
Copy link
Owner

No, If you want you can run the commands for fan count in existing functions and fail fast. But don't create additional/new functions. We could discuss changing the driver interface, but for now I want to keep it static until I can asses better ways of doing what we need for both HID and Asetek.

@malakudi
Copy link
Contributor Author

malakudi commented Dec 4, 2017

It can't work that way. For example, in main.c you have a fixed iteration loop for temperature sensors (ii < 3) but HID api has a way to report the number of temperature sensors.

I have almost completed my code, I have made the following changes in driver API.

--- ../OpenCorsairLink/driver.h	2017-12-03 21:09:55.651115944 +0200
+++ OpenCorsairLink/driver.h	2017-12-04 16:26:14.574344740 +0200
@@ -35,17 +35,19 @@
 	
 	int (*led)(struct corsair_device_info *dev, struct libusb_device_handle *handle, struct color *l, struct color *w, uint8_t t, uint8_t e);
 	int (*temperature)(struct corsair_device_info *dev, struct libusb_device_handle *handle, uint8_t select, uint16_t *temp);
-
+	int (*tempsensorscount)(struct corsair_device_info *dev, struct libusb_device_handle *handle, uint8_t *temperature_sensors_count);
 	struct fan_functions {
-		int (*profile)(struct corsair_device_info *dev, struct libusb_device_handle *handle, uint8_t profile);
-		int (*custom)(struct corsair_device_info *dev, struct libusb_device_handle *handle, struct fan_table *custom_profile);
-		int (*speed)(struct corsair_device_info *dev, struct libusb_device_handle *handle, uint8_t selector, uint16_t *speed);
+		int (*count)(struct corsair_device_info *dev, struct libusb_device_handle *handle, uint8_t *fan_count);
+		int (*profile)(struct corsair_device_info *dev, struct libusb_device_handle *handle, uint8_t selector, uint8_t *profile, uint16_t *data);
+		int (*custom)(struct corsair_device_info *dev, struct libusb_device_handle *handle, uint8_t selector, struct fan_table *custom_profile);
+		int (*speed)(struct corsair_device_info *dev, struct libusb_device_handle *handle, uint8_t selector, uint16_t *speed, uint16_t *maxspeed);
+		int (*print_mode)(uint8_t mode, uint16_t data, char *modestr);
 	} fan;
 
 	struct pump_functions {
-		int (*profile)(struct corsair_device_info *dev, struct libusb_device_handle *handle, uint8_t profile);
+		int (*profile)(struct corsair_device_info *dev, struct libusb_device_handle *handle, uint8_t *profile);
 		int (*custom)(struct corsair_device_info *dev, struct libusb_device_handle *handle, struct fan_table *custom_profile);
-		int (*speed)(struct corsair_device_info *dev, struct libusb_device_handle *handle, uint8_t selector, uint16_t *speed);
+		int (*speed)(struct corsair_device_info *dev, struct libusb_device_handle *handle, uint16_t *speed, uint16_t *maxspeed);
 	} pump;
 
 	struct power_functions {
  • tempsensorscount function sets the number of supported sensors in variable temperature_sensors_count, passed by reference.
  • fan.count function sets the number of supported fans in variable fan_count, passed by reference.
  • fan.profile function has been changed to accept values selector (to select specific fan), profile has been changed to pass by reference and added another parameter called data, also by reference. This is needed in order to be able to either read profile and value (for example, read the set RPM value for fixed rpm mode) or write them.
  • fan.speed function has been changed to also retreive the max recorded speed value in parameter maxspeed, passed by reference
  • fan.print_mode is a function that converts the numeric mode (profile) value and optional data value to a string, for printing.
  • pump.profile function has been changed so that profile parameter is passed by reference, in order to read current mode of pump operation or write the new mode.
  • pump.speed function has been changed to also retreive the max recorded speed value in parameter maxspeed, passed by reference.

Current output of my code for example:

Dev=0, CorsairLink Device Found: H110i Extreme!

Vendor: Corsair
Product: H110i Extreme
Firmware: 32.0.0.0
Temperature 0: 25.18
Fan 0: Quiet Mode (4PIN), Current/Max Speed 938/2105 RPM
Fan 1: Quiet Mode (4PIN), Current/Max Speed 946/2252 RPM
Pump: Mode 0x85, Current Speed 2366 , Max Speed 3003

And another one with different values for fan (Fan 0 fixed pwm set to 140, Fan 1 fixed RPM set to 1380)

./OpenCorsairLink.elf --device 0
Dev=0, CorsairLink Device Found: H110i Extreme!

Vendor: Corsair
Product: H110i Extreme
Firmware: 32.0.0.0
Temperature 0: 25.27
Fan 0: Fixed PWM Mode (4PIN), value 140, Current/Max Speed 1327/2105 RPM
Fan 1: Fixed RPM Mode (4PIN), value 1380, Current/Max Speed 1360/2252 RPM
Pump: Mode 0x85, Current Speed 2357 , Max Speed 3003

And this is the help which shows what else is implemented (most code copied from your legacy branch)

./OpenCorsairLink.elf -h
./OpenCorsairLink.elf: invalid option -- 'h'
OpenCorsairLink [options]
Options:
	--help				:Prints this Message
	--version			:Displays version.
	--debug				:Displays enhanced Debug Messages.
	--dump				:Implies --debug. Dump the raw data recieved from the device.
	--device <Device Number>	:Select device.

	LED:
	--led <HTML Color Code>			:Define Color for LED.
	--led-warn <HTML Color Code>		:Define Color for Warning Temp.
	--led-temp <Temperature in Celsius>	:Define Warning Temperature.

	Fan:
	--fan <fan number>{:<fan number>} :Selects a fan to setup. Accepted values are 1, 2, 3 or 4.
	--fan-mode <fan mode> :Sets the mode for the selected fan
		Modes:
		 0 - Fixed PWM (requires to specify the PWM)
		 1 - Fixed RPM (requires to specify the RPM)
		 2 - Default
		 3 - Quiet
		 4 - Balanced
		 5 - Performance
		 6 - Custom Curve
	--fan-pwm <fan PWM> :The desired PWM speed for the selected fan. NOTE: it only works when fan mode is set to Fixed PWM
	--fan-rpm <fan RPM> :The desired RPM for the selected fan. NOTE: it works only when fan mode is set to Fixed RPM
	--fan-temps <CSV of Temperatures>	:Define Comma Separated Values of Temperatures for Fan.
	--fan-speeds <CSV of Speed Percentage>	:Define Comma Separated Values of RPM Percentage for Fan.

	Pump mode:
	--pump-mode <mode>	:set to 3 for quiet, and 5 for performance

 Without options, OpenCorsairLink will show the status of any detected Corsair Link device.

Looking forward for your comments

PS: For all the above function changes, I only implement the HID part, Asetek part stays compatible but non-implemented.
PS2: To finish, I still need to implement the pump mode printing (now just shows the hex value), the firmware version correct printing in BCD notation, correctly configure the custom curve option and finally add the external sensor value option.

@audiohacked
Copy link
Owner

audiohacked commented Dec 5, 2017

I'm actually fine with the changes. I just wish Asetek had a way to report back the fan/etc counts.

@Far0n
Copy link

Far0n commented Jan 20, 2018

@malakudi Would it be possible to provide the necessary changes in order to be able to change the pump rpm for the H110i? Seems you made it work for your device. Thanks!

@Littlejth
Copy link

@malakudi Is there a testing branch available anywhere? I noticed none of that code is in your fork and I'd like to help best I can.

@malakudi
Copy link
Contributor Author

The problem is I have a big diff file of 32KB of changes and want to break it to specific patches in order to have a description of what I did. Will try to do that in the next days (I was quite busy the last 2 months).

@malakudi
Copy link
Contributor Author

OK it wasn't that time consuming after all. I broke patch in 5 sections and created pull request.

@audiohacked
Copy link
Owner

Patch has been merged in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants