fix(meade): accept the unsigned site longitude INDI sends

src/core/meade/MeadeParser.hpp

@@ -182,7 +182,14 @@ struct MeadeLatitude {
     bool negative;
 };
 
-/** @brief Site longitude: magnitude 0..180 in `degrees`, sign in `negative`. */
+/** @brief Site longitude: magnitude 0..180 in `degrees`, sign in `negative`.
+ *
+ * EAST-POSITIVE: `negative` means west of Greenwich. The Meade wire is the
+ * other way round -- :Sg/:Gg are east-negative -- so readLongitude and
+ * writeLongitude both flip the sign, and they have to stay in step. The
+ * convention is recorded here because the struct alone cannot show it, which
+ * is how a flip on both sides at once once went unnoticed.
+ */
 struct MeadeLongitude {
     uint16_t degrees;
     uint8_t minutes;

src/core/meade/MeadeParserHelpers.cpp

@@ -243,7 +243,14 @@ void writeLatitude(MeadeResponse &r, const MeadeLatitude &l)
 
 void writeLongitude(MeadeResponse &r, const MeadeLongitude &l)
 {
-    writeChar(r, l.negative ? '-' : '+');
+    // :Gg is east-negative (MeadeProtocol.hpp), while MeadeLongitude is
+    // east-positive, so the sign flips on the way out. This has to move with
+    // readLongitude: if only one side flips, a client sets its site, reads it
+    // back mirrored, and pushes the mirror straight back on the next connect.
+    // Greenwich has no side, and a bare '-000*00' reads as a negative zero, so
+    // it goes out positive.
+    const bool atGreenwich = (l.degrees == 0) && (l.minutes == 0);
+    writeChar(r, (l.negative || atGreenwich) ? '+' : '-');
     writeUnsignedPadded(r, l.degrees, 3);
     writeChar(r, '*');
     writeUnsignedPadded(r, l.minutes, 2);

src/core/meade/MeadeParserSet.cpp

@@ -79,18 +79,68 @@ bool readLatitude(Cursor &c, MeadeLatitude &out)
     return true;
 }
 
-// Format: "[+-]DDD<sep>MM" where sep in {'*', ':'}.
+// Unsigned :Sg is the legacy 0..360 count running WESTWARD from Greenwich.
+// East-positive is what the mount stores, so negate modulo a full circle (which is
+// what `fullCircle - arcminutes` is) and wrap into (-180, 180].
+// The tempting mistake is the other reflection, the one that lands Greenwich on 180
+// — `fullCircle / 2 - arcminutes`, which is what Longitude::ParseFromMeade computes.
+// It turns a 121d53' west site into 58d07' east, exactly 180 degrees (12 hours of
+// local sidereal time) from where it should be.
+// A signed wire value negates the same way, so this is the single mapping for both
+// forms: `arcminutes` is the westward count, positive or negative, and never more
+// than one full circle from zero.
+long westwardToEastPositiveArcminutes(long arcminutes)
+{
+    const long fullCircle = 360L * 60L;
+    long east             = fullCircle - arcminutes;
+    while (east > fullCircle / 2)
+    {
+        east -= fullCircle;
+    }
+    return east;
+}
+
+// Format: "[+-]?DDD<sep>MM" where sep in {'*', ':'}.
+//
+// The sign is optional. INDI omits it — ":Sg121*53#" goes on the wire for a site
+// 121d53' WEST — so demanding one answers INDI's site push with "0" and the mount
+// silently keeps whatever longitude it already had.
+//
+// Only the unsigned form is interpreted here. A signed value is passed through
+// unchanged; which hemisphere its sign denotes is a separate question that this
+// function deliberately does not answer.
 bool readLongitude(Cursor &c, MeadeLongitude &out)
 {
+    // The sign is optional, and both forms mean the same thing. MeadeProtocol.hpp
+    // documents :Sg/:Gg as east-negative, so the legacy unsigned 0..360 westward
+    // count that INDI sends is simply the sign == +1 case of the signed form:
+    // east = wrap(-value) either way, with no branch between them.
     int sign;
+    c.optionalSign(sign);
+
     unsigned ddd, mm;
-    if (!readMandatorySign(c, sign) || !c.digits(3, ddd) || !c.matchIn("*:") || !c.digits(2, mm))
+    if (!c.digits(3, ddd) || !c.matchIn("*:") || !c.digits(2, mm))
     {
         return false;
     }
-    out.degrees  = static_cast<uint16_t>(ddd);
-    out.minutes  = static_cast<uint8_t>(mm);
-    out.negative = (sign < 0);
+
+    // Reject anything outside one full circle, which nothing downstream does:
+    // core::Longitude(int, int, int) never calls checkHours(), and
+    // EEPROMStore::storeLongitude clamps degrees*100 into an int16, which destroys
+    // the mod-360 equivalence and persists a genuinely wrong site across reboots.
+    if ((ddd >= 360) || (mm >= 60))
+    {
+        return false;
+    }
+
+    const long westward  = sign * ((static_cast<long>(ddd) * 60L) + static_cast<long>(mm));
+    const long east      = westwardToEastPositiveArcminutes(westward);
+    const bool isWest    = (east < 0);
+    const long magnitude = isWest ? -east : east;
+
+    out.degrees  = static_cast<uint16_t>(magnitude / 60);
+    out.minutes  = static_cast<uint8_t>(magnitude % 60);
+    out.negative = isWest;
     return true;
 }
 

src/core/meade/MeadeProtocol.hpp

@@ -154,6 +154,7 @@
 //        "MM" is the minutes
 //      Remarks:
 //        Note that this is the actual longitude, but east coordinates are negative (opposite of normal cartographic coordinates)
+//        This is the exact inverse of :Sg, and the two have to stay in step: flipping one alone makes a client read its own site back mirrored
 //
 // :Gc#
 //      Description:
@@ -339,8 +340,8 @@
 //        "DDD" is the number of degrees
 //        "MM" is the minutes
 //      Remarks:
-//        When a sign is provided, longitudes are interpreted as given, with zero at Greenwich but negative coordinates going east (opposite of normal cartographic coordinates)
-//        When a sign is not provided, longitudes are from 0 to 360 going WEST with 180 at Greenwich. So 369 is 179W and 1 is 179E. 190 would be 10W and 170 would be 10E.
+//        Longitudes are east-negative: zero at Greenwich, negative coordinates going east (opposite of normal cartographic coordinates)
+//        The unsigned form is the legacy count running WESTWARD from Greenwich, 0 to 359, which is the same mapping with the sign taken as '+'. So "121*53" is 121d53' west, "301*53" is 58d07' east, and "180*00" is the antimeridian. A full circle ("360*00") is refused rather than wrapped.
 //
 // :SGsHH#
 //      Description:

unit_tests/test_core/meade/test_MeadeGet.cpp

@@ -323,13 +323,16 @@ TEST(MeadeGet, site_latitude_signed_two_digit_deg)
     EXPECT_STREQ("-12*45#", dispatch("t", h));
 }
 
