fix(meade): restore hemisphere conversion for DEC coordinates - #300
Open
ClutchplateDude wants to merge 1 commit into
Open
fix(meade): restore hemisphere conversion for DEC coordinates#300ClutchplateDude wants to merge 1 commit into
ClutchplateDude wants to merge 1 commit into
Conversation
The Meade parser refactor (#291) dropped the conversion between the mount's internal DEC axis coordinate (0 = pole above the mount) and celestial declination. As a result :GD#/:Gd# reported the raw axis coordinate and :Sd#/:CM stored wire DEC as an axis coordinate, so clients saw wrong values (e.g. +10*00'00 for celestial +80) and slews landed at the complement of the intended DEC. Move the transform into pure, unit-testable core helpers (Declination::axisToCelestialSeconds / celestialToAxisSeconds, fromTotalSeconds, DayTime::splitSeconds), expose wire-format accessors on the app-level Declination overlay (getCelestialDegrees / fromCelestialDegrees), and route all three Meade boundary handlers through them: decFrom() for :GD/:Gd, decFromWire() for :Sd and :CM. Behavior matches the pre-refactor code exactly; LCD/OLED/status paths were unaffected and are unchanged. Co-Authored-By: Claude <noreply@anthropic.com>
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
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.
Summary
The Meade parser refactor (#291) dropped the hemisphere conversion between the mount's internal DEC axis coordinate and celestial declination at the Meade protocol boundary. This restores it for all DEC paths crossing that boundary.
Background: the firmware stores DEC as a mount-axis coordinate — 0 at the pole above the mount, ±180 at the opposite pole — not as celestial declination. Every conversion to/from the wire format must apply:
celestial = 90° − |axis|celestial = |axis| − 90°The old code did this inside
Declination::formatString()(output) andDeclination::ParseFromMeade()(input). The refactor replaced those call sites with raw component extraction, dropping the transform.ParseFromMeadewas left with zero call sites.Symptoms
:GD#/:Gd#(get DEC)+10*00'00#; southern mounts flipped the sign; values outside ±90° were possible:Sd#(set target DEC):CMsync (DEC+RA):Sd#Slews commanded from Meade clients landed at the complement of the intended DEC. LCD menu, OLED, and
:GX#status were unaffected (they useMount::DECString()), which made this easy to miss.Changes
src/core/types/Declination.{hpp,cpp}— the transform now lives in pure, unit-testable statics:axisToCelestialSeconds(),celestialToAxisSeconds(),fromTotalSeconds(). Formulas are byte-for-byte the historical onessrc/core/types/DayTime.{hpp,cpp}—splitSeconds(): signed seconds → (signed deg, unsigned min/sec)src/Declination.{hpp,cpp}— app overlay gainsgetCelestialDegrees()/fromCelestialDegrees();formatString()now delegates to the shared core conversion instead of its own inline copysrc/MeadeCommandProcessor.cpp—decFrom()(:GD/:Gd) applies the hemisphere correction; newdecFromWire()used byonSetTargetDec()(:Sd) andonSyncCoordinates()(:CM)Verification
unit_tests/test_core/types/test_declination.cpppio run -e rampsbuilds clean with-Werrorpio run -e esp32fails in this workspace, but identically on the clean tree (missingRA_RX_PIN/DEC_RX_PINin local config) — pre-existing, unrelated🤖 Generated with Claude Code