diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/SequenceTracker.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/SequenceTracker.cs index 13c91bb3..d614f05b 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/SequenceTracker.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/SequenceTracker.cs @@ -88,8 +88,8 @@ public sealed class SequenceTracker } /// - /// Records a birth (NBIRTH/DBIRTH): restarts the sequence at the birth's seq and adopts - /// its bdSeq as the current session token. + /// Records an NBIRTH: restarts the sequence at the birth's seq and adopts its + /// bdSeq as the current session token. /// /// /// The birth payload's seq, which is @@ -105,11 +105,22 @@ public sealed class SequenceTracker /// /// /// - /// A birth is never a gap. It restarts the sequence by definition, so it must not + /// An NBIRTH is never a gap. It restarts the sequence by definition, so it must not /// be run through — doing so would flag a gap on the very message /// that just resynchronized everything, and answer a birth with a request for another one. /// /// + /// A DBIRTH must NOT be offered here — it belongs to . Only the + /// NBIRTH restarts the sequence; a DBIRTH carries the next number in the edge node's + /// ongoing stream, and only the NBIRTH (with the NDEATH) carries bdSeq at all. + /// Routing a DBIRTH here would do two wrong things at once: silently rebase the sequence + /// mid-stream, and — because the DBIRTH has no bdSeq to pass — wipe the node's + /// session token to null. A stale Last Will arriving afterwards would then find + /// nothing to be compared against, fall through the fail-toward-stale rule in + /// , and kill the live node this type exists to + /// protect. The method is named for the node deliberately, so that call site reads wrong. + /// + /// /// An in-range seq other than the spec-required 0 is adopted, not refused. /// Refusing it would leave the tracker unsynchronized against a publisher whose stream is /// otherwise perfectly followable, and every message that followed would demand a fresh @@ -122,7 +133,7 @@ public sealed class SequenceTracker /// thing that lets a stale will kill a live node. /// /// - public bool AcceptBirth(ulong? seq, ulong? bdSeq = null) + public bool AcceptNodeBirth(ulong? seq, ulong? bdSeq = null) { lock (_gate) { @@ -143,8 +154,8 @@ public sealed class SequenceTracker } /// - /// Offers the next sequenced message's seq (NDATA/DDATA/DDEATH) and reports whether the - /// stream is still contiguous. + /// Offers the next sequenced message's seqDBIRTH, NDATA, DDATA or DDEATH — + /// and reports whether the stream is still contiguous. /// /// /// The payload's seq, which is @@ -252,7 +263,7 @@ public sealed class SequenceTracker /// /// Pure. It answers a question and changes nothing, so the caller can ask it while /// logging or deciding. A death that is acted on needs no reset here: the node is - /// offline, and the next birth calls which restarts the sequence + /// offline, and the next birth calls which restarts the sequence /// anyway. /// /// diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/SequenceTrackerTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/SequenceTrackerTests.cs index 968a56c8..a8cb04ae 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/SequenceTrackerTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/SequenceTrackerTests.cs @@ -63,7 +63,7 @@ public sealed class SequenceTrackerTests public void Sequence_FullWrapCycles_NeverReportAGap() { var s = new SequenceTracker(); - s.AcceptBirth(0).ShouldBeTrue(); + s.AcceptNodeBirth(0).ShouldBeTrue(); for (var i = 1; i <= 600; i++) { @@ -200,7 +200,7 @@ public sealed class SequenceTrackerTests var s = new SequenceTracker(); s.Accept(200).ShouldBeTrue(); - s.AcceptBirth(0).ShouldBeTrue(); + s.AcceptNodeBirth(0).ShouldBeTrue(); s.LastSeq.ShouldBe((byte)0); s.IsBirthSynchronized.ShouldBeTrue(); @@ -216,7 +216,7 @@ public sealed class SequenceTrackerTests public void Birth_ThenSkippedValue_IsStillAGap() { var s = new SequenceTracker(); - s.AcceptBirth(0).ShouldBeTrue(); + s.AcceptNodeBirth(0).ShouldBeTrue(); s.Accept(1).ShouldBeTrue(); s.Accept(3).ShouldBeFalse(); @@ -233,7 +233,7 @@ public sealed class SequenceTrackerTests { var s = new SequenceTracker(); - s.AcceptBirth(7).ShouldBeTrue(); + s.AcceptNodeBirth(7).ShouldBeTrue(); s.LastSeq.ShouldBe((byte)7); s.IsBirthSynchronized.ShouldBeTrue(); @@ -249,12 +249,35 @@ public sealed class SequenceTrackerTests { var s = new SequenceTracker(); - s.AcceptBirth(256).ShouldBeFalse(); + s.AcceptNodeBirth(256).ShouldBeFalse(); s.LastSeq.ShouldBeNull(); s.IsBirthSynchronized.ShouldBeFalse(); } + /// + /// A DBIRTH continues the edge node's sequence — it does not restart it, and it must not be + /// routed through . Only the NBIRTH resets + /// seq, and only the NBIRTH/NDEATH carry bdSeq. Sending a DBIRTH to the node-birth + /// entry point would rebase the sequence mid-stream and — having no bdSeq to pass + /// — wipe the session token, at which point a stale Last Will finds nothing to be compared + /// against and kills the live node. This test pins the shape at the tracker's own boundary so + /// the ingest state machine has something to fail against if it wires the call sites the other + /// way round. + /// + [Fact] + public void DeviceBirth_ContinuesTheNodeSequence_AndLeavesTheSessionTokenAlone() + { + var s = new SequenceTracker(); + s.AcceptNodeBirth(0, bdSeq: 7).ShouldBeTrue(); + + s.Accept(1).ShouldBeTrue(); // the DBIRTH — next in the node's stream, not a restart + s.Accept(2).ShouldBeTrue(); // first DDATA + + s.BirthBdSeq.ShouldBe(7UL, "a device birth must not disturb the node's session token"); + s.IsDeathForCurrentSession(6).ShouldBeFalse("the stale-will guard must still have a token to compare"); + } + /// /// returns the tracker to its virgin state. This is the /// reconnect seam: after a dropped connection the driver has missed an unknown number of @@ -264,7 +287,7 @@ public sealed class SequenceTrackerTests public void Reset_ClearsTheBaselineAndTheBirthSynchronizedFlag() { var s = new SequenceTracker(); - s.AcceptBirth(0, bdSeq: 4).ShouldBeTrue(); + s.AcceptNodeBirth(0, bdSeq: 4).ShouldBeTrue(); s.Accept(1).ShouldBeTrue(); s.Reset(); @@ -289,8 +312,8 @@ public sealed class SequenceTrackerTests public void Death_WithStaleBdSeqArrivingAfterARebirth_DoesNotApply() { var s = new SequenceTracker(); - s.AcceptBirth(0, bdSeq: 7); // session 7 — the one that is about to die - s.AcceptBirth(0, bdSeq: 8); // session 8 — the node is back, and alive + s.AcceptNodeBirth(0, bdSeq: 7); // session 7 — the one that is about to die + s.AcceptNodeBirth(0, bdSeq: 8); // session 8 — the node is back, and alive s.IsDeathForCurrentSession(7).ShouldBeFalse("session 7's will must not kill session 8"); } @@ -300,7 +323,7 @@ public sealed class SequenceTrackerTests public void Death_WithMatchingBdSeq_Applies() { var s = new SequenceTracker(); - s.AcceptBirth(0, bdSeq: 8); + s.AcceptNodeBirth(0, bdSeq: 8); s.IsDeathForCurrentSession(8).ShouldBeTrue(); } @@ -327,7 +350,7 @@ public sealed class SequenceTrackerTests public void Death_WithNoBdSeq_Applies() { var s = new SequenceTracker(); - s.AcceptBirth(0, bdSeq: 8); + s.AcceptNodeBirth(0, bdSeq: 8); s.IsDeathForCurrentSession(null).ShouldBeTrue(); } @@ -337,7 +360,7 @@ public sealed class SequenceTrackerTests public void Death_WhenTheBirthCarriedNoBdSeq_Applies() { var s = new SequenceTracker(); - s.AcceptBirth(0, bdSeq: null); + s.AcceptNodeBirth(0, bdSeq: null); s.IsDeathForCurrentSession(7).ShouldBeTrue(); } @@ -352,7 +375,7 @@ public sealed class SequenceTrackerTests { var s = new SequenceTracker(); - s.AcceptBirth(256, bdSeq: 8).ShouldBeFalse(); + s.AcceptNodeBirth(256, bdSeq: 8).ShouldBeFalse(); s.BirthBdSeq.ShouldBe(8UL); s.IsDeathForCurrentSession(7).ShouldBeFalse();