Fixing review comments

Drv/Ip/test/ut/SocketTestHelper.cpp

@@ -10,6 +10,7 @@
 #include <unistd.h>
 #include <cerrno>
 #include <arpa/inet.h>
+#include <IpCfg.hpp>
 
 namespace Drv {
 namespace Test {
@@ -80,5 +81,11 @@ bool wait_on_started(Drv::IpSocket &socket, bool open, U32 iterations) {
     return false;
 }
 
+U64 get_configured_delay_ms() {
+    printf("%llu %llu\n", static_cast<U64>(SOCKET_RETRY_INTERVAL.getSeconds()), static_cast<U64>(SOCKET_RETRY_INTERVAL.getUSeconds()));
+    return (static_cast<U64>(SOCKET_RETRY_INTERVAL.getSeconds()) * 1000) +
+           (static_cast<U64>(SOCKET_RETRY_INTERVAL.getUSeconds()) / 1000);
+}
+
 };
 };

Drv/Ip/test/ut/SocketTestHelper.hpp

@@ -65,6 +65,12 @@ bool wait_on_change(Drv::IpSocket &socket, bool open, U32 iterations);
 */
 bool wait_on_started(Drv::IpSocket &socket, bool open, U32 iterations);
 
+/**
+ * Get the configured delay, converted to milliseconds
+ * @return SOCKET_RETRY_INTERVAL converted to milliseconds
+ */
+U64 get_configured_delay_ms();
+
 };
 };
 #endif

Drv/LinuxGpioDriver/LinuxGpioDriverComponentImpl.hpp

