[rbhbokka]

Regression from #291. INDI cannot set the site's UTC offset, so local time is wrong for every INDI/KStars session.

The defect

The :SG arm of handleMeadeSet() requires a leading +/- and exactly two digits. INDI sends one digit and a fractional part.

Captured off the wire on an INDI connect to an OpenAstroExplorer:

:SL10:42:31  :SC09/05/26  :SG+7.0  :Sg121*53  :St+37*20

:SG+7.0# is answered 0, the handler never fires, and the offset is silently left unset. The pre-#291 handler ran toInt() over the remainder (inCmd.substring(1, 4).toInt()), which accepted all of these forms.

The fix

A readUtcOffset() helper alongside the other read* helpers: the sign stays mandatory (as on develop — an unsigned offset is more likely a client bug than a deliberate +, and guessing wrong puts LST out by twice the offset), it accepts one or two digits, and leaves trailing characters unconsumed so +7.0 parses as +7 — matching the pre-#291 behaviour for everything INDI sends rather than inventing a new grammar.

Every form that already parsed behaves identically, sign included; +05 → 5 and -08 → −8 on both sides of the change.

Tests

Added under the existing ---- UTC Offset (G) ---- block: the exact INDI form; single-digit positive and negative (the suite never covered that intersection); unsigned — pinned as rejected, a deliberate narrowing versus pre-#291's toInt(); the sign-without-digits and non-numeric rejection paths; and the +5.5 half-hour case (India, Newfoundland), which truncates because the stored offset is whole hours — pinned so the limitation is documented rather than discovered in the field.

One existing test asserted that G+5 was rejected. That was pinning the regression, since the pre-#291 handler accepted it, so it is repointed at genuinely malformed input rather than removed; the rejection path is now covered by two tests instead of one.

pio test -e native: 277 pass at the original stacked tip (269 on then-develop + 9 added − 1 repointed). After the post-#304 rebase, CI on the single-commit branch reports 331/331 native test cases passing and all board builds green. Builds clean for oaeboardv1 and ramps.

Deliberately not included

No range validation. :SG+13 / :SG-15 are accepted on develop today and the tests pin that as-is — adding a -14..+14 check would change behaviour beyond the reported fault and could reject something a real client sends. Happy to fold it in if you'd rather have it here.

Scope is :SG only. :Sg (site longitude) had a related #291 regression of its own; it is fixed in #304 (merged), which also settled the sign-convention question by restoring the pre-#291 east-negative wire mapping.

Rebase note

Originally stacked on #304/#305. After both merged, rebased onto develop as a single commit containing only the :SG changes.

[ClutchplateDude]

I would contend that this is a INDI defect. The Meade Telescope Serial Command Protocol, Revision 2010.10 from 7 October 2010, is quite explicit that the command is
:SGsHH.H#
A sign and two digit hours, with one decimal.
That said, we should support the 0.5 case as well and since we supported a single digit pre-291, we should probably accept it here too, even if it does not adhere to the LX200 spec.
However, I don't think we should make the sign optional.

[ClutchplateDude]

See comment on main page.

[rbhbokka]

we should support the 0.5 case as well and since we supported a single digit pre-291, we should probably accept it here too [...] However, I don't think we should make the sign optional.

Fair on all three, and I agree it's an INDI deviation rather than a mount defect. I've put that in the :SG docs now, so the next person can see that accepting +7.0 is a compatibility call rather than us following the spec.

The sign is mandatory again. One test flipped; everything else already sent one. I also fixed the header line, which still read :SGsHH#.

One thing I'd rather mention than quietly skip: pre-291 the value went through String::toInt(), which accepts an unsigned number, so :SG07 did work back then. Requiring the sign is a narrowing, not a restoration. I still think it's the right call, since a missing sign is far more likely a client bug than a deliberate +, and guessing wrong puts sidereal time out by double the offset. It's also better than the old behaviour of replying "1" to :SGx having stored 0. I just didn't want to sell it as restoring old behaviour when it isn't.

