cr3ative / homebridge-apcaccess

what if apcaccess, but in homebridge??
MIT License
8 stars 5 forks source link

Invalid value for battery charge. NaN #10

Closed Daveblanche closed 1 year ago

Daveblanche commented 2 years ago

Hi there This plugin is just what I’m after. Something to tell me when my house IT falls down and I can tell my wife to shut things down…whilst I’m working in a foreign country. But I have a wee snag in that I can’t get it to work. Can I send you a debug report so you could tell me what I’ve done wrong? Probably best if I can send it to you direct as I do bones not computers. Not sure how to run through everything and anonymise it.

cr3ative commented 2 years ago

Sure, attach a log and I'll take a look - no promises though!

If you also wouldn't mind just running apcaccess on the command line too, so I can see what it's putting out for the charge value, that'd be great.

upcsymbol commented 2 years ago

This fixed for me


    // BCHARGE
    const percentage = parseFloat(parseInt(this.latestJSON.BCHARGE, 10));
cr3ative commented 2 years ago

Hm, that shouldn't make a difference - if parseInt fails, parseFloat would too, even wrapped. I'm very happy to publish a new version if someone can tell me what BCHARGE is set to.

image

For now, I'll see about catching this and reporting zero, so it's within tolerances.

cr3ative commented 2 years ago

I've published 0.11, which should output some useful debug, and set the charge level to zero if it's NaN:

https://github.com/cr3ative/homebridge-apcaccess/blob/master/index.js#L70

upcsymbol commented 2 years ago

I've published 0.11, which should output some useful debug, and set the charge level to zero if it's NaN:

https://github.com/cr3ative/homebridge-apcaccess/blob/master/index.js#L70

Results

[10/1/2022, 4:10:06 PM] [UPS] Low Battery?  BATTERY_LEVEL_NORMAL
[10/1/2022, 4:10:06 PM] [UPS] Battery level NaN problem; recorded as  undefined false
[10/1/2022, 4:10:06 PM] [homebridge-apcaccess] This plugin threw an error from the characteristic 'Battery Level': Unhandled error thrown inside read handler for characteristic: Assignment to constant variable.. See https://homebridge.io/w/JtMGR for more info.
[10/1/2022, 4:10:06 PM] [homebridge-apcaccess] TypeError: Assignment to constant variable.
    at APCAccess.getBatteryLevel (/usr/local/lib/node_modules/homebridge-apcaccess/index.js:72:18)
    at BatteryLevel.emit (node:events:526:28)
    at /usr/local/lib/node_modules/homebridge/node_modules/hap-nodejs/src/lib/Characteristic.ts:1592:14
    at new Promise (<anonymous>)
    at BatteryLevel.<anonymous> (/usr/local/lib/node_modules/homebridge/node_modules/hap-nodejs/src/lib/Characteristic.ts:1590:12)
    at step (/usr/local/lib/node_modules/homebridge/node_modules/tslib/tslib.js:144:27)
    at Object.next (/usr/local/lib/node_modules/homebridge/node_modules/tslib/tslib.js:125:57)
    at /usr/local/lib/node_modules/homebridge/node_modules/tslib/tslib.js:118:75
    at new Promise (<anonymous>)
    at __awaiter (/usr/local/lib/node_modules/homebridge/node_modules/tslib/tslib.js:114:16)
    at BatteryLevel.Characteristic.handleGetRequest (/usr/local/lib/node_modules/homebridge/node_modules/hap-nodejs/dist/lib/Characteristic.js:689:38)
    at BatteryLevel.<anonymous> (/usr/local/lib/node_modules/homebridge/node_modules/hap-nodejs/src/lib/Characteristic.ts:2207:22)
    at step (/usr/local/lib/node_modules/homebridge/node_modules/tslib/tslib.js:144:27)
    at Object.next (/usr/local/lib/node_modules/homebridge/node_modules/tslib/tslib.js:125:57)
    at /usr/local/lib/node_modules/homebridge/node_modules/tslib/tslib.js:118:75
    at new Promise (<anonymous>)
[10/1/2022, 4:10:06 PM] [UPS] Charging state:  CHARGING
cr3ative commented 2 years ago

Oh that’s fun. I’ve made an obvious code mistake, but also the JSON object coming through to parse is completely blank. Ok! I’ll look at this on Monday. On 1 Oct 2022, at 21:14, upcsymbol @.***> wrote:

I've published 0.11, which should output some useful debug, and set the charge level to zero if it's NaN: https://github.com/cr3ative/homebridge-apcaccess/blob/master/index.js#L70

Results [10/1/2022, 4:10:06 PM] [UPS] Low Battery? BATTERY_LEVEL_NORMAL [10/1/2022, 4:10:06 PM] [UPS] Battery level NaN problem; recorded as undefined false [10/1/2022, 4:10:06 PM] [homebridge-apcaccess] This plugin threw an error from the characteristic 'Battery Level': Unhandled error thrown inside read handler for characteristic: Assignment to constant variable.. See https://homebridge.io/w/JtMGR for more info. [10/1/2022, 4:10:06 PM] [homebridge-apcaccess] TypeError: Assignment to constant variable. at APCAccess.getBatteryLevel (/usr/local/lib/node_modules/homebridge-apcaccess/index.js:72:18) at BatteryLevel.emit (node:events:526:28) at /usr/local/lib/node_modules/homebridge/node_modules/hap-nodejs/src/lib/Characteristic.ts:1592:14 at new Promise () at BatteryLevel. (/usr/local/lib/node_modules/homebridge/node_modules/hap-nodejs/src/lib/Characteristic.ts:1590:12) at step (/usr/local/lib/node_modules/homebridge/node_modules/tslib/tslib.js:144:27) at Object.next (/usr/local/lib/node_modules/homebridge/node_modules/tslib/tslib.js:125:57) at /usr/local/lib/node_modules/homebridge/node_modules/tslib/tslib.js:118:75 at new Promise () at __awaiter (/usr/local/lib/node_modules/homebridge/node_modules/tslib/tslib.js:114:16) at BatteryLevel.Characteristic.handleGetRequest (/usr/local/lib/node_modules/homebridge/node_modules/hap-nodejs/dist/lib/Characteristic.js:689:38) at BatteryLevel. (/usr/local/lib/node_modules/homebridge/node_modules/hap-nodejs/src/lib/Characteristic.ts:2207:22) at step (/usr/local/lib/node_modules/homebridge/node_modules/tslib/tslib.js:144:27) at Object.next (/usr/local/lib/node_modules/homebridge/node_modules/tslib/tslib.js:125:57) at /usr/local/lib/node_modules/homebridge/node_modules/tslib/tslib.js:118:75 at new Promise () [10/1/2022, 4:10:06 PM] [UPS] Charging state: CHARGING

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you commented.Message ID: @.***>

cr3ative commented 2 years ago

Published 0.1.2 which will report zero if bad information is passed through. This failure looks transient, so it should just avoid a crash rather than causing bad reporting for a significant period of time.

grgmll commented 2 years ago

Hi, after installing the new version (0.1.2) the battery level value is always at 0%, with the previous version everything was working fine

cr3ative commented 2 years ago

Hm. It should only set to zero if we got NaN to avoid a crash. What's going on?

grgmll commented 2 years ago

with version 0.1.1

[05/10/2022, 11:06:51] [UPS Cantina] Low Battery? BATTERY_LEVEL_NORMAL [05/10/2022, 11:06:51] [UPS Cantina] Battery level NaN problem; recorded as undefined false [05/10/2022, 11:06:51] [homebridge-apcaccess] This plugin threw an error from the characteristic 'Battery Level': Unhandled error thrown inside read handler for characteristic: Assignment to constant variable.. See https://homebridge.io/w/JtMGR for more info. [05/10/2022, 11:06:51] [UPS Cantina] Charging state: CHARGING .... .... .... [05/10/2022, 11:07:35] [UPS Cantina] Low Battery? BATTERY_LEVEL_NORMAL [05/10/2022, 11:07:35] [UPS Cantina] Battery Level: 100 [05/10/2022, 11:07:35] [UPS Cantina] Charging state: NOT_CHARGING

with 0.1.2

0.1.2 [05/10/2022, 11:16:13] [UPS Cantina] Low Battery? BATTERY_LEVEL_NORMAL [05/10/2022, 11:16:13] [UPS Cantina] Battery Level: NaN [05/10/2022, 11:16:13] [homebridge-apcaccess] This plugin generated a warning from the characteristic 'Battery Level': characteristic value expected valid finite number and received "NaN" (number). See https://homebridge.io/w/JtMGR for more info. [05/10/2022, 11:16:13] [UPS Cantina] Charging state: CHARGING .... .... .... [05/10/2022, 11:17:02] [UPS Cantina] Low Battery? BATTERY_LEVEL_NORMAL [05/10/2022, 11:17:02] [UPS Cantina] Battery Level: 0 [05/10/2022, 11:17:02] [UPS Cantina] Charging state: NOT_CHARGING

upcsymbol commented 2 years ago

Hm. It should only set to zero if we got NaN to avoid a crash. What's going on?

The issue is that it's pulling the "100.0 Percent" from apcaccess - which is text. You either need to treat it as such or include the flag to drop value labels. My current fix to getBatteryLevel() below.

getBatteryLevel(callback) {
    // BCHARGE
    let battPctValue = 0;
    let battVal = this.latestJSON.BCHARGE;
    if(battVal != undefined) {
        const battArray = battVal.split(".");
        battPctValue = parseFloat(parseFloat(battArray[0]*-1)*-1);
        this.log('Battery Level: ', battPctValue);
    } else {
        battPctValue = 0;
        this.log('Battery Level: ', battPctValue);
    }
    callback(null, battPctValue);
  }
cr3ative commented 2 years ago

Brilliant, thank you for the incoming value and fix! I’ll get this rolled in soon.

cr3ative commented 2 years ago

Thanks for the logs. It looks like we might just revert as homebridge doesn’t crash even if presented with a bad value. 

cr3ative commented 2 years ago

Published + homebridge-apcaccess@0.1.3 with the fix by @upcsymbol above.

grgmll commented 2 years ago

Thanks, with this update everything seems to work.