From 94d81c0c6bef5cb0cd7252c4897d7a8f73de2d65 Mon Sep 17 00:00:00 2001 From: Dan Paulat Date: Sat, 9 Aug 2025 01:19:51 -0500 Subject: [PATCH 01/15] Initial NTP protocol functionality --- test/source/scwx/network/ntp_client.test.cpp | 21 +++ test/test.cmake | 3 +- wxdata/include/scwx/network/ntp_client.hpp | 32 +++++ wxdata/include/scwx/types/ntp_types.hpp | 61 ++++++++ wxdata/source/scwx/network/ntp_client.cpp | 143 +++++++++++++++++++ wxdata/source/scwx/types/ntp_types.cpp | 51 +++++++ wxdata/wxdata.cmake | 12 +- 7 files changed, 318 insertions(+), 5 deletions(-) create mode 100644 test/source/scwx/network/ntp_client.test.cpp create mode 100644 wxdata/include/scwx/network/ntp_client.hpp create mode 100644 wxdata/include/scwx/types/ntp_types.hpp create mode 100644 wxdata/source/scwx/network/ntp_client.cpp create mode 100644 wxdata/source/scwx/types/ntp_types.cpp diff --git a/test/source/scwx/network/ntp_client.test.cpp b/test/source/scwx/network/ntp_client.test.cpp new file mode 100644 index 00000000..cebd8cc2 --- /dev/null +++ b/test/source/scwx/network/ntp_client.test.cpp @@ -0,0 +1,21 @@ +#include + +#include + +namespace scwx +{ +namespace network +{ + +TEST(NtpClient, Poll) +{ + NtpClient client {}; + + client.Open("time.nist.gov", "123"); + //client.Open("pool.ntp.org", "123"); + //client.Open("time.windows.com", "123"); + client.Poll(); +} + +} // namespace network +} // namespace scwx diff --git a/test/test.cmake b/test/test.cmake index 0ae26b53..57fbc37e 100644 --- a/test/test.cmake +++ b/test/test.cmake @@ -17,7 +17,8 @@ set(SRC_AWIPS_TESTS source/scwx/awips/coded_location.test.cpp set(SRC_COMMON_TESTS source/scwx/common/color_table.test.cpp source/scwx/common/products.test.cpp) set(SRC_GR_TESTS source/scwx/gr/placefile.test.cpp) -set(SRC_NETWORK_TESTS source/scwx/network/dir_list.test.cpp) +set(SRC_NETWORK_TESTS source/scwx/network/dir_list.test.cpp + source/scwx/network/ntp_client.test.cpp) set(SRC_PROVIDER_TESTS source/scwx/provider/aws_level2_data_provider.test.cpp source/scwx/provider/aws_level3_data_provider.test.cpp source/scwx/provider/iem_api_provider.test.cpp diff --git a/wxdata/include/scwx/network/ntp_client.hpp b/wxdata/include/scwx/network/ntp_client.hpp new file mode 100644 index 00000000..55ef4204 --- /dev/null +++ b/wxdata/include/scwx/network/ntp_client.hpp @@ -0,0 +1,32 @@ +#pragma once + +#include +#include + +namespace scwx::network +{ + +/** + * @brief NTP Client + */ +class NtpClient +{ +public: + explicit NtpClient(); + ~NtpClient(); + + NtpClient(const NtpClient&) = delete; + NtpClient& operator=(const NtpClient&) = delete; + + NtpClient(NtpClient&&) noexcept; + NtpClient& operator=(NtpClient&&) noexcept; + + void Open(std::string_view host, std::string_view service); + void Poll(); + +private: + class Impl; + std::unique_ptr p; +}; + +} // namespace scwx::network diff --git a/wxdata/include/scwx/types/ntp_types.hpp b/wxdata/include/scwx/types/ntp_types.hpp new file mode 100644 index 00000000..39c1f12f --- /dev/null +++ b/wxdata/include/scwx/types/ntp_types.hpp @@ -0,0 +1,61 @@ +#pragma once + +#include +#include +#include + +namespace scwx::types::ntp +{ + +/* Adapted from: + * https://github.com/lettier/ntpclient/blob/master/source/c/main.c + * + * Copyright (c) 2014 David Lettier + * Copyright (c) 2020 Krystian Stasiowski + * Distributed under the BSD 3-Clause License (See + * https://github.com/lettier/ntpclient/blob/master/LICENSE) + */ + +#pragma pack(push, 1) + +struct NtpPacket +{ + union + { + std::uint8_t li_vn_mode; + struct + { + std::uint8_t mode : 3; // Client will pick mode 3 for client. + std::uint8_t vn : 3; // Version number of the protocol. + std::uint8_t li : 2; // Leap indicator. + } fields; + }; + + std::uint8_t stratum; // Stratum level of the local clock. + std::uint8_t poll; // Maximum interval between successive messages. + std::uint8_t precision; // Precision of the local clock. + + std::uint32_t rootDelay; // Total round trip delay time. + std::uint32_t rootDispersion; // Max error aloud from primary clock source. + std::uint32_t refId; // Reference clock identifier. + + std::uint32_t refTm_s; // Reference time-stamp seconds. + std::uint32_t refTm_f; // Reference time-stamp fraction of a second. + + std::uint32_t origTm_s; // Originate time-stamp seconds. + std::uint32_t origTm_f; // Originate time-stamp fraction of a second. + + std::uint32_t rxTm_s; // Received time-stamp seconds. + std::uint32_t rxTm_f; // Received time-stamp fraction of a second. + + std::uint32_t txTm_s; // The most important field the client cares about. + // Transmit time-stamp seconds. + std::uint32_t txTm_f; // Transmit time-stamp fraction of a second. + + static NtpPacket Parse(const std::span data); +}; +// Total: 48 bytes. + +#pragma pack(pop) + +} // namespace scwx::types::ntp diff --git a/wxdata/source/scwx/network/ntp_client.cpp b/wxdata/source/scwx/network/ntp_client.cpp new file mode 100644 index 00000000..7315eeec --- /dev/null +++ b/wxdata/source/scwx/network/ntp_client.cpp @@ -0,0 +1,143 @@ +#include +#include +#include +#include + +#include +#include +#include + +namespace scwx::network +{ + +static const std::string logPrefix_ = "scwx::network::ntp_client"; +static const auto logger_ = scwx::util::Logger::Create(logPrefix_); + +static constexpr std::size_t kReceiveBufferSize_ {48u}; + +class NtpClient::Impl +{ +public: + explicit Impl(); + ~Impl(); + Impl(const Impl&) = delete; + Impl& operator=(const Impl&) = delete; + Impl(const Impl&&) = delete; + Impl& operator=(const Impl&&) = delete; + + void Open(std::string_view host, std::string_view service); + void Poll(); + void ReceivePacket(std::size_t length); + + boost::asio::thread_pool threadPool_ {2u}; + + types::ntp::NtpPacket transmitPacket_ {}; + + boost::asio::ip::udp::socket socket_; + std::optional serverEndpoint_ {}; + std::array receiveBuffer_ {}; + + std::vector serverList_ { + "time.nist.gov", "ntp.pool.org", "time.windows.com"}; +}; + +NtpClient::NtpClient() : p(std::make_unique()) {} +NtpClient::~NtpClient() = default; + +NtpClient::NtpClient(NtpClient&&) noexcept = default; +NtpClient& NtpClient::operator=(NtpClient&&) noexcept = default; + +void NtpClient::Open(std::string_view host, std::string_view service) +{ + p->Open(host, service); +} + +void NtpClient::Poll() +{ + p->Poll(); +} + +NtpClient::Impl::Impl() : socket_ {threadPool_} +{ + transmitPacket_.fields.vn = 3; // Version + transmitPacket_.fields.mode = 3; // Client (3) +} + +NtpClient::Impl::~Impl() +{ + threadPool_.join(); +} + +void NtpClient::Impl::Open(std::string_view host, std::string_view service) +{ + boost::asio::ip::udp::resolver resolver(threadPool_); + boost::system::error_code ec; + + auto results = resolver.resolve(host, service, ec); + if (ec.value() == boost::system::errc::success && !results.empty()) + { + logger_->info("Using NTP server: {}", host); + serverEndpoint_ = *results.begin(); + socket_.open(serverEndpoint_->protocol()); + } + else + { + serverEndpoint_ = std::nullopt; + logger_->warn("Could not resolve host {}: {}", host, ec.message()); + } +} + +void NtpClient::Impl::Poll() +{ + using namespace std::chrono_literals; + + static constexpr auto kTimeout_ = 15s; + + try + { + std::size_t transmitPacketSize = sizeof(transmitPacket_); + // Send NTP request + socket_.send_to(boost::asio::buffer(&transmitPacket_, transmitPacketSize), + *serverEndpoint_); + + // Receive NTP response + auto future = + socket_.async_receive_from(boost::asio::buffer(receiveBuffer_), + *serverEndpoint_, + boost::asio::use_future); + std::size_t bytesReceived = 0; + + switch (future.wait_for(kTimeout_)) + { + case std::future_status::ready: + bytesReceived = future.get(); + ReceivePacket(bytesReceived); + break; + + case std::future_status::timeout: + case std::future_status::deferred: + logger_->warn("Timeout waiting for NTP response"); + socket_.cancel(); + break; + } + } + catch (const std::exception& ex) + { + logger_->error("Error polling: {}", ex.what()); + } +} + +void NtpClient::Impl::ReceivePacket(std::size_t length) +{ + if (length >= sizeof(types::ntp::NtpPacket)) + { + auto packet = types::ntp::NtpPacket::Parse(receiveBuffer_); + (void) packet; + } + else + { + logger_->warn("Received too few bytes: {}", length); + } +} + +} // namespace scwx::network diff --git a/wxdata/source/scwx/types/ntp_types.cpp b/wxdata/source/scwx/types/ntp_types.cpp new file mode 100644 index 00000000..cca0c422 --- /dev/null +++ b/wxdata/source/scwx/types/ntp_types.cpp @@ -0,0 +1,51 @@ +#include + +#include +#include + +#ifdef _WIN32 +# include +#else +# include +#endif + +namespace scwx::types::ntp +{ + +NtpPacket NtpPacket::Parse(const std::span data) +{ + NtpPacket packet; + + assert(data.size() >= sizeof(NtpPacket)); + + packet = *reinterpret_cast(data.data()); + + // Detect Kiss-o'-Death (KoD) packet + if (packet.stratum == 0) + { + // TODO + std::string kissCode = + std::string(reinterpret_cast(&packet.refId), 4); + (void) kissCode; + } + + packet.rootDelay = ntohl(packet.rootDelay); + packet.rootDispersion = ntohl(packet.rootDispersion); + packet.refId = ntohl(packet.refId); + + packet.refTm_s = ntohl(packet.refTm_s); + packet.refTm_f = ntohl(packet.refTm_f); + + packet.origTm_s = ntohl(packet.origTm_s); + packet.origTm_f = ntohl(packet.origTm_f); + + packet.rxTm_s = ntohl(packet.rxTm_s); + packet.rxTm_f = ntohl(packet.rxTm_f); + + packet.txTm_s = ntohl(packet.txTm_s); + packet.txTm_f = ntohl(packet.txTm_f); + + return packet; +} + +} // namespace scwx::types::ntp diff --git a/wxdata/wxdata.cmake b/wxdata/wxdata.cmake index ab23a4e7..bc489cb3 100644 --- a/wxdata/wxdata.cmake +++ b/wxdata/wxdata.cmake @@ -60,9 +60,11 @@ set(HDR_GR include/scwx/gr/color.hpp set(SRC_GR source/scwx/gr/color.cpp source/scwx/gr/placefile.cpp) set(HDR_NETWORK include/scwx/network/cpr.hpp - include/scwx/network/dir_list.hpp) + include/scwx/network/dir_list.hpp + include/scwx/network/ntp_client.hpp) set(SRC_NETWORK source/scwx/network/cpr.cpp - source/scwx/network/dir_list.cpp) + source/scwx/network/dir_list.cpp + source/scwx/network/ntp_client.cpp) set(HDR_PROVIDER include/scwx/provider/aws_level2_data_provider.hpp include/scwx/provider/aws_level2_chunks_data_provider.hpp include/scwx/provider/aws_level3_data_provider.hpp @@ -80,8 +82,10 @@ set(SRC_PROVIDER source/scwx/provider/aws_level2_data_provider.cpp source/scwx/provider/nexrad_data_provider.cpp source/scwx/provider/nexrad_data_provider_factory.cpp source/scwx/provider/warnings_provider.cpp) -set(HDR_TYPES include/scwx/types/iem_types.hpp) -set(SRC_TYPES source/scwx/types/iem_types.cpp) +set(HDR_TYPES include/scwx/types/iem_types.hpp + include/scwx/types/ntp_types.hpp) +set(SRC_TYPES source/scwx/types/iem_types.cpp + source/scwx/types/ntp_types.cpp) set(HDR_UTIL include/scwx/util/digest.hpp include/scwx/util/enum.hpp include/scwx/util/environment.hpp From f85bf9283ae52d2a1206a6fdfa83d886719d1164 Mon Sep 17 00:00:00 2001 From: Dan Paulat Date: Sat, 9 Aug 2025 01:20:30 -0500 Subject: [PATCH 02/15] Union access should not be flagged by clang-tidy --- .clang-tidy | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.clang-tidy b/.clang-tidy index 26230c78..c93d61fe 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -7,8 +7,9 @@ Checks: - 'modernize-*' - 'performance-*' - '-bugprone-easily-swappable-parameters' - - '-cppcoreguidelines-pro-type-reinterpret-cast' - '-cppcoreguidelines-avoid-do-while' + - '-cppcoreguidelines-pro-type-reinterpret-cast' + - '-cppcoreguidelines-pro-type-union-access' - '-misc-include-cleaner' - '-misc-non-private-member-variables-in-classes' - '-misc-use-anonymous-namespace' From 258466e02c3cc94ecc4755b3116bd51aae6823e5 Mon Sep 17 00:00:00 2001 From: Dan Paulat Date: Sun, 10 Aug 2025 00:24:48 -0500 Subject: [PATCH 03/15] NTP time offset calculation --- wxdata/include/scwx/types/ntp_types.hpp | 3 +- wxdata/source/scwx/network/ntp_client.cpp | 162 ++++++++++++++++++++-- wxdata/source/scwx/types/ntp_types.cpp | 9 -- 3 files changed, 154 insertions(+), 20 deletions(-) diff --git a/wxdata/include/scwx/types/ntp_types.hpp b/wxdata/include/scwx/types/ntp_types.hpp index 39c1f12f..cfb8f764 100644 --- a/wxdata/include/scwx/types/ntp_types.hpp +++ b/wxdata/include/scwx/types/ntp_types.hpp @@ -48,8 +48,7 @@ struct NtpPacket std::uint32_t rxTm_s; // Received time-stamp seconds. std::uint32_t rxTm_f; // Received time-stamp fraction of a second. - std::uint32_t txTm_s; // The most important field the client cares about. - // Transmit time-stamp seconds. + std::uint32_t txTm_s; // Transmit time-stamp seconds. std::uint32_t txTm_f; // Transmit time-stamp fraction of a second. static NtpPacket Parse(const std::span data); diff --git a/wxdata/source/scwx/network/ntp_client.cpp b/wxdata/source/scwx/network/ntp_client.cpp index 7315eeec..a20540c4 100644 --- a/wxdata/source/scwx/network/ntp_client.cpp +++ b/wxdata/source/scwx/network/ntp_client.cpp @@ -6,6 +6,7 @@ #include #include #include +#include namespace scwx::network { @@ -15,15 +16,86 @@ static const auto logger_ = scwx::util::Logger::Create(logPrefix_); static constexpr std::size_t kReceiveBufferSize_ {48u}; +class NtpTimestamp +{ +public: + // NTP epoch: January 1, 1900 + // Unix epoch: January 1, 1970 + // Difference = 70 years = 2,208,988,800 seconds + static constexpr std::uint32_t kNtpToUnixOffset_ = 2208988800UL; + + // NTP fractional part represents 1/2^32 of a second + static constexpr std::uint64_t kFractionalMultiplier_ = 0x100000000ULL; + + static constexpr std::uint64_t _1e9 = 1000000000ULL; + + std::uint32_t seconds_ {0}; + std::uint32_t fraction_ {0}; + + explicit NtpTimestamp() = default; + explicit NtpTimestamp(std::uint32_t seconds, std::uint32_t fraction) : + seconds_ {seconds}, fraction_ {fraction} + { + } + ~NtpTimestamp() = default; + + NtpTimestamp(const NtpTimestamp&) = default; + NtpTimestamp& operator=(const NtpTimestamp&) = default; + NtpTimestamp(NtpTimestamp&&) = default; + NtpTimestamp& operator=(NtpTimestamp&&) = default; + + template + std::chrono::time_point ToTimePoint() const + { + // Convert NTP seconds to Unix seconds + // Don't cast to a larger type to account for rollover, and this should + // work until 2106 + const std::uint32_t unixSeconds = seconds_ - kNtpToUnixOffset_; + + // Convert NTP fraction to nanoseconds + const auto nanoseconds = + static_cast(fraction_) * _1e9 / kFractionalMultiplier_; + + return std::chrono::time_point( + std::chrono::duration_cast( + std::chrono::seconds {unixSeconds} + + std::chrono::nanoseconds {nanoseconds})); + } + + template + static NtpTimestamp FromTimePoint(std::chrono::time_point timePoint) + { + // Convert to duration since Unix epoch + const auto unixDuration = timePoint.time_since_epoch(); + + // Extract seconds and nanoseconds + const auto unixSeconds = + std::chrono::duration_cast(unixDuration); + const auto nanoseconds = + std::chrono::duration_cast(unixDuration - + unixSeconds); + + // Convert Unix seconds to NTP seconds + const auto ntpSeconds = + static_cast(unixSeconds.count() + kNtpToUnixOffset_); + + // Convert nanoseconds to NTP fractional seconds + const auto ntpFraction = static_cast( + nanoseconds.count() * kFractionalMultiplier_ / _1e9); + + return NtpTimestamp(ntpSeconds, ntpFraction); + } +}; + class NtpClient::Impl { public: explicit Impl(); ~Impl(); - Impl(const Impl&) = delete; - Impl& operator=(const Impl&) = delete; - Impl(const Impl&&) = delete; - Impl& operator=(const Impl&&) = delete; + Impl(const Impl&) = delete; + Impl& operator=(const Impl&) = delete; + Impl(Impl&&) = delete; + Impl& operator=(Impl&&) = delete; void Open(std::string_view host, std::string_view service); void Poll(); @@ -31,14 +103,22 @@ public: boost::asio::thread_pool threadPool_ {2u}; + bool enabled_; + types::ntp::NtpPacket transmitPacket_ {}; boost::asio::ip::udp::socket socket_; std::optional serverEndpoint_ {}; std::array receiveBuffer_ {}; - std::vector serverList_ { - "time.nist.gov", "ntp.pool.org", "time.windows.com"}; + std::chrono::system_clock::duration timeOffset_ {}; + + std::vector serverList_ {"time.nist.gov", + "time.cloudflare.com", + "ntp.pool.org", + "time.aws.com", + "time.windows.com", + "time.apple.com"}; }; NtpClient::NtpClient() : p(std::make_unique()) {} @@ -59,6 +139,18 @@ void NtpClient::Poll() NtpClient::Impl::Impl() : socket_ {threadPool_} { + using namespace std::chrono_literals; + + const auto now = + std::chrono::floor(std::chrono::system_clock::now()); + + // The NTP timestamp will overflow in 2036. Overflow is handled in such a way + // that should work until 2106. Additional handling for subsequent eras is + // required. + static constexpr auto kMaxYear_ = 2106y; + + enabled_ = now < kMaxYear_ / 1 / 1; + transmitPacket_.fields.vn = 3; // Version transmitPacket_.fields.mode = 3; // Client (3) } @@ -95,10 +187,15 @@ void NtpClient::Impl::Poll() try { + const auto originTimestamp = + NtpTimestamp::FromTimePoint(std::chrono::system_clock::now()); + transmitPacket_.txTm_s = ntohl(originTimestamp.seconds_); + transmitPacket_.txTm_f = ntohl(originTimestamp.fraction_); + std::size_t transmitPacketSize = sizeof(transmitPacket_); // Send NTP request socket_.send_to(boost::asio::buffer(&transmitPacket_, transmitPacketSize), - *serverEndpoint_); + *serverEndpoint_); // Receive NTP response auto future = @@ -131,8 +228,55 @@ void NtpClient::Impl::ReceivePacket(std::size_t length) { if (length >= sizeof(types::ntp::NtpPacket)) { - auto packet = types::ntp::NtpPacket::Parse(receiveBuffer_); - (void) packet; + const auto destinationTime = std::chrono::system_clock::now(); + + const auto packet = types::ntp::NtpPacket::Parse(receiveBuffer_); + + if (packet.stratum == 0) + { + const std::uint32_t refId = ntohl(packet.refId); + const std::string kod = + std::string(reinterpret_cast(&refId), 4); + + logger_->warn("KoD packet received: {}", kod); + + if (kod == "DENY" || kod == "RSTR") + { + // TODO + // The client MUST demobilize any associations to that server and + // stop sending packets to that server + } + else if (kod == "RATE") + { + // TODO + // The client MUST immediately reduce its polling interval to that + // server and continue to reduce it each time it receives a RATE + // kiss code + } + } + else + { + const auto originTimestamp = + NtpTimestamp(packet.origTm_s, packet.origTm_f); + const auto receiveTimestamp = + NtpTimestamp(packet.rxTm_s, packet.rxTm_f); + const auto transmitTimestamp = + NtpTimestamp(packet.txTm_s, packet.txTm_f); + + const auto originTime = originTimestamp.ToTimePoint(); + const auto receiveTime = receiveTimestamp.ToTimePoint(); + const auto transmitTime = transmitTimestamp.ToTimePoint(); + + const auto& t0 = originTime; + const auto& t1 = receiveTime; + const auto& t2 = transmitTime; + const auto& t3 = destinationTime; + + // Update time offset + timeOffset_ = ((t1 - t0) + (t2 - t3)) / 2; + + logger_->debug("Time offset updated: {:%jd %T}", timeOffset_); + } } else { diff --git a/wxdata/source/scwx/types/ntp_types.cpp b/wxdata/source/scwx/types/ntp_types.cpp index cca0c422..9ff39095 100644 --- a/wxdata/source/scwx/types/ntp_types.cpp +++ b/wxdata/source/scwx/types/ntp_types.cpp @@ -20,15 +20,6 @@ NtpPacket NtpPacket::Parse(const std::span data) packet = *reinterpret_cast(data.data()); - // Detect Kiss-o'-Death (KoD) packet - if (packet.stratum == 0) - { - // TODO - std::string kissCode = - std::string(reinterpret_cast(&packet.refId), 4); - (void) kissCode; - } - packet.rootDelay = ntohl(packet.rootDelay); packet.rootDispersion = ntohl(packet.rootDispersion); packet.refId = ntohl(packet.refId); From dfb00b96df4a9f1113ff744ab3d14c42147d777d Mon Sep 17 00:00:00 2001 From: Dan Paulat Date: Sun, 10 Aug 2025 18:01:45 -0500 Subject: [PATCH 04/15] NTP polling --- test/source/scwx/network/ntp_client.test.cpp | 22 +- wxdata/include/scwx/network/ntp_client.hpp | 15 +- wxdata/source/scwx/network/ntp_client.cpp | 276 +++++++++++++++++-- 3 files changed, 281 insertions(+), 32 deletions(-) diff --git a/test/source/scwx/network/ntp_client.test.cpp b/test/source/scwx/network/ntp_client.test.cpp index cebd8cc2..bdfcb4ae 100644 --- a/test/source/scwx/network/ntp_client.test.cpp +++ b/test/source/scwx/network/ntp_client.test.cpp @@ -11,10 +11,24 @@ TEST(NtpClient, Poll) { NtpClient client {}; - client.Open("time.nist.gov", "123"); - //client.Open("pool.ntp.org", "123"); - //client.Open("time.windows.com", "123"); - client.Poll(); + const std::string firstServer = client.RotateServer(); + std::string currentServer = firstServer; + std::string lastServer = firstServer; + bool error = false; + + do + { + client.RunOnce(); + error = client.error(); + + EXPECT_EQ(error, false); + + // Loop until the current server repeats the first server, or fails to + // rotate + lastServer = currentServer; + currentServer = client.RotateServer(); + } while (currentServer != firstServer && currentServer != lastServer && + !error); } } // namespace network diff --git a/wxdata/include/scwx/network/ntp_client.hpp b/wxdata/include/scwx/network/ntp_client.hpp index 55ef4204..6a0a78b7 100644 --- a/wxdata/include/scwx/network/ntp_client.hpp +++ b/wxdata/include/scwx/network/ntp_client.hpp @@ -1,6 +1,8 @@ #pragma once +#include #include +#include #include namespace scwx::network @@ -21,8 +23,17 @@ public: NtpClient(NtpClient&&) noexcept; NtpClient& operator=(NtpClient&&) noexcept; - void Open(std::string_view host, std::string_view service); - void Poll(); + bool error(); + std::chrono::system_clock::duration time_offset() const; + + void Start(); + void Open(std::string_view host, std::string_view service); + void OpenCurrentServer(); + void Poll(); + std::string RotateServer(); + void RunOnce(); + + static std::shared_ptr Instance(); private: class Impl; diff --git a/wxdata/source/scwx/network/ntp_client.cpp b/wxdata/source/scwx/network/ntp_client.cpp index a20540c4..89aba5d0 100644 --- a/wxdata/source/scwx/network/ntp_client.cpp +++ b/wxdata/source/scwx/network/ntp_client.cpp @@ -4,6 +4,7 @@ #include #include +#include #include #include #include @@ -16,6 +17,12 @@ static const auto logger_ = scwx::util::Logger::Create(logPrefix_); static constexpr std::size_t kReceiveBufferSize_ {48u}; +// Reasonable min/max values for polling intervals. We don't want to poll too +// quickly and upset the server, but we don't want to poll too slowly in the +// event of a time jump. +static constexpr std::uint32_t kMinPollInterval_ = 6u; // 2^6 = 64 seconds +static constexpr std::uint32_t kMaxPollInterval_ = 9u; // 2^9 = 512 seconds + class NtpTimestamp { public: @@ -97,28 +104,42 @@ public: Impl(Impl&&) = delete; Impl& operator=(Impl&&) = delete; - void Open(std::string_view host, std::string_view service); - void Poll(); - void ReceivePacket(std::size_t length); + void Open(std::string_view host, std::string_view service); + void OpenCurrentServer(); + void Poll(); + void ReceivePacket(std::size_t length); + std::string RotateServer(); + void Run(); + void RunOnce(); boost::asio::thread_pool threadPool_ {2u}; - bool enabled_; + boost::asio::steady_timer pollTimer_ {threadPool_}; + std::uint32_t pollInterval_ {kMinPollInterval_}; + + bool enabled_ {true}; + bool error_ {false}; + bool disableServer_ {false}; + bool rotateServer_ {false}; types::ntp::NtpPacket transmitPacket_ {}; - boost::asio::ip::udp::socket socket_; + boost::asio::ip::udp::socket socket_ {threadPool_}; std::optional serverEndpoint_ {}; std::array receiveBuffer_ {}; std::chrono::system_clock::duration timeOffset_ {}; - std::vector serverList_ {"time.nist.gov", - "time.cloudflare.com", - "ntp.pool.org", - "time.aws.com", - "time.windows.com", - "time.apple.com"}; + const std::vector serverList_ {"time.nist.gov", + "time.cloudflare.com", + "pool.ntp.org", + "time.aws.com", + "time.windows.com", + "time.apple.com"}; + std::vector disabledServers_ {}; + + std::vector::const_iterator currentServer_ = + serverList_.begin(); }; NtpClient::NtpClient() : p(std::make_unique()) {} @@ -127,17 +148,7 @@ NtpClient::~NtpClient() = default; NtpClient::NtpClient(NtpClient&&) noexcept = default; NtpClient& NtpClient::operator=(NtpClient&&) noexcept = default; -void NtpClient::Open(std::string_view host, std::string_view service) -{ - p->Open(host, service); -} - -void NtpClient::Poll() -{ - p->Poll(); -} - -NtpClient::Impl::Impl() : socket_ {threadPool_} +NtpClient::Impl::Impl() { using namespace std::chrono_literals; @@ -145,8 +156,8 @@ NtpClient::Impl::Impl() : socket_ {threadPool_} std::chrono::floor(std::chrono::system_clock::now()); // The NTP timestamp will overflow in 2036. Overflow is handled in such a way - // that should work until 2106. Additional handling for subsequent eras is - // required. + // that dates prior to 1970 result in a Unix timestamp after 2036. Additional + // handling for the year 2106 and subsequent eras is required. static constexpr auto kMaxYear_ = 2106y; enabled_ = now < kMaxYear_ / 1 / 1; @@ -160,6 +171,51 @@ NtpClient::Impl::~Impl() threadPool_.join(); } +bool NtpClient::error() +{ + bool returnValue = p->error_; + p->error_ = false; + return returnValue; +} + +std::chrono::system_clock::duration NtpClient::time_offset() const +{ + return p->timeOffset_; +} + +void NtpClient::Start() +{ + if (p->enabled_) + { + boost::asio::post(p->threadPool_, [this]() { p->Run(); }); + } +} + +void NtpClient::Open(std::string_view host, std::string_view service) +{ + p->Open(host, service); +} + +void NtpClient::OpenCurrentServer() +{ + p->OpenCurrentServer(); +} + +void NtpClient::Poll() +{ + p->Poll(); +} + +std::string NtpClient::RotateServer() +{ + return p->RotateServer(); +} + +void NtpClient::RunOnce() +{ + p->RunOnce(); +} + void NtpClient::Impl::Open(std::string_view host, std::string_view service) { boost::asio::ip::udp::resolver resolver(threadPool_); @@ -176,9 +232,15 @@ void NtpClient::Impl::Open(std::string_view host, std::string_view service) { serverEndpoint_ = std::nullopt; logger_->warn("Could not resolve host {}: {}", host, ec.message()); + rotateServer_ = true; } } +void NtpClient::Impl::OpenCurrentServer() +{ + Open(*currentServer_, "123"); +} + void NtpClient::Impl::Poll() { using namespace std::chrono_literals; @@ -215,6 +277,7 @@ void NtpClient::Impl::Poll() case std::future_status::deferred: logger_->warn("Timeout waiting for NTP response"); socket_.cancel(); + error_ = true; break; } } @@ -242,17 +305,29 @@ void NtpClient::Impl::ReceivePacket(std::size_t length) if (kod == "DENY" || kod == "RSTR") { - // TODO // The client MUST demobilize any associations to that server and // stop sending packets to that server + disableServer_ = true; } else if (kod == "RATE") { - // TODO // The client MUST immediately reduce its polling interval to that // server and continue to reduce it each time it receives a RATE // kiss code + if (pollInterval_ < kMaxPollInterval_) + { + ++pollInterval_; + } + else + { + // The server wants us to reduce the polling interval lower than + // what we deem useful. Move to the next server. + rotateServer_ = true; + } } + + // Consider a KoD packet an error + error_ = true; } else { @@ -276,12 +351,161 @@ void NtpClient::Impl::ReceivePacket(std::size_t length) timeOffset_ = ((t1 - t0) + (t2 - t3)) / 2; logger_->debug("Time offset updated: {:%jd %T}", timeOffset_); + + // TODO: Signal } } else { logger_->warn("Received too few bytes: {}", length); + error_ = true; } } +std::string NtpClient::Impl::RotateServer() +{ + socket_.close(); + + bool newServerFound = false; + + // Save the current server + const auto oldServer = currentServer_; + + while (!newServerFound) + { + // Increment the current server + ++currentServer_; + + // If we are at the end of the list, start over at the beginning + if (currentServer_ == serverList_.end()) + { + currentServer_ = serverList_.begin(); + } + + // If we have reached the end of the list, give up + if (currentServer_ == oldServer) + { + enabled_ = false; + break; + } + + // If the current server is disabled, continue searching + while (std::find(disabledServers_.cbegin(), + disabledServers_.cend(), + *currentServer_) != disabledServers_.cend()) + { + continue; + } + + // A new server has been found + newServerFound = true; + } + + pollInterval_ = kMinPollInterval_; + rotateServer_ = false; + + return *currentServer_; +} + +void NtpClient::Impl::Run() +{ + RunOnce(); + + if (enabled_) + { + std::chrono::seconds pollIntervalSeconds {1u << pollInterval_}; + pollTimer_.expires_after(pollIntervalSeconds); + pollTimer_.async_wait( + [this](const boost::system::error_code& e) + { + if (e == boost::asio::error::operation_aborted) + { + logger_->debug("Poll timer cancelled"); + } + else if (e != boost::system::errc::success) + { + logger_->warn("Poll timer error: {}", e.message()); + } + else + { + try + { + Run(); + } + catch (const std::exception& ex) + { + logger_->error(ex.what()); + } + } + }); + } +} + +void NtpClient::Impl::RunOnce() +{ + if (disableServer_) + { + // Disable the current server + disabledServers_.push_back(*currentServer_); + + // Disable the NTP client if all servers are disabled + enabled_ = disabledServers_.size() == serverList_.size(); + + if (!enabled_) + { + error_ = true; + } + + disableServer_ = false; + rotateServer_ = enabled_; + } + + if (!enabled_ && socket_.is_open()) + { + // Sockets should be closed if the client is disabled + socket_.close(); + } + + if (rotateServer_) + { + // Rotate the server if requested + RotateServer(); + } + + if (enabled_ && !socket_.is_open()) + { + // Open the current server if it is not open + OpenCurrentServer(); + } + + if (socket_.is_open()) + { + // Send an NTP message to determine the current time offset + Poll(); + } + else if (enabled_) + { + // Did not poll this frame + error_ = true; + } +} + +std::shared_ptr NtpClient::Instance() +{ + static std::weak_ptr ntpClientReference_ {}; + static std::mutex instanceMutex_ {}; + + std::unique_lock lock(instanceMutex_); + + std::shared_ptr ntpClient = ntpClientReference_.lock(); + + if (ntpClient == nullptr) + { + ntpClient = std::make_shared(); + ntpClientReference_ = ntpClient; + } + + return ntpClient; +} + } // namespace scwx::network From 63e6ba770939953ececed6ecbb4ad9e4bfbdd91a Mon Sep 17 00:00:00 2001 From: Dan Paulat Date: Fri, 22 Aug 2025 19:25:01 -0500 Subject: [PATCH 05/15] Update initialization order to ensure initial log entries make it to log file --- scwx-qt/source/scwx/qt/main/main.cpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/scwx-qt/source/scwx/qt/main/main.cpp b/scwx-qt/source/scwx/qt/main/main.cpp index 13fb3a05..b255c7b6 100644 --- a/scwx-qt/source/scwx/qt/main/main.cpp +++ b/scwx-qt/source/scwx/qt/main/main.cpp @@ -53,10 +53,19 @@ int main(int argc, char* argv[]) args.push_back(argv[i]); } + if (!scwx::util::GetEnvironment("SCWX_TEST").empty()) + { + QStandardPaths::setTestModeEnabled(true); + } + // Initialize logger auto& logManager = scwx::qt::manager::LogManager::Instance(); logManager.Initialize(); + QCoreApplication::setApplicationName("Supercell Wx"); + + logManager.InitializeLogFile(); + logger_->info("Supercell Wx v{}.{} ({})", scwx::qt::main::kVersionString_, scwx::qt::main::kBuildNumber_, @@ -66,7 +75,6 @@ int main(int argc, char* argv[]) QApplication a(argc, argv); - QCoreApplication::setApplicationName("Supercell Wx"); scwx::network::cpr::SetUserAgent( fmt::format("SupercellWx/{}", scwx::qt::main::kVersionString_)); @@ -77,11 +85,6 @@ int main(int argc, char* argv[]) QCoreApplication::installTranslator(&translator); } - if (!scwx::util::GetEnvironment("SCWX_TEST").empty()) - { - QStandardPaths::setTestModeEnabled(true); - } - // Test to see if scwx was run with high privilege scwx::qt::main::PrivilegeChecker privilegeChecker; if (privilegeChecker.pre_settings_check()) @@ -116,7 +119,6 @@ int main(int argc, char* argv[]) Aws::InitAPI(awsSdkOptions); // Initialize application - logManager.InitializeLogFile(); scwx::qt::config::RadarSite::Initialize(); scwx::qt::config::CountyDatabase::Initialize(); scwx::qt::manager::SettingsManager::Instance().Initialize(); From 88d968a533ab0731f292d823c22cc58ec39a9409 Mon Sep 17 00:00:00 2001 From: Dan Paulat Date: Fri, 22 Aug 2025 20:04:21 -0500 Subject: [PATCH 06/15] Run NTP client in the background --- scwx-qt/scwx-qt.cmake | 2 ++ scwx-qt/source/scwx/qt/main/main.cpp | 3 ++ .../source/scwx/qt/manager/task_manager.cpp | 29 +++++++++++++++++++ .../source/scwx/qt/manager/task_manager.hpp | 9 ++++++ wxdata/include/scwx/network/ntp_client.hpp | 4 ++- wxdata/source/scwx/network/ntp_client.cpp | 8 +++++ 6 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 scwx-qt/source/scwx/qt/manager/task_manager.cpp create mode 100644 scwx-qt/source/scwx/qt/manager/task_manager.hpp diff --git a/scwx-qt/scwx-qt.cmake b/scwx-qt/scwx-qt.cmake index 194601e9..ecd26ef9 100644 --- a/scwx-qt/scwx-qt.cmake +++ b/scwx-qt/scwx-qt.cmake @@ -109,6 +109,7 @@ set(HDR_MANAGER source/scwx/qt/manager/alert_manager.hpp source/scwx/qt/manager/radar_product_manager_notifier.hpp source/scwx/qt/manager/resource_manager.hpp source/scwx/qt/manager/settings_manager.hpp + source/scwx/qt/manager/task_manager.hpp source/scwx/qt/manager/text_event_manager.hpp source/scwx/qt/manager/thread_manager.hpp source/scwx/qt/manager/timeline_manager.hpp @@ -126,6 +127,7 @@ set(SRC_MANAGER source/scwx/qt/manager/alert_manager.cpp source/scwx/qt/manager/radar_product_manager_notifier.cpp source/scwx/qt/manager/resource_manager.cpp source/scwx/qt/manager/settings_manager.cpp + source/scwx/qt/manager/task_manager.cpp source/scwx/qt/manager/text_event_manager.cpp source/scwx/qt/manager/thread_manager.cpp source/scwx/qt/manager/timeline_manager.cpp diff --git a/scwx-qt/source/scwx/qt/main/main.cpp b/scwx-qt/source/scwx/qt/main/main.cpp index b255c7b6..a2416cec 100644 --- a/scwx-qt/source/scwx/qt/main/main.cpp +++ b/scwx-qt/source/scwx/qt/main/main.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -121,6 +122,7 @@ int main(int argc, char* argv[]) // Initialize application scwx::qt::config::RadarSite::Initialize(); scwx::qt::config::CountyDatabase::Initialize(); + scwx::qt::manager::TaskManager::Initialize(); scwx::qt::manager::SettingsManager::Instance().Initialize(); scwx::qt::manager::ResourceManager::Initialize(); @@ -181,6 +183,7 @@ int main(int argc, char* argv[]) // Shutdown application scwx::qt::manager::ResourceManager::Shutdown(); scwx::qt::manager::SettingsManager::Instance().Shutdown(); + scwx::qt::manager::TaskManager::Shutdown(); // Shutdown AWS SDK Aws::ShutdownAPI(awsSdkOptions); diff --git a/scwx-qt/source/scwx/qt/manager/task_manager.cpp b/scwx-qt/source/scwx/qt/manager/task_manager.cpp new file mode 100644 index 00000000..130318f5 --- /dev/null +++ b/scwx-qt/source/scwx/qt/manager/task_manager.cpp @@ -0,0 +1,29 @@ +#include +#include +#include + +namespace scwx::qt::manager::TaskManager +{ + +static const std::string logPrefix_ = "scwx::qt::manager::task_manager"; +static const auto logger_ = scwx::util::Logger::Create(logPrefix_); + +static std::shared_ptr ntpClient_ {}; + +void Initialize() +{ + logger_->debug("Initialize"); + + ntpClient_ = network::NtpClient::Instance(); + + ntpClient_->Start(); +} + +void Shutdown() +{ + logger_->debug("Shutdown"); + + ntpClient_->Stop(); +} + +} // namespace scwx::qt::manager::TaskManager diff --git a/scwx-qt/source/scwx/qt/manager/task_manager.hpp b/scwx-qt/source/scwx/qt/manager/task_manager.hpp new file mode 100644 index 00000000..bb50fb3b --- /dev/null +++ b/scwx-qt/source/scwx/qt/manager/task_manager.hpp @@ -0,0 +1,9 @@ +#pragma once + +namespace scwx::qt::manager::TaskManager +{ + +void Initialize(); +void Shutdown(); + +} // namespace scwx::qt::manager::TaskManager diff --git a/wxdata/include/scwx/network/ntp_client.hpp b/wxdata/include/scwx/network/ntp_client.hpp index 6a0a78b7..6462a6e9 100644 --- a/wxdata/include/scwx/network/ntp_client.hpp +++ b/wxdata/include/scwx/network/ntp_client.hpp @@ -26,7 +26,9 @@ public: bool error(); std::chrono::system_clock::duration time_offset() const; - void Start(); + void Start(); + void Stop(); + void Open(std::string_view host, std::string_view service); void OpenCurrentServer(); void Poll(); diff --git a/wxdata/source/scwx/network/ntp_client.cpp b/wxdata/source/scwx/network/ntp_client.cpp index 89aba5d0..45c0ad74 100644 --- a/wxdata/source/scwx/network/ntp_client.cpp +++ b/wxdata/source/scwx/network/ntp_client.cpp @@ -191,6 +191,14 @@ void NtpClient::Start() } } +void NtpClient::Stop() +{ + p->enabled_ = false; + p->socket_.cancel(); + p->pollTimer_.cancel(); + p->threadPool_.join(); +} + void NtpClient::Open(std::string_view host, std::string_view service) { p->Open(host, service); From c76c9b57ed93f841daa437a04c0c8aeaab000216 Mon Sep 17 00:00:00 2001 From: Dan Paulat Date: Fri, 22 Aug 2025 22:28:55 -0500 Subject: [PATCH 07/15] Wait for an initial offset prior to proceeding with initialization --- .../source/scwx/qt/manager/task_manager.cpp | 1 + wxdata/include/scwx/network/ntp_client.hpp | 2 + wxdata/source/scwx/network/ntp_client.cpp | 49 +++++++++++++++++-- 3 files changed, 49 insertions(+), 3 deletions(-) diff --git a/scwx-qt/source/scwx/qt/manager/task_manager.cpp b/scwx-qt/source/scwx/qt/manager/task_manager.cpp index 130318f5..66b7285a 100644 --- a/scwx-qt/source/scwx/qt/manager/task_manager.cpp +++ b/scwx-qt/source/scwx/qt/manager/task_manager.cpp @@ -17,6 +17,7 @@ void Initialize() ntpClient_ = network::NtpClient::Instance(); ntpClient_->Start(); + ntpClient_->WaitForInitialOffset(); } void Shutdown() diff --git a/wxdata/include/scwx/network/ntp_client.hpp b/wxdata/include/scwx/network/ntp_client.hpp index 6462a6e9..beb4be21 100644 --- a/wxdata/include/scwx/network/ntp_client.hpp +++ b/wxdata/include/scwx/network/ntp_client.hpp @@ -35,6 +35,8 @@ public: std::string RotateServer(); void RunOnce(); + void WaitForInitialOffset(); + static std::shared_ptr Instance(); private: diff --git a/wxdata/source/scwx/network/ntp_client.cpp b/wxdata/source/scwx/network/ntp_client.cpp index 45c0ad74..0a340f87 100644 --- a/wxdata/source/scwx/network/ntp_client.cpp +++ b/wxdata/source/scwx/network/ntp_client.cpp @@ -3,6 +3,9 @@ #include #include +#include +#include + #include #include #include @@ -112,6 +115,8 @@ public: void Run(); void RunOnce(); + void FinishInitialization(); + boost::asio::thread_pool threadPool_ {2u}; boost::asio::steady_timer pollTimer_ {threadPool_}; @@ -122,6 +127,10 @@ public: bool disableServer_ {false}; bool rotateServer_ {false}; + std::mutex initializationMutex_ {}; + std::condition_variable initializationCondition_ {}; + std::atomic initialized_ {false}; + types::ntp::NtpPacket transmitPacket_ {}; boost::asio::ip::udp::socket socket_ {threadPool_}; @@ -164,6 +173,14 @@ NtpClient::Impl::Impl() transmitPacket_.fields.vn = 3; // Version transmitPacket_.fields.mode = 3; // Client (3) + + // If the NTP client is enabled, wait until the first refresh to consider + // "initialized". Otherwise, mark as initialized immediately to prevent a + // deadlock. + if (!enabled_) + { + initialized_ = true; + } } NtpClient::Impl::~Impl() @@ -253,7 +270,7 @@ void NtpClient::Impl::Poll() { using namespace std::chrono_literals; - static constexpr auto kTimeout_ = 15s; + static constexpr auto kTimeout_ = 5s; try { @@ -359,8 +376,6 @@ void NtpClient::Impl::ReceivePacket(std::size_t length) timeOffset_ = ((t1 - t0) + (t2 - t3)) / 2; logger_->debug("Time offset updated: {:%jd %T}", timeOffset_); - - // TODO: Signal } } else @@ -496,6 +511,34 @@ void NtpClient::Impl::RunOnce() // Did not poll this frame error_ = true; } + + FinishInitialization(); +} + +void NtpClient::Impl::FinishInitialization() +{ + if (!initialized_) + { + // Set initialized to true + std::unique_lock lock(initializationMutex_); + initialized_ = true; + lock.unlock(); + + // Notify any threads waiting for initialization + initializationCondition_.notify_all(); + } +} + +void NtpClient::WaitForInitialOffset() +{ + std::unique_lock lock(p->initializationMutex_); + + // While not yet initialized + while (!p->initialized_) + { + // Wait for initialization + p->initializationCondition_.wait(lock); + } } std::shared_ptr NtpClient::Instance() From 719142ca12c25febead8edc5a23f5aa5fd3cfe0f Mon Sep 17 00:00:00 2001 From: Dan Paulat Date: Fri, 22 Aug 2025 22:29:25 -0500 Subject: [PATCH 08/15] UTC in the main window should use the NTP time offset --- scwx-qt/source/scwx/qt/main/main_window.cpp | 4 +-- wxdata/include/scwx/util/time.hpp | 23 ++++++++++++++---- wxdata/source/scwx/util/time.cpp | 27 +++++++++++++++++++-- 3 files changed, 45 insertions(+), 9 deletions(-) diff --git a/scwx-qt/source/scwx/qt/main/main_window.cpp b/scwx-qt/source/scwx/qt/main/main_window.cpp index 4ab5d56b..4fa2f28a 100644 --- a/scwx-qt/source/scwx/qt/main/main_window.cpp +++ b/scwx-qt/source/scwx/qt/main/main_window.cpp @@ -1300,8 +1300,8 @@ void MainWindowImpl::ConnectOtherSignals() this, [this]() { - timeLabel_->setText(QString::fromStdString( - util::TimeString(std::chrono::system_clock::now()))); + timeLabel_->setText( + QString::fromStdString(util::TimeString(util::time::now()))); timeLabel_->setVisible(true); }); clockTimer_.start(1000); diff --git a/wxdata/include/scwx/util/time.hpp b/wxdata/include/scwx/util/time.hpp index e31e8ca9..fe052756 100644 --- a/wxdata/include/scwx/util/time.hpp +++ b/wxdata/include/scwx/util/time.hpp @@ -10,9 +10,7 @@ # include #endif -namespace scwx -{ -namespace util +namespace scwx::util::time { #if (__cpp_lib_chrono >= 201907L) @@ -34,6 +32,9 @@ typedef scwx::util:: ClockFormat GetClockFormat(const std::string& name); const std::string& GetClockFormatName(ClockFormat clockFormat); +template +std::chrono::time_point now(); + std::chrono::system_clock::time_point TimePoint(uint32_t modifiedJulianDate, uint32_t milliseconds); @@ -46,5 +47,17 @@ template std::optional> TryParseDateTime(const std::string& dateTimeFormat, const std::string& str); -} // namespace util -} // namespace scwx +} // namespace scwx::util::time + +namespace scwx::util +{ +// Add types and functions to scwx::util for compatibility +using time::ClockFormat; +using time::ClockFormatIterator; +using time::GetClockFormat; +using time::GetClockFormatName; +using time::time_zone; +using time::TimePoint; +using time::TimeString; +using time::TryParseDateTime; +} // namespace scwx::util diff --git a/wxdata/source/scwx/util/time.cpp b/wxdata/source/scwx/util/time.cpp index 563aea1b..1a687d32 100644 --- a/wxdata/source/scwx/util/time.cpp +++ b/wxdata/source/scwx/util/time.cpp @@ -8,6 +8,7 @@ # define __cpp_lib_format 202110L #endif +#include #include #include #include @@ -21,7 +22,7 @@ # include #endif -namespace scwx::util +namespace scwx::util::time { static const std::string logPrefix_ = "scwx::util::time"; @@ -32,6 +33,8 @@ static const std::unordered_map clockFormatName_ { {ClockFormat::_24Hour, "24-hour"}, {ClockFormat::Unknown, "?"}}; +static std::shared_ptr ntpClient_ {nullptr}; + SCWX_GET_ENUM(ClockFormat, GetClockFormat, clockFormatName_) const std::string& GetClockFormatName(ClockFormat clockFormat) @@ -39,6 +42,26 @@ const std::string& GetClockFormatName(ClockFormat clockFormat) return clockFormatName_.at(clockFormat); } +template +std::chrono::time_point now() +{ + if (ntpClient_ == nullptr) + { + ntpClient_ = network::NtpClient::Instance(); + } + + if (ntpClient_ != nullptr) + { + return Clock::now() + ntpClient_->time_offset(); + } + else + { + return Clock::now(); + } +} + +template std::chrono::time_point now(); + std::chrono::system_clock::time_point TimePoint(uint32_t modifiedJulianDate, uint32_t milliseconds) { @@ -153,4 +176,4 @@ template std::optional> TryParseDateTime(const std::string& dateTimeFormat, const std::string& str); -} // namespace scwx::util +} // namespace scwx::util::time From e0a4dee72bde110ac3860a57a8db11985263d575 Mon Sep 17 00:00:00 2001 From: Dan Paulat Date: Fri, 22 Aug 2025 22:57:03 -0500 Subject: [PATCH 09/15] Don't flag non-const global variables --- .clang-tidy | 1 + 1 file changed, 1 insertion(+) diff --git a/.clang-tidy b/.clang-tidy index c93d61fe..0ca21696 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -8,6 +8,7 @@ Checks: - 'performance-*' - '-bugprone-easily-swappable-parameters' - '-cppcoreguidelines-avoid-do-while' + - '-cppcoreguidelines-avoid-non-const-global-variables' - '-cppcoreguidelines-pro-type-reinterpret-cast' - '-cppcoreguidelines-pro-type-union-access' - '-misc-include-cleaner' From bd27d0e56246dcfb203bd7be36d4a7f784beb6a1 Mon Sep 17 00:00:00 2001 From: Dan Paulat Date: Fri, 22 Aug 2025 23:42:08 -0500 Subject: [PATCH 10/15] Update most instances of current time to use a time offset --- scwx-qt/source/scwx/qt/gl/draw/geo_icons.cpp | 5 +++-- scwx-qt/source/scwx/qt/gl/draw/geo_lines.cpp | 5 +++-- scwx-qt/source/scwx/qt/gl/draw/placefile_icons.cpp | 5 +++-- scwx-qt/source/scwx/qt/gl/draw/placefile_images.cpp | 3 ++- scwx-qt/source/scwx/qt/gl/draw/placefile_lines.cpp | 5 +++-- .../source/scwx/qt/gl/draw/placefile_polygons.cpp | 3 ++- scwx-qt/source/scwx/qt/gl/draw/placefile_text.cpp | 3 ++- .../source/scwx/qt/gl/draw/placefile_triangles.cpp | 3 ++- scwx-qt/source/scwx/qt/manager/alert_manager.cpp | 7 ++++--- .../source/scwx/qt/manager/radar_product_manager.cpp | 9 +++++---- .../source/scwx/qt/manager/text_event_manager.cpp | 2 +- scwx-qt/source/scwx/qt/manager/timeline_manager.cpp | 12 ++++++------ scwx-qt/source/scwx/qt/map/alert_layer.cpp | 4 ++-- scwx-qt/source/scwx/qt/model/alert_proxy_model.cpp | 3 ++- scwx-qt/source/scwx/qt/ui/animation_dock_widget.cpp | 4 ++-- scwx-qt/source/scwx/qt/view/overlay_product_view.cpp | 2 +- .../provider/aws_level2_chunks_data_provider.cpp | 5 ++--- .../scwx/provider/aws_nexrad_data_provider.cpp | 4 ++-- wxdata/source/scwx/provider/warnings_provider.cpp | 2 +- 19 files changed, 48 insertions(+), 38 deletions(-) diff --git a/scwx-qt/source/scwx/qt/gl/draw/geo_icons.cpp b/scwx-qt/source/scwx/qt/gl/draw/geo_icons.cpp index ba3162e9..05cc26a5 100644 --- a/scwx-qt/source/scwx/qt/gl/draw/geo_icons.cpp +++ b/scwx-qt/source/scwx/qt/gl/draw/geo_icons.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include @@ -313,7 +314,7 @@ void GeoIcons::Render(const QMapLibre::CustomLayerRenderParameters& params, // Selected time std::chrono::system_clock::time_point selectedTime = (p->selectedTime_ == std::chrono::system_clock::time_point {}) ? - std::chrono::system_clock::now() : + scwx::util::time::now() : p->selectedTime_; glUniform1i( p->uSelectedTimeLocation_, @@ -930,7 +931,7 @@ bool GeoIcons::RunMousePicking( // If no time has been selected, use the current time std::chrono::system_clock::time_point selectedTime = (p->selectedTime_ == std::chrono::system_clock::time_point {}) ? - std::chrono::system_clock::now() : + scwx::util::time::now() : p->selectedTime_; // For each pickable icon diff --git a/scwx-qt/source/scwx/qt/gl/draw/geo_lines.cpp b/scwx-qt/source/scwx/qt/gl/draw/geo_lines.cpp index aa376b00..e35cca4f 100644 --- a/scwx-qt/source/scwx/qt/gl/draw/geo_lines.cpp +++ b/scwx-qt/source/scwx/qt/gl/draw/geo_lines.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include @@ -284,7 +285,7 @@ void GeoLines::Render(const QMapLibre::CustomLayerRenderParameters& params) // Selected time std::chrono::system_clock::time_point selectedTime = (p->selectedTime_ == std::chrono::system_clock::time_point {}) ? - std::chrono::system_clock::now() : + scwx::util::time::now() : p->selectedTime_; glUniform1i( p->uSelectedTimeLocation_, @@ -723,7 +724,7 @@ bool GeoLines::RunMousePicking( // If no time has been selected, use the current time std::chrono::system_clock::time_point selectedTime = (p->selectedTime_ == std::chrono::system_clock::time_point {}) ? - std::chrono::system_clock::now() : + scwx::util::time::now() : p->selectedTime_; // For each pickable line diff --git a/scwx-qt/source/scwx/qt/gl/draw/placefile_icons.cpp b/scwx-qt/source/scwx/qt/gl/draw/placefile_icons.cpp index 9ce7dc8f..21dd25c2 100644 --- a/scwx-qt/source/scwx/qt/gl/draw/placefile_icons.cpp +++ b/scwx-qt/source/scwx/qt/gl/draw/placefile_icons.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include @@ -295,7 +296,7 @@ void PlacefileIcons::Render( // Selected time std::chrono::system_clock::time_point selectedTime = (p->selectedTime_ == std::chrono::system_clock::time_point {}) ? - std::chrono::system_clock::now() : + scwx::util::time::now() : p->selectedTime_; glUniform1i( p->uSelectedTimeLocation_, @@ -720,7 +721,7 @@ bool PlacefileIcons::RunMousePicking( // If no time has been selected, use the current time std::chrono::system_clock::time_point selectedTime = (p->selectedTime_ == std::chrono::system_clock::time_point {}) ? - std::chrono::system_clock::now() : + scwx::util::time::now() : p->selectedTime_; // For each pickable icon diff --git a/scwx-qt/source/scwx/qt/gl/draw/placefile_images.cpp b/scwx-qt/source/scwx/qt/gl/draw/placefile_images.cpp index 16d4d19b..b14a2723 100644 --- a/scwx-qt/source/scwx/qt/gl/draw/placefile_images.cpp +++ b/scwx-qt/source/scwx/qt/gl/draw/placefile_images.cpp @@ -2,6 +2,7 @@ #include #include #include +#include #include #include @@ -264,7 +265,7 @@ void PlacefileImages::Render( // Selected time std::chrono::system_clock::time_point selectedTime = (p->selectedTime_ == std::chrono::system_clock::time_point {}) ? - std::chrono::system_clock::now() : + scwx::util::time::now() : p->selectedTime_; glUniform1i( p->uSelectedTimeLocation_, diff --git a/scwx-qt/source/scwx/qt/gl/draw/placefile_lines.cpp b/scwx-qt/source/scwx/qt/gl/draw/placefile_lines.cpp index 134d9c6b..b908389c 100644 --- a/scwx-qt/source/scwx/qt/gl/draw/placefile_lines.cpp +++ b/scwx-qt/source/scwx/qt/gl/draw/placefile_lines.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include @@ -248,7 +249,7 @@ void PlacefileLines::Render( // Selected time std::chrono::system_clock::time_point selectedTime = (p->selectedTime_ == std::chrono::system_clock::time_point {}) ? - std::chrono::system_clock::now() : + scwx::util::time::now() : p->selectedTime_; glUniform1i( p->uSelectedTimeLocation_, @@ -526,7 +527,7 @@ bool PlacefileLines::RunMousePicking( // If no time has been selected, use the current time std::chrono::system_clock::time_point selectedTime = (p->selectedTime_ == std::chrono::system_clock::time_point {}) ? - std::chrono::system_clock::now() : + scwx::util::time::now() : p->selectedTime_; // For each pickable line diff --git a/scwx-qt/source/scwx/qt/gl/draw/placefile_polygons.cpp b/scwx-qt/source/scwx/qt/gl/draw/placefile_polygons.cpp index 646739ef..a30991f9 100644 --- a/scwx-qt/source/scwx/qt/gl/draw/placefile_polygons.cpp +++ b/scwx-qt/source/scwx/qt/gl/draw/placefile_polygons.cpp @@ -1,6 +1,7 @@ #include #include #include +#include #include @@ -259,7 +260,7 @@ void PlacefilePolygons::Render( // Selected time std::chrono::system_clock::time_point selectedTime = (p->selectedTime_ == std::chrono::system_clock::time_point {}) ? - std::chrono::system_clock::now() : + scwx::util::time::now() : p->selectedTime_; glUniform1i( p->uSelectedTimeLocation_, diff --git a/scwx-qt/source/scwx/qt/gl/draw/placefile_text.cpp b/scwx-qt/source/scwx/qt/gl/draw/placefile_text.cpp index 832a1292..93aa4461 100644 --- a/scwx-qt/source/scwx/qt/gl/draw/placefile_text.cpp +++ b/scwx-qt/source/scwx/qt/gl/draw/placefile_text.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include @@ -127,7 +128,7 @@ void PlacefileText::Impl::RenderTextDrawItem( // If no time has been selected, use the current time std::chrono::system_clock::time_point selectedTime = (selectedTime_ == std::chrono::system_clock::time_point {}) ? - std::chrono::system_clock::now() : + scwx::util::time::now() : selectedTime_; const bool thresholdMet = diff --git a/scwx-qt/source/scwx/qt/gl/draw/placefile_triangles.cpp b/scwx-qt/source/scwx/qt/gl/draw/placefile_triangles.cpp index 5ad54bc7..222713ae 100644 --- a/scwx-qt/source/scwx/qt/gl/draw/placefile_triangles.cpp +++ b/scwx-qt/source/scwx/qt/gl/draw/placefile_triangles.cpp @@ -1,6 +1,7 @@ #include #include #include +#include #include @@ -203,7 +204,7 @@ void PlacefileTriangles::Render( // Selected time std::chrono::system_clock::time_point selectedTime = (p->selectedTime_ == std::chrono::system_clock::time_point {}) ? - std::chrono::system_clock::now() : + scwx::util::time::now() : p->selectedTime_; glUniform1i( p->uSelectedTimeLocation_, diff --git a/scwx-qt/source/scwx/qt/manager/alert_manager.cpp b/scwx-qt/source/scwx/qt/manager/alert_manager.cpp index 748e0943..41e74d7a 100644 --- a/scwx-qt/source/scwx/qt/manager/alert_manager.cpp +++ b/scwx-qt/source/scwx/qt/manager/alert_manager.cpp @@ -2,12 +2,13 @@ #include #include #include +#include #include +#include #include #include #include -#include -#include +#include #include #include @@ -172,7 +173,7 @@ void AlertManager::Impl::HandleAlert(const types::TextEventKey& key, // If the event has ended or is inactive, or if the alert is not enabled, // skip it - if (eventEnd < std::chrono::system_clock::now() || !alertActive || + if (eventEnd < scwx::util::time::now() || !alertActive || !audioSettings.alert_enabled(phenomenon).GetValue()) { continue; diff --git a/scwx-qt/source/scwx/qt/manager/radar_product_manager.cpp b/scwx-qt/source/scwx/qt/manager/radar_product_manager.cpp index 13787496..bda6e232 100644 --- a/scwx-qt/source/scwx/qt/manager/radar_product_manager.cpp +++ b/scwx-qt/source/scwx/qt/manager/radar_product_manager.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -821,7 +822,7 @@ void RadarProductManagerImpl::RefreshDataSync( auto latestTime = providerManager->provider_->FindLatestTime(); auto updatePeriod = providerManager->provider_->update_period(); auto lastModified = providerManager->provider_->last_modified(); - auto sinceLastModified = std::chrono::system_clock::now() - lastModified; + auto sinceLastModified = scwx::util::time::now() - lastModified; // For the default interval, assume products are updated at a // constant rate. Expect the next product at a time based on the @@ -939,7 +940,7 @@ RadarProductManager::GetActiveVolumeTimes( [&](const auto& date) { // Don't query for a time point in the future - if (date > std::chrono::system_clock::now()) + if (date > scwx::util::time::now()) { return; } @@ -1259,7 +1260,7 @@ void RadarProductManagerImpl::PopulateProductTimes( [&](const auto& date) { // Don't query for a time point in the future - if (date > std::chrono::system_clock::now()) + if (date > scwx::util::time::now()) { return; } @@ -1556,7 +1557,7 @@ RadarProductManager::GetLevel2Data(wsr88d::rda::DataBlockType dataBlockType, bool needArchive = true; static const auto maxChunkDelay = std::chrono::minutes(10); const std::chrono::system_clock::time_point firstValidChunkTime = - (isEpox ? std::chrono::system_clock::now() : time) - maxChunkDelay; + (isEpox ? scwx::util::time::now() : time) - maxChunkDelay; // See if we have this one in the chunk provider. auto chunkFile = std::dynamic_pointer_cast( diff --git a/scwx-qt/source/scwx/qt/manager/text_event_manager.cpp b/scwx-qt/source/scwx/qt/manager/text_event_manager.cpp index d7759275..009f2441 100644 --- a/scwx-qt/source/scwx/qt/manager/text_event_manager.cpp +++ b/scwx-qt/source/scwx/qt/manager/text_event_manager.cpp @@ -678,7 +678,7 @@ void TextEventManager::Impl::Refresh() // If the time jumps, we should attempt to load from no later than the // previous load time auto loadTime = - std::chrono::floor(std::chrono::system_clock::now()); + std::chrono::floor(scwx::util::time::now()); auto startTime = loadTime - loadHistoryDuration_; if (prevLoadTime_ != std::chrono::sys_time {}) diff --git a/scwx-qt/source/scwx/qt/manager/timeline_manager.cpp b/scwx-qt/source/scwx/qt/manager/timeline_manager.cpp index 0bcf4f68..70c4b2ed 100644 --- a/scwx-qt/source/scwx/qt/manager/timeline_manager.cpp +++ b/scwx-qt/source/scwx/qt/manager/timeline_manager.cpp @@ -218,7 +218,7 @@ void TimelineManager::AnimationStepBegin() p->pinnedTime_ == std::chrono::system_clock::time_point {}) { // If the selected view type is live, select the current products - p->SelectTimeAsync(std::chrono::system_clock::now() - p->loopTime_); + p->SelectTimeAsync(scwx::util::time::now() - p->loopTime_); } else { @@ -385,8 +385,8 @@ TimelineManager::Impl::GetLoopStartAndEndTimes() if (viewType_ == types::MapTime::Live || pinnedTime_ == std::chrono::system_clock::time_point {}) { - endTime = std::chrono::floor( - std::chrono::system_clock::now()); + endTime = + std::chrono::floor(scwx::util::time::now()); } else { @@ -656,8 +656,8 @@ void TimelineManager::Impl::Step(Direction direction) { if (direction == Direction::Back) { - newTime = std::chrono::floor( - std::chrono::system_clock::now()); + newTime = + std::chrono::floor(scwx::util::time::now()); } else { @@ -688,7 +688,7 @@ void TimelineManager::Impl::Step(Direction direction) newTime += 1min; // If the new time is more than 2 minutes in the future, stop stepping - if (newTime > std::chrono::system_clock::now() + 2min) + if (newTime > scwx::util::time::now() + 2min) { break; } diff --git a/scwx-qt/source/scwx/qt/map/alert_layer.cpp b/scwx-qt/source/scwx/qt/map/alert_layer.cpp index 6599e0e5..7bea5938 100644 --- a/scwx-qt/source/scwx/qt/map/alert_layer.cpp +++ b/scwx-qt/source/scwx/qt/map/alert_layer.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include @@ -581,8 +582,7 @@ void AlertLayer::Impl::ScheduleRefresh() // Expires at the top of the next minute std::chrono::system_clock::time_point now = - std::chrono::floor( - std::chrono::system_clock::now()); + std::chrono::floor(scwx::util::time::now()); refreshTimer_.expires_at(now + 1min); refreshTimer_.async_wait( diff --git a/scwx-qt/source/scwx/qt/model/alert_proxy_model.cpp b/scwx-qt/source/scwx/qt/model/alert_proxy_model.cpp index 4dfeca41..0fa55d96 100644 --- a/scwx-qt/source/scwx/qt/model/alert_proxy_model.cpp +++ b/scwx-qt/source/scwx/qt/model/alert_proxy_model.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include @@ -67,7 +68,7 @@ bool AlertProxyModel::filterAcceptsRow(int sourceRow, .value(); // Compare end time to current - if (endTime < std::chrono::system_clock::now()) + if (endTime < scwx::util::time::now()) { acceptAlertActiveFilter = false; } diff --git a/scwx-qt/source/scwx/qt/ui/animation_dock_widget.cpp b/scwx-qt/source/scwx/qt/ui/animation_dock_widget.cpp index 85fdb74f..eac16c44 100644 --- a/scwx-qt/source/scwx/qt/ui/animation_dock_widget.cpp +++ b/scwx-qt/source/scwx/qt/ui/animation_dock_widget.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include @@ -81,8 +82,7 @@ AnimationDockWidget::AnimationDockWidget(QWidget* parent) : p->timeZone_ = date::get_tzdb().locate_zone("UTC"); #endif const std::chrono::sys_seconds currentTimePoint = - std::chrono::floor( - std::chrono::system_clock::now()); + std::chrono::floor(scwx::util::time::now()); p->SetTimePoint(currentTimePoint); // Update maximum date on a timer diff --git a/scwx-qt/source/scwx/qt/view/overlay_product_view.cpp b/scwx-qt/source/scwx/qt/view/overlay_product_view.cpp index 3200dcca..ccc41b9d 100644 --- a/scwx-qt/source/scwx/qt/view/overlay_product_view.cpp +++ b/scwx-qt/source/scwx/qt/view/overlay_product_view.cpp @@ -187,7 +187,7 @@ void OverlayProductView::Impl::LoadProduct( header.date_of_message(), header.time_of_message() * 1000); // If the record is from the last 30 minutes - if (productTime + 30min >= std::chrono::system_clock::now() || + if (productTime + 30min >= scwx::util::time::now() || (selectedTime_ != std::chrono::system_clock::time_point {} && productTime + 30min >= selectedTime_)) { diff --git a/wxdata/source/scwx/provider/aws_level2_chunks_data_provider.cpp b/wxdata/source/scwx/provider/aws_level2_chunks_data_provider.cpp index 74a9afd7..f7de36ca 100644 --- a/wxdata/source/scwx/provider/aws_level2_chunks_data_provider.cpp +++ b/wxdata/source/scwx/provider/aws_level2_chunks_data_provider.cpp @@ -302,7 +302,7 @@ AwsLevel2ChunksDataProvider::Impl::GetScanTime(const std::string& prefix) if (scanTimeIt != scanTimes_.cend()) { // If the time is greater than 2 hours ago, it may be a new scan - auto replaceBy = system_clock::now() - hours {2}; + auto replaceBy = util::time::now() - hours {2}; if (scanTimeIt->second > replaceBy) { return scanTimeIt->second; @@ -333,8 +333,7 @@ AwsLevel2ChunksDataProvider::Impl::ListObjects() size_t newObjects = 0; const size_t totalObjects = 0; - const std::chrono::system_clock::time_point now = - std::chrono::system_clock::now(); + const std::chrono::system_clock::time_point now = util::time::now(); if (currentScan_.valid_ && !currentScan_.hasAllFiles_ && lastTimeListed_ + std::chrono::minutes(2) > now) diff --git a/wxdata/source/scwx/provider/aws_nexrad_data_provider.cpp b/wxdata/source/scwx/provider/aws_nexrad_data_provider.cpp index dceb45d8..97528a9e 100644 --- a/wxdata/source/scwx/provider/aws_nexrad_data_provider.cpp +++ b/wxdata/source/scwx/provider/aws_nexrad_data_provider.cpp @@ -352,7 +352,7 @@ std::pair AwsNexradDataProvider::Refresh() logger_->debug("Refresh()"); - auto today = floor(system_clock::now()); + auto today = floor(util::time::now()); auto yesterday = today - days {1}; std::unique_lock lock(p->refreshMutex_); @@ -388,7 +388,7 @@ void AwsNexradDataProvider::Impl::PruneObjects() { using namespace std::chrono; - auto today = floor(system_clock::now()); + auto today = floor(util::time::now()); auto yesterday = today - days {1}; std::unique_lock lock(objectsMutex_); diff --git a/wxdata/source/scwx/provider/warnings_provider.cpp b/wxdata/source/scwx/provider/warnings_provider.cpp index 78eb687c..a762cd99 100644 --- a/wxdata/source/scwx/provider/warnings_provider.cpp +++ b/wxdata/source/scwx/provider/warnings_provider.cpp @@ -108,7 +108,7 @@ WarningsProvider::LoadUpdatedFiles( std::vector> updatedFiles; const std::chrono::sys_time now = - std::chrono::floor(std::chrono::system_clock::now()); + std::chrono::floor(util::time::now()); std::chrono::sys_time currentHour = (startTime != std::chrono::sys_time {}) ? startTime : From c5b858021f0bc2192699fedb81cd977a9c387fed Mon Sep 17 00:00:00 2001 From: Dan Paulat Date: Sun, 24 Aug 2025 22:14:30 -0500 Subject: [PATCH 11/15] #include is required --- wxdata/source/scwx/network/ntp_client.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/wxdata/source/scwx/network/ntp_client.cpp b/wxdata/source/scwx/network/ntp_client.cpp index 0a340f87..e95599dd 100644 --- a/wxdata/source/scwx/network/ntp_client.cpp +++ b/wxdata/source/scwx/network/ntp_client.cpp @@ -5,6 +5,7 @@ #include #include +#include #include #include From 193f42318ca34ffa1e3e9f8dba4d8641069ab6a0 Mon Sep 17 00:00:00 2001 From: Dan Paulat Date: Sun, 24 Aug 2025 22:16:30 -0500 Subject: [PATCH 12/15] li/vn/mode struct should not be anonymous --- wxdata/include/scwx/types/ntp_types.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wxdata/include/scwx/types/ntp_types.hpp b/wxdata/include/scwx/types/ntp_types.hpp index cfb8f764..92c18fa7 100644 --- a/wxdata/include/scwx/types/ntp_types.hpp +++ b/wxdata/include/scwx/types/ntp_types.hpp @@ -23,7 +23,7 @@ struct NtpPacket union { std::uint8_t li_vn_mode; - struct + struct LiVnMode { std::uint8_t mode : 3; // Client will pick mode 3 for client. std::uint8_t vn : 3; // Version number of the protocol. From 072865d2e5277048abac77d1c4da3091950760cc Mon Sep 17 00:00:00 2001 From: Dan Paulat Date: Sun, 24 Aug 2025 22:28:38 -0500 Subject: [PATCH 13/15] Addressing clang-tidy findings --- test/.clang-tidy | 4 ++++ test/source/scwx/network/ntp_client.test.cpp | 7 ++----- wxdata/include/scwx/network/ntp_client.hpp | 5 +++-- wxdata/source/scwx/network/ntp_client.cpp | 20 ++++++++++++++------ wxdata/source/scwx/types/ntp_types.cpp | 2 +- 5 files changed, 24 insertions(+), 14 deletions(-) diff --git a/test/.clang-tidy b/test/.clang-tidy index d5079a03..254b44ad 100644 --- a/test/.clang-tidy +++ b/test/.clang-tidy @@ -8,9 +8,13 @@ Checks: - 'performance-*' - '-bugprone-easily-swappable-parameters' - '-cppcoreguidelines-avoid-magic-numbers' + - '-cppcoreguidelines-avoid-do-while' + - '-cppcoreguidelines-avoid-non-const-global-variables' - '-cppcoreguidelines-pro-type-reinterpret-cast' + - '-cppcoreguidelines-pro-type-union-access' - '-misc-include-cleaner' - '-misc-non-private-member-variables-in-classes' + - '-misc-use-anonymous-namespace' - '-modernize-return-braced-init-list' - '-modernize-use-trailing-return-type' FormatStyle: 'file' diff --git a/test/source/scwx/network/ntp_client.test.cpp b/test/source/scwx/network/ntp_client.test.cpp index bdfcb4ae..1450b324 100644 --- a/test/source/scwx/network/ntp_client.test.cpp +++ b/test/source/scwx/network/ntp_client.test.cpp @@ -2,9 +2,7 @@ #include -namespace scwx -{ -namespace network +namespace scwx::network { TEST(NtpClient, Poll) @@ -31,5 +29,4 @@ TEST(NtpClient, Poll) !error); } -} // namespace network -} // namespace scwx +} // namespace scwx::network diff --git a/wxdata/include/scwx/network/ntp_client.hpp b/wxdata/include/scwx/network/ntp_client.hpp index beb4be21..8be912b3 100644 --- a/wxdata/include/scwx/network/ntp_client.hpp +++ b/wxdata/include/scwx/network/ntp_client.hpp @@ -23,8 +23,9 @@ public: NtpClient(NtpClient&&) noexcept; NtpClient& operator=(NtpClient&&) noexcept; - bool error(); - std::chrono::system_clock::duration time_offset() const; + bool error(); + + [[nodiscard]] std::chrono::system_clock::duration time_offset() const; void Start(); void Stop(); diff --git a/wxdata/source/scwx/network/ntp_client.cpp b/wxdata/source/scwx/network/ntp_client.cpp index e95599dd..494aadde 100644 --- a/wxdata/source/scwx/network/ntp_client.cpp +++ b/wxdata/source/scwx/network/ntp_client.cpp @@ -56,7 +56,7 @@ public: NtpTimestamp& operator=(NtpTimestamp&&) = default; template - std::chrono::time_point ToTimePoint() const + [[nodiscard]] std::chrono::time_point ToTimePoint() const { // Convert NTP seconds to Unix seconds // Don't cast to a larger type to account for rollover, and this should @@ -191,8 +191,8 @@ NtpClient::Impl::~Impl() bool NtpClient::error() { - bool returnValue = p->error_; - p->error_ = false; + const bool returnValue = p->error_; + p->error_ = false; return returnValue; } @@ -273,6 +273,13 @@ void NtpClient::Impl::Poll() static constexpr auto kTimeout_ = 5s; + if (!serverEndpoint_.has_value()) + { + logger_->error("Server endpoint not set"); + error_ = true; + return; + } + try { const auto originTimestamp = @@ -280,7 +287,7 @@ void NtpClient::Impl::Poll() transmitPacket_.txTm_s = ntohl(originTimestamp.seconds_); transmitPacket_.txTm_f = ntohl(originTimestamp.fraction_); - std::size_t transmitPacketSize = sizeof(transmitPacket_); + const std::size_t transmitPacketSize = sizeof(transmitPacket_); // Send NTP request socket_.send_to(boost::asio::buffer(&transmitPacket_, transmitPacketSize), *serverEndpoint_); @@ -310,6 +317,7 @@ void NtpClient::Impl::Poll() catch (const std::exception& ex) { logger_->error("Error polling: {}", ex.what()); + error_ = true; } } @@ -437,7 +445,7 @@ void NtpClient::Impl::Run() if (enabled_) { - std::chrono::seconds pollIntervalSeconds {1u << pollInterval_}; + const std::chrono::seconds pollIntervalSeconds {1u << pollInterval_}; pollTimer_.expires_after(pollIntervalSeconds); pollTimer_.async_wait( [this](const boost::system::error_code& e) @@ -547,7 +555,7 @@ std::shared_ptr NtpClient::Instance() static std::weak_ptr ntpClientReference_ {}; static std::mutex instanceMutex_ {}; - std::unique_lock lock(instanceMutex_); + const std::unique_lock lock(instanceMutex_); std::shared_ptr ntpClient = ntpClientReference_.lock(); diff --git a/wxdata/source/scwx/types/ntp_types.cpp b/wxdata/source/scwx/types/ntp_types.cpp index 9ff39095..daf5d46b 100644 --- a/wxdata/source/scwx/types/ntp_types.cpp +++ b/wxdata/source/scwx/types/ntp_types.cpp @@ -14,7 +14,7 @@ namespace scwx::types::ntp NtpPacket NtpPacket::Parse(const std::span data) { - NtpPacket packet; + NtpPacket packet {}; assert(data.size() >= sizeof(NtpPacket)); From d9a7858c2da84f884aceb419424775c6257c8c6d Mon Sep 17 00:00:00 2001 From: Dan Paulat Date: Sun, 24 Aug 2025 23:13:42 -0500 Subject: [PATCH 14/15] Fix nested structure compliance with C++ standard --- wxdata/include/scwx/types/ntp_types.hpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/wxdata/include/scwx/types/ntp_types.hpp b/wxdata/include/scwx/types/ntp_types.hpp index 92c18fa7..db1aa61f 100644 --- a/wxdata/include/scwx/types/ntp_types.hpp +++ b/wxdata/include/scwx/types/ntp_types.hpp @@ -1,6 +1,5 @@ #pragma once -#include #include #include @@ -20,15 +19,17 @@ namespace scwx::types::ntp struct NtpPacket { + struct LiVnMode + { + std::uint8_t mode : 3; // Client will pick mode 3 for client. + std::uint8_t vn : 3; // Version number of the protocol. + std::uint8_t li : 2; // Leap indicator. + }; + union { std::uint8_t li_vn_mode; - struct LiVnMode - { - std::uint8_t mode : 3; // Client will pick mode 3 for client. - std::uint8_t vn : 3; // Version number of the protocol. - std::uint8_t li : 2; // Leap indicator. - } fields; + LiVnMode fields; }; std::uint8_t stratum; // Stratum level of the local clock. From 89de7718e6bb6e7a28707596e012128e915d8881 Mon Sep 17 00:00:00 2001 From: Dan Paulat Date: Mon, 25 Aug 2025 23:27:11 -0500 Subject: [PATCH 15/15] Don't run NTP tests on CI --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 19e80256..093cfcbf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -393,7 +393,7 @@ jobs: env: MAPBOX_API_KEY: ${{ secrets.MAPBOX_API_KEY }} MAPTILER_API_KEY: ${{ secrets.MAPTILER_API_KEY }} - run: ctest -C ${{ matrix.build_type }} --exclude-regex "test_mln.*|UpdateManager.*" + run: ctest -C ${{ matrix.build_type }} --exclude-regex "test_mln.*|NtpClient.*|UpdateManager.*" - name: Upload Test Logs if: ${{ !cancelled() }}