Alignment: deprecate old Transform* shims, migrate all in-tree callers to *JD#2388
Merged
Merged
Conversation
…CelestialJD Add new *JD variants to MathPlugin and all built-in plugins (BasicMathPlugin, NearestMathPlugin, SPKMathPlugin, DummyMathPlugin) that accept an absolute Julian Date instead of reading the system clock internally. The old TransformCelestialToTelescope / TransformTelescopeToCelestial methods become thin shims (resolving ln_get_julian_from_sys() + JulianOffset) to preserve binary ABI compatibility with external DSO plugins. MathPluginManagement gains matching *JD dispatch methods; its old methods are also reduced to shims. Test suite migrated from the joff = fixedJD - ln_get_julian_from_sys() hack to direct *JD calls with a static constexpr fixedJD, making all tests deterministic and clock-independent.
Fix MathPlugin default TransformCelestialToTelescopeJD/TransformTelescopeToCelestialJD fallback to pass JulianDate - ln_get_julian_from_sys() as the offset, so legacy external plugins that add ln_get_julian_from_sys() internally recover the correct absolute JD. Add override keyword to *JD method declarations in BasicMathPlugin, NearestMathPlugin, and DummyMathPlugin for consistency with SPKMathPlugin and compiler-enforced signature checking. Remove dead pTransformCelestialToTelescope and pTransformTelescopeToCelestial function pointer members from MathPluginManagement (dispatch now goes through direct virtual calls on pLoadedMathPlugin). Remove stale commented-out ln_get_julian_from_sys() lines from BasicMathPlugin.cpp and the trivial JDD alias variables from NearestMathPlugin.cpp. Add two regression tests (LegacyPlugin_JD_Fallback_*) that verify the fallback offset computation is correct for legacy external plugins.
…s to *JD Mark MathPluginManagement::TransformCelestialToTelescope and TransformTelescopeToCelestial [[deprecated]] so callers get compiler warnings. Thread an explicit JD parameter through AlignmentSubsystemForDrivers (default = ln_get_julian_from_sys() for source compatibility), and update all in-tree driver and example call sites to use TransformCelestialToTelescopeJD / TransformTelescopeToCelestialJD directly. skywatcherAPIMount non-zero offset sites (JulianOffset axis, bracket tracking) now compute the absolute JD explicitly before calling *JD instead of passing a relative offset. The JDnow variable is hoisted before the if/else so both branches share the same sample. External math plugins are unaffected: MathPlugin.h pure virtuals are intentionally left un-deprecated.
Contributor
Author
|
How would we fix the drivers in indi-3rdparty? i.e. what is the sequence of events to make a clean transition? If we merge this, then a build with Merge #2387, then fix 3rdparty, then merge #2388? |
Contributor
|
Thanks. Now the 3rd party drivers needs to be fixed next |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reviewer note
This PR is stacked on #2387 and targets that branch as its base. After #2387 merges to master, this branch will be rebased onto master and the
PR base retargeted accordingly.
Summary
Follow-up to #2387. That PR added
TransformCelestialToTelescopeJD/TransformTelescopeToCelestialJDto the math plugin stack and converted the oldTransformCelestialToTelescope/TransformTelescopeToCelestialmethods inMathPluginManagementinto compat shims. This PR completes Phase 2 and Phase 3 ofthe migration plan described in that PR's follow-up section:
[[deprecated]]inMathPluginManagement.hso any caller that has not yet migrated gets a compiler warning.
JDparameter throughAlignmentSubsystemForDriversand update every in-tree driver and example callsite to use the
*JDvariants directly.Changes
libs/alignment/MathPluginManagement.hAdded
[[deprecated("Use TransformCelestialToTelescopeJD with an explicit Julian Date")]]to both old public shim methods. Their implementations are unchanged.
Note: the old pure-virtual methods in
MathPlugin.hare intentionally leftun-deprecated. External math plugins must still implement them; their deprecation is
Phase 4 work after the broader ecosystem migrates.
libs/alignment/AlignmentSubsystemForDrivers.h/.cppAdded
double JD = ln_get_julian_from_sys()as a trailing default parameter to allsix public helper methods:
AddAlignmentEntryEquatorial/AddAlignmentEntryAltAz-- useJDforObservationJulianDate(previously always capturedln_get_julian_from_sys())SkyToTelescopeEquatorial/SkyToTelescopeAltAz-- callTransformCelestialToTelescopeJD(ra, dec, JD, TDV)instead of the old shimTelescopeEquatorialToSky/TelescopeAltAzToSky-- callTransformTelescopeToCelestialJD(TDV, ra, dec, JD)instead of the old shimExisting callers that omit the
JDargument see no source change and identicalbehavior (the default evaluates
ln_get_julian_from_sys()at the call site).Callers that want simulated time support can now pass an explicit JD.
Driver and example call sites
All remaining in-tree uses of the old
MathPluginManagementshims are updated tocall
*JDdirectly withln_get_julian_from_sys():drivers/telescope/skywatcherAPIMount.cppdrivers/telescope/astrotrac.cppdrivers/telescope/telescope_simulator.cppdrivers/telescope/temmadriver.cppdrivers/telescope/dsc.cppexamples/tutorial_seven/simple_telescope_simulator.cppskywatcherAPIMountnon-zero offset sites (JulianOffsetaxis property and brackettracking) now compute the absolute JD explicitly
(
ln_get_julian_from_sys() + JDOffset) instead of passing a relative offset throughthe shim. The
JDnowvariable used by the bracket tracking else-branch is hoistedbefore the if/else so both branches sample the clock at the same instant.
Backward compatibility
No behavior change for any existing caller. All drivers that go through
AlignmentSubsystemForDriverspick up the defaultJDargument and continue tocall the system clock as before. Third-party drivers (
indi-celestronaux,indi-eqmod,etc.) that call
MathPluginManagementdirectly will see deprecation warnings ontheir next rebuild but continue to function correctly through the shim.
Test results
Follow-up (not in this PR)
indi-3rdparty driver migration: Third-party drivers that call
TransformCelestialToTelescope/TransformTelescopeToCelestialdirectly(notably
indi-celestronauxandindi-eqmod) will see deprecation warnings ontheir next rebuild. A follow-up PR to
indi-3rdpartyshould update those callsites to use
*JDwith explicitln_get_julian_from_sys(), mirroring thepattern applied here.
Phase 4 -- Remove: Once external plugins have migrated, the compat layer can be
fully removed: make
*JDpure virtual inMathPlugin.h, remove the old purevirtuals and their shim overrides in built-in plugins, remove the deprecated
MathPluginManagementshim methods. The full sequence is described in thefollow-up section of #2387.