Skip to content

Commit f970392

Browse files
Copilotromanettmarcschier
authored
Fix SourceTimestamp and ServerTimestamp mismatch in ServerStatus children (#3386)
* Initial plan * Fix ServerTimestamp to match SourceTimestamp in CoreNodeManager Set ServerTimestamp equal to SourceTimestamp after node read operations to ensure consistency. Previously, ServerTimestamp was set early (before the node's read callback), while SourceTimestamp was set during the callback, causing timestamp mismatches for nodes like ServerStatus children. Added test to verify timestamp matching (currently failing - more investigation needed) Co-authored-by: romanett <[email protected]> * Apply timestamp fix to CustomNodeManager Extended the ServerTimestamp fix to CustomNodeManager which handles ServerStatus and other diagnostic nodes. CustomNodeManager was setting ServerTimestamp early and separately from SourceTimestamp, causing the mismatch. Now both timestamps are synchronized after reading values. All tests pass including the new ServerStatusTimestampsMatchAsync test. Co-authored-by: romanett <[email protected]> * Improve comments based on code review feedback Clarified that timestamp synchronization applies specifically to Value attributes, and explained the difference in handling between Value and non-Value attributes (non-Value attributes only use ServerTimestamp). Co-authored-by: romanett <[email protected]> * Fix and update serilog package --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: romanett <[email protected]> Co-authored-by: Marc Schier <[email protected]> Co-authored-by: Marc Schier <[email protected]>
1 parent dc85d52 commit f970392

File tree

5 files changed

+79
-13
lines changed

5 files changed

+79
-13
lines changed

‎Directory.Packages.props‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
<PackageVersion Include="SharpFuzz" Version="2.2.0" />
3737
<PackageVersion Include="Serilog" Version="4.3.0" />
3838
<PackageVersion Include="Serilog.Expressions" Version="5.0.0" />
39-
<PackageVersion Include="Serilog.Extensions.Logging" Version="9.0.2" />
39+
<PackageVersion Include="Serilog.Extensions.Logging" Version="10.0.0" />
4040
<PackageVersion Include="Serilog.Sinks.Console" Version="6.1.1" />
4141
<PackageVersion Include="Serilog.Sinks.Debug" Version="3.0.0" />
4242
<PackageVersion Include="Serilog.Sinks.File" Version="7.0.0" />

‎Libraries/Opc.Ua.Client/Subscription/MonitoredItem.cs‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -594,12 +594,7 @@ public void SaveValueInCache(IEncodeable newValue)
594594
{
595595
m_eventCache.OnNotification(eventchange);
596596
}
597-
598-
var handler = m_Notification;
599-
if (handler != null)
600-
{
601-
handler.Invoke(this, new MonitoredItemNotificationEventArgs(newValue));
602-
}
597+
m_Notification?.Invoke(this, new MonitoredItemNotificationEventArgs(newValue));
603598
}
604599
}
605600

‎Libraries/Opc.Ua.Server/Diagnostics/CustomNodeManager.cs‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1633,7 +1633,7 @@ public virtual void Read(
16331633
DataValue value = values[ii] = new DataValue();
16341634

16351635
value.Value = null;
1636-
value.ServerTimestamp = DateTime.UtcNow;
1636+
value.ServerTimestamp = DateTime.MinValue; // Will be set after ReadAttribute
16371637
value.SourceTimestamp = DateTime.MinValue;
16381638
value.StatusCode = StatusCodes.Good;
16391639

@@ -1656,6 +1656,26 @@ public virtual void Read(
16561656
nodeToRead.ParsedIndexRange,
16571657
nodeToRead.DataEncoding,
16581658
value);
1659+
1660+
// Set timestamps after ReadAttribute to ensure consistency
1661+
// For Value attributes, match ServerTimestamp to SourceTimestamp
1662+
// For other attributes, just ensure ServerTimestamp is set
1663+
if (nodeToRead.AttributeId == Attributes.Value)
1664+
{
1665+
if (value.SourceTimestamp == DateTime.MinValue)
1666+
{
1667+
value.SourceTimestamp = DateTime.UtcNow;
1668+
}
1669+
value.ServerTimestamp = value.SourceTimestamp;
1670+
}
1671+
else
1672+
{
1673+
// For non-value attributes, only ServerTimestamp is relevant
1674+
if (value.ServerTimestamp == DateTime.MinValue)
1675+
{
1676+
value.ServerTimestamp = DateTime.UtcNow;
1677+
}
1678+
}
16591679
#if DEBUG
16601680
if (nodeToRead.AttributeId == Attributes.Value)
16611681
{

‎Libraries/Opc.Ua.Server/NodeManager/CoreNodeManager.cs‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -717,7 +717,7 @@ public void Read(
717717
DataValue value = values[ii] = new DataValue();
718718

719719
value.Value = null;
720-
value.ServerTimestamp = DateTime.UtcNow;
720+
value.ServerTimestamp = DateTime.MinValue; // Will be set later
721721
value.SourceTimestamp = DateTime.MinValue;
722722
value.StatusCode = StatusCodes.BadAttributeIdInvalid;
723723

@@ -786,11 +786,17 @@ public void Read(
786786

787787
value.Value = defaultValue;
788788

789-
// don't replace timestamp if it was set in the NodeSource
789+
// Set SourceTimestamp if not already set by the node
790790
if (value.SourceTimestamp == DateTime.MinValue)
791791
{
792792
value.SourceTimestamp = DateTime.UtcNow;
793793
}
794+
795+
// Set ServerTimestamp to match SourceTimestamp for Value attributes
796+
// This ensures ServerTimestamp and SourceTimestamp are equal,
797+
// which is important for nodes like ServerStatus children where
798+
// the node's read callback sets a specific timestamp
799+
value.ServerTimestamp = value.SourceTimestamp;
794800
}
795801
}
796802
}

‎Tests/Opc.Ua.Server.Tests/ReferenceServerTest.cs‎

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,8 @@ public async Task GetOperationLimitsAsync()
245245
requestHeader,
246246
kMaxAge,
247247
TimestampsToReturn.Neither,
248-
readIdCollection, CancellationToken.None).ConfigureAwait(false);
248+
readIdCollection,
249+
CancellationToken.None).ConfigureAwait(false);
249250
ServerFixtureUtils.ValidateResponse(readResponse.ResponseHeader, readResponse.Results, readIdCollection);
250251
ServerFixtureUtils.ValidateDiagnosticInfos(
251252
readResponse.DiagnosticInfos,
@@ -982,6 +983,49 @@ public async Task ServerEventNotifierHistoryReadBitAsync()
982983
"Server EventNotifier should have SubscribeToEvents bit set");
983984
}
984985

