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

Add getRiseAndSetAtSolarAngle method #135

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

MatthiasKunnen
Copy link

@MatthiasKunnen MatthiasKunnen commented Jun 26, 2019

I have a use case where I need to get the time on which the sun will be at multiple angles. Since these altitudes are unknown I can't really name them. Therefore I've added the getRiseAndSetAtSolarAngle method.

The method can be seen as a shortcut to addTime followed by getTimes that does not pollute the times array. It saves a method call and does not calculate any other times when it doesn't need to.

@MatthiasKunnen
Copy link
Author

@mourner, are you accepting PRs at the moment?

@mourner
Copy link
Owner

mourner commented Dec 27, 2019

Sorry for not reviewing this in time — I got swamped by other commitments and have fallen behind on this project. :( Closing as the PR is superseded by #137, which is more accurate and complete.

@mourner mourner closed this Dec 27, 2019
@MatthiasKunnen
Copy link
Author

@mourner, that issue seems to address a different issue. #137 changes the height of the observer. This PR allows passing of the angle of the sun you want to know the sun rise and sun set time of. Am I incorrect and does #137 provide a way to do this?

@mourner
Copy link
Owner

mourner commented Feb 3, 2020

Ah apologies, I misread the description — reopening.

@mourner mourner reopened this Feb 3, 2020
@MatthiasKunnen
Copy link
Author

The name might be confusing indeed. What do you think about getTimesAtAngle?

@MatthiasKunnen
Copy link
Author

@mourner, I've made some additions:

  • Rename the method to be more intuiitive
  • Add elevation parameter as was added to getTimes
  • Accounted for atmospheric refraction
    It's not 100% correct. I tried using the astroRefraction function but that seems to be off by a a few seconds. I'll add a commit to showcase this

SunCalc.getRiseAndSetAtSolarAngle = function (date, angle, lat, lng, elevation) {

elevation = elevation || 0;
angle = angle - astroRefraction(angle) * 100;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any idea why I need to multiply by 100?

@MatthiasKunnen MatthiasKunnen changed the title Added the getTimesAtAltitude method Add getTimesAtSolarAngle method Feb 8, 2020
@gtim
Copy link

gtim commented Jan 21, 2022

This saved me some time, thank you for sharing your solution @MatthiasKunnen !

@MatthiasKunnen MatthiasKunnen changed the title Add getTimesAtSolarAngle method Add getRiseAndSetAtSolarAngle method Jan 21, 2022
@Hypnos3
Copy link

Hypnos3 commented Mar 13, 2022

published a new version which include this as https://github.com/Hypnos3/suncalc3

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.

None yet

4 participants