homebridge-milight icon indicating copy to clipboard operation
homebridge-milight copied to clipboard

Command Queuing

Open dotsam opened this issue 9 years ago • 8 comments

Many HomeKit applications send out commands at a very quick rate (ie. using a brightness slider, and sending every brightness value that the user moves over). With the way the MiLight protocol works and the way the node-milight-promise library queues commands, this becomes an issue.

Need to figure out the best way to handle this to improve responsiveness. Some way of throwing away excess commands.

dotsam avatar Dec 10 '15 22:12 dotsam

Have you ever thougt to set a timer, when a command comes? When a new command comes, you could quit the first timer... The callback of the timer could actually send the commamd...

kohlsalem avatar Jan 17 '16 20:01 kohlsalem

I changed this, and now the sliders as well work like a charm for me... :-)

var brightnessTimer;

MiLightAccessory.prototype.setBrightness = function(level, callback) {

  clearTimeout(brightnessTimer);
  brightnessTimer = setTimeout(setBrightnessTimer, 100, this, level);

  callback(null);
}

function setBrightnessTimer(lightAccessory, level) {
  if (level == 0) {
    // If brightness is set to 0, turn off the lamp
    lightAccessory.log("[" + lightAccessory.name + "] Setting brightness to 0 (off)");
    lightAccessory.light.sendCommands(commands[lightAccessory.type].off(lightAccessory.zone));
  } else if (level <= 5 && (lightAccessory.type == "rgbw" || lightAccessory.type == "white")) {
    // If setting brightness to 5 or lower, instead set night mode for lamps that support it
    lightAccessory.log("[" + lightAccessory.name + "] Setting night mode");

    lightAccessory.light.sendCommands(commands[lightAccessory.type].off(lightAccessory.zone));
    // Ensure we're pausing for 100ms between these commands as per the spec
    lightAccessory.light.pause(100);
    lightAccessory.light.sendCommands(commands[lightAccessory.type].nightMode(lightAccessory.zone));

  } else {
    lightAccessory.log("[" + lightAccessory.name + "] Setting brightness to %s", level);

    // Send on command to ensure we're addressing the right bulb
    lightAccessory.light.sendCommands(commands[lightAccessory.type].on(lightAccessory.zone));

    // If this is an rgbw lamp, set the absolute brightness specified
    if (lightAccessory.type == "rgbw") {
      // Compress down the scale to account for setting night mode at brightness 1-5%
      lightAccessory.light.sendCommands(commands.rgbw.brightness(level-4));
    } else {
      // If this is an rgb or a white lamp, they only support brightness up and down.
      // Set brightness up when value is >50 and down otherwise. Not sure how well this works real-world.
      if (level >= 50) {
        if (lightAccessory.type == "white" && level == 100) {
          // But the white lamps do have a "maximum brightness" command
          lightAccessory.light.sendCommands(commands.white.maxBright(lightAccessory.zone));
        } else {
          lightAccessory.light.sendCommands(commands[lightAccessory.type].brightUp());
        }
      } else {
        lightAccessory.light.sendCommands(commands[lightAccessory.type].brightDown());
      }
    }
  }

}

var hueTimer;

MiLightAccessory.prototype.setHue = function(value, callback) {

  clearTimeout(hueTimer);
  hueTimer = setTimeout(setHueTimer, 100, this, value);

  callback(null);
}

function setHueTimer(lightAccessory, value) {
  lightAccessory.log("[" + lightAccessory.name + "] Setting hue to %s", value);

  var hue = Array(value, 0, 0);

  // Send on command to ensure we're addressing the right bulb
  lightAccessory.light.sendCommands(commands[lightAccessory.type].on(lightAccessory.zone));

  if (lightAccessory.type == "rgbw") {
    if (value == 0) {
      lightAccessory.light.sendCommands(commands.rgbw.whiteMode(lightAccessory.zone));
      // Set saturation to 0 because only with hue and saturation at 0 should we be white
      lightAccessory.lightbulbService.setCharacteristic(Characteristic.Saturation, 0);
    } else {
      lightAccessory.light.sendCommands(commands.rgbw.hue(commands.rgbw.hsvToMilightColor(hue)));
    }
  } else if (lightAccessory.type == "rgb") {
    lightAccessory.light.sendCommands(commands.rgb.hue(commands.rgbw.hsvToMilightColor(hue)));
  } else if (lightAccessory.type == "white") {
    // Again, white lamps don't support setting an absolue colour temp, so trying to do warmer/cooler step at a time based on colour
    if (value >= 180) {
      lightAccessory.light.sendCommands(commands.white.cooler());
    } else {
      lightAccessory.light.sendCommands(commands.white.warmer());
    }
  }

}

kohlsalem avatar Jan 18 '16 20:01 kohlsalem

This looks pretty good. I might work it so the timeout follows the delay and repeat set in the config, but otherwise, this looks to be the solution.

dotsam avatar Jan 20 '16 00:01 dotsam

:-) I failed in accessing the delay, thought about this.

Another (better?) option would be to check the chain of open promises, but this was several levels to hard....

