-
Notifications
You must be signed in to change notification settings - Fork 42
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
NBIRTH into Ignition #5
Comments
I can confirm everything works as expected so far. Encountered issue is Cirrus Link specific: https://forum.cirrus-link.com/t/topic-segment-group-id-fails-to-parse/126/2 |
Can you give me some hints on how I can test this with Ignition? I have tried, but honestly didn't understand how Ignition works properly :D |
Sure.
You should be able to connect to :1883 MQTT broker. If you would like, we can get on a call and go through it. -chris |
Reopening this as it is related. Observation on a working SpB NBIRTH from Ignition built-in example where
Here is an example of non-working SpB NBIRTH from custom client using SparkplugbNet 1.2.0 where
However the same message decoded in MQTT.fx shows
Here is a recent topic on the issue: https://forum.cirrus-link.com/t/nbirth-keeps-getting-rejected-for-missing-sequence/244
I can confirm this observation with Wireshark. |
@SeppPenner does this mean that the proto file needs to be updated? And if so, then that means that SpB spec needs to be updated to pb3. |
Hi, |
I'm looking back at the change logs and it looks like protobuf-net 3.0.73+ has been used since February 16, 2021. I guess my next step is to start downgrading and see when things actually break in Ignition. |
Went to 1.0.0 but ran into a null reference with LWT message UserProperties. |
Ok that was pretty simple after getting lost for a couple hours. SparkplugNet/src/SparkplugNet/VersionB/ProtoBuf/ProtobufPayload.cs Lines 26 to 27 in f3cb22a
Has to include
https://code.google.com/archive/p/protobuf-net/wikis/Attributes.wiki Any side effects anyone can think of?
|
@MRIIOT did the isRequired flag help you? After closer inspection, I can see that all my payloads are accepted, except the NBIRTH. This is the first message and seq=0. Looking at the parsed result in Ignition, I can now see that seq=null. Not sure why. I've been testing different methods, but cannot a solution yet. Other clients read the first seq value as 0. Edit: I now see your mentioned issue. These are probably related. |
I did update protobuf-net to latest. Did you add the parameter to the correct seq? |
It worked. Added it to the wrong payload. I had two windows. Will test more. Thank you so much @MRIIOT :) |
I did the same :) |
Once you test, someone should PR. |
Sorry guys for the late replies. I need to check this (If someone has a clue already if the required flag is used incorrectly somehow), just tell me or add a PR if you want :) Have been quite busy at work the last weeks... |
I need to do more testing. I am seeing other fields besides 'seq' causing issues as well. |
Since Eclipse brought out the TCK kit for testing (3.0 compatibility), I hope to get this done soon and maybe find some more issues as well... |
Been away from this for a couple of weeks now. In regards to publishing/producing, I currently have good results into Ignition after changing the isRequired flag. But I have to do more testing as well. |
Hi, I'm also in trouble with the NBIRTH message into ignition. After reading this exchange I've made the test of setting the "IsRequired" property on sequence as proposed. [global::ProtoBuf.ProtoMember(3, Name = @"seq", IsRequired = true)] it works like a charm. @Kvargefar could you make some extra testing and by the way, can you plan a PR ? Thanks |
I'm now testing with the official V3.0 TCK, Please follow #39 for updates. |
Great work on the library! Have you gotten a clean NBIRTH into Ignition? I am running into Ignition throwing exceptions and will be investigating why that is. Will keep you updated.
The text was updated successfully, but these errors were encountered: