fix: respect an explicit zero observer/timestamp in getSatelliteInfo - #61
Open
DanMat wants to merge 1 commit into
Open
fix: respect an explicit zero observer/timestamp in getSatelliteInfo#61DanMat wants to merge 1 commit into
DanMat wants to merge 1 commit into
Conversation
getSatelliteInfo used `observerLat || default` (and the same for lng, height, and rawTimestamp), so an explicitly-passed 0 — a valid observer at the equator / prime meridian / sea level, or the Unix epoch — was silently replaced by the default Santa Cruz observer. The function's own docstring example passes elevation 0, so the documented usage hit this. Use nullish coalescing (??) so only undefined falls back to the default. Adds a regression test.
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
1 participant
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.
Bug
getSatelliteInfousesobserverLat || default(and the same forobserverLng,observerHeight, andrawTimestamp), so an explicitly-passed0— a valid observer at the equator / prime meridian / sea level, or the Unix epoch — is silently replaced by the default Santa Cruz observer (36.96°N, −122.03°E, 0.37 km). The function's own docstring example passes0for elevation, so the documented usage hits this.Reproduced locally
Same ISS TLE + timestamp, observer at the origin vs a near-identical point:
(0, 0, 0)(1e-7, 1e-7, 1e-7)Before,
(0,0,0)diverges wildly because it's swapped for the default observer; after, it matches the near-origin result.Fix
Use nullish coalescing (
??) so onlyundefinedfalls back to the default. Added a regression test asserting(0,0,0)matches a near-origin observer (placed at the end of the suite so it doesn't perturb the cache-size assertion in the existingclearCachetest). Full suite (89 tests) + eslint pass.Written with AI assistance; I've reviewed and tested the change and will maintain it.