diff --git a/TelescopeSimulator/TelescopeHardware.cs b/TelescopeSimulator/TelescopeHardware.cs index 9c2284b5..75471973 100644 --- a/TelescopeSimulator/TelescopeHardware.cs +++ b/TelescopeSimulator/TelescopeHardware.cs @@ -270,6 +270,31 @@ 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 / + /// 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 +382,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 +670,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 +730,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(); + } } /// @@ -828,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: @@ -840,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}"); @@ -1278,7 +1366,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 +1532,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 +1710,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 +1792,20 @@ 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; + slewStallTicks = 0; + lastSlewProgressX = mountAxes.X; + lastSlewProgressY = mountAxes.Y; + ChangePark(false); + } } public static void Park() @@ -1828,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);