Fixes and improvements: LVGL, services and kernel symbols (#587)

Modules/lvgl-module/include/lvgl/module.h

@@ -40,8 +40,7 @@ struct LvglModuleConfig {
 
 /**
  * @brief Configures the LVGL module.
- *
- * @warning This must be called before starting the module.
+ * @warning Must not be called when module is started.
  * @param config The configuration to apply.
  */
 void lvgl_module_configure(struct LvglModuleConfig config);

Modules/lvgl-module/source/arch/lvgl_esp32.c

@@ -6,8 +6,11 @@
 #include <lvgl/lvgl.h>
 #include <lvgl/module.h>
 #include <tactility/error.h>
+#include <tactility/log.h>
 #include <tactility/time.h>
 
+#define TAG "lvgl_esp32"
+
 extern struct LvglModuleConfig lvgl_module_config;
 extern void lvgl_devices_attach();
 extern void lvgl_devices_detach();
@@ -67,7 +70,6 @@ error_t lvgl_arch_stop() {
     }
 
     initialized = false;
-
     return ERROR_NONE;
 }
 

Modules/lvgl-module/source/module.c

@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: Apache-2.0
 #include <lvgl/module.h>
 #include <string.h>
+#include <tactility/check.h>
 #include <tactility/module.h>
 #include <lvgl/lvgl.h>
 
@@ -22,8 +23,9 @@ struct LvglModuleConfig lvgl_module_config = {
 };
 
 void lvgl_module_configure(const struct LvglModuleConfig config) {
+    check(!is_running);
+    lvgl_module_config = config;
     is_configured = true;
-    memcpy(&lvgl_module_config, &config, sizeof(struct LvglModuleConfig));
 }
 
 static error_t start() {

Tactility/Include/Tactility/service/ServiceRegistration.h

@@ -17,6 +17,18 @@ void addService(std::shared_ptr<const ServiceManifest> manifest, bool autoStart
  */
 void addService(const ServiceManifest& manifest, bool autoStart = true);
 
+/** Unregister a service. Stops it first if it is running.
+ * @param[in] the service manifest
+ * @return true on success
+ */
+bool removeService(const std::string& id);
+
+/** Unregister a service. Stops it first if it is running.
+ * @param[in] the service manifest
+ * @return true on success
+ */
+bool removeService(const ServiceManifest& manifest);
+
 /** Start a service.
  * @param[in] the service id as defined in its manifest
  * @return true on success

Tactility/Include/Tactility/service/rtctime/RtcTimeService.h

@@ -2,8 +2,6 @@
 
 #include <Tactility/SystemEvents.h>
 #include <Tactility/service/Service.h>
-#include <Tactility/service/ServiceContext.h>
-#include <Tactility/service/rtctime/RtcTime.h>
 
 #include <memory>
 

Tactility/Private/Tactility/service/displayidle/DisplayIdleService.h

@@ -9,8 +9,8 @@
 #include <memory>
 
 // Forward declarations
-typedef struct _lv_obj_t lv_obj_t;
-typedef struct _lv_event_t lv_event_t;
+typedef _lv_obj_t lv_obj_t;
+typedef _lv_event_t lv_event_t;
 
 namespace tt::service::displayidle {
 

Tactility/Source/Tactility.cpp

@@ -41,7 +41,6 @@
 #endif
 
 #include "Tactility/Paths.h"
-#include "Tactility/SystemEvents.h"
 #include "Tactility/hal/SdCard.h"
 
 #include <Tactility/bluetooth/Bluetooth.h>
@@ -296,27 +295,7 @@ static void registerInstalledAppsFromFileSystems() {
     });
 }
 
-static void registerAndStartSecondaryServices() {
-    LOG_I(TAG, "Registering and starting secondary system services");
-    addService(service::loader::manifest);
-    addService(service::gui::manifest);
-    addService(service::statusbar::manifest);
-    addService(service::memorychecker::manifest);
-#if defined(ESP_PLATFORM)
-    if (device_exists_of_type(&RTC_TYPE)) {
-        addService(service::rtctime::manifest);
-    }
-    addService(service::displayidle::manifest);
-#if defined(CONFIG_TT_TDECK_WORKAROUND)
-    addService(service::keyboardidle::manifest);
-#endif
-#endif
-#if TT_FEATURE_SCREENSHOT_ENABLED
-    addService(service::screenshot::manifest);
-#endif
-}
-
-static void registerAndStartPrimaryServices() {
+static void registerAndStartServices() {
     LOG_I(TAG, "Registering and starting primary system services");
     if (device_exists_of_type(&AUDIO_STREAM_TYPE)) {
         addService(service::audio::manifest);
@@ -331,6 +310,12 @@ static void registerAndStartPrimaryServices() {
 #endif
 #ifdef ESP_PLATFORM
     addService(service::webserver::manifest);
+#endif
+    addService(service::loader::manifest);
+#if defined(ESP_PLATFORM)
+    if (device_exists_of_type(&RTC_TYPE)) {
+        addService(service::rtctime::manifest);
+    }
 #endif
 }
 
@@ -365,6 +350,36 @@ void registerApps() {
     registerInstalledAppsFromFileSystems();
 }
 
+static void onLvglStarted() {
+    addService(service::gui::manifest);
+    addService(service::statusbar::manifest);
+    addService(service::memorychecker::manifest);
+#if defined(ESP_PLATFORM)
+    addService(service::displayidle::manifest);
+#endif
+#if defined(CONFIG_TT_TDECK_WORKAROUND)
+    addService(service::keyboardidle::manifest);
+#endif
+#if TT_FEATURE_SCREENSHOT_ENABLED
+    addService(service::screenshot::manifest);
+#endif
+}
+
+static void onLvglStopped() {
+#if TT_FEATURE_SCREENSHOT_ENABLED
+    check(service::removeService(service::screenshot::manifest.id));
+#endif
+#if defined(CONFIG_TT_TDECK_WORKAROUND)
+    check(service::removeService(service::keyboardidle::manifest.id));
+#endif
+#if defined(ESP_PLATFORM)
+    check(service::removeService(service::displayidle::manifest.id));
+#endif
+    check(service::removeService(service::memorychecker::manifest.id));
+    check(service::removeService(service::statusbar::manifest.id));
+    check(service::removeService(service::gui::manifest.id));
+}
+
 void run(Module* dtsModules[], DtsDevice dtsDevices[]) {
     LOG_I(TAG, "Tactility v%s on %s (%s)", TT_VERSION, CONFIG_TT_DEVICE_NAME, CONFIG_TT_DEVICE_ID);
 
@@ -390,14 +405,14 @@ void run(Module* dtsModules[], DtsDevice dtsDevices[]) {
     network::ntp::init();
     bluetooth::systemStart();
 
-    registerAndStartPrimaryServices();
+    registerAndStartServices();
 
     // Must start right before LVGL
     initFileMutexForLvgl();
 
     lvgl_module_configure((LvglModuleConfig) {
-        .on_start = nullptr,
-        .on_stop = nullptr,
+        .on_start = onLvglStarted,
+        .on_stop = onLvglStopped,
         .task_priority = THREAD_PRIORITY_HIGHER,
         /** Minimum seems to be about 3500. In some scenarios, the WiFi app crashes at 8192,
          * so we now have 9120 to run in a stable manner. We should figure out a way to avoid this.
@@ -409,8 +424,6 @@ void run(Module* dtsModules[], DtsDevice dtsDevices[]) {
     });
     check(module_ensure_started(&lvgl_module) == ERROR_NONE);
 
-    registerAndStartSecondaryServices();
-
     LOG_I(TAG, "Core systems ready");
 
     LOG_I(TAG, "Starting boot app");

Tactility/Source/service/ServiceRegistration.cpp

@@ -48,9 +48,9 @@ void addService(std::shared_ptr<const ServiceManifest> manifest, bool autoStart)
         return;
     }
 
-    // Intentionally never freed: services are registered once and live for the
-    // process lifetime (there is no removeService()). Keeps id's backing string
-    // alive for cManifest.id below.
+    // Intentionally never freed: removeService() only unregisters the manifest
+    // from the kernel, it doesn't own this allocation. Keeps id's backing
+    // string alive for cManifest.id below.
     auto* persistentManifest = new std::shared_ptr(manifest);
     auto* cManifest = new ::ServiceManifest {
         .id = (*persistentManifest)->id.c_str(),
@@ -70,6 +70,29 @@ void addService(const ServiceManifest& manifest, bool autoStart) {
     addService(std::make_shared<const ServiceManifest>(manifest), autoStart);
 }
 
+bool removeService(const std::string& id) {
+    if (service_manager_get_state(id.c_str()) != SERVICE_STATE_STOPPED) {
+        LOG_I(TAG, "Stopping %s before removal", id.c_str());
+        if (!stopService(id)) {
+            LOG_E(TAG, "Failed to stop %s before removal", id.c_str());
+            return false;
+        }
+    }
+
+    LOG_I(TAG, "Removing %s", id.c_str());
+    error_t error = service_manager_remove(id.c_str());
+    if (error != ERROR_NONE) {
+        LOG_E(TAG, "Failed to remove service %s: %s", id.c_str(), error_to_string(error));
+        return false;
+    }
+    LOG_I(TAG, "Removed %s", id.c_str());
+    return true;
+}
+
+bool removeService(const ServiceManifest& manifest) {
+    return removeService(manifest.id);
+}
+
 const ::ServiceManifest* findManifestById(const std::string& id) {
     return service_manager_find_manifest(id.c_str());
 }

Tactility/Source/service/gui/GuiService.cpp

@@ -178,10 +178,21 @@ void GuiService::redraw() {
         return;
     }
 
-    while (!lvgl_try_lock(1000)) {
+    bool lvgl_locked = false;
+    while (lvgl_is_running() && !(lvgl_locked = lvgl_try_lock(1000))) {
         LOG_W(TAG, LOG_MESSAGE_MUTEX_LOCK_FAILED_FMT, "GuiService LVGL");
     }
 
+    if (!lvgl_locked) {
+        unlock();
+        return;
+    }
+    if (!lvgl_is_running()) {
+        lvgl_unlock();
+        unlock();
+        return;
+    }
+
     lv_obj_clean(appRootWidget);
 
     if (appToRender != nullptr) {

Tactility/Source/service/rtctime/RtcTimeService.cpp

@@ -2,7 +2,6 @@
 
 #include <Tactility/service/rtctime/RtcTimeService.h>
 
-#include <Tactility/service/ServiceContext.h>
 #include <Tactility/service/ServiceManifest.h>
 #include <Tactility/service/ServiceRegistration.h>
 

Tactility/Source/service/statusbar/Statusbar.cpp

@@ -20,7 +20,6 @@
 #include <tactility/filesystem/file_system.h>
 
 #include <tactility/log.h>
-#include <tactility/module.h>
 
 #include <lvgl/lvgl.h>
 #include <lvgl/lvgl_icon_statusbar.h>

TactilityKernel/source/symbols.c

@@ -74,6 +74,7 @@ const struct ModuleSymbol KERNEL_SYMBOLS[] = {
     DEFINE_MODULE_SYMBOL(device_lock),
     DEFINE_MODULE_SYMBOL(device_try_lock),
     DEFINE_MODULE_SYMBOL(device_unlock),
+    DEFINE_MODULE_SYMBOL(device_get_child_count),
     DEFINE_MODULE_SYMBOL(device_get_type),
     DEFINE_MODULE_SYMBOL(device_for_each),
     DEFINE_MODULE_SYMBOL(device_for_each_child),
@@ -83,6 +84,14 @@ const struct ModuleSymbol KERNEL_SYMBOLS[] = {
     DEFINE_MODULE_SYMBOL(device_find_first_active_by_type),
     DEFINE_MODULE_SYMBOL(device_find_first_by_type),
     DEFINE_MODULE_SYMBOL(device_find_first_by_compatible),
+    DEFINE_MODULE_SYMBOL(device_get),
+    DEFINE_MODULE_SYMBOL(device_put),
+    DEFINE_MODULE_SYMBOL(device_get_by_name),
+    DEFINE_MODULE_SYMBOL(device_get_first_by_type),
+    DEFINE_MODULE_SYMBOL(device_get_first_active_by_type),
+    DEFINE_MODULE_SYMBOL(device_has_active_by_type),
+    DEFINE_MODULE_SYMBOL(device_get_first_by_compatible),
+    DEFINE_MODULE_SYMBOL(device_is_constructed),
     // driver
     DEFINE_MODULE_SYMBOL(driver_construct),
     DEFINE_MODULE_SYMBOL(driver_destruct),