[Nut-upsuser] Cyberpower Value1200E might not need 0.667 battery scaling

[Matthew Stapleton]

Attached is the first 32 seconds of the driver output after applying the 
patch which fixes the battery scaling problem for this UPS.

Matthew Stapleton
Email: matthew4196 at gmail.com

On 10/08/14 01:27, Charles Lepple wrote:
> On Aug 8, 2014, at 9:15 AM, Charles Lepple <clepple at gmail.com> wrote:
>
>> On Aug 7, 2014, at 10:52 PM, Charles Lepple <clepple at gmail.com> wrote:
>>
>>> On Aug 7, 2014, at 10:18 PM, Matthew Stapleton <matthew4196 at gmail.com> wrote:
>>>
>>>> I just got a Cyberpower SOHO Value 1200 ELCD UPS and even with nut 2.7.2, it appears to report battery voltage too low due to the battery scaling function (In drivers/cps-hid.c). Even though the ups has usb id: 0764:0501, UPS.PowerSummary.Voltage reports 26.6 for the 24V batteries so when the 0.667 battery scale is applied that goes down to 17.7V.
>>> I'm wondering if that scale factor is only needed for that one Dynex UPS model. We should be able to do a string match, which is ugly, but should work (especially if we default to no scaling).
>>>
>>> Logged: https://github.com/networkupstools/nut/issues/142
>> It's not just Dynex models - there are some early "UPS VALUE" and "CP 1000D" units that seem to need the correction factor. I'll try to add a check that looks to see if the battery voltage makes sense.
> Matthew, can you try the attached patch, and run the driver it with -DDDD?
>
> https://github.com/networkupstools/nut/commit/c8950dee9c91ce45d05d8f220ea26891fb92329e
>
> Also, if anyone else has a Cyberpower UPS with the 0501 productID, I'd appreciate any additional testing.
>
> Thanks,
>
>
> cps_hid_scale.patch
>
>
> commit c8950dee9c91ce45d05d8f220ea26891fb92329e
> Author: Charles Lepple <clepple+nut at gmail.com>
> Date:   Sat Aug 9 11:19:27 2014 -0400
>
>      usbhid-ups (CPS): determine battery.voltage scale factor at runtime
>      
>      If the battery.voltage reading is greater than 1.4x battery.voltage.nominal,
>      apply a scale factor of 2/3 to bring the voltage back in line.
>      
>      Closes networkupstools/nut#142
>
> diff --git a/drivers/cps-hid.c b/drivers/cps-hid.c
> index a6d64ba..78c952f 100644
> --- a/drivers/cps-hid.c
> +++ b/drivers/cps-hid.c
> @@ -28,29 +28,36 @@
>   #include "cps-hid.h"
>   #include "usb-common.h"
>   
> -#define CPS_HID_VERSION      "CyberPower HID 0.3"
> +#define CPS_HID_VERSION      "CyberPower HID 0.4"
>   
>   /* Cyber Power Systems */
>   #define CPS_VENDORID 0x0764
>   
> -/*
> +/*! Battery voltage scale factor.
>    * 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).
>    */
>   static double	battery_scale = 1;
> +static int	might_need_battery_scale = 0;
> +static int	battery_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.
> + */
> +static const double battery_voltage_sanity_check = 1.4;
>   
>   static void *cps_battery_scale(USBDevice_t *device)
>   {
> -	battery_scale = 0.667;
> +	might_need_battery_scale = 1;
>   	return NULL;
>   }
>   
>   /* USB IDs device table */
>   static usb_device_id_t cps_usb_device_table[] = {
> -	/* 900AVR/BC900D, CP1200AVR/BC1200D */
> +	/* 900AVR/BC900D */
>   	{ USB_DEVICE(CPS_VENDORID, 0x0005), NULL },
> -	/* Dynex DX-800U? */
> +	/* Dynex DX-800U?, CP1200AVR/BC1200D, CP825AVR-G, CP1000AVRLCD, CP1000PFCLCD, CP1500C, CP550HG, etc. */
>   	{ USB_DEVICE(CPS_VENDORID, 0x0501), &cps_battery_scale },
>   	/* OR2200LCDRM2U, OR700LCDRM1U, PR6000LCDRTXL5U */
>   	{ USB_DEVICE(CPS_VENDORID, 0x0601), NULL },
> @@ -59,12 +66,48 @@ static usb_device_id_t cps_usb_device_table[] = {
>   	{ -1, -1, NULL }
>   };
>   
> +/*! Adjusts @a battery_scale if voltage is well above nominal.
> + */
> +static void cps_adjust_battery_scale(double batt_volt)
> +{
> +	const char *batt_volt_nom_str;
> +	double batt_volt_nom;
> +
> +	if(battery_scale_checked) {
> +		return;
> +	}
> +
> +	batt_volt_nom_str = dstate_getinfo("battery.voltage.nominal");
> +	if(!batt_volt_nom_str) {
> +		upsdebugx(2, "%s: 'battery.voltage.nominal' not available yet; skipping scale determination", __func__);
> +		return;
> +	}
> +
> +	batt_volt_nom = strtod(batt_volt_nom_str, NULL);
> +	if(batt_volt_nom == 0) {
> +		upsdebugx(3, "%s: 'battery.voltage.nominal' is %s", __func__, batt_volt_nom_str);
> +		return;
> +	}
> +
> +	if( (batt_volt / batt_volt_nom) > battery_voltage_sanity_check ) {
> +		upslogx(LOG_INFO, "%s: battery readings will be scaled by 2/3", __func__);
> +		battery_scale = 2.0/3;
> +	}
> +
> +	battery_scale_checked = 1;
> +}
> +
>   /* returns statically allocated string - must not use it again before
>      done with result! */
>   static const char *cps_battvolt_fun(double value)
>   {
>   	static char	buf[8];
>   
> +	if(might_need_battery_scale) {
> +		cps_adjust_battery_scale(value);
> +	}
> +
> +	upsdebugx(5, "%s: battery_scale = %.3f", __func__, battery_scale);
>   	snprintf(buf, sizeof(buf), "%.1f", battery_scale * value);
>   
>   	return buf;

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.alioth.debian.org/pipermail/nut-upsuser/attachments/20140811/86140247/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cyberpower_debug_log.txt.gz
Type: application/x-gzip
Size: 6728 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/nut-upsuser/attachments/20140811/86140247/attachment.bin>