Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Additional Arguments Support #15

Open
mia-n opened this issue Oct 20, 2023 · 5 comments
Open

Additional Arguments Support #15

mia-n opened this issue Oct 20, 2023 · 5 comments

Comments

@mia-n
Copy link

mia-n commented Oct 20, 2023

Hello! I don't know if this is actively maintained, but I was playing around with it and I noticed that you're hardcoding the CENTER param to the sun's body center.

This gives pretty nonsensical results if examining natural satellites. For instance, for ephemeris of saturn's moon Titan you'd really want the CENTER to be either 500@699 (the body center of Saturn itself) or 500@6 (the Saturn System Barycenter).

For my purposes I forked your repo and just added an extra arg (centerID) to the client function instead of passing it in hardcoded.

Then I started playing around and realized there were even more arguments I'd like to be able to tweak - for instance, passing in STEP_SIZE in addition to START_TIME and STOP_TIME, or passing in the TLIST parameter instead of START_TIME and STOP_TIME.

Adjusting REF_PLANE is also super useful for getting correct-looking output for natural satellites.

I'm happy to make a pull request with some adjustments to this effect, but I thought I'd write this up to get your input first.

There's a few ways one could go about it and I don't have a strong opinion on style. Some options:

  1. Update ephemeris_orbital_elements and ephemeris_vector to accept many more arguments in their function signature
  2. Update them to accept a struct of Optional ephemeris-specific parameters (and then serialize that into the query-string tuple vec, leaving out any that aren't specified). I think this is a little cleaner and more flexible, but it does require potential clients to build the request struct.
  3. Both of the above break the existing API contract, which if you want to avoid I could just make totally new functions that act like one of the above options but leaving the existing API contract intact.
  4. Something else?
@NiklasVousten
Copy link
Contributor

I am currently working on a Command Builder, which allows using these additional arguments. My last update was a while ago, but I plan to continue working on it soon.

See here: CommandBuilder Branch

@mia-n
Copy link
Author

mia-n commented Oct 20, 2023

Oh, awesome! Cool :D Well, if you want assistance or contributions to some other part let me know.

@mvernacc
Copy link

mvernacc commented Apr 1, 2024

@g1aeder I saw your CommandBuilder branch and it looks useful for a project I'm working on. Do you expect to finish it?

@podusowski
Copy link
Owner

Would be happy to accept it once ready. :) Sorry for not paying too much attention to this crate.

@NiklasVousten
Copy link
Contributor

NiklasVousten commented Apr 19, 2024

Yes I am still working on it. I pushed the current progress to weeks ago, with an API change (Old one available with a linked commit). Not sure how the interfacing should be done. The current idea is to use structs and enums to create the command, which is additionally validated.
Not sure if this is the best approach.
Validation would be also done by the Horizon-API, so this might not be needed.

Happy for input from your sides

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants