Missing or incorrect definitions for Metric message

[lothar7]

The proto definition of the Metric message seems to be missing or have incorrect definition.

The int_value is defined as uint32, long_value is defined as uint64 and there is no corresponding signed int or long value.
The specification defines that the datatype field for the metric message can be all the supported data types described by the standard - including all variants of int and long.

Here is the part of the metric message

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;
} 

Ideally the int_value and long_value should have their datatype changed since it seems misleading to call it int_value while the datatype is unsigned. In addition the message format should be updated to include signed versions of int and long as a minimum. for example

oneof value 
        {
...

            int32   int32_value                      = 20;
            int64   int64_value                 = 21;          
        }

Or have I misunderstood something here as to why the proto specification is defined like this?

[lothar7]

I see now there are several postings related to this topic with the uint vs int problem
eclipse-tahu/tahu#131
eclipse-tahu/tahu#71

As far as I am concerned a new version of the standard should be created which uses the appropriate data types and which is not misleading. The problem now is that one has to transmit negative values either as uints and the receiver has to "know" that it must be convert back to int again. Or negative values must be sent as floats...

If signed members are added to "oneof" description with new names then the standard will be backwards compatible to a certain extent when receiving data. Obviously sending data will no longer be compatible and one has to have separate checks in the code for talking to older Sparkplug B "servers"

[I-Campbell]

The correct format is described fully in the standard.
The receiver "knows" how to interpret the int_value and long_value, because it has the mandatory Metric.datatype field.

See develop branch, chapter 6,
"datatype
...This MUST be included in Metric Definitions in NBIRTH and DBIRTH messages"

In the same chapter under "Datatype Details" it says for example:
"Int32
Signed 32-bit integer
Google Protocol Buffer Type: uint32"

So here you would interpret the 32 bits received in int_value as an Int32

[lothar7]

I dont see why you wouldn't want to use the protocol buffer types for int32 or sint32 for signed data. Sending everything as unsigned makes little sense to me.

Doesn't this mean if wanting to transmit an negative value in a metric which is serialized to JSON (instead of a binary format) then the payload can no longer be interpreted directly. Converting the value to unsigned and serializing to json will give you a different value than the original unsigned value or am I missing something here?

When serializing to json a payload for a value of '-1' with datatype 3 (int32) then it looks like

{
	"metric": [{
			"name": "sdf",
			"alias": "1",
			"datatype": 3,
			"isHistorical": true,
			"intValue": 4294967295
		}
	]
}

This to me seems rather awkward. There should have been a good way of sending that value as -1 directly.

Here is the c# sample code for the payload example above

var payload2 = Payload();
payload2.Metric.Add(new Payload.Types.Metric()
                {
                    Name = "sdf",
                    Alias = 1,
                    IntValue = unchecked((uint)-1),
                    Datatype = (uint)SparkPlugDataTypeEnum.Int32,
                    IsHistorical = true
                }); ;
                var jsonMessage = JsonFormatter.Default.Format(payload2);

[jbrzozoski]

There is a related issue open on the project for maintaining the specification document:
#62

[wes-johnson]

We've decided not to make any changes to the specification for this release. However, the Java implementation had an issue with encoding that is now fixed: eclipse-tahu/tahu#202. This issue has now been deferred to a future release.

[SeppPenner]

So, this is an issue of encoding / decoding the data properly? I agree with @lothar7 that this doesn't make sense at all. To put it provocatively:

Why do we need data types at all, we could just use standard encodings and put all values to byte[]?