From 77a32a2440a9ec72ac5f70696a897ccb343e7cd4 Mon Sep 17 00:00:00 2001 From: "Igor von Nyssen (Claude Code)" Date: Tue, 30 Jun 2026 19:50:53 -0700 Subject: [PATCH 1/2] fix(telescope): serialize shared slew state and fix Init() timer leak TelescopeHardware shares its slew state (mountAxes, targetAxes, slewing, SlewState, currentRaDec, AtPark) across Kestrel request threads and the s_wTimer tick with no synchronization. StartSlewAxes' writes are not ordered against DoSlew's reads, so on weak memory (AArch64) or under JIT register caching the tick can observe SlewState==SlewRaDec while slewing is still stale-false. DoSlew bails, MoveAxes' tail switch has no SlewRaDec case, and the slew never advances: IsSlewing stays true indefinitely. The same race on the unsynchronized AtPark accessor lets a client polling AtPark after Park() miss the tick's park-completion write and spin until its own deadline. Init() compounds this. It runs on every Telescope (re)construction and allocated a fresh AutoReset System.Timers.Timer each call without stopping/disposing the previous one, so the runtime kept every prior timer alive and firing M_wTimer_Tick on the shared static slew state - one leaked tick source per reconstruction, racing the slew engine. It also reset the slew-engine state outside any lock and never cleared the slewing flag. Fixes: - Add one hardwareLock, taken on both sides: the timer tick (M_wTimer_Tick -> MoveAxes/DoSlew), StartSlewAxes, AbortSlew, SyncToTarget, SyncToAltAzm, and the IsSlewing / RightAscension / Declination / AtPark / SlewState accessors. This orders the writes against the timer's reads and gives the Slewing/RA/Dec/AtPark pollers a consistent view. Monitor is reentrant, so the tick writing these while already holding the lock is safe. - In Init(), stop/unsubscribe/Dispose the existing timer before allocating a new one, so at most one timer ever drives MoveAxes, and reset the slew-engine state (incl. slewing=false, rateMoveAxes=0) under hardwareLock so it is ordered against any in-flight tick. Co-Authored-By: Claude Opus 4.8 (1M context) --- TelescopeSimulator/TelescopeHardware.cs | 161 +++++++++++++++++++----- 1 file changed, 129 insertions(+), 32 deletions(-) diff --git a/TelescopeSimulator/TelescopeHardware.cs b/TelescopeSimulator/TelescopeHardware.cs index 9c2284b5..17917b92 100644 --- a/TelescopeSimulator/TelescopeHardware.cs +++ b/TelescopeSimulator/TelescopeHardware.cs @@ -270,6 +270,19 @@ public static class TelescopeHardware private static TrackingMode trackingMode; private static bool slewing; + /// + /// Synchronises access to the slew-engine state shared between the Kestrel + /// HTTP request threads (StartSlewAxes / SyncTo* / AbortSlew / Slewing / RA / + /// Dec) and the s_wTimer tick thread (MoveAxes -> DoSlew). These previously + /// used plain unsynchronised static fields (mountAxes, targetAxes, slewing, + /// SlewState), so StartSlewAxes' writes were not ordered against the timer + /// thread's reads: the tick can observe SlewState == SlewRaDec while still + /// seeing slewing == false, bail out of DoSlew, and never advance the slew, so + /// IsSlewing stays true indefinitely. Taking this lock on both sides restores + /// that ordering. + /// + private static readonly object hardwareLock = new object(); + private static DateTime lastUpdateTime; #endregion Private variables @@ -357,6 +370,21 @@ public static void Init() { try { + // Dispose the previously-created timer before allocating a new + // one. Init() runs on every Telescope (re)construction — including + // a "restart to a clean state" reset (e.g. issued between test or + // conformance runs). Without this the prior AutoReset timer is + // never stopped/unsubscribed; the runtime keeps it alive and it + // keeps firing M_wTimer_Tick on the shared static slew state, so + // every reset leaked another tick source racing the single slew + // engine. + if (s_wTimer != null) + { + s_wTimer.Stop(); + s_wTimer.Elapsed -= M_wTimer_Tick; + s_wTimer.Dispose(); + } + s_wTimer = new System.Timers.Timer(); s_wTimer.Interval = (int)(SharedResources.TIMER_INTERVAL * 1000); s_wTimer.Elapsed += M_wTimer_Tick; @@ -630,11 +658,21 @@ public static void Init() SlewSettleTime = 0; ChangePark(AtPark); - // invalid target position - targetRaDec = new Vector(double.NaN, double.NaN); - SlewState = SlewType.SlewNone; + // Reset the slew-engine state under hardwareLock so these writes + // are ordered against any still-in-flight M_wTimer_Tick. `slewing` + // in particular was never cleared on reset, so a slew left in + // flight by a prior Telescope instance could keep IsSlewing stuck + // true after a "restart to clean state". + lock (hardwareLock) + { + // invalid target position + targetRaDec = new Vector(double.NaN, double.NaN); + SlewState = SlewType.SlewNone; + slewing = false; + rateMoveAxes = new Vector(); - mountAxes = MountFunctions.ConvertAltAzmToAxes(altAzm); // Convert the start position AltAz coordinates into the current axes representation and set this as the simulator start position + mountAxes = MountFunctions.ConvertAltAzmToAxes(altAzm); // Convert the start position AltAz coordinates into the current axes representation and set this as the simulator start position + } LogMessage("TelescopeHardware New", string.Format("Startup mode: {0}, Azimuth: {1}, Altitude: {2}", startupMode, altAzm.X.ToString(CultureInfo.InvariantCulture), altAzm.Y.ToString(CultureInfo.InvariantCulture))); LogMessage("TelescopeHardware New", "Successfully initialised hardware"); @@ -680,7 +718,16 @@ public static void Start() //Update the Telescope Based on Timed Events private static void M_wTimer_Tick(object sender, EventArgs e) { - MoveAxes(); + // Hold hardwareLock for the entire tick so MoveAxes / DoSlew see a + // consistent, ordered view of the slew state that HTTP request threads + // mutate (StartSlewAxes / AbortSlew / SyncTo*). MoveAxes is only ever + // called from here, so this also serialises any reentrant tick that + // System.Timers.Timer (AutoReset = true) fires on a second ThreadPool + // thread when a tick overruns TIMER_INTERVAL. + lock (hardwareLock) + { + MoveAxes(); + } } /// @@ -1278,7 +1325,24 @@ public static double Altitude set { altAzm.Y = value; } } - public static bool AtPark { get; private set; } + private static bool atPark; + + /// + /// `true` once a park slew has completed. Written by + /// — from the timer-tick completion path and from StartSlewAxes, both of + /// which hold hardwareLock — and read by the Alpaca `AtPark` poller + /// on a Kestrel request thread. The lock orders the tick's write against the + /// poller's read; without it a client polling `AtPark` right after `Park()` + /// can miss the completion on weak memory (AArch64) / under JIT register + /// caching and spin until its own deadline. Same race class as the + /// IsSlewing / RightAscension / Declination accessors; this closes the + /// park-side gap. + /// + public static bool AtPark + { + get { lock (hardwareLock) { return atPark; } } + private set { lock (hardwareLock) { atPark = value; } } + } public static double Azimuth { @@ -1427,17 +1491,31 @@ public static double DeclinationRate public static double Declination { - get { return currentRaDec.Y; } - set { currentRaDec.Y = value; } + get { lock (hardwareLock) { return currentRaDec.Y; } } + set { lock (hardwareLock) { currentRaDec.Y = value; } } } public static double RightAscension { - get { return currentRaDec.X; } - set { currentRaDec.X = value; } + get { lock (hardwareLock) { return currentRaDec.X; } } + set { lock (hardwareLock) { currentRaDec.X = value; } } } - public static SlewType SlewState { get; private set; } + private static SlewType slewStateField; + + /// + /// The slew-engine state. Written under hardwareLock (timer tick, + /// StartSlewAxes, AbortSlew) and also read off the hardware thread — e.g. + /// `Telescope.cs` traffic logging reads it directly — so the accessor takes + /// the same lock for a consistent cross-thread view. (`private set` keeps + /// the writer internal; the lock is reentrant, so the tick writing it while + /// already holding the lock is fine.) + /// + public static SlewType SlewState + { + get { lock (hardwareLock) { return slewStateField; } } + private set { lock (hardwareLock) { slewStateField = value; } } + } public static SlewSpeed SlewSpeed { get; set; } @@ -1591,36 +1669,48 @@ public static bool IsSlewing { get { - if (SlewState != SlewType.SlewNone) - return true; - if (slewing) - return true; - if (rateMoveAxes.LengthSquared != 0) - return true; - //if (rateRaDec.LengthSquared != 0) // Commented out by Peter 4th August 2018 because the Telescope specification says that RightAscensionRate and DeclinationRate do not affect the Slewing state - // return true; - return slewing && rateMoveAxes.Y != 0 && rateMoveAxes.X != 0; + lock (hardwareLock) + { + if (SlewState != SlewType.SlewNone) + return true; + if (slewing) + return true; + if (rateMoveAxes.LengthSquared != 0) + return true; + //if (rateRaDec.LengthSquared != 0) // Commented out by Peter 4th August 2018 because the Telescope specification says that RightAscensionRate and DeclinationRate do not affect the Slewing state + // return true; + return slewing && rateMoveAxes.Y != 0 && rateMoveAxes.X != 0; + } } } public static void AbortSlew() { - slewing = false; - rateMoveAxes = new Vector(); - rateRaDecOffsetInternal = new Vector(); - SlewState = SlewType.SlewNone; + lock (hardwareLock) + { + slewing = false; + rateMoveAxes = new Vector(); + rateRaDecOffsetInternal = new Vector(); + SlewState = SlewType.SlewNone; + } } public static void SyncToTarget() { - mountAxes = MountFunctions.ConvertRaDecToAxes(targetRaDec, true); - UpdatePositions(); + lock (hardwareLock) + { + mountAxes = MountFunctions.ConvertRaDecToAxes(targetRaDec, true); + UpdatePositions(); + } } public static void SyncToAltAzm(double targetAzimuth, double targetAltitude) { - mountAxes = MountFunctions.ConvertAltAzmToAxes(new Vector(targetAzimuth, targetAltitude)); - UpdatePositions(); + lock (hardwareLock) + { + mountAxes = MountFunctions.ConvertAltAzmToAxes(new Vector(targetAzimuth, targetAltitude)); + UpdatePositions(); + } } public static void StartSlewRaDec(double rightAscension, double declination, bool doSideOfPier) @@ -1661,10 +1751,17 @@ public static void StartSlewAxes(double primaryAxis, double secondaryAxis, SlewT /// The position. public static void StartSlewAxes(Vector targetPosition, SlewType slewState) { - targetAxes = targetPosition; - SlewState = slewState; - slewing = true; - ChangePark(false); + // Order these writes against the timer thread's reads in DoSlew. Without + // the lock the tick can see slewing == false (stale) after SlewState has + // already become SlewRaDec, bail out of DoSlew, and wedge IsSlewing at + // true forever. + lock (hardwareLock) + { + targetAxes = targetPosition; + SlewState = slewState; + slewing = true; + ChangePark(false); + } } public static void Park() From bfb473e14784f1a3a6b02854ad1ed04489e91656 Mon Sep 17 00:00:00 2001 From: "Igor von Nyssen (Claude Code)" Date: Tue, 30 Jun 2026 19:52:05 -0700 Subject: [PATCH 2/2] fix(telescope): don't wedge IsSlewing when a GEM goto crosses the HA limit On a German equatorial mount, DoSlew rotates the primary axis the shortest way (delta wrapped to (-180, 180]). When a goto's target sits ~180 deg away in axis space - e.g. a sync onto one pier side followed by a slew whose natural side is the other - that shortest rotation can drive the primary axis through the hour-angle software limit (180 + hourAngleLimit). CheckAxisLimits then undoes the move every 100 ms tick, so `finished` is never reached, `slewing` stays true, and IsSlewing is stuck true forever; the client polls until it times out. Fix, in two parts: - DoSlew: when the shortest-path primary rotation would leave the GEM mechanical range [-hourAngleLimit, 180 + hourAngleLimit] but the SAME target is reachable the other way round, take the longer rotation. It reaches the identical target axes, so the pier side is unchanged (no SideOfPier / conformance impact), and it is a no-op for any slew whose short path stays in range. - A no-progress guard in MoveAxes: if a slew makes < slewSpeedSlow/2 of progress for 0.5 s it is stopped (Slewing -> false) instead of reporting IsSlewing forever - a backstop for genuinely unreachable targets. Tracking drift is far below the threshold, so healthy slews never trip it. Verified: the former-wedge geometry now converges (~10 s to target, on the natural pier side) instead of hanging; ConformU telescope conformance reports "SideOfPier Write OK" and the guard never fires. Co-Authored-By: Claude Opus 4.8 (1M context) --- TelescopeSimulator/TelescopeHardware.cs | 68 ++++++++++++++++++++++++- 1 file changed, 67 insertions(+), 1 deletion(-) diff --git a/TelescopeSimulator/TelescopeHardware.cs b/TelescopeSimulator/TelescopeHardware.cs index 17917b92..75471973 100644 --- a/TelescopeSimulator/TelescopeHardware.cs +++ b/TelescopeSimulator/TelescopeHardware.cs @@ -270,6 +270,18 @@ public static class TelescopeHardware private static TrackingMode trackingMode; private static bool slewing; + // Slew no-progress guard. Tracks per-tick slew progress so a slew that + // can make no headway — e.g. CheckAxisLimits + // keeps undoing a move that drives the primary axis into the hour-angle + // limit — is stopped instead of leaving IsSlewing stuck true forever. + private static double lastSlewProgressX; + private static double lastSlewProgressY; + private static int slewStallTicks; + // 5 * TIMER_INTERVAL(0.1 s) = 0.5 s of no real progress => give up. + // Tracking drift (~0.0004 deg/tick) is far below slewSpeedSlow/2, so a + // healthy slew (>= slewSpeedSlow/tick) never trips this. + private const int SLEW_STALL_TICKS = 5; + /// /// Synchronises access to the slew-engine state shared between the Kestrel /// HTTP request threads (StartSlewAxes / SyncTo* / AbortSlew / Slewing / RA / @@ -875,7 +887,7 @@ private static void MoveAxes() // update the displayed values UpdatePositions(); - // check and update slew state + // check and update slew state switch (SlewState) { case SlewType.SlewSettle: @@ -887,6 +899,35 @@ private static void MoveAxes() break; } + // No-progress guard. If DoSlew did not finish the + // slew this tick (slewing still true) yet the axes did not actually + // advance, the slew is wedged — typically CheckAxisLimits is undoing a + // move into the hour-angle limit. A real mount stops at its limit; stop + // too, rather than reporting IsSlewing == true forever. Tracking drift + // (~0.0004 deg/tick) is far below slewSpeedSlow/2, so a healthy slew + // (which advances >= slewSpeedSlow/tick) never trips this. + if (slewing) + { + double progressed = Math.Abs(mountAxes.X - lastSlewProgressX) + + Math.Abs(mountAxes.Y - lastSlewProgressY); + if (progressed < slewSpeedSlow / 2.0) + { + if (++slewStallTicks >= SLEW_STALL_TICKS) + { + slewing = false; + SlewState = SlewType.SlewNone; + slewStallTicks = 0; + LogMessage("DoSlew", "Slew stopped: axis limit reached, target unreachable in this pier state"); + } + } + else + { + slewStallTicks = 0; + } + lastSlewProgressX = mountAxes.X; + lastSlewProgressY = mountAxes.Y; + } + // List changes this cycle TL.LogMessage(LogLevel.Verbose, $"MoveAxes (Final)", $"RA: {currentRaDec.X.ToHMS()}, Dec: {currentRaDec.Y.ToDMS()}, Hour angle: {Utilities.ConditionHA(SiderealTime - currentRaDec.X).ToHMS()}, Sidereal Time: {SiderealTime.ToHMS()}"); TL.LogMessage(LogLevel.Verbose, $"MoveAxes (Final)", $"Azimuth: {altAzm.X.ToDMS()}, Altitude: {altAzm.Y.ToDMS()}, Pointing state: {SideOfPier}"); @@ -1760,6 +1801,9 @@ public static void StartSlewAxes(Vector targetPosition, SlewType slewState) targetAxes = targetPosition; SlewState = slewState; slewing = true; + slewStallTicks = 0; + lastSlewProgressX = mountAxes.X; + lastSlewProgressY = mountAxes.Y; ChangePark(false); } } @@ -1925,6 +1969,28 @@ private static Vector DoSlew() if (delta < -180) delta += 360; if (delta > 180) delta -= 360; } + + // The shortest-path rotation above can drive the primary axis through + // the GEM hour-angle limit (CheckAxisLimits then + // undoes every step, so the slew never finishes and IsSlewing wedges + // forever). When that happens but the SAME target is reachable the + // other way round, take the longer rotation instead. This reaches the + // identical target (so the pier side is unchanged — no conformance + // impact) and is a no-op for any slew whose short path stays in range. + if (alignmentMode == AlignmentMode.GermanPolar) + { + double endShort = mountAxes.X + delta; + if (endShort < -hourAngleLimit || endShort > 180.0 + hourAngleLimit) + { + double deltaLong = delta + (delta > 0 ? -360.0 : 360.0); + double endLong = mountAxes.X + deltaLong; + if (endLong >= -hourAngleLimit && endLong <= 180.0 + hourAngleLimit) + { + delta = deltaLong; + } + } + } + int signDelta = delta < 0 ? -1 : +1; delta = Math.Abs(delta);