Implement changes suggested during review

- Move implementation to common/time.
- Use camelCase for function names.
- Store just one value for the RTC, the real time when system
was started.
- Store real time as the number of milliseconds from 1970-01-01.
- Remove BB functions for manipulating time, query the RTC
directly when BB is started.

Makefile

@@ -548,6 +548,7 @@ COMMON_SRC = \
             common/maths.c \
             common/printf.c \
             common/streambuf.c \
+            common/time.c \
             common/typeconversion.c \
             common/string_light.c \
             config/config_eeprom.c \
@@ -575,7 +576,6 @@ COMMON_SRC = \
             drivers/pwm_mapping.c \
             drivers/pwm_output.c \
             drivers/rcc.c \
-            drivers/rtc.c \
             drivers/rx_pwm.c \
             drivers/serial.c \
             drivers/serial_uart.c \

src/main/blackbox/blackbox.c

@@ -34,6 +34,7 @@
 #include "common/axis.h"
 #include "common/encoding.h"
 #include "common/maths.h"
+#include "common/time.h"
 #include "common/utils.h"
 
 #include "config/feature.h"
@@ -42,7 +43,6 @@
 
 #include "drivers/accgyro/accgyro.h"
 #include "drivers/compass/compass.h"
-#include "drivers/rtc.h"
 #include "drivers/sensor.h"
 #include "drivers/time.h"
 
@@ -1003,8 +1003,6 @@ void blackboxStart(void)
         return;
     }
 
-    blackboxSaveStartDateTime();
-
     memset(&gpsHistory, 0, sizeof(gpsHistory));
 
     blackboxHistory[0] = &blackboxHistoryRing[0];
@@ -1297,6 +1295,17 @@ static bool sendFieldDefinition(char mainFrameChar, char deltaFrameChar, const v
     return xmitState.headerIndex < headerCount;
 }
 
+// Buf must be at least FORMATTED_DATE_TIME_BUFSIZE
+static char *blackboxGetStartDateTime(char *buf)
+{
+    dateTime_t dt;
+    // rtcGetDateTime will fill dt with 0000-01-01T00:00:00
+    // when time is not known.
+    rtcGetDateTime(&dt);
+    dateTimeFormat(buf, &dt);
+    return buf;
+}
+
 #ifndef BLACKBOX_PRINT_HEADER_LINE
 #define BLACKBOX_PRINT_HEADER_LINE(name, format, ...) case __COUNTER__: \
                                                 blackboxPrintfHeaderLine(name, format, __VA_ARGS__); \
@@ -1317,11 +1326,13 @@ static bool blackboxWriteSysinfo(void)
         return false;
     }
 
+    char buf[FORMATTED_DATE_TIME_BUFSIZE];
+
     switch (xmitState.headerIndex) {
         BLACKBOX_PRINT_HEADER_LINE("Firmware type", "%s",                   "Cleanflight");
         BLACKBOX_PRINT_HEADER_LINE("Firmware revision", "INAV %s (%s) %s",  FC_VERSION_STRING, shortGitRevision, targetName);
         BLACKBOX_PRINT_HEADER_LINE("Firmware date", "%s %s",                buildDate, buildTime);
-        BLACKBOX_PRINT_HEADER_LINE("Log start datetime", "%s",              blackboxGetStartDateTime());
+        BLACKBOX_PRINT_HEADER_LINE("Log start datetime", "%s",              blackboxGetStartDateTime(buf));
         BLACKBOX_PRINT_HEADER_LINE("Craft name", "%s",                      systemConfig()->name);
         BLACKBOX_PRINT_HEADER_LINE("P interval", "%d/%d",                   blackboxConfig()->rate_num, blackboxConfig()->rate_denom);
         BLACKBOX_PRINT_HEADER_LINE("minthrottle", "%d",                     motorConfig()->minthrottle);
@@ -1708,32 +1719,6 @@ void blackboxUpdate(timeUs_t currentTimeUs)
     }
 }
 
