diff --git a/src/CanKit.Pro.Actor/ProtocolActor.cs b/src/CanKit.Pro.Actor/ProtocolActor.cs index 26e2466..1e5b719 100644 --- a/src/CanKit.Pro.Actor/ProtocolActor.cs +++ b/src/CanKit.Pro.Actor/ProtocolActor.cs @@ -151,7 +151,7 @@ public sealed class ProtocolActor : IProtocolActor var ticks = entry.DueTimestamp - _time.GetTimestamp(); return ticks <= 0 ? TimeSpan.Zero - : TimeSpan.FromSeconds(ticks / (double)_time.Frequency); + : TickMath.RemainingFromTicks(ticks, _time.Frequency); } return null; @@ -853,14 +853,11 @@ private void ThrowIfDisposed() // a floor ("not before"), never a target to be missed on the low side. private long DueTimestamp(TimeSpan delay) { - var now = _time.GetTimestamp(); - var ticks = delay.TotalSeconds * _time.Frequency; - - // TimeSpan reaches ~29 000 years; the tick counter does not. Saturating is the right - // answer for a delay nothing in this process will ever outlive anyway. - if (ticks >= long.MaxValue - now) return long.MaxValue; - - return now + (long)Math.Ceiling(ticks); + // Exact, see TickMath: through a double about one millisecond value in twelve (35, 70, + // 85, 101 ms ...) became due a tick late. TimeSpan reaches ~29 000 years and the tick + // counter does not; TickMath saturates, the right answer for a delay nothing in this + // process will ever outlive anyway. + return TickMath.DueAt(_time.GetTimestamp(), delay, _time.Frequency); } // Rounds *up*: truncating a 0.4 ms remainder to a 0 ms wait made the loop spin on the diff --git a/src/CanKit.Pro.Actor/TickMath.cs b/src/CanKit.Pro.Actor/TickMath.cs new file mode 100644 index 0000000..7a7f9d3 --- /dev/null +++ b/src/CanKit.Pro.Actor/TickMath.cs @@ -0,0 +1,44 @@ +using System; + +namespace CanKit.Pro.Actor; + +/// +/// Converts between and the ticks of an exactly. +/// +/// +/// A is a whole number of 100 ns ticks and a source's frequency a whole +/// number per second, so every conversion here is a product and a division by 10^7, both exact in +/// . Done through a instead (TotalSeconds * frequency, +/// TimeSpan.FromSeconds), about one whole millisecond in twelve came out one tick off in +/// each direction, and FromSeconds rounds to whole milliseconds on .NET Framework, which +/// hid it there. A delay or a remaining time is a floor ("not before"), so those round up; an +/// elapsed time is "at least", so that rounds down. +/// +internal static class TickMath +{ + /// The source ticks spans, rounded up, saturating at + /// for a span no counter will ever reach. + internal static long ToTicks(TimeSpan span, long frequency) + { + var ticks = Math.Ceiling((decimal)span.Ticks * frequency / TimeSpan.TicksPerSecond); + return ticks >= long.MaxValue ? long.MaxValue : (long)ticks; + } + + /// As , for a span measured from + /// : saturates the sum, not the span. + internal static long DueAt(long now, TimeSpan span, long frequency) + { + var ticks = Math.Ceiling((decimal)span.Ticks * frequency / TimeSpan.TicksPerSecond); + return ticks >= long.MaxValue - now ? long.MaxValue : now + (long)ticks; + } + + /// The span of a source covers, rounded up: a time + /// still to wait. + internal static TimeSpan RemainingFromTicks(long sourceTicks, long frequency) + => TimeSpan.FromTicks((long)Math.Ceiling((decimal)sourceTicks * TimeSpan.TicksPerSecond / frequency)); + + /// The span of a source covers, rounded down: a time + /// that has at least passed. + internal static TimeSpan ElapsedFromTicks(long sourceTicks, long frequency) + => TimeSpan.FromTicks((long)Math.Floor((decimal)sourceTicks * TimeSpan.TicksPerSecond / frequency)); +} diff --git a/src/CanKit.Pro.CANopen/CanOpenNode.Pdo.cs b/src/CanKit.Pro.CANopen/CanOpenNode.Pdo.cs index b485459..b29729b 100644 --- a/src/CanKit.Pro.CANopen/CanOpenNode.Pdo.cs +++ b/src/CanKit.Pro.CANopen/CanOpenNode.Pdo.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; +using CanKit.Pro.Actor; using CanKit.Pro.CANopen.Emcy; using CanKit.Pro.CANopen.Nmt; using CanKit.Pro.CANopen.Pdo; @@ -426,7 +427,7 @@ private void RequestEventDrivenTransmission(TpdoRuntime rt) } var time = _actor.TimeSource; long elapsedTicks = time.GetTimestamp() - rt.LastTransmission; - var elapsed = TimeSpan.FromTicks((long)(elapsedTicks * (TimeSpan.TicksPerSecond / (double)time.Frequency))); + var elapsed = TickMath.ElapsedFromTicks(elapsedTicks, time.Frequency); if (elapsed >= rt.InhibitTime) { EmitTpdo(rt); diff --git a/src/CanKit.Pro.IsoTp/IsoTpFunctionalClient.cs b/src/CanKit.Pro.IsoTp/IsoTpFunctionalClient.cs index 2f72063..8ca464b 100644 --- a/src/CanKit.Pro.IsoTp/IsoTpFunctionalClient.cs +++ b/src/CanKit.Pro.IsoTp/IsoTpFunctionalClient.cs @@ -324,7 +324,7 @@ private async Task> CollectFromSubscripti // The window's end is also held as an arrival stamp: the timer's callback and this // method's continuations are scheduling, and a frame that arrived after the deadline // but before they ran is not the window's (Codex on #150). - long deadline = _time.GetTimestamp() + (long)(window.TotalSeconds * _time.Frequency); + long deadline = TickMath.DueAt(_time.GetTimestamp(), window, _time.Frequency); using var windowEnd = new FunctionalWindow(_clock, window, cancellationToken); var windowToken = windowEnd.Token; diff --git a/src/CanKit.Pro.IsoTp/IsoTpFunctionalListener.cs b/src/CanKit.Pro.IsoTp/IsoTpFunctionalListener.cs index 4a8305c..c127f82 100644 --- a/src/CanKit.Pro.IsoTp/IsoTpFunctionalListener.cs +++ b/src/CanKit.Pro.IsoTp/IsoTpFunctionalListener.cs @@ -85,7 +85,7 @@ public async Task> CollectAsync( TakeBuffered(responses, now); return responses.AsReadOnly(); } - long deadline = now + (long)(window.TotalSeconds * _time.Frequency); + long deadline = TickMath.DueAt(now, window, _time.Frequency); using var windowEnd = new FunctionalWindow(_clock, window, cancellationToken); try { diff --git a/src/CanKit.Pro.J1939/J1939NodeImpl.cs b/src/CanKit.Pro.J1939/J1939NodeImpl.cs index b4ba1f9..49203fd 100644 --- a/src/CanKit.Pro.J1939/J1939NodeImpl.cs +++ b/src/CanKit.Pro.J1939/J1939NodeImpl.cs @@ -65,7 +65,7 @@ internal sealed class J1939NodeImpl : IJ1939Node /// Ticks of as a . private TimeSpan TimeSpanFromTicks(long ticks) - => TimeSpan.FromSeconds(ticks / (double)_time.Frequency); + => TickMath.ElapsedFromTicks(ticks, _time.Frequency); private readonly J1939NodeOptions _options; private readonly J1939Name _name; private readonly ProtocolActor _actor; @@ -1800,7 +1800,7 @@ private sealed class PeriodicSchedule : IDisposable public PeriodicSchedule(J1939NodeImpl owner, J1939Message message, TimeSpan period) { _owner = owner; - _periodTicks = (long)(period.TotalSeconds * owner._time.Frequency); + _periodTicks = TickMath.ToTicks(period, owner._time.Frequency); // Snapshot the caller's payload into an owned array so the wire traffic is // frozen at Start-time regardless of whether the caller mutates the buffer that diff --git a/src/CanKit.Pro.Uds/SuppressedResponseWindows.cs b/src/CanKit.Pro.Uds/SuppressedResponseWindows.cs index dcdc071..e82adc0 100644 --- a/src/CanKit.Pro.Uds/SuppressedResponseWindows.cs +++ b/src/CanKit.Pro.Uds/SuppressedResponseWindows.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Diagnostics; +using CanKit.Pro.Actor; namespace CanKit.Pro.Uds; @@ -32,7 +33,7 @@ public void Note(byte sid, long sentTimestamp, TimeSpan window, long ticksPerSec => Extend(sid, sentTimestamp + Ticks(window, ticksPerSecond)); internal static long Ticks(TimeSpan window, long ticksPerSecond) - => (long)(window.TotalSeconds * ticksPerSecond); + => TickMath.ToTicks(window, ticksPerSecond); /// Moves the window for out to , if later. public void Extend(byte sid, long until) @@ -96,6 +97,6 @@ public static TimeSpan Remaining(long until) public static TimeSpan Remaining(long until, long now, long ticksPerSecond) { var ticks = until - now; - return ticks <= 0 ? TimeSpan.Zero : TimeSpan.FromSeconds((double)ticks / ticksPerSecond); + return ticks <= 0 ? TimeSpan.Zero : TickMath.RemainingFromTicks(ticks, ticksPerSecond); } } diff --git a/src/CanKit.Pro.Uds/UdsClientImpl.cs b/src/CanKit.Pro.Uds/UdsClientImpl.cs index b7da0d7..8cb2279 100644 --- a/src/CanKit.Pro.Uds/UdsClientImpl.cs +++ b/src/CanKit.Pro.Uds/UdsClientImpl.cs @@ -1529,7 +1529,7 @@ private TimeSpan ElapsedSince(long startTimestamp, long? endTimestamp = null) var end = endTimestamp ?? Now(); var ticks = end - startTimestamp; if (ticks <= 0) return TimeSpan.Zero; - return TimeSpan.FromSeconds((double)ticks / _time.Frequency); + return TickMath.ElapsedFromTicks(ticks, _time.Frequency); } private void ThrowIfDisposed() diff --git a/tests/CanKit.Pro.Tests/Infrastructure/ManualTimeSource.cs b/tests/CanKit.Pro.Tests/Infrastructure/ManualTimeSource.cs index e31a2a0..28d22a8 100644 --- a/tests/CanKit.Pro.Tests/Infrastructure/ManualTimeSource.cs +++ b/tests/CanKit.Pro.Tests/Infrastructure/ManualTimeSource.cs @@ -28,8 +28,14 @@ internal sealed class ManualTimeSource : ITimeSource private long _timestamp; private long _reads; + public ManualTimeSource() : this(TimeSpan.TicksPerSecond) { } + + /// A source ticking times a second: 10 000 000 is + /// Stopwatch on Windows, 1 000 000 000 on Linux and macOS. + public ManualTimeSource(long frequency) => Frequency = frequency; + /// - public long Frequency => TimeSpan.TicksPerSecond; + public long Frequency { get; } /// /// How often the actor has asked for the time since the last . @@ -50,7 +56,7 @@ public long GetTimestamp() public void Advance(TimeSpan by) { if (by < TimeSpan.Zero) throw new ArgumentOutOfRangeException(nameof(by), "A monotonic clock cannot go backwards."); - Interlocked.Add(ref _timestamp, by.Ticks); + Interlocked.Add(ref _timestamp, (long)Math.Ceiling((decimal)by.Ticks * Frequency / TimeSpan.TicksPerSecond)); } public void ResetReadCount() => Interlocked.Exchange(ref _reads, 0); diff --git a/tests/CanKit.Pro.Tests/TestCases/IsoTp/IsoTpFunctionalClientTests.cs b/tests/CanKit.Pro.Tests/TestCases/IsoTp/IsoTpFunctionalClientTests.cs index abd1fdc..5b8909a 100644 --- a/tests/CanKit.Pro.Tests/TestCases/IsoTp/IsoTpFunctionalClientTests.cs +++ b/tests/CanKit.Pro.Tests/TestCases/IsoTp/IsoTpFunctionalClientTests.cs @@ -710,8 +710,7 @@ public async Task Functional_Collect_On_An_Injected_Clock_Drains_By_That_Clocks_ var call = client.SendAndCollectAsync(new byte[] { 0x22, 0xF1, 0x90 }, window); await clock.WaitUntilTimerArmedAsync(actor, window, ShortTimeout); // The clock has not moved since the collection read it, so its deadline is this. - long deadline = actor.TimeSource.GetTimestamp() - + (long)(window.TotalSeconds * actor.TimeSource.Frequency); + long deadline = TickMath.DueAt(actor.TimeSource.GetTimestamp(), window, actor.TimeSource.Frequency); service.Deliver(SingleFrameView(0x7E8, new byte[] { 0x62, 0xF1, 0x90, 0x01 }), deadline); service.Deliver(SingleFrameView(0x7E9, new byte[] { 0x62, 0xF1, 0x90, 0x02 }), deadline + 1); service.Deliver(SingleFrameView(0x7EA, new byte[] { 0x62, 0xF1, 0x90, 0x03 })); diff --git a/tests/CanKit.Pro.Tests/TestCases/ProtocolActorTests.cs b/tests/CanKit.Pro.Tests/TestCases/ProtocolActorTests.cs index 279effa..5be03f3 100644 --- a/tests/CanKit.Pro.Tests/TestCases/ProtocolActorTests.cs +++ b/tests/CanKit.Pro.Tests/TestCases/ProtocolActorTests.cs @@ -214,7 +214,9 @@ public async Task Schedule_Fires_Callback_After_The_Configured_Delay() using var handle = actor.Schedule(TimeSpan.FromMilliseconds(200), () => tcs.TrySetResult(true)); (await Task.WhenAny(tcs.Task, Task.Delay(TimeSpan.FromSeconds(5)))).Should().Be(tcs.Task); - sw.Elapsed.Should().BeGreaterThanOrEqualTo(TimeSpan.FromMilliseconds(150)); + // The stopwatch started before the schedule call and the actor's clock after it, so the + // wait can never be shorter than the delay: "not before it is due" is the one promise. + sw.Elapsed.Should().BeGreaterThanOrEqualTo(TimeSpan.FromMilliseconds(200)); } [Fact] diff --git a/tests/CanKit.Pro.Tests/TestCases/ProtocolActorTimerTests.cs b/tests/CanKit.Pro.Tests/TestCases/ProtocolActorTimerTests.cs index 4ee5fb9..000ca25 100644 --- a/tests/CanKit.Pro.Tests/TestCases/ProtocolActorTimerTests.cs +++ b/tests/CanKit.Pro.Tests/TestCases/ProtocolActorTimerTests.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; using AwesomeAssertions; @@ -23,6 +24,53 @@ public class ProtocolActorTimerTests { private static readonly TimeSpan Bounded = TimeSpan.FromSeconds(5); + // #20 / the 2026-09-30 review: a delay of 35 ms became due 100 ns late and one of 43 ms was + // reported 100 ns short, because both conversions went through a double. Both frequencies a + // Stopwatch has in practice are checked, over every whole millisecond up to two seconds. + [Theory] + [InlineData(10_000_000L)] + [InlineData(1_000_000_000L)] + public async Task A_Timer_Is_Armed_Exactly_As_Far_Away_As_Asked(long frequency) + { + var clock = new ManualTimeSource(frequency); + using var actor = new ProtocolActor(ActorExecutionMode.DedicatedThread, null, clock, null); + + var wrong = new List(); + for (var ms = 1; ms <= 2000; ms++) + { + var asked = TimeSpan.FromMilliseconds(ms); + using var handle = actor.Schedule(asked, () => { }); + if (await actor.NextTimerDelayAsync() != asked) wrong.Add(ms); + } + + wrong.Should().BeEmpty("the reported delay of a freshly armed timer is the delay it was armed with"); + } + + [Theory] + [InlineData(10_000_000L)] + [InlineData(1_000_000_000L)] + public async Task A_Timer_Fires_When_The_Clock_Has_Advanced_By_Exactly_Its_Delay(long frequency) + { + var clock = new ManualTimeSource(frequency); + using var actor = new ProtocolActor(ActorExecutionMode.DedicatedThread, null, clock, null); + + var late = new List(); + for (var ms = 1; ms <= 400; ms++) + { + var asked = TimeSpan.FromMilliseconds(ms); + var fired = 0; + using var handle = actor.Schedule(asked, () => Interlocked.Increment(ref fired)); + clock.Advance(asked); + // Two round trips: the first wakes the loop, the second returns after the timers that + // became due have run. + await actor.PostAsync(() => 0); + await actor.PostAsync(() => 0); + if (Volatile.Read(ref fired) != 1) late.Add(ms); + } + + late.Should().BeEmpty("advancing the clock by exactly the delay makes the timer due"); + } + [Fact] public async Task Timer_Due_Times_Ignore_The_Wall_Clock_And_Follow_The_Monotonic_Source() { @@ -204,7 +252,10 @@ public async Task A_Clean_Dispose_Reports_Nothing() { // Guards the test above from passing for the wrong reason: the timeout must be reported // only when the loop genuinely could not be joined, never on every Dispose. - using var actor = new ProtocolActor(ActorExecutionMode.DedicatedThread, null, null, TimeSpan.FromMilliseconds(500)); + // The default join timeout: Dispose returns when the loop ends, not when the timeout does, + // so a long one costs nothing -- and a short one is a wall-clock margin on the thread + // wake-up that a loaded host can exceed, reporting a timeout that did not happen. + using var actor = new ProtocolActor(ActorExecutionMode.DedicatedThread, null, null, null); Exception? observed = null; actor.BackgroundExceptionOccurred += (_, ex) => observed ??= ex; @@ -223,7 +274,7 @@ public async Task ScheduleAt_Fires_At_The_Instant_Not_A_Delay_From_When_It_Was_A using var clock = new VirtualClock(); var actor = clock.NewActor(); var time = actor.TimeSource; - long Ms(int ms) => (long)(ms / 1000.0 * time.Frequency); + long Ms(int ms) => TickMath.ToTicks(TimeSpan.FromMilliseconds(ms), time.Frequency); var deadline = time.GetTimestamp() + Ms(50); // the caller's reading, and its deadline await clock.AdvanceAsync(TimeSpan.FromMilliseconds(30)); // the clock moves before the arming diff --git a/tests/CanKit.Pro.Tests/TestCases/TickMathTests.cs b/tests/CanKit.Pro.Tests/TestCases/TickMathTests.cs new file mode 100644 index 0000000..b14c3e7 --- /dev/null +++ b/tests/CanKit.Pro.Tests/TestCases/TickMathTests.cs @@ -0,0 +1,89 @@ +using System; +using System.Collections.Generic; +using AwesomeAssertions; +using CanKit.Pro.Actor; +using CanKit.Pro.Uds; +using Xunit; + +namespace CanKit.Pro.Tests.TestCases; + +/// +/// The conversions between and the ticks of a time source are exact. They +/// used to go through a double, which put about one whole millisecond in twelve a tick off, and +/// .NET Framework's TimeSpan.FromSeconds rounds to milliseconds and hid it there until the +/// actor's own conversion became exact (the 2026-09-30 review, A1). +/// +public class TickMathTests +{ + public static TheoryData Frequencies => new() { 10_000_000L, 1_000_000_000L, 1_000_000L, 1_000L }; + + [Theory] + [MemberData(nameof(Frequencies))] + public void A_Span_Of_Whole_Milliseconds_Converts_To_Exactly_Its_Ticks(long frequency) + { + var wrong = new List(); + for (var ms = 1; ms <= 2000; ms++) + { + var expected = (decimal)ms * frequency / 1000m; + if (TickMath.ToTicks(TimeSpan.FromMilliseconds(ms), frequency) != (long)Math.Ceiling(expected)) + wrong.Add(ms); + } + + wrong.Should().BeEmpty(); + } + + [Theory] + [InlineData(10_000_000L)] + [InlineData(1_000_000_000L)] + public void Converting_There_And_Back_Returns_The_Span(long frequency) + { + var wrong = new List(); + for (var ms = 1; ms <= 2000; ms++) + { + var span = TimeSpan.FromMilliseconds(ms); + var ticks = TickMath.ToTicks(span, frequency); + if (TickMath.RemainingFromTicks(ticks, frequency) != span || TickMath.ElapsedFromTicks(ticks, frequency) != span) + wrong.Add(ms); + } + + wrong.Should().BeEmpty(); + } + + // The UDS client's response windows are noted and measured in source ticks; the window of + // 100 ms the failing net48 test used is one of the values the double conversion got wrong. + [Theory] + [InlineData(10_000_000L)] + [InlineData(1_000_000_000L)] + public void A_Response_Window_Is_Counted_And_Measured_In_Exact_Ticks(long frequency) + { + var wrong = new List(); + for (var ms = 1; ms <= 2000; ms++) + { + var window = TimeSpan.FromMilliseconds(ms); + var ticks = SuppressedResponseWindows.Ticks(window, frequency); + if (ticks != (long)((decimal)ms * frequency / 1000m) + || SuppressedResponseWindows.Remaining(ticks, 0, frequency) != window) + wrong.Add(ms); + } + + wrong.Should().BeEmpty(); + } + + [Fact] + public void A_Remaining_Time_Rounds_Up_And_An_Elapsed_Time_Rounds_Down() + { + // 1 tick of a 10^9 Hz source is 1 ns: a tenth of a TimeSpan tick. + TickMath.RemainingFromTicks(1, 1_000_000_000).Should().Be(TimeSpan.FromTicks(1)); + TickMath.ElapsedFromTicks(1, 1_000_000_000).Should().Be(TimeSpan.Zero); + TickMath.RemainingFromTicks(101, 1_000_000_000).Should().Be(TimeSpan.FromTicks(2)); + TickMath.ElapsedFromTicks(101, 1_000_000_000).Should().Be(TimeSpan.FromTicks(1)); + } + + [Fact] + public void A_Span_No_Counter_Reaches_Saturates_Instead_Of_Overflowing() + { + TickMath.ToTicks(TimeSpan.MaxValue, 1_000_000_000).Should().Be(long.MaxValue); + TickMath.DueAt(5, TimeSpan.MaxValue, 1_000_000_000).Should().Be(long.MaxValue); + TickMath.DueAt(5, TimeSpan.FromMilliseconds(35), 10_000_000).Should().Be(5 + 350_000); + } +} diff --git a/tests/CanKit.Pro.Tests/TestCases/Uds/UdsExpiredDeadlineTests.cs b/tests/CanKit.Pro.Tests/TestCases/Uds/UdsExpiredDeadlineTests.cs index 71577ce..8760077 100644 --- a/tests/CanKit.Pro.Tests/TestCases/Uds/UdsExpiredDeadlineTests.cs +++ b/tests/CanKit.Pro.Tests/TestCases/Uds/UdsExpiredDeadlineTests.cs @@ -841,7 +841,7 @@ public IReadOnlyList GetReceptionsInProgress() private long FinalArrivalStamp() => _arrivalStamp + Ticks(PendingToFinalArrivalGap); private long Ticks(TimeSpan span) - => (long)(span.TotalSeconds * Frequency); + => TickMath.ToTicks(span, Frequency); public async Task ReceiveAsync(CancellationToken cancellationToken = default) => (await ReceiveWithArrivalAsync(cancellationToken).ConfigureAwait(false)).Pdu;