RafaelKa / node-serialport-enocean-parser

ESP3 parser for nodes serialport
Do What The F*ck You Want To Public License
6 stars 2 forks source link

Creating packets #42

Open Holger-Will opened 6 years ago

Holger-Will commented 6 years ago

WIP

RafaelKa commented 6 years ago

Apropos setter IMHO: We do not need them, because it is very complex to use them as API. What we need, are the Factory/Builder methods in ESP3 Protocol class, which create the Packets in one single shot.

As far as I understand, the single reason for creating Packets is to be able to send them via TCM300/310 as telegram. So modifying Packets with setters makes no sense, therefore I proposed to lock the Packets.

All above can be false. Those are currently my speculations based on evaluation-memories of 2013/2014 . To be sure I/we must read current Enocean docs.

Holger-Will commented 6 years ago

What you construct will mostly be RadioERP1 telegrams, with the occasional CommonCommand. I believe in most cases the real telegram creation will happen in classes that inherit from these two classes. Also in most cases you would send your telegram multible times only changing a few bytes. Let the user change these bytes without bothering her with protocol details...

Holger-Will commented 6 years ago

Do you have any usecase where we gain something by locking the packets?

Holger-Will commented 6 years ago

these are just some sketch up of ideas. i will have a look at your factory proposal in more detail later today.

RafaelKa commented 6 years ago

Do you have any usecase where we gain something by locking the packets?

For example storing/saving or using multiple callbacks. By changing received packet in one callback, other callbacks could make many troubles. I think we must consider each packet as Value-Object. https://de.wikipedia.org/wiki/Value_Object Each Packet property can be changed directly. Therefore i proposed the freezing/locking the Packets.

Holger-Will commented 6 years ago

mmmh still not convinced. Especially when saving one might want to add a timestamp, or lat/lon coordinates or other signatures...

if you want multible callbacks in parallel, just use two separate streams. so instead of:

port.pipe(parser)
parser.on("data",callback1)
parser.on("data",booooom)

do this

port.pipe(parser1)
port.pipe(parser2)
parser1.on("data",callback1)
parser2.on("data",callback2)

then both callbacks have their own data to work with.

and if you want a chain of manipulation you can still

port.pipe(parser1).pipe(parser2)

and be sure of the order in which the data gets processed. I think we shouldn't try to be to clever, and prevent users from failure when that means restricting other user who know what they are doing ;-)

I still don't see why Packets should be a Value-Object. For the receive case, ok that might make some sense, but not really, see arguments above. But when it comes to creating packets, that does not make much sense at all. Telegrams represent real world objects which constantly change state. I think it should be possible for objects to reflect that. I thought about object.seal() but that still does not make much sense for the saving case described above.

And if you really want an immutable representation of a telegram why not turn it into a string. that is easier to copy and work with than custom immutable structures.

Holger-Will commented 6 years ago

that said, one way to have some immutability in the receive case but still have flexible objects to work with would be to decouple the parser even more.

Reducing the parser to the absolute minimum so it just detects complete packets and returns immutable structs (No ESP3Packet object, but just the struct), and design the whole enocean implementation as a chain of transform streams would work. Thinking about this, i start to believe that this might be the best way because my goal is to modularize the node-enocean architecture anyways.

Holger-Will commented 6 years ago

Examples of value types ... in the Internet domain they are protocol names, domain names, and URLs ...

it's not tcp packets....