Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

All SparkplugMessageGenerator tests regarding version B fails #63

Closed
mikolajwalus opened this issue Jan 14, 2024 · 8 comments
Closed

All SparkplugMessageGenerator tests regarding version B fails #63

mikolajwalus opened this issue Jan 14, 2024 · 8 comments
Assignees
Labels
bug Something isn't working

Comments

@mikolajwalus
Copy link

Like can be seen here all tests are failing.

image

Question is: does library works correctly or does it needs to be adressed?

@mikolajwalus mikolajwalus changed the title All MessageGenerator tests regarding version B fails All SparkplugMessageGenerator tests regarding version B fails Jan 14, 2024
@juanredondo
Copy link

Same problem here. Using it on my project always receive 0 as Value on Sparkplug Aplication sended by Sparkplug Node

@SeppPenner SeppPenner self-assigned this Jan 17, 2024
@SeppPenner SeppPenner added the bug Something isn't working label Jan 17, 2024
@SeppPenner
Copy link
Owner

Good question... I thought that the tests were ok before the last release...

@mikolajwalus
Copy link
Author

The bug is in PayloadConverter. This particular changes where performed in this 39a878b

I don't know if it will fix all the problems but in terms of metrics:

Now the code looks like this:

    private static VersionBProtoBuf.ProtoBufPayload.Metric ConvertVersionBMetric(Metric metric)
    {
        return new()
        {
            Alias = metric.Alias,
            Datatype = metric.ValueCase,
            IsHistorical = metric.IsHistorical,
            IsNull = metric.IsNull,
            IsTransient = metric.IsTransient,
            Metadata = ConvertVersionBMetaData(metric.Metadata),
            Name = metric.Name,
            Properties = ConvertVersionBPropertySet(metric.Properties),
            Timestamp = metric.Timestamp,
            DatasetValue = ConvertVersionBDataSet(metric.DataSetValue),
            BooleanValue = metric.BooleanValue,
            BytesValue = metric.BytesValue,
            DoubleValue = metric.DoubleValue,
            // Todo: How to handle this properly?
            ExtensionValue = (metric.ExtensionValue is null) ? null :
                new VersionBProtoBuf.ProtoBufPayload.Metric.MetricValueExtension { },
            FloatValue = metric.FloatValue,
            IntValue = metric.IntValue,
            LongValue = metric.LongValue,
            StringValue = metric.StringValue,
            TemplateValue = ConvertVersionBTemplate(metric.TemplateValue)
        };
    }

The case is that the last used value in constructor -> Template Value sets in generated protobuf class wrong discriminator (one for template):

            [global::ProtoBuf.ProtoMember(18, Name = @"template_value")]
            public Template? TemplateValue
            {
                get => __pbn__value.Is(18) ? ((Template)__pbn__value.Object) : default;
                set => __pbn__value = new global::ProtoBuf.DiscriminatedUnion64Object(18, value);
            }
            public bool ShouldSerializeTemplateValue() => __pbn__value.Is(18);
            public void ResetTemplateValue() => global::ProtoBuf.DiscriminatedUnion64Object.Reset(ref __pbn__value, 18);

The result is that other data types are not Serialized at all. I don't know why the previous code was removed but that big switch seemed ok for me.

    private static VersionBProtoBuf.ProtoBufPayload.Metric ConvertVersionBMetric(Metric metric)
    {
        VersionBProtoBuf.ProtoBufPayload.Metric pbMetric = new()
        {
            Alias = metric.Alias,
            Datatype = metric.ValueCase,
            IsHistorical = metric.IsHistorical,
            IsNull = metric.IsNull,
            IsTransient = metric.IsTransient,
            Metadata = ConvertVersionBMetaData(metric.Metadata),
            Name = metric.Name,
            Properties = ConvertVersionBPropertySet(metric.Properties),
            Timestamp = metric.Timestamp
        };

        switch (metric.DataType)
        {
            case VersionBDataTypeEnum.Int8:
            case VersionBDataTypeEnum.Int16:
            case VersionBDataTypeEnum.Int32:
            case VersionBDataTypeEnum.UInt8:
            case VersionBDataTypeEnum.UInt16:
            case VersionBDataTypeEnum.UInt32:
                pbMetric.IntValue = metric.IntValue;
                break;
            case VersionBDataTypeEnum.Int64:
            case VersionBDataTypeEnum.UInt64:
            case VersionBDataTypeEnum.DateTime:
                pbMetric.LongValue = metric.LongValue;
                break;
            case VersionBDataTypeEnum.Float:
                pbMetric.FloatValue = metric.FloatValue;
                break;
            case VersionBDataTypeEnum.Double:
                pbMetric.DoubleValue = metric.DoubleValue;
                break;
            case VersionBDataTypeEnum.Boolean:
                pbMetric.BooleanValue = metric.BooleanValue;
                break;
            case VersionBDataTypeEnum.String:
            case VersionBDataTypeEnum.Text:
            case VersionBDataTypeEnum.Uuid:
                pbMetric.StringValue = metric.StringValue;
                break;
            case VersionBDataTypeEnum.DataSet:
                pbMetric.DatasetValue = ConvertVersionBDataSet(metric.DataSetValue);
                break;
            case VersionBDataTypeEnum.Template:
                pbMetric.TemplateValue = ConvertVersionBTemplate(metric.TemplateValue);
                break;
            case VersionBDataTypeEnum.Bytes:
                pbMetric.BytesValue = metric.BytesValue;
                break;
            case VersionBDataTypeEnum.Unknown:
            case VersionBDataTypeEnum.File:
            case VersionBDataTypeEnum.PropertySet:
            case VersionBDataTypeEnum.PropertySetList:
            default:
                pbMetric.ExtensionValue = (metric.ExtensionValue is null) ? null :
                    new VersionBProtoBuf.ProtoBufPayload.Metric.MetricValueExtension { Details = metric.ExtensionValue.Details };
                break;
        }

        return pbMetric;
    }

@SeppPenner
Copy link
Owner

The changes were done to fit the data format for Version B (The updated model in the 3.0 spec) and therefore I re-generated the Proto classes. That might have caused the issues.

@juanredondo
Copy link

Is there any forecast for the release of a nuget with this bug fixed?

@SeppPenner
Copy link
Owner

Is there any forecast for the release of a nuget with this bug fixed?

I hope to fix it tomorrow.

@SeppPenner
Copy link
Owner

#64 and #66 are related.

@SeppPenner
Copy link
Owner

Version 1.3.2 is out soon and should fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants