Fix Meade DEC conversion and 64-byte response truncation - #299
Open
2006benny wants to merge 1 commit into
Open
Conversation
Two protocol regressions in the refactored Meade layer, both verified against an OpenAstroExplorer (ESP32, v1.13.20): 1. DEC convention leak: the mount stores declination internally as 'degrees from the pole' (0 = pole) but decFrom()/onSetTargetDec()/ onSyncCoordinates() passed raw internal values across the protocol boundary. :GD# reported +00*00'00 at home instead of +90*00'00 (the :GX# path converts correctly via Declination::formatString, which is why OATControl looked right while INDI/KStars was off by 90 degrees), and :Sd/:CM interpreted celestial DEC as pole distance, sending GOTO slews toward the ground. Convert at the boundary in both directions using the same arithmetic as Declination::formatString/ParseFromMeade. Validated by a native roundtrip test against core::Declination over -89..+89 deg in both hemispheres, including the observed value pair (-1*37' wire vs +88*22' actual). 2. MeadeResponse::Capacity was 64 bytes; the :XGM# reply (board, two stepper descriptors, full addon list) exceeds 100 bytes on an OAE, so it was silently truncated mid-token and the '#' terminator was dropped, hanging every client that reads to the terminator. Raise the capacity to 160. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KSo3nJCWH137eBiNTL7JVo
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two protocol regressions in the refactored Meade layer, both verified against an OpenAstroExplorer (ESP32, v1.13.20):
DEC convention leak: the mount stores declination internally as 'degrees from the pole' (0 = pole) but decFrom()/onSetTargetDec()/ onSyncCoordinates() passed raw internal values across the protocol boundary. :GD# reported +0000'00 at home instead of +9000'00 (the :GX# path converts correctly via Declination::formatString, which is why OATControl looked right while INDI/KStars was off by 90 degrees), and :Sd/:CM interpreted celestial DEC as pole distance, sending GOTO slews toward the ground. Convert at the boundary in both directions using the same arithmetic as Declination::formatString/ParseFromMeade. Validated by a native roundtrip test against core::Declination over -89..+89 deg in both hemispheres, including the observed value pair (-137' wire vs +8822' actual).
MeadeResponse::Capacity was 64 bytes; the :XGM# reply (board, two stepper descriptors, full addon list) exceeds 100 bytes on an OAE, so it was silently truncated mid-token and the '#' terminator was dropped, hanging every client that reads to the terminator. Raise the capacity to 160.