diff --git a/NEWS.adoc b/NEWS.adoc index 17bbb19206..48162caf0a 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -220,6 +220,10 @@ https://github.com/networkupstools/nut/milestone/11 * in `cps-hid` subdriver, `cps_fix_report_desc()` method should now handle mismatched `LogMax` ranges for input and output voltages, whose USB Report Descriptors are wrongly encoded by some firmware versions. [#1512] + * in `cps-hid` subdriver, try to fix frequency scaling based on the values + we see from the device and/or configuration overrides (low, nominal, high) + so `499.0 Hz` reading that comes from some firmware versions gets reported + properly as `49.9Hz`. [#2717] * USB parameters (per `usb_communication_subdriver_t`) are now set back to their default values during enumeration after probing each subdriver. Having an unrelated device connected with a VID:PID matching the diff --git a/drivers/cps-hid.c b/drivers/cps-hid.c index e303e14fb2..b184b6ec0f 100644 --- a/drivers/cps-hid.c +++ b/drivers/cps-hid.c @@ -3,7 +3,7 @@ * Copyright (C) * 2003 - 2008 Arnaud Quette * 2005 - 2006 Peter Selinger - * 2020 - 2024 Jim Klimov + * 2020 - 2025 Jim Klimov * 2024 Alejandro González * * Note: this subdriver was initially generated as a "stub" by the @@ -32,7 +32,7 @@ #include "cps-hid.h" #include "usb-common.h" -#define CPS_HID_VERSION "CyberPower HID 0.82" +#define CPS_HID_VERSION "CyberPower HID 0.83" /* Cyber Power Systems */ #define CPS_VENDORID 0x0764 @@ -54,10 +54,13 @@ * For some devices, the reported battery voltage is off by factor * of 1.5 so we need to apply a scale factor to it to get the real * battery voltage. By default, the factor is 1 (no scaling). + * Similarly, some firmwares do not report the exponent well, so + * frequency values are seen as e.g. "499.0" (in "0.1 Hz" units not + * explicitly stated), instead of "49.9 Hz". */ -static double battery_scale = 1; -static int might_need_battery_scale = 0; -static int battery_scale_checked = 0; +static double battery_scale = 1, input_freq_scale = 1, output_freq_scale = 1; +static int might_need_battery_scale = 0, might_need_freq_scale = 0; +static int battery_scale_checked = 0, input_freq_scale_checked = 0, output_freq_scale_checked = 0; /*! If the ratio of the battery voltage to the nominal battery voltage exceeds * this factor, we assume that the battery voltage needs to be scaled by 2/3. @@ -69,6 +72,8 @@ static void *cps_battery_scale(USBDevice_t *device) NUT_UNUSED_VARIABLE(device); might_need_battery_scale = 1; + might_need_freq_scale = 1; + return NULL; } @@ -88,6 +93,153 @@ static usb_device_id_t cps_usb_device_table[] = { { 0, 0, NULL } }; +/*! Adjusts frequency if it is order(s) of magnitude off + * is_input = 1 for input.frequency and 0 for output.frequency + */ +static void cps_adjust_frequency_scale(double freq_report, int is_input) +{ + const char *freq_low_str, *freq_high_str, *freq_nom_str; + double freq_low = 0, freq_high = 0, freq_nom = 0; + + if ((is_input && input_freq_scale_checked) || (!is_input && output_freq_scale_checked)) + return; + + /* May be not available from device itself; but still + * may be set by user as default/override options; + * if not, we default for 50Hz and/or 60Hz range +- 10%. + */ + freq_nom_str = dstate_getinfo(is_input ? "input.frequency.nominal" : "output.frequency.nominal"); + freq_low_str = dstate_getinfo(is_input ? "input.frequency.low" : "output.frequency.low"); + freq_high_str = dstate_getinfo(is_input ? "input.frequency.high" : "output.frequency.high"); + + if (freq_nom_str) + freq_nom = strtod(freq_nom_str, NULL); + if (freq_low_str) + freq_low = strtod(freq_low_str, NULL); + if (freq_high_str) + freq_high = strtod(freq_high_str, NULL); + + if (d_equal(freq_nom, 0)) { + if (45 < freq_low && freq_low <= 50) + freq_nom = 50; + else if (50 <= freq_high && freq_high <= 55) + freq_nom = 50; + else if (45 < freq_report && freq_report <= 55) + freq_nom = 50; + else if (450 < freq_report && freq_report <= 550) + freq_nom = 50; + else if (55 < freq_low && freq_low <= 60) + freq_nom = 60; + else if (60 <= freq_high && freq_high <= 65) + freq_nom = 60; + else if (55 < freq_report && freq_report <= 65) + freq_nom = 60; + else if (550 < freq_report && freq_report <= 650) + freq_nom = 60; + + upsdebugx(3, "%s: '%sput.frequency.nominal' is %s, guessed %0.1f%s", + __func__, is_input ? "in" : "out", NUT_STRARG(freq_nom_str), + d_equal(freq_nom, 0) ? 55 : freq_nom, + d_equal(freq_nom, 0) ? " (50 or 60Hz range)" : ""); + } + + if (d_equal(freq_low, 0)) { + if (d_equal(freq_nom, 0)) + freq_low = 45.0; + else + freq_low = freq_nom * 0.95; + + upsdebugx(3, "%s: '%sput.frequency.low' is %s, defaulting to %0.1f", + __func__, is_input ? "in" : "out", NUT_STRARG(freq_low_str), freq_low); + } + + if (d_equal(freq_high, 0)) { + if (d_equal(freq_nom, 0)) + freq_high = 65.0; + else + freq_high = freq_nom * 1.05; + + upsdebugx(3, "%s: '%sput.frequency.high' is %s, defaulting to %0.1f", + __func__, is_input ? "in" : "out", NUT_STRARG(freq_high_str), freq_high); + } + + if (freq_low <= freq_report && freq_report <= freq_high) { + if (is_input) { + input_freq_scale = 1.0; + input_freq_scale_checked = 1; + } else { + output_freq_scale = 1.0; + output_freq_scale_checked = 1; + } + /* We should be here once per freq type... */ + upsdebugx(1, "%s: Determined scaling factor " + "needed for '%sput.frequency': 1.0", + __func__, is_input ? "in" : "out"); + } + else + if (freq_low <= freq_report/10.0 && freq_report/10.0 <= freq_high) { + if (is_input) { + input_freq_scale = 0.1; + input_freq_scale_checked = 1; + } else { + output_freq_scale = 0.1; + output_freq_scale_checked = 1; + } + /* We should be here once per freq type... */ + upsdebugx(1, "%s: Determined scaling factor " + "needed for '%sput.frequency': 0.1", + __func__, is_input ? "in" : "out"); + } + else + { + /* We might return here, so do not log too loudly */ + upsdebugx(2, "%s: Could not determine scaling factor " + "needed for '%sput.frequency', will report " + "it as is (and might detect better later)", + __func__, is_input ? "in" : "out"); + } +} + +/* returns statically allocated string - must not use it again before + done with result! */ +static const char *cps_input_freq_fun(double value) +{ + static char buf[8]; + + if (might_need_freq_scale) { + cps_adjust_frequency_scale(value, 1); + } + + upsdebugx(5, "%s: input_freq_scale = %.3f", __func__, input_freq_scale); + snprintf(buf, sizeof(buf), "%.1f", input_freq_scale * value); + + return buf; +} + +static info_lkp_t cps_input_freq[] = { + { 0, NULL, &cps_input_freq_fun, NULL } +}; + +/* returns statically allocated string - must not use it again before + done with result! */ +static const char *cps_output_freq_fun(double value) +{ + static char buf[8]; + + if (might_need_freq_scale) { + cps_adjust_frequency_scale(value, 0); + } + + upsdebugx(5, "%s: output_freq_scale = %.3f", __func__, output_freq_scale); + snprintf(buf, sizeof(buf), "%.1f", output_freq_scale * value); + + return buf; +} + +static info_lkp_t cps_output_freq[] = { + { 0, NULL, &cps_output_freq_fun, NULL } +}; + /*! Adjusts @a battery_scale if voltage is well above nominal. */ static void cps_adjust_battery_scale(double batt_volt) @@ -247,18 +399,22 @@ static hid_info_t cps_hid2nut[] = { { "BOOL", 0, 0, "UPS.Output.Overload", NULL, NULL, 0, overload_info }, /* Input page */ - { "input.frequency", 0, 0, "UPS.Input.Frequency", NULL, "%.1f", 0, NULL }, + /* FIXME: Check if something like "UPS.Flow([N]?).ConfigFrequency" + * is available for "input.frequency.nominal" */ + { "input.frequency", 0, 0, "UPS.Input.Frequency", NULL, "%.1f", 0, cps_input_freq }, { "input.voltage.nominal", 0, 0, "UPS.Input.ConfigVoltage", NULL, "%.0f", 0, NULL }, { "input.voltage", 0, 0, "UPS.Input.Voltage", NULL, "%.1f", 0, NULL }, { "input.transfer.low", ST_FLAG_RW | ST_FLAG_STRING, 10, "UPS.Input.LowVoltageTransfer", NULL, "%.0f", HU_FLAG_SEMI_STATIC, NULL }, { "input.transfer.high", ST_FLAG_RW | ST_FLAG_STRING, 10, "UPS.Input.HighVoltageTransfer", NULL, "%.0f", HU_FLAG_SEMI_STATIC, NULL }, - /* used by CP1350EPFCLCD */ + /* used by CP1350EPFCLCD; why oh why "UPS.Output"?.. */ { "input.transfer.low", ST_FLAG_RW | ST_FLAG_STRING, 10, "UPS.Output.LowVoltageTransfer", NULL, "%.0f", HU_FLAG_SEMI_STATIC, NULL }, { "input.transfer.high", ST_FLAG_RW | ST_FLAG_STRING, 10, "UPS.Output.HighVoltageTransfer", NULL, "%.0f", HU_FLAG_SEMI_STATIC, NULL }, { "input.sensitivity", ST_FLAG_RW | ST_FLAG_STRING, 0, "UPS.Output.CPSInputSensitivity", NULL, "%s", HU_FLAG_SEMI_STATIC | HU_FLAG_ENUM, cps_sensitivity_info }, /* Output page */ - { "output.frequency", 0, 0, "UPS.Output.Frequency", NULL, "%.1f", 0, NULL }, + /* FIXME: Check if something like "UPS.Flow([N]?).ConfigFrequency" + * is available for "output.frequency.nominal" */ + { "output.frequency", 0, 0, "UPS.Output.Frequency", NULL, "%.1f", 0, cps_output_freq }, { "output.voltage", 0, 0, "UPS.Output.Voltage", NULL, "%.1f", 0, NULL }, { "output.voltage.nominal", 0, 0, "UPS.Output.ConfigVoltage", NULL, "%.0f", 0, NULL },