Skip to content

Commit 22ecf20

Browse files
sevenelevenseveneleven
committed
Clean up some warnings and PR findings
1 parent e71c949 commit 22ecf20

File tree

7 files changed

+59
-34
lines changed

7 files changed

+59
-34
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
{

‎src/Moryx.Drivers.OpcUa/Exceptions/InvalidCertificateException.cs‎

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/OpcUaDriver.cs‎

Lines changed: 10 additions & 15 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,14 +187,14 @@ 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

196195
//TODO: Internal property just for tests, use xml also in tests
197196
internal ApplicationConfigurationFactory ApplicationConfigurationFactory { get; set; } = new();
198-
197+
199198
/// <summary>
200199
/// Convert an OpcUaNode to an entity to be shown on the UI
201200
/// </summary>
@@ -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: 2 additions & 1 deletion
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;
@@ -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/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

0 commit comments

Comments
 (0)