Refactor shared utils and cleanup warnings

Moves common error-printing and string constants into shared utils headers, then updates network, remoteconfig, EMAC, and timer code to use them. Also tightens several conditional compilation checks, replaces magic values with named constants, and cleans up a few small naming and formatting issues.

Author: vanvught

common/include/common/utils/utils_print.h

@@ -0,0 +1,44 @@
+/**
+ * @file utils_print.h
+ *
+ */
+/* Copyright (C) 2026 by Arjan van Vught mailto:info@gd32-dmx.org
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef COMMON_UTILS_UTILS_PRINT_H_
+#define COMMON_UTILS_UTILS_PRINT_H_
+
+#include <cstdio>
+
+#include "firmware/ansi_colour.h"
+
+namespace common::print {
+inline void Error(const char* func, const char* string) {
+    printf("%s%s: %s%s\n", ansi::Colours::Fg::kRed, func, string, ansi::Colours::Fg::kDefault);
+}
+} // namespace common::print
+
+#define ERROR(s)                             \
+    do {                                     \
+        common::print::Error(__func__, (s)); \
+    } while (false)
+
+#endif // COMMON_UTILS_UTILS_PRINT_H_

common/include/common/utils/utils_string.h

@@ -29,6 +29,15 @@
 #include <cstdint>
 
 namespace common {
+inline constexpr char kWarning[] = "Warning";
+inline constexpr char kError[] = "Error";
+inline constexpr char kSuccess[] = "Success";
+inline constexpr char kUnknown[] = "Unknown"; 
+
+constexpr const char* IsSuccess(bool is_success) {
+  return is_success ? kSuccess : kError;
+}
+
 constexpr uint32_t ConstStrLen(const char* str) {
     uint32_t len = 0;
     while (str[len] != '\0') {

common/include/common/utils/utils_units.h

@@ -29,13 +29,16 @@
 #include <cstdint>
 
 namespace common::units {
-inline constexpr uint32_t kUsPerMs = 1'000U;
-inline constexpr uint32_t kMsPerSecond = 1'000U;
-inline constexpr uint32_t kUsPerSecond = 1'000'000U;
+inline constexpr int32_t kSecondPerMinute = 60;
 
-inline constexpr uint32_t kNsPerUs = 1'000U;
-inline constexpr uint32_t kNsPerMs = 1'000'000U;
-inline constexpr uint32_t kNsPerSecond = 1'000'000'000U;
+inline constexpr uint32_t kMsPerSecond = 1'000;
+
+inline constexpr uint32_t kUsPerMs = 1'000;
+inline constexpr uint32_t kUsPerSecond = 1'000'000;
+
+inline constexpr uint32_t kNsPerUs = 1'000;
+inline constexpr uint32_t kNsPerMs = 1'000'000;
+inline constexpr uint32_t kNsPerSecond = 1'000'000'000;
 
 inline constexpr uint32_t k1KiB = 1024;
 

lib-flashcodeinstall/src/flashcodeinstall.cpp

@@ -30,6 +30,7 @@
 #include "firmware.h"
 #include "display.h" // IWYU pragma: keep
 #include "watchdog.h"
+#include "common/utils/utils_print.h"
 
 bool FlashCodeInstall::WriteFirmware(std::span<const uint8_t> firmware) {
     FLASHCODE_INSTALL_DEBUG_ENTRY();
@@ -68,7 +69,7 @@ bool FlashCodeInstall::WriteFirmware(std::span<const uint8_t> firmware) {
     }
 
     if (flashcode::Result::kError == result) {
-        puts("Error: flash erase");
+        ERROR("flash erase");
         return false;
     }
 
@@ -78,7 +79,7 @@ bool FlashCodeInstall::WriteFirmware(std::span<const uint8_t> firmware) {
     }
 
     if (flashcode::Result::kError == result) {
-        puts("Error: flash write");
+        ERROR("flash write");
         return false;
     }
 

lib-network/src/core/ipv4/dhcp.cpp

@@ -41,6 +41,7 @@
 #include "core/ip4/acd.h"
 #endif // CONFIG_NET_DHCP_USE_ACD
 #include "firmware/debug/debug_debug.h"
+#include "common/utils/utils_print.h"
 
 #ifdef DEBUG_NETWORK_DHCP
 #define DHCP_DEBUG_ENTRY() DEBUG_ENTRY()
@@ -68,9 +69,11 @@ static TimerHandle_t s_timer_id;
 
 // https://tools.ietf.org/html/rfc1541
 namespace network::dhcp {
-static Message s_dhcp_message SECTION_NETWORK ALIGNED;
+static constexpr char kNoHandle[] = "No handle";
+namespace {
+Message s_dhcp_message SECTION_NETWORK ALIGNED;
 
-static void MessageInit() {
+void MessageInit() {
     std::memset(&s_dhcp_message, 0, sizeof(dhcp::Message));
 
     s_dhcp_message.op = dhcp::OpCode::kBootrequest;
@@ -87,7 +90,7 @@ static void MessageInit() {
     s_dhcp_message.options[5] = 0x01;
 }
 
-static void UpdateMsg(uint8_t message_type) {
+void UpdateMsg(uint8_t message_type) {
     auto* dhcp = reinterpret_cast<struct dhcp::Dhcp*>(netif::global::netif_default.dhcp);
     assert(dhcp != nullptr);
 
@@ -111,7 +114,7 @@ static void UpdateMsg(uint8_t message_type) {
     }
 }
 
-static void SendDiscover() {
+void SendDiscover() {
     DHCP_DEBUG_ENTRY();
     auto* dhcp = reinterpret_cast<struct dhcp::Dhcp*>(netif::global::netif_default.dhcp);
     assert(dhcp != nullptr);
@@ -147,7 +150,7 @@ static void SendDiscover() {
     DHCP_DEBUG_EXIT();
 }
 
-static void SendRequest() {
+void SendRequest() {
     DHCP_DEBUG_ENTRY();
     auto* dhcp = reinterpret_cast<struct dhcp::Dhcp*>(netif::global::netif_default.dhcp);
     assert(dhcp != nullptr);
@@ -199,7 +202,7 @@ static void SendRequest() {
     DHCP_DEBUG_EXIT();
 }
 
-static void SendRelease(uint32_t destination_ip) {
+void SendRelease(uint32_t destination_ip) {
     DHCP_DEBUG_ENTRY();
     DHCP_DEBUG_PRINTF(IPSTR, IP2STR(destination_ip));
 
@@ -221,6 +224,7 @@ static void SendRelease(uint32_t destination_ip) {
 
     DHCP_DEBUG_EXIT();
 }
+} // namespace
 
 void Input(const uint8_t* buffer, uint32_t size, [[maybe_unused]] uint32_t from_ip, uint16_t from_port) {
     DHCP_DEBUG_ENTRY();
@@ -250,7 +254,7 @@ void Inform() {
 
     const auto kHandle = network::udp::Begin(network::iana::Ports::kPortDhcpClient, nullptr);
     if (kHandle < 0) {
-        ERROR("No handle\n");
+        ERROR(kNoHandle);
         return;
     }
 
@@ -689,14 +693,14 @@ bool Start() {
     dhcp->handle = network::udp::Begin(network::iana::Ports::kPortDhcpClient, dhcp::Input);
 
     if (dhcp->handle < 0) {
-        ERROR("No handle.\n");
+        ERROR(kNoHandle);
         DHCP_DEBUG_EXIT();
         return false;
     }
 
     MessageInit();
 
-#if defined(CONFIG_NET_DHCP_USE_ACD)
+#ifdef CONFIG_NET_DHCP_USE_ACD
     network::acd::Add(&dhcp->acd, ConflictCallback);
 #endif // CONFIG_NET_DHCP_USE_ACD
 

lib-network/src/core/ipv4/igmp.cpp

@@ -43,6 +43,7 @@
 #include "core/protocol/igmp.h"
 #include "softwaretimers.h" // IWYU pragma: keep
 #include "firmware/debug/debug_debug.h"
+#include "common/utils/utils_print.h"
 
 #ifdef DEBUG_NETWORK_IGMP
 #define IGMP_DEBUG_ENTRY() DEBUG_ENTRY()

lib-network/src/core/network_memory.h

@@ -30,21 +30,24 @@
 #include <cstring>
 #include <cassert>
 
-#include "network_private.h"
+#include "common/utils/utils_print.h"
 
 namespace network::memory {
+inline constexpr uint32_t kBlocksMin = 1;
+inline constexpr uint32_t kBlocksMax = 32;
+
 inline constexpr uint32_t kBlocks =
-#if !defined(CONFIG_NETWORK_MEMORY_BLOCKS)
+#ifndef CONFIG_NETWORK_MEMORY_BLOCKS
     12;
 #else
     CONFIG_NETWORK_MEMORY_BLOCKS;
 #endif // CONFIG_NETWORK_MEMORY_BLOCKS
 
-static_assert(kBlocks >= 1);
-static_assert(kBlocks <= 32);
+static_assert(kBlocks >= kBlocksMin);
+static_assert(kBlocks <= kBlocksMax);
 
 inline constexpr uint32_t kBlockSize =
-#if !defined(CONFIG_NETWORK_MEMORY_BLOCKSIZE)
+#ifndef CONFIG_NETWORK_MEMORY_BLOCKSIZE
     1460;
 #else
     CONFIG_NETWORK_MEMORY_BLOCKSIZE;
@@ -79,16 +82,16 @@ class Allocator {
     Allocator(Allocator&&) = delete;
     Allocator& operator=(Allocator&&) = delete;
 
-    bool IsEmpty() const { return free_mask_ == kAllMask; }
-    bool IsFull() const { return free_mask_ == 0; }
+    [[nodiscard]] bool IsEmpty() const { return free_mask_ == kAllMask; }
+    [[nodiscard]] bool IsFull() const { return free_mask_ == 0; }
 
     uint8_t* Allocate() {
         if (IsFull()) {
-            network::Error(__func__, "Allocate:Full!");
+            ERROR("Allocate:Full!");
             return nullptr;
         }
 
-        const uint32_t kIndex = static_cast<uint32_t>(__builtin_ctz(free_mask_));
+        const auto kIndex = static_cast<uint32_t>(__builtin_ctz(free_mask_));
         free_mask_ &= ~(1U << kIndex);
 
         Status();
@@ -102,11 +105,11 @@ class Allocator {
         assert(size <= kBlockSize);
 
         if (IsFull()) {
-            network::Error(__func__, "Allocate:Full!");
+            ERROR("Allocate:Full!");
             return UINT16_MAX;
         }
 
-        const uint32_t kIndex = static_cast<uint32_t>(__builtin_ctz(free_mask_));
+        const auto kIndex = static_cast<uint32_t>(__builtin_ctz(free_mask_));
         free_mask_ &= ~(1U << kIndex);
 
         size_[kIndex] = size;
@@ -155,7 +158,7 @@ class Allocator {
     }
 
     void Status() const {
-#if defined DEBUG_NETWORK_MEMORY
+#ifdef DEBUG_NETWORK_MEMORY
         const uint32_t kUsedMask = (~free_mask_) & kAllMask;
         printf("free_mask=0x%08x used_mask=0x%08x free=%u used=%u\n", free_mask_, kUsedMask, __builtin_popcount(free_mask_), __builtin_popcount(kUsedMask));
         printf("IsEmpty=%c IsFull=%c\n", IsEmpty() ? 'Y' : 'N', IsFull() ? 'Y' : 'N');
@@ -164,7 +167,7 @@ class Allocator {
 
    private:
     Allocator() = default;
-    static constexpr uint32_t kAllMask = (kBlocks == 32) ? UINT32_MAX : ((1U << kBlocks) - 1U);
+    static constexpr uint32_t kAllMask = (kBlocks == kBlocksMax) ? UINT32_MAX : ((1U << kBlocks) - 1U);
     uint32_t free_mask_{0};
     uint16_t size_[kBlocks]{0};
 };

lib-network/src/core/network_private.h

@@ -53,12 +53,6 @@ void FreePkt();
 } // namespace emac::eth
 
 namespace network {
-inline void Error(const char* func, const char* string) {
-    printf("%s%s: %s%s\n", ansi::Colours::Fg::kRed, func, string, ansi::Colours::Fg::kDefault);
-}
-
-#define ERROR(s) Error(__func__, (s))
-
 namespace global {
 extern uint32_t broadcast_mask;
 extern uint32_t on_network_mask;

lib-network/src/core/tcp.cpp

@@ -72,6 +72,7 @@
 #include "network_memory.h"
 #include "network_tcp_datasegmentqueue.h"
 #include "common/utils/utils_math.h"
+#include "common/utils/utils_print.h"
 
 #if defined(DEBUG_TCP)
 #define TCP_DEBUG_ENTRY() DEBUG_ENTRY()

lib-network/src/core/udp.cpp

@@ -43,6 +43,7 @@
 #include "network_memcpy.h"
 #include "firmware/debug/debug_debug.h"
 #include "common/utils/utils_math.h"
+#include "common/utils/utils_print.h"
 
 #if defined(DEBUG_UDP)
 #define UDP_DEBUG_ENTRY() DEBUG_ENTRY()