itavero / homebridge-z2m

Expose your Zigbee devices to HomeKit with ease, by integrating 🐝 Zigbee2MQTT with 🏠 Homebridge.
https://z2m.dev
Apache License 2.0
319 stars 49 forks source link

Refactor code + use new exposes info provided by zigbee2mqtt #18

Closed itavero closed 3 years ago

itavero commented 4 years ago

I noticed platformAccessory.ts is getting very long (it's over 900 lines). Some of the interfaces should move to separate sources, as well as all the different ServiceWrapper implementations.

itavero commented 3 years ago

I've also noticed that some keys can apply to multiple services. For instance, several CO/gas/smoke detectors also have a tamper detection and some of them to for instance both gas and CO. This means that the tamper key, if present, can be used for an optional Status Tampered characteristic in both the Carbon Monoxide Sensor and the Leak Sensor service.

With the current design/implementation this is however not possible.

I think I need to keep track of which keys have been used, but still give other ServiceWrappers also the possibility to use them. And ofcourse, I probably also need an additional abstraction to reuse the code for these "reusable" characteristics that can be used by multiple Services.

itavero commented 3 years ago

For the refactoring, I'm also waiting on the new exposes attribute to be released for zigbee2mqtt (see Koenkk/zigbee2mqtt#4466). This also means that I'm "postponing" the major release.

Once implemented, users will be required to use a certain minimum version of zigbee2mqtt and have the new api enabled. This might be a bit of additional work for users to updated and get things working again, but I think it outways the benefits. This plugin will have a far better out of the box experience, as it will be better at detecting the right mapping and it will do so on startup, instead of having to wait for the right data to pass by.

itavero commented 3 years ago

The mentioned attribute is in the latest release, so I'll start refactoring code and mechanisms soon.

itavero commented 3 years ago

Currently busy implementing this in the z2m-exposes branch. Already implemented support for lights. I'll continue with the rest in the coming weeks. My goal is to have a new version out before the end of the year, that supports at least the devices it supports right now, but via the new exposes information.

I'll try to post updates in this issue while I'm implementing.

itavero commented 3 years ago

Short update: currently I have the lights and some sensors working. I already noticed a few things that I want to clean up though (one of the "problems" of writing code is that you gain new insights while doing it, which often lead to other ideas and therefor to refactoring code you've just written). πŸ˜…

I also want to add unit tests to verify the implementations (so that the regression tests become automated and reproducible, instead of me just running the plugin and trying some stuff at random). I might add the unit tests later and focus on getting the currently supported devices in there first, so I can put a "pre-release" out there for people to try out.

itavero commented 3 years ago

Did some more work on the new version this evening. In general the internal APIs and architecture are still fluid, although I think they are getting more stable.

Current state of what has been implemented and what not (based on the services mentioned in the README of the current version):

Even though I might have checked of certain services in the list above, decent testing/verification is still required for most of them. Also, as mentioned, I still want to write automated tests to prove the conversions and such are setup correctly and taken over from the exposes information provided by zigbee2mqtt correctly.

Next up will probably be the Window Covering as that is one of the most used accessories in my home (after the Lightbulbs of course).

With regards to the Lock Mechanism, I don't have an actual device to verify it with, so I can only do "theoretical" verification based on the code of Koenkk/zigbee-herdsman-converters.

itavero commented 3 years ago

Just implemented Window Covering. (updated the previous comment)

itavero commented 3 years ago

Just implemented the Battery Service. (updated the comment above)

Next up will probably be the Air Pressure Sensor or starting on the unit tests.

jwilling commented 3 years ago

I've been following along with your progress on this and it's looking amazing! Can't wait to try it out once it's released.

itavero commented 3 years ago

@jwilling Thanks for your message and your support. It really means a lot to me. πŸ‘

It feels like I'm still on track for making a pre-release before the end of the year.

The main "risk" will be in the automated tests, as I've never wrote tests in Typescript before and there aren't a lot of resources specifically on testing homebridge plugins. But I'll figure it out, one way or the other πŸ˜‰

itavero commented 3 years ago

Implemented Air Pressure Sensor. (updated list from earlier comment)

Tried to start on the Lock Mechanism, but up on diving into the zigbee-herdsman-converters I noticed there isn't a single lock exposing the lock_state, so I'm not sure how reliable the exposes data for the locks is at the moment.

itavero commented 3 years ago

lock_state exposes have been added just now in zigbee-herdsman-converters. Next time I have some time left, I'll probably implement the Lock Mechanism, followed by the Energy measurement stuff.

After that I'm planning to dive into the automated tests and the documentation.

itavero commented 3 years ago

I tried working on the Lock Mechanism converter/handler yesterday, but I noticed that the lock_state isn't part of the lock type's features, which I would expect. Proposed this change in Koenkk/zigbee-herdsman-converters#1930.

Just now I've added some documentation on the implemented services/characteristics (in the docs folder).

itavero commented 3 years ago

PR fo zigbee-hersmand-converters got merged, so I've implemented the Lock Mechanism handler. (I've updated the list above).

This weekend I hope to implement the energy metering stuff. If I succeed, it should be doable to publish a "pre release" version before Christmas.

A pre-release would allow users to give the new version a try and give feedback, while I'm still doing more verification using automated tests.

itavero commented 3 years ago

I've decided to leave the energy metering stuff out. I personally did not have a use case for this information, so I figured I'd better spend my time and effort on other stuff I do actually use / care about. If you miss this, please open up a feature request and also let me know what this information is useful for within a HomeKit context.

itavero commented 3 years ago

A pre-release before Christmas won't happen.

So far I've only written a few tests for the Lightbulb and Switch handlers and I've already found a couple of silly mistakes. I feel I should do some more automated tests before publishing it, just to gain some more confidence. πŸ˜…

Besides that, this week I had to (read: wanted to) spent a bit more time helping my wife with her hobby (🐴 πŸ¦™) than I anticipated.

I still think a pre-release before the year is over is doable. Next week I should have plenty of time to wrap things up.

itavero commented 3 years ago

Added more tests today for the "basic" sensor devices. Also found and fixed some bugs along the way.

The following services have no automated tests (yet):

I'll have a look at the coverage report tomorrow to see if I didn't miss any important stuff in the other tests. The current test setup for Lights / Switches can still be improved as the tests currently slightly depend on each other, which of course is not a good practice when writing tests.

I will probably also go over all the changes one more time, just to see if I still see some things that I need to polish before making a release. πŸ˜‰

itavero commented 3 years ago

Added some more tests today (mainly for the Window Covering) and made some additional changes.

I plan to merge #31 tomorrow and make a release available under homebridge-z2m@next after that, which I will be installing on my "production" environment at home as well.

itavero commented 3 years ago

Version 1.0.2 is now available via npm install homebridge-z2m@next. I'm currently running it at home.

Note that all accessories will be recreated at this point. I might have a look if I can improve on this by just removing all the services instead, before recreating them.

llnnse commented 3 years ago

The newest version doesn’t seem to recognize my devices. Currently Iβ€˜m using the newest version (1.16.2) of z2m. Do I still need to do anything else to make it work?

itavero commented 3 years ago

@tkrtz You should not need to do anything else. Can you share your logs maybe?

Which version of MQTT are you running? Do you see a message on the zigbee2mqtt/bridge/devices topic?

llnnse commented 3 years ago

Iβ€˜m using the latest version of mosquitto and there sadly isnβ€˜t a topic called zigbee2mqtt/bridge/devices. Which log do you want to see? Edit: I already tried reinstalling homebridge, the plugins (including this one), mosquito, z2m and nodejs.

itavero commented 3 years ago

Have you also configured zigbee2mqtt and homebridge-z2m to use MQTT version 5? I think it might be because zigbee2mqtt publishes the message to zigbee2mqtt/bridge/devices as retained, but now I'm doubting if this would work if the MQTT version is 4 or earlier.

I'll browse through the code of zigbee2mqtt.

I'm mainly interested in the logs of homebridge when it starts, it should list the accessories it is creating or restoring.

Update: As far as I can tell, zigbee2mqtt only publishes this message on start-up and when the device list changes (for instance, when you add a new device in z2m or rename one).

llnnse commented 3 years ago

Okay for MQTT version 4:

^[[37m[31/12/2020, 16:12:45]^[[0m ^[[36m[HB Supervisor]^[[0m Restarting Homebridge...
^[[37m[31/12/2020, 16:12:45]^[[0m ^[[36m[HB Supervisor]^[[0m Starting Homebridge with extra flags: -I
^[[37m[31/12/2020, 16:12:45]^[[0m ^[[36m[HB Supervisor]^[[0m Started Homebridge v1.1.7 with PID: 5104
^[[37m[31/12/2020, 16:13:03]^[[0m ^[[36m[HB Supervisor]^[[0m OS: Linux 5.4.79+ arm
^[[37m[31/12/2020, 16:13:03]^[[0m ^[[36m[HB Supervisor]^[[0m Node.js v15.5.0 /usr/local/bin/node
^[[37m[31/12/2020, 16:13:03]^[[0m ^[[36m[HB Supervisor]^[[0m Homebridge Path: /usr/local/lib/node_modules/homebridge/bin/homebridge
^[[37m[31/12/2020, 16:13:03]^[[0m ^[[36m[HB Supervisor]^[[0m UI Path: /usr/local/lib/node_modules/homebridge-config-ui-x/dist/bin/s$^[[37m[31/12/2020, 16:13:03]^[[0m ^[[36m[HB Supervisor]^[[0m Delaying Homebridge startup by 20 seconds on low powered server
^[[37m[31/12/2020, 16:13:24]^[[0m ^[[36m[HB Supervisor]^[[0m Starting Homebridge with extra flags: -I
^[[37m[31/12/2020, 16:13:24]^[[0m ^[[36m[HB Supervisor]^[[0m Started Homebridge v1.1.7 with PID: 5134
^[[0;37m[31/12/2020, 16:13:47] ^[[0m^[[0;36m[Homebridge UI]^[[0m ^[[0;33mHomebridge Config UI X v4.36.0 is listening on :: port 858$^[[37m[31/12/2020, 16:13:37] ^[[39mLoaded config.json with 0 accessories and 3 platforms.
^[[37m[31/12/2020, 16:13:38] ^[[39m---
^[[37m[31/12/2020, 16:13:53] ^[[39mLoaded plugin: homebridge-config-ui-x@4.36.0
^[[37m[31/12/2020, 16:13:53] ^[[39mRegistering platform 'homebridge-config-ui-x.config'
^[[37m[31/12/2020, 16:13:53] ^[[39m---
^[[37m[31/12/2020, 16:13:53] ^[[39mLoaded plugin: homebridge-intercom-automation-hat@1.0.3
^[[37m[31/12/2020, 16:13:53] ^[[39mRegistering platform 'homebridge-intercom-automation-hat.IntercomAutomationHAT'
^[[37m[31/12/2020, 16:13:53] ^[[39mRegistering accessory 'homebridge-intercom-automation-hat.IntercomAutomationHATLockAccessory'
^[[37m[31/12/2020, 16:13:53] ^[[39mRegistering accessory 'homebridge-intercom-automation-hat.IntercomAutomationHATBellAccessory'
^[[37m[31/12/2020, 16:13:53] ^[[39m---
^[[37m[31/12/2020, 16:13:55] ^[[39mLoaded plugin: homebridge-z2m@1.0.2
^[[37m[31/12/2020, 16:13:55] ^[[39mRegistering platform 'homebridge-z2m.zigbee2mqtt'
^[[37m[31/12/2020, 16:13:55] ^[[39m---
^[[37m[31/12/2020, 16:13:55] ^[[39mLoading 3 platforms...

^[[37m[31/12/2020, 16:13:55] ^[[39m^[[36m[Config] ^[[39mInitializing config platform...
^[[37m[31/12/2020, 16:13:55] ^[[39m^[[36m[Config] ^[[39mRunning in Service Mode
^[[37m[31/12/2020, 16:13:55] ^[[39m^[[36m[Intercom] ^[[39mInitializing IntercomAutomationHAT platform...
^[[37m[31/12/2020, 16:13:55] ^[[39m^[[36m[Intercom] ^[[39mIntercomPlatform constructor
^[[37m[31/12/2020, 16:13:55] ^[[39m^[[36m[Intercom] ^[[39mSetting up pyhton shell. scriptPath = /usr/local/lib/node_modules/homebri$^[[37m[31/12/2020, 16:13:56] ^[[39m^[[36m[Intercom] ^[[39mCreating IntercomPlatform accessories...
^[[37m[31/12/2020, 16:13:56] ^[[39m^[[36m[Intercom] ^[[39mIntercomLockAccessory constructor
^[[37m[31/12/2020, 16:13:56] ^[[39m^[[36m[Intercom] ^[[39mprepareLockAccessory
^[[37m[31/12/2020, 16:13:56] ^[[39m^[[36m[Intercom] ^[[39mprepareLockService
^[[37m[31/12/2020, 16:13:56] ^[[39m^[[36m[Intercom] ^[[39mIntercomBellAccessory constructor
^[[37m[31/12/2020, 16:13:56] ^[[39m^[[36m[Intercom] ^[[39mprepareBellAccessory
^[[37m[31/12/2020, 16:13:56] ^[[39m^[[36m[Intercom] ^[[39mprepareDoorbellService
^[[37m[31/12/2020, 16:13:56] ^[[39m^[[36m[Intercom] ^[[39mconnectListingToDoorbel
^[[37m[31/12/2020, 16:13:56] ^[[39mInitializing platform accessory 'IntercomLock'...
^[[37m[31/12/2020, 16:13:56] ^[[39mInitializing platform accessory 'IntercomBell'...
^[[37m[31/12/2020, 16:13:56] ^[[39m^[[36m[zigbee2mqtt] ^[[39mInitializing zigbee2mqtt platform...
 ^[[37m[31/12/2020, 16:13:56] ^[[39m^[[36m[zigbee2mqtt] ^[[39mConnecting to MQTT server at mqtt://localhost:1883
Setup Payload:
[…]

^[[37m[31/12/2020, 16:13:59] ^[[39mHomebridge is running on port 51521.
^[[37m[31/12/2020, 16:14:00] ^[[39m^[[36m[zigbee2mqtt] ^[[39mConnected to MQTT server
^[[0;37m[31/12/2020, 16:16:48] ^[[0m^[[0;36m[Homebridge UI]^[[0m Changes to config.json saved.

And for v5 I get into a bootloop with this error:

^[[37m[31/12/2020, 16:29:45] ^[[39mHomebridge is running on port 51521.
^[[37m[31/12/2020, 16:29:45] ^[[39m^[[31mRangeError [ERR_BUFFER_OUT_OF_BOUNDS]: Attempt to access memory outside buffer bounds^[[39m^[[31m    at new NodeError (node:internal/errors:278:15)^[[39m
^[[31m    at boundsError (node:internal/buffer:82:11)^[[39m
^[[31m    at Buffer.readUInt8 (node:internal/buffer:250:5)^[[39m
^[[31m    at BufferListStream.BufferList.<computed> [as readUInt8] (/usr/local/lib/node_modules/homebridge-z2m/node_modules/bl/Buff$^[[31m    at Parser._parseVarByteNum (/usr/local/lib/node_modules/homebridge-z2m/node_modules/mqtt-packet/parser.js:552:28)^[[39m
^[[31m    at Parser._parseProperties (/usr/local/lib/node_modules/homebridge-z2m/node_modules/mqtt-packet/parser.js:623:25)^[[39m
^[[31m    at Parser._parseConnack (/usr/local/lib/node_modules/homebridge-z2m/node_modules/mqtt-packet/parser.js:259:31)^[[39m
^[[31m    at Parser._parsePayload (/usr/local/lib/node_modules/homebridge-z2m/node_modules/mqtt-packet/parser.js:94:16)^[[39m
^[[31m    at Parser.parse (/usr/local/lib/node_modules/homebridge-z2m/node_modules/mqtt-packet/parser.js:43:45)^[[39m
^[[31m    at Writable.writable._write (/usr/local/lib/node_modules/homebridge-z2m/node_modules/mqtt/lib/client.js:334:12)^[[39m
^[[37m[31/12/2020, 16:29:45] ^[[39mGot SIGTERM, shutting down Homebridge...
itavero commented 3 years ago

Which MQTT server are you using? I'm running mosquitto v1.6.12.

If you check the logs of zigbee2mqtt, do you see the message to zigbee2mqtt/bridge/devices being published?

There should be an entry starting with something like this:


Zigbee2MQTT:info  2020-12-31 16:35:26: MQTT publish: topic 'zigbee2mqtt/bridge/devices', payload '[{"definition":null,"endpoints":{"1":{"bindings":[],"clusters":{"input":[],"output":[]}},"11":{"bindings":[],"clusters":{"input":[],"output":["ssIasZone"]}},"110":{"bindings":[],"clusters":{"input":[],"output":[]}},"12":{"bindings":[],"clusters":{"input":[],"output":[]}},"13":{"bindings":[],"clusters":{"input":["genOta"],"output":[]}},"2":{"bindings":[],"clusters":{"input":[],"output":[]}},"242":{"bindings":[],"clusters":{"input":[],"output":[]}},"3":{"bindings":[],"clusters":{"input":[],"output":[]}},"4":{"bindings":[],"clusters":{"input":[],"output":[]}},"47":{"bindings":[],"clusters":{"input":[],"output":[]}},"5":{"bindings":[],"clusters":{"input":[],"output":[]}},"6":{"bindings":[],"clusters":{"input":[],"output":[]}},"8":{"bindings":[],"clusters":{"input":[],"output":[]}}},"friendly_name":"Coordinator"```
llnnse commented 3 years ago

I'm running mosquitto 1.5.7 (as this is the newest for the Pi zero) and there isn't even a topic with zigbee2mqtt/bridge/devices.

itavero commented 3 years ago

@tkrtz I might have some time next week to setup a test environment in Docker with an older version of MQTT. If the topic is not there, I think it's more likely that the issue is caused by some incompatibility with the newer versions of zigbee2mqtt and the version of mosquitto you are running.

Raspberry Pi Zero has an ARMv6 instruction set, right? There are newer versions of the eclipse-mosquitto container available on Docker Hub for ARMv6.

llnnse commented 3 years ago

Okay, so installed the newer version of mosquitto (as the have published an ARMv6 build for version 1.6.12) and downgraded nodejs to v12.19.1. Turns out the newest version of z2m had troubles installing while running the newest version of node. So by purging z2m and reinstalling it Iβ€˜ve gotten the /bridge/devices topic πŸŽ‰ All my zigbee devices are displayed in the HomeKit app!

Sorry to cause you troubles. At least I (or maybe we) know now, that using the latest version (or the recommended one) is quite important :)

And a happy new year!

itavero commented 3 years ago

@tkrtz Happy new year! Glad to hear it's working now.

itavero commented 3 years ago

Spend quite some time on #2 (support for remote controls) the last couple of days. I think I'll be able to release that in the next channel as well in the coming weeks.

I still plan to check if I can make the migration from previous versions to this new major version better. Unfortunately that means I have to setup a second Homebridge environment with the old version of the plugin, as I've been using the new major version at home since I published it. I haven't ran into any major issues (yet), except for the migration from the old version (which is why I would want to improve on it if possible).

itavero commented 3 years ago

Just published v1.1.0-beta.0 on the next channel.

This adds support for push buttons (#2) and should improve the "migration" experience when coming from a pre-v1.0.0 version, as it tries to "recycle" existing services.

itavero commented 3 years ago

Closing this issue, as I feel that most "major" refactoring work is done.

If you run into issues with the latest beta, feel free to add a comment here or open up a new issue.

Diederik98 commented 3 years ago

Hi @itavero,

I'm looking forward to test your beta version! However I get an error that the dev branch of zigbee2mqtt is not supported. I'm using the v1.17.0-dev version atm.

Is this something I can fix myself? (The dev branch is more stable for me...)

Thanks!

itavero commented 3 years ago

@Diederik98 Can you send me the error message? Then I can do some testing.

I think I need to make the checkZigbee2MqttVersion less strict.

Apparently semver.lt considers not having a prerelease greater than having one (which is correct, but is not how zigbee2mqtt is versioned). This is the correct behavior according to the Semantic Versioning spec.

Diederik98 commented 3 years ago

@itavero Here you go Schermafbeelding 2021-01-12 om 09 26 06

Thanks!

itavero commented 3 years ago

Easy solution is probably to just ignore the -dev suffix if it's present. I'll see if I can add that as a quick workaround tonight.

itavero commented 3 years ago

@Diederik98 As you might have noticed I did not get around to it yet, unfortunately. Luckily it's the weekend now, so it should be easier to find a moment to fix this. I'll let you know once the fix is available.

itavero commented 3 years ago

@Diederik98 The fix is available in v1.1.0-beta.1, available on the next channel (so npm i homebridge-z2m@next should fix things for you) πŸ˜‰

Diederik98 commented 3 years ago

@itavero Thanks! The small ikea remotes (E1743) are nice to have for manual toggles. Nice work! πŸŽ‰ I'm glad homekit shows which button has been pressed for the automations πŸ˜…

itavero commented 3 years ago

Indeed. You can also see the mapping in the Homebridge logs, by the way.