Various fixes and improvements (#435)

- Fix for `sdkconfig` generation: the keys that contained the MCU type in its name weren't properly upper-cased.
- Moved WiFi configuration property files to the user data path of the app instead of a fixed location.
- Moved more properties from `device.py` to `sdkconfig/default.properties`
- Fix for `device.cmake` device id parsing: separate basic property parsing from device id validation
- Created internal `tt::service::wifi::findServiceContext()`
- Changed Wi-Fi service id to lowercase (will change it for other services later)

Buildscripts/device.cmake

@@ -14,7 +14,7 @@ function(INIT_TACTILITY_GLOBALS SDKCONFIG_FILE)
         message(FATAL_ERROR "sdkconfig file not found:\nMake sure you select a device by running \"python device.py [device-id]\"\n")
     endif ()
     file(READ ${SDKCONFIG_FILE_ABS} sdkconfig_text)
-    string(REGEX MATCH "(CONFIG_TT_DEVICE_ID\=\"[a-z0-9_\-]*\")" sdkconfig_device_id "${sdkconfig_text}")
+    string(REGEX MATCH "(CONFIG_TT_DEVICE_ID\=\"[^\"]*\")" sdkconfig_device_id "${sdkconfig_text}")
     if (sdkconfig_device_id STREQUAL "CONFIG_TT_DEVICE_ID=\"\"" OR sdkconfig_device_id STREQUAL "")
         message(FATAL_ERROR "CONFIG_TT_DEVICE_ID not found in sdkconfig:\nMake sure you select a device with 'python device.py device-id'")
     endif ()
@@ -24,6 +24,11 @@ function(INIT_TACTILITY_GLOBALS SDKCONFIG_FILE)
     math(EXPR id_length "${sdkconfig_device_id_length} - 22")
     # Skip 'CONFIG_TT_DEVICE_ID="' then read the relevant (remaining) chars
     string(SUBSTRING ${sdkconfig_device_id} 21 ${id_length} device_id)
+    # Validate device id
+    if (NOT device_id MATCHES "^[a-z0-9\-]*$")
+        message(FATAL_ERROR "Device identifier ${device_id} contains invalid characters. Valid characters: a-z 0-9 \"-\"")
+    endif ()
+    # Output results
     message("Device identifier: ${Cyan}${device_id}${ColorReset}")
     set(TACTILITY_DEVICE_PROJECT ${device_id})
     message("Device project path: ${Cyan}Devices/${TACTILITY_DEVICE_PROJECT}${ColorReset}\n")

Buildscripts/sdkconfig/default.properties

@@ -2,6 +2,16 @@
 CONFIG_ESP_SYSTEM_EVENT_TASK_STACK_SIZE=3072
 # Ensure large enough stack for network operations
 CONFIG_ESP_MAIN_TASK_STACK_SIZE=6144
+# Fixes static assertion: FLASH and PSRAM Mode configuration are not supported
+CONFIG_IDF_EXPERIMENTAL_FEATURES=y
+# Free up IRAM
+CONFIG_FREERTOS_PLACE_FUNCTIONS_INTO_FLASH=y
+CONFIG_FREERTOS_PLACE_SNAPSHOT_FUNS_INTO_FLASH=y
+CONFIG_HEAP_PLACE_FUNCTION_INTO_FLASH=y
+CONFIG_RINGBUF_PLACE_FUNCTIONS_INTO_FLASH=y
+# EmbedTLS
+CONFIG_MBEDTLS_SSL_PROTO_TLS1_3=y
+# LVGL
 CONFIG_LV_FONT_MONTSERRAT_14=y
 CONFIG_LV_FONT_MONTSERRAT_18=y
 CONFIG_LV_USE_USER_DATA=y
@@ -31,6 +41,3 @@ CONFIG_WL_SECTOR_SIZE_512=y
 CONFIG_WL_SECTOR_SIZE=512
 CONFIG_WL_SECTOR_MODE_SAFE=y
 CONFIG_WL_SECTOR_MODE=1
-CONFIG_MBEDTLS_SSL_PROTO_TLS1_3=y
-# Fixes static assertion: FLASH and PSRAM Mode configuration are not supported
-CONFIG_IDF_EXPERIMENTAL_FEATURES=y

Tactility/Private/Tactility/service/wifi/WifiPrivate.h

@@ -0,0 +1,10 @@
+#pragma once
+
+#include <Tactility/service/ServiceContext.h>
+#include <memory>
+
+namespace tt::service::wifi {
+
+std::shared_ptr<ServiceContext> findServiceContext();
+
+}

Tactility/Source/service/wifi/Wifi.cpp

@@ -1,5 +1,8 @@
 #include "Tactility/service/wifi/Wifi.h"
 
+#include <Tactility/service/ServiceManifest.h>
+#include <Tactility/service/ServiceRegistration.h>
+
 namespace tt::service::wifi {
 
 const char* radioStateToString(RadioState state) {
@@ -21,4 +24,10 @@ const char* radioStateToString(RadioState state) {
     tt_crash("not implemented");
 }
 
+extern const ServiceManifest manifest;
+
+std::shared_ptr<ServiceContext> findServiceContext() {
+    return findServiceContextById(manifest.id);
+}
+
 }

Tactility/Source/service/wifi/WifiApSettings.cpp

@@ -1,10 +1,11 @@
-#include "Tactility/service/wifi/WifiApSettings.h"
+#include <Tactility/service/wifi/WifiPrivate.h>
-#include "Tactility/file/PropertiesFile.h"
+#include <Tactility/service/wifi/WifiApSettings.h>
+#include <Tactility/file/PropertiesFile.h>
 
 #include <Tactility/crypt/Crypt.h>
 #include <Tactility/file/File.h>
 
-#include <cstring>
+#include <Tactility/service/ServicePaths.h>
 #include <format>
 #include <iomanip>
 #include <ranges>
@@ -15,14 +16,13 @@ namespace tt::service::wifi::settings {
 
 constexpr auto* TAG = "WifiApSettings";
 
-constexpr auto* AP_SETTINGS_FORMAT = "/data/settings/{}.ap.properties";
+constexpr auto* AP_SETTINGS_FORMAT = "{}/{}.ap.properties";
 
 constexpr auto* AP_PROPERTIES_KEY_SSID = "ssid";
 constexpr auto* AP_PROPERTIES_KEY_PASSWORD = "password";
 constexpr auto* AP_PROPERTIES_KEY_AUTO_CONNECT = "autoConnect";
 constexpr auto* AP_PROPERTIES_KEY_CHANNEL = "channel";
 
-
 std::string toHexString(const uint8_t *data, int length) {
     std::stringstream stream;
     stream << std::hex;
@@ -48,8 +48,9 @@ bool readHex(const std::string& input, uint8_t* buffer, int length) {
     return true;
 }
 
-static std::string getApPropertiesFilePath(const std::string& ssid) {
+// TODO: The SSID could contain invalid filename characters (e.g. "/", "\" and more) so we have to refactor this.
-    return std::format(AP_SETTINGS_FORMAT, ssid);
+static std::string getApPropertiesFilePath(std::shared_ptr<ServicePaths> paths, const std::string& ssid) {
+    return std::format(AP_SETTINGS_FORMAT, paths->getUserDataDirectory(), ssid);
 }
 
 static bool encrypt(const std::string& ssidInput, std::string& ssidOutput) {
@@ -111,12 +112,20 @@ static bool decrypt(const std::string& ssidInput, std::string& ssidOutput) {
 }
 
 bool contains(const std::string& ssid) {
-    const auto file_path = getApPropertiesFilePath(ssid);
+    auto service_context = findServiceContext();
+    if (service_context == nullptr) {
+        return false;
+    }
+    const auto file_path = getApPropertiesFilePath(service_context->getPaths(), ssid);
     return file::isFile(file_path);
 }
 
 bool load(const std::string& ssid, WifiApSettings& apSettings) {
-    const auto file_path = getApPropertiesFilePath(ssid);
+    auto service_context = findServiceContext();
+    if (service_context == nullptr) {
+        return false;
+    }
+    const auto file_path = getApPropertiesFilePath(service_context->getPaths(), ssid);
     std::map<std::string, std::string> map;
     if (!file::loadPropertiesFile(file_path, map)) {
         return false;
@@ -165,7 +174,12 @@ bool save(const WifiApSettings& apSettings) {
         return false;
     }
 
-    const auto file_path = getApPropertiesFilePath(apSettings.ssid);
+    auto service_context = findServiceContext();
+    if (service_context == nullptr) {
+        return false;
+    }
+
+    const auto file_path = getApPropertiesFilePath(service_context->getPaths(), apSettings.ssid);
 
     std::map<std::string, std::string> map;
 
@@ -187,7 +201,12 @@ bool save(const WifiApSettings& apSettings) {
 }
 
 bool remove(const std::string& ssid) {
-    const auto path = getApPropertiesFilePath(ssid);
+    auto service_context = findServiceContext();
+    if (service_context == nullptr) {
+        return false;
+    }
+
+    const auto path = getApPropertiesFilePath(service_context->getPaths(), ssid);
     if (!file::isFile(path)) {
         return false;
     }

Tactility/Source/service/wifi/WifiEsp.cpp

@@ -959,7 +959,7 @@ public:
 };
 
 extern const ServiceManifest manifest = {
-    .id = "Wifi",
+    .id = "wifi",
     .createService = create<WifiService>
 };
 

Tactility/Source/service/wifi/WifiMock.cpp

@@ -153,7 +153,7 @@ public:
 };
 
 extern const ServiceManifest manifest = {
-    .id = "Wifi",
+    .id = "wifi",
     .createService = create<WifiService>
 };
 

Tactility/Source/service/wifi/WifiSettings.cpp

@@ -1,13 +1,14 @@
 #include <Tactility/service/wifi/WifiSettings.h>
 
+#include <Tactility/Log.h>
 #include <Tactility/file/File.h>
 #include <Tactility/file/PropertiesFile.h>
-#include <Tactility/Log.h>
+#include <Tactility/service/ServicePaths.h>
+#include <Tactility/service/wifi/WifiPrivate.h>
 
 namespace tt::service::wifi::settings {
 
 constexpr auto* TAG = "WifiSettings";
-constexpr auto* SETTINGS_FILE = "/data/settings/wifi.properties";
 constexpr auto* SETTINGS_KEY_ENABLE_ON_BOOT = "enableOnBoot";
 
 struct WifiSettings {
@@ -21,8 +22,14 @@ static WifiSettings cachedSettings {
 static bool cached = false;
 
 static bool load(WifiSettings& settings) {
+    auto service_context = findServiceContext();
+    if (service_context == nullptr) {
+        return false;
+    }
+
     std::map<std::string, std::string> map;
-    if (!file::loadPropertiesFile(SETTINGS_FILE, map)) {
+    std::string settings_path = service_context->getPaths()->getUserDataPath("settings.properties");
+    if (!file::loadPropertiesFile(settings_path, map)) {
         return false;
     }
 
@@ -36,15 +43,20 @@ static bool load(WifiSettings& settings) {
 }
 
 static bool save(const WifiSettings& settings) {
+    auto service_context = findServiceContext();
+    if (service_context == nullptr) {
+        return false;
+    }
     std::map<std::string, std::string> map;
     map[SETTINGS_KEY_ENABLE_ON_BOOT] = settings.enableOnBoot ? "true" : "false";
-    return file::savePropertiesFile(SETTINGS_FILE, map);
+    std::string settings_path = service_context->getPaths()->getUserDataPath("settings.properties");
+    return file::savePropertiesFile(settings_path, map);
 }
 
 WifiSettings getCachedOrLoad() {
     if (!cached) {
         if (!load(cachedSettings)) {
-            TT_LOG_E(TAG, "Failed to load %s", SETTINGS_FILE);
+            TT_LOG_E(TAG, "Failed to load");
         } else {
             cached = true;
         }
@@ -56,7 +68,7 @@ WifiSettings getCachedOrLoad() {
 void setEnableOnBoot(bool enable) {
     cachedSettings.enableOnBoot = enable;
     if (!save(cachedSettings)) {
-        TT_LOG_E(TAG, "Failed to save %s", SETTINGS_FILE);
+        TT_LOG_E(TAG, "Failed to save");
     }
 }
 

device.py

@@ -113,8 +113,8 @@ def write_core_variables(output_file, device_properties: ConfigParser):
     output_file.write("# CPU\n")
     output_file.write(f"CONFIG_ESP_DEFAULT_CPU_FREQ_MHZ_240=y\n")
     output_file.write(f"CONFIG_ESP_DEFAULT_CPU_FREQ_MHZ=240\n")
-    output_file.write(f"CONFIG_{idf_target}_DEFAULT_CPU_FREQ_240=y\n")
+    output_file.write(f"CONFIG_{idf_target.upper()}_DEFAULT_CPU_FREQ_240=y\n")
-    output_file.write(f"CONFIG_{idf_target}_DEFAULT_CPU_FREQ_MHZ=240\n")
+    output_file.write(f"CONFIG_{idf_target.upper()}_DEFAULT_CPU_FREQ_MHZ=240\n")
 
 def write_flash_variables(output_file, device_properties: ConfigParser):
     flash_size = get_property_or_exit(device_properties, "hardware", "flashSize")
@@ -138,9 +138,11 @@ def write_spiram_variables(output_file, device_properties: ConfigParser):
     if has_spiram != "true":
         return
     output_file.write("# SPIRAM\n")
+    # Boot speed optimization
+    output_file.write("CONFIG_SPIRAM_MEMTEST=n\n")
     # Enable
     output_file.write("CONFIG_SPIRAM=y\n")
-    output_file.write(f"CONFIG_{idf_target}_SPIRAM_SUPPORT=y\n")
+    output_file.write(f"CONFIG_{idf_target.upper()}_SPIRAM_SUPPORT=y\n")
     mode = get_property_or_exit(device_properties, "hardware", "spiRamMode")
     # Mode
     if mode != "AUTO":
@@ -162,13 +164,6 @@ def write_spiram_variables(output_file, device_properties: ConfigParser):
 
 def write_performance_improvements(output_file, device_properties: ConfigParser):
     idf_target = get_property_or_exit(device_properties, "hardware", "target").lower()
-    output_file.write("# Free up IRAM\n")
-    output_file.write("CONFIG_FREERTOS_PLACE_FUNCTIONS_INTO_FLASH=y\n")
-    output_file.write("CONFIG_FREERTOS_PLACE_SNAPSHOT_FUNS_INTO_FLASH=y\n")
-    output_file.write("CONFIG_HEAP_PLACE_FUNCTION_INTO_FLASH=y\n")
-    output_file.write("CONFIG_RINGBUF_PLACE_FUNCTIONS_INTO_FLASH=y\n")
-    output_file.write("# Boot speed optimization\n")
-    output_file.write("CONFIG_SPIRAM_MEMTEST=n\n")
     if idf_target == "esp32s3":
         output_file.write("# Performance improvement: Fixes glitches in the RGB display driver when rendering new screens/apps\n")
         output_file.write("CONFIG_ESP32S3_DATA_CACHE_LINE_64B=y\n")