Steve-Mcl / node-red-contrib-buffer-parser

A node-red node to convert values in a buffer, integer array or hex string into many different data type(s). Supports Big/Little Endian, BCD, byte swapping and much more.
MIT License
24 stars 8 forks source link

Issue with output property #5

Closed mikeS7 closed 3 years ago

mikeS7 commented 3 years ago

Output property doesn't process "child" correctly. I have an message with property named "parsed" which is an object: { "header": {}, "payload": {} }

if I set Output property to parsed.header I've got: image

new property "parsed.header" is created instead of updating the existing one.

The flow to reproduce: [{"id":"2ab1070c.384058","type":"inject","z":"90b3f59b.fae398","name":"","props":[{"p":"payload"},{"p":"topic","vt":"str"}],"repeat":"","crontab":"","once":false,"onceDelay":0.1,"topic":"check","payload":"[34,48,98,116]","payloadType":"bin","x":220,"y":120,"wires":[["439ffd34.3d2a74"]]},{"id":"ab1d2743.b49f88","type":"debug","z":"90b3f59b.fae398","name":"","active":true,"tosidebar":true,"console":false,"tostatus":false,"complete":"true","targetType":"full","statusVal":"","statusType":"auto","x":790,"y":120,"wires":[]},{"id":"439ffd34.3d2a74","type":"change","z":"90b3f59b.fae398","name":"","rules":[{"t":"set","p":"parsed","pt":"msg","to":"{\"header\":{},\"payload\":{}}","tot":"json"}],"action":"","property":"","from":"","to":"","reg":false,"x":430,"y":120,"wires":[["be6fa509.c6c408"]]},{"id":"be6fa509.c6c408","type":"buffer-parser","z":"90b3f59b.fae398","name":"","data":"payload","dataType":"msg","specification":"spec","specificationType":"ui","items":[{"type":"int16be","name":"item1","offset":0,"length":1,"offsetbit":0,"scale":1,"mask":""},{"type":"int16be","name":"item2","offset":2,"length":1,"offsetbit":0,"scale":1,"mask":""}],"swap1":"","swap2":"","swap3":"","swap1Type":"swap","swap2Type":"swap","swap3Type":"swap","msgProperty":"parsed.header","msgPropertyType":"str","resultType":"keyvalue","resultTypeType":"output","multipleResult":false,"setTopic":true,"x":630,"y":120,"wires":[["ab1d2743.b49f88"]]}]

Steve-Mcl commented 3 years ago

Hi Mike,

Are you saying the input msg already has a property (object) of msg.parsed and you want results to be written to msg.parsed.header but instead the result is being written to `msg["parsed.header"]?

Is that an accurate description of your issue?

Please let me know if I am missing the point & I will take a look later today.

In the meantime, so that you can progress (I am sure you know this) but you can use a change node to move that to the desired location after the buffer-parser (until I get a fix pushed out) :)

thanks for the feedback BTW.

mikeS7 commented 3 years ago

Are you saying the input msg already has a property (object) of msg.parsed and you want results to be written to msg.parsed.header but instead the result is being written to `msg["parsed.header"]?

You are absolutely right

PS Proposed workaround already implemented, but it become complex if object is not empty ;)

Steve-Mcl commented 3 years ago

fixed.

thanks for taking the time to report the issue.

Will publish on flows library in the next few minutes - if you could update when you see V3.0.3 & let me know either way?

cheers, Steve.

mikeS7 commented 3 years ago

Steve, thank you!

Unfortunately object (property) is overwritten completely, but I expect that it should be updated with new items. For ex, I have { "header": { "test": "m7" }, "payload": {} }

and I expect that new items will be added to the object, but in case object is created from scratch

may be RED.util.setMessageProperty should be used

Steve-Mcl commented 3 years ago

I expect that new items will be added to the object, but in case object is created from scratch

eh? I tested that :(

let me have another look.

Steve-Mcl commented 3 years ago

ok, so I understand whats going on ...

So although my release yesterday broke it, it was never a feature :-/ not even RED.util.setObjectProperty handles that.

what to do, what to do!

The issue I see is if the property already exists in the msg object BUT it is NOT an object, it will cause an exception if I try to set a property on it.

I suppose, I could perform a merge...

How does that sound?

mikeS7 commented 3 years ago

I suppose, I could perform a merge...

  • if the result msg property exists and it is an object?

    • merge parser result with existing msg property
  • else

    • replace the msg property with the parser result object.

How does that sound?

For me it sounds as brilliant idea

Steve-Mcl commented 3 years ago

Ah - a problem.

Because often, people leave input and output as msg.payload this means the offered solution tries to merge the built object and the source data buffer - not good (although it does work, just ike you can add objects and properties to an array - they are all objects after all, its just odd and the extra properties appended to the buffer object arent displayed in the node-red debug output either)

image

image

Unsure where to go with this - can you suggest a solid idea that handles the edge cases?

Steve-Mcl commented 3 years ago

So, I think it has to be as per original.

My reasonings are...

consider the CSV node or the XML node or pretty much any node that returns value in msg.payload - they dont attempt to merge the data (nor do they add to the payload - they replace it)

So while I think it was a good idea, in practice, it would be a bit off standard & perhaps a bit counter intuitive.

you can certainly set the result to msg.payload.header.a_different_object and msg.payload.header.test will remain untouched.

Let me know your thoughts..

Cheers.

mikeS7 commented 3 years ago

If input and output properties are same ... then it should be replaced. If they are different ... I prefer to merge (update), but may be simply add checkbox for choice.

About the result of typeof, yes it's complicated, the only way I see is checking for all possible types: isArray, isBuffer, ==null etc. But it will be user's responsibility to set property as an object in case if there will be checkbox for merging .

There are a lot of "if", in my usecase I can add properties after parsing

Feel free to close the issue. Thank you

mikeS7 commented 3 years ago

Original issue is solved No reason to continue