+// :Gg is east-negative and MeadeLongitude is east-positive, so the sign on the
+// wire is the opposite of `negative`. This has to stay the exact inverse of
+// readLongitude; see the round trips below.
 TEST(MeadeGet, site_longitude_signed_three_digit_deg)
 {
     FakeHandlers h;
-    h.longitude = {12, 30, false};
-    EXPECT_STREQ("+012*30#", dispatch("g", h));
-    h.longitude = {122, 45, true};
-    EXPECT_STREQ("-122*45#", dispatch("g", h));
+    h.longitude = {12, 30, false};  // 12d30' east
+    EXPECT_STREQ("-012*30#", dispatch("g", h));
+    h.longitude = {122, 45, true};  // 122d45' west
+    EXPECT_STREQ("+122*45#", dispatch("g", h));
 }
 
 // ---- Sign of zero -----------------------------------------------------
@@ -359,10 +362,10 @@ TEST(MeadeGet, site_latitude_zero_degrees_keeps_south_sign)
 TEST(MeadeGet, site_longitude_zero_degrees_keeps_sign)
 {
     FakeHandlers h;
-    h.longitude = {0, 5, true};
-    EXPECT_STREQ("-000*05#", dispatch("g", h));
-    h.longitude = {0, 5, false};
+    h.longitude = {0, 5, true};  // 5' west
     EXPECT_STREQ("+000*05#", dispatch("g", h));
+    h.longitude = {0, 5, false};  // 5' east
+    EXPECT_STREQ("-000*05#", dispatch("g", h));
 }
 
 // ---- Set -> Get round trips -------------------------------------------
