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

Astro: Sun phase calculation fix #4158

Merged
merged 3 commits into from Aug 30, 2017

Conversation

Projects
None yet
4 participants
@gerrieg
Copy link
Contributor

commented Aug 30, 2017

fixes #4138

gerrieg added some commits Aug 30, 2017

Fixed sun phase calculation
Signed-off-by: Gerhard Riegler <gerhard.riegler@gmail.com>
Removed noon from sun phase, because it is not a real phase
Signed-off-by: Gerhard Riegler <gerhard.riegler@gmail.com>
long matchStart = start != null ? start.getTimeInMillis()
: DateTimeUtils.truncateToMidnight(cal).getTimeInMillis();
long matchEnd = end != null ? end.getTimeInMillis() : DateTimeUtils.truncateToMidnight(cal).getTimeInMillis();
return cal.getTimeInMillis() >= matchStart && cal.getTimeInMillis() < matchEnd;

This comment has been minimized.

Copy link
@sjka

sjka Aug 30, 2017

Contributor

If no start/end are given, then both conditions are comparing against DateTimeUtils.truncateToMidnight(cal). How can this ever be true? Doesn't matchEnd needs 24h added in this case?

This comment has been minimized.

Copy link
@gerrieg

gerrieg Aug 30, 2017

Author Contributor

Range start/end are always for the current day. If no start/end are given (e.g night in northern hemisphere), there is nothing to match and should return false. So it's OK.

This comment has been minimized.

Copy link
@sjka

sjka Aug 30, 2017

Contributor

oh, I see. Thanks for the clarification!

This comment has been minimized.

Copy link
@sjka

sjka Aug 30, 2017

Contributor

No, wait, I still don't fully get it - can it happen that only one is given? I.e. if there is only a start, then it would also return false (because the second part is using the previous midnight), even if cal is after matchStart.

This comment has been minimized.

Copy link
@gerrieg

gerrieg Aug 30, 2017

Author Contributor

you are right! end must be truncated to endOfDay and not to midnight. Thanks!!!

Fixed range matching
Signed-off-by: Gerhard Riegler <gerhard.riegler@gmail.com>

@sjka sjka merged commit 0d932b7 into eclipse:master Aug 30, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
ip-validation
Details
@sjka

This comment has been minimized.

Copy link
Contributor

commented Aug 30, 2017

Thank you!

@kaikreuzer kaikreuzer added this to the 0.9.0 milestone Nov 30, 2017

@kaikreuzer kaikreuzer added the bug label Dec 15, 2017

@gerrieg gerrieg deleted the gerrieg:astro branch Dec 22, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.