Raw telegram event #48
Conversation
For handling unparsed data this commit adds an 'telegram' event (only with data, otherwise use "read" event) emits `"telegram", event, src, dest, buffer`
tools.adr2str --> tools.addr2str;
|
Older versions are broken because of |
|
I am not very proficient in node < 4.0 (that was the LTS when I started), but I'll give it a try: |
Buffer.from() existed in node 4.4 - use Buffer.alloc() as test for old node versions
|
... and failed, as Buffer.from() existed in older node versions, but is neither documented nor does it work as expected. As test I now use Buffer.alloc(), which really was not existing in node 4.4 and before. |
|
Well, I found a npm packet that is solving the problem. I don't want node-eibd to depend on it. Because we only need to know if we are new api or old api. See https://github.com/LinusU/buffer-from/blob/master/index.js We could add a method to our tools like this isModern and use it to determine if we got the new buffer api in parser.js. |
|
Hi André, I've compared that to what I did - and it's basically the same. It uses Buffer.alloc to determine if the "modern" Buffer is available. Of course it has a lot of more features (such as determining if the parameter passed to Buffer.from() is an array or a string, but we don't need all this. Questions: Do you want to have a central place in eibd to determine the modernity of node once and put that into a variable, and define the buffer initializer and copier in one place (better if we would need it somewhere else, too), or leave it in the parse.js where it is now (single purpose)? |
|
But I found that I don't seem to need to create the buffer first and then copy the data to it, there was a Buffer constructor that took a buffer, too. So it's basically down to 4 lines. Fingers crossed that travis shows good! |
lib/parser.js
Outdated
| var valbuffer; | ||
| if (len<=8) { | ||
| val = telegram[telegram.length-1]; | ||
| if (Buffer.alloc) { // use Buffer.alloc as a means of distinction, as Buffer.from existed in node 4.4 but did not work as expected |
There was a problem hiding this comment.
// val is an integer, to to pass it to Buffer in old api we need to make it an Array of length 1
valbuffer = isModernBuffer ? Buffer.from(val) : new Buffer([val]);
lib/parser.js
Outdated
| val = telegram.slice(10, telegram.length); | ||
| if (Buffer.alloc) { // see above |
There was a problem hiding this comment.
valbuffer = isModernBuffer ? Buffer.from(val) : new Buffer(val);
|
|
||
| val = telegram[telegram.length-1]; | ||
| if(len > 8) { | ||
| var valbuffer; |
There was a problem hiding this comment.
Doing node api detection in one place
var isModernBuffer = (
typeof Buffer.alloc === 'function' &&
typeof Buffer.allocUnsafe === 'function' &&
typeof Buffer.from === 'function'
)There was a problem hiding this comment.
Now in the head of the module.
|
Leaved some comments about how to get the code more readable . |
|
HI André, if changed the lines according to the tips you've left for me. Much better readable now, and fewer lines of code, too. |
|
I am so confident now I have even increased the version number in package.json. |
|
Hi, something wrong with new parser - now my home bridge-knx setup broken: �[37m[4/12/2017, 10:33:56 PM]�[39m �[36m[Marantz]�[39m current power state is: OFF |
|
Which node version? |
|
Hi andreek, |
|
andreek, I have compatibility problems with the latest stable node. |
|
hi andreek, tried node v6.10.0 |
|
@Demonderik should be fixed. See #49 |
When using an exernal decoder for telegram data, the event system could not be used fully as the data was pre-parsed by decoder.js decode() which might not have been guessing the type right (e.g. for a lot of scaled data types)
Refers to #47