[Question] Strategy for manufacturer-specific attributes in standard clusters?

[felixstorm]

As already mentioned in my last pull request, I am implementing support for some ubisys devices at the moment. To e.g. calibrate their shutter controller to the run-time from fully open to fully closed, they use quite a lot of manufacturer-specific attributes within standard clusters (e.g. attributes 0x1000 and up in closuresWindowCovering).
Currently, I have these implemented as numeric values in code but thought it might be nicer to have them somewhere in a central location, but could not really figure out an obvious way to do it. One idea is to implement them in additional clusters that have numerical ids with the manufacturer id as upper bytes, e.g. for ubisys (manufacturer id is 0x10f2) and the cluster closuresWindowCovering (258 = 0x102) it would be 0x10f20102 = 284295426. But even this would mean that I will need to touch quite some central code inside zigbee-shepherd and zigbee2mqtt to implement it correctly...
Do you have any opinion or suggestions for me or should I rather stick with the way I have it implemented currently?

Current implementation in my code: https://github.com/felixstorm/zigbee2mqtt/blob/f41f754135f918a9b60ad14501311e48f5ae1114/lib/extension/ubisys.js#L166
ubisys documentation (if interested): https://www.ubisys.de/wp-content/uploads/ubisys-j1-technical-reference.pdf

Thanks,
Felix

[Koenkk]

Also the answer to #5, I would just add them zcl-id/zcl-packet and prefix it with ubysis.

The aim of zigbee-herdsman is more on "real world usability" instead of the "perfectly follows ZCL specification" part.

[felixstorm]

Ok, I'll check that - but are you sure that adding a new cluster with the same numeric id as an existing one (see #5 ) is a good idea? I would be a bit worried when it comes to reverse lookup on incoming Zigbee packages...

[Koenkk]

In this case, the cluster should be renamed, e.g. manuSpecificPhilipsUbisys, as id's cannot be duplicated.

[felixstorm]

Ok, but then we will be in trouble again as soon as we hit duplicate attribute ids (ubisys uses e.g. 0 and 1).

I would rather go for clearer definitions, and I only need the additional clusters and attributes for configuring the devices inside the ubisys extension where I anyway have my own read and write attribute functions. Would it therefore be ok for you if I define them as suggested above with the manufacturer id prefixed and only parse them inside my own code without touching anything general?

[Koenkk]

My biggest struggle with https://github.com/felixstorm/zigbee2mqtt/blob/f41f754135f918a9b60ad14501311e48f5ae1114/lib/extension/ubisys.js#L166 is that it is a lot of code (= extra maintenance) while adding very less feature wise (only for ubysis users, which you are the only reported one now).

[Koenkk]

As I'm currently completely refactoring zigbee-herdsman anyway, I'm thinking it maybe nice to have 'conditional' manuf specific clusters, e.g. only when the manufcode matches. This would solve your problem and keep all parsing inside zigbee-herdsman.

[felixstorm]