-/*
- * Formats start time in ISO 8601 format, YYYY-MM-DDThh:mm:ss
- * Year value of "0000" indicates time not set.
- */
-
-static char started_buf[FORMATTED_DATE_TIME_BUFSIZE];
-const char * blackboxGetStartDateTime()
-{
-    return started_buf;
-}
-
-void blackboxSaveStartDateTime()
-{
-    date_time_t dt;
-    if (!rtc_get_dt(&dt)) {
-        dt.year = 0;
-        dt.month = 1;
-        dt.day = 1;
-        dt.hours = 0;
-        dt.minutes = 0;
-        dt.seconds = 0;
-        dt.nanos = 0;
-    }
-    date_time_format(started_buf, &dt);
-}
-
 static bool canUseBlackboxWithCurrentConfiguration(void)
 {
     return feature(FEATURE_BLACKBOX);

src/main/blackbox/blackbox.h

@@ -34,8 +34,6 @@ void blackboxLogEvent(FlightLogEvent event, flightLogEventData_t *data);
 
 void blackboxInit(void);
 void blackboxUpdate(timeUs_t currentTimeUs);
-const char *blackboxGetStartDateTime(void);
-void blackboxSaveStartDateTime(void);
 void blackboxStart(void);
 void blackboxFinish(void);
 bool blackboxMayEditConfig(void);

src/main/common/time.c

@@ -0,0 +1,120 @@
+#include "common/printf.h"
+#include "common/time.h"
+
+#include "drivers/time.h"
+
+#define UNIX_REFERENCE_YEAR 1970
+
+// rtcTime_t when the system was started.
+// Calculated in rtcSet().
+static rtcTime_t started = 0;
+
+static const uint16_t days[4][12] =
+{
+    {   0,  31,     60,     91,     121,    152,    182,    213,    244,    274,    305,    335},
+    { 366,  397,    425,    456,    486,    517,    547,    578,    609,    639,    670,    700},
+    { 731,  762,    790,    821,    851,    882,    912,    943,    974,    1004,   1035,   1065},
+    {1096,  1127,   1155,   1186,   1216,   1247,   1277,   1308,   1339,   1369,   1400,   1430},
+};
+
+
+static rtcTime_t dateTimeToRtcTime(dateTime_t *dt)
+{
+    unsigned int second = dt->seconds;  // 0-59
+    unsigned int minute = dt->minutes;  // 0-59
+    unsigned int hour = dt->hours;      // 0-23
+    unsigned int day = dt->day - 1;     // 0-30
+    unsigned int month = dt->month - 1; // 0-11
+    unsigned int year = dt->year - UNIX_REFERENCE_YEAR; // 0-99
+    int32_t unix = (((year / 4 * (365 * 4 + 1) + days[year % 4][month] + day) * 24 + hour) * 60 + minute) * 60 + second;
+    return rtcTimeMake(unix, dt->millis);
+}
+
+static void rtcTimeToDateTime(dateTime_t *dt, rtcTime_t *t)
+{
+    int32_t unix = *t / 1000;
+    dt->seconds = unix % 60;
+    unix /= 60;
+    dt->minutes = unix % 60;
+    unix /= 60;
+    dt->hours = unix % 24;
+    unix /= 24;
+
+    unsigned int years = unix / (365 * 4 + 1) * 4;
+    unix %= 365 * 4 + 1;
+
+    unsigned int year;
+    for (year = 3; year > 0; year--) {
+        if (unix >= days[year][0]) {
+            break;
+        }
+    }
+
+    unsigned int month;
+    for (month = 11; month > 0; month--) {
+        if (unix >= days[year][month]) {
+            break;
+        }
+    }
+
+    dt->year = years + year + UNIX_REFERENCE_YEAR;
+    dt->month = month + 1;
+    dt->day = unix - days[year][month] + 1;
+    dt->millis = *t % 1000;
+}
+
+rtcTime_t rtcTimeMake(int32_t secs, uint16_t millis)
+{
+    return ((rtcTime_t)secs) * 1000 + millis;
+}
+
+void dateTimeFormat(char *buf, dateTime_t *dt)
+{
+    tfp_sprintf(buf, "%04u-%02u-%02uT%02u:%02u:%02u.%03d",
+        dt->year, dt->month, dt->day,
+        dt->hours, dt->minutes, dt->seconds, dt->millis);
+}
+
+bool rtcHasTime()
+{
+    return started != 0;
+}
+
+bool rtcGet(rtcTime_t *t)
+{
+    if (!rtcHasTime()) {
+        return false;
+    }
+    *t = started + millis();
+    return true;
+}
+
+bool rtcSet(rtcTime_t *t)
+{
+    started = *t - millis();
+    return true;
+}
+
+bool rtcGetDateTime(dateTime_t *dt)
+{
+    rtcTime_t t;
+    if (rtcGet(&t)) {
+        rtcTimeToDateTime(dt, &t);
+        return true;
+    }
+    // No time stored, fill dt with 0000-01-01T00:00:00.000
+    dt->year = 0;
+    dt->month = 1;
+    dt->day = 1;
+    dt->hours = 0;
+    dt->minutes = 0;
+    dt->seconds = 0;
+    dt->millis = 0;
+    return false;
+}
+
+bool rtcSetDateTime(dateTime_t *dt)
+{
+    rtcTime_t t = dateTimeToRtcTime(dt);
+    return rtcSet(&t);
+}

src/main/common/time.h

@@ -17,6 +17,7 @@
 
 #pragma once
 
+#include <stdbool.h>
 #include <stdint.h>
 
 #include "platform.h"
@@ -35,3 +36,38 @@ typedef uint32_t timeUs_t;
 #endif
 
 static inline timeDelta_t cmpTimeUs(timeUs_t a, timeUs_t b) { return (timeDelta_t)(a - b); }
+
+// Milliseconds since Jan 1 1970
+typedef int64_t rtcTime_t;
+
+rtcTime_t rtcTimeMake(int32_t secs, uint16_t millis);
+
+typedef struct _dateTime_s {
+    // full year
+    uint16_t year;
+    // 1-12
+    uint8_t month;
+    // 1-31
+    uint8_t day;
+    // 0-23
+    uint8_t hours;
+    // 0-59
+    uint8_t minutes;
+    // 0-59
+    uint8_t seconds;
+    // 0-999
+    uint16_t millis;
+} dateTime_t;
+
+#define FORMATTED_DATE_TIME_BUFSIZE 24
+
+// buf must be at least FORMATTED_DATE_TIME_BUFSIZE
+void dateTimeFormat(char *buf, dateTime_t *dt);
+
+bool rtcHasTime();
+
+bool rtcGet(rtcTime_t *t);
+bool rtcSet(rtcTime_t *t);
+
+bool rtcGetDateTime(dateTime_t *dt);
+bool rtcSetDateTime(dateTime_t *dt);

src/main/drivers/rtc.c

@@ -1,136 +0,0 @@
-#include "common/printf.h"
-
-#include "drivers/rtc.h"
-#include "drivers/time.h"
-
-#define UNIX_REFERENCE_YEAR 1970
-#define NANOS_PER_SECOND 1000000000
-
-// millis() when rtc_set() was called
-static timeMs_t rtcMillisSet = 0;
-// timestamp_t set via set_rtc_ts(), used
-// as an offset to count from.
-static timestamp_t rtcTsSet = {0,};
-
-timestamp_t timestamp_unix(int32_t unix, uint32_t nanos)
-{
-    while (nanos >= NANOS_PER_SECOND) {
-        nanos -= NANOS_PER_SECOND;
-        unix++;
-    }
-    return (timestamp_t){ .unix = unix, nanos = nanos};
-}
-
-int32_t timestamp_get_unix(timestamp_t ts)
-{
-    return ts.unix;
-}
-
-uint32_t timestamp_get_nanos(timestamp_t ts)
-{
-    return ts.nanos;
-}
-
-static timestamp_t timestamp_add_millis(timestamp_t ts, timeMs_t ms) {
-    uint32_t seconds = ms / 1000;
-    uint32_t rem = ms % 1000;
-    return timestamp_unix(timestamp_get_unix(ts) + seconds, timestamp_get_nanos(ts) + rem * 1000000);
-}
-
-static uint16_t days[4][12] =
-{
-    {   0,  31,  60,  91, 121, 152, 182, 213, 244, 274, 305, 335},
-    { 366, 397, 425, 456, 486, 517, 547, 578, 609, 639, 670, 700},
-    { 731, 762, 790, 821, 851, 882, 912, 943, 974,1004,1035,1065},
-    {1096,1127,1155,1186,1216,1247,1277,1308,1339,1369,1400,1430},
-};
-
-
-static timestamp_t date_time_to_timestamp(date_time_t *dt)
-{
-    unsigned int second = dt->seconds;  // 0-59
-    unsigned int minute = dt->minutes;  // 0-59
-    unsigned int hour = dt->hours;      // 0-23
-    unsigned int day = dt->day - 1;     // 0-30
-    unsigned int month = dt->month - 1; // 0-11
-    unsigned int year = dt->year - UNIX_REFERENCE_YEAR; // 0-99
-    int32_t unix = (((year/4*(365*4+1)+days[year%4][month]+day)*24+hour)*60+minute)*60+second;
-    return timestamp_unix(unix, dt->nanos);
-}
-
-static void timestamp_to_date_time(date_time_t *dt, timestamp_t ts)
-{
-    int32_t unix = timestamp_get_unix(ts);
-    dt->seconds = unix%60;
-    unix /= 60;
-    dt->minutes = unix%60;
-    unix /= 60;
-    dt->hours = unix%24;
-    unix /= 24;
-
-    unsigned int years = unix/(365*4+1)*4;
-    unix %= 365*4+1;
-
-    unsigned int year;
-    for (year=3; year>0; year--) {
-        if (unix >= days[year][0]) {
-            break;
-        }
-    }
-
-    unsigned int month;
-    for (month = 11; month > 0; month--) {
-        if (unix >= days[year][month]) {
-            break;
-        }
-    }
-
-    dt->year = years + year + UNIX_REFERENCE_YEAR;
-    dt->month = month + 1;
-    dt->day = unix - days[year][month] + 1;
-    dt->nanos = timestamp_get_nanos(ts);
-}
-
-void date_time_format(char *buf, date_time_t *dt)
-{
-    tfp_sprintf(buf, "%04u-%02u-%02uT%02u:%02u:%02u.%09d",
-        dt->year, dt->month, dt->day,
-        dt->hours, dt->minutes, dt->seconds, dt->nanos);
-}
-
-bool rtc_has_time()
-{
-    return rtcMillisSet > 0;
-}
-
-bool rtc_get(timestamp_t *ts)
-{
-    if (!rtc_has_time()) {
-        return false;
-    }
-    timeMs_t delta = millis() - rtcMillisSet;
-    *ts = timestamp_add_millis(rtcTsSet, delta);
-    return true;
-}
-
-bool rtc_set(timestamp_t ts)
-{
-    rtcMillisSet = millis();
-    rtcTsSet = ts;
-    return true;
-}
-
-bool rtc_get_dt(date_time_t *dt)
-{
-    timestamp_t ts;
-    if (rtc_get(&ts)) {
-        timestamp_to_date_time(dt, ts);
-        return true;
-    }
-    return false;
-}
-
-bool rtc_set_dt(date_time_t *dt)
-{
-    return rtc_set(date_time_to_timestamp(dt));
-}

src/main/drivers/rtc.h

@@ -1,44 +0,0 @@
-#pragma once
-
-#include <stdbool.h>
-#include <stdint.h>
-
-// Don't access the fields directly. Use the getter
-// functions below.
-typedef struct {
-    int32_t unix;
-    uint32_t nanos;
-} timestamp_t;
-
-timestamp_t timestamp_unix(int32_t unix, uint32_t nanos);
-int32_t timestamp_get_unix(timestamp_t ts);
-uint32_t timestamp_get_nanos(timestamp_t ts);
-
-typedef struct _date_time_s {
-    // full year
-    uint16_t year;
-    // 1-12
-    uint8_t month;
-    // 1-31
-    uint8_t day;
-    // 0-23
-    uint8_t hours;
-    // 0-59
-    uint8_t minutes;
-    // 0-59
-    uint8_t seconds;
-    uint32_t nanos;
-} date_time_t;
-
-#define FORMATTED_DATE_TIME_BUFSIZE 30
-
-// buf must be at least FORMATTED_DATE_TIME_BUFSIZE
-void date_time_format(char *buf, date_time_t *dt);
-
-bool rtc_has_time();
-
-bool rtc_get(timestamp_t *ts);
-bool rtc_set(timestamp_t ts);
-
-bool rtc_get_dt(date_time_t *dt);
-bool rtc_set_dt(date_time_t *dt);

src/main/fc/cli.c

@@ -43,8 +43,9 @@ extern uint8_t __config_end;
 #include "common/color.h"
 #include "common/maths.h"
 #include "common/printf.h"
-#include "common/typeconversion.h"
 #include "common/string_light.h"
+#include "common/time.h"
+#include "common/typeconversion.h"
 
 #include "config/config_eeprom.h"
 #include "config/feature.h"
@@ -58,7 +59,6 @@ extern uint8_t __config_end;
 #include "drivers/io.h"
 #include "drivers/io_impl.h"
 #include "drivers/logging.h"
-#include "drivers/rtc.h"
 #include "drivers/rx_pwm.h"
 #include "drivers/sdcard.h"
 #include "drivers/sensor.h"
@@ -2321,17 +2321,13 @@ static void cliStatus(char *cmdline)
 {
     UNUSED(cmdline);
 
+    char buf[FORMATTED_DATE_TIME_BUFSIZE];
+    dateTime_t dt;
+
     cliPrintLinef("System Uptime: %d seconds", millis() / 1000);
-    cliPrint("Current Time: ");
+    rtcGetDateTime(&dt);
-    date_time_t dt;
+    dateTimeFormat(buf, &dt);
-    if (rtc_get_dt(&dt)) {
+    cliPrintLinef("Current Time: %s", buf);
-        char buf[FORMATTED_DATE_TIME_BUFSIZE];
-        date_time_format(buf, &dt);
-        cliPrint(buf);
-    } else {
-        cliPrint("unknown");
-    }
-    cliPrintLinefeed();
     cliPrintLinef("Voltage: %d * 0.1V (%dS battery - %s)", vbat, batteryCellCount, getBatteryStateString());
     cliPrintf("CPU Clock=%dMHz", (SystemCoreClock / 1000000));
 

src/main/fc/fc_msp.c

@@ -32,6 +32,7 @@
 #include "common/maths.h"
 #include "common/streambuf.h"
 #include "common/bitarray.h"
+#include "common/time.h"
 #include "common/utils.h"
 
 #include "drivers/accgyro/accgyro.h"
@@ -39,7 +40,6 @@
 #include "drivers/compass/compass.h"
 #include "drivers/max7456.h"
 #include "drivers/pwm_mapping.h"
-#include "drivers/rtc.h"
 #include "drivers/sdcard.h"
 #include "drivers/serial.h"
 #include "drivers/system.h"
@@ -2116,9 +2116,13 @@ static mspResult_e mspFcProcessInCommand(uint8_t cmdMSP, sbuf_t *src)
 
     case MSP_SET_RTC:
         {
-            int32_t unix = (int32_t)sbufReadU32(src);
+            // Use seconds and milliseconds to make senders
-            uint32_t nanos = sbufReadU32(src);
+            // easier to implement. Generating a 64 bit value
-            rtc_set(timestamp_unix(unix, nanos));
+            // might not be trivial in some platforms.
+            int32_t secs = (int32_t)sbufReadU32(src);
+            uint16_t millis = sbufReadU16(src);
+            rtcTime_t t = rtcTimeMake(secs, millis);
+            rtcSet(&t);
         }
         break;
 

src/main/io/gps.c

@@ -148,8 +148,8 @@ void gpsSetState(gpsState_e state)
 
 static void gpsUpdateTime(void)
 {
-    if (!rtc_has_time() && gpsSol.flags.validTime) {
+    if (!rtcHasTime() && gpsSol.flags.validTime) {
-        rtc_set_dt(&gpsSol.time);
+        rtcSetDateTime(&gpsSol.time);
     }
 }
 

src/main/io/gps.h

@@ -21,7 +21,7 @@
 
 #include "config/parameter_group.h"
 
-#include "drivers/rtc.h"
+#include "common/time.h"
 
 #define GPS_DBHZ_MIN 0
 #define GPS_DBHZ_MAX 55
@@ -133,7 +133,7 @@ typedef struct gpsSolutionData_s {
 
     uint16_t hdop;  // generic HDOP value (*100)
 
-    date_time_t time; // Time in UTC
+    dateTime_t time; // GPS time in UTC
 
 } gpsSolutionData_t;
 

src/main/io/gps_ublox.c

@@ -489,7 +489,7 @@ static bool gpsParceFrameUBLOX(void)
             gpsSol.time.hours = _buffer.pvt.hour;
             gpsSol.time.minutes = _buffer.pvt.min;
             gpsSol.time.seconds = _buffer.pvt.sec;
-            gpsSol.time.nanos = _buffer.pvt.nano;
+            gpsSol.time.millis = _buffer.pvt.nano / (1000*1000);
 
             gpsSol.flags.validTime = 1;
         } else {

src/main/msp/msp_protocol.h

@@ -319,7 +319,7 @@
 #define MSP_SERVO_MIX_RULES      241    //out message         Returns servo mixer configuration
 #define MSP_SET_SERVO_MIX_RULE   242    //in message          Sets servo mixer configuration
 #define MSP_SET_4WAY_IF          245    //in message          Sets 4way interface
-#define MSP_SET_RTC              246    //in message          Sets the RTC clock - TODO: Make sure this code is not used by other forks
+#define MSP_SET_RTC              246    //in message          Sets the RTC clock (args: secs(i32) millis(u16))
 
 // MSPv2 includes
 #include "msp_protocol_v2_inav.h"