anthonywebb / homebridge-cbus

CBus plugin for homebridge
MIT License
35 stars 20 forks source link

Adds Enable Control Accessory #111

Open JasonY00 opened 3 years ago

JasonY00 commented 3 years ago

Addition of Enable Control Accessory to support C-Bus Enable Control Application

JasonY00 commented 3 years ago

Hi Anthony,

would you mind adding this enable control accessory to the repository?

It facilitates the enable control application 202 in c-bus and presents it as a switch service in the same way as the trigger control application 203.

The current work around to having enable control is to use the lighting application 56 for enable control. This is not ideal particularly if there are already a lot of group addresses on application 56.

The addition of the enable control accessory adheres more to the the c-bus framework of a separate enable control application to perform this function.

This accessory has been achieved by making changes to the trigger application accessory already in the plugin. This pull request has been requested as a response to https://github.com/anthonywebb/homebridge-cbus/issues/110 opened today.

Kind regards

Jason

DaveGraham123456 commented 3 years ago

Hi there, has this been merged yet?

I think it will let me control my Airtopia Aircon that is C-Bus integrated via Modbus

JasonY00 commented 3 years ago

Hi Dave,

You can try it via my branch of the plugin to see if it suits your need. Just copy the files over your current c-bus plugin.

If its not what you need, then just reinstall the plugin.

It's up the Anthony, the author of the c-bus plugin, whether he wants to incorporate the pull request for enable control or not.

Cheers

Jason

barrya24 commented 3 years ago

Hi @JasonY00 Thanks for adding this. I have copied the code from your PR as I use a number of enable switches and it would be useful to be able to turn these on and off from my iphone. I have a question though. The "set" is working fine and setting the enable application group ON. I have confirmed this in Toolkit, and I see the 200 OK message below. However when the subsequent response is received (also below), it comes through as a code 702, so I am getting "Assertion failed: message.level must be defined"

May 08 13:48:37 homebridge.local homebridge[6008]: 2021-05-08T01:48:37.061Z cbus:client sent command '[267] enable set //HOME/254/203/3 255' May 08 13:48:37 homebridge.local homebridge[6008]: 2021-05-08T01:48:37.064Z cbus:client rx response { commandId: 267, code: 200, matched: false, processed: true, type: 'response', raw: '[267] 200 OK: //HOME/254/203/3' } May 08 13:48:37 homebridge.local homebridge[6008]: 2021-05-08T01:48:37.064Z cbus:client matched request '[267] enable set //HOME/254/203/3 255' with response '[267] 200 OK: //HOME/254/203/3' (0 pending requests) May 08 13:48:37 homebridge.local homebridge[6008]: 2021-05-08T01:48:37.066Z cbus:client rx event { time: '20210508-134837', code: 735, processed: false, message: '//HOME/254 905b17b0-8844-1039-aa06-bd8bbb10dd33 cc365 sent cmd (fastpci): May 08 13:48:37 homebridge.local homebridge[6008]: 2021-05-08T01:48:37.205Z cbus:client rx event { time: '20210508-134837', code: 765, processed: false, message: '//HOME/254 905b17b0-8844-1039-aa06-bd8bbb10dd33 got packet confirm: h.', t May 08 13:48:37 homebridge.local homebridge[6008]: 2021-05-08T01:48:37.205Z cbus:client rx event { time: '20210508-134837', code: 734, processed: false, message: '//HOME/254 905b17b0-8844-1039-aa06-bd8bbb10dd33 response: \05CB000203FF p May 08 13:48:37 homebridge.local homebridge[6008]: 2021-05-08T01:48:37.206Z cbus:client rx event { time: '20210508-134837', code: 702, processed: true, netId: //HOME/254/203/3, application: 'enable', value: 255, sourceUnit: 1, sessionId: May 08 13:48:37 homebridge.local homebridge[6008]: Assertion failed: message.level must be defined

Looking through the code, this seems to be due to the condition in CBusPlatform.prototype._processEvent which calls accessory.processClientData when the code is 730 or 702. I'm not sure what those codes represent.

Given this is similar code to triggers, I have compared this to my trigger events and they are behaving the same way. However I had not noticed this before since the trigger event is typically momentary and then the trigger action is cleared. In this case for enable application though, I would expect the enable accessory to remain 'lit' on my iphone once it is turned on, and then send a corresponding action:0 when the accessory is turned off (like a switch). Have I understood this correctly?

JasonY00 commented 3 years ago

Hi Barrya24,

Thanks for trying out the enable accessory. You are correct that it is generally just a rewrite of the Trigger Accessory as both work the same way in CBus with a slightly different command.

With regards to C-Gate responses 702 and 734, these are informational and indicate the following (from the C-Gate Manual):

702 - Application information event - the text of this event varies with the particular C-Bus application that is using it.

and

734 - Response line: (from a Command Event)

Looking at your above event log we have a set command which is successful (C-Gate 200), Command was sent (C-Gate 735), the packet was confirmed (C-Gate 765), the response was sent (C-Gate 734) and the information was returned (C-Gate 702).

The condition in CBusPlatform.prototype._processEvent is there to facilitate the processing and updating of broadcast "Measurement" Application events from things like temperature sensors. Perhaps this is the cause of the problem and another condition is warranted to pass through the trigger/enable information...

I will have to spark up my laptop to get my head around and test the code (as it's been a little while), but you are correct in saying that you should be able to see the enable accessory state on your phone. I will look into this as the Trigger Accessory is impacted the same way. I think that the enable state should remain "lit" and display the current "level". I am unsure if this can be displayed as a 0-255 level or at all on the home app on iOS as it is defined as a switch which by definition is ON/OFF. I imagine that the Enable "switch" should remain "ON" on iOS until it is turned "OFF" or another enable level for the save accessory is selected.

The trigger accessory in homebridge-cbus has a 500ms timeout that then resets the value to "0" or "OFF" automatically (i.e. a bell press). I will need to retest the enable accessory without this option so that it stays "ON" as it should. (An oversight of mine). This may fix the "lit" icon problem.

The error relating to the "Assertion failed: message.level must be defined" could be due to the "value" of the message not being equal 0 or 1 (On/Off for the iOS switch definitaion) but an integer which is the action selector value (0-255) for the Enable/Trigger control. Perhaps a test is needed within the accessory definition that says "if the action selector value is not equal to the enable accessory defined value (in config.json) then set the value to 0 (turn the enable control tile in iOS off). This, however, may cause it's own problems....thinking out loud!

Something to ponder...

Cheers and thanks for highlighting this behavior to me.

Jason

barrya24 commented 3 years ago

Thanks for the extra info Jason. I'm not a java developer, but have experimented with a few debug statements in the code based on your feedback and confirmed that the reason the assertion is appearing is due to the message.level having the value of 'undefined'. This makes sense for the trigger and enable applications as the message.value variable is used instead. I modified my local enable.accessory.js to have the following code and this resolved the assertion error

CBusEnableAccessory.prototype.processClientData = function (err, message) { if (!err) { console.assert(typeof message.value !== undefined, message.value must be defined); } };

I think I have found the 500ms timeout that you mentioned in SetEnable. I changed it to 5000 as an experiment and the tile was turned off after 5 seconds, so that makes me think this is the code that needs to be changed. My question is whether there needs to be an extra condition in SetEnable to cater for the enable switch being turned off (similar to the Switch accessory). If you agree I can try and have a go at cloning the code from switch accessory.

When Homekit is first started and the levels (or values as the case may be) are returned for each accessory, the enable tile is not turned ON. I haven't been able to find this code. Does this need a GetOn equivalent called GetEnable? I have not been able to find where this would get called from, but assume it must be in an initialise procedure somewhere?

Sorry for so many questions. I am learning a lot going through the code and your feedback has been really helpful.

JasonY00 commented 3 years ago

Hi Barrya24,

Great work re the value/message error message. The timer that you mentioned is the correct one. It automatically turns the trigger off after half a second. Now in Enable, this is not what we really want as the enable should stay on until we choose to "disable" something so the timer is not really needed or maybe the duration can be defined in the accessory like in the switch accessory that has an active duration defined.

The catch is that Enable Control (and trigger control for that matter) in c-gate is not a binary state like a switch. Both can take any value from 0-255 and this should possibly be reflected in the code. I use trigger control (with different action selectors) in c-gate for IR control where the trigger values of a Group Address are mapped to an IR code from the remote control of the device via the Clipsal CIRCA Software. I then have multiple definitions in Homebridge for triggering this. The config below changes my amp between HDMI inputs.

 {"type": "trigger", "application": 202, "id": 20, "action": 25, "name": "Yamaha 4KBluray"},

 {"type": "trigger", "application": 202, "id": 20, "action": 28,  "name": "Yamaha 3DBluray"},

 {"type": "trigger", "application": 202, "id": 20, "action": 30,  "name": "Yamaha Foxtel"},

 {"type": "trigger", "application": 202, "id": 20, "action": 33,  "name": "Yamaha AppleTV"},

After the 500ms delay, Homekit displays the tiles for these as OFF (assigns a value of 0 to the trigger value in homekit which turns the iOS tile) as expected. Note that the action selector value in c-gate is unchanged by this which is technically not correct.

A switch in Homekit is defined as either ON or OFF and can only take on the values of 0 or 1.

So the logic is that if the value for the c-gate action selector is non-zero (1-255), it is assigned a value of 1in homekit and is ON. Otherwise it is 0 or OFF. This is how the switch represents the trigger/enable control.

As mentioned, C-Gate has 0-255 values for trigger/enable but homekit uses 0 or 1 for the switch. So perhaps a test may be needed in a "Get" function in the enable control accessory code that compares the value requested from c-gate with the defined "action selector" value in config.json and then portrays it on the iOS tile as OFF if the values are not equal and ON if they are.

You will have noticed that you can turn the trigger/enable ON/OFF from the iOS tile with the current code, so that is already done. If the option above for comparing the values of the action selector was implemented in the enable control accessory, I am unsure that if another "enable" accessory with the same group address but different action selector is selected (like in my trigger example above) whether it would extinguish the other enable tile in iOS. (Sorry, that's a difficult sentence to digest)

I will think about it but by all means if you want to have a go then then go for it! This is how I started with this whole homebridge-cbus thing and programming in node.js is certainly something that I am not a guru at!

Cheers

Jason

barrya24 commented 3 years ago

Hi Jason

That makes perfect sense. I have changed the processClientData and this now sets the tile active when the value matches the actionSelector in the config.json.

CBusEnableAccessory.prototype.processClientData = function (err, message) {
        if (!err) {
                console.assert(typeof message.value !== `undefined`, `message.value must be defined`);
                const level = message.value;

                this.service.getCharacteristic(Characteristic.On).setValue((level === this.action) ? 1 : 0, undefined, `event`);
        }
};

However when I first start Homekit and it does a 'get' for all accessories to determine state, the Rx Response (code 300) is not being parsed correctly as it turns on all my enable accessory tiles, even though some are turned off in cgate. E.g. Below has Enable GA 2 set to OFF and Enable GA 3 set to ON. However the tiles for both are switched "ON" at startup.

May 15 02:22:50 homebridge.local homebridge[26004]: 2021-05-14T14:22:50.648Z cbus:client sent command '[249] get //HOME/254/203/2 level # getEnable'
May 15 02:22:50 homebridge.local homebridge[26004]: 2021-05-14T14:22:50.648Z cbus:client sent command '[250] get //HOME/254/203/3 level # getEnable'
...
...
May 15 02:22:50 homebridge.local homebridge[26004]: 2021-05-14T14:22:50.702Z cbus:client rx response { commandId: 249, code: 300, matched: false, processed: true, netId: //HOME/254/203/2, level: 0, type: 'response', raw: '[249] 300 //HOM
May 15 02:22:50 homebridge.local homebridge[26004]: 2021-05-14T14:22:50.702Z cbus:client matched request '[249] get //HOME/254/203/2 level # getEnable' with response '[249] 300 //HOME/254/203/2: level=0' (4 pending requests)
May 15 02:22:50 homebridge.local homebridge[26004]: 2021-05-14T14:22:50.702Z cbus:client rx response { commandId: 250, code: 300, matched: false, processed: true, netId: //HOME/254/203/3, level: 100, type: 'response', raw: '[250] 300 //H
May 15 02:22:50 homebridge.local homebridge[26004]: 2021-05-14T14:22:50.702Z cbus:client matched request '[250] get //HOME/254/203/3 level # getEnable' with response '[250] 300 //HOME/254/203/3: level=255' (3 pending requests)

The other accessories are setting the correct state at startup. Any subsequent enable changes set the tile state correctly. Below is the getEnable that I have added if you think anything looks incorrect in that. I reverted the isOn boolean back to checking 255 in case the this.action variable was not working, but that didn't fix the issue either.

CBusEnableAccessory.prototype.getEnable = function (callback) {
        this.client.receiveLevel(this.netId, message => {
                this.isOn = message.value = 255;    //TODO change back to      this.action
                this._log(FILE_ID, `getEnable`, `status = '${this.isOn ? `on` : `off`}'`);
                callback(false, this.isOn ? 1 : 0);
        }, `getEnable`);
};

I tried adding a second config.json entry for the same enable GA but with a different ActionSelector. As you predicted it did not work correctly.

JasonY00 commented 3 years ago

Hi Barrya24,

that's looking very promising! Thanks for taking this on. I think the problem with the comparison of whether the iOS tile should be lit based on the comparison of the Enable Control Actions Selector value in the function for the enable control accessory is that when the data comes back from c-gate it is converted to a binary 0:1 value for the comparison of ON/OFF.

Thats what the callback(false, this.isOn ? 1 : 0); is all about.

We need to get the 0-255 level from c-gate (bypass any 0-255 to 0-100 or 0-1) conversion in the code, if its there, and compare the raw "message.value" returned from c-gate with the value defined in the config.json file. If it is a match then the tile (accessory) can be set to "1" or ON and if it's not a match then the accessory can be set to "0" or OFF.

So the outcome is that the iOS tile just indicates whether the appropriate action selector defined in the config.json file is selected. This then provides the option to have up to 255 enable levels for a single group address which is how c-bus works.

The only thing to think about is what value to set the enable control to when you manually turn a tile off in iOS. For example, if you had say three tiles in iOS with the same c-gate group address but three action selector values of 0, 127 and 255 defined in the config.json file, if you selected the 127 tile, then it would be set to ON, the 0 tile would be set to OFF and 255 tiles would remain OFF. If you then "turned off" the 127 tile in iOS, what value should it take? 0 perhaps? If so, then the 0 tile would light up and the 127 tile should go out and the 255 tile should stay off. This would then truly reflect what is happening in c-gate.

This code could then be applied to the trigger accessory (including the 500ms timeout).

Currently, in the trigger accessory, if you select a trigger value of say 127 in iOS (that is assign 127 to the action selector for a group address in c-gate), this value is assigned to the group address in c-gate then 500ms later the tile turns OFF in iOS, BUT, the value of 127 is not updated in c-gate. The value is still 127 and not 0. At this point, iOS (homebridge) and c-gate are out of sync. Should we do something about this?

Perhaps anyone else reading this may have an opinion? Let us know if you do please...

Got to go now.

Cheers

Jason

barrya24 commented 3 years ago

Hi Jason

I have this working now. I discovered that the message.value was undefined and that the message.level was in percent as you predicted. I parse the message.raw and extract the rawlevel and then use that to compare to the config.json actionSelector value.

CBusEnableAccessory.prototype.getEnable = function (callback) {
        this.client.receiveLevel(this.netId, message => {
                const RESPONSE_REGEX = /^.*: level=(\d{0,3}).*$/;
                let parsed = message.raw.match(RESPONSE_REGEX);
                let rawlevel = parseInt(parsed[1], 10);

                this.isOn = (rawlevel === this.action);
                this._log(FILE_ID, `getEnable`, `status = '${this.isOn ? `on` : `off`}'`);
                callback(false, this.isOn ? 1 : 0);
        }, `getEnable`);
};

It may have been cleaner to create message.rawlevel in the _parseResponse(line) function before it converts it to a percent rather than re-parsing the message.

Note that defining multiple enable accessories in the config.json for the same GroupAddress but with different ActionSelectors does not work 100%. The command is sent to CGate for each tile press, but only the tile state of the latest defined accessory in config.json for that GroupAddress is updated. I suspect this is because the array that is built up for the accessories, overwrites the earlier entries for that same NetId.

This code could then be applied to the trigger accessory (including the 500ms timeout).

This may be why the trigger code was treated like a bellpress with the 500ms timeout, so that multiple actions on the same trigger groupaddress did not cause confusion with incorrect tiles staying turned on.

It's taken me a while but I have learnt a lot. Thanks for all your help.

JasonY00 commented 3 years ago

Wow! That's coming along nicely.

I suspect that if you navigate away from the page (room) in the Home app in iOS that has the enable control accessory and then navigate back to it (or close and reopen the app, then all of the enable tiles will refresh as expected and all be be well again as the app does a refresh request (get everything relevant from c-gate) and all accessory states are updated in iOS.

Although this does not "solve" the problem it points to a possibility that there may (or may not) be an option to issue a "get" command within the enable accessory code that refreshes all enable accessories defined in the config.json file. That is, it forces c-gate to send a status for every enable device to homebridge and refresh all tiles. This would only create a small amount of overhead, but would be the final piece in the enable control puzzle.

I will look into the c-gate manual to see if there is enough granularity to issue a "get all enable-control group-address action-selector status' only" command that can be issued (and supported by the homebridge-cbus plugin) that will fix this anomaly.

Cheers

Jason