Clarification on the proto definition in alignment with SparkplugB specs
While studying the sparkplugB specification and the protofile created I had a couple of doubts
-
The SparkplugB spect allows for Array types to be defined
◦ [tck-id-payloads-metric-datatype-value] The datatype MUST be one of the enumerated values as shown in the valid Sparkplug Data Types.
But the proto spec specifies the value as oneof.
oneof value { uint32 int_value = 10; uint64 long_value = 11; float float_value = 12; double double_value = 13; bool boolean_value = 14; string string_value = 15; bytes bytes_value = 16; // Bytes, File DataSet dataset_value = 17; Template template_value = 18; MetricValueExtension extension_value = 19; }should there not be additional entries which
repeatedphrase against the value? Probably something like// Array value fields repeated uint32 int_values = 20; repeated uint64 long_values = 21; repeated float float_values = 22; repeated double double_values = 23; repeated bool bool_values = 24; repeated string string_values = 25; repeated bytes bytes_values = 26; repeated DataSet dataset_values = 27; repeated Template template_values = 28;I am still trying to learn the protobuf programming so not yet sure on the exact code
-
do we have to give different names to the value i.e. int_value, string_value etc. can we not have just one field name "value" like it is specified in the sparkplug_b.json
-
Are there any plans to upgrade from proto2 to proto3?
Not a maintainer of this repo but I can answer the first two.
- In section 6.4.17 "Datatype Details" (page 80) of the Sparkplug B specification, it explains that the array types are all to be encoded as an array of bytes. They are stored as a single entry in the
bytes_valuefield. This is stored alongside a type indicating what type of array it is so you can know how to decode the bytes. - Protobuf fields must have one of the types from this table https://protobuf.dev/programming-guides/proto2/#scalar, think of it like programming in a strongly typed language like C. Therefore the way to implement a value field with multiple types is to make it
oneoffield, with all the potential types as subfields.
Thanks for the clarification, This opens a second set of questions regarding the arrays
- for the Int8Arrays , Int16Array, Int32Array, Int64Array do we need to convert the signed integers into it's unsigned values? Currently it looks like from the spec this doesn't need to be done.
- In the specs for the example given for Int8Array [-23, 123] document mentioned the boolean array to be [0xEF, 0x7B] which appears wrong, should it not be [0xE9, 0x7B]
- Yes, you can see that being done in the python example here.
- Yes that is wrong, there a few errors in that part of the spec. Documented in the Sparkplug repo and will be fixed in the next release.
Thanks I went that and found some limitations as well as a lack of unit tests Hence In created an alternative implementation https://github.com/mkashwin/unifiednamespace/blob/main/02_mqtt-cluster/src/uns_sparkplugb/uns_spb_helper.py WIth the associated test cases in https://github.com/mkashwin/unifiednamespace/tree/main/02_mqtt-cluster/test
Key updates
- comprehensive unit tests based on the specs
- simplified the encoding and decoding of values by encapsulating the logic in the associated enum for the data type
- works with Python 3.12
- support for PropertySets, PropertySetLists, DataSets, Templates as well as Arrays and metadata for the metrics If that adds value to this project please feel free to use it or if you deem it good enough to raise a PR let me know.
If the expert group is in the phase of upgrading the specs I would recommend the following enhancements
- Use the field
datatypeconsistently instead oftypeandtypes(for datasets perhaps datatypes wouls be good) - update the specs for the test data including details of converting bytes from little to big endian for Float and Double Arrays
- A few more elaborating and clarifications on the propertyset usage in the specs will be helpful.
We're always open to contributions/PRs ;)
Will try to do that by next weekend. Is it ok for the code to be python 3.12 compliant? The current code did not seem to have any limitation ( other than python 3)?