[astro] Fix returning wrong sun phase name (#14078)

* Add tests and fix very minor bug
* Correct wrong test
* Update tests and fix sorting
* Some checkstyle improvements

Signed-off-by: lsiepel <leosiepel@gmail.com>

diff --git a/bundles/org.openhab.binding.astro/src/main/java/org/openhab/binding/astro/internal/calc/SunCalc.java b/bundles/org.openhab.binding.astro/src/main/java/org/openhab/binding/astro/internal/calc/SunCalc.java
index 1ab31c5bb267987c19a6c9e2a23e972171ac835b..3861cf1323de6373a5e86dedbf8633c6c6c89452 100644 (file)
--- a/bundles/org.openhab.binding.astro/src/main/java/org/openhab/binding/astro/internal/calc/SunCalc.java
+++ b/bundles/org.openhab.binding.astro/src/main/java/org/openhab/binding/astro/internal/calc/SunCalc.java
@@ -12,7 +12,13 @@
  */
 package org.openhab.binding.astro.internal.calc;
 
+import java.util.ArrayList;
 import java.util.Calendar;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
 import java.util.Map.Entry;
 
 import org.openhab.binding.astro.internal.model.Eclipse;
@@ -251,9 +257,9 @@ public class SunCalc {
         sun.setSeason(seasonCalc.getSeason(calendar, latitude, useMeteorologicalSeason));
 
         // phase
-        for (Entry<SunPhaseName, Range> rangeEntry : sun.getAllRanges().entrySet()) {
+        for (Entry<SunPhaseName, Range> rangeEntry : sortByValue(sun.getAllRanges()).entrySet()) {
             SunPhaseName entryPhase = rangeEntry.getKey();
-            if (rangeEntry.getValue().matches(Calendar.getInstance())) {
+            if (rangeEntry.getValue().matches(calendar)) {
                 if (entryPhase == SunPhaseName.MORNING_NIGHT || entryPhase == SunPhaseName.EVENING_NIGHT) {
                     sun.getPhase().setName(SunPhaseName.NIGHT);
                 } else {
@@ -336,4 +342,24 @@ public class SunCalc {
     private double getSunriseJulianDate(double jtransit, double jset) {
         return jtransit - (jset - jtransit);
     }
+
+    public static Map<SunPhaseName, Range> sortByValue(Map<SunPhaseName, Range> map) {
+        List<Entry<SunPhaseName, Range>> list = new ArrayList<>(map.entrySet());
+
+        Collections.sort(list, new Comparator<Entry<SunPhaseName, Range>>() {
+            @Override
+            public int compare(Entry<SunPhaseName, Range> p1, Entry<SunPhaseName, Range> p2) {
+                Range p1Range = p1.getValue();
+                Range p2Range = p2.getValue();
+                return p1Range.compareTo(p2Range);
+            }
+        });
+
+        Map<SunPhaseName, Range> result = new LinkedHashMap<>();
+        for (Entry<SunPhaseName, Range> entry : list) {
+            result.put(entry.getKey(), entry.getValue());
+        }
+
+        return result;
+    }
 }

diff --git a/bundles/org.openhab.binding.astro/src/main/java/org/openhab/binding/astro/internal/calc/SunZodiacCalc.java b/bundles/org.openhab.binding.astro/src/main/java/org/openhab/binding/astro/internal/calc/SunZodiacCalc.java
index 06e1f0aa59cbefa14ac8476684418bd8e5a9fe39..0a78737341e126b4cd889fb829aa653542680d28 100644 (file)
--- a/bundles/org.openhab.binding.astro/src/main/java/org/openhab/binding/astro/internal/calc/SunZodiacCalc.java
+++ b/bundles/org.openhab.binding.astro/src/main/java/org/openhab/binding/astro/internal/calc/SunZodiacCalc.java
@@ -37,7 +37,6 @@ public class SunZodiacCalc {
      * Returns the zodiac for the specified calendar.
      */
     public Optional<SunZodiac> getZodiac(Calendar calendar) {
-
         int year = calendar.get(Calendar.YEAR);
         List<SunZodiac> zodiacs;
 

diff --git a/bundles/org.openhab.binding.astro/src/main/java/org/openhab/binding/astro/internal/job/Job.java b/bundles/org.openhab.binding.astro/src/main/java/org/openhab/binding/astro/internal/job/Job.java
index 3b495288b17dd8279801a5f870862d0e2f7285f2..6c4a95113b1c9ae16a6d781411b563317444f486 100644 (file)
--- a/bundles/org.openhab.binding.astro/src/main/java/org/openhab/binding/astro/internal/job/Job.java
+++ b/bundles/org.openhab.binding.astro/src/main/java/org/openhab/binding/astro/internal/job/Job.java
@@ -86,7 +86,6 @@ public interface Job extends SchedulerRunnable, Runnable {
      */
     public static void scheduleEvent(String thingUID, AstroThingHandler astroHandler, Calendar eventAt,
             List<String> events, String channelId, boolean configAlreadyApplied) {
-
         if (events.isEmpty()) {
             return;
         }

diff --git a/bundles/org.openhab.binding.astro/src/main/java/org/openhab/binding/astro/internal/model/MoonPhase.java b/bundles/org.openhab.binding.astro/src/main/java/org/openhab/binding/astro/internal/model/MoonPhase.java
index 2787c5afe7c466f7be271f42545657e8b134f282..bc6fb61aecfe01ca1ea3a2d04db4cd375aca432b 100644 (file)
--- a/bundles/org.openhab.binding.astro/src/main/java/org/openhab/binding/astro/internal/model/MoonPhase.java
+++ b/bundles/org.openhab.binding.astro/src/main/java/org/openhab/binding/astro/internal/model/MoonPhase.java
@@ -31,7 +31,7 @@ public class MoonPhase {
     private Calendar firstQuarter;
     private Calendar full;
     private Calendar thirdQuarter;
-    private Calendar _new;
+    private Calendar newCalendar;
     private double age;
     private double illumination;
     private double agePercent;
@@ -85,14 +85,14 @@ public class MoonPhase {
      * Returns the date of the new moon.
      */
     public Calendar getNew() {
-        return _new;
+        return newCalendar;
     }
 
     /**
      * Sets the date of the new moon.
      */
-    public void setNew(Calendar _new) {
-        this._new = _new;
+    public void setNew(Calendar newCalendar) {
+        this.newCalendar = newCalendar;
     }
 
     /**

diff --git a/bundles/org.openhab.binding.astro/src/main/java/org/openhab/binding/astro/internal/model/Range.java b/bundles/org.openhab.binding.astro/src/main/java/org/openhab/binding/astro/internal/model/Range.java
index 9ec600f2d4fa2853f833e7974387f3ebd5bd7967..c40303de88e921b2165072fc52c5ff4dd2a1b2b7 100644 (file)
--- a/bundles/org.openhab.binding.astro/src/main/java/org/openhab/binding/astro/internal/model/Range.java
+++ b/bundles/org.openhab.binding.astro/src/main/java/org/openhab/binding/astro/internal/model/Range.java
@@ -15,6 +15,7 @@ package org.openhab.binding.astro.internal.model;
 import static org.openhab.core.library.unit.MetricPrefix.MILLI;
 
 import java.util.Calendar;
+import java.util.Comparator;
 
 import javax.measure.quantity.Time;
 
@@ -81,4 +82,13 @@ public class Range {
         long matchEnd = end != null ? end.getTimeInMillis() : DateTimeUtils.endOfDayDate(cal).getTimeInMillis();
         return cal.getTimeInMillis() >= matchStart && cal.getTimeInMillis() < matchEnd;
     }
+
+    private static Comparator<Calendar> nullSafeCalendarComparator = Comparator.nullsFirst(Calendar::compareTo);
+
+    private static Comparator<Range> rangeComparator = Comparator.comparing(Range::getStart, nullSafeCalendarComparator)
+            .thenComparing(Range::getEnd, nullSafeCalendarComparator);
+
+    public int compareTo(Range that) {
+        return rangeComparator.compare(this, that);
+    }
 }

diff --git a/bundles/org.openhab.binding.astro/src/test/java/org/openhab/binding/astro/internal/calc/SunCalcTest.java b/bundles/org.openhab.binding.astro/src/test/java/org/openhab/binding/astro/internal/calc/SunCalcTest.java
index 4c756e5cf2fe54bfdfbcfce8804ed29afb714bce..90758426ebf4295f53226ba250c2dc248cf1b4c5 100644 (file)
--- a/bundles/org.openhab.binding.astro/src/test/java/org/openhab/binding/astro/internal/calc/SunCalcTest.java
+++ b/bundles/org.openhab.binding.astro/src/test/java/org/openhab/binding/astro/internal/calc/SunCalcTest.java
@@ -80,8 +80,8 @@ public class SunCalcTest {
         assertNotNull(sun.getMorningNight());
         assertNotNull(sun.getEveningNight());
 
-        // for an old date the phase is always null
-        assertNull(sun.getPhase().getName());
+        // for an old date the phase should also be calculated
+        assertNotNull(sun.getPhase().getName());
     }
 
     @Test
@@ -274,6 +274,34 @@ public class SunCalcTest {
                 sun.getAllRanges().get(SunPhaseName.EVENING_NIGHT).getStart());
     }
 
+    @Test
+    public void testIssue7642CivilDawnEnd() {
+        TimeZone tZone = TimeZone.getTimeZone("Europe/London");
+        Calendar tDate = SunCalcTest.newCalendar(2020, Calendar.MAY, 13, 5, 12, tZone);
+
+        Sun sun = sunCalc.getSunInfo(tDate, 53.524695, -2.4, 0.0, true);
+        assertEquals(SunPhaseName.CIVIL_DAWN, sun.getPhase().getName());
+    }
+
+    @Test
+    public void testIssue7642SunRiseStart() {
+        // SunCalc.ranges was not sorted, causing unexpected output in corner cases.
+        TimeZone tZone = TimeZone.getTimeZone("Europe/London");
+        Calendar tDate = SunCalcTest.newCalendar(2020, Calendar.MAY, 13, 5, 13, tZone);
+
+        Sun sun = sunCalc.getSunInfo(tDate, 53.524695, -2.4, 0.0, true);
+        assertEquals(SunPhaseName.SUN_RISE, sun.getPhase().getName());
+    }
+
+    @Test
+    public void testIssue7642DaylightStart() {
+        TimeZone tZone = TimeZone.getTimeZone("Europe/London");
+        Calendar tDate = SunCalcTest.newCalendar(2020, Calendar.MAY, 13, 5, 18, tZone);
+
+        Sun sun = sunCalc.getSunInfo(tDate, 53.524695, -2.4, 0.0, true);
+        assertEquals(SunPhaseName.DAYLIGHT, sun.getPhase().getName());
+    }
+
     /***
      * Constructs a <code>GregorianCalendar</code> with the given date and time set
      * for the provided time zone.

diff --git a/bundles/org.openhab.binding.astro/src/test/java/org/openhab/binding/astro/internal/model/SunTest.java b/bundles/org.openhab.binding.astro/src/test/java/org/openhab/binding/astro/internal/model/SunTest.java
index adc33a911b65e6493617ee7dfea5f67623c8406f..10fefc2b21c3954b2e774ad7ddacb0a5057ba150 100644 (file)
--- a/bundles/org.openhab.binding.astro/src/test/java/org/openhab/binding/astro/internal/model/SunTest.java
+++ b/bundles/org.openhab.binding.astro/src/test/java/org/openhab/binding/astro/internal/model/SunTest.java
@@ -37,7 +37,7 @@ public class SunTest {
     private Sun sun;
     private AstroChannelConfig config;
 
-    private static ZoneId ZONE = ZoneId.systemDefault();
+    private static final ZoneId ZONE = ZoneId.systemDefault();
 
     @BeforeEach
     public void init() {