Feature request: PropertySet

[wz2b]

I'm interested in adding support for property sets. The main issue I'm having is when receiving a Sparkplug B message and translating it to InfluxDB - or any other TSDB that makes use of labels - then there's sometimes a need for tags above and beyond the groupId, deviceId etc. in the MQTT topic. Currently all you can really do is to add this metadata using a decorator and another source of static data. The Sparkplug protocol supports PropertySet though and my thought is there must be some way to merge in some static tags, staying compliant with the protocol, and allowing the sparkplug "input" node to split these out for other uses (like influx line protocol serialization).

Any thoughts on this?

[thomassorensen2650]

Are you offering to create a PR with support for PropertySets or is this more like a feature request? It would be nice with a design suggestion on how it would be implemented.

I think it will be pretty straight forward to add support for property sets, the tricky part will be to add the extra functionality to the UI without making it too busy/complicated for the enduser.

[wz2b]

If you think it's a good/useful idea, then sure! I could do either. I'm absolutely willing to submit it as a P.R. but just didn't want to expend the time until I knew it would likely be accepted.

If you want you could enable 'discussions' on this repository and I will propose this in more detail there. That would let us keep the discussion open but let you close this issue.

[wz2b]

the tricky part will be to add the extra functionality to the UI without making it too busy/complicated for the end user.

If you're okay with it, I would like to propose doing this in several small features, with the UI additions coming later (if ever). I'm thinking of this as a more advanced use case and that it would be better to leave it all in JSON for the time being.

I'm proposing two ways to specify these. The first is you feed them into the definition. In this case, the birth message will be emitted with a metric with no value. The value is a oneof so that's permitted sparkplug_b.proto. Note that a property can contain a type but it's optional. If one isn't specified, I'd like to assume the type is string.

msg = {
   ...msg,
   definition: {
      gasmeter: {
         dataType: "Int64",
      },
   },
   payload: {
      metrics: [
         {
            "name": "airmeter1",
            "properties": {
                 "meter": "abcd1234",
                 "manufacturer": { type: "string", value: "itron" }
            }
         }
      ]
   }
}         

The second use case would be to emit it with every record:

msg.payload = {
   metrics: [
      name: "airmeter1",
      properties: {
         meter: { type: "string", value: "abcd1234" },
         manufacturer: "itron"
      }
   ]
};

My thought here is that if you do emit the tags as part of the birth message that the receiving end could decide to attach these tags to every record before they get written to the database. The 'device' node would remember them, just like it remembers that the type of the metric is an "Int64." I need to think this through because maybe there are better ideas. What I'd like to avoid is having to send the properties with every single metric update - even though I think that's the intent of the standard. I don't see in the spec the behavior that I would like - that if you get a metric update, you apply the properties from the birth message. Does your read of the spec allow for this?

[wz2b]

Digging through all this source code is making me wonder why this doesn't work already. Maybe it does. If it does, all I'm going to do is make doc changes.

[thomassorensen2650]

I have only seen property sets used on birth, I don’t know if it will add any value to attach properties in every DATA message (and if its even supported). We need to verify that this is supported if we choose to go that route.

I can see you converted the key/value array to Objects in our example. It looks clean, but a easier way would be set the properties in a proper “property set” format in the definition:

msg = {
    "definition" : {
        "TEST/TEST" : {
            "dataType" : "Int32"
            "properties": {
                "keys": [
                    "engUnit",
                ],
                "values": [
                    {
                        "type": "string",
                        "value": “%”
                    }
                ]
            }
        }
    },
    "payload" : {
        "metrics" : [
        {
            "name" : "TEST/TEST",
            "value" : 5
        }]
    }
};

This would be a very simple change, and I think you are right - It should already work.

Here are the changes that I see:

1. input validation to the properties object:
   1. keys.lenth === values.length
   2. All values have a type (we can add type if it does not exist?)
   3. All values have a value attribute.
2. Add unit test
3. Update documentation

[thomassorensen2650]

And good point about the UI - I agree that this is an advanced feature that we should not add to the UI.

[wz2b]

I have only seen property sets used on birth

You seem to have the experience that I lack - namely, how properties get used in the real world. I'm writing my own senders and receivers, which means I'm subjecting myself to symmetrical errors and assumptions. I wrote both ends, so of course it works :) That doesn't mean somebody else's implementation won't get confused by it.

I think what I'd really like is to put them in the DBIRTH message, then whenever I receive a message I attach (copy) the properties from DBIRTH.

I don’t know if it will add any value to attach properties in every DATA message (and if its even supported).

It's definitely supported by the .proto but that's because the .proto doesn't seem to make any distinction between the encoding of a DBIRTH vs a DDATA.

That doesn't mean that my idea is the right thing to do, though. If it's okay in the .proto is it implicitly OK per the spec? I feel like caching properties from DBIRTH does seem like it's behavior that isn't addressed by the spec, so I'm getting into shaky territory doing that.

[wz2b]

Regarding this:

"properties": {
   "keys": ["engUnit"],
   "values": [ "volts" ]
}

Maybe. What you are suggesting definitely matches the proto. But when I actually tried this, I injected this (once):