@@ -412,6 +415,34 @@ TEST(MeadeGet, site_longitude_round_trip_preserves_nonzero_degrees)
     EXPECT_STREQ("+097*34#", setThenGet("g+097*34", "g", h));
 }
 
+// The reader and the writer both flip the sign, so the wire value is unchanged
+// by a round trip -- which is exactly why a flip on one side alone is invisible
+// to a client and has to be caught by the struct-level assertions above.
+TEST(MeadeGet, site_longitude_round_trip_is_unchanged_at_the_meridians)
+{
+    FakeHandlers h;
+    EXPECT_STREQ("+000*00#", setThenGet("g+000*00", "g", h));
+    EXPECT_STREQ("+000*00#", setThenGet("g-000*00", "g", h));
+    EXPECT_STREQ("-180*00#", setThenGet("g-180*00", "g", h));
+}
+
+// The form INDI actually sends: unsigned, counting westward. It comes back in
+// the signed form, on the same meridian.
+TEST(MeadeGet, site_longitude_unsigned_round_trips_to_the_same_meridian)
+{
+    FakeHandlers h;
+    EXPECT_STREQ("+121*53#", setThenGet("g121*53", "g", h));
+    EXPECT_EQ(static_cast<uint16_t>(121), h.longitude.degrees);
+    EXPECT_EQ(static_cast<uint8_t>(53), h.longitude.minutes);
+    EXPECT_TRUE(h.longitude.negative);  // west, east-positive internally
+
+    FakeHandlers e;
+    EXPECT_STREQ("-058*07#", setThenGet("g301*53", "g", e));
+    EXPECT_EQ(static_cast<uint16_t>(58), e.longitude.degrees);
+    EXPECT_EQ(static_cast<uint8_t>(7), e.longitude.minutes);
+    EXPECT_FALSE(e.longitude.negative);
+}
+
 TEST(MeadeGet, utc_offset_signs_and_pads)
 {
     FakeHandlers h;

unit_tests/test_core/meade/test_MeadeParserHelpers.cpp

@@ -110,13 +110,28 @@ TEST(MeadeParserHelpers, write_latitude_emits_sign_for_zero_degrees)
     EXPECT_STREQ("-00*30#", bytes(r));
 }
 
-TEST(MeadeParserHelpers, write_longitude_pads_to_three_digits_and_keeps_sign)
+// The struct is east-positive and the wire is east-negative, so the sign
+// inverts on the way out: `negative` (west of Greenwich) emits '+'.
+TEST(MeadeParserHelpers, write_longitude_pads_to_three_digits_and_inverts_the_sign)
 {
     meade::MeadeResponse r;
     writeLongitude(r, meade::MeadeLongitude {0, 5, true});
-    EXPECT_STREQ("-000*05#", bytes(r));
+    EXPECT_STREQ("+000*05#", bytes(r));
 
     meade::MeadeResponse r2;
     writeLongitude(r2, meade::MeadeLongitude {122, 45, false});
-    EXPECT_STREQ("+122*45#", bytes(r2));
+    EXPECT_STREQ("-122*45#", bytes(r2));
+}
+
+// Greenwich is on neither side, and "-000*00" would read as a negative zero,
+// so the zero meridian always goes out positive.
+TEST(MeadeParserHelpers, write_longitude_emits_greenwich_as_positive)
+{
+    meade::MeadeResponse r;
+    writeLongitude(r, meade::MeadeLongitude {0, 0, false});
+    EXPECT_STREQ("+000*00#", bytes(r));
+
+    meade::MeadeResponse r2;
+    writeLongitude(r2, meade::MeadeLongitude {0, 0, true});
+    EXPECT_STREQ("+000*00#", bytes(r2));
 }