tonesto7 / homebridge-hubitat-tonesto7

Hubitat Homebridge Plugin
108 stars 34 forks source link

[Device Support] (BUG) Hampton Bay Zigbee Fan Controller - Incorrect Speed Enumeration #206

Closed Modem-Tones closed 7 months ago

Modem-Tones commented 1 year ago

Description

Homebridge plugin converts HomeKit 4-speed fan speed percentages according to the following lookup table:

medium-low : 25,
medium : 50,
medium-high : 75,
high : 100

This lookup table is inconsistent with the built in Hampton Bay Zigbee Fan Controller lookup which instead uses the following:

low : 25,
medium-low : 50,
medium : 75,
medium-high : 100,
high : 100

The Homebridge plugin OR the Hubitat app should surface a configuration parameter to control this lookup.

Collect Device Debug Data

Gather the following information before submitting an issue

Here is a sample where we set 75% from HomeKit. This gets converted by the Homebridge plugin into medium-high. Since Hubitat device driver treats medium-high and high both as 100%, you see the update to 100% pushed back to Homebridge (which causes the jumpy UX behavior).

[app:8](http://192.168.1.6/logs#)2023-10-16 07:34:23.207 PM[debug](http://192.168.1.6/logs#)Homebridge (v2.9.2) | Send to plugin Completed | Process Time: (27ms)
[app:8](http://192.168.1.6/logs#)2023-10-16 07:34:23.204 PM[debug](http://192.168.1.6/logs#)Homebridge (v2.9.2) | asyncHttpCmdResp | Src: EventUpdate | Resp: {"evtSource":"Homebridge_Hubitat-v2_8","evtType":"attrUpdStatus","evtDevice":"Dining Room Fan","evtAttr":"level","evtStatus":"Failed"} | Status: 200 | Data: [execDt:1697510063179, src:EventUpdate, evtLog:true]
[app:8](http://192.168.1.6/logs#)2023-10-16 07:34:23.177 PM[info](http://192.168.1.6/logs#) Homebridge (v2.9.2) | Sending DEVICE Event (Dining Room Fan | LEVEL: 100%) to Homebridge at (192.168.1.4:8001)
[app:8](http://192.168.1.6/logs#)2023-10-16 07:34:23.130 PM[debug](http://192.168.1.6/logs#)Homebridge (v2.9.2) | Send to plugin Completed | Process Time: (28ms)
[app:8](http://192.168.1.6/logs#)2023-10-16 07:34:23.127 PM[debug](http://192.168.1.6/logs#)Homebridge (v2.9.2) | asyncHttpCmdResp | Src: EventUpdate | Resp: {"evtSource":"Homebridge_Hubitat-v2_8","evtType":"attrUpdStatus","evtDevice":"Dining Room Fan","evtAttr":"speed","evtStatus":"OK"} | Status: 200 | Data: [execDt:1697510063101, src:EventUpdate, evtLog:true]
[app:8](http://192.168.1.6/logs#)2023-10-16 07:34:23.099 PM[info](http://192.168.1.6/logs#) Homebridge (v2.9.2) | Sending DEVICE Event (Dining Room Fan | SPEED: high) to Homebridge at (192.168.1.4:8001)
[dev:15](http://192.168.1.6/logs#)2023-10-16 07:34:23.029 PM[info](http://192.168.1.6/logs#)Dining Room Ceiling Fan fan speed was set to high
[app:8](http://192.168.1.6/logs#)2023-10-16 07:34:22.880 PM[info](http://192.168.1.6/logs#) Homebridge (v2.9.2) | Name: Dining Room Fan | Command: [setSpeed()] | Process Time: (119ms)
[app:8](http://192.168.1.6/logs#)2023-10-16 07:34:22.873 PM[info](http://192.168.1.6/logs#) Homebridge (v2.9.2) | Command Successful for Device | Name: Dining Room Fan | Command: [setSpeed(medium-high)]
[app:8](http://192.168.1.6/logs#)2023-10-16 07:34:22.757 PM[info](http://192.168.1.6/logs#) Homebridge (v2.9.2) | Plugin called Process Command | DeviceId: 26 | Command: (setSpeed) | Param1: (medium-high)

Issue Description

I utilized the following workaround to resolve the issue locally, but this is obviously not a long term solution:

...
            } else if (value1 != null) {
                if (command == "setSpeed" && (isDeviceInInput('fan3SpdList', devId) || isDeviceInInput('fan4SpdList', devId) || isDeviceInInput('fan5SpdList', devId))) {
                    // Convert the string speed to a number for Homebridge (Hampton Bay)
                    Integer newSpdVal = convertHomekitToHamptonBaySpeed(value1)
                    logDebug("Hampton Bay Fan Speed: ${value1} | New Speed: ${newSpdVal}")
                    logDebug("Hampton Bay Fan: ${device}")
                    device."setLevel"(newSpdVal, 0)
                    if (shw) { cmdS = cmdS + "$value1)]".toString() }
                }
                else {
                    device."$command"(value1)
                    if (shw) { cmdS = cmdS + "$value1)]".toString() }
                }
            }
...        

private static Integer convertHomekitToHamptonBaySpeed(String speed) {
    Map<String, String> speedMappings = [
        "low": 0,
        "medium-low": 25,
        "medium": 50,
        "medium-high": 75,
        "high": 100,
        "on": 100,
        "off": 0,
        "auto": 50  // You can adjust this based on your needs
    ]

    Integer speedPercentage
    speedPercentage = speedMappings[speed] ?: 0
    return speedPercentage
}
jonnyborbs commented 1 year ago

I have exactly this same issue with Inovelli fan control switches, where the Home app sends 5 speed commands to Homebridge, and the funky math in the functions from lines 1983-2003 causes wrong values to be sent to the controller.

Most 3 speed fans, which is really just "most fans" expect 0 [off], low [33], medium [66], high [100]

I have tried messing with the values in the map and removing the rounding/percentage/toInt math that I don't quite understand the purpose of and it still just keeps sending "medium-low" requests that my fans can't understand.

Would really appreciate a way to make these configurable as well, or at least some clarity around what the private static Integer getFanSpeedInteger function is doing here. I'm not sure why the changes I attempted didn't work.

private static Integer getFanSpeedInteger(String speed, Integer numberOfSpeeds = 3) {
    Map<String, Integer> speedMappings = [
//        "low": 0,
//        "medium-low": 25,
//        "medium": 50,
//        "medium-high": 75,
//        "high": 100,
//JS changed values here
        "low": 33,
        "medium-low" : 33,
        "medium" : 66,
        "medium-high": 66,
        "high": 100,
        "on": 100,
        "off": 0,
        "auto": 50  // You can adjust this based on your needs
    ]

    Integer speedPercentage
    speedPercentage = speedMappings[speed] ?: 0
    // Adjust percentage based on the number of speeds
    //  -1 because index is 0-based
    //speedPercentage = Math.round( ((speedPercentage / 100.0) * (numberOfSpeeds - 1)) * 100).toInteger()
    //JS removed above line
    return speedPercentage

}
jonnyborbs commented 1 year ago

Actually, digging around a bit more I think the issue lies in these two functions here:

https://github.com/tonesto7/homebridge-hubitat-tonesto7/blob/42b42722a4511ea23c56b22bc66c89479c69a978/src/HE_Transforms.js#L486

And here

https://github.com/tonesto7/homebridge-hubitat-tonesto7/blob/42b42722a4511ea23c56b22bc66c89479c69a978/src/HE_Transforms.js#L504

This is on the JS side within Homebridge itself, and is sending string values based on rigidly defined numerics. Those string values are sent to the devices and interpreted for zwave/zigbee execution.

I don't think the issue is really within the Groovy code at all but something that needs manipulation in the homebridge plugin itself

jonnyborbs commented 1 year ago

Yeah, this tracks with what I am observing in the logs perfectly.

Assume the slider in the Home app has 4 positions: 0=Off 1=33% 2=66% 3=100%

When I set the slider in Apple Home to position 1, which reads "33%" on the slider, this is the log output I see in Homebridge

[25/10/2023, 18:15:54] [Hubitat-v2] Sending Device Command: setSpeed | Value: {"value1":"medium-low"} | Name: (Living Room Fan) | DeviceID: (30) | UsingCloud: (false)
[25/10/2023, 18:15:56] [Hubitat-v2] [Device Event]: (Living Room Fan) [LEVEL] is 99
[25/10/2023, 18:15:56] [Hubitat-v2] [Device Event]: (Living Room Fan) [SPEED] is medium
[25/10/2023, 18:15:56] [Hubitat-v2] [Device Event]: (Living Room Fan) [LEVEL] is 58
[25/10/2023, 18:15:58] [Hubitat-v2] [Device Event]: (Living Room Fan) [LEVEL] is 40

Now, look at the conversion function:

    fanSpeedConversion(speedVal) {
        // console.log("speedVal: ", speedVal);
        if (speedVal <= 0) {
            return "off";
        } else if (speedVal > 0 && speedVal <= 20) {
            return "low";
        } else if (speedVal > 20 && speedVal <= 40) {
            return "medium-low";
        } else if (speedVal > 40 && speedVal <= 60) {
            return "medium";
        } else if (speedVal > 60 && speedVal <= 80) {
            return "medium-high";
        } else if (speedVal > 80 && speedVal <= 100) {
            return "high";
        }
    }

So, because the fanSpeedConversion function interprets "33%" as "medium-low" it sends that command to the switch.

Then the following fanSpeedToLevel function is returning a value of 40 for medium-low,

switch (speedVal) {
            case "off":
                return 0;
            case "low":
                return 33;
            case "medium-low":
                return 40;
            case "medium":
                return 66;
            case "medium-high":
                return 80;
            case "high":
                return 100;

But "medium-low" isn't a value a 3 speed switch understands, and so this is causing the bouncing issue.

jonnyborbs commented 1 year ago

It's also worth noting that since a 3 speed switch sees "40" as past the threshold of 33, it runs at medium when you set low, high when you set medium, and has no ability to run at low at all.

So this being the case, I actually think this is not just an enhancement but a bug - the logic for working with 3 speed controls appears to be incomplete, there are accommodations made for it in the Groovy side that are not reflected deeply enough in the plugin side.

tonesto7 commented 1 year ago

In the home bridge app under Hubitat are you guys placing the device under the correct fan speed inputs

jonnyborbs commented 1 year ago

Yup, for sure all of mine are in the fan3speed list CleanShot 2023-10-27 at 13 21 26

I have tried them in the 4 and 5 speed lists to see if it would make the behavior more predictable, even if imperfect - but it didn't really help.

tonesto7 commented 1 year ago

image Those inputs help the HomeKit plugin know how many available speeds there which then defines what each increment is on the slider

jonnyborbs commented 1 year ago

Yeah, our messages crossed paths here! Confirmed I am using the correct list, cannot speak for OP but I suspect the same

jonnyborbs commented 1 year ago

And that aspect works per https://github.com/tonesto7/homebridge-hubitat-tonesto7/issues/206#issuecomment-1780271570 - the sliders in HomeKit have 3 steps, but the values get all out of wack on the backend.

jonnyborbs commented 1 year ago

Hi @tonesto7 was just wondering if you had any other ideas we could try here! I've been trying to follow the logic used in the plugin to see if I could author a PR here, but I think I'm missing something. I know you asked whether the fans were in the proper speed list, but I can't seem to find where that list is used for anything other than setting the number of stops on the slider control. The trail goes cold for me there, trying to connect that to the logic used to translate the text speeds to numeric values.

Appreciate any help and thanks for creating this/looking into the question!

jonnyborbs commented 7 months ago

Hey y'all, just wanted to prod gently on this again. I've been poking at ways to try and fix this in a private fork but am not an expert at Node at all, so I'm kinda stuck. It's still an issue even after replacing the fan controllers; I've moved from GE/Jasco to Inovelli ones but they still have 3 speed steps and respond exactly the same when controlled via this plugin.

tonesto7 commented 7 months ago

Hello! Sorry, I haven't followed up. I don't know where the issue is for you... I have both GE/jasco fan controllers and Innovelli Fan controllers and it works fine for me.
Try enabling debug logs on both the Hubitat app and the homebridge plugin and see if something stands out. If you could show me the flaw, I will be sure to fix the issue as soon as possible. I'm just not seeing it on my side

jonnyborbs commented 7 months ago

I think the issue is identified as clearly as I can do in https://github.com/tonesto7/homebridge-hubitat-tonesto7/issues/206#issuecomment-1780271570 - the logs are there from Homebridge directly; the value ping-pongs around and resets the speeds