kohlsalem avatar Jan 20 '16 16:01 kohlsalem

In case you did not find it by yourself: my coding proposal has a nasty stupid bug: If e.g. a scene macro sends several brightness commands to different bulbs, only the last would survive.

Somehow the concept with timers would need a timer per bulb.

kohlsalem avatar Jan 28 '16 19:01 kohlsalem

@dotsam, Ok, i fixed that and added the delay from config. Now i have no Issues on scene macros AND a smooth slider behavior. :-)

var brightnessTimer = new Array ();

MiLightAccessory.prototype.setBrightness = function(level, callback) {

  clearTimeout(brightnessTimer[this.name]);
  brightnessTimer[this.name] = setTimeout(setBrightnessTimer, this.light._delayBetweenCommands, this, level);

  callback(null);
}

function setBrightnessTimer(lightAccessory, level) {

  if (level == 0) {
    // If brightness is set to 0, turn off the lamp
    lightAccessory.log("[" + lightAccessory.name + "] Setting brightness to 0 (off)");
    lightAccessory.light.sendCommands(commands[lightAccessory.type].off(lightAccessory.zone));
  } else if (level <= 5 && (lightAccessory.type == "rgbw" || lightAccessory.type == "white")) {
    // If setting brightness to 5 or lower, instead set night mode for lamps that support it
    lightAccessory.log("[" + lightAccessory.name + "] Setting night mode");

    lightAccessory.light.sendCommands(commands[lightAccessory.type].off(lightAccessory.zone));
    // Ensure we're pausing for 100ms between these commands as per the spec
    lightAccessory.light.pause(100);
    lightAccessory.light.sendCommands(commands[lightAccessory.type].nightMode(lightAccessory.zone));

  } else {
    lightAccessory.log("[" + lightAccessory.name + "] Setting brightness to %s", level);

    // Send on command to ensure we're addressing the right bulb
    lightAccessory.light.sendCommands(commands[lightAccessory.type].on(lightAccessory.zone));

    // If this is an rgbw lamp, set the absolute brightness specified
    if (lightAccessory.type == "rgbw") {
      // Compress down the scale to account for setting night mode at brightness 1-5%
      lightAccessory.light.sendCommands(commands.rgbw.brightness(level-4));
    } else {
      // If this is an rgb or a white lamp, they only support brightness up and down.
      // Set brightness up when value is >50 and down otherwise. Not sure how well this works real-world.
      if (level >= 50) {
        if (lightAccessory.type == "white" && level == 100) {
          // But the white lamps do have a "maximum brightness" command
          lightAccessory.light.sendCommands(commands.white.maxBright(lightAccessory.zone));
        } else {
          lightAccessory.light.sendCommands(commands[lightAccessory.type].brightUp());
        }
      } else {
        lightAccessory.light.sendCommands(commands[lightAccessory.type].brightDown());
      }
    }
  }

}

var hueTimer = new Array ();

MiLightAccessory.prototype.setHue = function(value, callback) {

  clearTimeout(hueTimer[this.name]);
  hueTimer[this.name] = setTimeout(setHueTimer, this.light._delayBetweenCommands, this, value);

  callback(null);
}

function setHueTimer(lightAccessory, value) {
  lightAccessory.log("[" + lightAccessory.name + "] Setting hue to %s", value);

  var hue = Array(value, 0, 0);

  // Send on command to ensure we're addressing the right bulb
  lightAccessory.light.sendCommands(commands[lightAccessory.type].on(lightAccessory.zone));

  if (lightAccessory.type == "rgbw") {
    if (value == 0) {
      lightAccessory.light.sendCommands(commands.rgbw.whiteMode(lightAccessory.zone));
      // Set saturation to 0 because only with hue and saturation at 0 should we be white
      lightAccessory.lightbulbService.setCharacteristic(Characteristic.Saturation, 0);
    } else {
      lightAccessory.light.sendCommands(commands.rgbw.hue(commands.rgbw.hsvToMilightColor(hue)));
    }
  } else if (lightAccessory.type == "rgb") {
    lightAccessory.light.sendCommands(commands.rgb.hue(commands.rgbw.hsvToMilightColor(hue)));
  } else if (lightAccessory.type == "white") {
    // Again, white lamps don't support setting an absolue colour temp, so trying to do warmer/cooler step at a time based on colour
    if (value >= 180) {
      lightAccessory.light.sendCommands(commands.white.cooler());
    } else {
      lightAccessory.light.sendCommands(commands.white.warmer());
    }
  }
}

kohlsalem avatar Jan 28 '16 20:01 kohlsalem

@dotsam Any chance you had time to look into this proposal from @kohlsalem . This looks pretty good to me and I think this would make changing brightness a lot more smooth.

doooh avatar Nov 14 '16 18:11 doooh

Hi @dooh,

Yes, this is still something that I want to implement, but haven't gotten around to just yet. I'll probably implement this in tandem with updated functionality for setting the colour/brightness of a bulb (right now there can be issue going in/out of white/colour mode)

dotsam avatar Nov 22 '16 23:11 dotsam