dotsam / homebridge-milight

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

Lost Commands #6

Closed kohlsalem closed 8 years ago

kohlsalem commented 8 years ago

Hi dotsam,

this might be dupe to the command quing bug, but I#m not sure there. In case i steer my milights "one by one", the plugin works perfectly fine. However, in "complex" scenarios, where i want to set all 4 lights of one wifi controler to different states & settings, commands get lost.

I read a bit in the source code. In the way the MilightController handles promises in send_request, this should be just impossible to happen.

But than i noticed, that in the implementation MiLightAccessory() in your plugin, you are creating a milight controller per lamp, not per bridge.

So in my case with 3 lamps at one wifi controller, the MilightController will exist three times.

Wouldn't this cause parallel commands to the 3 lamps to be send as 3 parallel requests, instead of a proper serialization?

I cant do it by myself, but i would rather recommend to create new MilightController once in then _addLamps function, and pass it to the MiLightAccessory as a parameter.

Best Michael

dotsam commented 8 years ago

Hi Michael,

Yes, I've been noticing this too and am not wasn't sure if it was the aggressive command delay I had set, or the multiple instances of the MilightController, but I suspect you're right. I'll take a look at this and try and get a fix out in the next few days.

kohlsalem commented 8 years ago

I know, this is not in the sense of GIT, but I'm a bit to lazy to work into git for that.

By correction proposal (looks like it works for me):

 zonesLength = 4;
  }

   lightController = new Milight({
    ip: bridgeConfig["ip_address"],
    port: bridgeConfig["port"],
    delayBetweenCommands: bridgeConfig["delay"],
    commandRepeat: bridgeConfig["repeat"]
  });

  // Create lamp accessories for all of the defined zones
  for (var i = 0; i < zonesLength; i++) {
    if ( !! bridgeConfig.zones[i]) {
      bridgeConfig["name"] = bridgeConfig.zones[i];
      bridgeConfig["zone"] = i + 1;
      lamp = new MiLightAccessory(this.log, bridgeConfig, lightController);
      zones.push(lamp);
    }
  }

  return zones;
}

function MiLightAccessory(log, lampConfig, lampController) {
  this.log = log;
  // config info
  this.name = lampConfig["name"];
  this.zone = lampConfig["zone"];
  this.type = lampConfig["type"];

  this.light = lampController;

}