986+
/// <summary>
987+
/// Verify that ServerStatus children have matching SourceTimestamp and ServerTimestamp.
988+
/// </summary>
989+
[Test]
990+
public async Task ServerStatusTimestampsMatchAsync()
991+
{
992+
var logger = m_telemetry.CreateLogger<ReferenceServerTests>();
993+
994+
// Read ServerStatus children (CurrentTime, StartTime, State, etc.)
995+
var nodesToRead = new ReadValueIdCollection
996+
{
997+
new ReadValueId { NodeId = VariableIds.Server_ServerStatus_CurrentTime, AttributeId = Attributes.Value },
998+
new ReadValueId { NodeId = VariableIds.Server_ServerStatus_StartTime, AttributeId = Attributes.Value },
999+
new ReadValueId { NodeId = VariableIds.Server_ServerStatus_State, AttributeId = Attributes.Value }
1000+
};
1001+
1002+
m_requestHeader.Timestamp = DateTime.UtcNow;
1003+
var readResponse = await m_server.ReadAsync(
1004+
m_secureChannelContext,
1005+
m_requestHeader,
1006+
0,
1007+
TimestampsToReturn.Both,
1008+
nodesToRead,
1009+
CancellationToken.None).ConfigureAwait(false);
1010+
1011+
ServerFixtureUtils.ValidateResponse(readResponse.ResponseHeader, readResponse.Results, nodesToRead);
1012+
Assert.AreEqual(3, readResponse.Results.Count);
1013+
1014+
// Verify that SourceTimestamp and ServerTimestamp are equal for all ServerStatus children
1015+
for (int i = 0; i < readResponse.Results.Count; i++)
1016+
{
1017+
var result = readResponse.Results[i];
1018+
logger.LogInformation(
1019+
"NodeId: {NodeId}, SourceTimestamp: {SourceTimestamp}, ServerTimestamp: {ServerTimestamp}",
1020+
nodesToRead[i].NodeId,
1021+
result.SourceTimestamp,
1022+
result.ServerTimestamp);
1023+
1024+
Assert.AreEqual(result.SourceTimestamp, result.ServerTimestamp,
1025+
$"SourceTimestamp and ServerTimestamp should be equal for {nodesToRead[i].NodeId}");
1026+
}
1027+
}
1028+
9851029
/// <summary>
9861030
/// Test that the Int32Value node (ns=3;i=2808) allows historical data access.
9871031
/// Verifies the fix for issue #2520 where the node was marked as historizing
@@ -1016,8 +1060,8 @@ public async Task HistoryReadInt32ValueNodeAsync()
10161060
ReadResponse readResponse = await m_server.ReadAsync(
10171061
m_secureChannelContext,
10181062
m_requestHeader,
1019-
0,
1020-
TimestampsToReturn.Both,
1063+
kMaxAge,
1064+
TimestampsToReturn.Neither,
10211065
readIdCollection,
10221066
CancellationToken.None).ConfigureAwait(false);
10231067

@@ -1091,6 +1135,7 @@ public async Task HistoryReadInt32ValueNodeAsync()
10911135
}
10921136
}
10931137

1138+
/// <summary>
10941139
/// Test provisioning mode - server should start with limited namespace.
10951140
/// </summary>
10961141
[Test]

0 commit comments

Comments
 (0)