Clients unable to parse published propertyset

[oceanic76]

At birth or ndata the published propertyset type of a SparkplugNet.VersionB.SparkplugNode cannot be parsed by MQTT.fx (Sparkplug Decoder) or Node-RED Sparkplug node . Tried 1.3.4 and 1.3.3 using both SparkplugMqttProtocolVersions and SparkplugSpecificationVersions. The objects appear okay up to the serialization and then I cannot tell if it is serialized correctly.

Code to build Metric object:
var ps = new PropertySet();
ps.Keys.Add("A");
var aValue = new PropertyValue(DataType.String, "testValue");
ps.Values.Add(aValue);
var m1 = new Metric("TestPropertySet", DataType.PropertySet, ps);

Node-RED: "Unable to decode Unknown message. the error is : TypeError: Cannot read properties of undefined (reading 'keys')"

[SeppPenner]

Maybe a spelling issue in the library (or a Proto member name is set to invalid). I need to check this.

[oceanic76]

The Sparkplug 3 specification document does not list PropertySet as a oneof data type for a Metric's value. The PropertySet (20) type is referenced for the "Properties" field. Using the Eclipse proto in an independent C# project, the clients also failed to parse the data. However, when the Metric's data type is set to string and a string value, then the message can be parsed by the client and the properties are present as children of the "properties" item. I have not figured out how to do it with SparkplugNet.

This is not how I expected the PropertySet to work, however I also have not used it before.

{
"metrics": [
{
"name": "testMetric",
"dataType": "String",
"properties": {
"testKey": {
"type": "String",
"value": "testValue"
}
},
"value": "test"
}
],
"seq": 0
}

[shouidar]

Yes, I looked at the eclipse tahu java implementation and the PropertySet is not part of the Metric value. It is used in the properties field (9) of the Metric itself as mentioned in the spec: ( This is a PropertySet object associated with the metric for including custom key/value pairs of metadata associated with a metric. This is covered in the). This is also true for version 2.2 version of the spec.

[SeppPenner]

I'm not sure what the issue is here. We internally store the data in the metric's value, but when converting from / to Protobuf, it's stored in the Properties and taken from the properties. Check

SparkplugNet/src/SparkplugNet/VersionB/PayloadConverter.cs

Line 270 in 6929ea4

protoMetric.PropertySetValue = ConvertVersionBPropertySet(metric.Value.ConvertOrDefaultTo<PropertySet>());

and

SparkplugNet/src/SparkplugNet/VersionB/PayloadConverter.cs

Line 127 in 6929ea4

metric.SetValue(VersionBDataTypeEnum.PropertySet, ConvertVersionBPropertySet(protoMetric.PropertySetValue));

And this is the property set in the metric:

SparkplugNet/src/SparkplugNet/VersionB/ProtoBuf/ProtobufPayload.cs

Line 330 in 6929ea4

public PropertySet? PropertySetValue

[SeppPenner]

I can't reproduce this... By the way, this has nothing to do with SparkplugMqttProtocolVersion or SparkplugSpecificationVersion (Just because it was mentioned).

I have added a test for the use case, but can't reproduce: 07a49ac. The test can be found here:

SparkplugNet/src/SparkplugNet.Tests/Payloads/SparkplugPayloadConverterTestVersionB.cs

Line 1848 in 07a49ac

public void TestPropertySetConversion()

.

[SeppPenner]

@shouidar I don't want to disturb you, but since you seem to be the person to ask for that kind of tricky stuff, do you see where the issue is located?

[shouidar]

As I mentioned above the property and propertySet are not value objects for then metric, they are part of the Metric property fields (like name, alias etc...) on the spec there is a properties field that can take a propertyset.

I have already tested this on my local repo: I just added a Properties field to the Metric class and removed all the propertySet value from the value property.

public MetaData? MetaData { get; set; }

/// <summary>
/// 
/// </summary>
public PropertySet? Properties { get; set; } 
/// <summary>
/// Gets the value.
/// </summary>
public override object? Value => base.Value ?? this.GetValue();

This was decoded properly by MqttFx as discribed here: #80 (comment)

I can submit a pull request for this if you like.

[SeppPenner]

@shouidar Yeah, but it's only stored in the value in the internal metric for simplicity reasons. When converting to / from Proto, the converter uses the Properties field in the Proto metric. Check out my test for this, please. This should work properly if you ask me.

[shouidar]

@SeppPenner sorry did not check the last update. I will check and update here.

[shouidar]

I think the Properties filed should be stored separately as a Metric can have both value and Properties. The purpose of the properties filed in my understanding of the spec and as I could see on the Tahu reference implementation is to provide additional details about the metric and is not a value by itself.

I checked by adding the metric to list of metrics in the test application (see code bellow).

private static readonly List<VersionBData.Metric> VersionBMetricsNode =
    [
        new VersionBData.Metric("temperatureNode", VersionBData.DataType.Float, 1.243f),
        new VersionBData.Metric("climateactiveNode", VersionBData.DataType.Boolean, true),
        new VersionBData.Metric("Test22", VersionBData.DataType.PropertySet, new VersionBData.PropertySet
        {
            Keys = ["Test1", "Test2"],
            Values =
            [
                new(VersionBData.DataType.Int8, 1)
                {
                },
                new(VersionBData.DataType.Int64, (long)2)
                {
                }
            ]
        })
    ];

The payload is serialized as expected with the Properties field set to the given PropertySet, and the DataType 20. as can be seen on screenshot below: (payload decoded by Protobufpall.

But because other sparkplug clients do not expect a Metric of Datatype 20, they fail to decode it:

[shouidar]

By adding the Properties field to the Metric class:

/// <summary>
    /// Gets or sets the Properties field.
    /// </summary>
    public PropertySet? Properties { get; set; }

And assigning it to the PropertySetValue during conversion (independently of the Metric value which is still converted separately depending on the metric DataType):

var protoMetric = new VersionBProtoBuf.ProtoBufPayload.Metric()
        {
            Alias = metric.Alias,
            DataType = (uint?)ConvertVersionBDataType(metric.DataType),
            IsHistorical = metric.IsHistorical,
            IsNull = metric.IsNull,
            IsTransient = metric.IsTransient,
            MetaData = ConvertVersionBMetaData(metric.MetaData),
            Name = metric.Name,
            Timestamp = metric.Timestamp,
            PropertySetValue = ConvertVersionBPropertySet(metric.Properties.ConvertOrDefaultTo<PropertySet>())
        };

Then creating the metric as follows:

new VersionBData.Metric("climateactiveNode", VersionBData.DataType.Boolean,null)
        {
            Properties = new VersionBData.PropertySet
            {
                Keys = ["ON", "OFF"],
                Values =
                [
                    new(VersionBData.DataType.Int8, 1)
                    {
                    },
                    new(VersionBData.DataType.Int8, 0)
                    {
                    }
                ]
            }
        }

It gets decoded properly by MqttFx:

[SeppPenner]

So, the main problem is the data type... (Ok, and theoretically that a metric could have a value and a property set at the same time)...

I agree, we can change that. Since you already seem to have a working version, would you mind to put it into a PR @shouidar?