Skip to content

Commit

Permalink
drivers/cps-hid.c, NEWS.adoc: try to fix frequency scaling dynamically [
Browse files Browse the repository at this point in the history
networkupstools#2717]

Signed-off-by: Jim Klimov <[email protected]>
  • Loading branch information
jimklimov committed Jan 5, 2025
1 parent 99ea972 commit 75efd0c
Show file tree
Hide file tree
Showing 2 changed files with 168 additions and 8 deletions.
4 changes: 4 additions & 0 deletions NEWS.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
172 changes: 164 additions & 8 deletions drivers/cps-hid.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* Copyright (C)
* 2003 - 2008 Arnaud Quette <[email protected]>
* 2005 - 2006 Peter Selinger <[email protected]>
* 2020 - 2024 Jim Klimov <[email protected]>
* 2020 - 2025 Jim Klimov <[email protected]>
* 2024 Alejandro González <[email protected]>
*
* Note: this subdriver was initially generated as a "stub" by the
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -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;
}

Expand All @@ -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)
Expand Down Expand Up @@ -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 },

Expand Down

0 comments on commit 75efd0c

Please sign in to comment.