refactor(clock): refactor clock to avoid large stack frames and fetching the ntp address every time

src/clock.rs

@@ -13,19 +13,49 @@ use sntpc_time_embassy::EmbassyTimestampGenerator;
 // NTP server (pool.ntp.org is the usual choice)
 const NTP_SERVER: &str = "pool.ntp.org";
 const NTP_PORT: u16 = 123;
+// Unix epoch is 1900-01-01 + 70 years = 2_208_988_800 seconds.
+const NTP_UNIX_OFFSET: u64 = 2_208_988_800;
+
+static NTP_BOOT_OFFSET: AtomicU64 = AtomicU64::new(0);
+
+#[allow(
+    clippy::large_stack_frames,
+    reason = "it's not unusual to allocate larger objects in service's start functions"
+)]
+pub fn start_ntp_sync(spawner: Spawner, net_stack: Stack<'static>) {
+    let service = crate::mk_static!(
+        NtpClient,
+        NtpClient {
+            net_stack,
+
+            rx_meta: [PacketMetadata::EMPTY; 16],
+            rx_buf: [0u8; 48],
+            tx_meta: [PacketMetadata::EMPTY; 16],
+            tx_buf: [0u8; 48],
+        }
+    );
+
+    spawner.spawn(handler(service).unwrap());
+}
 
 pub fn now() -> embassy_time::Instant {
     embassy_time::Instant::now() + Duration::from_secs(NTP_BOOT_OFFSET.load(Ordering::Relaxed))
 }
 
-pub fn start_ntp_sync(spawner: Spawner, net_stack: Stack<'static>) {
-    spawner.spawn(handler(net_stack).unwrap());
-}
-
 #[embassy_executor::task]
-async fn handler(net_stack: Stack<'static>) {
+async fn handler(service: &'static mut NtpClient) {
+    let address = loop {
+        match service.fetch_ntp_address(NTP_SERVER, NTP_PORT).await {
+            Ok(addr) => break addr,
+            Err(err) => {
+                defmt::error!("Failed to get the NTP server's address: {}", err);
+                Timer::after(Duration::from_secs(10)).await
+            }
+        }
+    };
+
     loop {
-        match fetch_ntp_time(net_stack).await {
+        match service.fetch_ntp_time(address).await {
             Ok(unix_secs) => {
                 NTP_BOOT_OFFSET.store(
                     unix_secs - embassy_time::Instant::now().as_secs(),
@@ -41,46 +71,50 @@ async fn handler(net_stack: Stack<'static>) {
     }
 }
 
-static NTP_BOOT_OFFSET: AtomicU64 = AtomicU64::new(0);
+struct NtpClient {
+    net_stack: Stack<'static>,
 
-pub async fn fetch_ntp_time(net_stack: Stack<'static>) -> Result<u64, &'static str> {
-    // Resolve the NTP server hostname
-    let server_addr = net_stack
-        .dns_query(NTP_SERVER, embassy_net::dns::DnsQueryType::A)
-        .await
-        .map_err(|_| "DNS resolution failed")?
-        .first()
-        .copied()
-        .ok_or("No DNS result")?;
+    rx_meta: [PacketMetadata; 16],
+    rx_buf: [u8; 48],
+    tx_meta: [PacketMetadata; 16],
+    tx_buf: [u8; 48],
+}
 
-    let socket_addr = SocketAddr::new(server_addr.into(), NTP_PORT);
+impl NtpClient {
+    async fn fetch_ntp_address(&self, server: &str, port: u16) -> Result<SocketAddr, &'static str> {
+        // Resolve the NTP server hostname
+        let server_addr = self
+            .net_stack
+            .dns_query(server, embassy_net::dns::DnsQueryType::A)
+            .await
+            .map_err(|_| "DNS resolution failed")?
+            .first()
+            .copied()
+            .ok_or("No DNS result")?;
 
-    // Allocate UDP buffers (can be small — SNTP packets are tiny)
-    let mut rx_meta = [PacketMetadata::EMPTY; 16];
-    let mut rx_buf = [0u8; 48];
-    let mut tx_meta = [PacketMetadata::EMPTY; 16];
-    let mut tx_buf = [0u8; 48];
+        Ok(SocketAddr::new(server_addr.into(), port))
+    }
 
-    let mut socket = UdpSocket::new(
-        net_stack,
-        &mut rx_meta,
-        &mut rx_buf,
-        &mut tx_meta,
-        &mut tx_buf,
-    );
-    socket.bind(0).map_err(|_| "bind failed")?;
+    async fn fetch_ntp_time(&mut self, address: SocketAddr) -> Result<u64, &'static str> {
+        let mut socket = UdpSocket::new(
+            self.net_stack,
+            &mut self.rx_meta,
+            &mut self.rx_buf,
+            &mut self.tx_meta,
+            &mut self.tx_buf,
+        );
+        socket.bind(0).map_err(|_| "bind failed")?;
 
-    let wrapped = UdpSocketWrapper::from(socket);
-    let ctx = NtpContext::new(EmbassyTimestampGenerator::default());
+        let wrapped = UdpSocketWrapper::from(socket);
+        let ctx = NtpContext::new(EmbassyTimestampGenerator::default());
 
-    let result = get_time(socket_addr, &wrapped, ctx)
-        .await
-        .map_err(|_| "SNTP request failed")?;
+        let result = get_time(address, &wrapped, ctx)
+            .await
+            .map_err(|_| "SNTP request failed")?;
 
-    // result.sec is the NTP timestamp in seconds since 1900-01-01.
-    // Unix epoch is 1900-01-01 + 70 years = 2_208_988_800 seconds.
-    const NTP_UNIX_OFFSET: u64 = 2_208_988_800;
-    let unix_secs = result.sec().saturating_sub(NTP_UNIX_OFFSET);
+        // result.sec is the NTP timestamp in seconds since 1900-01-01.
+        let unix_secs = result.sec().saturating_sub(NTP_UNIX_OFFSET);
 
-    Ok(unix_secs)
+        Ok(unix_secs)
+    }
 }