Most feedback covered

Author: nateinaction

FprimeZephyrReference/Components/Drv/Rv3028Manager/Rv3028Manager.cpp

@@ -25,20 +25,20 @@ Rv3028Manager ::~Rv3028Manager() {}
 void Rv3028Manager ::timeGetPort_handler(FwIndexType portNum, Fw::Time& time) {
     // Check device readiness
     if (!device_is_ready(this->rv3028)) {
-        Fw::Logger::log("Rv2038 not ready");
+        // Use logger instead of events since this fn is in a critical path for FPrime
+        // to get time. Events require time, if this method fails an event will fail.
+        Fw::Logger::log("RV2038 not ready");
         return;
     }
 
-    // Fetch time from RTC
-    struct rtc_time time_rtc = {};
-    rtc_get_time(this->rv3028, &time_rtc);
-
-    // Convert time to POSIX time_t format
-    struct tm* time_tm = rtc_time_to_tm(&time_rtc);
-    time_t time_posix = timeutil_timegm(time_tm);
+    I32 posix_time = this->getPosixTime();
+    if (posix_time < 0) {
+        Fw::Logger::log("getPosixTime returned invalid time");
+        return;
+    }
 
     // Set FPrime time object
-    time.set(TimeBase::TB_WORKSTATION_TIME, 0, static_cast<U32>(time_posix), 0);
+    time.set(TimeBase::TB_WORKSTATION_TIME, 0, static_cast<U32>(posix_time), 0);
 }
 
 U32 Rv3028Manager ::timeRead_handler(FwIndexType portNum) {
@@ -47,16 +47,16 @@ U32 Rv3028Manager ::timeRead_handler(FwIndexType portNum) {
         this->log_WARNING_HI_DeviceNotReady();
         return 0;
     }
+    this->log_WARNING_HI_DeviceNotReady_ThrottleClear();
 
-    // Fetch time from RTC
-    struct rtc_time time_rtc = {};
-    rtc_get_time(this->rv3028, &time_rtc);
-
-    // Convert time to POSIX time_t format
-    struct tm* time_tm = rtc_time_to_tm(&time_rtc);
-    time_t time_posix = timeutil_timegm(time_tm);
+    I32 posix_time = this->getPosixTime();
+    if (posix_time < 0) {
+        this->log_WARNING_HI_InvalidTime();
+        return 0;
+    }
+    this->log_WARNING_HI_InvalidTime_ThrottleClear();
 
-    return time_posix;
+    return static_cast<U32>(posix_time);
 }
 
 void Rv3028Manager ::timeSet_handler(FwIndexType portNum, const Drv::TimeData& t) {
@@ -65,20 +65,24 @@ void Rv3028Manager ::timeSet_handler(FwIndexType portNum, const Drv::TimeData& t
         this->log_WARNING_HI_DeviceNotReady();
         return;
     }
+    this->log_WARNING_HI_DeviceNotReady_ThrottleClear();
 
     // Populate rtc_time structure from TimeData
     const struct rtc_time time_rtc = {
         .tm_sec = 0,
-        .tm_min = t.get_Minute(),
-        .tm_hour = t.get_Hour(),
-        .tm_mday = t.get_Day(),
-        .tm_mon = t.get_Month() - 1,     // month [0-11]
-        .tm_year = t.get_Year() - 1900,  // year since 1900
+        .tm_min = static_cast<int>(t.get_Minute()),
+        .tm_hour = static_cast<int>(t.get_Hour()),
+        .tm_mday = static_cast<int>(t.get_Day()),
+        .tm_mon = static_cast<int>(t.get_Month() - 1),     // month [0-11]
+        .tm_year = static_cast<int>(t.get_Year() - 1900),  // year since 1900
         .tm_wday = 0,
         .tm_yday = 0,
         .tm_isdst = 0,
     };
 
+    // Event current time to correlate prior events when new time is set
+    this->log_ACTIVITY_HI_TimeGet(this->getPosixTime());
+
     // Set time on RTC
     const int status = rtc_set_time(this->rv3028, &time_rtc);
 
@@ -90,4 +94,24 @@ void Rv3028Manager ::timeSet_handler(FwIndexType portNum, const Drv::TimeData& t
     }
 }
 
