fix: handle zenith/nadir azimuth in ecef2geodetic_los#1131
Merged
Conversation
The azimuth at zenith and nadir is geometrically undefined, so set
it to 0.0 by convention. The pole case (latitude > POLELATZZZ) and
the default ENU-based azimuth are handled in the calling function
ecef2geodetic_los rather than inside enu2los, because:
* enu2los is a pure ENU -> LOS conversion that has no notion of
position, latitude, or the original ECEF components. The pole
fallback needs decef, and the zenith/nadir check is a
caller-specific convention, not a property of the ENU direction
itself.
* Keeping enu2los free of geographic context preserves it as a
reusable utility (e.g. the los2enu / enu2los pair in this file
is symmetric, and los2enu carries no special handling either).
Adds a matplotlib slider-driven script that round-trips a geodetic position and LOS direction through geodetic_los2ecef and back, comparing the production ecef2geodetic_los against a Python re-implementation of the legacy iterative path used in test_geodetic_aa.cpp.
edc2c8d to
77be881
Compare
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.
The
ecef2geodetic_losfunction previously relied onenu2losfor azimuth, which becomes geometrically undefined at zenith and nadir. This PR detects those cases and returns azimuth 0.0 by convention, while restoring the pole/ENU-based fallback for other viewing geometries.A small refactor swaps the manual Euclidean norm in
ecef_distanceforstd::hypot.A new interactive matplotlib test exercises the full LOS round-trip against a Python port of the legacy iterative solver.
Changes
src/core/geodesy/geodetic.cpp: Inecef2geodetic_los, detect zenith/nadir viewing geometries and return azimuth 0.0; otherwise preserve the existing pole-aware and ENU-based azimuth fallbacks. Switchecef_distancetostd::hypot.src/core/tests/geodetic_interactive.py: Interactive matplotlib tool that round-trips geodetic LOS throughgeodetic_los2ecefand both the production and a Python port of the retired iterativeecef2geodetic_los, plotting azimuth, zenith, altitude, latitude, and longitude with their differences over a full azimuth sweep driven by sliders.