Koenkk / zigbee2mqtt

Zigbee šŸ to MQTT bridge šŸŒ‰, get rid of your proprietary Zigbee bridges šŸ”Ø
https://www.zigbee2mqtt.io
GNU General Public License v3.0
11.95k stars 1.67k forks source link

[feature] expand extension framework to make it easier to use #3297

Closed sjorge closed 4 years ago

sjorge commented 4 years ago

Some discussion in #3229 led to the need for an easier way to add extension that would not require them to live in z2m repo.

Koenkk:

I think for these use cases it would be better to allow users to write own extensions. E.g. create an extension directory in the data directory which zigbee2mqtt will automatically load. This will also allow users to write automations directly in Zigbee2mqtt.

sjorge:

Excellent, I'll clean this up some more then and it can be a example module.

Perhaps we can move some of the on/off toggles to load extensions to a list where we list what we want to load. (Keeping the old behavior for ha and availability) That would remove the need to add a new field for each extension to the internal configuration file definition.

extensions:
 - name: device_initialize
   configuration:
     config: option
     another: option here too
 - device_availability
 - homekit

initialize for example can then be mapped to some predefined filename e.g.

Not sure how we can easily allow and extension to add per device config though šŸ¤”

sjorge commented 4 years ago

Possible extension ideas:

Probably some sort of 'event' system is needed to react to those events somehow. Although I think most bits are here already.

sjorge commented 4 years ago

Some more food for thought,

sjorge commented 4 years ago

That can probably also lead to a refactor of entityPublish.js where there are some higher level methods expose via the api to do stuff like get and set that take a json payload of key-value pairs.

The existing entityPublish.js can just pass through most stuff from mqtt, but then things like DevicePull can simple call get with the list of keys configured to poll the device. That would eliminate a lot of the code duplication that the current ugly hacked one has.

Koenkk commented 4 years ago

I've implemented the extension loading. Extensions that are put in data/extension will automatically be loaded.

E.g. by putting https://gist.github.com/Koenkk/16f00544fb7e46f6681495363dfad078 in data/extension/example.js.

sjorge commented 4 years ago

Is there an easy way to get to the logger? I tried with

const logger = require('../util/logger');

and

const logger = require('../../util/logger');

But that just makes z2m fail to start, perphaps also make errors in extensions non fatal

sjorge commented 4 years ago

So I was adapting the silly deviceInitialize extension to the new framework and ran into some issues.

For the this.publishEntityState() call I am getting this(now I might be using the call wrong):

(node:39577) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'constructor' of undefined
    at Zigbee.resolveEntity (/opt/zigbee2mqtt/lib/zigbee.js:148:17)
    at Controller.publishEntityState (/opt/zigbee2mqtt/lib/controller.js:246:44)
    at Initializer.onDeviceAnnounce (/opt/zigbee2mqtt/data/extension/initializer.js:40:14)
    at Initializer.onZigbeeEvent (/opt/zigbee2mqtt/data/extension/initializer.js:45:18)
    at Controller.callExtensionMethod (/opt/zigbee2mqtt/lib/controller.js:348:44)
(node:39577) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:39577) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

I see other extensions in lib/ that use this call super() inside the constructor, but adding that didn't work... because we don't inherit from BaseExtension I think?

Edit: working restore.js example: https://gist.github.com/sjorge/5018d939402f35b57a1f9b9849f19d91

Koenkk commented 4 years ago
sjorge commented 4 years ago

I update the above a bit with another issue, I was initially confusing with a bad constructor, as I just had the file open in vim and the .swp was causing z2m to also fail.

We should perhaps also have a way for an extension to extend the schema: https://github.com/Koenkk/zigbee2mqtt/blob/d22e73ebb41f7fc0a72dae08890e0803f16c864b/lib/util/settings.js#L125

I now just glob stuff in under the initial_state key for a device and it works, but if we would somehow validate the schema in the future it might break.

Koenkk commented 4 years ago

await this.publishEntityState(resolvedEntity.ieeeAddr, state); should be await this.publishEntityState(resolvedEntity.device.ieeeAddr, state); (otherwise it will be null which explains the error).

sjorge commented 4 years ago

Sadly the call did not do what I though xD... I was hoping it would also send the message to the device.

Enough messing about for today, but let me summarize the above few comments:

I guess for the last point we could move some of the logic from entityPublish.js into controller ? There may already be a way though, I'll need to take a closer look at homeassistant.js as I think it also publishes states to the device. Looks like HA does this via mqtt messages so homeassistant.js doesn't deal with it.

Koenkk commented 4 years ago

To send commands to device this can be used (its a bit hacky but good enough).


this.mqtt.emit('message', {topic: "zigbee2mqtt/FRIENDLY_NAME/set`, message: JSON.stringify({state: "ON"})}`
sjorge commented 4 years ago

I'll try that tomorrow, just need to find a way to get the zigbee2mqtt part for the topic.

I also opened https://github.com/Koenkk/zigbee2mqtt/pull/3409 to fix the other crashing issue, the one that shouldn't be fatal.

sjorge commented 4 years ago

Is there an easy way to get the settings object to fetch the topic: ${settings.get().mqtt.base_topic}

I guess we need to pass it in like settings, I'll create a PR. Feel free to close that one if there is a better way. PR: #3410

I'll see if I can get the basic thing working, the mqtt.emit might work for basic cases and I want to stub out some fancy example extensions first using that. But I think in the long run it would be nice to have a few more 'premade' functions available.

Should I/we create a repo somewhere that hold 'best effort' extensions that people can plug and use by just downloading the .js from the repo?

sjorge commented 4 years ago

Alright I have a minimal working 'restore' plugging that can set state on deviceAnnouncement!

https://gist.github.com/sjorge/5018d939402f35b57a1f9b9849f19d91, I'll look into some of the mentioned things above over the weekend:

sjorge commented 4 years ago

@Koenkk I have an idea on how to extend the schema... An optional function for the extensions onSettingsSchema() that then additional schema data to merge... although I think that might be a chicken-egg issue as settings is already loaded and passes to the extension right?

Koenkk commented 4 years ago

@sjorge settings can already be added to configuration.yaml without problems. Not sure if adding the schema validation is worth it, it complicates things and does not really add much value?

sjorge commented 4 years ago

I did notice the validate function was never called, but I guess it would be nice to have it work though?

My example extension does add stuff to device_options or the device config.

Koenkk commented 4 years ago

I would be a nice too have indeed, but I currently don't see how to easily implement it.

sjorge commented 4 years ago

I'm not good enough with node to know if something like this could work

If the in the extension construct set this.controller = controller, then access the other stuff via this.controller.getLogger(), this.controller.getSettings(), ...

But not sure how all the ordering would work in node, this works okish in python.

sjorge commented 4 years ago

So I was already looking at the poll extension and I need a way to do a read that will wait for a return value... is that even possible?

Like read the genOnOff, then return once it is read (ideally with the value)... but while thats done also publish the mqtt message without having to deal with that in the extension.

Koenkk commented 4 years ago

This should work: await endpoint.read('genOnOff', ['onOff']);, bulb will respond where the message will be picked up by receive.js which will publish it to MQTT

sjorge commented 4 years ago

Been a bit busy with other stuff, I'll try to pick this back up soon.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.