Refactor app launching (#174)

- Refactor the way apps work: Instead of a C interface, they are now C++ classes. The main reasoning is that attaching data to an app was cumbersome. Having different implementations for different kinds of apps was cumbersome too. (3 or 4 layers of manifest nesting for the TactilityC project)
- External apps are still written in C, but they get a createData/destroyData in their manifest, so:
- External apps now have their own manifest.
- All functions in the original AppManifest are removed and replaced by a single `createApp` function
- External apps now automatically register (each app individually!) when they run the first time. As a side-effect they become visible in the `AppList` app!
- Adapted all apps for the new interface.
- Adapted all internal logic for these changes (Gui, ViewPort, Loader, AppContext, AppInstance, etc.)
- Rewrote parts of Loader to use std::shared_ptr to make the code much safer.
- Added a refcount check for the `AppInstance` and `App` at the end of their lifecycle. Show warning if refcount is too high.

Tactility/Source/service/gui/Gui.cpp

@@ -19,9 +19,8 @@ Gui* gui = nullptr;
 void onLoaderMessage(const void* message, TT_UNUSED void* context) {
     auto* event = static_cast<const loader::LoaderEvent*>(message);
     if (event->type == loader::LoaderEventTypeApplicationShowing) {
-        app::AppContext& app = event->app_showing.app;
-        const app::AppManifest& app_manifest = app.getManifest();
-        showApp(app, app_manifest.onShow, app_manifest.onHide);
+        auto app_instance = service::loader::getCurrentAppContext();
+        showApp(app_instance);
     } else if (event->type == loader::LoaderEventTypeApplicationHiding) {
         hideApp();
     }
@@ -94,27 +93,25 @@ void requestDraw() {
     thread_flags_set(thread_id, GUI_THREAD_FLAG_DRAW);
 }
 
-void showApp(app::AppContext& app, ViewPortShowCallback on_show, ViewPortHideCallback on_hide) {
+void showApp(std::shared_ptr<app::AppContext> app) {
     lock();
-    tt_check(gui->appViewPort == nullptr);
-    gui->appViewPort = view_port_alloc(app, on_show, on_hide);
+    tt_check(gui->appToRender == nullptr);
+    gui->appToRender = std::move(app);
     unlock();
     requestDraw();
 }
 
 void hideApp() {
     lock();
-    ViewPort* view_port = gui->appViewPort;
-    tt_check(view_port != nullptr);
+    tt_check(gui->appToRender != nullptr);
 
     // We must lock the LVGL port, because the viewport hide callbacks
     // might call LVGL APIs (e.g. to remove the keyboard from the screen root)
     tt_check(lvgl::lock(configTICK_RATE_HZ));
-    view_port_hide(view_port);
+    gui->appToRender->getApp()->onHide(*gui->appToRender);
     lvgl::unlock();
 
-    view_port_free(view_port);
-    gui->appViewPort = nullptr;
+    gui->appToRender = nullptr;
     unlock();
 }
 

Tactility/Source/service/gui/Gui.h

@@ -1,7 +1,7 @@
 #pragma once
 
+#include "app/AppInstance.h"
 #include "app/AppContext.h"
-#include "ViewPort.h"
 
 namespace tt::service::gui {
 
@@ -10,10 +10,8 @@ typedef struct Gui Gui;
 /**
  * Set the app viewport in the gui state and request the gui to draw it.
  * @param[in] app
- * @param[in] onShow
- * @param[in] onHide
  */
-void showApp(app::AppContext& app, ViewPortShowCallback onShow, ViewPortHideCallback onHide);
+void showApp(std::shared_ptr<app::AppContext> app);
 
 /**
  * Hide the current app's viewport.

Tactility/Source/service/gui/GuiDraw.cpp

@@ -9,9 +9,10 @@ namespace tt::service::gui {
 
 #define TAG "gui"
 
-static lv_obj_t* createAppViews(Gui* gui, lv_obj_t* parent, app::AppContext& app) {
+static lv_obj_t* createAppViews(Gui* gui, lv_obj_t* parent) {
     lv_obj_send_event(gui->statusbarWidget, LV_EVENT_DRAW_MAIN, nullptr);
     lv_obj_t* child_container = lv_obj_create(parent);
+    lv_obj_set_style_pad_all(child_container, 0, 0);
     lv_obj_set_width(child_container, LV_PCT(100));
     lv_obj_set_flex_grow(child_container, 1);
 
@@ -34,19 +35,17 @@ void redraw(Gui* gui) {
     if (lvgl::lock(1000)) {
         lv_obj_clean(gui->appRootWidget);
 
-        ViewPort* view_port = gui->appViewPort;
-        if (view_port != nullptr) {
-            app::AppContext& app = view_port->app;
+        if (gui->appToRender != nullptr) {
 
-            app::Flags flags = app.getFlags();
+            app::Flags flags = std::static_pointer_cast<app::AppInstance>(gui->appToRender)->getFlags();
             if (flags.showStatusbar) {
                 lv_obj_remove_flag(gui->statusbarWidget, LV_OBJ_FLAG_HIDDEN);
             } else {
                 lv_obj_add_flag(gui->statusbarWidget, LV_OBJ_FLAG_HIDDEN);
             }
 
-            lv_obj_t* container = createAppViews(gui, gui->appRootWidget, app);
-            view_port_show(view_port, container);
+            lv_obj_t* container = createAppViews(gui, gui->appRootWidget);
+            gui->appToRender->getApp()->onShow(*gui->appToRender, container);
         } else {
             TT_LOG_W(TAG, "nothing to draw");
         }

Tactility/Source/service/gui/ViewPort.cpp

@@ -1,44 +0,0 @@
-#include "ViewPort.h"
-
-#include "Check.h"
-#include "service/gui/ViewPort_i.h"
-#include "lvgl/Style.h"
-
-namespace tt::service::gui {
-
-#define TAG "viewport"
-
-ViewPort* view_port_alloc(
-    app::AppContext& app,
-    ViewPortShowCallback on_show,
-    ViewPortHideCallback on_hide
-) {
-    return new ViewPort(
-        app,
-        on_show,
-        on_hide
-    );
-}
-
-void view_port_free(ViewPort* view_port) {
-    tt_assert(view_port);
-    delete view_port;
-}
-
-void view_port_show(ViewPort* view_port, lv_obj_t* parent) {
-    tt_assert(view_port);
-    tt_assert(parent);
-    if (view_port->onShow) {
-        lvgl::obj_set_style_no_padding(parent);
-        view_port->onShow(view_port->app, parent);
-    }
-}
-
-void view_port_hide(ViewPort* view_port) {
-    tt_assert(view_port);
-    if (view_port->onHide) {
-        view_port->onHide(view_port->app);
-    }
-}
-
-} // namespace

Tactility/Source/service/gui/ViewPort.h

@@ -1,47 +0,0 @@
-#pragma once
-
-#include "app/AppContext.h"
-#include "lvgl.h"
-
-namespace tt::service::gui {
-
-/** ViewPort Draw callback
- * @warning    called from GUI thread
- */
-typedef void (*ViewPortShowCallback)(app::AppContext& app, lv_obj_t* parent);
-typedef void (*ViewPortHideCallback)(app::AppContext& app);
-
-// TODO: Move internally, use handle publicly
-
-typedef struct ViewPort {
-    app::AppContext& app;
-    ViewPortShowCallback onShow;
-    ViewPortHideCallback _Nullable onHide;
-
-    ViewPort(
-        app::AppContext& app,
-        ViewPortShowCallback on_show,
-        ViewPortHideCallback _Nullable on_hide
-    ) : app(app), onShow(on_show), onHide(on_hide) {}
-} ViewPort;
-
-/** ViewPort allocator
- * always returns view_port or stops system if not enough memory.
- * @param app
- * @param onShow Called to create LVGL widgets
- * @param onHide Called before clearing the LVGL widget parent
- * @return ViewPort instance
- */
-ViewPort* view_port_alloc(
-    app::AppContext& app,
-    ViewPortShowCallback onShow,
-    ViewPortHideCallback onHide
-);
-
-/** ViewPort destruction
- * Ensure that view_port was unregistered in GUI system before use.
- * @param viewPort ViewPort instance
- */
-void view_port_free(ViewPort* viewPort);
-
-} // namespace

Tactility/Source/service/loader/Loader.cpp

@@ -6,8 +6,9 @@
 #include "RtosCompat.h"
 
 #ifdef ESP_PLATFORM
-#include "esp_heap_caps.h"
 #include "TactilityHeadless.h"
+#include "app/ElfApp.h"
+#include "esp_heap_caps.h"
 
 #else
 #include "lvgl/LvglSync.h"
@@ -61,17 +62,22 @@ void stopApp() {
     loader_singleton->dispatcherThread->dispatch(onStopAppMessage, nullptr);
 }
 
-app::AppContext* _Nullable getCurrentApp() {
+std::shared_ptr<app::AppContext> _Nullable getCurrentAppContext() {
     tt_assert(loader_singleton);
     if (loader_singleton->mutex.lock(10 / portTICK_PERIOD_MS)) {
-        app::AppInstance* app = loader_singleton->appStack.top();
+        auto app = loader_singleton->appStack.top();
         loader_singleton->mutex.unlock();
-        return dynamic_cast<app::AppContext*>(app);
+        return std::move(app);
     } else {
         return nullptr;
     }
 }
 
+std::shared_ptr<app::App> _Nullable getCurrentApp() {
+    auto app_context = getCurrentAppContext();
+    return app_context != nullptr ? app_context->getApp() : nullptr;
+}
+
 std::shared_ptr<PubSub> getPubsub() {
     tt_assert(loader_singleton);
     // it's safe to return pubsub without locking
@@ -97,9 +103,9 @@ static const char* appStateToString(app::State state) {
     }
 }
 
-static void transitionAppToState(app::AppInstance& app, app::State state) {
-    const app::AppManifest& manifest = app.getManifest();
-    const app::State old_state = app.getState();
+static void transitionAppToState(std::shared_ptr<app::AppInstance> app, app::State state) {
+    const app::AppManifest& manifest = app->getManifest();
+    const app::State old_state = app->getState();
 
     TT_LOG_I(
         TAG,
@@ -111,49 +117,35 @@ static void transitionAppToState(app::AppInstance& app, app::State state) {
 
     switch (state) {
         case app::StateInitial:
-            app.setState(app::StateInitial);
+            app->setState(app::StateInitial);
             break;
         case app::StateStarted:
-            if (manifest.onStart != nullptr) {
-                manifest.onStart(app);
-            }
-            app.setState(app::StateStarted);
+            app->getApp()->onStart(*app);
+            app->setState(app::StateStarted);
             break;
         case app::StateShowing: {
-            LoaderEvent event_showing = {
-                .type = LoaderEventTypeApplicationShowing,
-                .app_showing = {
-                    .app = app
-                }
-            };
+            LoaderEvent event_showing = { .type = LoaderEventTypeApplicationShowing };
             tt_pubsub_publish(loader_singleton->pubsubExternal, &event_showing);
-            app.setState(app::StateShowing);
+            app->setState(app::StateShowing);
             break;
         }
         case app::StateHiding: {
-            LoaderEvent event_hiding = {
-                .type = LoaderEventTypeApplicationHiding,
-                .app_hiding = {
-                    .app = app
-                }
-            };
+            LoaderEvent event_hiding = { .type = LoaderEventTypeApplicationHiding };
             tt_pubsub_publish(loader_singleton->pubsubExternal, &event_hiding);
-            app.setState(app::StateHiding);
+            app->setState(app::StateHiding);
             break;
         }
         case app::StateStopped:
-            if (manifest.onStop) {
-                manifest.onStop(app);
-            }
-            app.setData(nullptr);
-            app.setState(app::StateStopped);
+            // TODO: Verify manifest
+            app->getApp()->onStop(*app);
+            app->setState(app::StateStopped);
             break;
     }
 }
 
 static LoaderStatus startAppWithManifestInternal(
-    const app::AppManifest* manifest,
-    std::shared_ptr<const Bundle> _Nullable parameters
+    const std::shared_ptr<app::AppManifest>& manifest,
+    const std::shared_ptr<const Bundle> _Nullable& parameters
 ) {
     tt_check(loader_singleton != nullptr);
 
@@ -165,29 +157,23 @@ static LoaderStatus startAppWithManifestInternal(
     }
 
     auto previous_app = !loader_singleton->appStack.empty() ? loader_singleton->appStack.top() : nullptr;
-    auto new_app = new app::AppInstance(*manifest, parameters);
-    new_app->mutableFlags().showStatusbar = (manifest->type != app::TypeBoot);
+
+    auto new_app = std::make_shared<app::AppInstance>(manifest, parameters);
+
+    new_app->mutableFlags().showStatusbar = (manifest->type != app::Type::Boot);
 
     loader_singleton->appStack.push(new_app);
-    transitionAppToState(*new_app, app::StateInitial);
-    transitionAppToState(*new_app, app::StateStarted);
+    transitionAppToState(new_app, app::StateInitial);
+    transitionAppToState(new_app, app::StateStarted);
 
     // We might have to hide the previous app first
     if (previous_app != nullptr) {
-        transitionAppToState(*previous_app, app::StateHiding);
+        transitionAppToState(previous_app, app::StateHiding);
     }
 
-    transitionAppToState(*new_app, app::StateShowing);
+    transitionAppToState(new_app, app::StateShowing);
 
-    LoaderEventInternal event_internal = {.type = LoaderEventTypeApplicationStarted};
-    tt_pubsub_publish(loader_singleton->pubsubInternal, &event_internal);
-
-    LoaderEvent event_external = {
-        .type = LoaderEventTypeApplicationStarted,
-        .app_started = {
-            .app = *new_app
-        }
-    };
+    LoaderEvent event_external = { .type = LoaderEventTypeApplicationStarted };
     tt_pubsub_publish(loader_singleton->pubsubExternal, &event_external);
 
     return LoaderStatus::Ok;
@@ -208,7 +194,7 @@ static LoaderStatus startAppInternal(
 ) {
     TT_LOG_I(TAG, "Start by id %s", id.c_str());
 
-    const app::AppManifest* manifest = app::findAppById(id);
+    auto manifest = app::findAppById(id);
     if (manifest == nullptr) {
         TT_LOG_E(TAG, "App not found: %s", id.c_str());
         return LoaderStatus::ErrorUnknownApp;
@@ -233,75 +219,75 @@ static void stopAppInternal() {
     }
 
     // Stop current app
-    app::AppInstance* app_to_stop = loader_singleton->appStack.top();
+    auto app_to_stop = loader_singleton->appStack.top();
 
-    if (original_stack_size == 1 && app_to_stop->getManifest().type != app::TypeBoot) {
+    if (original_stack_size == 1 && app_to_stop->getManifest().type != app::Type::Boot) {
         TT_LOG_E(TAG, "Stop app: can't stop root app");
         return;
     }
 
-    auto result_holder = std::move(app_to_stop->getResult());
+    bool result_set = false;
+    app::Result result;
+    std::unique_ptr<Bundle> result_bundle;
+    if (app_to_stop->getApp()->moveResult(result, result_bundle)) {
+        result_set = true;
+    }
 
-    const app::AppManifest& manifest = app_to_stop->getManifest();
-    transitionAppToState(*app_to_stop, app::StateHiding);
-    transitionAppToState(*app_to_stop, app::StateStopped);
+    transitionAppToState(app_to_stop, app::StateHiding);
+    transitionAppToState(app_to_stop, app::StateStopped);
 
     loader_singleton->appStack.pop();
-    delete app_to_stop;
+
+    // We only expect the app to be referenced within the current scope
+    if (app_to_stop.use_count() > 1) {
+        TT_LOG_W(TAG, "Memory leak: Stopped %s, but use count is %ld", app_to_stop->getManifest().id.c_str(), app_to_stop.use_count() - 1);
+    }
+
+    // Refcount is expected to be 2: 1 within app_to_stop and 1 within the current scope
+    if (app_to_stop->getApp().use_count() > 2) {
+        TT_LOG_W(TAG, "Memory leak: Stopped %s, but use count is %ld", app_to_stop->getManifest().id.c_str(), app_to_stop->getApp().use_count() - 2);
+    }
 
 #ifdef ESP_PLATFORM
     TT_LOG_I(TAG, "Free heap: %zu", heap_caps_get_free_size(MALLOC_CAP_INTERNAL));
 #endif
 
-    app::AppOnResult on_result = nullptr;
-    app::AppInstance* app_to_resume = nullptr;
+    std::shared_ptr<app::AppInstance> instance_to_resume;
     // If there's a previous app, resume it
     if (!loader_singleton->appStack.empty()) {
-        app_to_resume = loader_singleton->appStack.top();
-        tt_assert(app_to_resume);
-        transitionAppToState(*app_to_resume, app::StateShowing);
-
-        on_result = app_to_resume->getManifest().onResult;
+        instance_to_resume = loader_singleton->appStack.top();
+        tt_assert(instance_to_resume);
+        transitionAppToState(instance_to_resume, app::StateShowing);
     }
 
     // Unlock so that we can send results to app and they can also start/stop new apps while processing these results
     scoped_lock->unlock();
     // WARNING: After this point we cannot change the app states from this method directly anymore as we don't have a lock!
 
-    LoaderEventInternal event_internal = {.type = LoaderEventTypeApplicationStopped};
-    tt_pubsub_publish(loader_singleton->pubsubInternal, &event_internal);
-
-    LoaderEvent event_external = {
-        .type = LoaderEventTypeApplicationStopped,
-        .app_stopped = {
-            .manifest = manifest
-        }
-    };
+    LoaderEvent event_external = { .type = LoaderEventTypeApplicationStopped };
     tt_pubsub_publish(loader_singleton->pubsubExternal, &event_external);
 
-    if (on_result != nullptr && app_to_resume != nullptr) {
-        if (result_holder != nullptr) {
-            auto result_bundle = result_holder->resultData.get();
+    if (instance_to_resume != nullptr) {
+        if (result_set) {
             if (result_bundle != nullptr) {
-                on_result(
-                    *app_to_resume,
-                    result_holder->result,
-                    *result_bundle
+                instance_to_resume->getApp()->onResult(
+                    *instance_to_resume,
+                    result,
+                    std::move(result_bundle)
                 );
             } else {
-                const Bundle empty_bundle;
-                on_result(
-                    *app_to_resume,
-                    result_holder->result,
-                    empty_bundle
+                instance_to_resume->getApp()->onResult(
+                    *instance_to_resume,
+                    result,
+                    nullptr
                 );
             }
         } else {
             const Bundle empty_bundle;
-            on_result(
-                *app_to_resume,
-                app::ResultCancelled,
-                empty_bundle
+            instance_to_resume->getApp()->onResult(
+                *instance_to_resume,
+                app::Result::Cancelled,
+                nullptr
             );
         }
     }

Tactility/Source/service/loader/Loader.h

@@ -21,8 +21,11 @@ void startApp(const std::string& id, const std::shared_ptr<const Bundle>& _Nulla
 /** @brief Stop the currently showing app. Show the previous app if any app was still running. */
 void stopApp();
 
+/** @return the currently running app context (it is only ever null before the splash screen is shown) */
+std::shared_ptr<app::AppContext> _Nullable getCurrentAppContext();
+
 /** @return the currently running app (it is only ever null before the splash screen is shown) */
-app::AppContext* _Nullable getCurrentApp();
+std::shared_ptr<app::App> _Nullable getCurrentApp();
 
 /**
  * @brief PubSub for LoaderEvent

Tactility/Source/service/screenshot/ScreenshotTask.cpp

@@ -87,9 +87,9 @@ void ScreenshotTask::taskMain() {
                 }
             }
         } else if (work.type == TASK_WORK_TYPE_APPS) {
-            app::AppContext* _Nullable app = loader::getCurrentApp();
-            if (app) {
-                const app::AppManifest& manifest = app->getManifest();
+            auto appContext = loader::getCurrentAppContext();
+            if (appContext != nullptr) {
+                const app::AppManifest& manifest = appContext->getManifest();
                 if (manifest.id != last_app_id) {
                     kernel::delayMillis(100);
                     last_app_id = manifest.id;