SteveCohen / homebridge-BOMgovau

Makes BOM.gov.au data available in your Homebridge(Homekit) setup
5 stars 1 forks source link

Homebridge v2.0 Breaking Updates #14

Open mitch7391 opened 1 month ago

mitch7391 commented 1 month ago

Hey @SteveCohen I am not sure if you are aware but the next version of Homebridge (v2.0) comes with breaking changes for a lot of Homebridge plug-ins and it has been put on the developers to update their plug-ins.

For Plug-In Developers

I have tested this plug-in on the beta version and it will crash Homebridge.

Describe the solution you'd like: Please have a read of the link above and see what changes are required for this plug-in.

Describe alternatives you've considered: All users of this plug-in will have to remain on Homebridge v1.8.4.

Additional context: image

Logs:

[27/07/2024, 3:50:29 pm] [Location] Initializing BOMgovau accessory...
[27/07/2024, 3:50:29 pm] [Location] [BOM gov au] Enabling sensors
[27/07/2024, 3:50:29 pm] [Location] [BOM gov au] Sensor temperature = true
[27/07/2024, 3:50:29 pm] [Location] [BOM gov au] Sensor humidity = true
[27/07/2024, 3:50:29 pm] [Location] [BOM gov au] Sensor apparentTemp = true
[27/07/2024, 3:50:29 pm] [Location] [BOM gov au] Station Name is Location
[27/07/2024, 3:50:29 pm] [Location] [BOM gov au] Station URL is http://www.bom.gov.au/fwo/IDWXXXXX/IDWXXXXX.XXXXX.json
[27/07/2024, 3:50:29 pm] TypeError: Class constructor Characteristic cannot be invoked without 'new'
    at new CommunityTypes.AtmosphericPressureLevel (/usr/local/lib/node_modules/homebridge-bomgovau/node_modules/hap-nodejs-community-types/types.js:445:20)
    at TemperatureSensor.addCharacteristic (/usr/local/lib/node_modules/homebridge/node_modules/hap-nodejs/src/lib/Service.ts:528:57)
    at new BOMgovau (/usr/local/lib/node_modules/homebridge-bomgovau/index.js:87:33)
    at /usr/local/lib/node_modules/homebridge/src/server.ts:362:50
    at Array.forEach (<anonymous>)
    at Server.loadAccessories (/usr/local/lib/node_modules/homebridge/src/server.ts:282:29)
    at Server.start (/usr/local/lib/node_modules/homebridge/src/server.ts:167:12)
[27/07/2024, 3:50:29 pm] Got SIGTERM, shutting down Homebridge...
[27/07/2024, 3:50:29 pm] AssertionError [ERR_ASSERTION]: Cannot generate setupURI on an accessory that isn't published yet!
    at Bridge.setupURI (/usr/local/lib/node_modules/homebridge/node_modules/hap-nodejs/src/lib/Accessory.ts:897:11)
    at Server.setServerStatus (/usr/local/lib/node_modules/homebridge/src/server.ts:143:45)
    at Server.teardown (/usr/local/lib/node_modules/homebridge/src/server.ts:187:10)
    at signalHandler (/usr/local/lib/node_modules/homebridge/src/cli.ts:90:12)
    at process.emit (node:events:519:28)
    at process.emit (/usr/local/lib/node_modules/homebridge/node_modules/source-map-support/source-map-support.js:516:21)
[27/07/2024, 3:50:34 pm] [HB Supervisor] Homebridge Process Ended. Code: 143, Signal: null
[27/07/2024, 3:50:39 pm] [HB Supervisor] Restarting Homebridge...
mitch7391 commented 1 month ago

This could be a very good time to tackle #4 and #7 as you said these would be breaking changes... Considering Homebridge themselves are about to break this plug-in, now would be the best time :)

benwebbbenwebb commented 3 weeks ago

Hi @mitch7391

That error actually looks to be coming from within the hap-nodejs-community-types implementation...

Taking a quick look I can also see there would be other issues with Homebridge v2.0 (usage of Characteristic.Formats and Characteristic.Perms for example) even after fixing the Class constructor Characteristic cannot be invoked without 'new' error.

