rafaelka / node-serialport-enocean-parser Goto Github PK

we need to log some fails in byte stream e.g.:

1. node-serialport-enocean-parser/ESP3Parser.js

   Lines 58 to 69 in 2514534

   	fetchCRC8ForHeaderAndCheck(byte) {
   	if (this.getCrc8(this.tmp.header) != byte) {
   	this.callbackForNextByte = this.waitForSyncByte;
   	// @todo : Log this fail
   	return;
   	}
   	this.currentESP3Packet.crc8Header = byte;
   	this.currentESP3Packet.data = Buffer.alloc(this.currentESP3Packet.header.dataLength);
   	// @todo: is 0 bytes buffer really needed? -> maybe "if (currentESP3Packet.header.optionalLength > 0)"?
   	this.currentESP3Packet.optionalData = Buffer.alloc(this.currentESP3Packet.header.optionalLength);
   	this.callbackForNextByte = this.fillData;
   	}

2. node-serialport-enocean-parser/ESP3Parser.js

   Lines 93 to 105 in 2514534

   	fetchCRC8ForDataAndCheck(byte){
   	this.callbackForNextByte = this.waitForSyncByte;
   	var datas = Buffer.concat(
   	[this.currentESP3Packet.data, this.currentESP3Packet.optionalData],
   	(this.currentESP3Packet.header.dataLength + this.currentESP3Packet.header.optionalLength)
   	);
   	if (this.getCrc8(datas) != byte) {
   	// todo : Log this fail
   	return;
   	}
   	this.currentESP3Packet.crc8Data = byte;
   	this.emitFetchedESP3Packet();
   	}

• Is there standard logger in node or serial port available?
• Is it possible and makes it sense to pass some Prototype/Object/Callback to pass messages?

the examples are outdated... so write new ones and/or update existng ones.

new Buffer(...) is deprecated and unsafe. Could it be replaced with Buffer.from(...) in these places?

We must evaluate functionality of _flush() properly.

• Is it possible, that not fulfilled Packet will be pushed, if _flush() is called?
• Is it possible, that the same Packet can be emitted twice? Also for example if _flush() is called in same tick with emitFetchedESP3Packet().

Something else is important?

row 82 is not covered by tests:

See: https://coveralls.io/builds/20408130/source?filename=ESP3Parser.js#L82

Some Packet without optional data is required.

When sending data to a compatible device and having a response from the TCMXXX, it is common that the packet does not have optional data (optional data length == 0), is then causes a fault in the state machine which does not trigger the "data" event and makes the next packet to be dropped in order to the machine to be in its correct state.

A fix is proposed in the #5 Pull Request

is not covered by

See: https://coveralls.io/builds/20408130/source?filename=ESP3Parser.js#L101

Originally posted by @RafaelKa in #27

i think this is in your module, can you take a look at this?

enocean-js/node-enocean#15

this parser does not seem to work with serialport 5.x. Would be great having support for it, going forward. cheers.

a follow up for the disussion in #20

The current version 0.1.0 is now outdated. Any ETA to have the corresponding changes pushed to npmjs?

check every function and method for proper error handling and add tests passing invalid data everywhere.
I propose to try to recover from most bogus inputs as much as we can and only throw if absolutely necessary.
For cases where we recover it's best to log somewhere and outside of the parser we are not in a stream so emit("error") does not work... :-(
the question is again how to log.

we should add linting check in travis, to prevent messy code in master branch.

ERP3Packet should have a pack method allowing basic encoding. see #36

i propose to split the code into two repositories. Following the Unix principle of doing one thing,well.
Here is the place for the simple parser with the only purpose of detecting packets.
it's a transformable stream that emits structs which should be immutable.
It' should be configurable to emit string or buffer instead.

Everithing else should move to the ESP3 Protocol implementation.

Here can be discussed which coding standards are currently for node important.

Following stuff was interesting for me by coding on tests:

• indentation
  2 spaces seems to be most used on JS
• semicolons presence
  are not needed, but sometimes mandatory
  👍 for semicolons -> https://github.com/douglascrockford/howjavascriptworks

see implementation in #36

I'm using your parser in node-enocean now. My receiving and testing code relies on having the complete telegram avaiable as a Buffer. currently i have to convert the ESPPackat returned by the parser back to a Buffer. It woud be very convienient if this would be part of the ESPPacket.

{
  syncByte: 85,
  header: { dataLength: 5, optionalLength: 1, packetType: 2 },
  crc8Header: 219,
  data: <Buffer 00 ff a0 87 00>,
  optionalData: <Buffer 09>,
  crc8Data: 228,
  buffer: <Buffer 55 00 05 00 01...the complete packet as a Buffer...> 
}

0.3.0

we should not only export the parsers, but also the classes and structs.

the crc8 calculation is useful in many places in a full enocean implementation. i would like to export that as well.

serialport and engine seem to be wrong for current implementation