mwittig / node-milight-promise

A node module to control Milight LED bulbs and OEM equivalents such as Rocket LED, Limitless LED Applamp, Easybulb, s`luce, iLight, iBulb, and Kreuzer
MIT License
114 stars 27 forks source link

Checksum calculation seems wrong for connection payload #35

Closed markg85 closed 6 years ago

markg85 commented 6 years ago

Hi,

The connection payload doesn't have the checksum (it's commented out because the 'calcChecksum' function is doing it). Or is it? ;) calcChecksum does calculate a checksum, but only for the last 11 bytes. The resulting checksum is not 0x1E for the connection.

I can make a pull request for you, but you might be easier off fixing it the way you like it. What i did:

        self._rpc([
          0x20, 0x00, 0x00, 0x00, 0x16, 0x02, 0x62, 0x3A,
          0xD5, 0xED, 0xA3, 0x01, 0xAE, 0x08, 0x2D, 0x46,
          0x61, 0x41, 0xA7, 0xF6, 0xDC, 0xAF, 0xD3, 0xE6,
          0x00, 0x00, 0x1E
        ]).then(function (response) {
        // ...

And in calcChecksum i merely changed the if to:

  if (bytes.length >= 11 && bytes.length != 27) {

Which now neglects the connection payload and gives it the appropriate checksum. All in milight-v6-mixin.js obviously.

I found this while debugging the bytes send. I can't get my ibox2 to turn on/off lights with this library. I am getting packet responses so there is nothing wrong with connecting. Just with the packets it seems, but i'm still investigating that. This one bug is worth a fix imho, even though it didn't seem to bother connecting (works before and after this change).

Best regards, Mark

mwittig commented 6 years ago

At a first glance I don't think it is checksum issue, unless the new firmware shipped with recent iBox2 makes has changed the way checksum calculation is performed.

Another source of error might be the controller model selected via the mobile app. E.g. If you pair your bulbs with the 8-channel controller via the App, the control via node-milight-promise won't work as it is based on the command sets of the (former) 4-channel controllers. Btw. support for the 8-channel controller is on the way and there is already some code on master.

mwittig commented 6 years ago

The protocol sketch described in Section 3 at http://www.limitlessled.com/dev/ is suggesting that there is no checksum for the "start session":

UDP.SEND hex bytes: 20 00 00 00 16 02 62 3A D5 ED A3 01 AE 08 2D 46 61 41 A7 F6 DC AF (D3 E6) 00 00 1E <-- Send this to the ip address of the wifi bridge v6

Octets given in brackets usually denote a variable, but there is no variable defined in this context. Actually, I found the last the 5 bytes (including the stop byte 0x1E) aren't handled by the the bridge at all! Basically I am using the checksum here as it appeared reasonable to me. It can be regarded as a bug as the checksum isn't 0x1E, obviously, on the other hand the stop byte is not checked at all by the current bridge implementations. As this also works with iBox2 I'd prefer to leave it as it is for now.

Regarding your on/off issue see my previous post and get back to me. See also:

markg85 commented 6 years ago

Hi,

What you described seems logical to me. However, if things don't work (which is very possible with this kind of stuff) the first thing you start to look into are the packets being send. If another one would follow the same route, the other person would also figure out the checksum being wrong (even though it doesn't matter). I think it's best to stick to what the limitlessled api tells and force the 0x1E checksum as last byte. Just so that the output is as you would expect to ease debugging pain. That's just my opinion though :)

Regarding the on/off stuff. Now we do deviate from the topic here, if we proceed with more posts i probably need to make a new issue for it. I am using a 4 zone remote with RGBWW (your RgbwCommand class). The class - as is - didn't work for me.

for instance:

RgbwCommand.prototype.on = function(zone) {
  var zn = Math.min(Math.max(zone, 0x00), 0x04);
  return [0x31, 0x00, 0x00, 0x07, 0x03, 0x01, 0x00, 0x00, 0x00, zn]
};

I had to replace the 0x07, 0x03 bytes to: 0x08, 0x04

The annoying thing is that http://www.limitlessled.com/dev/ talks about both. The sample node.js code uses 0x07, 0x03 The documentation uses 0x08, 0x04

I don't know if one is wrong. For me the later one works, but perhaps there are devices where it needs to be 0x7, 0x03?

After changing the 2 bytes to 0x08, 0x04 the commands (on and off) started working.

mwittig commented 6 years ago

I don't know if one is wrong. For me the later one works, but perhaps there are devices where it needs to be 0x7, 0x03?

The bulbs you have don't appear to be RGBWW but the new bulbs which add saturation control. Try using the "fullColor" command set instead of "rwbw". This should work as expected.

markg85 commented 6 years ago

Ahh right, my mistake there. I thought i had RGBWW light.. I do, but not in the same bulb / led strips ;)

It's RGBW for both bulbs and strips. And one other strip with CW/WW

mwittig commented 6 years ago

I am closing this now. Please open a new issue if you have further questions