Given that, it might be best to bring the code block to define the AtmosphericPressureLevel characteristic in to this project instead and remove the hap-nodejs-community-types dependency.

Code from hap-nodejs-community-types (click to expand) ```javascript CommunityTypes.AtmosphericPressureLevel = function () { Characteristic.call(this, 'Barometric Pressure', CommunityTypes.AtmosphericPressureLevel.UUID); this.setProps({ format: Characteristic.Formats.UINT8, // should probably be UINT16 unit: "mbar", minValue: 800, maxValue: 1200, minStep: 1, perms: [ Characteristic.Perms.READ, Characteristic.Perms.NOTIFY ] }); this.value = this.getDefaultValue(); }; CommunityTypes.AtmosphericPressureLevel.UUID = '28FDA6BC-9C2A-4DEA-AAFD-B49DB6D155AB'; inherits(CommunityTypes.AtmosphericPressureLevel, Characteristic); ```

Maybe something along the lines of:

(click to expand) ```javascript const AtmosphericPressureProps = { format: api.hap.Formats.FLOAT, unit: "hPa", minValue: 800, maxValue: 1200, minStep: 0.1, perms: [ api.hap.Perms.READ, api.hap.Perms.NOTIFY ] } const AtmosphericPressureCharacteristic = new Characteristic('AtmosphericPressure', '28FDA6BC-9C2A-4DEA-AAFD-B49DB6D155AB', AtmosphericPressureProps) this.temperatureService.addCharacteristic(AtmosphericPressureCharacteristic) .on('get', this.getStatePressue.bind(this)); // may not be required if Pressure isn't shown in app? BOMgovau.prototype.getStatePressue = function(callback) { this.updateObservations(); callback(null, this.obs.press); } ```

You'd also need to update the this.temperatureService.setCharacteristic(CommunityTypes.AtmosphericPressureLevel, ...) line to point to the new Characteristic.

Completely untested, but I think it's on the right track.

benwebbbenwebb commented 3 weeks ago

Otherwise another package could potentially be used, for example homebridge-lib includes an Air Pressure characteristic (originally from Eve products).

I think it can be pulled in via either the eve API Wrapper or EveHomeKitTypes utility class?

oandpg commented 3 weeks ago

I use this plugin and would be sad to see it go. Looks like Steve hasn’t made any changes for a few years. Can someone take on the task? My coding days are long over. :)

mitch7391 commented 3 weeks ago

Thanks for the guidance @benwebbbenwebb! I am still learning my ways through the meatier side of plugins, even though I do manage my own that piggybacks off another but mostly dabbles in shell…

@oandpg I can try take a lot but am struggling to keep up with my own plugin before my twins arrived, now I am very much struggling to find the time these days! We would still need @SteveCohen to log in and accept a PR even if I was to do the work for it to be then merged into this plugin…

oandpg commented 3 weeks ago

@mitch7391 I think your need for sleep is more important than my wish for this plugin!

SteveCohen commented 4 days ago

Hi folks, sorry I’ve been quiet. I’d be happy to accept a PR but have little capacity to test it. I, too, have some personal things going on. Then I’d need to remember how to merge back to NPM… but happy to try if you have confirmed working code!

Let’s talk alternatives for a second to save all our sanity. Frankly, my ideal approach to getting BOM data into Home has changed. If I had to do this again, I’d simply write a Python/similar script to load the data through MQTT, leveraging something like homebridge-mqttthing plugin (or whatever is supported in homebridge 2.0). If we’re going to do breaking changes… tbh that’s where I’d put my effort.

Otherwise I’d put my time into a Matter-based solution, bypassing homebridge entirely. That’s a bit more future proofed but still a new ecosystem.

Anyway. If you want to do PRs I’ll get round to accepting them and try to merge to NPM. Otherwise I suggest looking into MQTTThing for a longer term solution. I’d be happy to link to other repositories to send users there, if you do one of these paths.

thanks both for your enthusiasm in the meantime. Let me know what you want to do.