Great, sounds very good to me! If you find a way to also take care of manufacturer-specific properties inside standard clusters that would sure be helpful as well (so we won't get in trouble in case of duplicate attribute ids either).

I'll then go ahead with my personal implementation for now and will update it to the new spec as soon as it is available.

Thanks,
Felix

[felixstorm]

Ok, I have done some more work and implemented everything inside the ubisys extension: https://github.com/felixstorm/zigbee2mqtt/blob/master/lib/extension/ubisys.js

My JavaScript knowledge really is a bit rusty and I need to get used to promises, but I like it really a lot better than Python (used to play around with https://github.com/zigpy/zigpy also) 😉

If you have any comments or suggestions, please feel free to do so. I need to do some more testing (especially binding an input module C4 to dimmers or shutter controllers) and might also implement the metering part and then I will do a PR.

By the way, I can really recommend the ubisys devices. They are not cheap, but small, work very reliably, are very well documented (e.g. Shutter Control J1 Technical Reference) and provide a lot of different functions directly on the Zigbee level. I have around 15 of them in my house for 2 years now and I am perfectly satisfied.

[Koenkk]

@felixstorm great! Maybe I misunderstood but as mentioned in #4 (comment) I cannot merge this extensions.

• Either wait for [Question] Strategy for manufacturer-specific attributes in standard clusters? #4 (comment)
• Or as a temp workaround: [Question] Strategy for manufacturer-specific attributes in standard clusters? #4 (comment)

[felixstorm]

Sorry - this really seems to be a misunderstanding. I thought it would be helpful to keep everything self-contained inside ubisys.js? From my side, there is currently no need anymore to have the ubisys manufacturer specific clusters or attributes inside zigbee-herdsman since they are only needed to configure the devices and this is now all inside ubisys.js.
Or do you think that ubisys.js in general is too much code to take into zigbee2mqtt? But I hardly see another way to configure the devices. Or is there any other interface I could use to communicate with zigbee2mqtt on a low level so I could put the ubisys configuration code into a separate project?

[Koenkk]

All configuration code can be put here: https://github.com/Koenkk/zigbee-shepherd-converters/blob/master/devices.js#L571

[felixstorm]

Maybe that was not clear, but the whole stuff inside ubisys.js is for one-time-configuration.
An example: the shutter controller needs to know how many seconds the blinds take to go from fully open to fully closed and also for the other way around (which sometimes differs a bit) in order to be able to go to a specific position, e.g. to close the blinds 80 percent. Therefore there is a one-time calibration procedure that can be controlled by setting specific attributes, moving up and down in a predefined order and in the end setting the attributes back to normal.
Another example: The dimmer can work with spring loaded buttons or with a normal rocker switch and can potentially have separate up and down buttons or just one - this can be configured by means of attributes in the manufacturer-specific cluster...

[Koenkk]

@felixstorm the two examples you mention can be done perfectly fine in the toZigbee converters, this seems similar to me as e.g. setting a sensor sensitivity (https://github.com/Koenkk/zigbee-shepherd-converters/blob/master/converters/toZigbee.js#L903). Note that you can also very easily address non-standard clusters there (just put the hex number as show in the link).

[felixstorm]

@Koenkk that was exactly the route I had taken in the first place, but then did not find a way to insert the necessary delays to wait for a complete run up or down (60-90 seconds) during shutter calibration. Therefore I had then switched to implementing an extension instead.

Anyhow I am fine for the ubisys extension to not be merged into the mainline currently if there apparently is no demand for it besides myself (I can simply keep and maintain it in my repo for now).

As support for ubisys devices in general does not depend on the extension, I will test it a bit more in my environment and then (probably in a few weeks when I am back from vacation) open a PR for it excluding the extension.
And in case other people should show interest for the calibration stuff in the future, then just drop me a note and we can then figure out what the best way is to implement it.

Thanks again for spending your time and effort on creating and maintaining this great ZigBee solution and feel free to close this issue if there is nothing more to add from your side.

[Koenkk]

@felixstorm could you provide me a bit more context on how such a calibration is done? We can look into the possibilities of adapting zigbee-shepherd-converters.

[felixstorm]

@Koenkk basic calibration sequence is as follows:

• send command "open"
• wait 60 seconds (or better: regularly read (or monitor (previously configured) reporting) attribute to get notified when the motor stops at the top)
• write some (manufacturer specific) attributes
• send command "down"
• wait 5 seconds
• send command "up"
• again wait 10 secs or until motor has stopped at the top again
• send command "down"
• again wait 60 secs or until motor has stopped at the bottom
• send command "up"
• again wait 60 secs or until motor has stopped at the top
• write some (manufacturer specific) attributes

Just some ideas:

• If the converters in toZigbee.js would somehow get access to the zigbee-object then calibration could be handled in there.
• A more advanced version that would probably make the code cleaner might be if the converters could return - as an alternative to the current array of result objects - an array of promises (or similar) which would then be executed sequentially by DevicePublish.onMQTTMessage (but they would still need the zigbee-object).

[Koenkk]

Option 2 sounds great, I think we can also do it without the zigbee object by passing the result of the previous action?

[felixstorm]

Sorry for the delay, just came back from vacation.

Option 2 without the zigbee object could probably work if the array returned from the converters is allowed to contain both objects to publish to zigbee as well as promises which would then all be processed sequentially so that we can write an attribute, wait, send a command, wait etc.

[Koenkk]

@felixstorm zigbee-herdsman has now landed in the zigbee2mqtt dev branch, you should be able add the calibration to https://github.com/Koenkk/zigbee-shepherd-converters/blob/zigbee-herdsman-converters/converters/toZigbee.js.

Could you also make a PR with the manufacturer specific clusters? I will then modify this further.

[felixstorm]

@Koenkk Cool, looks good, thanks!

I am a bit busy at the moment, so it might take a few days until I get to it, but I will let you know as soon as I have added the calibration.

[felixstorm]

@Koenkk I did a first implementation:
felixstorm@bd0a8e0
felixstorm/zigbee-herdsman-converters@f306327
Does this look ok for you or would you like me to make some changes (console.log is just in there as a placeholder, it's definitely not meant to go into production 😉)?

There are some open questions where I could use some advice from you:

• Cluster manuSpecificUbisys: I have just added it to the list of clusters, but it has the same id as manuSpecificPhilips, so it will probably not work on incoming stuff from Zigbee. Is this ok for now and you will modify this further or would you like me to make some changes to it?
• Logging: The calibration can be a rather lengthy process, so IMHO it would be helpful for the user to get some feedback in between. Maybe passing a logger along as part of meta would be a way to solve this - what do you think?
• Configuration output when reading from Zigbee: I am not really sure if it is helpful to return the read configuration data as a normal state since when being used with Home Assistant, this information is being retained and passed to HA again and again. Also, it's not really a state - again a logger might be a way to solve it?

[Koenkk]

1. Yes, just make this PR and I will continue with that.
2. OK with that
3. Can't we abstract this to a higher level? e.g. it might be useful to the user at which point it has been calibrated.

[felixstorm]

1. Thanks!
2. Thanks again!
3. To be honest, I do not think that reading the configuration parameters is of much use besides troubleshooting or verifying that everything is configured correctly. It might be an idea to have some sort of repository similar to state.json were various (configuration) parameters from all devices can be saved for reference (potentially with a timestamp like LastRead or so), but I did not deem it important enough at the moment to justify the effort...

[Koenkk]

3. I agree, just remove the convertGet for now.

[felixstorm]

3. Couldn't we just leave convertGet in there and use a logger to output the results? Then there would be at least some possibility to read the attributes in order to verify that the calibration has generally succeeded for a specific device. I would have no problem streamlineing the logger output for readability a bit more than just raw JSON...

[Koenkk]

Also ok with that!

[felixstorm]

Ok, thanks - then I'll wait for the logger to become part of meta and then file a PR for the ubisys stuff, or would you like me to also file a PR for adding a logger to meta?

[Koenkk]

@felixstorm you can also file a pr for that, it shouldn't be more than changing https://github.com/Koenkk/zigbee2mqtt/blob/dev/lib/extension/entityPublish.js#L137

const meta = {
                endpoint_name: topic.postfix,
                options,
                message: json,
            };

to

const meta = {
                endpoint_name: topic.postfix,
                options,
                message: json,
                logger,
            };

[Koenkk]

Necessary changes are applied so I assume that this can be closed now.

[felixstorm]

Thanks! Will open a PR for it now...