dotsam / homebridge-milight

MiLight/LimitlessLED/Easybulb Plugin for Homebridge
MIT License
63 stars 12 forks source link

Command Queuing #3

Open dotsam opened 8 years ago

dotsam commented 8 years ago

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.

kohlsalem commented 8 years ago

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 commented 8 years ago

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());
    }
  }

}
dotsam commented 8 years ago

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.

kohlsalem commented 8 years ago

:-) 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 commented 8 years ago

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 commented 8 years ago

@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());
    }
  }
}
doooh commented 7 years ago

@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.

dotsam commented 7 years ago

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)