sch: keep sun .last from updating when .next stays the same

update tests & externalize event hanlding implementation
prevent spurious .last == .next when .next never did or cannot happen

ref. 758ff84, in case update routine is called repeatedly
ref. #2629, 'event' should output coherent values

code/espurna/scheduler.cpp

@@ -236,16 +236,6 @@ bool check_parsed(const Schedule& schedule) {
 #if SCHEDULER_SUN_SUPPORT
 namespace sun {
 
-struct EventMatch : public Event {
-    datetime::Date date;
-    TimeMatch time;
-};
-
-struct Match {
-    EventMatch rising;
-    EventMatch setting;
-};
-
 Location location;
 Match match;
 
@@ -828,59 +818,6 @@ EventMatch* find_event_match(const Schedule& schedule) {
     return find_event_match(schedule.time);
 }
 
-tm make_utc_date_time(datetime::Seconds seconds) {
-    tm out{};
-
-    time_t timestamp{ seconds.count() };
-    gmtime_r(&timestamp, &out);
-
-    return out;
-}
-
-datetime::Date make_date(const tm& date_time) {
-    datetime::Date out;
-
-    out.year = date_time.tm_year + 1900;
-    out.month = date_time.tm_mon + 1;
-    out.day = date_time.tm_mday;
-
-    return out;
-}
-
-TimeMatch make_time_match(const tm& date_time) {
-    TimeMatch out;
-
-    out.hour[date_time.tm_hour] = true;
-    out.minute[date_time.tm_min] = true;
-    out.flags = FlagUtc;
-
-    return out;
-}
-
-void update_event_match(EventMatch& match, datetime::Clock::time_point time_point) {
-    const auto next_valid = event::is_valid(match.next);
-    if (!event::is_valid(time_point)) {
-        if (next_valid) {
-            match.last = match.next;
-        }
-
-        match.next = event::DefaultTimePoint;
-        return;
-    }
-
-    const auto duration = time_point.time_since_epoch();
-
-    const auto date_time = make_utc_date_time(duration);
-    match.date = make_date(date_time);
-    match.time = make_time_match(date_time);
-
-    if (next_valid) {
-        match.last = match.next;
-    }
-
-    match.next = time_point;
-}
-
 void update_schedule_from(Schedule& schedule, const EventMatch& match) {
     schedule.date.day[match.date.day] = true;
     schedule.date.month[match.date.month] = true;

code/espurna/scheduler_sun.ipp

@@ -72,6 +72,75 @@ namespace {
 
 namespace sun {
 
+// Generic event fields plus date & time pair, to be used in scheduler matching code
+struct EventMatch : public Event {
+    datetime::Date date;
+    TimeMatch time;
+};
+
+// Sunrise & Sunset state for the runtime
+struct Match {
+    EventMatch rising;
+    EventMatch setting;
+};
+
+tm make_utc_date_time(datetime::Seconds seconds) {
+    tm out{};
+
+    time_t timestamp{ seconds.count() };
+    gmtime_r(&timestamp, &out);
+
+    return out;
+}
+
+datetime::Date make_date(const tm& date_time) {
+    datetime::Date out;
+
+    out.year = date_time.tm_year + 1900;
+    out.month = date_time.tm_mon + 1;
+    out.day = date_time.tm_mday;
+
+    return out;
+}
+
+TimeMatch make_time_match(const tm& date_time) {
+    TimeMatch out;
+
+    out.hour[date_time.tm_hour] = true;
+    out.minute[date_time.tm_min] = true;
+    out.flags = FlagUtc;
+
+    return out;
+}
+
+void update_event_match(EventMatch& match, datetime::Clock::time_point time_point) {
+    if (match.next == time_point) {
+        return;
+    }
+
+    const auto next_valid = event::is_valid(match.next);
+    if (!event::is_valid(time_point)) {
+        if (next_valid) {
+            match.last = match.next;
+        }
+
+        match.next = event::DefaultTimePoint;
+        return;
+    }
+
+    const auto duration = time_point.time_since_epoch();
+
+    const auto date_time = make_utc_date_time(duration);
+    match.date = make_date(date_time);
+    match.time = make_time_match(date_time);
+
+    if (next_valid) {
+        match.last = match.next;
+    }
+
+    match.next = time_point;
+}
+
 constexpr double Pi { M_PI };
 constexpr double Pi2 { M_PI_2 };
 

code/test/unit/src/scheduler/scheduler.cpp

@@ -1449,6 +1449,81 @@ void test_sun() {
         result.sunset.time_since_epoch().count());
 }
 
+void test_sun_event() {
+    sun::EventMatch match;
+
+    TEST_ASSERT(match.last == event::DefaultTimePoint);
+    TEST_ASSERT(match.next == event::DefaultTimePoint);
+
+    auto last = event::DefaultTimePoint;
+    auto next = datetime::Clock::time_point{ datetime::Seconds(ReferenceTimestamp) };
+
+#define TEST_DATE(X) \
+    TEST_ASSERT_EQUAL(2006, (X).year);\
+    TEST_ASSERT_EQUAL(1, (X).month);\
+    TEST_ASSERT_EQUAL(2, (X).day)
+
+#define TEST_TIME(X, MINUTE) \
+    TEST_ASSERT_EQUAL(1 << 22, (X).hour.to_ullong());\
+    TEST_ASSERT_EQUAL(1 << (MINUTE), (X).minute.to_ullong());\
+    TEST_ASSERT(((X).flags & scheduler::FlagUtc) > 0)
+
+    // Initial input updates .next exactly once and .last stays 'Default'
+
+    for (int times = 0; times < 2; ++times) {
+        sun::update_event_match(match, next);
+
+        TEST_DATE(match.date);
+        TEST_TIME(match.time, 4);
+
+        TEST_ASSERT(last == event::DefaultTimePoint);
+
+        TEST_ASSERT(match.last == event::DefaultTimePoint);
+        TEST_ASSERT(match.next == next);
+    }
+
+    // Different timestamp swaps .next and .last and replaces .next with the input
+    // TODO input & .next are compared for equality, order does not matter
+
+    last = next;
+    next += datetime::Minutes(1);
+
+    sun::update_event_match(match, next);
+
+    TEST_DATE(match.date);
+    TEST_TIME(match.time, 5);
+
+    TEST_ASSERT(match.last == last);
+    TEST_ASSERT(match.next == next);
+
+    // 'Default' input updates .last and resets .next exactly once
+
+    for (int times = 0; times < 2; ++times) {
+        sun::update_event_match(match, event::DefaultTimePoint);
+
+        TEST_DATE(match.date);
+        TEST_TIME(match.time, 5);
+
+        TEST_ASSERT(match.last == next);
+        TEST_ASSERT(match.next == event::DefaultTimePoint);
+    }
+
+    last = next;
+    next += datetime::Minutes(1);
+
+    // Updating from .next = 'Default' preserves existing last
+
+    for (int times = 0; times < 2; ++times) {
+        sun::update_event_match(match, next);
+
+        TEST_DATE(match.date);
+        TEST_TIME(match.time, 6);
+
+        TEST_ASSERT(match.last == last);
+        TEST_ASSERT(match.next == next);
+    }
+}
+
 void test_datetime_parsing() {
     datetime::DateHhMmSs parsed{};
     bool utc { false };
@@ -1543,6 +1618,7 @@ int main(int, char**) {
     RUN_TEST(test_schedule_parsing_weekdays_range);
     RUN_TEST(test_search_bits);
     RUN_TEST(test_sun);
+    RUN_TEST(test_sun_event);
     RUN_TEST(test_time_impl);
     RUN_TEST(test_time_invalid_parsing);
     RUN_TEST(test_time_parsing);