search

SeppPenner / SparkplugNet

SparkplugNet is a library to use the Sparkplug industrial IoT (IIoT) standard in .Net. It uses MQTTnet in the background.
MIT License
83 stars 42 forks source link

Question: Application discovering new metrics #43

Closed Kvargefar closed 7 months ago

Kvargefar commented 1 year ago

Hi,

Validation of incoming metrics

I've been working a little with the library for a while. I noticed that the application ignores metrics that do not already exist in KnownMetrics through DataReceived. I therefore wonder if the only way for the application to discover new metrics from a fresh or old node/device is through birth? I will either have to ask for rebirth when I come online, or store old metrics in a file and use old known metrics in the application's constructor?

DeviceStates and NodeStates

Moreover, the DeviceStates and NodeStates dicts store all nodes and devices discovered, with their belonging MetricState - which is great. But I noticed that the key for these dicts is a simple string representing the DeviceId or NodeId. It doesn't seem to store additional information about these. If you take a device as an example, it only stores the device identifier, but not the EoN identifier - which you need if you want to target this device with a command, for instance. Unless this info is possible to get somehow? For my case, I want to store all metrics in a database. I might have to append the metric name to the device or node it originated from.

Kvargefar commented 1 year ago

It seems like this line of code makes the application ignore incoming data. How can the application discover new nodes and new data, if it ignores DDATA and NDATA payloads? The application needs to be able to discover new nodes and devices, and get their name, to be able to ask for a rebirth.

this.KnownMetricsStorage.ValidateIncomingMetrics(metricsWithoutSequenceMetric);

SeppPenner commented 1 year ago

For the first question: According to 2.2 spec, this is by design, check out https://github.com/SeppPenner/SparkplugNet/issues/6#issuecomment-1044441756 as well. No idea how this is according to the 3.0 spec or if it's better specified in there. Need to check this.

For the second question: You are right. Seems to be a good idea to store the EON node identifier there, too. I need to rework the library according to 3.0 spec either way, so I hope to check all the open issues within this step.

BoBiene commented 1 year ago

You can provide your own metric storage and implement a custom logic to validate the metric in your client code.

SeppPenner commented 1 year ago

After reading through the specs, this is according to the 3.0 version as well: (Excerpt from page 21 which is PDF page 28):

The NBIRTH MUST include every metric the Edge Node will ever report on. At a minimum each metric MUST include the metric name, datatype, and current value. If Template instances will be published by this Edge Node or any devices, all Template definitions MUST be published in the NBIRTH.

So, to be clear: The library works according to the spec and I'm not willing to change this.

Storing the EON node identifier in the dictionary key is ok, I will implement that.

EDIT: Maybe the requested feature to trigger a device / node rebirth is an option (I need to check whether this is allowed by the spec or not).

EricRosenfeld1 commented 12 months ago

Correct me if I'm wrong, but don't the specs indicate that the NBIRTH must include all the metrics the node will report? Right now the SparkplugApplication needs to already know these metrics.

The call to this.KnownMetricsStorage.ValidateIncomingMetrics(metricsWithoutSequenceMetric); needs to exclude NBIRTH topics.

jeff-pf commented 11 months ago

Node birth message retain flag must be set to false - only "Sparkplug Aware" (section 10.1.4 of v3.0.0 spec) brokers retain those messages. There needs to be a method to send rebirth message to nodes that are already online and send data after the application starts for brokers that are not "Sparkplug Aware". Instead of doing this automatically - which does not give the application any control - have the event of data published without a birth message and a method to call to send a rebirth. If the application is interested in that node it can send a rebirth.

jeff-pf commented 11 months ago

@EricRosenfeld1

This call does exclude NBIRTH messages because metricsWithoutSequenceMetric is actually messages without bdSeq metric - which is the birth / death sequence number.

    var metricsWithoutSequenceMetric = payload.Metrics.Where(m => m.Name != Constants.SessionNumberMetricName);
SeppPenner commented 7 months ago

Node birth message retain flag must be set to false - only "Sparkplug Aware" (section 10.1.4 of v3.0.0 spec) brokers retain those messages. There needs to be a method to send rebirth message to nodes that are already online and send data after the application starts for brokers that are not "Sparkplug Aware". Instead of doing this automatically - which does not give the application any control - have the event of data published without a birth message and a method to call to send a rebirth. If the application is interested in that node it can send a rebirth.

This was already done sometime before.

SeppPenner commented 7 months ago

Moreover, the DeviceStates and NodeStates dicts store all nodes and devices discovered, with their belonging MetricState - which is great. But I noticed that the key for these dicts is a simple string representing the DeviceId or NodeId. It doesn't seem to store additional information about these. If you take a device as an example, it only stores the device identifier, but not the EoN identifier

I added this.