Remove local_time from ActiveTextLogger. Fixes: #2815 (#3104)

* ActiveTextLogger: make tests runnable * ActiveTextLogger: fix unit tests * ActiveTextLogger: use camelCase for test names * ActiveTextLogger: add test for workstation timestamp * ActiveTextLogger: remove logging of local workstation time (issue #2815) * ActiveTextLogger: mention file truncation in the SDD * Changing PRId32 to PRIu32 for time fields. * Assert of string compare to ASSERT_STREQ --------- Co-authored-by: M Starch <[email protected]>
nasa · Jan 7, 2025 · f5b1bb0 · f5b1bb0
1 parent 02a351c
commit f5b1bb0
Show file tree

Hide file tree

Showing 7 changed files with 83 additions and 38 deletions.
diff --git a/Svc/ActiveTextLogger/ActiveTextLogger.cpp b/Svc/ActiveTextLogger/ActiveTextLogger.cpp
@@ -76,31 +76,11 @@ namespace Svc {
         // TODO: Add calling task id to format string
         char textStr[FW_INTERNAL_INTERFACE_STRING_MAX_SIZE];
 
-        if (timeTag.getTimeBase() == TB_WORKSTATION_TIME) {
-
-            time_t t = timeTag.getSeconds();
-            // Using localtime_r prevents any other calls to localtime (from another thread for example) from
-            // interfering with our time object before we use it. However, the null pointer check is still needed
-            // to ensure a successful call
-            tm tm;
-            if (localtime_r(&t, &tm) == nullptr) {
-                return;
-            }
-
-            (void) snprintf(textStr,
-                            FW_INTERNAL_INTERFACE_STRING_MAX_SIZE,
-                            "EVENT: (%" PRI_FwEventIdType ") (%04d-%02d-%02dT%02d:%02d:%02d.%06" PRIu32 ") %s: %s\n",
-                            id, tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday, tm.tm_hour,
-                            tm.tm_min,tm.tm_sec,timeTag.getUSeconds(),
-                            severityString,text.toChar());
-        }
-        else {
-
-            (void) snprintf(textStr,
-                            FW_INTERNAL_INTERFACE_STRING_MAX_SIZE,
-                            "EVENT: (%" PRI_FwEventIdType ") (%" PRI_FwTimeBaseStoreType ":%" PRId32 ",%" PRId32 ") %s: %s\n",
-                            id, static_cast<FwTimeBaseStoreType>(timeTag.getTimeBase()),timeTag.getSeconds(),timeTag.getUSeconds(),severityString,text.toChar());
-        }
+        (void) snprintf(textStr,
+                        FW_INTERNAL_INTERFACE_STRING_MAX_SIZE,
+                        "EVENT: (%" PRI_FwEventIdType ") (%" PRI_FwTimeBaseStoreType ":%" PRIu32 ",%" PRIu32 ") %s: %s\n",
+                        id, static_cast<FwTimeBaseStoreType>(timeTag.getTimeBase()), timeTag.getSeconds(), timeTag.getUSeconds(),
+                        severityString, text.toChar());
 
         // Call internal interface so that everything else is done on component thread,
         // this helps ensure consistent ordering of the printed text:

diff --git a/Svc/ActiveTextLogger/CMakeLists.txt b/Svc/ActiveTextLogger/CMakeLists.txt
@@ -17,6 +17,6 @@ register_fprime_module()
 set(UT_SOURCE_FILES
   "${FPRIME_FRAMEWORK_PATH}/Svc/ActiveTextLogger/ActiveTextLogger.fpp"
   "${CMAKE_CURRENT_LIST_DIR}/test/ut/ActiveTextLoggerTester.cpp"
-  "${CMAKE_CURRENT_LIST_DIR}/test/ut/ActiveTextLoggerTester.cpp"
+  "${CMAKE_CURRENT_LIST_DIR}/test/ut/ActiveTextLoggerTestMain.cpp"
 )
 register_fprime_ut()
diff --git a/Svc/ActiveTextLogger/LogFile.cpp b/Svc/ActiveTextLogger/LogFile.cpp
@@ -140,7 +140,7 @@ namespace Svc {
         }
 
         // Open the file (using CREATE so that it truncates an already existing file):
-        Os::File::Status stat = this->m_file.open(fileNameFinal, Os::File::OPEN_CREATE, Os::File::OverwriteType::NO_OVERWRITE);
+        Os::File::Status stat = this->m_file.open(fileNameFinal, Os::File::OPEN_CREATE, Os::File::OverwriteType::OVERWRITE);
 
         // Bad status when trying to open the file:
         if (stat != Os::File::OP_OK) {

diff --git a/Svc/ActiveTextLogger/docs/sdd.md b/Svc/ActiveTextLogger/docs/sdd.md
@@ -16,7 +16,7 @@ ISF-ATL-002 | The `Svc::ActiveTextLogger` component shall write received log tex
 ISF-ATL-003 | The `Svc::ActiveTextLogger` component shall format the passed log text on the calling thread context, and perform all other processing on the component's thread. | Inspection
 ISF-ATL-004 | The `Svc::ActiveTextLogger` component shall stop writing to the optional file if it would exceed its max size. | Unit Test
 ISF-ATL-005 | The `Svc::ActiveTextLogger` component shall provide a public method to supply the filename to write to and max size. | Unit Test
-ISF-ATL-006 | The `Svc::ActiveTextLogger` component shall attempt to create a new file to write to if the supplied one already exists.  It will try up to ten times, by adding an integer suffix to the filename, ie "file","file0","file1"..."file9" | Unit Test
+ISF-ATL-006 | The `Svc::ActiveTextLogger` component shall attempt to create a new file to write to if the supplied one already exists.  It will try up to ten times, by adding an integer suffix to the filename, ie "file","file0","file1"..."file9". After the last possible attempt is failed, the initial file shall be overwritten; ie if "file9" already exists, the "file" shall be truncated and used as a log file. | Unit Test
 
 
 ## 3. Design
@@ -45,7 +45,7 @@ The `Svc::ActiveTextLogger` component provides a text logging function for the s
 
 Once a valid file and max size is supplied via a public function call, the `Svc::ActiveTextLogger` component writes to that file as well as standard output.  The `Svc::ActiveTextLogger` component will stop writing to the file if it would exceed the maximum size.
 
-If the file supplied already exists, the `Svc::ActiveTextLogger` component will attempt to create a new file up to ten times by appending an integer suffix to end of the file name.
+If the file supplied already exists, the `Svc::ActiveTextLogger` component will attempt to create a new file up to ten times by appending an integer suffix to end of the file name. After the last possible attempt is failed, the initial file is overwritten.
 
 ### 3.3 Scenarios
 

diff --git a/Svc/ActiveTextLogger/test/ut/ActiveTextLoggerTestMain.cpp b/Svc/ActiveTextLogger/test/ut/ActiveTextLoggerTestMain.cpp
@@ -0,0 +1,28 @@
+// ======================================================================
+// \title  ActiveTextLoggerTestMain.cpp
+// \author AlesKus
+// \brief  cpp file for ActiveTextLogger component test main function
+// ======================================================================
+
+#include "ActiveTextLoggerTester.hpp"
+
+
+TEST(Nominal, Logging) {
+    Svc::ActiveTextLoggerTester tester;
+    tester.runNominalTest();
+}
+
+TEST(Nominal, WorkstationTime) {
+    Svc::ActiveTextLoggerTester tester;
+    tester.testWorkstationTimestamp();
+}
+
+TEST(OffNominal, FileHandling) {
+    Svc::ActiveTextLoggerTester tester;
+    tester.runOffNominalTest();
+}
+
+int main(int argc, char** argv) {
+  ::testing::InitGoogleTest(&argc, argv);
+  return RUN_ALL_TESTS();
+}
diff --git a/Svc/ActiveTextLogger/test/ut/ActiveTextLoggerTester.cpp b/Svc/ActiveTextLogger/test/ut/ActiveTextLoggerTester.cpp
@@ -38,7 +38,7 @@ namespace Svc {
   // ----------------------------------------------------------------------
 
   void ActiveTextLoggerTester ::
-  run_nominal_test()
+  runNominalTest()
   {
       printf("Testing writing to console\n");
 
@@ -106,7 +106,7 @@ namespace Svc {
               snprintf(textStr, sizeof(textStr),
                       "EVENT: (%d) (%d:%d,%d) %s: %s",
                        id,timeTag.getTimeBase(),timeTag.getSeconds(),timeTag.getUSeconds(),severityString,text.toChar());
-              ASSERT_EQ(0,strcmp(textStr,buf));
+              ASSERT_STREQ(textStr,buf);
               (void) Fw::StringUtils::string_copy(oldLine, buf, static_cast<FwSizeType>(sizeof(oldLine)));
           }
       }
@@ -160,7 +160,7 @@ namespace Svc {
   }
 
   void ActiveTextLoggerTester ::
-  run_off_nominal_test()
+  runOffNominalTest()
   {
       // TODO file errors- use the Os/Stubs?
 
@@ -327,7 +327,6 @@ namespace Svc {
       // Create 11th which will fail and re-use the original:
       stat = this->component.set_log_file(baseName,50);
 
-      snprintf(baseNameWithSuffix, sizeof(baseNameWithSuffix), "%s%d",baseName,i);
       ASSERT_TRUE(stat);
       ASSERT_TRUE(this->component.m_log_file.m_openFile);
       printf("<< %s %s\n",baseName,this->component.m_log_file.m_fileName.toChar());
@@ -347,6 +346,46 @@ namespace Svc {
 
   }
 
+  void ActiveTextLoggerTester ::
+    testWorkstationTimestamp()
+  {
+      printf("Testing workstation timestamp\n");
+
+      // Setup file for writing to:
+      const char* logFileName = "test_file";
+      bool stat = this->component.set_log_file(logFileName,512);
+      ASSERT_TRUE(stat);
+      ASSERT_TRUE(this->component.m_log_file.m_openFile);
+
+      // Log message:
+      FwEventIdType id = 1;
+      Fw::Time timeTag(TB_WORKSTATION_TIME, 3, 6);
+      Fw::LogSeverity severity = Fw::LogSeverity::ACTIVITY_HI;
+      const char* severityString = "ACTIVITY_HI";
+      Fw::TextLogString text("This line has a valid timestamp.");
+      this->invoke_to_TextLogger(0, id, timeTag, severity, text);
+      this->component.doDispatch();
+
+      // Read file to verify contents:
+      std::ifstream logStream(logFileName);
+      while(logStream) {
+          char buf[256];
+          logStream.getline(buf, 256);
+          if (logStream) {
+              std::cout << "readLine: " << buf << std::endl;
+              char textStr[512];
+              snprintf(textStr, sizeof(textStr),
+                      "EVENT: (%d) (%d:%d,%d) %s: %s",
+                       id,timeTag.getTimeBase(), timeTag.getSeconds(), timeTag.getUSeconds(), severityString, text.toChar());
+              ASSERT_EQ(0, strcmp(textStr, buf));
+          }
+      }
+      logStream.close();
+
+      // Clean up:
+      remove(logFileName);
+  }
+
   // ----------------------------------------------------------------------
   // Helper methods
   // ----------------------------------------------------------------------
@@ -361,9 +400,6 @@ namespace Svc {
         this->component.get_TextLogger_InputPort(0)
     );
 
-
-
-
   }
 
   void ActiveTextLoggerTester ::

diff --git a/Svc/ActiveTextLogger/test/ut/ActiveTextLoggerTester.hpp b/Svc/ActiveTextLogger/test/ut/ActiveTextLoggerTester.hpp
@@ -35,8 +35,9 @@ namespace Svc {
       // Tests
       // ----------------------------------------------------------------------
 
-      void run_nominal_test();
-      void run_off_nominal_test();
+      void runNominalTest();
+      void runOffNominalTest();
+      void testWorkstationTimestamp();
 
     private: