fix(meade): build declination from wire components without losing the sign

src/Declination.cpp

@@ -92,7 +92,14 @@ void Declination::getCelestialDegrees(int &deg, int &min, int &sec) const
 
 Declination Declination::fromCelestialDegrees(int deg, int min, int sec)
 {
-    const long wireSecs = ((60L * deg) + min) * 60L + sec;
+    // deg carries the only sign on the wire, so min and sec are unsigned
+    // magnitudes and must move away from zero. joinSeconds is the inverse of the
+    // splitSeconds that getCelestialDegrees uses.
+    // Declinations between -1 and 0 degrees still cannot round-trip here: they
+    // arrive as deg == 0, and integer 0 has no sign, so -00*30:00 reads the same
+    // as +00*30:00. Fixing that means carrying the sign separately from the
+    // magnitude across this boundary, not changing the join.
+    const long wireSecs = core::DayTime::joinSeconds(deg, min, sec);
     Declination result;
     result.totalSeconds = core::Declination::celestialToAxisSeconds(wireSecs, inNorthernHemisphere);
     result.checkHours();

src/core/types/DayTime.cpp

@@ -15,9 +15,7 @@ DayTime::DayTime(const DayTime &other)
 
 DayTime::DayTime(int h, int m, int s)
 {
-    long sgn     = sign(h);
-    h            = abs(h);
-    totalSeconds = sgn * ((60L * h + m) * 60L + s);
+    totalSeconds = joinSeconds(h, m, s);
 }
 
 DayTime::DayTime(float timeInHours)
@@ -85,6 +83,11 @@ void DayTime::splitSeconds(long secs, int &h, int &m, int &s)
     h *= sign(secs);
 }
 
+long DayTime::joinSeconds(int h, int m, int s)
+{
+    return sign(h) * ((60L * labs(h) + m) * 60L + s);
+}
+
 void DayTime::set(int h, int m, int s)
 {
     DayTime dt(h, m, s);

src/core/types/DayTime.hpp

@@ -35,6 +35,13 @@ class DayTime
     // Split signed seconds into (signed) hours plus unsigned minutes/seconds.
     static void splitSeconds(long secs, int &h, int &m, int &s);
 
+    // Join hours/minutes/seconds into signed seconds. The sign is taken from h
+    // alone; m and s are added to the magnitude as given, so a negative m or s
+    // subtracts from it, exactly as the DayTime(int, int, int) constructor has
+    // always done. Inverts splitSeconds for every value except -3599..-1
+    // seconds, where splitSeconds reports h == 0 and the sign is already lost.
+    static long joinSeconds(int h, int m, int s);
+
     virtual void set(int h, int m, int s);
     void set(const DayTime &other);
 

unit_tests/test_core/types/test_daytime.cpp

@@ -241,3 +241,63 @@ TEST(DayTimeTest, FormatStringCustomSecsNegative)
     dt.formatString(buf, "{d}:{m}:{s}", &customSecs);
     EXPECT_STREQ("-100:30:15", buf);
 }
