la1k / libpredict

A satellite orbit prediction library
GNU General Public License v2.0
39 stars 24 forks source link

Issue62 - Prediction of maximum elevation #67

Closed bjorgan closed 7 years ago

bjorgan commented 7 years ago

Adds a function predict_max_elevation(...) for predicting the time for the maximum elevation of the next or the current pass. Solves issue #62.

This turned out to be more problematic than I thought, since satellites in deep space can have multiple maxima. However, assuming there can only be at most two, the function is accurate enough for most purposes (though dependent on predict_next_aos/los' ability to find a proper pass). Tests check the derivative, sample the pass for higher elevations and check the consistency of the found time when having different starting points throughout the pass. This is checked for various passes and various types of orbits.

AOS/LOS functions were changed a bit for higher accuracy in problematic cases, and the tests were rewritten to check AOS/LOS in a more proper way for several passes. This should also be good preparation for increasing the numerical accuracy for #18.

Discussion questions:

  1. The function returns the time for the max elevation, like predict_next_aos/los returns a time. Very consistent. But would it make more sense to return a struct predict_observation? In most cases, the user needs the additional properties, not only the time, and would have to call predict_orbit and predict_observe_orbit again after calling predict_max_elevation. struct predict_observation also has a time field.

  2. In general, not sure about the name predict_max_elevation, since it doesn't return an elevation.

ryeng commented 7 years ago

Thanks for the PR!

  1. I agree. The caller would probably want the predict_observation struct. The same goes for predict_next_aos/los. I also agree that all three should be consistent. Should we create an issue to change those too?

  2. predict_at_max_elevation?

bjorgan commented 7 years ago

@ryeng: Renamed to predict_at_max_elevation and modified to return the struct. Also squashed some commits.

I agree that predict_next_aos/los also should return structs. Added a new issue (https://github.com/la1k/libpredict/issues/68).