From b850833f45d7fe7cf9f5f1d65a3be40b4ebdd6f0 Mon Sep 17 00:00:00 2001 From: Lutz Date: Sat, 29 Aug 2026 23:16:37 -0700 Subject: [PATCH] fix(meade): restore hemisphere conversion for DEC coordinates The Meade parser refactor (#291) dropped the conversion between the mount's internal DEC axis coordinate (0 = pole above the mount) and celestial declination. As a result :GD#/:Gd# reported the raw axis coordinate and :Sd#/:CM stored wire DEC as an axis coordinate, so clients saw wrong values (e.g. +10*00'00 for celestial +80) and slews landed at the complement of the intended DEC. Move the transform into pure, unit-testable core helpers (Declination::axisToCelestialSeconds / celestialToAxisSeconds, fromTotalSeconds, DayTime::splitSeconds), expose wire-format accessors on the app-level Declination overlay (getCelestialDegrees / fromCelestialDegrees), and route all three Meade boundary handlers through them: decFrom() for :GD/:Gd, decFromWire() for :Sd and :CM. Behavior matches the pre-refactor code exactly; LCD/OLED/status paths were unaffected and are unchanged. Co-Authored-By: Claude --- src/Declination.cpp | 18 +++- src/Declination.hpp | 8 ++ src/MeadeCommandProcessor.cpp | 20 +++- src/core/types/DayTime.cpp | 10 ++ src/core/types/DayTime.hpp | 4 + src/core/types/Declination.cpp | 26 ++++++ src/core/types/Declination.hpp | 10 ++ .../test_core/types/test_declination.cpp | 91 +++++++++++++++++++ 8 files changed, 180 insertions(+), 7 deletions(-) diff --git a/src/Declination.cpp b/src/Declination.cpp index aa75f70c..202f99b9 100644 --- a/src/Declination.cpp +++ b/src/Declination.cpp @@ -80,7 +80,21 @@ Declination Declination::FromSeconds(long seconds) const char *Declination::formatString(char *targetBuffer, const char *format, long *) const { - long secs - = inNorthernHemisphere ? (arcSecondsPerHemisphere / 2) - labs(totalSeconds) : -(arcSecondsPerHemisphere / 2) + labs(totalSeconds); + long secs = core::Declination::axisToCelestialSeconds(totalSeconds, inNorthernHemisphere); return core::DayTime::formatString(targetBuffer, format, &secs); } + +void Declination::getCelestialDegrees(int °, int &min, int &sec) const +{ + const long celestialSecs = core::Declination::axisToCelestialSeconds(totalSeconds, inNorthernHemisphere); + core::DayTime::splitSeconds(celestialSecs, deg, min, sec); +} + +Declination Declination::fromCelestialDegrees(int deg, int min, int sec) +{ + const long wireSecs = ((60L * deg) + min) * 60L + sec; + Declination result; + result.totalSeconds = core::Declination::celestialToAxisSeconds(wireSecs, inNorthernHemisphere); + result.checkHours(); + return result; +} diff --git a/src/Declination.hpp b/src/Declination.hpp index 8bb88b46..220fb612 100644 --- a/src/Declination.hpp +++ b/src/Declination.hpp @@ -17,6 +17,14 @@ class Declination : public core::Declination virtual const char *ToString() const; virtual const char *formatString(char *targetBuffer, const char *format, long *pSeconds = nullptr) const; + // Split into celestial (hemisphere-corrected) degree components, as used + // on the Meade wire: signed degrees, unsigned minutes/seconds. + void getCelestialDegrees(int °, int &min, int &sec) const; + + // Build from celestial (Meade wire) components: signed degrees, unsigned + // minutes/seconds. + static Declination fromCelestialDegrees(int deg, int min, int sec); + const char *ToDisplayString(char sep1, char sep2) const; static Declination ParseFromMeade(String const &s); diff --git a/src/MeadeCommandProcessor.cpp b/src/MeadeCommandProcessor.cpp index 419f7630..ecb4d531 100644 --- a/src/MeadeCommandProcessor.cpp +++ b/src/MeadeCommandProcessor.cpp @@ -99,12 +99,22 @@ meade::RaCoordinate raFrom(const DayTime &t) meade::DecCoordinate decFrom(const Declination &d) { + // The mount stores DEC as an axis coordinate (0 = pole above the mount); + // Meade clients expect celestial declination. Apply the hemisphere + // correction before splitting into components. + int deg, min, sec; + d.getCelestialDegrees(deg, min, sec); return meade::DecCoordinate { - static_cast(d.getHours()), - static_cast(d.getMinutes()), - static_cast(d.getSeconds()), + static_cast(deg), + static_cast(min), + static_cast(sec), }; } + +Declination decFromWire(meade::DecCoordinate const &d) +{ + return Declination::fromCelestialDegrees(d.degrees, d.minutes, d.seconds); +} } // namespace meade::RaCoordinate MeadeCommandProcessor::onCurrentRa() @@ -248,7 +258,7 @@ void MeadeCommandProcessor::onSyncToTarget() ///////////////////////////// bool MeadeCommandProcessor::onSetTargetDec(meade::DecCoordinate dec) { - _mount->targetDEC() = Declination(static_cast(dec.degrees), static_cast(dec.minutes), static_cast(dec.seconds)); + _mount->targetDEC() = decFromWire(dec); LOG(DEBUG_MEADE, "[MEADE]: SetInfo: Received Target DEC: %s", _mount->targetDEC().ToString()); return true; } @@ -282,7 +292,7 @@ bool MeadeCommandProcessor::onSetHourAngle(uint8_t hours, uint8_t minutes) bool MeadeCommandProcessor::onSyncCoordinates(meade::DecCoordinate dec, meade::RaCoordinate ra) { - Declination decValue(static_cast(dec.degrees), static_cast(dec.minutes), static_cast(dec.seconds)); + Declination decValue = decFromWire(dec); DayTime raValue(static_cast(ra.hours), static_cast(ra.minutes), static_cast(ra.seconds)); _mount->syncPosition(raValue, decValue); return true; diff --git a/src/core/types/DayTime.cpp b/src/core/types/DayTime.cpp index d0b29bde..35d9fad4 100644 --- a/src/core/types/DayTime.cpp +++ b/src/core/types/DayTime.cpp @@ -75,6 +75,16 @@ void DayTime::getTime(int &h, int &m, int &s) const h *= sign(totalSeconds); } +void DayTime::splitSeconds(long secs, int &h, int &m, int &s) +{ + long remainder = labs(secs); + h = static_cast(remainder / 3600L); + remainder = remainder - (h * 3600L); + m = static_cast(remainder / 60L); + s = static_cast(remainder - (m * 60L)); + h *= sign(secs); +} + void DayTime::set(int h, int m, int s) { DayTime dt(h, m, s); diff --git a/src/core/types/DayTime.hpp b/src/core/types/DayTime.hpp index 55e98866..4bdd5a9e 100644 --- a/src/core/types/DayTime.hpp +++ b/src/core/types/DayTime.hpp @@ -31,6 +31,10 @@ class DayTime long getTotalSeconds() const; void getTime(int &h, int &m, int &s) const; + + // Split signed seconds into (signed) hours plus unsigned minutes/seconds. + static void splitSeconds(long secs, int &h, int &m, int &s); + virtual void set(int h, int m, int s); void set(const DayTime &other); diff --git a/src/core/types/Declination.cpp b/src/core/types/Declination.cpp index 1e2a36ec..de29194a 100644 --- a/src/core/types/Declination.cpp +++ b/src/core/types/Declination.cpp @@ -48,4 +48,30 @@ void Declination::checkHours() } } +Declination Declination::fromTotalSeconds(long totalSeconds) +{ + Declination d; + d.totalSeconds = totalSeconds; + d.checkHours(); + return d; +} + +long Declination::axisToCelestialSeconds(long axisSeconds, bool northernHemisphere) +{ + // Northern: celestial = 90 - |axis|. Southern: celestial = |axis| - 90. + // Mirrors the hemisphere tables in src/Declination.cpp. + const long hemiArcsecs = arcSecondsPerHemisphere / 2; + return northernHemisphere ? hemiArcsecs - labs(axisSeconds) : labs(axisSeconds) - hemiArcsecs; +} + +long Declination::celestialToAxisSeconds(long celestialSeconds, bool northernHemisphere) +{ + // Inverse of axisToCelestialSeconds on the mount's home-side branch: + // northern mounts keep the axis non-negative, southern non-positive. + // Note this intentionally does NOT take labs() of the input — celestial + // values on the far side of the equator push the axis past 90 degrees. + const long hemiArcsecs = arcSecondsPerHemisphere / 2; + return northernHemisphere ? hemiArcsecs - celestialSeconds : -hemiArcsecs - celestialSeconds; +} + } // namespace core diff --git a/src/core/types/Declination.hpp b/src/core/types/Declination.hpp index d9eeb2f8..dda22d1a 100644 --- a/src/core/types/Declination.hpp +++ b/src/core/types/Declination.hpp @@ -24,6 +24,16 @@ class Declination : public DayTime // Get total degrees (-180..180) float getTotalDegrees() const; + // Hemisphere-aware conversions between the mount-axis coordinate (0 at the + // pole above the mount, +/-180 at the opposite pole) and celestial + // declination arc-seconds (-90 at the south celestial pole, +90 at the + // north celestial pole). + static long axisToCelestialSeconds(long axisSeconds, bool northernHemisphere); + static long celestialToAxisSeconds(long celestialSeconds, bool northernHemisphere); + + // Construct from total (axis) seconds directly, avoiding float rounding. + static Declination fromTotalSeconds(long totalSeconds); + protected: virtual void checkHours() override; diff --git a/unit_tests/test_core/types/test_declination.cpp b/unit_tests/test_core/types/test_declination.cpp index eefa49e4..9788c28d 100644 --- a/unit_tests/test_core/types/test_declination.cpp +++ b/unit_tests/test_core/types/test_declination.cpp @@ -62,3 +62,94 @@ TEST(DeclinationTest, GetTotalDegrees) Declination dec(30, 0, 0); EXPECT_FLOAT_EQ(30.0f, dec.getTotalDegrees()); } + +// --------------------------------------------------------------------------- +// Hemisphere conversion (mount-axis coordinate <-> celestial declination). +// +// The mount stores DEC as an axis coordinate: 0 at the pole above the mount, +// +/-180 at the opposite pole. Meade clients speak celestial declination. +// These tests pin the exact relationship documented in src/Declination.cpp. +// --------------------------------------------------------------------------- + +TEST(DeclinationTest, AxisToCelestialNorthernPole) +{ + // Axis 0 is the north celestial pole in the northern hemisphere. + EXPECT_EQ(90L * 3600L, Declination::axisToCelestialSeconds(0, true)); +} + +TEST(DeclinationTest, AxisToCelestialNorthernEquator) +{ + // Both equator crossings (+90 and -90 axis) are celestial 0. + EXPECT_EQ(0L, Declination::axisToCelestialSeconds(90L * 3600L, true)); + EXPECT_EQ(0L, Declination::axisToCelestialSeconds(-90L * 3600L, true)); +} + +TEST(DeclinationTest, AxisToCelestialNorthernSouthPole) +{ + EXPECT_EQ(-90L * 3600L, Declination::axisToCelestialSeconds(180L * 3600L, true)); + EXPECT_EQ(-90L * 3600L, Declination::axisToCelestialSeconds(-180L * 3600L, true)); +} + +TEST(DeclinationTest, AxisToCelestialSouthernPole) +{ + // Axis 0 is the south celestial pole in the southern hemisphere. + EXPECT_EQ(-90L * 3600L, Declination::axisToCelestialSeconds(0, false)); +} + +TEST(DeclinationTest, AxisToCelestialSouthernEquator) +{ + EXPECT_EQ(0L, Declination::axisToCelestialSeconds(90L * 3600L, false)); + EXPECT_EQ(0L, Declination::axisToCelestialSeconds(-90L * 3600L, false)); +} + +TEST(DeclinationTest, AxisToCelestialSouthernNorthPole) +{ + EXPECT_EQ(90L * 3600L, Declination::axisToCelestialSeconds(180L * 3600L, false)); + EXPECT_EQ(90L * 3600L, Declination::axisToCelestialSeconds(-180L * 3600L, false)); +} + +TEST(DeclinationTest, AxisToCelestialNorthernSignFlip) +{ + // Northern mount with axis +100 points 10 degrees below the equator: + // celestial sign is negative while the axis coordinate stays positive. + EXPECT_EQ(-10L * 3600L, Declination::axisToCelestialSeconds(100L * 3600L, true)); +} + +TEST(DeclinationTest, CelestialToAxisNorthern) +{ + EXPECT_EQ(10L * 3600L, Declination::celestialToAxisSeconds(80L * 3600L, true)); + EXPECT_EQ(0L, Declination::celestialToAxisSeconds(90L * 3600L, true)); + EXPECT_EQ(180L * 3600L, Declination::celestialToAxisSeconds(-90L * 3600L, true)); +} + +TEST(DeclinationTest, CelestialToAxisSouthern) +{ + EXPECT_EQ(-10L * 3600L, Declination::celestialToAxisSeconds(-80L * 3600L, false)); + EXPECT_EQ(0L, Declination::celestialToAxisSeconds(-90L * 3600L, false)); + EXPECT_EQ(-180L * 3600L, Declination::celestialToAxisSeconds(90L * 3600L, false)); +} + +TEST(DeclinationTest, CelestialAxisRoundTrip) +{ + // The axis->celestial mapping is two-to-one (|axis|): the arm at +100 and + // -100 both point at celestial -10, on opposite sides of the meridian. + // The inverse maps back to the home branch only: non-negative axis in the + // northern hemisphere, non-positive in the southern. + for (long axis = 0; axis <= 180L * 3600L; axis += 1800L) + { + const long celestial = Declination::axisToCelestialSeconds(axis, true); + EXPECT_EQ(axis, Declination::celestialToAxisSeconds(celestial, true)) << "axis=" << axis; + } + for (long axis = 0; axis >= -180L * 3600L; axis -= 1800L) + { + const long celestial = Declination::axisToCelestialSeconds(axis, false); + EXPECT_EQ(axis, Declination::celestialToAxisSeconds(celestial, false)) << "axis=" << axis; + } +} + +TEST(DeclinationTest, FromTotalSecondsClamps) +{ + EXPECT_EQ(180L * 3600L, Declination::fromTotalSeconds(200L * 3600L).getTotalSeconds()); + EXPECT_EQ(-180L * 3600L, Declination::fromTotalSeconds(-200L * 3600L).getTotalSeconds()); + EXPECT_EQ(12345L, Declination::fromTotalSeconds(12345L).getTotalSeconds()); +}