Skip to content

Alignment: add TransformCelestialToTelescopeJD / TransformTelescopeToCelestialJD#2387

Open
ckemper67 wants to merge 2 commits into
indilib:masterfrom
ckemper67:feat/mathplugin-jd-refactor
Open

Alignment: add TransformCelestialToTelescopeJD / TransformTelescopeToCelestialJD#2387
ckemper67 wants to merge 2 commits into
indilib:masterfrom
ckemper67:feat/mathplugin-jd-refactor

Conversation

@ckemper67
Copy link
Copy Markdown
Contributor

Reviewer note

This PR changes a core alignment subsystem API (MathPlugin, MathPluginManagement)
that is used by multiple drivers inside this repo and by third-party drivers in
indi-3rdparty (notably indi-celestronaux and indi-eqmod). Would appreciate
architectural feedback before merge.

Summary

Refactors the INDI alignment subsystem so that the core transform methods no longer read
the system clock internally. Two new methods are added to MathPlugin that accept an
absolute Julian Date directly:

  • TransformCelestialToTelescopeJD(RA, Dec, JulianDate, TDV)
  • TransformTelescopeToCelestialJD(TDV, RA, Dec, JulianDate)

All built-in plugins (BasicMathPlugin / SVDMathPlugin, NearestMathPlugin,
SPKMathPlugin, DummyMathPlugin) implement the new methods as their primary
implementation. The old TransformCelestialToTelescope / TransformTelescopeToCelestial
methods become thin compat shims that resolve ln_get_julian_from_sys() + JulianOffset
and delegate to the new JD variants. MathPluginManagement gains matching *JD dispatch
methods; its old public methods are similarly shimmed.

Motivation

The old interface forced every plugin to call ln_get_julian_from_sys() internally.
This had two consequences:

  1. Tests were non-deterministic. The test suite worked around this with a
    joff = fixedJD - ln_get_julian_from_sys() hack to re-anchor transforms to a fixed
    point in time. This was fragile and obscured intent.

  2. Simulated time was impossible. Any future support for injected/simulated time
    (e.g. KStars time simulation) requires callers to supply the JD — the plugins cannot
    intercept ln_get_julian_from_sys() from the outside.

Backward compatibility

External plugins that have not yet adopted the new interface are unaffected at the
source and binary level. The new *JD methods are non-pure virtual. The base-class
default computes JulianOffset = JulianDate - ln_get_julian_from_sys() and calls the
old pure-virtual, so an unmodified external plugin recovers the correct absolute JD
when it adds ln_get_julian_from_sys() internally.

All existing drivers that call the old TransformCelestialToTelescope /
TransformTelescopeToCelestial interface — including skywatcherAPIMount,
indi-celestronaux, eqmod, telescope_simulator, astrotrac, temmadriver, dsc
continue to work unchanged through the compat shims. Non-zero JulianOffset calls
(e.g. bracket tracking in skywatcherAPIMount and celestronaux) are correctly
handled: the shim resolves the absolute JD before dispatching.

Changes

libs/alignment/MathPlugin.h/.cpp

  • Add TransformCelestialToTelescopeJD and TransformTelescopeToCelestialJD as
    non-pure virtual methods with default fallback implementations.

libs/alignment/BasicMathPlugin.h/.cpp, NearestMathPlugin, SPKMathPlugin, DummyMathPlugin

  • Override the new *JD methods as the primary implementation (clock-free).
  • Reduce the old Transform* overrides to one-liner shims.

libs/alignment/MathPluginManagement.h/.cpp

  • Add public TransformCelestialToTelescopeJD / TransformTelescopeToCelestialJD
    dispatch methods (direct virtual call on pLoadedMathPlugin).
  • Convert old public Transform* methods to compat shims.
  • Remove now-dead pTransformCelestialToTelescope / pTransformTelescopeToCelestial
    function pointer members (dispatch goes through direct virtual calls).

test/alignment/test_alignment_plugins.cpp

  • Remove the joff = fixedJD - ln_get_julian_from_sys() workaround from all test
    blocks; replace with direct *JD calls using static constexpr double fixedJD.
  • Add two regression tests (LegacyPlugin_JD_Fallback_CelestialToTelescope and
    LegacyPlugin_JD_Fallback_TelescopeToCelestial) that verify the base-class fallback
    correctly converts an absolute JD to the offset expected by legacy external plugins.

Test results

[==========] 110 tests from 2 test suites ran.
[  PASSED  ] 110 tests.

Follow-up (not in this PR)

Phase 2 — Deprecate: Mark the old Transform* pure-virtual methods [[deprecated]]
to produce compiler warnings for external plugins that haven't migrated.

Phase 3 — Driver API: Thread a JD parameter (with default = ln_get_julian_from_sys())
through the six public methods of AlignmentSubsystemForDrivers
(SkyToTelescopeEquatorial, TelescopeEquatorialToSky, etc.) and update the internal
call sites to use *JD directly. Migrate direct MathPluginManagement callers
(skywatcherAPIMount, indi-celestronaux) to use *JD with explicit JD values.

Phase 4 — Remove: Once no callers use the old interface, make *JD pure virtual and
remove the compat layer entirely.

…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.
@ckemper67 ckemper67 force-pushed the feat/mathplugin-jd-refactor branch from 248f5bd to 642a9ca Compare May 9, 2026 01:23
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.
@ckemper67 ckemper67 force-pushed the feat/mathplugin-jd-refactor branch from 642a9ca to 8911f90 Compare May 9, 2026 15:56
@knro
Copy link
Copy Markdown
Contributor

knro commented May 10, 2026

Great work! I agree this would make all the testing and simulation deterministic now. You can mark the shim functions as deprecated. At the same time, another PR for all affected drivers should be prepared to migrate them to the new functions.

@ckemper67
Copy link
Copy Markdown
Contributor Author

Yes I will send the follow-up PRs once we have this submitted. Do you want the depreciation as part of this pull request or in a follow up as well?

@knro
Copy link
Copy Markdown
Contributor

knro commented May 10, 2026

Sure you can add it to this PR as well.

@ckemper67
Copy link
Copy Markdown
Contributor Author

ckemper67 commented May 10, 2026

On second thought, I would prefer to combine the deprecation declaration with the PR that fixes the drivers. That way we don't introduce deprecation warnings/errors into the build. What do you think?

#2388 is the followup. I based it on master, so it contains this PR as well. The last commit are the deprecation changes and the cleanups.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants