search

mourner / suncalc

A tiny JavaScript library for calculating sun/moon positions and phases.
BSD 2-Clause "Simplified" License
3.08k stars 412 forks source link

Wrong azimuth angle #6

Open dinhnhat0401 opened 11 years ago

dinhnhat0401 commented 11 years ago

Sorry but i find that the sun azimuth must plus 180 degree so: function getAzimuth(H, phi, dec) { return atan(sin(H), cos(H) * sin(phi) - tan(dec) * cos(phi)); }

must be: function getAzimuth(H, phi, dec) { return PI + atan(sin(H), cos(H) * sin(phi) - tan(dec) * cos(phi)); }

mourner commented 11 years ago

You're right, I used south-based azimuth whereas north-based azimuth is usually used for sun position...

florianmski commented 11 years ago

Thank you ! I'm currently writing an Android app and I've ported this awesome tool to java and I didn't understand where was my mistake concerning the sun azimuth :)

xqjibz commented 9 years ago

I created PR #35 to address this with tests.

ghost commented 9 years ago

just noticed this bug myself, thanks for the great work.

The pull request seems to fix it for me, will be double checking in the next days.

spencerthayer commented 9 years ago

Will fix this be merged into the main branch soon?

mourner commented 9 years ago

Probably, but I'd suggest just adding a PI instead of waiting for it to be merged.

warpling commented 9 years ago

Is there a reason #35 hasn't been merged?

mourner commented 9 years ago

Yeah, hesitating to break backwards compatibility, leading to even more confusion :) I'll deal with this eventually, just didn't get my hands on it yet.

warpling commented 9 years ago

Makes sense! Crazy how many forks are downstream also…

ghost commented 9 years ago

If breaking backwards compatibility is a concern than it might be good to add a new interface method which would give the correct result and leave the old one for those who were doing their own workarounds.

xqjibz commented 9 years ago

I did not consider backwards functionality when I did the PR, that's my bad, I simply updated my library here and ran with it. @mourner if you have another solution, I'd be happy to work something up, otherwise no harm no foul on the PR.

ultrafez commented 7 years ago

According to semver, the way to avoid breaking users existing apps is to release a new version with a major version number change (i.e. release as 2.0.0). This way, the vast majority of users won't pull it into their project automatically.