search

JuliaSpace / SatelliteToolbox.jl

A toolbox for satellite analysis written in julia language.
MIT License
248 stars 33 forks source link

💄 Return orbital elements from propagate! #56

Closed andreas-theo closed 3 years ago

andreas-theo commented 3 years ago

Hi,

First of all thank you for your work on SatelliteToolbox, I've been using it for some time to simulate satellite orbits.

I noticed than in v0.9 the propagate! function no longer returns an array of orbital elements, yet on line 65 of api.jl you are creating the vector of KeplerianElements that would hold this output but never populate it. This pull request populates result_orb with the orbital elements at each time step using the new get_mean_elements function. I couldn't tell if you simply forgot to erase the line that allocates the result_orb array or if you added it because you intended to return it in the future. If it's the former then please close this PR.

I have tried to change all the calls to propagate to handle the new output. I have not updated the documentation but I will do so if you find this change useful.

ronisbr commented 3 years ago

Hi @andreas-theo !

Thanks for the PR, but I really decided to avoid returning mean orbital elements in the API. Some propagators, like the numerical ones, compute the osculating state vector and there is not an easy way to retrieve the mean orbital elements. If we keep this information in the propagate! API, then the numerical propagators would have to execute some kind of interpolation to provide it. Thus, I decided to only return the osculating state vector and, if the user wants, use the get_mean_elements to obtain the mean Keplerian elements. Makes sense?

I wrote the API specification here: https://github.com/JuliaSpace/SatelliteToolbox.jl/blob/master/src/orbit/propagators/PROPAGATORS.md

andreas-theo commented 3 years ago

That makes sense yes. You can close this PR then. Sorry I didn't notice the API specification.

ronisbr commented 3 years ago

No problem! Thanks for the PR :)