Clean the action/data bytes interpretation in parser.js

[snowdd1]

telegram bytes 8+9 were missing bit masking and shifting to make interpretation according to "KNX Grundkursunterlagen" vol 2009, page 22.

This led to strange values for DPT1 raw data (two additional bits in the value) and strange action values (should be only 0,1,2, and 10)

Discovered this while working on a backend for CometVisu using knxd + node-eibd

[andreek]

LGTM

[andreek]

Published to npm registry with 0.4.1.

[snowdd1]

Hi André, I forgot to change the decoder at

Decoder.prototype.decode = function(len, data, callback) {

  var err = null;
  var type = 'UNKN';
  var value = null;

  // eis 1 / dpt 1.xxx
  if(len === 8) {
    type = 'DPT1';
    value = data-64;
    if(value > 1) {
      value = value-64;
    }
  }

we need to take out the -64 in value = data-64
that was only in there to cope with the wrong bit-mapping.

I cannot create a PR right now, possibly first change next weekend, but this might break applications.
Sorry, I used a different decoder in my app so it did not show up.

Best regards
Raoul

[andreek]

@snowdd1 thanks for the comment. I'll take care about it today. And don't hesitate... It's not your fault. Guess we need a test for the parser. I was this lazy "Oh, cool. Tests passed. Should work" 😃

    if(value > 1) {
      value = value-64;
    }

I'm curious about this line. Needs this line may also to be removed? It looks like real workaround, but I'm little bit out of context right now.
Edit: It wouldn't be reached anymore, right? Because DPT1 is only 0/1 ?

[snowdd1]

I checked it using direct outputs from DPT1 writes. The only values that are passed are 0 and 1 without any further processing. But to comply with the documentation we would need to add a bitmap masking of & 1 to make sure only the last bit is used.

So value = data & 1 is sufficient for DPT1.

Of course, if it isn't a DPT1 but a dimmer step or something similar which also comes in with only 8 bytes the interpretation is wrong, because those can use more than 1 bit (3 bits IRC).

[farmio]

I think I stumbled upon the same issue when i reviewed the decoders. I have not touched the parser but just the decoder.js:

// eis 1 / dpt 1.xxx
  if(len === 8) {
    type = 'DPT1';
    value = this.decodeDPT1(data);
  } 

Decoder.prototype.decodeDPT1 = function(buffer) {
  const numericBool = buffer.readUInt8(0) & 0b1;
  return !! numericBool;
};

https://github.com/farmio/node-eibd/blob/f885339859d558be40a7835efb1934befd6caea6/lib/decoder.js#L14

[andreek]

@snowdd1 thank you for the feedback.

Published with 0.4.2.

[snowdd1]

Great, and sorry for the issue at all. But now it should be so clean that any external DPT parser can use the "telegram" event to parse any data coming in, assuming that the DPT is known to the parser.