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

1.05.04 final release fixes #2893

Open
wants to merge 31 commits into
base: develop/main374
Choose a base branch
from

Conversation

opcfoundation-org
Copy link
Contributor

Proposed changes

Merges the changes needed for the 1.05.04 release.

Copy link

codecov bot commented Dec 9, 2024

Codecov Report

Attention: Patch coverage is 57.67717% with 215 lines in your changes missing coverage. Please review.

Project coverage is 54.78%. Comparing base (444b2c8) to head (d9ea078).

Files with missing lines Patch % Lines
Stack/Opc.Ua.Core/Types/Encoders/JsonEncoder.cs 47.20% 140 Missing and 11 partials ⚠️
Stack/Opc.Ua.Core/Types/Encoders/JsonDecoder.cs 68.18% 41 Missing and 8 partials ⚠️
Stack/Opc.Ua.Core/Schema/UANodeSet.cs 50.00% 3 Missing ⚠️
Stack/Opc.Ua.Core/Types/BuiltIn/ExtensionObject.cs 0.00% 1 Missing and 1 partial ⚠️
Stack/Opc.Ua.Core/Types/Encoders/BinaryDecoder.cs 0.00% 2 Missing ⚠️
Stack/Opc.Ua.Core/Types/Encoders/BinaryEncoder.cs 0.00% 2 Missing ⚠️
Stack/Opc.Ua.Core/Types/Encoders/XmlDecoder.cs 0.00% 2 Missing ⚠️
Stack/Opc.Ua.Core/Types/Encoders/XmlEncoder.cs 0.00% 2 Missing ⚠️
...es/Opc.Ua.PubSub/Transport/MqttPubSubConnection.cs 88.88% 1 Missing ⚠️
Stack/Opc.Ua.Core/Stack/Client/UserIdentity.cs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@                 Coverage Diff                 @@
##           develop/main374    #2893      +/-   ##
===================================================
- Coverage            54.79%   54.78%   -0.02%     
===================================================
  Files                  349      349              
  Lines                66372    66657     +285     
  Branches             13622    13733     +111     
===================================================
+ Hits                 36371    36515     +144     
- Misses               26011    26146     +135     
- Partials              3990     3996       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mregen mregen requested a review from marcschier December 17, 2024 06:16
Copy link
Contributor

@mregen mregen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, ready to merge?

@@ -95,6 +95,7 @@ public override void Encode(IEncoder encoder)
encoder.PushNamespace(XmlNamespace);

string fieldName = null;

if (encoder.UseReversibleEncoding)
{
encoder.WriteUInt32("SwitchField", m_switchField);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO? This implementation doesn't use the new switchfield API yet.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is also code which reads the encodingmask

@@ -134,6 +134,7 @@ public string PolicyId
public string DisplayName
{
get { return m_displayName; }
set { m_displayName = value; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was intentionally removed

@@ -418,6 +418,11 @@ public object Body
m_encoding = ExtensionObjectEncoding.Xml;
}

else if (m_body is Newtonsoft.Json.Linq.JObject)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this needed?

/// Encode the switch field for a union.
/// </summary>
void WriteSwitchField(string fieldName, uint switchField);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can fieldname be omitted?

@mregen mregen added this to the 1.5.374 January update milestone Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants