Fix KeyFrame not sent if no changed values (#3367)

* Initial plan

* Fix KeyFrame not sent if no changed values (Issue #2622)

The MessageCount was only incremented in OnMessagePublished() which is only called when a message is actually published. When there are no data changes, no message is published, so MessageCount stayed the same and KeyFrames were never sent after the initial one.

The fix moves the MessageCount increment to IsDeltaFrame() so it tracks publishing intervals instead of actual messages published. This ensures that after KeyFrameCount intervals, a key frame is sent regardless of whether data changed.

Co-authored-by: romanett <[email protected]>

* Address code review: Use direct cast instead of Convert.ToUInt32

Co-authored-by: romanett <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: romanett <[email protected]>

Author: Copilot

Libraries/Opc.Ua.PubSub/WriterGroupPublishState.cs

@@ -65,6 +65,7 @@ public WriterGroupPublishState()
 
         /// <summary>
         /// Returns TRUE if the next DataSetMessage is a delta frame.
+        /// Also increments the message count for each publishing interval.
         /// </summary>
         public bool IsDeltaFrame(DataSetWriterDataType writer, out uint sequenceNumber)
         {
@@ -73,7 +74,16 @@ public bool IsDeltaFrame(DataSetWriterDataType writer, out uint sequenceNumber)
                 DataSetState state = GetState(writer);
                 sequenceNumber = state.MessageCount + 1;
 
-                if (state.MessageCount % writer.KeyFrameCount != 0)
+                // Check if this is a key frame interval before incrementing
+                // This ensures the first message (MessageCount=0) is always a key frame
+                // and subsequent key frames occur every KeyFrameCount intervals
+                bool isDeltaFrame = state.MessageCount % writer.KeyFrameCount != 0;
+
+                // Increment the message count to track publishing intervals
+                // This ensures KeyFrameCount is based on intervals, not actual messages published
+                state.MessageCount++;
+
+                if (isDeltaFrame)
                 {
                     return true;
                 }
@@ -176,14 +186,13 @@ public DataSet ExcludeUnchangedFields(DataSetWriterDataType writer, DataSet data
         }
 
         /// <summary>
-        /// Increments the message counter.
+        /// Updates the state after a message is published.
         /// </summary>
         public void OnMessagePublished(DataSetWriterDataType writer, DataSet dataset)
         {
             lock (m_dataSetStates)
             {
                 DataSetState state = GetState(writer);
-                state.MessageCount++;
 
                 if (writer.KeyFrameCount > 1)
                 {

Tests/Opc.Ua.PubSub.Tests/PublishedData/WriterGroupPublishedStateTests.cs

@@ -404,5 +404,54 @@ private static void ValidateDataSetMessageData(
                 }
             }
         }
+
+        /// <summary>
+        /// Tests that key frames are sent after KeyFrameCount intervals even when there are no data changes.
+        /// This verifies the fix for issue #2622: KeyFrame is not sent if no changed values
+        /// </summary>
+        [Test(Description = "Verify KeyFrame is sent after KeyFrameCount intervals without data changes")]
+        public void KeyFrameSentWithoutDataChanges([Values(3, 5)] int keyFrameCount)
+        {
+            // Arrange - create a simple DataSetWriter with specified KeyFrameCount
+            var writer = new DataSetWriterDataType
+            {
+                DataSetWriterId = 1,
+                KeyFrameCount = (uint)keyFrameCount
+            };
+
+            var writerGroupPublishState = new WriterGroupPublishState();
+
+            // Act & Assert
+
+            // First call should be a key frame (interval 0)
+            bool isDelta = writerGroupPublishState.IsDeltaFrame(writer, out uint seq1);
+            Assert.That(isDelta, Is.False, "First message should be a key frame");
+            Assert.That(seq1, Is.EqualTo(1), "First sequence number should be 1");
+
+            // Subsequent calls before KeyFrameCount should be delta frames
+            for (int i = 1; i < keyFrameCount; i++)
+            {
+                isDelta = writerGroupPublishState.IsDeltaFrame(writer, out uint seqDelta);
+                Assert.That(isDelta, Is.True, $"Message {i + 1} should be a delta frame");
+                Assert.That(seqDelta, Is.EqualTo(i + 1), $"Sequence number should be {i + 1}");
+            }
+
+            // After KeyFrameCount intervals, we should get another key frame
+            isDelta = writerGroupPublishState.IsDeltaFrame(writer, out uint seqKeyFrame);
+            Assert.That(isDelta, Is.False, $"Message {keyFrameCount + 1} should be a key frame");
+            Assert.That(seqKeyFrame, Is.EqualTo(keyFrameCount + 1), $"Sequence number should be {keyFrameCount + 1}");
+
+            // Verify the cycle continues correctly
+            for (int i = 1; i < keyFrameCount; i++)
+            {
+                isDelta = writerGroupPublishState.IsDeltaFrame(writer, out _);
+                Assert.That(isDelta, Is.True, $"Message {keyFrameCount + i + 1} should be a delta frame");
+            }
+
+            // And another key frame
+            isDelta = writerGroupPublishState.IsDeltaFrame(writer, out uint seqKeyFrame2);
+            Assert.That(isDelta, Is.False, $"Message {2 * keyFrameCount + 1} should be a key frame");
+            Assert.That(seqKeyFrame2, Is.EqualTo(2 * keyFrameCount + 1), $"Sequence number should be {2 * keyFrameCount + 1}");
+        }
     }
 }