@@ -46,8 +46,8 @@ namespace Drv {
 
       //! Start interrupt task
       Os::Task::Status startIntTask(Os::Task::ParamType priority = Os::Task::TASK_DEFAULT,
-                                        Os::Task::ParamType stackSize = Os::Task::TASK_DEFAULT,
-                                        Os::Task::ParamType cpuAffinity = Os::Task::TASK_DEFAULT);
+                                    Os::Task::ParamType stackSize = Os::Task::TASK_DEFAULT,
+                                    Os::Task::ParamType cpuAffinity = Os::Task::TASK_DEFAULT);
 
       //! configure GPIO
       enum GpioDirection {

Drv/TcpClient/test/ut/TcpClientTester.cpp

@@ -16,7 +16,6 @@
 #include <Drv/Ip/test/ut/SocketTestHelper.hpp>
 
 Os::Log logger;
-const U64 SOCKET_RETRY_INTERVAL_MS = (SOCKET_RETRY_INTERVAL.getSeconds() * 1000) + (SOCKET_RETRY_INTERVAL.getUSeconds()/1000);
 
 namespace Drv {
 
@@ -56,7 +55,7 @@ void TcpClientTester ::test_with_loop(U32 iterations, bool recv_thread) {
         if (not recv_thread) {
             status1 = this->component.open();
         } else {
-            EXPECT_TRUE(Drv::Test::wait_on_change(this->component.getSocketHandler(), true, SOCKET_RETRY_INTERVAL_MS/10 + 1));
+            EXPECT_TRUE(Drv::Test::wait_on_change(this->component.getSocketHandler(), true, Drv::Test::get_configured_delay_ms()/10 + 1));
         }
         EXPECT_TRUE(this->component.getSocketHandler().isOpened());
         status2 = server.open();

Drv/TcpServer/test/ut/TcpServerTester.cpp

@@ -17,8 +17,6 @@
 
 Os::Log logger;
 
-const U64 SOCKET_RETRY_INTERVAL_MS = (SOCKET_RETRY_INTERVAL.getSeconds() * 1000) + (SOCKET_RETRY_INTERVAL.getUSeconds()/1000);
-
 namespace Drv {
 
 // ----------------------------------------------------------------------
@@ -40,7 +38,7 @@ void TcpServerTester ::test_with_loop(U32 iterations, bool recv_thread) {
     if (recv_thread) {
         Os::TaskString name("receiver thread");
         this->component.start(name, true, Os::Task::TASK_DEFAULT, Os::Task::TASK_DEFAULT);
-        EXPECT_TRUE(Drv::Test::wait_on_started(this->component.getSocketHandler(), true, SOCKET_RETRY_INTERVAL_MS/10 + 1));
+        EXPECT_TRUE(Drv::Test::wait_on_started(this->component.getSocketHandler(), true, Drv::Test::get_configured_delay_ms()/10 + 1));
     } else {
         serverStat = this->component.startup();
         ASSERT_EQ(serverStat, SOCK_SUCCESS)
@@ -61,7 +59,7 @@ void TcpServerTester ::test_with_loop(U32 iterations, bool recv_thread) {
         if (not recv_thread) {
             status1 = this->component.open();
         } else {
-            EXPECT_TRUE(Drv::Test::wait_on_change(this->component.getSocketHandler(), true, SOCKET_RETRY_INTERVAL_MS/10 + 1));
+            EXPECT_TRUE(Drv::Test::wait_on_change(this->component.getSocketHandler(), true, Drv::Test::get_configured_delay_ms()/10 + 1));
         }
         EXPECT_TRUE(this->component.getSocketHandler().isOpened());
 

Drv/Udp/test/ut/UdpTester.cpp

@@ -17,7 +17,6 @@
 #include <sys/socket.h>
 
 Os::Log logger;
-const U64 SOCKET_RETRY_INTERVAL_MS = (SOCKET_RETRY_INTERVAL.getSeconds() * 1000) + (SOCKET_RETRY_INTERVAL.getUSeconds()/1000);
 
 namespace Drv {
 
@@ -70,7 +69,7 @@ void UdpTester::test_with_loop(U32 iterations, bool recv_thread) {
                 << "Port2: " << port2;
 
         } else {
-            EXPECT_TRUE(Drv::Test::wait_on_change(this->component.getSocketHandler(), true, SOCKET_RETRY_INTERVAL_MS/10 + 1));
+            EXPECT_TRUE(Drv::Test::wait_on_change(this->component.getSocketHandler(), true, Drv::Test::get_configured_delay_ms()/10 + 1));
         }
         EXPECT_TRUE(this->component.getSocketHandler().isOpened());
 

Os/Posix/Task.cpp

@@ -17,7 +17,7 @@
 namespace Os {
 namespace Posix {
 namespace Task {
-    bool PosixTask::s_permissions_reported = false;
+    std::atomic<bool> PosixTask::s_permissions_reported(false);
     static const PlatformIntType SCHED_POLICY = SCHED_RR;
 
     typedef void* (*pthread_func_ptr)(void*);

Os/Posix/Task.hpp

@@ -5,6 +5,7 @@
 #ifndef Os_Posix_Task_hpp_
 #define Os_Posix_Task_hpp_
 
+#include <atomic>
 #include <pthread.h>
 #include <Os/Task.hpp>
 
@@ -105,7 +106,7 @@ namespace Task {
         Status create(const Os::Task::Arguments& arguments, const PosixTask::PermissionExpectation permissions);
 
         PosixTaskHandle m_handle; //!< Posix task tracking
-        static bool s_permissions_reported; //!< Permission errors have been reported
+        static std::atomic<bool> s_permissions_reported; //!< Permission errors have been reported
     };
 } // end namespace Task
 } // end namespace Posix

Os/Stub/test/DefaultTask.cpp

@@ -1,6 +1,6 @@
 // ======================================================================
-// \title Os/Posix/DefaultTask.cpp
-// \brief sets default Os::Task to posix implementation via linker
+// \title Os/Stub/test/DefaultTask.cpp
+// \brief sets default Os::Task to test stub implementation via linker
 // ======================================================================
 #include <cerrno>
 #include "Os/Task.hpp"
@@ -13,6 +13,8 @@ namespace Os {
         Os::Stub::Task::Test::StaticData::data.lastCalled = Os::Stub::Task::Test::StaticData::LastFn::DELAY_FN;
         Os::Stub::Task::Test::StaticData::data.delay = interval;
 
+        // For testing stub, the default implementation of delay for a "task" is a busy wait. This acts as a synthetic
+        // albeit inefficient implementation.
         timeval start;
         timeval end;
         if (gettimeofday(&start, nullptr) == 0) {

Os/Stub/test/ut/StubFileTests.cpp

@@ -211,5 +211,6 @@ TEST(FppTypes, FileModeEnum) {
 
 int main(int argc, char **argv) {
     ::testing::InitGoogleTest(&argc, argv);
+    STest::Random::seed();
     return RUN_ALL_TESTS();
 }

Os/Stub/test/ut/StubTaskTests.cpp

@@ -139,5 +139,6 @@ TEST_F(Interface, RegistryRemove) {
 
 int main(int argc, char **argv) {
     ::testing::InitGoogleTest(&argc, argv);
+    STest::Random::seed();
     return RUN_ALL_TESTS();
 }

Os/test/ut/task/CommonTests.cpp

@@ -8,7 +8,7 @@
 #include "Os/test/ut/task/RulesHeaders.hpp"
 #include "Fw/Types/String.hpp"
 
-static const U32 RANDOM_BOUND = 1000;
+static constexpr U32 RANDOM_BOUND = 1000;
 
 namespace Os {
 namespace Test {

Os/test/ut/task/TaskRules.cpp

@@ -42,8 +42,6 @@ bool Os::Test::Task::Tester::Start::precondition(
 void Os::Test::Task::Tester::Start::action(
         Os::Test::Task::Tester &state //!< The test state
 ) {
-    printf("--> Rule: Start \n");
-
     std::shared_ptr<TestTaskInfo> new_task = std::make_shared<TestTaskInfo>();
     state.m_tasks.push_back(new_task);
 
@@ -76,7 +74,6 @@ bool Os::Test::Task::Tester::Join::precondition(
 void Os::Test::Task::Tester::Join::action(
         Os::Test::Task::Tester &state //!< The test state
 ) {
-    printf("--> Rule: Join\n");
     TestTaskInfo joiner_task;
     const U32 random_index = STest::Pick::lowerUpper(0, state.m_tasks.size() - 1);
 
@@ -121,7 +118,6 @@ bool Os::Test::Task::Tester::CheckState::precondition(
 void Os::Test::Task::Tester::CheckState::action(
         Os::Test::Task::Tester &state //!< The test state
 ) {
-    printf("--> Rule: Check State\n");
     std::shared_ptr<TestTaskInfo> task;
     const U32 random_index = STest::Pick::lowerUpper(0, state.m_tasks.size());
 
@@ -153,7 +149,6 @@ bool Os::Test::Task::Tester::Delay::precondition(
 void Os::Test::Task::Tester::Delay::action(
         Os::Test::Task::Tester &state //!< The test state
 ) {
-    printf("--> Rule: Delay\n");
     const U32 delay_micro_seconds = 5; //STest::Pick::lowerUpper(0, MAX_DELAY_MICRO_SECONDS);
     Fw::Time delay(delay_micro_seconds / 1000000, delay_micro_seconds % 1000000);
 
@@ -187,7 +182,6 @@ bool Os::Test::Task::Tester::CheckTaskCount::precondition(
 void Os::Test::Task::Tester::CheckTaskCount::action(
         Os::Test::Task::Tester &state //!< The test state
 ) {
-    printf("--> Rule: CheckTaskCount \n");
     FwSizeType count = TestTaskInfo::s_task_count;
     ASSERT_EQ(Os::Task::getNumTasks(), count) << "Task count miss-match";
 }
@@ -212,8 +206,6 @@ bool Os::Test::Task::Tester::JoinInvalidState::precondition(
 void Os::Test::Task::Tester::JoinInvalidState::action(
         Os::Test::Task::Tester &state //!< The test state
 ) {
-    printf("--> Rule: JoinInvalidStatus\n");
-
     std::shared_ptr<TestTaskInfo> new_task = std::make_shared<TestTaskInfo>();
     ASSERT_NE(new_task->m_task.join(), Os::Task::Status::OP_OK);
 }
@@ -238,7 +230,6 @@ bool Os::Test::Task::Tester::StartIllegalRoutine::precondition(
 void Os::Test::Task::Tester::StartIllegalRoutine::action(
         Os::Test::Task::Tester &state //!< The test state
 ) {
-    printf("--> Rule: StartIllegalRoutine \n");
     std::shared_ptr<TestTaskInfo> new_task = std::make_shared<TestTaskInfo>();
     state.m_tasks.push_back(new_task);
 

Ref/Top/RefTopology.hpp

@@ -69,14 +69,13 @@ void teardownTopology(const TopologyState& state);
  *
  * The reference topology does not have a true 1Hz input clock for the rate group driver because it is designed to
  * operate across various computing endpoints (e.g. laptops) where a clear 1Hz source may not be easily and generically
- * achieved. This function mimics the cycling via a Task::delay(milliseconds) loop that manually invokes the ISR call
+ * achieved. This function mimics the cycling via a Task::delay(Fw::Time()) loop that manually invokes the ISR call
  * to the example block driver.
  *
  * This loop is stopped via a startSimulatedCycle call.
  *
  * Note: projects should replace this with a component that produces an output port call at the appropriate frequency.
  *
- * \param milliseconds: milliseconds to delay for each cycle. Default: 1000 or 1Hz.
  */
 void startSimulatedCycle(Fw::Time interval);
 

Svc/LinuxTimer/LinuxTimerComponentImplTaskDelay.cpp

@@ -19,7 +19,7 @@ namespace Svc {
 
   void LinuxTimerComponentImpl::startTimer(NATIVE_INT_TYPE interval) {
       while (true) {
-          Os::Task::delay(Fw::Time(static_cast<U32>(interval/1000), static_cast<U32>((interval*1000) % 1000000)));
+          Os::Task::delay(Fw::Time(static_cast<U32>(interval/1000), static_cast<U32>((interval % 1000) * 1000)));
           this->m_mutex.lock();
           bool quit = this->m_quit;
           this->m_mutex.unLock();

config/IpCfg.hpp

@@ -21,7 +21,7 @@ enum IpCfg {
     SOCKET_MAX_ITERATIONS = 0xFFFF,        // Maximum send/recv attempts before an error is returned
     SOCKET_MAX_HOSTNAME_SIZE = 256         // Maximum stored hostname
 };
-const Fw::Time SOCKET_RETRY_INTERVAL = Fw::Time(1, 0);
+static const Fw::Time SOCKET_RETRY_INTERVAL = Fw::Time(1, 0);
 
 
 #endif //REF_IPCFG_HPP