Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Broadcast Time Server Detection #521

Merged
merged 4 commits into from
Apr 4, 2021
Merged

Broadcast Time Server Detection #521

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a061792
Add method for determining if a timer server has been discovered.
bakerstu Mar 22, 2021
42aecce
Add test for time server discovery.
bakerstu Mar 22, 2021
92b9f7b
Make methods public.
bakerstu Mar 22, 2021
9cad083
Mask "server" detection for date events only.
bakerstu Apr 4, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/openlcb/BroadcastTime.hxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,10 @@ public:
return &tm_;
}

/// Has a time server been detected?
/// @return true if a time server has been detected, else false
virtual bool is_server_detected() = 0;

protected:
class SetFlow : public StateFlowBase
{
Expand Down
3 changes: 3 additions & 0 deletions src/openlcb/BroadcastTimeClient.cxxtest
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,8 @@ TEST_F(BroadcastTimeClientTest, Create)
EXPECT_EQ(client2_->time(), 0);
EXPECT_EQ(client2_->day_of_week(), BroadcastTimeDefs::THURSDAY);
EXPECT_EQ(client2_->day_of_year(), 0);
EXPECT_FALSE(client1_->is_server_detected());
EXPECT_FALSE(client2_->is_server_detected());
};

TEST_F(BroadcastTimeClientTest, Start)
Expand All @@ -265,6 +267,7 @@ TEST_F(BroadcastTimeClientTest, Start)
sync(client1_, 500);
wait();

EXPECT_TRUE(client1_->is_server_detected());

// check the time, we give it a finite range just in case of some OS jitter
EXPECT_TRUE(IsBetweenInclusive(client1_->time(), 60, 62));
Expand Down
12 changes: 12 additions & 0 deletions src/openlcb/BroadcastTimeClient.hxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ public:
, rolloverPending_(false)
, rolloverPendingDate_(false)
, rolloverPendingYear_(false)
, serverDetected_(false)
{
EventRegistry::instance()->register_handler(
EventRegistryEntry(this, eventBase_), 16);
Expand All @@ -73,6 +74,13 @@ public:
EventRegistry::instance()->unregister_handler(this);
}

/// Has a time server been detected?
/// @return true if a time server has been detected, else false
bool is_server_detected() override
{
return serverDetected_;
}

/// Handle requested identification message.
/// @param entry registry entry for the event range
/// @param event information about the incoming message
Expand Down Expand Up @@ -176,6 +184,9 @@ public:

if (event->state == EventState::VALID)
{
// We can only get here if there is a time server detected
serverDetected_ = true;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should exclude the start/stop/query events and the set-time=X set-date=X events from this, and look only for the time-is or date-is events. I guess that is a simple check of ((event_id & 0x8000) == 0).
All of those other events could be produced by LCC buttons that are meant to remote control the clock.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another option might be to only look for the date-is events for server detection (0x2 in the fourth nibble).

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, went with the date mask option.


// We only care about valid event state.
// Producer identified only happens when a clock synchronization
// is taking place. This voids previous date rollover events.
Expand Down Expand Up @@ -422,6 +433,7 @@ private:
uint16_t rolloverPending_ : 1; ///< a day rollover is about to occur
uint16_t rolloverPendingDate_ : 1; ///< a day rollover is about to occur
uint16_t rolloverPendingYear_ : 1; ///< a day rollover is about to occur
uint16_t serverDetected_ : 1; ///< has a time server been detected


DISALLOW_COPY_AND_ASSIGN(BroadcastTimeClient);
Expand Down
1 change: 1 addition & 0 deletions src/openlcb/BroadcastTimeServer.cxxtest
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ TEST_F(BroadcastTimeServerTest, Create)
EXPECT_EQ(server_->time(), 0);
EXPECT_EQ(server_->day_of_week(), BroadcastTimeDefs::THURSDAY);
EXPECT_EQ(server_->day_of_year(), 0);
EXPECT_TRUE(server_->is_server_detected());
};

TEST_F(BroadcastTimeServerTest, Time)
Expand Down
7 changes: 7 additions & 0 deletions src/openlcb/BroadcastTimeServer.hxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,13 @@ public:
/// Destructor.
~BroadcastTimeServer();

/// Has a time server been detected?
/// @return true if a time server has been detected, else false
bool is_server_detected() override
{
return true;
}

#if defined(GTEST)
void shutdown();

Expand Down