Skip to content

Commit 7eae03a

Browse files
committed
Clean up some warnings and PR findings
1 parent 1265189 commit 7eae03a

File tree

10 files changed

+67
-43
lines changed

10 files changed

+67
-43
lines changed

src/Moryx.Drivers.OpcUa/ApplicationConfigurationFactory.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// Copyright (c) 2025, Phoenix Contact GmbH & Co. KG
2+
// Licensed under the Apache License, Version 2.0
23

34
using Microsoft.Extensions.Logging;
5+
using Moryx.Drivers.OpcUa.Exceptions;
46
using Opc.Ua;
57
using Opc.Ua.Configuration;
68

@@ -40,7 +42,7 @@ public virtual async Task<ApplicationConfiguration> Create(ILogger logger, strin
4042
var haveAppCertificate = await application.CheckApplicationInstanceCertificate(false, 0);
4143
if (!haveAppCertificate)
4244
{
43-
throw new Exception("Application instance certificate invalid!");
45+
throw new InvalidCertificateException("Application instance certificate invalid!");
4446
}
4547
else
4648
{
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Copyright (c) 2025, Phoenix Contact GmbH & Co. KG
2+
// Licensed under the Apache License, Version 2.0
3+
4+
namespace Moryx.Drivers.OpcUa.Exceptions;
5+
6+
[Serializable]
7+
internal class InvalidCertificateException : Exception
8+
{
9+
public InvalidCertificateException()
10+
{
11+
}
12+
13+
public InvalidCertificateException(string message) : base(message)
14+
{
15+
}
16+
17+
public InvalidCertificateException(string message, Exception innerException) : base(message, innerException)
18+
{
19+
}
20+
}

src/Moryx.Drivers.OpcUa/Nodes/OpcUaNode.cs

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// Licensed under the Apache License, Version 2.0
33

44
using System.Globalization;
5-
using System.Text.RegularExpressions;
65
using Microsoft.Extensions.Logging;
76
using Moryx.AbstractionLayer.Drivers;
87
using Moryx.AbstractionLayer.Drivers.Message;
@@ -82,6 +81,9 @@ public event EventHandler<object> Received
8281
}
8382
}
8483

84+
/// <summary>
85+
/// Converts a NodeId string into an expanded NodeId string that matches the OPC UA specification;
86+
/// </summary>
8587
public static string CreateExpandedNodeId(string nodeId)
8688
{
8789
var result = nodeId.Trim();
@@ -90,15 +92,18 @@ public static string CreateExpandedNodeId(string nodeId)
9092
return result;
9193
}
9294

93-
private static string RemoveNamespaceIndexZero(string @string)
95+
private static string RemoveNamespaceIndexZero(string nodeId)
9496
{
95-
if (@string.StartsWith("ns=0;"))
97+
if (nodeId.StartsWith("ns=0;"))
9698
{
97-
return @string.Substring(5);
99+
return nodeId[5..];
98100
}
99-
return @string;
101+
return nodeId;
100102
}
101103

104+
/// <summary>
105+
/// Converts a NodeId into an expanded NodeId string that matches the OPC UA specification;
106+
/// </summary>
102107
public static string CreateExpandedNodeId(NodeId nodeId)
103108
{
104109
return CreateExpandedNodeId(nodeId.ToString());
@@ -114,17 +119,21 @@ private OpcUaNode(IOpcUaDriver driver, IModuleLogger logger)
114119

115120
}
116121

122+
/// <inheritdoc />
117123
public OpcUaNode(IOpcUaDriver driver, IModuleLogger logger, string namespaceUri, string nodeIdValue)
118124
: this(driver, logger)
119125
{
120126
NodeId = new ExpandedNodeId(nodeIdValue, namespaceUri);
121127
}
122128

129+
/// <inheritdoc />
123130
public OpcUaNode(IOpcUaDriver driver, IModuleLogger logger, ExpandedNodeId nodeId, NamespaceTable namespaceTable)
124131
: this(driver, logger)
125132
{
126133
NodeId = new ExpandedNodeId(nodeId.Identifier, nodeId.NamespaceIndex, namespaceTable.GetString(nodeId.NamespaceIndex), nodeId.ServerIndex);
127134
}
135+
136+
/// <inheritdoc />
128137
public OpcUaNode(IOpcUaDriver driver, IModuleLogger logger, string identifier)
129138
{
130139
Driver = driver;
@@ -150,7 +159,7 @@ public void Send(object payload)
150159
_logger?.Log(LogLevel.Error, "It is tried to read the value of node {NodeId}, but the node is no variable node", NodeId);
151160
return;
152161
}
153-
_driver.WriteNode(this.Identifier, payload);
162+
_driver.WriteNode(Identifier, payload);
154163
}
155164