On 0.5, I'd like to do that as a follow-up PR, because the parser part is the easy half:

• The offset is a single signed byte at UTC_OFFSET_ADDR = 39, in a map packed solid up to 73. There's no spare byte next to it and no unit tag, so reinterpreting it makes an existing mount's stored -5 read as -2:30 with nothing to warn you.
• DayTime::addHours takes a float, so changing the units of an int offset compiles clean and silently doubles the error.
• The lastFlashed migration hook isn't idempotent if someone downgrades and upgrades again. A dedicated ExtendedItemFlag bit would be.
• It also touches the LCD editor and the handler interface.

Three questions, since they're your calls:

1. Are downgrades supported? If not, I'd reuse byte 39 as half-hours behind a new flag bit. If they are, I'd leave 39 as whole hours so old firmware stays roughly right, and append a minutes field.
2. Does :GG stay sHH#? It's been that format forever, and sHH.H# is a 4 to 6 byte reply change that would break fixed-width clients. I can't tell from here whether INDI copes with a decimal.
3. Is :SG+5.3 rejected, or rounded?

Rebased on top of #304.

[rbhbokka]

@ClutchplateDude Sign is required again, and both +7 and +07 are accepted. Half-hour offsets still need a separate EEPROM change, so that is not in this PR.

[ClutchplateDude]

Answers to the three questions, with what the code and INDI's driver actually do (checked against INDI master today):

1. Downgrades — treat them as supported, because the safe layout is nearly free. Nothing in the repo promises downgrade support, but the web flasher lets anyone load any version, so downgrades happen whether we support them or not. The map is packed to STORE_SIZE = 74, but extending to 75 and putting a minutes byte at address 74 behind a new ExtendedItemFlag bit (bits are free above 0x0800) breaks nothing in either direction: byte 39 stays whole hours forever, old firmware after a downgrade reads byte 39 correctly and ignores byte 74, and new firmware reading old data sees the flag unset → 0 minutes. No reinterpretation of byte 39, no migration. That said, deferred — see #2; this is the plan of record for whenever half-hour support happens, not a request to build it now.

2. :GG stays sHH#. INDI cannot use a fractional reply even if we sent one: :GG goes through a header macro into a generic integer parser —

// lx200driver.h
#define getUTCOffset(fd, x) getCommandInt(fd, x, ":GG#")

— parsed into an int, then negated (*offset = lx200_utc_offset * -1). A +05.5# reply would at best be truncated by their integer parse, so the 4→5/6 byte reply change would gain INDI nothing while risking fixed-width clients. OAT has emitted sHH# forever (pre-#291 %+03d#; current tests pin +03#/-05#), which also matches the spec reply. Decision: we are not adding half-hour storage until INDI's driver can represent it. Until then, :SG+5.5 truncating to +5 is the least-wrong behavior and is pinned by tests.

3. :SG+5.3 is accepted and truncated toward zero — which is what this PR implements, and what we're keeping. readUtcOffset() consumes the sign plus one or two digits and leaves the rest unconsumed: +5.3 → +5, +5.9 → +5, -5.5 → -5. That is byte-for-byte what pre-#291's toInt() did (utc_offset_half_hour_zone_drops_the_fraction pins it). Rejecting would leave the offset unset — a 5.5-hour error instead of a 30-minute one — and rounding-to-nearest would deviate from pre-#291 for no gain, with a .5 tie-break problem on top.

For the record, your EEPROM constraints were all accurate — the map is packed solid through 73, the extended flags end at 0x0800, and the lastFlashed hook is one-way as described. None of the three questions block this PR: Q3 describes its current behavior, and Q1/Q2 belong to the (now deferred) follow-up.

Maintainer note: after #304 merged, I rebased this branch onto develop as a single commit (content unchanged) to clear the conflict flag, and updated the description's stale sections — the "optional sign" wording (the sign is required, and the tests pin unsigned input as rejected), the test counts, and the file-overlap section.