arachnetech / homebridge-mqttthing

A plugin for Homebridge allowing the integration of many different accessory types using MQTT.
Apache License 2.0
468 stars 104 forks source link

RGB Strip color is reset to EFEFFF0000 after HB restart #567

Open dxdc opened 2 years ago

dxdc commented 2 years ago

Every time HB restarts, it resets the color value in my LED strips to HEX code EFEFFF0000 (HSBColor 237,6,100).

To reproduce:

I inspected the MQTT command and confirmed how this happening.

This is what I see after restart:

tasmota/ledstrip1/cmnd/State (null)
tasmota/ledstrip1/cmnd/HSBColor 237,6,100
tasmota/ledstrip2/cmnd/State (null)
tasmota/ledstrip2/cmnd/HSBColor 237,6,100
tasmota/ledstrip3/cmnd/State (null)
tasmota/ledstrip3/cmnd/HSBColor 237,6,100
tasmota/ledstrip4/cmnd/State (null)
tasmota/ledstrip4/cmnd/HSBColor 237,6,100

I think this bug is being triggered here, which calls characteristics_HSVLight : https://github.com/arachnetech/homebridge-mqttthing/blob/499c5af7f11abf8113c775d9a337aad9e87e47d6/index.js#L2759-L2762

The relevant lines of code in characteristics_HSVLight are: https://github.com/arachnetech/homebridge-mqttthing/blob/499c5af7f11abf8113c775d9a337aad9e87e47d6/index.js#L540-L554

The problem is that there is the color is not cached, and, no MQTT message was polled before the publish() command is called. Note: This issue may be related to #552, and may also appear affect other functions (e.g., RGBLight).

Sample config:

    {
      "accessory": "mqttthing",
      "manufacturer": "Tasmota",
      "model": "Tasmota",
      "name": "LED Strip Light",
      "confirmationPeriodms": 1000,
      "retryLimit": 5,
      "integerValue": true,
      "onValue": "ON",
      "offValue": "OFF",
      "serialNumber": "1",
      "startPub": {
        "tasmota/ledstrip1/cmnd/State": ""
      },
      "topics": {
        "getColorTemperature": {
          "apply": "return JSON.parse(message).CT;",
          "topic": "tasmota/ledstrip1/stat/RESULT"
        },
        "setColorTemperature": "tasmota/ledstrip1/cmnd/CT",
        "getHSV": {
          "apply": "return JSON.parse(message).HSBColor;",
          "topic": "tasmota/ledstrip1/stat/RESULT"
        },
        "setHSV": "tasmota/ledstrip1/cmnd/HSBColor",
        "getOn": "tasmota/ledstrip1/stat/POWER",
        "setOn": "tasmota/ledstrip1/cmnd/Power",
        "getOnline": {
          "apply": "return /online/i.test(message) ? 'ON' : 'OFF';",
          "topic": "tasmota/ledstrip1/tele/LWT"
        }
      },
      "type": "lightbulb",
      "url": "mqtt://localhost",
      "username": "",
      "password": ""
    },
dxdc commented 2 years ago

After more research, I found out that the problem is here in lines 1444-1447:

https://github.com/arachnetech/homebridge-mqttthing/blob/499c5af7f11abf8113c775d9a337aad9e87e47d6/index.js#L1437-L1454

onSet is being initialized with 140 as a default value, which is throwing off the existing color.

As a temporary solution, I added this line, which seems to work:

     integerCharacteristic( service, 'colorTemperature', Characteristic.ColorTemperature, null, null, { 
         initialValue: 140, 
         onSet: ( value ) => { 
             if ( value === 140 ) { return; } // added to avoid onSet overriding HSV settings.

I don't understand the adaptive lighting part of the code enough yet, but that fixes the problem upon restarting.

dxdc commented 1 year ago

@arachnetech any chance you could look into this issue? There is a race condition being set up here. The onSet function is being called for characteristic_ColorTemperature_Internal before an actual value is received from MQTT. This causes the initial value of 140 to be used, which is not the desired behavior, and disturbs the saved value.

One potential cause could be the ordering of the characteristics setup. The characteristic_HSVLight and characteristic_ColorTemperature are called independently. This could lead to a race condition, where the onSet function for characteristic_ColorTemperature might be invoked before the MQTT value is actually received and the characteristic_HSVLight setup is completed.

There are different ways to solve it, including just setting a flag (or, just adding a simple timeout for example), but it may be a problem that's more pervasive in the codebase itself, not sure. For example, we could define a global mqtt received flag and check it before running the onSet function. I'm not sure the best approach.

let mqttValueRetrievedFlags = {}; // could be used to define/check if mqtt values are received. But, we'd need to consider what happens if they aren't received or stored.
...
else if( configType == "lightbulb" ) {
    service = new Service.Lightbulb( name, subtype );
    if( config.topics.getHSVColor ) {
        characteristic_HSVLight( service );
    } else {
        characteristic_On( service );
        if( config.topics.getHue || config.topics.setHue ) {
            characteristic_Hue( service );
        }
        if( config.topics.getSaturation || config.topics.setSaturation ) {
            characteristic_Saturation( service );
        }
    }
    if( config.topics.getOnline ) {
        state_Online();
    }
    if( config.topics.getColorTemperature || config.topics.setColorTemperature ) {
        characteristic_ColorTemperature( service );
    }
    if( config.topics.getBrightness || config.topics.setBrightness ) {
        characteristic_Brightness( service );
    }
}
lakenen commented 5 months ago

Any updates on this? I think it's related to https://github.com/arachnetech/homebridge-mqttthing/issues/617 ?

dxdc commented 5 months ago

It's a race condition within this plugin that is more than 2 years old... I'm not quite sure how to solve it, and it may involve quite a bit of refactoring.

lakenen commented 4 months ago

In case anyone else has this issue and doesn't need/use adaptive lighting, I found a fix that doesn't require code changes.

https://github.com/arachnetech/homebridge-mqttthing/blob/499c5af7f11abf8113c775d9a337aad9e87e47d6/index.js#L96

Just add "adaptiveLighting": false to the mqttthing accessory config.