156165
/// <summary>

src/Moryx.Drivers.OpcUa/Nodes/OpcUaNodeHelpers.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,5 @@ namespace Moryx.Drivers.OpcUa;
88
internal static partial class OpcUaNodeHelpers
99
{
1010
[GeneratedRegex(";ns=\\d+")]
11-
public static partial Regex StringWithUriAndNamespaceIndex();
11+
public static partial Regex NodeIdWithNamespaceIndexRegex();
1212
}

src/Moryx.Drivers.OpcUa/OpcUaDriver.cs

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,8 @@ namespace Moryx.Drivers.OpcUa;
2929
[Display(Name = nameof(Strings.OpcUaDriver_DisplayName), Description = nameof(Strings.OpcUaDriver_Description), ResourceType = typeof(Strings))]
3030
public class OpcUaDriver : Driver, IOpcUaDriver
3131
{
32-
//TODO 6.1 Invoke Methods
33-
3432
private const int NodeLayersShown = 5;
33+
3534
/// <summary>
3635
/// Current tate of the driver
3736
/// </summary>
@@ -188,8 +187,8 @@ internal List<NodeIdAlias> NodeIdAlias
188187
internal ISession _session; //TODO: Internal field just for tests
189188
private SessionReconnectHandler _reconnectHandler;
190189

191-
private readonly object _lock = new();
192-
private readonly object _stateLock = new();
190+
private readonly Lock _lock = new();
191+
private readonly Lock _stateLock = new();
193192

194193
private Subscription _subscription;
195194

@@ -452,10 +451,10 @@ public IMessageChannel Channel(string identifier)
452451
}
453452

