Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MQTT-Explorer 0.4.0-beta4 SparkplugB decode some strings as "{}" #792

Closed
LoicGoulefert opened this issue May 13, 2024 · 13 comments · Fixed by #793
Closed

MQTT-Explorer 0.4.0-beta4 SparkplugB decode some strings as "{}" #792

LoicGoulefert opened this issue May 13, 2024 · 13 comments · Fixed by #793
Labels
bug Something isn't working Priority

Comments

@LoicGoulefert
Copy link

In the latest release of MQTT-Explorer, some payloads seem to be decoded by SparkplugB when they're not meant to be.
This results in some topics to display "{}" instead of the actual payload.

@kc2zgu mentionned here that some strings (like 5000) are detected as SparkplugB payloads. I can't reproduce this in my application, but the string "empty" gets decoded by SparkplugB, and rendered as {} in MQTT-Explorer. Turns out that strings of length 5 starting with "e" are decoded wrongly, based on a few manual tests.

@kc2zgu
Copy link

kc2zgu commented May 13, 2024

5-character strings starting with "5" are what I noticed. So "50000" or "50.00" but not "5000"

@LoicGoulefert
Copy link
Author

True, did some more tests and all the strings that you mentioned turned into {} in MQTT-Explorer. I can't count 😞

@bj00rn
Copy link
Collaborator

bj00rn commented May 14, 2024

@thomasnordquist this is a release stopper i guess, sparkplugb decoding "detection" seems to be flaky..

image

The decoder choice logic is pretty naive at the moment it seems, it simply tries to decode the payload as sparkplug and if the result is !== undefined we return it. Any suggestion on how to improve the logic is welcome.

payload: SparkplugDecoder.decode(buffer) ?? Base64Message.fromBuffer(buffer),

A naive solution might be, to add a check to see if Object.keys(message).length === 0 here, and return undefined here? I lack deeper understanding of protobuf decoding, maybe there is a way to inspect the payload to determine if it's sparkplugb.

const message = Base64Message.fromString(

@LoicGoulefert
Copy link
Author

Tried to run some more tests, here's what i found:
Strings starting with any of the following characters: 'e', 'E', 'u', 'U', 'm', 'M', 5, with a length that is a multiple of 5, will be decoded as {}.

@thomasnordquist
Copy link
Owner

Thank you, I consider this rather critical.
Maybe we can use part of this implementation #493 to make the decoding customizable.

I believe currently we run a decoder chain which is kind of broken

tryToDecodeAsJson
else tryToDecodeAsSparkplugB
else ....

@bj00rn
Copy link
Collaborator

bj00rn commented May 14, 2024

Thank you, I consider this rather critical. Maybe we can use part of this implementation #493 to make the decoding customizable.

I believe currently we run a decoder chain which is kind of broken

tryToDecodeAsJson else tryToDecodeAsSparkplugB else ....

Thank you, I consider this rather critical. Maybe we can use part of this implementation #493 to make the decoding customizable.

I believe currently we run a decoder chain which is kind of broken

tryToDecodeAsJson else tryToDecodeAsSparkplugB else ....

Would be awesome to prepare for image support to while we are at it.
#734

@jst-mbs
Copy link

jst-mbs commented May 15, 2024

I lack deeper understanding of protobuf decoding, maybe there is a way to inspect the payload to determine if it's sparkplugb.

As someone that has knowledge about Sparkplug but not about JS/TS, I would suggest to inspect the topic structure to detemine if the message is sparkplugb. In my opinion, relying only on the message payload is a bad idea. The sparkplug specifikation can be found here: https://www.eclipse.org/tahu/spec/sparkplug_spec.pdf. The chapter 4.1 is describing the topic structure:

All MQTT clients using the Sparkplug specification MUST use the following topic namespace structure:

namespace/group_id/message_type/edge_node_id/[device_id]

The namespace element must be spBv1.0.

@tobehn
Copy link

tobehn commented May 15, 2024

I think @jst-mbs has a pretty good point.
spBv1.0 will always be in the topic of the message. It is at least a good way to double check if decoding might need to be used.
Though in some cases people send uncompressed messages to the spBv1.0 namespace. For example i do not know if all STATE messages in SparkplugB are send protobuf compressed.

@jst-mbs
Copy link

jst-mbs commented May 15, 2024

Though in some cases people send uncompressed messages to the spBv1.0 namespace. For example i do not know if all STATE messages in SparkplugB are send protobuf compressed.

You are right: The state messages of the 'Host application' is JSON encoded, see chapter '4.2 Sparkplug Host Application'.

@bj00rn
Copy link
Collaborator

bj00rn commented May 16, 2024

Sidenote, there is a package sparkplug-payload that can do decoding/encoding. Might be worth looking into instead of rolling own protobuf decoding.

@bj00rn
Copy link
Collaborator

bj00rn commented May 17, 2024

I think @jst-mbs has a pretty good point. spBv1.0 will always be in the topic of the message. It is at least a good way to double check if decoding might need to be used. Though in some cases people send uncompressed messages to the spBv1.0 namespace. For example i do not know if all STATE messages in SparkplugB are send protobuf compressed.

@tobehn @jst-mbs have a look at #793, Will (topic.match(/spBv1\.0\/[^/]+\/(DDATA|NDATA|NCMD|DCMD|NBIRTH|DBIRTH|NDEATH|DDEATH\/[^/]+\/)/u)) regexp suffice to identify encoded sparkplug payloads?

@tlifschitz
Copy link

Just for the record, in our application we use custom protobuf messages and the wrongly decoded messages we are seeing are not empty but with random invalid data, as shown below.

Hope @jst-mbs proposal and @bj00rn implementation fixes this issue, thanks!

Payload 26 bytes
b'*\x18\x1a\x16\x08\x0e\xf2\x01\x11\x08\x01\x12\r\x08\x03\x10\x01\x18\x02 \x02HUP\xf0.'
Decoded Sparkplug(?)
{"body":{"0":26,"1":22,"2":8,"3":14,"4":242,"5":1,"6":17,"7":8,"8":1,"9":18,"10":13,"11":8,"12":3,"13":16,"14":1,"15":24,"16":2,"17":32,"18":2,"19":72,"20":85,"21":80,"22":240,"23":46}}

Payload 36 bytes
b'\x12\x0fwmod-din-v0.08b*\x11f/wmod-din-v0.08b'
Decoded Sparkplug(?)
{"metrics":[{"isHistorical":true,"isNull":true,"doubleValue":3.0644533518218016e-75,"booleanValue":true}],"body":{"0":102,"1":47,"2":119,"3":109,"4":111,"5":100,"6":45,"7":100,"8":105,"9":110,"10":45,"11":118,"12":48,"13":46,"14":48,"15":56,"16":98}}

image

@bj00rn
Copy link
Collaborator

bj00rn commented May 21, 2024

Just for the record, in our application we use custom protobuf messages and the wrongly decoded messages we are seeing are not empty but with random invalid data, as shown below.

Hope @jst-mbs proposal and @bj00rn implementation fixes this issue, thanks!

Payload 26 bytes b'*\x18\x1a\x16\x08\x0e\xf2\x01\x11\x08\x01\x12\r\x08\x03\x10\x01\x18\x02 \x02HUP\xf0.' Decoded Sparkplug(?) {"body":{"0":26,"1":22,"2":8,"3":14,"4":242,"5":1,"6":17,"7":8,"8":1,"9":18,"10":13,"11":8,"12":3,"13":16,"14":1,"15":24,"16":2,"17":32,"18":2,"19":72,"20":85,"21":80,"22":240,"23":46}}

Payload 36 bytes b'\x12\x0fwmod-din-v0.08b*\x11f/wmod-din-v0.08b' Decoded Sparkplug(?) {"metrics":[{"isHistorical":true,"isNull":true,"doubleValue":3.0644533518218016e-75,"booleanValue":true}],"body":{"0":102,"1":47,"2":119,"3":109,"4":111,"5":100,"6":45,"7":100,"8":105,"9":110,"10":45,"11":118,"12":48,"13":46,"14":48,"15":56,"16":98}}

image

Thanks, there is clearly a bug with sparkplug decoding. Hopefully we'll get a new beta sorted soon that you can try.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants