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

src/core/meade/MeadeParserSet.cpp

@@ -62,15 +62,89 @@ 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.
+// `arcminutes` is the wire value, so at most one full circle.
+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)
 {
+    const bool hasSign = (c.peek() == '+') || (c.peek() == '-');
+
     int deg;
     unsigned mm;
-    if (!c.signed3(deg) || !c.matchIn("*:") || !c.digits(2, mm))
+    if (hasSign)
+    {
+        if (!c.signed3(deg))
+        {
+            return false;
+        }
+    }
+    else
+    {
+        // Exactly three digits, matching the signed path and the "DDD" that
+        // MeadeProtocol.hpp documents. Pre-#291 DayTime::ParseFromMeade also took
+        // two; widening that again belongs with the signed path, not here.
+        unsigned ddd;
+        if (!c.digits(3, ddd))
+        {
+            return false;
+        }
+        deg = static_cast<int>(ddd);
+    }
+    if (!c.matchIn("*:") || !c.digits(2, mm))
     {
         return false;
     }
+
+    // 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.
+    // This is not a -180..180 check — a signed +200 still passes.
+    if ((deg >= 360) || (deg <= -360) || (mm >= 60))
+    {
+        return false;
+    }
+
+    if (!hasSign)
+    {
+        const long east      = westwardToEastPositiveArcminutes(deg * 60L + static_cast<long>(mm));
+        const bool west      = (east < 0);
+        const long magnitude = west ? -east : east;
+        const long degrees   = magnitude / 60;
+
+        // KNOWN LIMITATION: MeadeLongitude carries the sign only in `degrees`, so a
+        // west longitude of under one degree (0d01' through 0d59' west) loses it
+        // here — `degrees` is 0, and the arcminutes end up stored as if east. The fix
+        // is to let the struct hold the sign separately; that is not this change.
+        deg = static_cast<int>(west ? -degrees : degrees);
+        mm  = static_cast<unsigned>(magnitude % 60);
+    }
+
     out.degrees = static_cast<int16_t>(deg);
     out.minutes = static_cast<uint8_t>(mm);
     return true;

src/core/meade/MeadeProtocol.hpp

@@ -340,7 +340,7 @@
 //        "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.
+//        When a sign is not provided, the value is the legacy count running WESTWARD from Greenwich, 0 to 359. 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_MeadeSet.cpp

@@ -301,6 +301,139 @@ TEST(MeadeSet, site_longitude_malformed_short_does_not_call_handler)
     EXPECT_EQ(nullptr, h.lastCall);
 }
 
+// Pins the signed path exactly as #291 left it: the value is passed through, so a
+// '-' reaches the mount as a west longitude. Whether that is the right reading of
+// the sign is disputed (see :Sg in MeadeProtocol.hpp, whose Remarks still say a
+// negative sign means east) and is not settled by these tests — this case exists so
+// that any later change to the sign convention has to move a green assertion.
+TEST(MeadeSet, site_longitude_signed_negative_passes_through_unchanged)
+{
+    FakeHandlers h;
+    EXPECT_STREQ("1", dispatch("g-121*53", h));
+    EXPECT_STREQ("lon", h.lastCall);
+    EXPECT_EQ(-121, h.lon.degrees);
+    EXPECT_EQ(static_cast<uint8_t>(53), h.lon.minutes);
+}
+
+// Unsigned longitudes count WESTWARD from Greenwich, 0..360, and are mirrored into
+// the east-positive range the mount stores. INDI sends this form: a San Jose site
+// at 121d53' west arrives as ":Sg121*53#" and must come back out as -121d53'.
+TEST(MeadeSet, site_longitude_unsigned_west_of_greenwich)
+{
+    FakeHandlers h;
+    EXPECT_STREQ("1", dispatch("g121*53", h));
+    EXPECT_STREQ("lon", h.lastCall);
+    EXPECT_EQ(-121, h.lon.degrees);
+    EXPECT_EQ(static_cast<uint8_t>(53), h.lon.minutes);
+}
+
+// Past 180 the westward count has gone round to the eastern hemisphere.
+TEST(MeadeSet, site_longitude_unsigned_east_of_greenwich)
+{
+    FakeHandlers h;
+    EXPECT_STREQ("1", dispatch("g301*53", h));
+    EXPECT_EQ(58, h.lon.degrees);
+    EXPECT_EQ(static_cast<uint8_t>(7), h.lon.minutes);
+}
+
+TEST(MeadeSet, site_longitude_unsigned_greenwich_is_zero)
+{
+    FakeHandlers h;
+    EXPECT_STREQ("1", dispatch("g000*00", h));
+    EXPECT_EQ(0, h.lon.degrees);
+    EXPECT_EQ(static_cast<uint8_t>(0), h.lon.minutes);
+}
+
+// 180 west and 180 east are the same meridian, so either sign would be right. This
+// pins the half of the choice the parser makes — it wraps into (-180, 180], keeping
+// the antimeridian positive — rather than leaving it for a reader to infer.
+TEST(MeadeSet, site_longitude_unsigned_antimeridian_stays_positive)
+{
+    FakeHandlers h;
+    EXPECT_STREQ("1", dispatch("g180*00", h));
+    EXPECT_EQ(180, h.lon.degrees);
+    EXPECT_EQ(static_cast<uint8_t>(0), h.lon.minutes);
+}
+
+// Top of the accepted range: one arcminute short of a full circle west is one
+// arcminute east.
+TEST(MeadeSet, site_longitude_unsigned_upper_bound_wraps_to_east)
+{
+    FakeHandlers h;
+    EXPECT_STREQ("1", dispatch("g359*59", h));
+    EXPECT_EQ(0, h.lon.degrees);
+    EXPECT_EQ(static_cast<uint8_t>(1), h.lon.minutes);
+}
+
+// KNOWN LIMITATION, pinned so that fixing it has to move a green assertion.
+// MeadeLongitude carries the sign only in `degrees`, so a west longitude smaller
+// than one degree cannot be represented: "000*30" (30' WEST) and "359*30" (30' east)
+// both come out {0, 30}, and downstream core::DayTime(0, 30, 0) reads that as 30'
+// EAST. The wire reply is still "1", so the error is silent. It is a sign flip, not a
+// truncation, so the worst case is "000*59": stored 118' — nearly 2 degrees, about
+// 7.9 minutes of local sidereal time — from the truth.
+// The fix is to let the struct hold the sign separately.
+TEST(MeadeSet, site_longitude_unsigned_sub_degree_west_loses_its_sign)
+{
+    FakeHandlers h;
+    EXPECT_STREQ("1", dispatch("g000*30", h));
+    EXPECT_STREQ("lon", h.lastCall);
+    EXPECT_EQ(0, h.lon.degrees);
+    EXPECT_EQ(static_cast<uint8_t>(30), h.lon.minutes);
+
+    // The genuinely-east neighbour that collides with it.
+    FakeHandlers e;
+    EXPECT_STREQ("1", dispatch("g359*30", e));
+    EXPECT_EQ(0, e.lon.degrees);
+    EXPECT_EQ(static_cast<uint8_t>(30), e.lon.minutes);
+}
+
+// 360 west is the same meridian as 000, but it is refused rather than wrapped: the
+// range check is what stops out-of-circle degrees reaching EEPROMStore, which clamps
+// them into an int16 and persists a site that is wrong rather than merely unwrapped.
+TEST(MeadeSet, site_longitude_unsigned_full_circle_is_rejected)
+{
+    FakeHandlers h;
+    EXPECT_STREQ("0", dispatch("g360*00", h));
+    EXPECT_EQ(nullptr, h.lastCall);
+}
+
+// The range check is shared, so it guards the signed path too.
+TEST(MeadeSet, site_longitude_signed_out_of_range_does_not_call_handler)
+{
+    FakeHandlers h;
+    EXPECT_STREQ("0", dispatch("g+400*00", h));
+    EXPECT_EQ(nullptr, h.lastCall);
+}
+
+TEST(MeadeSet, site_longitude_minutes_out_of_range_does_not_call_handler)
+{
+    FakeHandlers h;
+    EXPECT_STREQ("0", dispatch("g+121*99", h));
+    EXPECT_EQ(nullptr, h.lastCall);
+}
+
+// Degrees this far out would also push the westward arcminute count past INT16_MAX,
+// which is why the conversion works in `long` as well as rejecting the input.
+TEST(MeadeSet, site_longitude_unsigned_beyond_int16_arcminutes_is_rejected)
+{
+    FakeHandlers h;
+    EXPECT_STREQ("0", dispatch("g545*69", h));
+    EXPECT_EQ(nullptr, h.lastCall);
+}
+
+// Two-digit degrees are refused on both paths. Pre-#291 DayTime::ParseFromMeade took
+// two or three, so this is stricter than the legacy parser for a client that sends
+// ":Sg97*34#"; nothing observed on the wire does, MeadeProtocol.hpp documents "DDD",
+// and Cursor never backtracks, so accepting either width means hand-rolling the digit
+// reads. Relaxing it should relax the signed path at the same time.
+TEST(MeadeSet, site_longitude_unsigned_two_digit_degrees_does_not_call_handler)
+{
+    FakeHandlers h;
+    EXPECT_STREQ("0", dispatch("g97*34", h));
+    EXPECT_EQ(nullptr, h.lastCall);
+}
+
 // ---- UTC Offset (G) ---------------------------------------------------
 
 TEST(MeadeSet, utc_offset_positive)