From 91991daff64c1fd16c643c3a9d1ef2fca6313978 Mon Sep 17 00:00:00 2001 From: Kiryl Date: Sun, 6 Sep 2026 20:06:49 -0700 Subject: [PATCH] fix(meade): accept the UTC offset format INDI sends Regression from #291. The :SG arm of handleMeadeSet() uses Cursor::signed2(), which requires a leading sign and exactly two digits. INDI sends one digit and a fractional part, captured on connect to an OpenAstroExplorer: :SL10:42:31 :SC09/05/26 :SG+7.0 :Sg121*53 :St+37*20 So :SG+7.0# is answered "0", the handler never fires, and the site's UTC offset is silently left unset for every INDI/KStars session. Add a readUtcOffset() helper alongside the other read* helpers, accepting an optional sign and one or two digits and leaving trailing characters unconsumed so "+7.0" reads as +7. That matches the pre-#291 handler, which ran toInt() over the remainder, rather than inventing a new grammar. Every form that already parsed is unchanged, sign included. The one existing test that asserted ":SG+5" was rejected was pinning the regression -- the pre-#291 handler accepted it -- so it is repointed at genuinely malformed input; the rejection path is now covered by two tests instead of one. Range validation is deliberately not added: :SG+13 is accepted today and the tests pin that as-is, since rejecting it would change behaviour beyond the reported fault. Co-authored-by: Claude --- src/core/meade/MeadeParserSet.cpp | 30 +++++++- src/core/meade/MeadeProtocol.hpp | 9 ++- unit_tests/test_core/meade/test_MeadeSet.cpp | 73 +++++++++++++++++++- 3 files changed, 105 insertions(+), 7 deletions(-) diff --git a/src/core/meade/MeadeParserSet.cpp b/src/core/meade/MeadeParserSet.cpp index 909624a8..8a4be111 100644 --- a/src/core/meade/MeadeParserSet.cpp +++ b/src/core/meade/MeadeParserSet.cpp @@ -76,6 +76,32 @@ bool readLongitude(Cursor &c, MeadeLongitude &out) return true; } +// Format: "[+-]H[H]", with anything after the hours left unconsumed, so the +// ":SG+7.0#" that INDI sends on connect parses as +7 -- as it did before the +// refactor, when the handler ran toInt() over "+7.". Requiring a sign and +// exactly two digits makes INDI's UTC-offset push fail with "0" and leaves the +// site offset unset. +bool readUtcOffset(Cursor &c, int &out) +{ + const bool negative = c.match('-'); + if (!negative) + { + c.match('+'); // Optional; an unsigned offset is positive. + } + unsigned hh; + if (!c.digits(1, hh)) + { + return false; + } + unsigned secondDigit; + if (c.digits(1, secondDigit)) + { + hh = hh * 10 + secondDigit; + } + out = negative ? -static_cast(hh) : static_cast(hh); + return true; +} + // Set ack: "1" on success, "0" on failure. No framing terminator. void writeSetAck(MeadeResponse &r, bool ok) { @@ -222,9 +248,9 @@ void handleMeadeSet(MeadeResponse &r, const char *s, IMeadeSetHandlers &h) case 'G': { - // G
+ // G[] int hours; - if (!c.signed2(hours)) + if (!readUtcOffset(c, hours)) { writeChar(r, '0'); return; diff --git a/src/core/meade/MeadeProtocol.hpp b/src/core/meade/MeadeProtocol.hpp index b110ca0b..bd219f05 100644 --- a/src/core/meade/MeadeProtocol.hpp +++ b/src/core/meade/MeadeProtocol.hpp @@ -348,10 +348,13 @@ // Information: // This sets the offset of the timezone in which the mount is in hours from UTC. // Returns: -// "1" +// "1" if successfully set +// "0" otherwise // Parameters: -// "s" is the sign -// "HH" is the number of hours +// "s" (optional) is the sign, an unsigned offset being positive +// "HH" is the number of hours, one or two digits +// Remarks: +// Anything following the hours is ignored, so the ":SG+7.0#" that INDI sends is read as +7. The offset is whole hours only, so half-hour zones such as India and Newfoundland cannot be expressed. // // :SLHH:MM:SS# // Description: diff --git a/unit_tests/test_core/meade/test_MeadeSet.cpp b/unit_tests/test_core/meade/test_MeadeSet.cpp index 7c0074ff..79b809ca 100644 --- a/unit_tests/test_core/meade/test_MeadeSet.cpp +++ b/unit_tests/test_core/meade/test_MeadeSet.cpp @@ -318,10 +318,79 @@ TEST(MeadeSet, utc_offset_negative) EXPECT_EQ(-8, h.utc); } -TEST(MeadeSet, utc_offset_malformed_length_does_not_call_handler) +// The exact bytes INDI puts on the wire when it pushes the site on connect. +TEST(MeadeSet, utc_offset_indi_fractional_form) { FakeHandlers h; - EXPECT_STREQ("0", dispatch("G+5", h)); + EXPECT_STREQ("1", dispatch("G+7.0", h)); + EXPECT_STREQ("utc", h.lastCall); + EXPECT_EQ(7, h.utc); +} + +TEST(MeadeSet, utc_offset_single_digit_positive) +{ + FakeHandlers h; + EXPECT_STREQ("1", dispatch("G+5", h)); + EXPECT_EQ(5, h.utc); +} + +TEST(MeadeSet, utc_offset_single_digit_negative) +{ + FakeHandlers h; + EXPECT_STREQ("1", dispatch("G-3", h)); + EXPECT_EQ(-3, h.utc); +} + +TEST(MeadeSet, utc_offset_unsigned_is_positive) +{ + FakeHandlers h; + EXPECT_STREQ("1", dispatch("G07", h)); + EXPECT_EQ(7, h.utc); +} + +// Half-hour zones (India, Newfoundland) are unrepresentable: onSetUtcOffset takes +// whole hours, so ".5" is left unconsumed and the site lands 30 minutes out. +// Pinned here so the limitation is documented rather than discovered in the field. +TEST(MeadeSet, utc_offset_half_hour_zone_drops_the_fraction) +{ + FakeHandlers h; + EXPECT_STREQ("1", dispatch("G+5.5", h)); + EXPECT_EQ(5, h.utc); +} + +// Nothing range-checks the hours. "+13" is a real offset (Tonga); "-15" is not, +// and is taken all the same. Both pin the current permissive behaviour -- +// whether to reject impossible offsets is deliberately left to a follow-up. +// Note that IMeadeSetHandlers::onSetUtcOffset documents "@param hours Signed +// wire value (-12..+14)"; that range is stated but has never been enforced, +// here or before this parser accepted the unsigned and single-digit forms. +TEST(MeadeSet, utc_offset_two_digit_high_value) +{ + FakeHandlers h; + EXPECT_STREQ("1", dispatch("G+13", h)); + EXPECT_STREQ("utc", h.lastCall); + EXPECT_EQ(13, h.utc); +} + +TEST(MeadeSet, utc_offset_impossible_value_is_accepted) +{ + FakeHandlers h; + EXPECT_STREQ("1", dispatch("G-15", h)); + EXPECT_STREQ("utc", h.lastCall); + EXPECT_EQ(-15, h.utc); +} + +TEST(MeadeSet, utc_offset_sign_without_digits_does_not_call_handler) +{ + FakeHandlers h; + EXPECT_STREQ("0", dispatch("G+", h)); + EXPECT_EQ(nullptr, h.lastCall); +} + +TEST(MeadeSet, utc_offset_non_numeric_does_not_call_handler) +{ + FakeHandlers h; + EXPECT_STREQ("0", dispatch("Gx", h)); EXPECT_EQ(nullptr, h.lastCall); }