+// ----------------------------------------------------------------------
+// Private helper methods
+// ----------------------------------------------------------------------
+I32 Rv3028Manager ::getPosixTime() {
+    struct rtc_time time_rtc = {};
+    rtc_get_time(this->rv3028, &time_rtc);
+
+    // Convert time to POSIX time_t format
+    struct tm* time_tm = rtc_time_to_tm(&time_rtc);
+
+    errno = 0;
+    time_t posix_time = timeutil_timegm(time_tm);
+    if (errno == ERANGE) {
+        Fw::Logger::log("RV2038 returned invalid time");
+        return -1;
+    }
+
+    return posix_time;
+}
+
 }  // namespace Drv

FprimeZephyrReference/Components/Drv/Rv3028Manager/Rv3028Manager.fpp

@@ -33,13 +33,21 @@ module Drv {
         ##############################################################################
 
         @ DeviceNotReady event indicates that the RV3028 is not ready
-        event DeviceNotReady() severity warning high id 0 format "RV3028 not ready"
+        event DeviceNotReady() severity warning high id 0 format "RV3028 not ready" throttle 5
+
+        @ InvalidTime event indicates that the RV3028 returned an invalid time
+        event InvalidTime() severity warning high id 1 format "RV3028 returned invalid time" throttle 5
+
+        @ TimeGet event indicates that the time was read successfully
+        event TimeGet(
+            t: U32 @< POSIX time read from RTC
+        ) severity activity high id 2 format "RV3028 Time: {}"
 
         @ TimeSet event indicates that the time was set successfully
-        event TimeSet() severity activity high id 1 format "Time set on RV3028"
+        event TimeSet() severity activity high id 3 format "Time set on RV3028"
 
         @ TimeNotSet event indicates that the time was not set successfully
-        event TimeNotSet() severity warning high id 2 format "Time not set on RV3028"
+        event TimeNotSet() severity warning high id 4 format "Time not set on RV3028"
 
         ###############################################################################
         # Standard AC Ports: Required for Channels, Events, Commands, and Parameters  #

FprimeZephyrReference/Components/Drv/Rv3028Manager/Rv3028Manager.hpp

@@ -7,6 +7,9 @@
 #define Components_Rv3028Manager_HPP
 
 #include "FprimeZephyrReference/Components/Drv/Rv3028Manager/Rv3028ManagerComponentAc.hpp"
+
+#include <cerrno>
+
 #include <Fw/Logger/Logger.hpp>
 
 #include <zephyr/device.h>
@@ -56,6 +59,13 @@ class Rv3028Manager final : public Rv3028ManagerComponentBase {
     void timeSet_handler(FwIndexType portNum,  //!< The port number
                          const Drv::TimeData& time) override;
 
+    // ----------------------------------------------------------------------
+    // Private helper methods
+    // ----------------------------------------------------------------------
+
+    //! Helper method to get POSIX time from RTC
+    I32 getPosixTime();
+
     //! Zephyr device stores the initialized RV2038 sensor
     const struct device* rv3028;
 };

FprimeZephyrReference/Components/RtcManager/docs/sdd.md

@@ -66,9 +66,9 @@ classDiagram
 sequenceDiagram
     participant Ground Station
     participant RtcManager
-    participant RV3028 Driver
+    participant RV3028 Manager
     Ground Station-->>RtcManager: Send SET_TIME command with time data
-    RtcManager->>RV3028 Driver: Call timeSet port with time data
+    RtcManager->>RV3028 Manager: Call timeSet port with time data
 ```
 
 ### `GET_TIME` Command
@@ -78,10 +78,10 @@ sequenceDiagram
     participant Ground Station
     participant Event Log
     participant RtcManager
-    participant RV3028 Driver
+    participant RV3028 Manager
     Ground Station-->>RtcManager: Send GET_TIME command
-    RtcManager->>RV3028 Driver: Call timeRead port
-    RV3028 Driver->>RtcManager: Return time data
+    RtcManager->>RV3028 Manager: Call timeRead port
+    RV3028 Manager->>RtcManager: Return time data
     RtcManager->>Event Log: Emit GetTime event with retrieved time
 ```