Conversation
peteasa
commented
Feb 22, 2024
…racker-Firmware.git into near_meridian_fix
rbhbokka
added a commit
to rbhbokka/OpenAstroTracker-Firmware
that referenced
this pull request
Sep 19, 2026
Regression from OpenAstroTech#291. DecCoordinate, MeadeLatitude and MeadeLongitude carried the sign in the sign bit of `degrees`, which cannot represent a negative value whose degrees component is zero. Cursor::signed2() computes -(int)0, which is 0, so the sign was destroyed inside the struct before any handler saw it: :Sd-00*30:00# -> {0, 30, 0} sets +00*30:00 :St-00*30# -> {0, 30} equatorial sites :Sg-000*05# -> {0, 5} central London A one-degree error in a band straddling the celestial equator, and :CM sync writes it into the mount's home reference permanently. The pre-OpenAstroTech#291 DayTime::ParseFromMeade applied the sign to the whole total and was correct. Replace signed2/signed3 with Cursor::optionalSign(), which reports the sign without folding it into a magnitude, and give the three structs an explicit `negative` field. The readers keep sign and magnitude apart to the end, and the writers and the MeadeCommandProcessor boundary read the sign off the undivided total rather than off a divided degrees component. The accepted grammar is byte-for-byte unchanged -- readMandatorySign() preserves the existing requirement for an explicit sign, so this commit changes only what the parser does with a sign it already accepted. This supersedes OpenAstroTech#241, which diagnosed the same root cause and proposed the same remedy of carrying the sign as its own channel. Its two target functions, Longitude::formatString() and Longitude::formatStringForMeade(), have had no callers since OpenAstroTech#291 routed around them, so the idea is applied here where the code now lives. 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
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.
I noticed that setting Latitude of observatories close to the meridian fails. The failure occurs because -0 degrees is not necessarily a negative value.
This draft change is for your consideration and fixes this issue.