Skip to content
This repository has been archived by the owner. It is now read-only.

Astro: Sun phase calculation fix #4158

Merged
merged 3 commits into from Aug 30, 2017
Merged

Astro: Sun phase calculation fix #4158

merged 3 commits into from Aug 30, 2017

Conversation

@gerrieg
Copy link
Contributor

@gerrieg gerrieg commented Aug 30, 2017

fixes #4138

gerrieg added 2 commits Aug 30, 2017
Signed-off-by: Gerhard Riegler <gerhard.riegler@gmail.com>
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;
Copy link
Contributor

@sjsf sjsf Aug 30, 2017

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?

Copy link
Contributor Author

@gerrieg gerrieg Aug 30, 2017

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.

Copy link
Contributor

@sjsf sjsf Aug 30, 2017

oh, I see. Thanks for the clarification!

Copy link
Contributor

@sjsf sjsf Aug 30, 2017

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.

Copy link
Contributor Author

@gerrieg gerrieg Aug 30, 2017

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

Signed-off-by: Gerhard Riegler <gerhard.riegler@gmail.com>
@sjsf sjsf merged commit 0d932b7 into eclipse-archived:master Aug 30, 2017
2 checks passed
@sjsf
Copy link
Contributor

@sjsf sjsf 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 astro branch Dec 22, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants