Not able to publish any new metric via NDATA or NCMD which is not available in NBIRTH.

SeppPenner / SparkplugNet

SparkplugNet is a library to use the Sparkplug industrial IoT (IIoT) standard in .Net. It uses MQTTnet in the background.

MIT License

84 stars 42 forks source link

Not able to publish any new metric via NDATA or NCMD which is not available in NBIRTH. #85

Open malavvvakharia opened 8 months ago

malavvvakharia commented 8 months ago

Hello @SeppPenner , According to the sparkplug specification, we can also send the not known metric in NDATA and NCMD. However, from the code, it is not possible as it will filter the data and remove all unknown metrics before publishing NCMD or NDATA.

SS of code:

SS of specification:

SeppPenner commented 8 months ago

Basically a duplicate of #44 and #53.

malavvvakharia commented 8 months ago

Yes, it's partially connected with both tickets but not directly because in this ticket I mentioned that if you add any new metric that is not available in NBIRTH and try to publish that metric via NDATA or NCMD it will not pass it to HostApp as it's an unknown metric which you are filtering via the above-mentioned method.

SeppPenner commented 8 months ago

For me, the spec is misleading in this case. E.g. check DBIRTH:

6.4.20. DBIRTH The DBIRTH is responsible for informing the Host Application of all of the information about the device. This includes every metric it will publish data for in the future.

Same for NBIRTH. For me, this is not concludent whether all metrics need to be known in advance or could be added like this...

SeppPenner commented 8 months ago

I have asked the tahu team in https://github.com/eclipse/tahu/issues/370, but I guess I have an idea how this should be read:

Imagine a scenario with this library and another one that speak Sparkplug. Let's say the other library allows to send unknown metrics on NDATA or DDATA --> Theoretically, this library would need to tell these nodes to do a rebirth because the metrics are unknown. Therefore, the filtering / excluding of unknown metrics in this library is correct.

malavvvakharia commented 8 months ago

@SeppPenner , According to this ticket: https://github.com/SeppPenner/SparkplugNet/issues/58 and https://github.com/SeppPenner/SparkplugNet/pull/61 I think you have removed the logic of requiring the known metrics and also validating the same.

For me, the spec is misleading in this case. E.g. check DBIRTH:

6.4.20. DBIRTH The DBIRTH is responsible for informing the Host Application of all of the information about the device. This includes every metric it will publish data for in the future.

Same for NBIRTH. For me, this is not concludent whether all metrics need to be known in advance or could be added like this...

adityashahazilen commented 8 months ago

Therefore, the filtering / excluding of unknown metrics in this library is correct.

Agree with your statement "Theoretically, this library would need to tell these nodes to do a rebirth because the metrics are unknown". However as per sparkplug specification library should send rebirth request to edge node from host application not filter unknown metric. I would not agree on statement "Therefore, the filtering / excluding of unknown metrics in this library is correct."

malavvvakharia commented 8 months ago

@SeppPenner , have you checked it? Do you have any updates on it?

SeppPenner commented 8 months ago

I will keep this open until the TAHU team answers, but for the moment I will go with filtering.

zhcdorn commented 8 months ago

We are also thinking about Sparkplug and as we are .NET team interested in this library as it seems to have become really applicable over last weeks thanks to all your contributions. However, for us, in order that it is "scalable" we need this filtering deactivated. Knowing all metrics in advance creates a huge dependency on metrics, and as we have edge nodes with changing metrics (e.g. new versions of edge nodes) we need to "auto update" metrics anytime, anywhere.

Can we not agree on a flag to allow either case? (filtering on/off)

zhcdorn commented 7 months ago

I will keep this open until the TAHU team answers, but for the moment I will go with filtering.

Any news from TAHU team so far? Thx

adityashahazilen commented 6 months ago

Any update from TAHU team ? I see we are at now version 1.8.0 but we cannot use it with this filter.

SeppPenner commented 4 months ago

This is valid. From https://github.com/eclipse/tahu/issues/370:

Known means known. If you send an NDATA/DDATA with a new Metric, that would be invalid and the Host Application will require you to send a new NBIRTH/DBIRTH that contains your new Metric in order to move forward. In other words, sending an NDATA/DDATA with a new metric is wrong and should not be done.

So, to clearify this again: The behaviour is expected and if you want to send new / unknown metrics, you have to do a device death / device birth or node death / node birth aka "rebirth" with the new metrics list in it.

--> For the project, this means that I will not change the behaviour of the filtering, but will add a new method like "Rebirth" for nodes and devices to simplify the process.