+
+// ---------------------------------------------------------------------------
+// joinSeconds — the inverse of splitSeconds, and the shared implementation of
+// the DayTime(int, int, int) constructor.
+// ---------------------------------------------------------------------------
+
+TEST(DayTimeTest, JoinSeconds)
+{
+    EXPECT_EQ(0L, DayTime::joinSeconds(0, 0, 0));
+    EXPECT_EQ(3600L, DayTime::joinSeconds(1, 0, 0));
+    EXPECT_EQ(5445L, DayTime::joinSeconds(1, 30, 45));
+    EXPECT_EQ(-5445L, DayTime::joinSeconds(-1, 30, 45));
+}
+
+TEST(DayTimeTest, JoinSecondsIsUsedByHMSConstructor)
+{
+    EXPECT_EQ(DayTime::joinSeconds(1, 30, 45), DayTime(1, 30, 45).getTotalSeconds());
+    EXPECT_EQ(DayTime::joinSeconds(-2, 31, 18), DayTime(-2, 31, 18).getTotalSeconds());
+    // Callers that pass negative minutes/seconds keep subtracting from the
+    // magnitude, as NegativeConstructor and GetTimeRefNegative above rely on.
+    EXPECT_EQ(DayTime::joinSeconds(-2, -30, -15), DayTime(-2, -30, -15).getTotalSeconds());
+}
+
+TEST(DayTimeTest, JoinSecondsInvertsSplitSeconds)
+{
+    // Skips (-3600, 0): splitSeconds reports h == 0 there, which drops the sign
+    // (see JoinSecondsCannotSignSubHourValues).
+    for (long secs = -180L * 3600L; secs <= 180L * 3600L; secs += 907L)
+    {
+        if ((secs < 0L) && (secs > -3600L))
+        {
+            continue;
+        }
+        int h, m, s;
+        DayTime::splitSeconds(secs, h, m, s);
+        EXPECT_EQ(secs, DayTime::joinSeconds(h, m, s)) << "secs=" << secs;
+    }
+}
+
+TEST(DayTimeTest, JoinSecondsCannotSignSubHourValues)
+{
+    // Integer -0 is 0, so a value between -1 and 0 hours (or degrees) has no
+    // way to express its sign through h. Both spellings join to the same
+    // positive value; representing -00:30:00 needs a separate sign, not a
+    // different join.
+    EXPECT_EQ(1800L, DayTime::joinSeconds(0, 30, 0));
+    EXPECT_EQ(1800L, DayTime::joinSeconds(-0, 30, 0));
+}
+
+TEST(DayTimeTest, JoinSecondsNegativeDegreesMoveAwayFromZero)
+{
+    // Declinations off the Meade wire: the sign sits on the degrees and the
+    // minutes/seconds are unsigned magnitudes, so they must extend the value
+    // away from zero. Adding them toward zero instead put :Sd-05*30:00# back
+    // on the wire as -04*30:00 (see Declination::fromCelestialDegrees).
+    EXPECT_EQ(-(5L * 3600L + 30L * 60L), DayTime::joinSeconds(-5, 30, 0));
+    EXPECT_EQ(-(24L * 3600L + 23L * 60L), DayTime::joinSeconds(-24, 23, 0));
+    EXPECT_EQ(-(69L * 3600L + 6L * 60L), DayTime::joinSeconds(-69, 6, 0));
+    EXPECT_EQ(-(89L * 3600L + 59L * 60L + 59L), DayTime::joinSeconds(-89, 59, 59));
+}

unit_tests/test_core/types/test_declination.cpp

@@ -153,3 +153,35 @@ TEST(DeclinationTest, FromTotalSecondsClamps)
     EXPECT_EQ(-180L * 3600L, Declination::fromTotalSeconds(-200L * 3600L).getTotalSeconds());
     EXPECT_EQ(12345L, Declination::fromTotalSeconds(12345L).getTotalSeconds());
 }
+
+TEST(DeclinationTest, CelestialWireRoundTrip)
+{
+    // Reproduces the composition that Declination::fromCelestialDegrees and
+    // Declination::getCelestialDegrees perform for the Meade :Sd/:Gd/:CM
+    // commands. src/Declination.cpp itself needs Arduino String and the board
+    // configuration, so it cannot be linked here: this pins that the sequence
+    // is correct, not that src/Declination.cpp still uses it.
+    struct WireDec {
+        int deg;
+        int min;
+        int sec;
+    };
+    const WireDec cases[]    = {{-5, 30, 0}, {-24, 23, 0}, {-69, 6, 0}, {-89, 59, 59}, {5, 30, 0}, {0, 30, 0}, {45, 0, 0}, {89, 59, 59}};
+    const bool hemispheres[] = {true, false};
+
+    for (bool north : hemispheres)
+    {
+        for (const WireDec &wire : cases)
+        {
+            const long celestial = core::DayTime::joinSeconds(wire.deg, wire.min, wire.sec);
+            const Declination onAxis(Declination::fromTotalSeconds(Declination::celestialToAxisSeconds(celestial, north)));
+
+            int deg, min, sec;
+            core::DayTime::splitSeconds(Declination::axisToCelestialSeconds(onAxis.getTotalSeconds(), north), deg, min, sec);
+
+            EXPECT_EQ(wire.deg, deg) << "north=" << north << " deg=" << wire.deg;
+            EXPECT_EQ(wire.min, min) << "north=" << north << " deg=" << wire.deg;
+            EXPECT_EQ(wire.sec, sec) << "north=" << north << " deg=" << wire.deg;
+        }
+    }
+}