[education/kstars] kstars: Fix rotator auto-reverse detection and add direction-parity correction
Jasem Mutlaq <[email protected]>
| Newsgroups | gmane.comp.kde.cvs |
|---|---|
| Message-ID | <[email protected]> |
Git commit 2ee1010528e983588c1c6eac319792a2e6bcbacf by Jasem Mutlaq. Committed on 30/07/2026 at 12:51. Pushed by mutlaqja into branch 'master'. Fix rotator auto-reverse detection and add direction-parity correction The wrong-direction detector in align_solver.cpp never fired: the PA error tracker it relies on (m_PreviousPAError) was unconditionally cleared before the confirming solve could ever check it, both when the rotator reported reaching its (unverified, formula-predicted) target angle and when the mount slewed as part of the same post-rotation sync/verification cycle. This let a rotator that physically turns opposite to what KStars' offset math assumes silently chase a wrong position angle indefinitely, each retry making the error worse instead of better. - align_goto.cpp: stop clearing m_PreviousPAError/m_RotatorAutoReversed on the rotator "reached" event (a prediction, not a solve-verified fact); guard the mount-slew clear with !RotatorGOTO so it doesn't wipe the tracker mid-verification-cycle. - rotatorutils.cpp/h: add a software-only direction-parity correction, independent of the driver-mirrored ROTATOR_REVERSE state, combined via XOR wherever direction is used. Also fixes calcCameraAngle() and calcOffsetAngle(), which never branched on reversed state at all (latent bug, harmless under the old driver-reversal design, but required once direction correction is handled purely in software). - align_solver.cpp: when wrong direction is detected, trial the parity flag and immediately recalibrate the offset from the measurement already in hand (without this the retry reuses a stale offset and false-aborts a correct parity flip). Persist the correction only once a retry confirms it actually worked; revert it if the retry is still wrong. - kstars.kcfg/opsalign.ui: expose the learned parity as a plain "Rotator direction reversed" checkbox in Align settings (below Flip Policy), so it can also be set manually and doesn't require touching the driver's own ROTATOR_REVERSE switch. - align_settings.cpp: sync RotatorUtils to the active train from Align::refreshOpticalTrain() too, not just Capture's rotator panel, so m_Mount/pierside state can't go stale if Align switches trains first; track the pierSideChanged connection so re-initializing from multiple call sites doesn't stack duplicate connections. Co-Authored-By: Claude Sonnet 5 <[email protected]> M +14 -6 kstars/ekos/align/align_goto.cpp M +7 -0 kstars/ekos/align/align_settings.cpp M +21 -12 kstars/ekos/align/align_solver.cpp M +10 -0 kstars/ekos/align/opsalign.ui M +49 -8 kstars/ekos/auxiliary/rotatorutils.cpp M +30 -0 kstars/ekos/auxiliary/rotatorutils.h M +5 -1 kstars/kstars.kcfg https://invent.kde.org/education/kstars/-/commit/2ee1010528e983588c1c6eac319792a2e6bcbacf diff --git a/kstars/ekos/align/align_goto.cpp b/kstars/ekos/align/align_goto.cpp index edd5ca9064..eb213ffd0a 100644 --- a/kstars/ekos/align/align_goto.cpp +++ b/kstars/ekos/align/align_goto.cpp @@ -276,7 +276,11 @@ void Align::updateProperty(INDI::Property prop) // Mount is slewing — the camera PA will change relative to the sky, // so any previously stored PA error is no longer valid for comparing // rotation direction. Reset to avoid false-positive wrong-direction detection. - if (m_PreviousPAError >= 0) + // Skip this while RotatorGOTO is set: that flag is true exactly during the + // sync+slew-back that follows a rotation, i.e. the mount motion here is part + // of the same verification cycle the tracker is meant to survive until the + // confirming solve runs (see checkIfRotationRequired()/align_solver.cpp). + if (m_PreviousPAError >= 0 && !RotatorGOTO) { qCDebug(KSTARS_EKOS_ALIGN) << "Mount slew started. Clearing previous PA error tracker."; m_PreviousPAError = -1; @@ -347,11 +351,15 @@ void Align::updateProperty(INDI::Property prop) if (diff <= Options::astrometryRotatorThreshold()) { appendLogText(i18n("Rotator reached camera position angle.")); - // Rotation succeeded — clear the previous PA error tracker - // so it doesn't cause false-positive wrong-direction detection later. - m_PreviousPAError = -1; - m_RotatorAutoReversed = false; - qCDebug(KSTARS_EKOS_ALIGN) << "Rotator reached target PA. Clearing previous PA error tracker."; + // NOTE: this "reached" signal only means the raw encoder matches the value + // that was *commanded* (computed from the current offset/parity model) — it is + // not confirmed by an actual plate solve. Deliberately do NOT clear + // m_PreviousPAError/m_RotatorAutoReversed here: doing so would erase the + // baseline the wrong-direction check in align_solver.cpp needs to compare + // against once the next (real, solve-verified) measurement comes in. Both + // trackers are only cleared once that check has actually run and reached a + // verdict (success, retry, or abort). + qCDebug(KSTARS_EKOS_ALIGN) << "Rotator reached target PA."; if (m_RotateBeforeSolve) { // Rotate-first optimization: skip stale sync, go directly to capture diff --git a/kstars/ekos/align/align_settings.cpp b/kstars/ekos/align/align_settings.cpp index b26af7051f..797ac7fecd 100644 --- a/kstars/ekos/align/align_settings.cpp +++ b/kstars/ekos/align/align_settings.cpp @@ -19,6 +19,7 @@ #include "ekos/auxiliary/opticaltrainmanager.h" #include "ekos/auxiliary/opticaltrainsettings.h" #include "ekos/auxiliary/profilesettings.h" +#include "ekos/auxiliary/rotatorutils.h" #include "ekos/auxiliary/stellarsolverprofileeditor.h" #include "ekos/manager.h" #include "indi/indirotator.h" @@ -327,6 +328,12 @@ void Align::refreshOpticalTrain() auto rotator = OpticalTrainManager::Instance()->getRotator(name); setRotator(rotator); + // RotatorUtils is a singleton shared across Capture and Align; make sure it's + // synced to whatever train Align is now pointing at (in particular m_Mount, which + // drives the flipped-mount/pierside logic used by the rotator angle math) rather + // than relying on Capture's rotator panel having already (re)initialized it for + // this train. + RotatorUtils::Instance()->initRotatorUtils(name); auto dustcap = OpticalTrainManager::Instance()->getDustCap(name); setDustCap(dustcap); diff --git a/kstars/ekos/align/align_solver.cpp b/kstars/ekos/align/align_solver.cpp index d8d90f5045..049109a9cc 100644 --- a/kstars/ekos/align/align_solver.cpp +++ b/kstars/ekos/align/align_solver.cpp @@ -908,31 +908,35 @@ void Align::solverFinished(double orientation, double ra, double dec, double pix << "Auto-reversed:" << m_RotatorAutoReversed; if (newPAError > m_PreviousPAError + 0.5) { - // First wrong-direction detection: try auto-reversing the rotator + // First wrong-direction detection: try correcting via the software-only + // per-train parity flag. We never touch the driver's ROTATOR_REVERSE + // switch — that would depend on trusting this specific driver's + // implementation of it. if (!m_RotatorAutoReversed) { - auto reverseProperty = m_Rotator->getSwitch("ROTATOR_REVERSE"); - bool isReversed = reverseProperty ? (reverseProperty[0].getState() == ISS_ON) : false; - bool newReversed = !isReversed; - appendLogText(i18n("Rotator is moving in the wrong direction. " - "Automatically reversing rotator direction and retrying...")); + "Automatically compensating rotator direction and retrying...")); qCDebug(KSTARS_EKOS_ALIGN) << "Rotator wrong direction detected." << "Previous PA error:" << m_PreviousPAError << "New PA error:" << newPAError - << "Auto-reversing direction to:" - << (newReversed ? "reversed" : "normal"); + << "Toggling software parity correction."; + + RotatorUtils::Instance()->trialToggleParity(); + // Recalibrate the offset for the newly-flipped model using the + // measurement we already have. Without this, the retry would reuse + // an offset fit under the old (wrong) model and very likely still + // miss, making a correct parity flip look like a second failure. + double correctedOffset = RotatorUtils::Instance()->calcOffsetAngle(sRawAngle, solverPA); + RotatorUtils::Instance()->updateOffset(correctedOffset); - m_Rotator->setReversed(newReversed); - RotatorUtils::Instance()->setReversed(newReversed); m_RotatorAutoReversed = true; m_PreviousPAError = -1; - // Re-issue the rotation command with the reversed direction + // Re-issue the rotation command with the corrected parity checkIfRotationRequired(); return; } - // Auto-reverse already attempted — abort + // Parity correction already attempted — abort appendLogText(i18n("Rotator is moving in the wrong direction. " "The position angle error increased from %1° to %2° after rotation. " "Auto-reverse was already tried. Please check rotator configuration.", @@ -941,6 +945,7 @@ void Align::solverFinished(double orientation, double ra, double dec, double pix qCDebug(KSTARS_EKOS_ALIGN) << "Rotator wrong direction detected after auto-reverse." << "Previous PA error:" << m_PreviousPAError << "New PA error:" << newPAError; + RotatorUtils::Instance()->revertParity(); m_TargetPositionAngle = std::numeric_limits<double>::quiet_NaN(); m_PreviousPAError = -1; m_RotatorAutoReversed = false; @@ -951,6 +956,10 @@ void Align::solverFinished(double orientation, double ra, double dec, double pix return; } // PA error did not increase — rotation was in the correct direction. + // If this confirms a parity trial, persist it so this train never has to + // rediscover its direction again. + if (m_RotatorAutoReversed) + RotatorUtils::Instance()->commitParity(); // Reset error tracking since the rotation succeeded. qCDebug(KSTARS_EKOS_ALIGN) << "PA error decreased or unchanged — resetting previous error tracker."; m_PreviousPAError = -1; diff --git a/kstars/ekos/align/opsalign.ui b/kstars/ekos/align/opsalign.ui index 68eefea43b..0ef2ff5f54 100644 --- a/kstars/ekos/align/opsalign.ui +++ b/kstars/ekos/align/opsalign.ui @@ -367,6 +367,16 @@ </property> </widget> </item> + <item row="3" column="0" colspan="3"> + <widget class="QCheckBox" name="kcfg_RotatorParityReversed"> + <property name="toolTip"> + <string><html><head/><body><p>Enable if the rotator physically turns opposite to the direction expected. Detected and set automatically the first time the wrong-direction check confirms it; can also be set manually.</p></body></html></string> + </property> + <property name="text"> + <string>Rotator direction reversed</string> + </property> + </widget> + </item> </layout> </widget> </item> diff --git a/kstars/ekos/auxiliary/rotatorutils.cpp b/kstars/ekos/auxiliary/rotatorutils.cpp index edfb3b721d..c5b0e3499a 100644 --- a/kstars/ekos/auxiliary/rotatorutils.cpp +++ b/kstars/ekos/auxiliary/rotatorutils.cpp @@ -49,14 +49,21 @@ void RotatorUtils::initRotatorUtils(const QString &train) m_Offset = Options::pAOffset(); m_Mount = Ekos::OpticalTrainManager::Instance()->getMount(train); + disconnect(m_PierSideConnection); if (m_Mount) { - connect(m_Mount, &ISD::Mount::pierSideChanged, this, [this] (ISD::Mount::PierSide Side) + m_PierSideConnection = connect(m_Mount, &ISD::Mount::pierSideChanged, this, [this] (ISD::Mount::PierSide Side) { m_flippedMount = (Side != m_CalPierside); Q_EMIT changedPierside(Side); }); } + + // Load the learned software-only direction parity, if one was ever confirmed + // (see commitParity()) or set manually via the "Rotator direction reversed" + // checkbox in Align settings. This is a hardware fact about the rotator, so it + // should not need to be rediscovered every session. + m_PersistedParityReversed = m_ParityReversed = Options::rotatorParityReversed(); } double RotatorUtils::calcRotatorAngle(double PositionAngle) @@ -69,7 +76,9 @@ double RotatorUtils::calcRotatorAngle(double PositionAngle) // negated (offset-relative) angle so that the driver's own reversal cancels it out: // normal: command = range360(PA - offset) // reversed: command = range360(offset - PA) - if (m_Reversed) + // effectiveReversed() combines the driver-mirrored reversal (m_Reversed) with the + // software-only per-train parity correction (m_ParityReversed) via XOR. + if (effectiveReversed()) return KSUtils::range360(m_Offset - PositionAngle); else return KSUtils::range360(PositionAngle - m_Offset); @@ -80,16 +89,40 @@ void RotatorUtils::setReversed(bool reversed) m_Reversed = reversed; } +void RotatorUtils::trialToggleParity() +{ + m_ParityReversed = !m_ParityReversed; +} + +void RotatorUtils::commitParity() +{ + Options::setRotatorParityReversed(m_ParityReversed); + m_PersistedParityReversed = m_ParityReversed; +} + +void RotatorUtils::revertParity() +{ + m_ParityReversed = m_PersistedParityReversed; +} + double RotatorUtils::calcCameraAngle(double RotatorAngle, bool flippedImage) { double PositionAngle = 0; - if (RotatorAngle > 180) + // Algebraic inverse of calcRotatorAngle(): when effectively reversed, the raw angle was + // commanded as range360(offset - PA), so PA = offset - R here (instead of PA = R + offset). + if (effectiveReversed()) { - PositionAngle = (RotatorAngle - 360) + m_Offset; + if (RotatorAngle > 180) + PositionAngle = m_Offset - (RotatorAngle - 360); + else + PositionAngle = m_Offset - RotatorAngle; } else { - PositionAngle = RotatorAngle + m_Offset; + if (RotatorAngle > 180) + PositionAngle = (RotatorAngle - 360) + m_Offset; + else + PositionAngle = RotatorAngle + m_Offset; } if (!m_flippedMount != !flippedImage) // XOR { @@ -109,13 +142,21 @@ double RotatorUtils::calcCameraAngle(double RotatorAngle, bool flippedImage) double RotatorUtils::calcOffsetAngle(double RotatorAngle, double PositionAngle) { double OffsetAngle = 0; - if (RotatorAngle > 180) + // Algebraic inverse of calcRotatorAngle(): when effectively reversed, R = offset - PA, + // so offset = PA + R here (instead of offset = PA - R). + if (effectiveReversed()) { - OffsetAngle = PositionAngle - (RotatorAngle - 360); + if (RotatorAngle > 180) + OffsetAngle = PositionAngle + (RotatorAngle - 360); + else + OffsetAngle = PositionAngle + RotatorAngle; } else { - OffsetAngle = PositionAngle - RotatorAngle; + if (RotatorAngle > 180) + OffsetAngle = PositionAngle - (RotatorAngle - 360); + else + OffsetAngle = PositionAngle - RotatorAngle; } if (m_flippedMount) { diff --git a/kstars/ekos/auxiliary/rotatorutils.h b/kstars/ekos/auxiliary/rotatorutils.h index 300f23f4d5..24653e27a2 100644 --- a/kstars/ekos/auxiliary/rotatorutils.h +++ b/kstars/ekos/auxiliary/rotatorutils.h @@ -31,6 +31,23 @@ class RotatorUtils : public QObject * range360(PA - offset). */ void setReversed(bool reversed); + + /** + * @brief parityReversed Whether the software-only direction correction (Options::RotatorParityReversed, + * exposed as the "Rotator direction reversed" checkbox in Align settings) is currently active. + * Independent of setReversed()/m_Reversed, which mirrors the driver's own ROTATOR_REVERSE switch. + */ + bool parityReversed() const + { + return m_ParityReversed; + } + /// Flip the in-memory parity flag as a trial correction (not persisted). + void trialToggleParity(); + /// Persist the current trial parity value (and update the Align settings checkbox) as confirmed-good. + void commitParity(); + /// Undo an unconfirmed trial, restoring the last persisted parity value. + void revertParity(); + void setImagePierside(ISD::Mount::PierSide ImgPierside); ISD::Mount::PierSide getMountPierside(); double DiffPA(double diff); @@ -48,7 +65,20 @@ class RotatorUtils : public QObject double m_Offset {0}; bool m_flippedMount {false}; bool m_Reversed {false}; + // Software-only direction correction (global Options::RotatorParityReversed), independent + // of m_Reversed (which mirrors the driver's own ROTATOR_REVERSE switch). See parityReversed(). + bool m_ParityReversed {false}; + bool m_PersistedParityReversed {false}; + /// Combined effective direction: driver-mirrored reversal XOR software parity correction. + bool effectiveReversed() const + { + return m_Reversed != m_ParityReversed; + } ISD::Mount *m_Mount {nullptr}; + // initRotatorUtils() is called from multiple places (Capture's rotator panel, Align's + // refreshOpticalTrain()) whenever a train is (re)activated. Track the pierSideChanged + // connection so re-initializing doesn't stack duplicate connections onto the same mount. + QMetaObject::Connection m_PierSideConnection; double m_StartAngle, m_EndAngle {0}; double m_ShiftAngle, m_DiffAngle {0}; QTime m_StartTime, m_CurrentTime; diff --git a/kstars/kstars.kcfg b/kstars/kstars.kcfg index 1f39eaa85a..cba92d6fcd 100644 --- a/kstars/kstars.kcfg +++ b/kstars/kstars.kcfg @@ -2903,7 +2903,11 @@ <entry name="AstrometryFlipRotationAllowed" type="Bool"> <whatsthis>PA 180° rotation for rotator is accepted after mount flip.</whatsthis> <default>true</default> - </entry> + </entry> + <entry name="RotatorParityReversed" type="Bool"> + <whatsthis>The rotator physically turns opposite to the direction KStars' position-angle math assumes. Learned automatically the first time the auto-reverse wrong-direction check confirms it, or can be set manually.</whatsthis> + <default>false</default> + </entry> <entry name="AstrometrySolverWCS" type="Bool"> <label>World Coordinate System (WCS). WCS is used to encode RA/DEC coordinates in captured CCD images.</label> <default>true</default>