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

Fix compiler errors and ESP32 crash. Add minimumElevation to nextpass. #5

Merged
merged 3 commits into from
Feb 13, 2022
Merged

Conversation

PaulZC
Copy link
Contributor

@PaulZC PaulZC commented Jan 31, 2022

Hello @Hopperpop ,

I know this library is a few years old, but thank you for sharing it. It is a really nice piece of work!

I had some problems running the code on an ESP32 using Arduino 1.8.13 and esp32 by Espressif Systems (version 2.0.2). The code was crashing when I ran your Sgp4Predictor example. I traced the cause to a missing return statement in the overloaded copy of nextpass. This Pull Request corrects that.

I was also seeing two compiler errors while using a different version of the ESP32 compiler. It was complaining about missing parentheses and code ambiguity. So I fixed those too.

To be able to use this code to predict communication satellite passes, it is useful (actually essential) to be able to set a "minimum elevation" for a successful satellite pass. So I've added minimumElevation as a parameter for nextpass and have included an extra overload. The change is backward-compatible, it defaults to 0.0.

I have tested the code on ESP32, SAMD51, nRF52840 (Arduino Nano), Apollo3 (SparkFun Artemis) and STM32F4. It appears to work well on all platforms.

Thank you again for sharing, and I hope you like this Pull Request.

Very best wishes,
Paul

@Hopperpop Hopperpop merged commit b4df08a into Hopperpop:master Feb 13, 2022
@Hopperpop
Copy link
Owner

Thanks Paul for the fixes.
It's all a bit rusty for me, that's why it took some time.

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

Successfully merging this pull request may close these issues.

2 participants