diff --git a/src/openlcb/BroadcastTimeServer.cxxtest b/src/openlcb/BroadcastTimeServer.cxxtest index 6e66463e0..eed02578d 100644 --- a/src/openlcb/BroadcastTimeServer.cxxtest +++ b/src/openlcb/BroadcastTimeServer.cxxtest @@ -1,6 +1,7 @@ #include "utils/async_if_test_helper.hxx" #include "openlcb/BroadcastTimeServer.hxx" +#include "os/FakeClock.hxx" #if 1 #define PRINT_ALL_PACKETS() print_all_packets() @@ -68,6 +69,37 @@ protected: delete server_; } + /// Helper function for sleeping. + /// @param clk fake clock or nullptr if no fake clock exists + /// @param len_msec how long to sleep + /// @param step_msec what granularity to use for sleeping wiht fake clock. + void sleep_helper(FakeClock *clk, unsigned len_msec, + unsigned step_msec = 50) + { + if (clk) + { + for (unsigned i = 0; i < len_msec; i += step_msec) + { + clk->advance(MSEC_TO_NSEC(step_msec)); + wait(); + } + } + else + { + usleep(MSEC_TO_USEC(len_msec)); + } + } + + /// Performs the sleep sequence needed for the server to output the sync + /// events. + /// @param clk if not null, uses fake clock to advance. + /// @param step_msec how big steps should we advance the fake clock. + void sync_sleep(FakeClock *clk = nullptr, unsigned step_msec = 50) + { + // Sleep 3.2 seconds. + sleep_helper(clk, 3200, step_msec); + } + MOCK_METHOD0(update_callback, void()); BroadcastTimeServer *server_; @@ -81,10 +113,26 @@ TEST_F(BroadcastTimeServerTest, Create) EXPECT_EQ(server_->day_of_year(), 0); }; +TEST_F(BroadcastTimeServerTest, SleepTest) +{ + FakeClock clk; + long long t1 = os_get_time_monotonic(); + sync_sleep(&clk); + long long t2 = os_get_time_monotonic(); + EXPECT_NEAR(t2-t1, MSEC_TO_NSEC(3200), USEC_TO_NSEC(1)); +}; + TEST_F(BroadcastTimeServerTest, Query) { + FakeClock clk; ::testing::Sequence s1; + send_packet(":X195B4001N010100000100F000;"); // query + wait(); + + clear_expect(true); + // The server only responds a bit later in case multiple queries arrive. + // sync response expect_packet(":X1954422AN010100000100F001;").InSequence(s1); expect_packet(":X1954422AN0101000001004000;").InSequence(s1); @@ -92,9 +140,8 @@ TEST_F(BroadcastTimeServerTest, Query) expect_packet(":X1954422AN0101000001002101;").InSequence(s1); expect_packet(":X1954422AN0101000001000000;").InSequence(s1); - send_packet(":X195B4001N010100000100F000;"); // query - usleep(400000); // wait for response agrigation - wait_for_event_thread(); + clk.advance(MSEC_TO_NSEC(400)); + wait(); // time is not setup, clock is not running, expect 0 as default EXPECT_EQ(server_->time(), 0); @@ -104,6 +151,7 @@ TEST_F(BroadcastTimeServerTest, Query) TEST_F(BroadcastTimeServerTest, StartSetTime) { + FakeClock clk; ::testing::Sequence s1, s2; // set events @@ -130,7 +178,7 @@ TEST_F(BroadcastTimeServerTest, StartSetTime) // start expect_packet(":X195B422AN010100000100F002;").InSequence(s2); - // sync seqeunce + // sync sequence expect_packet(":X1954422AN010100000100F002;").InSequence(s2); expect_packet(":X1954422AN01010000010047D0;").InSequence(s2); expect_packet(":X1954422AN01010000010037B2;").InSequence(s2); @@ -141,12 +189,18 @@ TEST_F(BroadcastTimeServerTest, StartSetTime) // callbacks on clock update EXPECT_CALL(*this, update_callback()).Times(1); + LOG(INFO, "start server"); server_->start(); - - // allow time for the sync timeout - usleep(3200000); - wait_for_event_thread(); - + wait(); + LOG(INFO, "exec wait done"); + + // Time for the sync timeout. There are two sleeps there in order to send + // out two minute events, so we need to sleep twice too. + clk.advance(MSEC_TO_NSEC(3000)); + wait(); + clk.advance(MSEC_TO_NSEC(200)); + wait(); + // check the time, we give it a finite range just in case of some OS jitter EXPECT_TRUE(IsBetweenInclusive(server_->time(), 1599, 1602)); EXPECT_EQ(server_->day_of_week(), BroadcastTimeDefs::THURSDAY); @@ -155,6 +209,7 @@ TEST_F(BroadcastTimeServerTest, StartSetTime) TEST_F(BroadcastTimeServerTest, DateRolloverForward) { + FakeClock clk; ::testing::Sequence s1, s2; // set events @@ -179,6 +234,8 @@ TEST_F(BroadcastTimeServerTest, DateRolloverForward) clear_expect(true); + EXPECT_TRUE(IsBetweenInclusive(server_->time(), 86340, 86340)); + // start expect_packet(":X195B422AN010100000100F002;").InSequence(s2); @@ -186,7 +243,7 @@ TEST_F(BroadcastTimeServerTest, DateRolloverForward) expect_packet(":X195B422AN010100000100F003;").InSequence(s2); expect_packet(":X195B422AN0101000001000000;").InSequence(s2); - // sync seqeunce + // sync sequence expect_packet(":X1954422AN010100000100F002;").InSequence(s2); expect_packet(":X1954422AN01010000010047D0;").InSequence(s2); expect_packet(":X1954422AN01010000010037B2;").InSequence(s2); @@ -205,8 +262,12 @@ TEST_F(BroadcastTimeServerTest, DateRolloverForward) server_->start(); // allow time for the sync timeout - usleep(3200000); - wait_for_event_thread(); + + /// @todo this does not pass when using more than one msec of granularity + /// for the fake clock advances. That suggests there is an off by one bug + /// somewhere. The reported time is one tick lower than how much the + /// faketime advanced. + sync_sleep(&clk, 1); // check the time, we give it a finite range just in case of some OS jitter EXPECT_TRUE(IsBetweenInclusive(server_->time(), 87940, 87941)); @@ -216,6 +277,7 @@ TEST_F(BroadcastTimeServerTest, DateRolloverForward) TEST_F(BroadcastTimeServerTest, DateRolloverForwardOnTopOfSync) { + FakeClock clk; ::testing::Sequence s1, s2, s3; // set events @@ -265,17 +327,22 @@ TEST_F(BroadcastTimeServerTest, DateRolloverForwardOnTopOfSync) server_->start(); // allow time for the sync timeout - usleep(3200000); + sync_sleep(&clk, 1); wait_for_event_thread(); // check the time, we give it a finite range just in case of some OS jitter EXPECT_TRUE(IsBetweenInclusive(server_->time(), 87820, 87821)); EXPECT_EQ(server_->day_of_week(), BroadcastTimeDefs::FRIDAY); EXPECT_EQ(server_->day_of_year(), 1); + + // Clears out timer from queue. + clk.advance(MSEC_TO_NSEC(200)); + wait(); }; #if 1 TEST_F(BroadcastTimeServerTest, DateRolloverForwardAllEventBasedSetup) { + FakeClock clk; ::testing::Sequence s1, s2; // report events @@ -319,7 +386,8 @@ TEST_F(BroadcastTimeServerTest, DateRolloverForwardAllEventBasedSetup) send_packet(":X195B4001N010100000100F002;"); // start // allow time for the sync timeout - usleep(3200000); + sync_sleep(&clk, 1); + //usleep(3200000); wait_for_event_thread(); // check the time, we give it a finite range just in case of some OS jitter @@ -330,6 +398,7 @@ TEST_F(BroadcastTimeServerTest, DateRolloverForwardAllEventBasedSetup) #endif TEST_F(BroadcastTimeServerTest, DateRolloverBackward) { + FakeClock clk; ::testing::Sequence s1, s2; // set events @@ -379,7 +448,7 @@ TEST_F(BroadcastTimeServerTest, DateRolloverBackward) server_->start(); // allow time for the sync timeout - usleep(3200000); + sync_sleep(&clk, 1); wait_for_event_thread(); // check the time, we give it a finite range just in case of some OS jitter @@ -390,6 +459,7 @@ TEST_F(BroadcastTimeServerTest, DateRolloverBackward) TEST_F(BroadcastTimeServerTest, Subscribe) { + FakeClock clk; ::testing::Sequence s1, s2; // set events @@ -411,6 +481,7 @@ TEST_F(BroadcastTimeServerTest, Subscribe) server_->set_rate_quarters(2000); wait_for_event_thread(); + clear_expect(true); clear_expect(true); // start @@ -430,8 +501,7 @@ TEST_F(BroadcastTimeServerTest, Subscribe) server_->start(); // allow time for the sync timeout - usleep(3200000); - wait_for_event_thread(); + sync_sleep(&clk, 1); // subscribe to some times clear_expect(true); @@ -444,7 +514,7 @@ TEST_F(BroadcastTimeServerTest, Subscribe) send_packet(":X194C7001N010100000100003A;"); // subscribe to 00:58 send_packet(":X194C7001N010100000100003B;"); // subscribe to 00:59 send_packet(":X194C7001N0101000001000100;"); // subscribe to 01:00 - usleep(4100000); + sleep_helper(&clk, 4100, 1); // check the time, we give it a finite range just in case of some OS jitter EXPECT_TRUE(IsBetweenInclusive(server_->time(), 3650, 3651)); diff --git a/src/openlcb/IfCan.cxx b/src/openlcb/IfCan.cxx index 60799416e..704b16f23 100644 --- a/src/openlcb/IfCan.cxx +++ b/src/openlcb/IfCan.cxx @@ -758,7 +758,7 @@ void IfCan::delete_local_node(Node *node) { void IfCan::canonicalize_handle(NodeHandle *h) { - if (!h->id & !h->alias) + if (!h->id && !h->alias) return; if (!h->id) { diff --git a/src/os/FakeClock.cxxtest b/src/os/FakeClock.cxxtest index 9b5a4e949..e6a0a173b 100644 --- a/src/os/FakeClock.cxxtest +++ b/src/os/FakeClock.cxxtest @@ -20,6 +20,12 @@ TEST(FakeClockTest, advance) EXPECT_GT(tfreeze + 500, os_get_time_monotonic()); } + // Advance should be accurate. + long long t3 = os_get_time_monotonic(); + clk.advance(MSEC_TO_NSEC(3540)); + long long t4 = os_get_time_monotonic(); + EXPECT_NEAR(t4, t3 + MSEC_TO_NSEC(3540), 10); + // But still be monotonic. t1 = os_get_time_monotonic(); t2 = os_get_time_monotonic(); @@ -35,6 +41,27 @@ TEST(FakeClockTest, independent_test) EXPECT_LT(t1 + MSEC_TO_NSEC(20), t2); } +TEST(FakeClockTest, time_jumps_backwards) +{ + // This test verifies that when a fake clock is deleted, time will jump + // backwards. This is desirable, because the fake clock might have its time + // far in the future and we cannot affort to halt the monotonic clock until + // the real time catches up with the fake time. + long long t1 = os_get_time_monotonic(); + long long t2, t3, t4; + { + FakeClock clk; + t2 = os_get_time_monotonic(); + clk.advance(SEC_TO_NSEC(15)); + t3 = os_get_time_monotonic(); + } + t4 = os_get_time_monotonic(); + EXPECT_LT(t1, t2); + EXPECT_LT(t2, t3 - SEC_TO_NSEC(15)); + EXPECT_LT(t1, t4); + EXPECT_GT(t3, t4); +} + class CountingTimer : public Timer { public: diff --git a/src/os/os.c b/src/os/os.c index 899d35bf6..f956cd173 100644 --- a/src/os/os.c +++ b/src/os/os.c @@ -638,7 +638,7 @@ long long os_get_time_monotonic(void) long long fake_time = os_get_fake_time(); if (fake_time >= 0) { - time = fake_time; + return fake_time; } #endif // not GTEST