search

sean9keenan / homebridge-bigAssFans

A Homebridge plugin for Big Ass Fans
MIT License
28 stars 15 forks source link

Plugin causes Homebridge v1.3.0 beta to crash #37

Open mlamoure opened 3 years ago

mlamoure commented 3 years ago

Hello -

I answered the call for testers for the v1.3.0 beta of homebridge. Updating results in a crash loop for me with the following in the logs:

[1/2/2021, 2:16:23 PM] TypeError: Cannot read property 'omitEventUpdate' of null at Bridge.Accessory.handleCharacteristicChangeEvent (/usr/local/lib/node_modules/homebridge/node_modules/hap-nodejs/src/lib/Accessory.ts:1808:100) at Accessory.<anonymous> (/usr/local/lib/node_modules/homebridge/node_modules/hap-nodejs/src/lib/Accessory.ts:552:84) at Accessory.emit (events.js:315:20) at Accessory.handleCharacteristicChangeEvent (/usr/local/lib/node_modules/homebridge/node_modules/hap-nodejs/src/lib/Accessory.ts:1797:12) at Service.emit (events.js:315:20) at Characteristic.<anonymous> (/usr/local/lib/node_modules/homebridge/node_modules/hap-nodejs/src/lib/Service.ts:669:12) at Characteristic.emit (events.js:315:20) at Object.0.03283219355104894 (/homebridge/node_modules/@miblanchard/homebridge-big-ass-fans/index.js:180:20) at BigAssProperty.<anonymous> (/homebridge/node_modules/@miblanchard/homebridge-big-ass-fans/node_modules/BigAssFansAPI/BigAssApi.js:158:48) at BigAssFan.<anonymous> (/homebridge/node_modules/@miblanchard/homebridge-big-ass-fans/node_modules/BigAssFansAPI/BigAssApi.js:319:36)

jmissig commented 3 years ago

I saw the same issue, thanks for reporting

Supereg commented 3 years ago

The problem is this line https://github.com/sean9keenan/homebridge-bigAssFans/blob/4807275d64c0e91b4ab715cc0d6d12cd1e88641b/index.js#L180-L184.

A plugin MUST NOT fire the change event manually. This is NOT public API. For such scenarios where you want to push an updated value to HomeKit, use characteristic.updateValue(...).

mlamoure commented 3 years ago

The problem is this line

https://github.com/sean9keenan/homebridge-bigAssFans/blob/4807275d64c0e91b4ab715cc0d6d12cd1e88641b/index.js#L180-L184

. A plugin MUST NOT fire the change event manually. This is NOT public API. For such scenarios where you want to push an updated value to HomeKit, use characteristic.updateValue(...).

Understand the plugin is calling a private event here. Is there a way to mitigate where this does not cause a crash of Homebridge, or has the private event changed since prior to 1.3 and now the crash is because it's being called incorrectly despite never being intended to being called by a plugin in the first place?

Does anyone know if this plugin is actively maintained? I see a number of outstanding PR's for the motion sensor and no recent commits.

Supereg commented 3 years ago

[...] has the private event changed since prior to 1.3 and now the crash is because it's being called incorrectly despite never being intended to being called by a plugin in the first place?

That is exactly what is happening. I see if I may be able to mitigate that.

EDIT: Indeed latest beta should not crash anymore, as of this.

However the plugin MUST still change that, as emitting a change event does not properly persist the value in the characteristic cache.

mlamoure commented 3 years ago

@Supereg confirmed beta 41 does not crash. thank you.