454453
/// <inheritdoc/>
455-
public OpcUaNode GetNode(string identifier)
454+
public OpcUaNode GetNode(string nodeId)
456455
{
457-
var nodeId = OpcUaNode.CreateExpandedNodeId(GetNodeIdAsString(identifier));
458-
if (!_nodesFlat.TryGetValue(nodeId, out var value))
456+
var expandedNodeId = OpcUaNode.CreateExpandedNodeId(GetNodeIdAsString(nodeId));
457+
if (!_nodesFlat.TryGetValue(expandedNodeId, out var value))
459458
{
460459
return null;
461460
}
@@ -764,11 +763,7 @@ private void BrowseNodes(NodeId nodeId, NamespaceTable namespaceTable, List<OpcU
764763

765764
_savedIds.Add(node.Identifier);
766765

767-
if (!_nodesFlat.ContainsKey(node.Identifier))
768-
{
769-
_nodesFlat.Add(node.Identifier, node);
770-
}
771-
766+
_nodesFlat.TryAdd(node.Identifier, node);
772767
if (layer < NodeLayersShown)
773768
{
774769
list.Add(node);
@@ -864,7 +859,7 @@ private DataValueResult ReadNodeDataValue(string nodeId)
864859
{
865860
if (value.Error?.Exception != null)
866861
{
867-
Logger.Log(LogLevel.Error, value.Error.Exception, value.Error?.Message);
862+
Logger.Log(LogLevel.Error, value.Error.Exception, "Error reading node data.");
868863
return null;
869864
}
870865
}
@@ -1044,7 +1039,7 @@ private object CreateValue(BuiltInType type, string stringValue)
10441039
[EntrySerialize]
10451040
internal List<string> FindNodeId(string displayName)
10461041
{
1047-
var result = _nodesFlat.Where(x => x.Value.DisplayName.ToLower().Contains(displayName.ToLower()) || x.Value.DisplayName.ToLower().Equals(displayName.ToLower()))
1042+
var result = _nodesFlat.Where(x => x.Value.DisplayName.Contains(displayName, StringComparison.CurrentCultureIgnoreCase) || x.Value.DisplayName.ToLower().Equals(displayName.ToLower()))
10481043
.Select(x => x.Key).ToList();
10491044

10501045
return result;

src/Tests/Moryx.Drivers.OpcUa.Tests/DriverPropertiesTests.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// Copyright (c) 2025, Phoenix Contact GmbH & Co. KG
2+
// Licensed under the Apache License, Version 2.0
23

34
using Microsoft.Extensions.Logging;
45
using Moq;
@@ -28,7 +29,7 @@ public void DriverDoesntCrashWithInvalidServerUrl()
2829
//Arrange
2930
_driver.OpcUaServerUrl = "";
3031

31-
TestDelegate action = () => _driver.TryConnect(true).GetAwaiter().GetResult();
32+
void action() => _driver.TryConnect(true).GetAwaiter().GetResult();
3233

3334
//Assert
3435
Assert.DoesNotThrow(action);

src/Tests/Moryx.Drivers.OpcUa.Tests/HandlingCyclicNodesTests.cs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System.Text.Json;
55
using Moq;
6+
using Moryx.AbstractionLayer.Drivers;
67
using Moryx.Drivers.OpcUa.States;
78
using Moryx.Modules;
89
using Moryx.Tools;
@@ -75,7 +76,7 @@ public void DoNotShowDirectCyclicNodesInTheUi()
7576
var wait = new AutoResetEvent(false);
7677
_driver.StateChanged += (sender, e) =>
7778
{
78-
if (e is RunningState)
79+
if (e.Classification == StateClassification.Running)
7980
{
8081
//Assert II
8182
Assert.That(_driver.Nodes.Count, Is.EqualTo(1), "Number of browsed nodes doesn't fit");
@@ -123,7 +124,7 @@ public void DoNotShowIndirectCyclicNodesInTheUi()
123124
var wait = new AutoResetEvent(false);
124125
_driver.StateChanged += (sender, e) =>
125126
{
126-
if (e is RunningState)
127+
if (e.Classification == StateClassification.Running)
127128
{
128129
//Assert II
129130
var node = (OpcUaObjectDisplayNode)_driver.Nodes[0];
@@ -191,7 +192,7 @@ public void ShowSameNodeOnDifferentBranch()
191192
var wait = new AutoResetEvent(false);
192193
_driver.StateChanged += (sender, e) =>
193194
{
194-
if (e is RunningState)
195+
if (e.Classification == StateClassification.Running)
195196
{
196197
//Assert II
197198
Assert.That(_driver.Nodes.Count, Is.EqualTo(3), "Number of browsed nodes doesn't fit");

src/Tests/Moryx.Drivers.OpcUa.Tests/NodeHandlingTests.cs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the Apache License, Version 2.0
33

44
using Moq;
5+
using Moryx.AbstractionLayer.Drivers;
56
using Moryx.Drivers.OpcUa.States;
67
using Moryx.Modules;
78
using NUnit.Framework;
@@ -35,7 +36,7 @@ public void TestBrowsingNodes()
3536
var wait = new AutoResetEvent(false);
3637
_driver.StateChanged += (sender, e) =>
3738
{
38-
if (e is RunningState)
39+
if (e.Classification == StateClassification.Running)
3940
{
4041
//Assert II
4142
Assert.That(_driver.Nodes, Has.Count.EqualTo(_rootNodes.Count), "Number of browsed nodes doesn't fit");
@@ -72,7 +73,7 @@ public void TestUpdatingNodeAfterBrowsing()
7273
var wait = new AutoResetEvent(false);
7374
_driver.StateChanged += (sender, e) =>
7475
{
75-
if (e is RunningState)
76+
if (e.Classification == StateClassification.Running)
7677
{
7778
//Assert II
7879
Assert.That(node?.DisplayName, Is.EqualTo(expectedNode.Value.DisplayName.Text));
@@ -110,7 +111,7 @@ public void TestSubscribingMonitoredItemsAfterBrowsing()
110111
var wait = new AutoResetEvent(false);
111112
_driver.StateChanged += (sender, e) =>
112113
{
113-
if (e is RunningState)
114+
if (e.Classification == StateClassification.Running)
114115
{
115116
wait.Set();
116117
}
@@ -141,7 +142,7 @@ public void TestNodesCanOnlyBeSubscribedOnes()
141142
var wait = new AutoResetEvent(false);
142143
_driver.StateChanged += (sender, e) =>
143144
{
144-
if (e is RunningState)
145+
if (e.Classification == StateClassification.Running)
145146
{
146147
wait.Set();
147148
}
@@ -172,7 +173,7 @@ public void TestSubcribedValueChanges()
172173
var waitSubscription3 = new AutoResetEvent(false);
173174
_driver.StateChanged += (sender, e) =>
174175
{
175-
if (e is RunningState)
176+
if (e.Classification == StateClassification.Running)
176177
{
177178
wait.Set();
178179
}

src/Tests/Moryx.Drivers.OpcUa.Tests/NodeIdStringTests.cs

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@ public class NodeIdStringTests : OpcUaTestBase
1111
{
1212
private new readonly NamespaceTable _namespaceTable = new(["http://opcfoundation.org/UA/", "http://anothernamespace.org/"]);
1313

14+
private const string NodeIdWithUriOnly = "nsu=http://anothernamespace.org/;i=2994";
15+
private const string NodeIdWithNamespaceOnly = "ns=1;i=2994";
16+
private const string NodeIdWithDefaultNamespaceOnly = "ns=0;i=2994";
17+
private const string NodeIdWithUriAndIndex = "nsu=http://anothernamespace.org/;ns=1;i=2994";
18+
1419
[SetUp]
1520
public void Setup()
1621
{
@@ -19,36 +24,28 @@ public void Setup()
1924
[Test]
2025
public void NamespaceIndexIsIgnoredWhenUriIsSpecified()
2126
{
22-
var NodeIdWithUriAndIndex = "nsu=http://anothernamespace.org/;ns=1;i=2994";
23-
2427
var nodeIdString = OpcUaNode.CreateExpandedNodeId(NodeIdWithUriAndIndex);
2528
Assert.That(nodeIdString, Is.EqualTo("nsu=http://anothernamespace.org/;i=2994"));
2629
}
2730

2831
[Test]
2932
public void NamespaceUriIsUsedWhenIndexIsNotSpecified()
3033
{
31-
var NodeIdWithUriOnly = "nsu=http://anothernamespace.org/;i=2994";
32-
3334
var nodeIdString = OpcUaNode.CreateExpandedNodeId(NodeIdWithUriOnly);
3435
Assert.That(nodeIdString, Is.EqualTo("nsu=http://anothernamespace.org/;i=2994"));
3536
}
3637

3738
[Test]
3839
public void NamespaceIndexIsUsedWhenUriIsNotSpecified()
3940
{
40-
var NodeIdWithUriOnly = "ns=1;i=2994";
41-
42-
var nodeIdString = OpcUaNode.CreateExpandedNodeId(NodeIdWithUriOnly);
41+
var nodeIdString = OpcUaNode.CreateExpandedNodeId(NodeIdWithNamespaceOnly);
4342
Assert.That(nodeIdString, Is.EqualTo("ns=1;i=2994"));
4443
}
4544

4645
[Test]
4746
public void NamespaceIndexIgnoredWhenZeroAndUriIsNotSpecified()
4847
{
49-
var NodeIdWithUriOnly = "ns=0;i=2994";
50-
51-
var nodeIdString = OpcUaNode.CreateExpandedNodeId(NodeIdWithUriOnly);
48+
var nodeIdString = OpcUaNode.CreateExpandedNodeId(NodeIdWithDefaultNamespaceOnly);
5249
Assert.That(nodeIdString, Is.EqualTo("i=2994"));
5350
}
5451

src/Tests/Moryx.Drivers.OpcUa.Tests/OpcUaTestBase.cs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,11 @@ public class OpcUaTestBase
1616
private const ushort IndexNamespace1 = 1;
1717
private const ushort IndexNamespace2 = 2;
1818

19-
#pragma warning disable CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring as nullable.
2019
protected Mock<ISession> _sessionMock;
2120
protected Dictionary<NodeId, ReferenceDescription> _rootNodes;
2221
protected ReferenceDescription _root;
2322
protected NamespaceTable _namespaceTable = CreateNamespaceTable();
2423
protected OpcUaDriver _driver;
25-
#pragma warning restore CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring as nullable.
2624

2725
protected static NamespaceTable CreateNamespaceTable()
2826
{

0 commit comments

Comments
 (0)