{
   "definition": {
      "test": {
         "dataType":"Int32"
       }
   },
   "payload": {
       "metrics": [
          {"name":"test","properties":[{"name":"engUnits","type":"string","value":"inHg"}
       ]
   }
}

and that actually works as-is.

[wz2b]

I had this wrong. The metrics don't get encoded with a name, they're just properties:

{
    "metrics": [
        {
            "name": "test",
            "properties": {
                "engUnits": {
                    "type": "string",
                    "value": "inHg"
                }
            }
        }
    ]
}

works correctly with the encoder and decoder.

I think what's bothering me here is that there isn't an 'alias' mechanism for these properties, so you send not just the property value but also the key every time. It doesn't feel very efficient to send static properties with every metric publication, let alone their keys.

Currently this library doesn't support metric aliases anyway, as far as I can tell. What this means is that if you encountered a device that did use aliases it wouldn't work. If someone wanted to use this library with one of those devices, I think they would have to subscribe to both DBIRTH and DDATA and then add the logic to replace the alias with the property name themselves, which still doesn't work right - for this to work you have to enable 'retain' so that you can get the DBIRTH even if it went by before you connected. I tried this but it didn't work. I think for this to work you would have to set retain: true on DBIRTHs and I don't think you do.

[wz2b]

I keep finding parts of the spec I hadn't paid attention to before. There IS a way to set device-wide properties:

The DBIRTH message can also include optional ‘Properties’ of a device. The following are examples of Property metrics.
• Metric name: ‘Properties/Hardware Make’
o Used to transmit the hardware manufacturer of the device
• Metric name: ‘Properties/Hardware Model’
o Used to transmit the hardware model of the device
• Metric name: ‘Properties/FW’
o Used to transmit the firmware version of the device

Basically what this is saying is 'send them as metrics in the DBIRTH'

The trouble is, what if you miss the DBIRTH? The spec doesn't say a DBIRTH should be sent with retain=true but it seems to me it should ?

Update: the section of the spec that describes "SCADA Host State" has this:

The Birth Certificate Payload is the UTF-8 STRING “ONLINE”. The RETAIN flag for the Birth Certificate is set to TRUE, and the Quality of Service (QoS) is set to 1

[thomassorensen2650]

You are right, here is the code that convert the object to the key/value pair is already in the Sparkplug-payload package: Link

This currently works if properties are defined in payload. This is not the functionality we want, we want to define it in the definition, and then only send on DBIRTH.

Looks like we need a small code change after all.

This message should work:

msg = {
    "definition": {
        "TEST/TEST": {
            "dataType": "Int32",
            "properties": {
                "engUnits": {
                    "type": "string",
                    "value": "inHg"
                }
            } // properties end
        } // Metrics end
    },
    "payload" : {
        "metrics" : [
            {
                "name": "TEST/TEST",
                "value": 5,

            }]
    }
};

[thomassorensen2650]

If you miss DBIRTH then you need to send a REBIRTH, that's what most clients are doing.

[wz2b]

If you miss DBIRTH then you need to send a REBIRTH, that's what most clients are doing.

There is a way for a client to send a REBIRTH request ?

[thomassorensen2650]

Yes you can send a NCMD to write the follow tag:

metrics : [{
   "name" : "Node Control/Rebirth",
   "type" : "Boolean",
  "value": true
},

This will issue rebirth of the Node and all connected Devices.

This feature is "optional" per the specification, but most (if not all) clients require this feature to function correctly so it's definitely the correct way to handle situations like this.

[thomassorensen2650]

Back to property set support; I just looked though the code, and we need to modify the trySendBirth method to send a merged object with data from this.latestMetrics and properties from this.metrics to node.brokerConn.createMsg.

That should be all we need to do as far as I can see.

[wz2b]

If you miss DBIRTH then you need to send a REBIRTH, that's what most clients are doing.

I'm just documenting so nobody gets confused by my ignorance about this. I was questioning why doesn't the BIRTH message get sent as retained=true so clients with clean=false would get them immediately upon subscription. Turns out that's not allowed:

https://github.com/eclipse/sparkplug/blame/develop/specification/src/main/asciidoc/chapters/Sparkplug_4_Topics.adoc#L361

so what you suggested - sending the request as an NCMD - is the right way.

[wz2b]

Back to property set support; I just looked though the code, and we need to modify the trySendBirth method to send a merged object with data from this.latestMetrics and properties from this.metrics to node.brokerConn.createMsg.

The way I read this is that if someone sends metrics in with the birth message, then those are device properties. Those are the only ones that should be saved and sent again on a rebirth. I think if metrics come with a definition those need to be saved separately, and those are the only ones that get sent again in a rebirth. If someone wants to change one of those, they could send another 'definition.'

I'm thinking the change that's needed is to change the documentation to say that any metrics that come with a definition are device properties; cache/save those separately; send them again on a rebirth; and state that if someone wants to change those they send the definition again (which causes a rebirth, which is I think what you want). Fair?

The DBIRTH message can also include optional ‘Properties’ of a device. The following are examples of Property metrics.
   • Metric name: ‘Properties/Hardware Make’
      o Used to transmit the hardware manufacturer of the device
   • Metric name: ‘Properties/Hardware Model’
      o Used to transmit the hardware model of the device
   • Metric name: ‘Properties/FW’
      o Used to transmit the firmware version of the device

[thomassorensen2650]

Ok, I have have added a PR with metric property support, I'll include that in the next version. Metrics properties are different than "Properties metrics". You can use metrics properties with the version today as they are just normal metrics. Metric properties are used for meta data about the metrics. Engineering units is a good example of a metric property that most client support.