kiwi-cam / homebridge-broadlink-rm

[This fork supports TV accessories] Broadlink RM Mini and Pro plugin for homebridge: https://github.com/nfarina/homebridge
Apache License 2.0
304 stars 96 forks source link

humidifier-dehumidifier accessory is throwing an error when "noHumidity" is set to true #90

Closed CyberMrProper closed 3 years ago

CyberMrProper commented 3 years ago

This is the config I have in my setup:

{ "name": "Humidifier", "type": "humidifier-dehumidifier", "host": "192.168.4.23", "showSwingMode": false, "humidifierOnly": true, "showLockPhysicalControls": false, "noHumidity": true, "data": {} }

This is the error in the console:

[2020-10-25 14:19:15] [Broadlink RM] Humidifier getSwitchState: false [2020-10-25 14:19:16] ReferenceError: serviceManager is not defined     at HumidifierDehumidifierAccessory.getCurrentHumidity (/usr/lib/node_modules/homebridge-broadlink-rm-pro/accessories/humidifier-dehumidifier.js:182:7)     at CurrentRelativeHumidity.emit (events.js:314:20)     at CurrentRelativeHumidity.EventEmitter.emit (/usr/lib/node_modules/homebridge/node_modules/hap-nodejs/src/lib/EventEmitter.ts:42:22)     at CurrentRelativeHumidity.Characteristic._this.getValue (/usr/lib/node_modules/homebridge/node_modules/hap-nodejs/src/lib/Characteristic.ts:489:12)     at ServiceManager.refreshCharacteristicUI (/usr/lib/node_modules/homebridge-broadlink-rm-pro/helpers/serviceManager.js:27:44)     at Timeout._onTimeout (/usr/lib/node_modules/homebridge-broadlink-rm-pro/node_modules/homebridge-platform-helper/accessory.js:195:42)     at listOnTimeout (internal/timers.js:554:17)     at processTimers (internal/timers.js:497:7)

@kiwi-cam It would be great also to have the option to read current humidity from a file, same way aircon does. Otherwise, it will appear as 0% in the house. Thanks!!!

Faisalthe01 commented 3 years ago

i think it is form "noHumidity": true, can you get rid of that and check

CyberMrProper commented 3 years ago

Right, that is the property causing the issue. Ideally I would like to read the humidity value from a file, but since that is not implemented, then I wanted to get rid of the 0% setting that property to true. But it seems it's not working.

Faisalthe01 commented 3 years ago

@kiwi-cam i think the noHumidity you added doesn’t go along with line 229

    this.serviceManager.addGetCharacteristic({
      name: 'currentHumidity',
      type: Characteristic.CurrentRelativeHumidity,
      method: this.getCurrentHumidity,
      bind: this
    });
CyberMrProper commented 3 years ago

Ideally I'd like to read the value from a file and not configure a default value. I don't think we can remove currentHumidity from the Accessory as it's a required property.

Faisalthe01 commented 3 years ago

Yeah i know it cannot be removed as i added it there to work with the value received from the Broadlink rm4 but it does not go along with what’s in noHumidity that kiwi-cam added later. If we move the line 229-234 into if (noHumidity == false) and add the following into if (noHumidity). This will fix the error you are getting if you choose noHumidity: true. To get from the file would require more code

this.serviceManager.addToggleCharacteristic({
      name: 'currentRelativeHumidity',
      type: Characteristic.CurrentRelativeHumidity,
      getMethod: this.getCharacteristicValue,
      setMethod: this.setCharacteristicValue,
      bind: this,
      props: { }
    });
CyberMrProper commented 3 years ago

Right, lets wait for @kiwi-cam feedback on this.

kiwi-cam commented 3 years ago

I think you're over complicating things :-) noHumidity is only to control whether humidity readings are taken from the Broadlink device. humidiferOnly and dehumidifierOnly are to control the humidifier's functions. The later two shouldn't depend on the other (IMO). If noHumidity is set the accessory will function as originally developed by @lprhodes and I don't see an issue with using humidiferOnly or dehumidifierOnly in this mode.

The original error message posted is a missing definition which I'll fix up now.

Thoughts?

kiwi-cam commented 3 years ago

@CyberMrProper version homebridge-broadlink-rm-pro@4.3.4-beta.8 should sort the original error message for you. Could you confirm?

I'll look at adding humidity file support at some stage soon too.

CyberMrProper commented 3 years ago

Still not working with "noHumidity": true, here is the stacktrace:

[2020-10-26 21:51:48] RangeError: Maximum call stack size exceeded at Object.stringify () at LocalStorage.stringify (/usr/lib/node_modules/homebridge-broadlink-rm-pro/node_modules/node-persist/src/local-storage.js:530:29) at LocalStorage.copy (/usr/lib/node_modules/homebridge-broadlink-rm-pro/node_modules/node-persist/src/local-storage.js:547:32) at LocalStorage.setItemSync (/usr/lib/node_modules/homebridge-broadlink-rm-pro/node_modules/node-persist/src/local-storage.js:253:26) at Object.save (/usr/lib/node_modules/homebridge-broadlink-rm-pro/node_modules/homebridge-platform-helper/helpers/persistentState.js:30:22) at /usr/lib/node_modules/homebridge-broadlink-rm-pro/node_modules/homebridge-platform-helper/accessory.js:183:23 at Object.set (/usr/lib/node_modules/homebridge-broadlink-rm-pro/node_modules/homebridge-platform-helper/accessory.js:9:7) at HumidifierDehumidifierAccessory.getCurrentHumidity (/usr/lib/node_modules/homebridge-broadlink-rm-pro/accessories/humidifier-dehumidifier.js:174:37) at CurrentRelativeHumidity.emit (events.js:314:20) at CurrentRelativeHumidity.EventEmitter.emit (/usr/lib/node_modules/homebridge/node_modules/hap-nodejs/src/lib/EventEmitter.ts:42:22)

kiwi-cam commented 3 years ago

A new error is sign of progress 😀

Try updating to the latest beta and give it another go. Looks like a was a bit trigger happy with the copy and paste when I tweaked that logic.

CyberMrProper commented 3 years ago

Thank you for the quick reply! now this issue is fixed. I’ve found a couple of additional glitches in this accessory, but I should probably open new bugs. Eg:

  1. when humidifierOnly is true and the device is off, the device status in HK keeps saying “Rising to x %” instead of “off” (I think current humidity status is not updated to inactive when turning off the device)
  2. Auto off option is not working