From a2c63686c13f683de7a162768cabebe2c81053e3 Mon Sep 17 00:00:00 2001 From: Ken Van Hoeylandt Date: Sun, 5 Jul 2026 14:00:23 +0200 Subject: [PATCH 1/7] Refactor --- .../Include/Tactility/service/ServicePaths.h | 35 ++- .../Tactility/service/ServiceInstance.h | 37 ++-- Tactility/Source/service/ServiceInstance.cpp | 19 -- Tactility/Source/service/ServicePaths.cpp | 29 --- .../Source/service/ServiceRegistration.cpp | 160 +++++++------- TactilityKernel/include/tactility/paths.h | 24 ++ .../include/tactility/service/service.h | 42 ++++ .../tactility/service/service_context.h | 28 +++ .../tactility/service/service_instance.h | 78 +++++++ .../tactility/service/service_manifest.h | 42 ++++ .../include/tactility/service/service_paths.h | 56 +++++ .../tactility/service/service_registration.h | 82 +++++++ TactilityKernel/source/paths.cpp | 66 ++++++ .../source/service/service_instance.cpp | 81 +++++++ .../source/service/service_paths.cpp | 62 ++++++ .../source/service/service_registration.cpp | 207 ++++++++++++++++++ .../Source/ServicePathsTest.cpp | 69 ++++++ Tests/TactilityKernel/Source/ServiceTest.cpp | 162 ++++++++++++++ 18 files changed, 1132 insertions(+), 147 deletions(-) delete mode 100644 Tactility/Source/service/ServiceInstance.cpp delete mode 100644 Tactility/Source/service/ServicePaths.cpp create mode 100644 TactilityKernel/include/tactility/paths.h create mode 100644 TactilityKernel/include/tactility/service/service.h create mode 100644 TactilityKernel/include/tactility/service/service_context.h create mode 100644 TactilityKernel/include/tactility/service/service_instance.h create mode 100644 TactilityKernel/include/tactility/service/service_manifest.h create mode 100644 TactilityKernel/include/tactility/service/service_paths.h create mode 100644 TactilityKernel/include/tactility/service/service_registration.h create mode 100644 TactilityKernel/source/paths.cpp create mode 100644 TactilityKernel/source/service/service_instance.cpp create mode 100644 TactilityKernel/source/service/service_paths.cpp create mode 100644 TactilityKernel/source/service/service_registration.cpp create mode 100644 Tests/TactilityKernel/Source/ServicePathsTest.cpp create mode 100644 Tests/TactilityKernel/Source/ServiceTest.cpp diff --git a/Tactility/Include/Tactility/service/ServicePaths.h b/Tactility/Include/Tactility/service/ServicePaths.h index b9cbd524b..ee4f8685b 100644 --- a/Tactility/Include/Tactility/service/ServicePaths.h +++ b/Tactility/Include/Tactility/service/ServicePaths.h @@ -1,12 +1,17 @@ #pragma once +#include + +#include +#include +#include + #include #include namespace tt::service { -// Forward declarations -class ServiceManifest; +constexpr size_t PATH_BUFFER_SIZE = 255; class ServicePaths { @@ -20,26 +25,44 @@ class ServicePaths { * The user data directory is intended to survive OS upgrades. * The path will not end with a "/". */ - std::string getUserDataDirectory() const; + std::string getUserDataDirectory() const { + char buffer[PATH_BUFFER_SIZE]; + check(service_paths_get_user_data_directory(manifest->id.c_str(), buffer, sizeof(buffer)) == ERROR_NONE); + return buffer; + } /** * The user data directory is intended to survive OS upgrades. * Configuration data should be stored here. * @param[in] childPath the path without a "/" prefix */ - std::string getUserDataPath(const std::string& childPath) const; + std::string getUserDataPath(const std::string& childPath) const { + assert(!childPath.starts_with('/')); + char buffer[PATH_BUFFER_SIZE]; + check(service_paths_get_user_data_path(manifest->id.c_str(), childPath.c_str(), buffer, sizeof(buffer)) == ERROR_NONE); + return buffer; + } /** * You should not store configuration data here. * The path will not end with a "/". */ - std::string getAssetsDirectory() const; + std::string getAssetsDirectory() const { + char buffer[PATH_BUFFER_SIZE]; + check(service_paths_get_assets_directory(manifest->id.c_str(), buffer, sizeof(buffer)) == ERROR_NONE); + return buffer; + } /** * You should not store configuration data here. * @param[in] childPath the path without a "/" prefix */ - std::string getAssetsPath(const std::string& childPath) const; + std::string getAssetsPath(const std::string& childPath) const { + assert(!childPath.starts_with('/')); + char buffer[PATH_BUFFER_SIZE]; + check(service_paths_get_assets_path(manifest->id.c_str(), childPath.c_str(), buffer, sizeof(buffer)) == ERROR_NONE); + return buffer; + } }; } \ No newline at end of file diff --git a/Tactility/Private/Tactility/service/ServiceInstance.h b/Tactility/Private/Tactility/service/ServiceInstance.h index b37e14a0d..7d99f5243 100644 --- a/Tactility/Private/Tactility/service/ServiceInstance.h +++ b/Tactility/Private/Tactility/service/ServiceInstance.h @@ -1,36 +1,43 @@ #pragma once #include -#include -#include +#include +#include + +#include #include namespace tt::service { +/** + * Thin wrapper around the running TactilityKernel service context (::ServiceContext, + * which is the same object as ::ServiceInstance in the C API). Non-owning: valid only + * for as long as the underlying kernel instance is running. + */ class ServiceInstance final : public ServiceContext { - Mutex mutex; - std::shared_ptr manifest; - std::shared_ptr service; - State state = State::Stopped; + ::ServiceContext* cContext; + + std::shared_ptr& getCppManifest(::ServiceContext* cContext) const { + const auto* cManifest = service_context_get_manifest(cContext); + return *static_cast*>(cManifest->context); + } public: - explicit ServiceInstance(std::shared_ptr manifest); + explicit ServiceInstance(::ServiceContext* cContext) : cContext(cContext) {} ~ServiceInstance() override = default; /** @return a reference to the service's manifest */ - const ServiceManifest& getManifest() const override; + const ServiceManifest& getManifest() const override { + return *getCppManifest(cContext); + } /** Retrieve the paths that are relevant to this service */ - std::unique_ptr getPaths() const override; - - std::shared_ptr getService() const { return service; } - - State getState() const { return state; } - - void setState(State newState) { state = newState; } + std::unique_ptr getPaths() const override { + return std::make_unique(getCppManifest(cContext)); + } }; } diff --git a/Tactility/Source/service/ServiceInstance.cpp b/Tactility/Source/service/ServiceInstance.cpp deleted file mode 100644 index cf76beb54..000000000 --- a/Tactility/Source/service/ServiceInstance.cpp +++ /dev/null @@ -1,19 +0,0 @@ -#include - -#include -#include - -namespace tt::service { - -ServiceInstance::ServiceInstance(std::shared_ptr manifest) : - manifest(manifest), - service(manifest->createService()) -{} - -const ServiceManifest& ServiceInstance::getManifest() const { return *manifest; } - -std::unique_ptr ServiceInstance::getPaths() const { - return std::make_unique(manifest); -} - -} \ No newline at end of file diff --git a/Tactility/Source/service/ServicePaths.cpp b/Tactility/Source/service/ServicePaths.cpp deleted file mode 100644 index 0cf6d4306..000000000 --- a/Tactility/Source/service/ServicePaths.cpp +++ /dev/null @@ -1,29 +0,0 @@ -#include - -#include -#include - -#include -#include - -namespace tt::service { - -std::string ServicePaths::getUserDataDirectory() const { - return std::format("{}/service/{}", tt::getUserDataPath(), manifest->id); -} - -std::string ServicePaths::getUserDataPath(const std::string& childPath) const { - assert(!childPath.starts_with('/')); - return std::format("{}/{}", getUserDataDirectory(), childPath); -} - -std::string ServicePaths::getAssetsDirectory() const { - return std::format("{}/service/{}/assets", tt::getUserDataPath(), manifest->id); -} - -std::string ServicePaths::getAssetsPath(const std::string& childPath) const { - assert(!childPath.starts_with('/')); - return std::format("{}/{}", getAssetsDirectory(), childPath); -} - -} diff --git a/Tactility/Source/service/ServiceRegistration.cpp b/Tactility/Source/service/ServiceRegistration.cpp index ed2dfc78c..1254932d7 100644 --- a/Tactility/Source/service/ServiceRegistration.cpp +++ b/Tactility/Source/service/ServiceRegistration.cpp @@ -1,43 +1,87 @@ #include -#include #include #include -#include - +#include +#include #include +#include + namespace tt::service { constexpr auto* TAG = "ServiceRegistration"; -typedef std::unordered_map> ManifestMap; -typedef std::unordered_map> ServiceInstanceMap; +static State toCppState(ServiceState state) { + switch (state) { + case SERVICE_STATE_STARTING: return State::Starting; + case SERVICE_STATE_STARTED: return State::Started; + case SERVICE_STATE_STOPPING: return State::Stopping; + case SERVICE_STATE_STOPPED: + default: return State::Stopped; + } +} + +// Bridges the kernel's context-free C ServiceManifest/Service callbacks to the +// C++ Service instances they wrap. Declared extern "C" to match the linkage of +// the C function-pointer types they're assigned to (see e.g. gpio_controller.cpp). +extern "C" { + +static error_t cppOnStartTrampoline(::Service* cService, ::ServiceInstance* cContext) { + auto& servicePtr = *static_cast*>(cService->data); + ServiceInstance context(cContext); + return servicePtr->onStart(context) ? ERROR_NONE : ERROR_RESOURCE; +} + +static void cppOnStopTrampoline(::Service* cService, ::ServiceInstance* cContext) { + auto& servicePtr = *static_cast*>(cService->data); + ServiceInstance context(cContext); + servicePtr->onStop(context); +} -static ManifestMap service_manifest_map; -static ServiceInstanceMap service_instance_map; +static ::Service* cppCreateServiceTrampoline(void* context) { + auto& cppManifest = *static_cast*>(context); + auto* cService = new ::Service(); + cService->data = new std::shared_ptr(cppManifest->createService()); + cService->on_start = cppOnStartTrampoline; + cService->on_stop = cppOnStopTrampoline; + return cService; +} -static Mutex manifest_mutex; -static Mutex instance_mutex; +static void cppDestroyServiceTrampoline(::Service* cService, void* /*context*/) { + delete static_cast*>(cService->data); + delete cService; +} + +} // extern "C" void addService(std::shared_ptr manifest, bool autoStart) { assert(manifest != nullptr); - // We'll move the manifest pointer, but we'll need to id later const auto& id = manifest->id; LOG_I(TAG, "Adding %s", id.c_str()); - manifest_mutex.lock(); - if (service_manifest_map[id] == nullptr) { - service_manifest_map[id] = std::move(manifest); - } else { + if (service_registration_find_manifest(id.c_str()) != nullptr) { LOG_E(TAG, "Service id in use: %s", id.c_str()); + return; } - manifest_mutex.unlock(); - if (autoStart) { - startService(id); + // The bridging C manifest and the C++ manifest it carries in .context are + // intentionally never freed: services are registered once and live for the + // process lifetime (there is no removeService()), matching the previous + // implementation's static, never-erased manifest map. + auto* cppManifestPtr = new std::shared_ptr(manifest); + auto* cManifest = new ::ServiceManifest { + .id = (*cppManifestPtr)->id.c_str(), + .create_service = cppCreateServiceTrampoline, + .destroy_service = cppDestroyServiceTrampoline, + .context = cppManifestPtr + }; + + error_t error = service_registration_add(cManifest, autoStart); + if (error != ERROR_NONE) { + LOG_E(TAG, "Failed to add service %s: %s", id.c_str(), error_to_string(error)); } } @@ -46,93 +90,53 @@ void addService(const ServiceManifest& manifest, bool autoStart) { } std::shared_ptr findManifestById(const std::string& id) { - manifest_mutex.lock(); - auto iterator = service_manifest_map.find(id); - auto manifest = iterator != service_manifest_map.end() ? iterator->second : nullptr; - manifest_mutex.unlock(); - return manifest; -} - -static std::shared_ptr findServiceInstanceById(const std::string& id) { - manifest_mutex.lock(); - auto iterator = service_instance_map.find(id); - auto service = iterator != service_instance_map.end() ? iterator->second : nullptr; - manifest_mutex.unlock(); - return service; + const auto* cManifest = service_registration_find_manifest(id.c_str()); + if (cManifest == nullptr) { + return nullptr; + } + return *static_cast*>(cManifest->context); } -// TODO: Return proper error/status instead of BOOL? bool startService(const std::string& id) { LOG_I(TAG, "Starting %s", id.c_str()); - auto manifest = findManifestById(id); - if (manifest == nullptr) { - LOG_E(TAG, "manifest not found for service %s", id.c_str()); + error_t error = service_registration_start(id.c_str()); + if (error != ERROR_NONE) { + LOG_E(TAG, "Starting %s failed: %s", id.c_str(), error_to_string(error)); return false; } - - auto service_instance = std::make_shared(manifest); - - // Register first, so that a service can retrieve itself during onStart() - instance_mutex.lock(); - service_instance_map[manifest->id] = service_instance; - instance_mutex.unlock(); - - service_instance->setState(State::Starting); - if (service_instance->getService()->onStart(*service_instance)) { - service_instance->setState(State::Started); - } else { - LOG_E(TAG, "Starting %s failed", id.c_str()); - service_instance->setState(State::Stopped); - instance_mutex.lock(); - service_instance_map.erase(manifest->id); - instance_mutex.unlock(); - } - LOG_I(TAG, "Started %s", id.c_str()); - return true; } std::shared_ptr findServiceContextById(const std::string& id) { - return findServiceInstanceById(id); + auto* cContext = service_registration_find_context(id.c_str()); + if (cContext == nullptr) { + return nullptr; + } + return std::make_shared(cContext); } std::shared_ptr findServiceById(const std::string& id) { - auto instance = findServiceInstanceById(id); - return instance != nullptr ? instance->getService() : nullptr; + auto* cService = service_registration_find_service(id.c_str()); + if (cService == nullptr) { + return nullptr; + } + return *static_cast*>(cService->data); } bool stopService(const std::string& id) { LOG_I(TAG, "Stopping %s", id.c_str()); - auto service_instance = findServiceInstanceById(id); - if (service_instance == nullptr) { + error_t error = service_registration_stop(id.c_str()); + if (error != ERROR_NONE) { LOG_W(TAG, "Service not running: %s", id.c_str()); return false; } - - service_instance->setState(State::Stopping); - service_instance->getService()->onStop(*service_instance); - service_instance->setState(State::Stopped); - - instance_mutex.lock(); - service_instance_map.erase(id); - instance_mutex.unlock(); - - if (service_instance.use_count() > 1) { - LOG_W(TAG, "Possible memory leak: service %s still has %d references", service_instance->getManifest().id.c_str(), (int)(service_instance.use_count() - 1)); - } - LOG_I(TAG, "Stopped %s", id.c_str()); - return true; } State getState(const std::string& id) { - auto service_instance = findServiceInstanceById(id); - if (service_instance == nullptr) { - return State::Stopped; - } - return service_instance->getState(); + return toCppState(service_registration_get_state(id.c_str())); } } // namespace diff --git a/TactilityKernel/include/tactility/paths.h b/TactilityKernel/include/tactility/paths.h new file mode 100644 index 000000000..e8277c283 --- /dev/null +++ b/TactilityKernel/include/tactility/paths.h @@ -0,0 +1,24 @@ +// SPDX-License-Identifier: Apache-2.0 + +#pragma once + +#include +#include + +#ifdef __cplusplus +extern "C" { +#endif + +/** + * @brief Get the root path for user data. Survives OS upgrades. + * @param[out] out_path buffer to store the path (no trailing "/") + * @param[in] out_path_size size of the output buffer + * @retval ERROR_NOT_FOUND if the configured storage location isn't available (e.g. no SD card) + * @retval ERROR_BUFFER_OVERFLOW if out_path_size is too small + * @retval ERROR_NONE on success + */ +error_t paths_get_user_data_path(char* out_path, size_t out_path_size); + +#ifdef __cplusplus +} +#endif diff --git a/TactilityKernel/include/tactility/service/service.h b/TactilityKernel/include/tactility/service/service.h new file mode 100644 index 000000000..8dcf459d2 --- /dev/null +++ b/TactilityKernel/include/tactility/service/service.h @@ -0,0 +1,42 @@ +// SPDX-License-Identifier: Apache-2.0 + +#pragma once + +#include + +#ifdef __cplusplus +extern "C" { +#endif + +// ServiceContext (declared in service_context.h) is a typedef alias of ServiceInstance, +// so callbacks below are declared directly in terms of ServiceInstance to avoid a +// conflicting forward-declaration of an unrelated "ServiceContext" struct tag. +struct ServiceInstance; + +/** + * A service is a long-running background process (e.g. Wi-Fi, GPS, GUI). + * Concrete services keep their own state behind the `data` pointer. + */ +struct Service { + /** Service-specific data, owned by the service implementation. Can be NULL. */ + void* data; + /** + * Called when the service is starting. + * Can be NULL, in which case starting always succeeds. + * @param[in,out] service this service + * @param[in,out] context the context (a ServiceInstance) for this running service + * @return ERROR_NONE if the service started successfully + */ + error_t (*on_start)(struct Service* service, struct ServiceInstance* context); + /** + * Called when the service is stopping. + * Can be NULL, in which case stopping is a no-op. + * @param[in,out] service this service + * @param[in,out] context the context (a ServiceInstance) for this running service + */ + void (*on_stop)(struct Service* service, struct ServiceInstance* context); +}; + +#ifdef __cplusplus +} +#endif diff --git a/TactilityKernel/include/tactility/service/service_context.h b/TactilityKernel/include/tactility/service/service_context.h new file mode 100644 index 000000000..681886392 --- /dev/null +++ b/TactilityKernel/include/tactility/service/service_context.h @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: Apache-2.0 + +#pragma once + +#include + +#ifdef __cplusplus +extern "C" { +#endif + +/** + * The context handed to a service's on_start/on_stop callbacks. + * This is the same object as the ServiceInstance that owns the service (C has no + * interface/inheritance to distinguish a restricted "context" view from the full + * instance, so the two are deliberately the same type under different names). + */ +typedef struct ServiceInstance ServiceContext; + +/** + * @brief Get the manifest of the running service. + * @param[in] context non-null service context pointer + * @return the manifest + */ +const struct ServiceManifest* service_context_get_manifest(ServiceContext* context); + +#ifdef __cplusplus +} +#endif diff --git a/TactilityKernel/include/tactility/service/service_instance.h b/TactilityKernel/include/tactility/service/service_instance.h new file mode 100644 index 000000000..0c8907af0 --- /dev/null +++ b/TactilityKernel/include/tactility/service/service_instance.h @@ -0,0 +1,78 @@ +// SPDX-License-Identifier: Apache-2.0 + +#pragma once + +#include +#include + +#ifdef __cplusplus +extern "C" { +#endif + +struct ServiceInstanceInternal; + +/** The lifecycle state of a ServiceInstance. */ +typedef enum { + SERVICE_STATE_STARTING, + SERVICE_STATE_STARTED, + SERVICE_STATE_STOPPING, + SERVICE_STATE_STOPPED, +} ServiceState; + +/** Represents a running (or about-to-run) instance of a service. */ +struct ServiceInstance { + /** The manifest that spawned this instance. */ + const struct ServiceManifest* manifest; + /** The service created via manifest->create_service(). */ + struct Service* service; + /** + * Internal state managed by the kernel. + * ServiceInstance implementers should initialize this to NULL. + * Do not access or modify directly; use service_instance_* functions. + */ + struct ServiceInstanceInternal* internal; +}; + +/** + * @brief Construct a service instance. + * @details This calls manifest->create_service() to build the service. + * @param[in,out] instance a service instance with manifest set and internal set to NULL + * @param[in] manifest non-null manifest to construct the instance from + * @retval ERROR_OUT_OF_MEMORY if internal data allocation failed + * @retval ERROR_NONE on success + */ +error_t service_instance_construct(struct ServiceInstance* instance, const struct ServiceManifest* manifest); + +/** + * @brief Destruct a service instance. + * @details This calls manifest->destroy_service() on the service. + * @param[in,out] instance non-null service instance pointer + * @retval ERROR_INVALID_STATE if the instance is not stopped + * @retval ERROR_NONE on success + */ +error_t service_instance_destruct(struct ServiceInstance* instance); + +/** + * @brief Get the manifest of a service instance. + * @param[in] instance non-null service instance pointer + * @return the manifest + */ +const struct ServiceManifest* service_instance_get_manifest(struct ServiceInstance* instance); + +/** + * @brief Get the service of a service instance. + * @param[in] instance non-null service instance pointer + * @return the service + */ +struct Service* service_instance_get_service(struct ServiceInstance* instance); + +/** + * @brief Get the state of a service instance. + * @param[in] instance non-null service instance pointer + * @return the current state + */ +ServiceState service_instance_get_state(struct ServiceInstance* instance); + +#ifdef __cplusplus +} +#endif diff --git a/TactilityKernel/include/tactility/service/service_manifest.h b/TactilityKernel/include/tactility/service/service_manifest.h new file mode 100644 index 000000000..dbf08877a --- /dev/null +++ b/TactilityKernel/include/tactility/service/service_manifest.h @@ -0,0 +1,42 @@ +// SPDX-License-Identifier: Apache-2.0 + +#pragma once + +#include + +#ifdef __cplusplus +extern "C" { +#endif + +/** + * Allocates and initializes a new Service instance. + * @param[in] context the manifest's context (ServiceManifest::context) + * @return the new service, never NULL + */ +typedef struct Service* (*ServiceCreate)(void* context); + +/** + * Frees a Service instance that was created by the matching ServiceCreate function. + * @param[in] service the service to free + * @param[in] context the manifest's context (ServiceManifest::context) + */ +typedef void (*ServiceDestroy)(struct Service* service, void* context); + +/** + * Describes a registrable service type. + * One manifest exists per service id, shared across start/stop cycles. + */ +struct ServiceManifest { + /** Unique service identifier. Should never be NULL. */ + const char* id; + /** Allocates and initializes a new Service instance. Should never be NULL. */ + ServiceCreate create_service; + /** Frees a Service instance created by create_service. Should never be NULL. */ + ServiceDestroy destroy_service; + /** Opaque context passed to create_service and destroy_service. Can be NULL. */ + void* context; +}; + +#ifdef __cplusplus +} +#endif diff --git a/TactilityKernel/include/tactility/service/service_paths.h b/TactilityKernel/include/tactility/service/service_paths.h new file mode 100644 index 000000000..e1d95e08c --- /dev/null +++ b/TactilityKernel/include/tactility/service/service_paths.h @@ -0,0 +1,56 @@ +// SPDX-License-Identifier: Apache-2.0 + +#pragma once + +#include +#include + +#ifdef __cplusplus +extern "C" { +#endif + +/** + * @brief Get the user data directory for a service. Survives OS upgrades. No trailing "/". + * @param[in] service_id non-null service id + * @param[out] out_path buffer to store the path + * @param[in] out_path_size size of the output buffer + * @retval ERROR_BUFFER_OVERFLOW if out_path_size is too small + * @retval ERROR_NONE on success + */ +error_t service_paths_get_user_data_directory(const char* service_id, char* out_path, size_t out_path_size); + +/** + * @brief Get a path within the user data directory for a service. + * @param[in] service_id non-null service id + * @param[in] child_path path without a "/" prefix + * @param[out] out_path buffer to store the path + * @param[in] out_path_size size of the output buffer + * @retval ERROR_BUFFER_OVERFLOW if out_path_size is too small + * @retval ERROR_NONE on success + */ +error_t service_paths_get_user_data_path(const char* service_id, const char* child_path, char* out_path, size_t out_path_size); + +/** + * @brief Get the assets directory for a service. Do not store configuration data here. No trailing "/". + * @param[in] service_id non-null service id + * @param[out] out_path buffer to store the path + * @param[in] out_path_size size of the output buffer + * @retval ERROR_BUFFER_OVERFLOW if out_path_size is too small + * @retval ERROR_NONE on success + */ +error_t service_paths_get_assets_directory(const char* service_id, char* out_path, size_t out_path_size); + +/** + * @brief Get a path within the assets directory for a service. + * @param[in] service_id non-null service id + * @param[in] child_path path without a "/" prefix + * @param[out] out_path buffer to store the path + * @param[in] out_path_size size of the output buffer + * @retval ERROR_BUFFER_OVERFLOW if out_path_size is too small + * @retval ERROR_NONE on success + */ +error_t service_paths_get_assets_path(const char* service_id, const char* child_path, char* out_path, size_t out_path_size); + +#ifdef __cplusplus +} +#endif diff --git a/TactilityKernel/include/tactility/service/service_registration.h b/TactilityKernel/include/tactility/service/service_registration.h new file mode 100644 index 000000000..9d93f7ee6 --- /dev/null +++ b/TactilityKernel/include/tactility/service/service_registration.h @@ -0,0 +1,82 @@ +// SPDX-License-Identifier: Apache-2.0 + +#pragma once + +#include +#include +#include +#include +#include + +#ifdef __cplusplus +extern "C" { +#endif + +/** + * @brief Register a service manifest. + * @param[in] manifest non-null manifest to register + * @param[in] auto_start if true, the service is started immediately after registration + * @retval ERROR_INVALID_ARGUMENT if a manifest with the same id is already registered + * @retval ERROR_RESOURCE if auto_start is true and starting the service failed + * @retval ERROR_NONE on success + */ +error_t service_registration_add(const struct ServiceManifest* manifest, bool auto_start); + +/** + * @brief Unregister a previously-added manifest. + * @param[in] id non-null service id + * @retval ERROR_INVALID_STATE if the service is still running + * @retval ERROR_NOT_FOUND if no manifest with this id is registered + * @retval ERROR_NONE on success + */ +error_t service_registration_remove(const char* id); + +/** + * @brief Start a registered service by id. + * @param[in] id non-null service id + * @retval ERROR_NOT_FOUND if no manifest with this id is registered + * @retval ERROR_INVALID_STATE if the service is already running + * @retval ERROR_RESOURCE if the service's on_start callback failed + * @retval ERROR_NONE on success + */ +error_t service_registration_start(const char* id); + +/** + * @brief Stop a running service by id. + * @param[in] id non-null service id + * @retval ERROR_NOT_FOUND if no service with this id is running + * @retval ERROR_NONE on success + */ +error_t service_registration_stop(const char* id); + +/** + * @brief Get the state of a service by id. + * @param[in] id non-null service id + * @return the current state, or SERVICE_STATE_STOPPED if the id is unknown + */ +ServiceState service_registration_get_state(const char* id); + +/** + * @brief Find a registered manifest by id. + * @param[in] id non-null service id + * @return the manifest, or NULL if not found + */ +const struct ServiceManifest* service_registration_find_manifest(const char* id); + +/** + * @brief Find the context of a running service by id. + * @param[in] id non-null service id + * @return the context, or NULL if the service isn't running + */ +ServiceContext* service_registration_find_context(const char* id); + +/** + * @brief Find the service instance of a running service by id. + * @param[in] id non-null service id + * @return the service, or NULL if not running + */ +struct Service* service_registration_find_service(const char* id); + +#ifdef __cplusplus +} +#endif diff --git a/TactilityKernel/source/paths.cpp b/TactilityKernel/source/paths.cpp new file mode 100644 index 000000000..08e86c6c4 --- /dev/null +++ b/TactilityKernel/source/paths.cpp @@ -0,0 +1,66 @@ +// SPDX-License-Identifier: Apache-2.0 + +#include +#include +#include +#include + +#include +#include + +static error_t get_user_data_root_path(char* out_path, size_t out_path_size) { +#if defined(CONFIG_TT_USER_DATA_LOCATION_INTERNAL) +#ifdef ESP_PLATFORM + const char* mount_point = "/data"; +#else + const char* mount_point = "data"; +#endif + if (std::strlen(mount_point) + 1 > out_path_size) { + return ERROR_BUFFER_OVERFLOW; + } + std::strcpy(out_path, mount_point); + return ERROR_NONE; +#elif defined(CONFIG_TT_USER_DATA_LOCATION_SD) + struct FileSystem* found = nullptr; + file_system_for_each(&found, [](FileSystem* fs, void* context) { + auto* owner = file_system_get_owner(fs); + if (owner == nullptr || device_get_type(owner) != &SDCARD_TYPE) { + return true; + } + *static_cast(context) = fs; + return false; + }); + if (found == nullptr) { + return ERROR_NOT_FOUND; + } + return file_system_get_path(found, out_path, out_path_size); +#else +#error CONFIG_TT_USER_DATA_* not set or unsupported +#endif +} + +extern "C" { + +error_t paths_get_user_data_path(char* out_path, size_t out_path_size) { +#ifdef ESP_PLATFORM + char root[64]; + error_t error = get_user_data_root_path(root, sizeof(root)); + if (error != ERROR_NONE) { + return error; + } + int written = std::snprintf(out_path, out_path_size, "%s/tactility", root); + if (written < 0 || (size_t)written >= out_path_size) { + return ERROR_BUFFER_OVERFLOW; + } + return ERROR_NONE; +#else + const char* fixed_path = "data"; + if (std::strlen(fixed_path) + 1 > out_path_size) { + return ERROR_BUFFER_OVERFLOW; + } + std::strcpy(out_path, fixed_path); + return ERROR_NONE; +#endif +} + +} // extern "C" diff --git a/TactilityKernel/source/service/service_instance.cpp b/TactilityKernel/source/service/service_instance.cpp new file mode 100644 index 000000000..ebb98d9fb --- /dev/null +++ b/TactilityKernel/source/service/service_instance.cpp @@ -0,0 +1,81 @@ +// SPDX-License-Identifier: Apache-2.0 + +#include +#include +#include +#include + +#include + +#define TAG "service_instance" + +struct ServiceInstanceInternal { + Mutex mutex {}; + ServiceState state = SERVICE_STATE_STOPPED; +}; + +extern "C" { + +error_t service_instance_construct(ServiceInstance* instance, const ServiceManifest* manifest) { + instance->internal = new(std::nothrow) ServiceInstanceInternal; + if (instance->internal == nullptr) { + return ERROR_OUT_OF_MEMORY; + } + mutex_construct(&instance->internal->mutex); + + instance->manifest = manifest; + instance->service = manifest->create_service(manifest->context); + + LOG_D(TAG, "construct %s", manifest->id); + return ERROR_NONE; +} + +error_t service_instance_destruct(ServiceInstance* instance) { + auto* internal = instance->internal; + + mutex_lock(&internal->mutex); + if (internal->state != SERVICE_STATE_STOPPED) { + mutex_unlock(&internal->mutex); + return ERROR_INVALID_STATE; + } + mutex_unlock(&internal->mutex); + + LOG_D(TAG, "destruct %s", instance->manifest->id); + + instance->manifest->destroy_service(instance->service, instance->manifest->context); + instance->service = nullptr; + + instance->internal = nullptr; + mutex_destruct(&internal->mutex); + delete internal; + + return ERROR_NONE; +} + +const ServiceManifest* service_instance_get_manifest(ServiceInstance* instance) { + return instance->manifest; +} + +Service* service_instance_get_service(ServiceInstance* instance) { + return instance->service; +} + +ServiceState service_instance_get_state(ServiceInstance* instance) { + mutex_lock(&instance->internal->mutex); + ServiceState state = instance->internal->state; + mutex_unlock(&instance->internal->mutex); + return state; +} + +/** Internal-only: mutate the state of a service instance. Used by service_registration.cpp. */ +void service_instance_set_state(ServiceInstance* instance, ServiceState state) { + mutex_lock(&instance->internal->mutex); + instance->internal->state = state; + mutex_unlock(&instance->internal->mutex); +} + +const ServiceManifest* service_context_get_manifest(ServiceContext* context) { + return service_instance_get_manifest(context); +} + +} // extern "C" diff --git a/TactilityKernel/source/service/service_paths.cpp b/TactilityKernel/source/service/service_paths.cpp new file mode 100644 index 000000000..b3156d286 --- /dev/null +++ b/TactilityKernel/source/service/service_paths.cpp @@ -0,0 +1,62 @@ +// SPDX-License-Identifier: Apache-2.0 + +#include +#include + +#include + +extern "C" { + +error_t service_paths_get_user_data_directory(const char* service_id, char* out_path, size_t out_path_size) { + char root[192]; + error_t error = paths_get_user_data_path(root, sizeof(root)); + if (error != ERROR_NONE) { + return error; + } + int written = std::snprintf(out_path, out_path_size, "%s/service/%s", root, service_id); + if (written < 0 || (size_t)written >= out_path_size) { + return ERROR_BUFFER_OVERFLOW; + } + return ERROR_NONE; +} + +error_t service_paths_get_user_data_path(const char* service_id, const char* child_path, char* out_path, size_t out_path_size) { + char directory[224]; + error_t error = service_paths_get_user_data_directory(service_id, directory, sizeof(directory)); + if (error != ERROR_NONE) { + return error; + } + int written = std::snprintf(out_path, out_path_size, "%s/%s", directory, child_path); + if (written < 0 || (size_t)written >= out_path_size) { + return ERROR_BUFFER_OVERFLOW; + } + return ERROR_NONE; +} + +error_t service_paths_get_assets_directory(const char* service_id, char* out_path, size_t out_path_size) { + char directory[224]; + error_t error = service_paths_get_user_data_directory(service_id, directory, sizeof(directory)); + if (error != ERROR_NONE) { + return error; + } + int written = std::snprintf(out_path, out_path_size, "%s/assets", directory); + if (written < 0 || (size_t)written >= out_path_size) { + return ERROR_BUFFER_OVERFLOW; + } + return ERROR_NONE; +} + +error_t service_paths_get_assets_path(const char* service_id, const char* child_path, char* out_path, size_t out_path_size) { + char directory[224]; + error_t error = service_paths_get_assets_directory(service_id, directory, sizeof(directory)); + if (error != ERROR_NONE) { + return error; + } + int written = std::snprintf(out_path, out_path_size, "%s/%s", directory, child_path); + if (written < 0 || (size_t)written >= out_path_size) { + return ERROR_BUFFER_OVERFLOW; + } + return ERROR_NONE; +} + +} // extern "C" diff --git a/TactilityKernel/source/service/service_registration.cpp b/TactilityKernel/source/service/service_registration.cpp new file mode 100644 index 000000000..685634944 --- /dev/null +++ b/TactilityKernel/source/service/service_registration.cpp @@ -0,0 +1,207 @@ +// SPDX-License-Identifier: Apache-2.0 + +#include +#include +#include + +#include +#include +#include + +#define TAG "service_registration" + +// Defined in service_instance.cpp. Internal-only: lets the registry drive state +// transitions without exposing mutation on the public ServiceInstance API. +extern "C" void service_instance_set_state(ServiceInstance* instance, ServiceState state); + +struct ManifestLedger { + std::unordered_map manifests; + Mutex mutex {}; + + ManifestLedger() { mutex_construct(&mutex); } + ~ManifestLedger() { mutex_destruct(&mutex); } +}; + +struct InstanceLedger { + std::unordered_map instances; + Mutex mutex {}; + + InstanceLedger() { mutex_construct(&mutex); } + ~InstanceLedger() { mutex_destruct(&mutex); } +}; + +static ManifestLedger& get_manifest_ledger() { + static ManifestLedger ledger; + return ledger; +} + +static InstanceLedger& get_instance_ledger() { + static InstanceLedger ledger; + return ledger; +} + +#define manifest_ledger get_manifest_ledger() +#define instance_ledger get_instance_ledger() + +extern "C" { + +error_t service_registration_add(const ServiceManifest* manifest, bool auto_start) { + mutex_lock(&manifest_ledger.mutex); + if (manifest_ledger.manifests.contains(manifest->id)) { + mutex_unlock(&manifest_ledger.mutex); + LOG_E(TAG, "Manifest with id '%s' is already registered", manifest->id); + return ERROR_INVALID_ARGUMENT; + } + manifest_ledger.manifests[manifest->id] = manifest; + mutex_unlock(&manifest_ledger.mutex); + + LOG_I(TAG, "add %s", manifest->id); + + if (auto_start) { + return service_registration_start(manifest->id); + } + + return ERROR_NONE; +} + +error_t service_registration_remove(const char* id) { + if (service_registration_find_context(id) != nullptr) { + return ERROR_INVALID_STATE; + } + + mutex_lock(&manifest_ledger.mutex); + const auto iterator = manifest_ledger.manifests.find(id); + if (iterator == manifest_ledger.manifests.end()) { + mutex_unlock(&manifest_ledger.mutex); + return ERROR_NOT_FOUND; + } + manifest_ledger.manifests.erase(iterator); + mutex_unlock(&manifest_ledger.mutex); + + LOG_I(TAG, "remove %s", id); + return ERROR_NONE; +} + +error_t service_registration_start(const char* id) { + mutex_lock(&manifest_ledger.mutex); + const auto manifest_iterator = manifest_ledger.manifests.find(id); + if (manifest_iterator == manifest_ledger.manifests.end()) { + mutex_unlock(&manifest_ledger.mutex); + return ERROR_NOT_FOUND; + } + const ServiceManifest* manifest = manifest_iterator->second; + mutex_unlock(&manifest_ledger.mutex); + + mutex_lock(&instance_ledger.mutex); + if (instance_ledger.instances.contains(id)) { + mutex_unlock(&instance_ledger.mutex); + return ERROR_INVALID_STATE; + } + + auto* instance = new(std::nothrow) ServiceInstance { .manifest = nullptr, .service = nullptr, .internal = nullptr }; + if (instance == nullptr) { + mutex_unlock(&instance_ledger.mutex); + return ERROR_OUT_OF_MEMORY; + } + + error_t error = service_instance_construct(instance, manifest); + if (error != ERROR_NONE) { + mutex_unlock(&instance_ledger.mutex); + delete instance; + return error; + } + + // Register before on_start() so a service can find itself while starting. + instance_ledger.instances[id] = instance; + mutex_unlock(&instance_ledger.mutex); + + service_instance_set_state(instance, SERVICE_STATE_STARTING); + + LOG_I(TAG, "start %s", id); + Service* service = instance->service; + error = (service->on_start != nullptr) ? service->on_start(service, instance) : ERROR_NONE; + + if (error == ERROR_NONE) { + service_instance_set_state(instance, SERVICE_STATE_STARTED); + return ERROR_NONE; + } + + LOG_E(TAG, "Failed to start %s: %s", id, error_to_string(error)); + service_instance_set_state(instance, SERVICE_STATE_STOPPED); + + mutex_lock(&instance_ledger.mutex); + instance_ledger.instances.erase(id); + mutex_unlock(&instance_ledger.mutex); + + service_instance_destruct(instance); + delete instance; + + return ERROR_RESOURCE; +} + +error_t service_registration_stop(const char* id) { + mutex_lock(&instance_ledger.mutex); + const auto iterator = instance_ledger.instances.find(id); + if (iterator == instance_ledger.instances.end()) { + mutex_unlock(&instance_ledger.mutex); + return ERROR_NOT_FOUND; + } + ServiceInstance* instance = iterator->second; + mutex_unlock(&instance_ledger.mutex); + + LOG_I(TAG, "stop %s", id); + + service_instance_set_state(instance, SERVICE_STATE_STOPPING); + + Service* service = instance->service; + if (service->on_stop != nullptr) { + service->on_stop(service, instance); + } + + service_instance_set_state(instance, SERVICE_STATE_STOPPED); + + mutex_lock(&instance_ledger.mutex); + instance_ledger.instances.erase(id); + mutex_unlock(&instance_ledger.mutex); + + service_instance_destruct(instance); + delete instance; + + return ERROR_NONE; +} + +ServiceState service_registration_get_state(const char* id) { + mutex_lock(&instance_ledger.mutex); + const auto iterator = instance_ledger.instances.find(id); + if (iterator == instance_ledger.instances.end()) { + mutex_unlock(&instance_ledger.mutex); + return SERVICE_STATE_STOPPED; + } + ServiceInstance* instance = iterator->second; + mutex_unlock(&instance_ledger.mutex); + + return service_instance_get_state(instance); +} + +const ServiceManifest* service_registration_find_manifest(const char* id) { + mutex_lock(&manifest_ledger.mutex); + const auto iterator = manifest_ledger.manifests.find(id); + const ServiceManifest* manifest = (iterator != manifest_ledger.manifests.end()) ? iterator->second : nullptr; + mutex_unlock(&manifest_ledger.mutex); + return manifest; +} + +ServiceContext* service_registration_find_context(const char* id) { + mutex_lock(&instance_ledger.mutex); + const auto iterator = instance_ledger.instances.find(id); + ServiceInstance* instance = (iterator != instance_ledger.instances.end()) ? iterator->second : nullptr; + mutex_unlock(&instance_ledger.mutex); + return instance; +} + +Service* service_registration_find_service(const char* id) { + ServiceInstance* instance = service_registration_find_context(id); + return (instance != nullptr) ? instance->service : nullptr; +} + +} // extern "C" diff --git a/Tests/TactilityKernel/Source/ServicePathsTest.cpp b/Tests/TactilityKernel/Source/ServicePathsTest.cpp new file mode 100644 index 000000000..289994e8d --- /dev/null +++ b/Tests/TactilityKernel/Source/ServicePathsTest.cpp @@ -0,0 +1,69 @@ +#include "doctest.h" +#include +#include + +#include +#include + +TEST_CASE("paths_get_user_data_path returns a non-empty path") { + char buffer[192]; + CHECK_EQ(paths_get_user_data_path(buffer, sizeof(buffer)), ERROR_NONE); + CHECK_GT(std::strlen(buffer), 0); +} + +TEST_CASE("paths_get_user_data_path reports overflow for a too-small buffer") { + char buffer[1]; + CHECK_EQ(paths_get_user_data_path(buffer, sizeof(buffer)), ERROR_BUFFER_OVERFLOW); +} + +TEST_CASE("service_paths_get_user_data_directory includes the service id") { + char root[192]; + REQUIRE_EQ(paths_get_user_data_path(root, sizeof(root)), ERROR_NONE); + + char buffer[224]; + CHECK_EQ(service_paths_get_user_data_directory("my-service", buffer, sizeof(buffer)), ERROR_NONE); + + std::string expected = std::string(root) + "/service/my-service"; + CHECK_EQ(std::string(buffer), expected); +} + +TEST_CASE("service_paths_get_user_data_path appends the child path") { + char directory[224]; + REQUIRE_EQ(service_paths_get_user_data_directory("my-service", directory, sizeof(directory)), ERROR_NONE); + + char buffer[256]; + CHECK_EQ(service_paths_get_user_data_path("my-service", "settings.properties", buffer, sizeof(buffer)), ERROR_NONE); + + std::string expected = std::string(directory) + "/settings.properties"; + CHECK_EQ(std::string(buffer), expected); +} + +TEST_CASE("service_paths_get_assets_directory is nested under the user data directory") { + char directory[224]; + REQUIRE_EQ(service_paths_get_user_data_directory("my-service", directory, sizeof(directory)), ERROR_NONE); + + char buffer[256]; + CHECK_EQ(service_paths_get_assets_directory("my-service", buffer, sizeof(buffer)), ERROR_NONE); + + std::string expected = std::string(directory) + "/assets"; + CHECK_EQ(std::string(buffer), expected); +} + +TEST_CASE("service_paths_get_assets_path appends the child path") { + char directory[224]; + REQUIRE_EQ(service_paths_get_assets_directory("my-service", directory, sizeof(directory)), ERROR_NONE); + + char buffer[256]; + CHECK_EQ(service_paths_get_assets_path("my-service", "icon.png", buffer, sizeof(buffer)), ERROR_NONE); + + std::string expected = std::string(directory) + "/icon.png"; + CHECK_EQ(std::string(buffer), expected); +} + +TEST_CASE("service_paths functions report overflow for a too-small buffer") { + char buffer[1]; + CHECK_EQ(service_paths_get_user_data_directory("my-service", buffer, sizeof(buffer)), ERROR_BUFFER_OVERFLOW); + CHECK_EQ(service_paths_get_user_data_path("my-service", "child", buffer, sizeof(buffer)), ERROR_BUFFER_OVERFLOW); + CHECK_EQ(service_paths_get_assets_directory("my-service", buffer, sizeof(buffer)), ERROR_BUFFER_OVERFLOW); + CHECK_EQ(service_paths_get_assets_path("my-service", "child", buffer, sizeof(buffer)), ERROR_BUFFER_OVERFLOW); +} diff --git a/Tests/TactilityKernel/Source/ServiceTest.cpp b/Tests/TactilityKernel/Source/ServiceTest.cpp new file mode 100644 index 000000000..82bca637c --- /dev/null +++ b/Tests/TactilityKernel/Source/ServiceTest.cpp @@ -0,0 +1,162 @@ +#include "doctest.h" +#include + +static int create_called = 0; +static int destroy_called = 0; +static int on_start_called = 0; +static int on_stop_called = 0; +static error_t on_start_result = ERROR_NONE; +static void* last_create_context = nullptr; +static void* last_destroy_context = nullptr; + +static Service* test_create_service(void* context) { + create_called++; + last_create_context = context; + static Service service; + service.data = nullptr; + service.on_start = [](Service*, ServiceContext*) -> error_t { + on_start_called++; + return on_start_result; + }; + service.on_stop = [](Service*, ServiceContext*) { + on_stop_called++; + }; + return &service; +} + +static void test_destroy_service(Service*, void* context) { + destroy_called++; + last_destroy_context = context; +} + +static void reset_counters() { + create_called = 0; + destroy_called = 0; + on_start_called = 0; + on_stop_called = 0; + on_start_result = ERROR_NONE; + last_create_context = nullptr; + last_destroy_context = nullptr; +} + +TEST_CASE("ServiceInstance construction and destruction") { + reset_counters(); + + static int context_marker = 0; + static const ServiceManifest manifest = { + .id = "instance-test", + .create_service = test_create_service, + .destroy_service = test_destroy_service, + .context = &context_marker + }; + + ServiceInstance instance = { .manifest = nullptr, .service = nullptr, .internal = nullptr }; + + CHECK_EQ(service_instance_construct(&instance, &manifest), ERROR_NONE); + CHECK_NE(instance.internal, nullptr); + CHECK_EQ(instance.manifest, &manifest); + CHECK_NE(instance.service, nullptr); + CHECK_EQ(create_called, 1); + CHECK_EQ(last_create_context, &context_marker); + CHECK_EQ(service_instance_get_state(&instance), SERVICE_STATE_STOPPED); + + CHECK_EQ(service_instance_destruct(&instance), ERROR_NONE); + CHECK_EQ(instance.internal, nullptr); + CHECK_EQ(destroy_called, 1); + CHECK_EQ(last_destroy_context, &context_marker); +} + +TEST_CASE("service_registration_add rejects duplicate ids") { + reset_counters(); + + static const ServiceManifest manifest = { + .id = "duplicate-test", + .create_service = test_create_service, + .destroy_service = test_destroy_service + }; + + CHECK_EQ(service_registration_add(&manifest, false), ERROR_NONE); + CHECK_EQ(service_registration_add(&manifest, false), ERROR_INVALID_ARGUMENT); + + CHECK_EQ(service_registration_remove("duplicate-test"), ERROR_NONE); +} + +TEST_CASE("service_registration start/stop lifecycle") { + reset_counters(); + + static const ServiceManifest manifest = { + .id = "lifecycle-test", + .create_service = test_create_service, + .destroy_service = test_destroy_service + }; + + CHECK_EQ(service_registration_add(&manifest, false), ERROR_NONE); + CHECK_EQ(service_registration_get_state("lifecycle-test"), SERVICE_STATE_STOPPED); + + CHECK_EQ(service_registration_start("lifecycle-test"), ERROR_NONE); + CHECK_EQ(on_start_called, 1); + CHECK_EQ(service_registration_get_state("lifecycle-test"), SERVICE_STATE_STARTED); + CHECK_NE(service_registration_find_context("lifecycle-test"), nullptr); + CHECK_NE(service_registration_find_service("lifecycle-test"), nullptr); + + // Starting again while already started should fail + CHECK_EQ(service_registration_start("lifecycle-test"), ERROR_INVALID_STATE); + + // Removing while running should fail + CHECK_EQ(service_registration_remove("lifecycle-test"), ERROR_INVALID_STATE); + + CHECK_EQ(service_registration_stop("lifecycle-test"), ERROR_NONE); + CHECK_EQ(on_stop_called, 1); + CHECK_EQ(service_registration_get_state("lifecycle-test"), SERVICE_STATE_STOPPED); + CHECK_EQ(service_registration_find_context("lifecycle-test"), nullptr); + + // Stopping again while already stopped should fail + CHECK_EQ(service_registration_stop("lifecycle-test"), ERROR_NOT_FOUND); + + CHECK_EQ(service_registration_remove("lifecycle-test"), ERROR_NONE); +} + +TEST_CASE("service_registration_add with auto_start") { + reset_counters(); + + static const ServiceManifest manifest = { + .id = "auto-start-test", + .create_service = test_create_service, + .destroy_service = test_destroy_service + }; + + CHECK_EQ(service_registration_add(&manifest, true), ERROR_NONE); + CHECK_EQ(on_start_called, 1); + CHECK_EQ(service_registration_get_state("auto-start-test"), SERVICE_STATE_STARTED); + + CHECK_EQ(service_registration_stop("auto-start-test"), ERROR_NONE); + CHECK_EQ(service_registration_remove("auto-start-test"), ERROR_NONE); +} + +TEST_CASE("service_registration_start failure leaves service stopped") { + reset_counters(); + on_start_result = ERROR_RESOURCE; + + static const ServiceManifest manifest = { + .id = "failing-start-test", + .create_service = test_create_service, + .destroy_service = test_destroy_service + }; + + CHECK_EQ(service_registration_add(&manifest, false), ERROR_NONE); + CHECK_EQ(service_registration_start("failing-start-test"), ERROR_RESOURCE); + CHECK_EQ(service_registration_get_state("failing-start-test"), SERVICE_STATE_STOPPED); + CHECK_EQ(service_registration_find_context("failing-start-test"), nullptr); + + CHECK_EQ(service_registration_remove("failing-start-test"), ERROR_NONE); +} + +TEST_CASE("service_registration lookup functions with unknown id") { + CHECK_EQ(service_registration_get_state("unknown-service-id"), SERVICE_STATE_STOPPED); + CHECK_EQ(service_registration_find_manifest("unknown-service-id"), nullptr); + CHECK_EQ(service_registration_find_context("unknown-service-id"), nullptr); + CHECK_EQ(service_registration_find_service("unknown-service-id"), nullptr); + CHECK_EQ(service_registration_start("unknown-service-id"), ERROR_NOT_FOUND); + CHECK_EQ(service_registration_stop("unknown-service-id"), ERROR_NOT_FOUND); + CHECK_EQ(service_registration_remove("unknown-service-id"), ERROR_NOT_FOUND); +} From d4793596137dae64259c3bf411e6fa34012a8412 Mon Sep 17 00:00:00 2001 From: Ken Van Hoeylandt Date: Sun, 5 Jul 2026 14:00:29 +0200 Subject: [PATCH 2/7] Fix BT paths --- .../Source/bluetooth/BluetoothPairedDevice.cpp | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/Tactility/Source/bluetooth/BluetoothPairedDevice.cpp b/Tactility/Source/bluetooth/BluetoothPairedDevice.cpp index a6b2c99dc..c425b48aa 100644 --- a/Tactility/Source/bluetooth/BluetoothPairedDevice.cpp +++ b/Tactility/Source/bluetooth/BluetoothPairedDevice.cpp @@ -1,5 +1,7 @@ #include +#include "Tactility/Paths.h" + #include #include #include @@ -16,13 +18,16 @@ namespace tt::bluetooth::settings { constexpr auto* TAG = "BluetoothPairedDevice"; // Use the same directory as the old service for backward compatibility. -constexpr auto* DATA_DIR = "/data/service/bluetooth"; constexpr auto* DEVICE_SETTINGS_FORMAT = "{}/{}.device.properties"; constexpr auto* KEY_NAME = "name"; constexpr auto* KEY_ADDR = "addr"; constexpr auto* KEY_AUTO_CONNECT = "autoConnect"; constexpr auto* KEY_PROFILE_ID = "profileId"; +static std::string getSettingsFilePath() { + return getUserDataPath() + "/service/bluetooth"; +} + std::string addrToHex(const std::array& addr) { std::stringstream stream; stream << std::hex; @@ -52,7 +57,7 @@ static bool hexToAddr(const std::string& hex, std::array& addr) { } static std::string getFilePath(const std::string& addr_hex) { - return std::format(DEVICE_SETTINGS_FORMAT, DATA_DIR, addr_hex); + return std::format(DEVICE_SETTINGS_FORMAT, getSettingsFilePath(), addr_hex); } bool hasFileForDevice(const std::string& addr_hex) { @@ -102,7 +107,10 @@ bool remove(const std::string& addr_hex) { std::vector loadAll() { std::vector entries; - file::scandir(DATA_DIR, entries, [](const dirent* entry) -> int { + if (!file::isDirectory(getSettingsFilePath())) { + return {}; + } + file::scandir(getSettingsFilePath(), entries, [](const dirent* entry) -> int { if (entry->d_type != file::TT_DT_REG && entry->d_type != file::TT_DT_UNKNOWN) return -1; std::string name = entry->d_name; return name.ends_with(".device.properties") ? 0 : -1; From 2433aeb2d88281413b76a7004cb311488c6fe159 Mon Sep 17 00:00:00 2001 From: Ken Van Hoeylandt Date: Sun, 5 Jul 2026 17:22:26 +0200 Subject: [PATCH 3/7] Remove C++ service state --- Tactility/Include/Tactility/service/Service.h | 9 +++------ Tactility/Include/Tactility/service/ServicePaths.h | 1 + Tactility/Source/lvgl/Lvgl.cpp | 4 ++-- Tactility/Source/service/ServiceRegistration.cpp | 12 +----------- 4 files changed, 7 insertions(+), 19 deletions(-) diff --git a/Tactility/Include/Tactility/service/Service.h b/Tactility/Include/Tactility/service/Service.h index dfb81b877..12aef3587 100644 --- a/Tactility/Include/Tactility/service/Service.h +++ b/Tactility/Include/Tactility/service/Service.h @@ -1,15 +1,12 @@ #pragma once +#include + #include namespace tt::service { -enum class State { - Starting, - Started, - Stopping, - Stopped -}; +using State = ::ServiceState; // Forward declaration class ServiceContext; diff --git a/Tactility/Include/Tactility/service/ServicePaths.h b/Tactility/Include/Tactility/service/ServicePaths.h index ee4f8685b..a219b613b 100644 --- a/Tactility/Include/Tactility/service/ServicePaths.h +++ b/Tactility/Include/Tactility/service/ServicePaths.h @@ -6,6 +6,7 @@ #include #include +#include #include #include diff --git a/Tactility/Source/lvgl/Lvgl.cpp b/Tactility/Source/lvgl/Lvgl.cpp index 672ab46fb..48c596345 100644 --- a/Tactility/Source/lvgl/Lvgl.cpp +++ b/Tactility/Source/lvgl/Lvgl.cpp @@ -102,7 +102,7 @@ void attachDevices() { // We search for the manifest first, because during the initial start() during boot // the service won't be registered yet. if (service::findManifestById("Gui") != nullptr) { - if (service::getState("Gui") == service::State::Stopped) { + if (service::getState("Gui") == SERVICE_STATE_STOPPED) { service::startService("Gui"); } else { LOG_E(TAG, "Gui service is not in Stopped state"); @@ -112,7 +112,7 @@ void attachDevices() { // We search for the manifest first, because during the initial start() during boot // the service won't be registered yet. if (service::findManifestById("Statusbar") != nullptr) { - if (service::getState("Statusbar") == service::State::Stopped) { + if (service::getState("Statusbar") == SERVICE_STATE_STOPPED) { service::startService("Statusbar"); } else { LOG_E(TAG, "Statusbar service is not in Stopped state"); diff --git a/Tactility/Source/service/ServiceRegistration.cpp b/Tactility/Source/service/ServiceRegistration.cpp index 1254932d7..5cb53464d 100644 --- a/Tactility/Source/service/ServiceRegistration.cpp +++ b/Tactility/Source/service/ServiceRegistration.cpp @@ -13,16 +13,6 @@ namespace tt::service { constexpr auto* TAG = "ServiceRegistration"; -static State toCppState(ServiceState state) { - switch (state) { - case SERVICE_STATE_STARTING: return State::Starting; - case SERVICE_STATE_STARTED: return State::Started; - case SERVICE_STATE_STOPPING: return State::Stopping; - case SERVICE_STATE_STOPPED: - default: return State::Stopped; - } -} - // Bridges the kernel's context-free C ServiceManifest/Service callbacks to the // C++ Service instances they wrap. Declared extern "C" to match the linkage of // the C function-pointer types they're assigned to (see e.g. gpio_controller.cpp). @@ -136,7 +126,7 @@ bool stopService(const std::string& id) { } State getState(const std::string& id) { - return toCppState(service_registration_get_state(id.c_str())); + return service_registration_get_state(id.c_str()); } } // namespace From 469cf0d2756ec533fc09dfd5169b9b0444b12631 Mon Sep 17 00:00:00 2001 From: Ken Van Hoeylandt Date: Sun, 5 Jul 2026 22:34:15 +0200 Subject: [PATCH 4/7] Improvements --- .../Source/service/ServiceRegistration.cpp | 45 ++++--- .../include/tactility/service/service.h | 42 ------- .../tactility/service/service_instance.h | 36 +++++- ...rvice_registration.h => service_manager.h} | 21 ++-- .../tactility/service/service_manifest.h | 24 ++-- .../source/service/service_instance.cpp | 34 ++++- ...e_registration.cpp => service_manager.cpp} | 35 +++--- Tests/TactilityKernel/Source/ServiceTest.cpp | 119 +++++++++++------- 8 files changed, 193 insertions(+), 163 deletions(-) delete mode 100644 TactilityKernel/include/tactility/service/service.h rename TactilityKernel/include/tactility/service/{service_registration.h => service_manager.h} (74%) rename TactilityKernel/source/service/{service_registration.cpp => service_manager.cpp} (82%) diff --git a/Tactility/Source/service/ServiceRegistration.cpp b/Tactility/Source/service/ServiceRegistration.cpp index 5cb53464d..f7a56a49f 100644 --- a/Tactility/Source/service/ServiceRegistration.cpp +++ b/Tactility/Source/service/ServiceRegistration.cpp @@ -3,9 +3,9 @@ #include #include -#include #include #include +#include #include @@ -18,30 +18,27 @@ constexpr auto* TAG = "ServiceRegistration"; // the C function-pointer types they're assigned to (see e.g. gpio_controller.cpp). extern "C" { -static error_t cppOnStartTrampoline(::Service* cService, ::ServiceInstance* cContext) { - auto& servicePtr = *static_cast*>(cService->data); +static error_t cppOnStartTrampoline(::ServiceInstance* cContext) { + auto& servicePtr = *static_cast*>(cContext->data); ServiceInstance context(cContext); return servicePtr->onStart(context) ? ERROR_NONE : ERROR_RESOURCE; } -static void cppOnStopTrampoline(::Service* cService, ::ServiceInstance* cContext) { - auto& servicePtr = *static_cast*>(cService->data); +static void cppOnStopTrampoline(::ServiceInstance* cContext) { + auto& servicePtr = *static_cast*>(cContext->data); ServiceInstance context(cContext); servicePtr->onStop(context); } -static ::Service* cppCreateServiceTrampoline(void* context) { +static void cppCreateServiceTrampoline(::ServiceInstance* cContext, void* context) { auto& cppManifest = *static_cast*>(context); - auto* cService = new ::Service(); - cService->data = new std::shared_ptr(cppManifest->createService()); - cService->on_start = cppOnStartTrampoline; - cService->on_stop = cppOnStopTrampoline; - return cService; + cContext->data = new std::shared_ptr(cppManifest->createService()); + cContext->on_start = cppOnStartTrampoline; + cContext->on_stop = cppOnStopTrampoline; } -static void cppDestroyServiceTrampoline(::Service* cService, void* /*context*/) { - delete static_cast*>(cService->data); - delete cService; +static void cppDestroyServiceTrampoline(::ServiceInstance* cContext, void* /*context*/) { + delete static_cast*>(cContext->data); } } // extern "C" @@ -52,7 +49,7 @@ void addService(std::shared_ptr manifest, bool autoStart) LOG_I(TAG, "Adding %s", id.c_str()); - if (service_registration_find_manifest(id.c_str()) != nullptr) { + if (service_manager_find_manifest(id.c_str()) != nullptr) { LOG_E(TAG, "Service id in use: %s", id.c_str()); return; } @@ -69,7 +66,7 @@ void addService(std::shared_ptr manifest, bool autoStart) .context = cppManifestPtr }; - error_t error = service_registration_add(cManifest, autoStart); + error_t error = service_manager_add(cManifest, autoStart); if (error != ERROR_NONE) { LOG_E(TAG, "Failed to add service %s: %s", id.c_str(), error_to_string(error)); } @@ -80,7 +77,7 @@ void addService(const ServiceManifest& manifest, bool autoStart) { } std::shared_ptr findManifestById(const std::string& id) { - const auto* cManifest = service_registration_find_manifest(id.c_str()); + const auto* cManifest = service_manager_find_manifest(id.c_str()); if (cManifest == nullptr) { return nullptr; } @@ -89,7 +86,7 @@ std::shared_ptr findManifestById(const std::string& id) { bool startService(const std::string& id) { LOG_I(TAG, "Starting %s", id.c_str()); - error_t error = service_registration_start(id.c_str()); + error_t error = service_manager_start(id.c_str()); if (error != ERROR_NONE) { LOG_E(TAG, "Starting %s failed: %s", id.c_str(), error_to_string(error)); return false; @@ -99,7 +96,7 @@ bool startService(const std::string& id) { } std::shared_ptr findServiceContextById(const std::string& id) { - auto* cContext = service_registration_find_context(id.c_str()); + auto* cContext = service_manager_find_context(id.c_str()); if (cContext == nullptr) { return nullptr; } @@ -107,16 +104,16 @@ std::shared_ptr findServiceContextById(const std::string& id) { } std::shared_ptr findServiceById(const std::string& id) { - auto* cService = service_registration_find_service(id.c_str()); - if (cService == nullptr) { + auto* cContext = service_manager_find_context(id.c_str()); + if (cContext == nullptr) { return nullptr; } - return *static_cast*>(cService->data); + return *static_cast*>(cContext->data); } bool stopService(const std::string& id) { LOG_I(TAG, "Stopping %s", id.c_str()); - error_t error = service_registration_stop(id.c_str()); + error_t error = service_manager_stop(id.c_str()); if (error != ERROR_NONE) { LOG_W(TAG, "Service not running: %s", id.c_str()); return false; @@ -126,7 +123,7 @@ bool stopService(const std::string& id) { } State getState(const std::string& id) { - return service_registration_get_state(id.c_str()); + return service_manager_get_state(id.c_str()); } } // namespace diff --git a/TactilityKernel/include/tactility/service/service.h b/TactilityKernel/include/tactility/service/service.h deleted file mode 100644 index 8dcf459d2..000000000 --- a/TactilityKernel/include/tactility/service/service.h +++ /dev/null @@ -1,42 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 - -#pragma once - -#include - -#ifdef __cplusplus -extern "C" { -#endif - -// ServiceContext (declared in service_context.h) is a typedef alias of ServiceInstance, -// so callbacks below are declared directly in terms of ServiceInstance to avoid a -// conflicting forward-declaration of an unrelated "ServiceContext" struct tag. -struct ServiceInstance; - -/** - * A service is a long-running background process (e.g. Wi-Fi, GPS, GUI). - * Concrete services keep their own state behind the `data` pointer. - */ -struct Service { - /** Service-specific data, owned by the service implementation. Can be NULL. */ - void* data; - /** - * Called when the service is starting. - * Can be NULL, in which case starting always succeeds. - * @param[in,out] service this service - * @param[in,out] context the context (a ServiceInstance) for this running service - * @return ERROR_NONE if the service started successfully - */ - error_t (*on_start)(struct Service* service, struct ServiceInstance* context); - /** - * Called when the service is stopping. - * Can be NULL, in which case stopping is a no-op. - * @param[in,out] service this service - * @param[in,out] context the context (a ServiceInstance) for this running service - */ - void (*on_stop)(struct Service* service, struct ServiceInstance* context); -}; - -#ifdef __cplusplus -} -#endif diff --git a/TactilityKernel/include/tactility/service/service_instance.h b/TactilityKernel/include/tactility/service/service_instance.h index 0c8907af0..b8c528f3d 100644 --- a/TactilityKernel/include/tactility/service/service_instance.h +++ b/TactilityKernel/include/tactility/service/service_instance.h @@ -23,8 +23,21 @@ typedef enum { struct ServiceInstance { /** The manifest that spawned this instance. */ const struct ServiceManifest* manifest; - /** The service created via manifest->create_service(). */ - struct Service* service; + /** Service-specific data, owned by the service implementation. Can be NULL. */ + void* data; + /** + * Called when the service is starting. + * Can be NULL, in which case starting always succeeds. + * @param[in,out] instance this service instance (also its own ServiceContext) + * @return ERROR_NONE if the service started successfully + */ + error_t (*on_start)(struct ServiceInstance* instance); + /** + * Called when the service is stopping. + * Can be NULL, in which case stopping is a no-op. + * @param[in,out] instance this service instance (also its own ServiceContext) + */ + void (*on_stop)(struct ServiceInstance* instance); /** * Internal state managed by the kernel. * ServiceInstance implementers should initialize this to NULL. @@ -60,11 +73,11 @@ error_t service_instance_destruct(struct ServiceInstance* instance); const struct ServiceManifest* service_instance_get_manifest(struct ServiceInstance* instance); /** - * @brief Get the service of a service instance. + * @brief Get the data of a service instance. * @param[in] instance non-null service instance pointer - * @return the service + * @return the data (can be NULL) */ -struct Service* service_instance_get_service(struct ServiceInstance* instance); +void* service_instance_get_data(struct ServiceInstance* instance); /** * @brief Get the state of a service instance. @@ -73,6 +86,19 @@ struct Service* service_instance_get_service(struct ServiceInstance* instance); */ ServiceState service_instance_get_state(struct ServiceInstance* instance); +/** + * @brief Try to claim usage for this service instance. Increases reference count internally. + * @param instance non-null service instance pointer + * @return true when the instance is started and ref count was increased. + */ +bool service_instance_try_get(struct ServiceInstance* instance); + +/** + * @brief Release a claim for usage of this service instance. Decreases reference count internally. + * @param instance non-null service instance pointer + */ +void service_instance_put(struct ServiceInstance* instance); + #ifdef __cplusplus } #endif diff --git a/TactilityKernel/include/tactility/service/service_registration.h b/TactilityKernel/include/tactility/service/service_manager.h similarity index 74% rename from TactilityKernel/include/tactility/service/service_registration.h rename to TactilityKernel/include/tactility/service/service_manager.h index 9d93f7ee6..69c1f6336 100644 --- a/TactilityKernel/include/tactility/service/service_registration.h +++ b/TactilityKernel/include/tactility/service/service_manager.h @@ -20,7 +20,7 @@ extern "C" { * @retval ERROR_RESOURCE if auto_start is true and starting the service failed * @retval ERROR_NONE on success */ -error_t service_registration_add(const struct ServiceManifest* manifest, bool auto_start); + error_t service_manager_add(const struct ServiceManifest* manifest, bool auto_start); /** * @brief Unregister a previously-added manifest. @@ -29,7 +29,7 @@ error_t service_registration_add(const struct ServiceManifest* manifest, bool au * @retval ERROR_NOT_FOUND if no manifest with this id is registered * @retval ERROR_NONE on success */ -error_t service_registration_remove(const char* id); +error_t service_manager_remove(const char* id); /** * @brief Start a registered service by id. @@ -39,7 +39,7 @@ error_t service_registration_remove(const char* id); * @retval ERROR_RESOURCE if the service's on_start callback failed * @retval ERROR_NONE on success */ -error_t service_registration_start(const char* id); +error_t service_manager_start(const char* id); /** * @brief Stop a running service by id. @@ -47,35 +47,28 @@ error_t service_registration_start(const char* id); * @retval ERROR_NOT_FOUND if no service with this id is running * @retval ERROR_NONE on success */ -error_t service_registration_stop(const char* id); +error_t service_manager_stop(const char* id); /** * @brief Get the state of a service by id. * @param[in] id non-null service id * @return the current state, or SERVICE_STATE_STOPPED if the id is unknown */ -ServiceState service_registration_get_state(const char* id); +ServiceState service_manager_get_state(const char* id); /** * @brief Find a registered manifest by id. * @param[in] id non-null service id * @return the manifest, or NULL if not found */ -const struct ServiceManifest* service_registration_find_manifest(const char* id); +const struct ServiceManifest* service_manager_find_manifest(const char* id); /** * @brief Find the context of a running service by id. * @param[in] id non-null service id * @return the context, or NULL if the service isn't running */ -ServiceContext* service_registration_find_context(const char* id); - -/** - * @brief Find the service instance of a running service by id. - * @param[in] id non-null service id - * @return the service, or NULL if not running - */ -struct Service* service_registration_find_service(const char* id); +ServiceContext* service_manager_find_context(const char* id); #ifdef __cplusplus } diff --git a/TactilityKernel/include/tactility/service/service_manifest.h b/TactilityKernel/include/tactility/service/service_manifest.h index dbf08877a..989b8e115 100644 --- a/TactilityKernel/include/tactility/service/service_manifest.h +++ b/TactilityKernel/include/tactility/service/service_manifest.h @@ -2,25 +2,35 @@ #pragma once -#include +#include #ifdef __cplusplus extern "C" { #endif +// ServiceContext (declared in service_context.h) is a typedef alias of ServiceInstance, +// so ServiceCreate/ServiceDestroy below are declared directly in terms of ServiceInstance +// to avoid a conflicting forward-declaration of an unrelated "ServiceContext" struct tag. +struct ServiceInstance; + /** - * Allocates and initializes a new Service instance. + * Initializes a newly-registered service instance in place. + * Implementations should set instance->data, instance->on_start and instance->on_stop + * as needed; all three may be left at their zeroed defaults (NULL) if the service has + * no state or lifecycle callbacks. + * @param[in,out] instance the instance to populate (manifest and internal are already set) * @param[in] context the manifest's context (ServiceManifest::context) - * @return the new service, never NULL */ -typedef struct Service* (*ServiceCreate)(void* context); +typedef void (*ServiceCreate)(struct ServiceInstance* instance, void* context); /** - * Frees a Service instance that was created by the matching ServiceCreate function. - * @param[in] service the service to free + * Tears down state that was set up by the matching ServiceCreate function + * (e.g. frees instance->data). Should not clear instance->data/on_start/on_stop; + * the caller does that. + * @param[in,out] instance the instance to tear down * @param[in] context the manifest's context (ServiceManifest::context) */ -typedef void (*ServiceDestroy)(struct Service* service, void* context); +typedef void (*ServiceDestroy)(struct ServiceInstance* instance, void* context); /** * Describes a registrable service type. diff --git a/TactilityKernel/source/service/service_instance.cpp b/TactilityKernel/source/service/service_instance.cpp index ebb98d9fb..05e100927 100644 --- a/TactilityKernel/source/service/service_instance.cpp +++ b/TactilityKernel/source/service/service_instance.cpp @@ -12,6 +12,7 @@ struct ServiceInstanceInternal { Mutex mutex {}; ServiceState state = SERVICE_STATE_STOPPED; + uint32_t use_count = 0; }; extern "C" { @@ -24,7 +25,10 @@ error_t service_instance_construct(ServiceInstance* instance, const ServiceManif mutex_construct(&instance->internal->mutex); instance->manifest = manifest; - instance->service = manifest->create_service(manifest->context); + instance->data = nullptr; + instance->on_start = nullptr; + instance->on_stop = nullptr; + manifest->create_service(instance, manifest->context); LOG_D(TAG, "construct %s", manifest->id); return ERROR_NONE; @@ -42,8 +46,10 @@ error_t service_instance_destruct(ServiceInstance* instance) { LOG_D(TAG, "destruct %s", instance->manifest->id); - instance->manifest->destroy_service(instance->service, instance->manifest->context); - instance->service = nullptr; + instance->manifest->destroy_service(instance, instance->manifest->context); + instance->data = nullptr; + instance->on_start = nullptr; + instance->on_stop = nullptr; instance->internal = nullptr; mutex_destruct(&internal->mutex); @@ -56,8 +62,8 @@ const ServiceManifest* service_instance_get_manifest(ServiceInstance* instance) return instance->manifest; } -Service* service_instance_get_service(ServiceInstance* instance) { - return instance->service; +void* service_instance_get_data(ServiceInstance* instance) { + return instance->data; } ServiceState service_instance_get_state(ServiceInstance* instance) { @@ -78,4 +84,22 @@ const ServiceManifest* service_context_get_manifest(ServiceContext* context) { return service_instance_get_manifest(context); } +bool service_instance_try_get(struct ServiceInstance* instance) { + mutex_lock(&instance->internal->mutex); + bool acquired = instance->internal->state == SERVICE_STATE_STARTED; + if (acquired) { + instance->internal->use_count++; + } + mutex_unlock(&instance->internal->mutex); + return acquired; +} + +void service_instance_put(struct ServiceInstance* instance) { + mutex_lock(&instance->internal->mutex); + if (instance->internal->use_count > 0) { + instance->internal->use_count--; + } + mutex_unlock(&instance->internal->mutex); +} + } // extern "C" diff --git a/TactilityKernel/source/service/service_registration.cpp b/TactilityKernel/source/service/service_manager.cpp similarity index 82% rename from TactilityKernel/source/service/service_registration.cpp rename to TactilityKernel/source/service/service_manager.cpp index 685634944..35d921314 100644 --- a/TactilityKernel/source/service/service_registration.cpp +++ b/TactilityKernel/source/service/service_manager.cpp @@ -1,6 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 -#include +#include #include #include @@ -45,7 +45,7 @@ static InstanceLedger& get_instance_ledger() { extern "C" { -error_t service_registration_add(const ServiceManifest* manifest, bool auto_start) { +error_t service_manager_add(const ServiceManifest* manifest, bool auto_start) { mutex_lock(&manifest_ledger.mutex); if (manifest_ledger.manifests.contains(manifest->id)) { mutex_unlock(&manifest_ledger.mutex); @@ -58,14 +58,14 @@ error_t service_registration_add(const ServiceManifest* manifest, bool auto_star LOG_I(TAG, "add %s", manifest->id); if (auto_start) { - return service_registration_start(manifest->id); + return service_manager_start(manifest->id); } return ERROR_NONE; } -error_t service_registration_remove(const char* id) { - if (service_registration_find_context(id) != nullptr) { +error_t service_manager_remove(const char* id) { + if (service_manager_find_context(id) != nullptr) { return ERROR_INVALID_STATE; } @@ -82,7 +82,7 @@ error_t service_registration_remove(const char* id) { return ERROR_NONE; } -error_t service_registration_start(const char* id) { +error_t service_manager_start(const char* id) { mutex_lock(&manifest_ledger.mutex); const auto manifest_iterator = manifest_ledger.manifests.find(id); if (manifest_iterator == manifest_ledger.manifests.end()) { @@ -98,7 +98,7 @@ error_t service_registration_start(const char* id) { return ERROR_INVALID_STATE; } - auto* instance = new(std::nothrow) ServiceInstance { .manifest = nullptr, .service = nullptr, .internal = nullptr }; + auto* instance = new(std::nothrow) ServiceInstance { .manifest = nullptr, .data = nullptr, .on_start = nullptr, .on_stop = nullptr, .internal = nullptr }; if (instance == nullptr) { mutex_unlock(&instance_ledger.mutex); return ERROR_OUT_OF_MEMORY; @@ -118,8 +118,7 @@ error_t service_registration_start(const char* id) { service_instance_set_state(instance, SERVICE_STATE_STARTING); LOG_I(TAG, "start %s", id); - Service* service = instance->service; - error = (service->on_start != nullptr) ? service->on_start(service, instance) : ERROR_NONE; + error = (instance->on_start != nullptr) ? instance->on_start(instance) : ERROR_NONE; if (error == ERROR_NONE) { service_instance_set_state(instance, SERVICE_STATE_STARTED); @@ -139,7 +138,7 @@ error_t service_registration_start(const char* id) { return ERROR_RESOURCE; } -error_t service_registration_stop(const char* id) { +error_t service_manager_stop(const char* id) { mutex_lock(&instance_ledger.mutex); const auto iterator = instance_ledger.instances.find(id); if (iterator == instance_ledger.instances.end()) { @@ -153,9 +152,8 @@ error_t service_registration_stop(const char* id) { service_instance_set_state(instance, SERVICE_STATE_STOPPING); - Service* service = instance->service; - if (service->on_stop != nullptr) { - service->on_stop(service, instance); + if (instance->on_stop != nullptr) { + instance->on_stop(instance); } service_instance_set_state(instance, SERVICE_STATE_STOPPED); @@ -170,7 +168,7 @@ error_t service_registration_stop(const char* id) { return ERROR_NONE; } -ServiceState service_registration_get_state(const char* id) { +ServiceState service_manager_get_state(const char* id) { mutex_lock(&instance_ledger.mutex); const auto iterator = instance_ledger.instances.find(id); if (iterator == instance_ledger.instances.end()) { @@ -183,7 +181,7 @@ ServiceState service_registration_get_state(const char* id) { return service_instance_get_state(instance); } -const ServiceManifest* service_registration_find_manifest(const char* id) { +const ServiceManifest* service_manager_find_manifest(const char* id) { mutex_lock(&manifest_ledger.mutex); const auto iterator = manifest_ledger.manifests.find(id); const ServiceManifest* manifest = (iterator != manifest_ledger.manifests.end()) ? iterator->second : nullptr; @@ -191,7 +189,7 @@ const ServiceManifest* service_registration_find_manifest(const char* id) { return manifest; } -ServiceContext* service_registration_find_context(const char* id) { +ServiceContext* service_manager_find_context(const char* id) { mutex_lock(&instance_ledger.mutex); const auto iterator = instance_ledger.instances.find(id); ServiceInstance* instance = (iterator != instance_ledger.instances.end()) ? iterator->second : nullptr; @@ -199,9 +197,4 @@ ServiceContext* service_registration_find_context(const char* id) { return instance; } -Service* service_registration_find_service(const char* id) { - ServiceInstance* instance = service_registration_find_context(id); - return (instance != nullptr) ? instance->service : nullptr; -} - } // extern "C" diff --git a/Tests/TactilityKernel/Source/ServiceTest.cpp b/Tests/TactilityKernel/Source/ServiceTest.cpp index 82bca637c..96e6456c8 100644 --- a/Tests/TactilityKernel/Source/ServiceTest.cpp +++ b/Tests/TactilityKernel/Source/ServiceTest.cpp @@ -1,5 +1,8 @@ #include "doctest.h" -#include +#include + +// Defined in service_instance.cpp. Internal-only, exposed here to test try_get/put gating. +extern "C" void service_instance_set_state(ServiceInstance* instance, ServiceState state); static int create_called = 0; static int destroy_called = 0; @@ -9,22 +12,20 @@ static error_t on_start_result = ERROR_NONE; static void* last_create_context = nullptr; static void* last_destroy_context = nullptr; -static Service* test_create_service(void* context) { +static void test_create_service(ServiceInstance* instance, void* context) { create_called++; last_create_context = context; - static Service service; - service.data = nullptr; - service.on_start = [](Service*, ServiceContext*) -> error_t { + instance->data = nullptr; + instance->on_start = [](ServiceInstance*) -> error_t { on_start_called++; return on_start_result; }; - service.on_stop = [](Service*, ServiceContext*) { + instance->on_stop = [](ServiceInstance*) { on_stop_called++; }; - return &service; } -static void test_destroy_service(Service*, void* context) { +static void test_destroy_service(ServiceInstance*, void* context) { destroy_called++; last_destroy_context = context; } @@ -50,12 +51,13 @@ TEST_CASE("ServiceInstance construction and destruction") { .context = &context_marker }; - ServiceInstance instance = { .manifest = nullptr, .service = nullptr, .internal = nullptr }; + ServiceInstance instance = { .manifest = nullptr, .data = nullptr, .on_start = nullptr, .on_stop = nullptr, .internal = nullptr }; CHECK_EQ(service_instance_construct(&instance, &manifest), ERROR_NONE); CHECK_NE(instance.internal, nullptr); CHECK_EQ(instance.manifest, &manifest); - CHECK_NE(instance.service, nullptr); + CHECK_NE(instance.on_start, nullptr); + CHECK_NE(instance.on_stop, nullptr); CHECK_EQ(create_called, 1); CHECK_EQ(last_create_context, &context_marker); CHECK_EQ(service_instance_get_state(&instance), SERVICE_STATE_STOPPED); @@ -66,7 +68,36 @@ TEST_CASE("ServiceInstance construction and destruction") { CHECK_EQ(last_destroy_context, &context_marker); } -TEST_CASE("service_registration_add rejects duplicate ids") { +TEST_CASE("service_instance_try_get/put reference counting") { + reset_counters(); + + static const ServiceManifest manifest = { + .id = "refcount-test", + .create_service = test_create_service, + .destroy_service = test_destroy_service + }; + + ServiceInstance instance = { .manifest = nullptr, .data = nullptr, .on_start = nullptr, .on_stop = nullptr, .internal = nullptr }; + CHECK_EQ(service_instance_construct(&instance, &manifest), ERROR_NONE); + + // Not started yet: claiming usage should fail + CHECK_FALSE(service_instance_try_get(&instance)); + + service_instance_set_state(&instance, SERVICE_STATE_STARTED); + + CHECK(service_instance_try_get(&instance)); + CHECK(service_instance_try_get(&instance)); + service_instance_put(&instance); + service_instance_put(&instance); + + // Extra put beyond the claimed count should not underflow + service_instance_put(&instance); + + service_instance_set_state(&instance, SERVICE_STATE_STOPPED); + CHECK_EQ(service_instance_destruct(&instance), ERROR_NONE); +} + +TEST_CASE("service_manager_add rejects duplicate ids") { reset_counters(); static const ServiceManifest manifest = { @@ -75,10 +106,10 @@ TEST_CASE("service_registration_add rejects duplicate ids") { .destroy_service = test_destroy_service }; - CHECK_EQ(service_registration_add(&manifest, false), ERROR_NONE); - CHECK_EQ(service_registration_add(&manifest, false), ERROR_INVALID_ARGUMENT); + CHECK_EQ(service_manager_add(&manifest, false), ERROR_NONE); + CHECK_EQ(service_manager_add(&manifest, false), ERROR_INVALID_ARGUMENT); - CHECK_EQ(service_registration_remove("duplicate-test"), ERROR_NONE); + CHECK_EQ(service_manager_remove("duplicate-test"), ERROR_NONE); } TEST_CASE("service_registration start/stop lifecycle") { @@ -90,33 +121,32 @@ TEST_CASE("service_registration start/stop lifecycle") { .destroy_service = test_destroy_service }; - CHECK_EQ(service_registration_add(&manifest, false), ERROR_NONE); - CHECK_EQ(service_registration_get_state("lifecycle-test"), SERVICE_STATE_STOPPED); + CHECK_EQ(service_manager_add(&manifest, false), ERROR_NONE); + CHECK_EQ(service_manager_get_state("lifecycle-test"), SERVICE_STATE_STOPPED); - CHECK_EQ(service_registration_start("lifecycle-test"), ERROR_NONE); + CHECK_EQ(service_manager_start("lifecycle-test"), ERROR_NONE); CHECK_EQ(on_start_called, 1); - CHECK_EQ(service_registration_get_state("lifecycle-test"), SERVICE_STATE_STARTED); - CHECK_NE(service_registration_find_context("lifecycle-test"), nullptr); - CHECK_NE(service_registration_find_service("lifecycle-test"), nullptr); + CHECK_EQ(service_manager_get_state("lifecycle-test"), SERVICE_STATE_STARTED); + CHECK_NE(service_manager_find_context("lifecycle-test"), nullptr); // Starting again while already started should fail - CHECK_EQ(service_registration_start("lifecycle-test"), ERROR_INVALID_STATE); + CHECK_EQ(service_manager_start("lifecycle-test"), ERROR_INVALID_STATE); // Removing while running should fail - CHECK_EQ(service_registration_remove("lifecycle-test"), ERROR_INVALID_STATE); + CHECK_EQ(service_manager_remove("lifecycle-test"), ERROR_INVALID_STATE); - CHECK_EQ(service_registration_stop("lifecycle-test"), ERROR_NONE); + CHECK_EQ(service_manager_stop("lifecycle-test"), ERROR_NONE); CHECK_EQ(on_stop_called, 1); - CHECK_EQ(service_registration_get_state("lifecycle-test"), SERVICE_STATE_STOPPED); - CHECK_EQ(service_registration_find_context("lifecycle-test"), nullptr); + CHECK_EQ(service_manager_get_state("lifecycle-test"), SERVICE_STATE_STOPPED); + CHECK_EQ(service_manager_find_context("lifecycle-test"), nullptr); // Stopping again while already stopped should fail - CHECK_EQ(service_registration_stop("lifecycle-test"), ERROR_NOT_FOUND); + CHECK_EQ(service_manager_stop("lifecycle-test"), ERROR_NOT_FOUND); - CHECK_EQ(service_registration_remove("lifecycle-test"), ERROR_NONE); + CHECK_EQ(service_manager_remove("lifecycle-test"), ERROR_NONE); } -TEST_CASE("service_registration_add with auto_start") { +TEST_CASE("service_manager_add with auto_start") { reset_counters(); static const ServiceManifest manifest = { @@ -125,15 +155,15 @@ TEST_CASE("service_registration_add with auto_start") { .destroy_service = test_destroy_service }; - CHECK_EQ(service_registration_add(&manifest, true), ERROR_NONE); + CHECK_EQ(service_manager_add(&manifest, true), ERROR_NONE); CHECK_EQ(on_start_called, 1); - CHECK_EQ(service_registration_get_state("auto-start-test"), SERVICE_STATE_STARTED); + CHECK_EQ(service_manager_get_state("auto-start-test"), SERVICE_STATE_STARTED); - CHECK_EQ(service_registration_stop("auto-start-test"), ERROR_NONE); - CHECK_EQ(service_registration_remove("auto-start-test"), ERROR_NONE); + CHECK_EQ(service_manager_stop("auto-start-test"), ERROR_NONE); + CHECK_EQ(service_manager_remove("auto-start-test"), ERROR_NONE); } -TEST_CASE("service_registration_start failure leaves service stopped") { +TEST_CASE("service_manager_start failure leaves service stopped") { reset_counters(); on_start_result = ERROR_RESOURCE; @@ -143,20 +173,19 @@ TEST_CASE("service_registration_start failure leaves service stopped") { .destroy_service = test_destroy_service }; - CHECK_EQ(service_registration_add(&manifest, false), ERROR_NONE); - CHECK_EQ(service_registration_start("failing-start-test"), ERROR_RESOURCE); - CHECK_EQ(service_registration_get_state("failing-start-test"), SERVICE_STATE_STOPPED); - CHECK_EQ(service_registration_find_context("failing-start-test"), nullptr); + CHECK_EQ(service_manager_add(&manifest, false), ERROR_NONE); + CHECK_EQ(service_manager_start("failing-start-test"), ERROR_RESOURCE); + CHECK_EQ(service_manager_get_state("failing-start-test"), SERVICE_STATE_STOPPED); + CHECK_EQ(service_manager_find_context("failing-start-test"), nullptr); - CHECK_EQ(service_registration_remove("failing-start-test"), ERROR_NONE); + CHECK_EQ(service_manager_remove("failing-start-test"), ERROR_NONE); } TEST_CASE("service_registration lookup functions with unknown id") { - CHECK_EQ(service_registration_get_state("unknown-service-id"), SERVICE_STATE_STOPPED); - CHECK_EQ(service_registration_find_manifest("unknown-service-id"), nullptr); - CHECK_EQ(service_registration_find_context("unknown-service-id"), nullptr); - CHECK_EQ(service_registration_find_service("unknown-service-id"), nullptr); - CHECK_EQ(service_registration_start("unknown-service-id"), ERROR_NOT_FOUND); - CHECK_EQ(service_registration_stop("unknown-service-id"), ERROR_NOT_FOUND); - CHECK_EQ(service_registration_remove("unknown-service-id"), ERROR_NOT_FOUND); + CHECK_EQ(service_manager_get_state("unknown-service-id"), SERVICE_STATE_STOPPED); + CHECK_EQ(service_manager_find_manifest("unknown-service-id"), nullptr); + CHECK_EQ(service_manager_find_context("unknown-service-id"), nullptr); + CHECK_EQ(service_manager_start("unknown-service-id"), ERROR_NOT_FOUND); + CHECK_EQ(service_manager_stop("unknown-service-id"), ERROR_NOT_FOUND); + CHECK_EQ(service_manager_remove("unknown-service-id"), ERROR_NOT_FOUND); } From e801dcf3972ec384271b044770311a8ffb94c644 Mon Sep 17 00:00:00 2001 From: Ken Van Hoeylandt Date: Sun, 5 Jul 2026 22:45:08 +0200 Subject: [PATCH 5/7] Remove try_get and put functions for services --- .../tactility/service/service_instance.h | 13 --------- .../tactility/service/service_manager.h | 14 ++++----- .../source/service/service_instance.cpp | 19 ------------ Tests/TactilityKernel/Source/ServiceTest.cpp | 29 ------------------- 4 files changed, 7 insertions(+), 68 deletions(-) diff --git a/TactilityKernel/include/tactility/service/service_instance.h b/TactilityKernel/include/tactility/service/service_instance.h index b8c528f3d..87dc1d681 100644 --- a/TactilityKernel/include/tactility/service/service_instance.h +++ b/TactilityKernel/include/tactility/service/service_instance.h @@ -86,19 +86,6 @@ void* service_instance_get_data(struct ServiceInstance* instance); */ ServiceState service_instance_get_state(struct ServiceInstance* instance); -/** - * @brief Try to claim usage for this service instance. Increases reference count internally. - * @param instance non-null service instance pointer - * @return true when the instance is started and ref count was increased. - */ -bool service_instance_try_get(struct ServiceInstance* instance); - -/** - * @brief Release a claim for usage of this service instance. Decreases reference count internally. - * @param instance non-null service instance pointer - */ -void service_instance_put(struct ServiceInstance* instance); - #ifdef __cplusplus } #endif diff --git a/TactilityKernel/include/tactility/service/service_manager.h b/TactilityKernel/include/tactility/service/service_manager.h index 69c1f6336..d4cfc74b7 100644 --- a/TactilityKernel/include/tactility/service/service_manager.h +++ b/TactilityKernel/include/tactility/service/service_manager.h @@ -31,6 +31,13 @@ extern "C" { */ error_t service_manager_remove(const char* id); +/** + * @brief Find a registered manifest by id. + * @param[in] id non-null service id + * @return the manifest, or NULL if not found + */ +const struct ServiceManifest* service_manager_find_manifest(const char* id); + /** * @brief Start a registered service by id. * @param[in] id non-null service id @@ -56,13 +63,6 @@ error_t service_manager_stop(const char* id); */ ServiceState service_manager_get_state(const char* id); -/** - * @brief Find a registered manifest by id. - * @param[in] id non-null service id - * @return the manifest, or NULL if not found - */ -const struct ServiceManifest* service_manager_find_manifest(const char* id); - /** * @brief Find the context of a running service by id. * @param[in] id non-null service id diff --git a/TactilityKernel/source/service/service_instance.cpp b/TactilityKernel/source/service/service_instance.cpp index 05e100927..768c78f32 100644 --- a/TactilityKernel/source/service/service_instance.cpp +++ b/TactilityKernel/source/service/service_instance.cpp @@ -12,7 +12,6 @@ struct ServiceInstanceInternal { Mutex mutex {}; ServiceState state = SERVICE_STATE_STOPPED; - uint32_t use_count = 0; }; extern "C" { @@ -84,22 +83,4 @@ const ServiceManifest* service_context_get_manifest(ServiceContext* context) { return service_instance_get_manifest(context); } -bool service_instance_try_get(struct ServiceInstance* instance) { - mutex_lock(&instance->internal->mutex); - bool acquired = instance->internal->state == SERVICE_STATE_STARTED; - if (acquired) { - instance->internal->use_count++; - } - mutex_unlock(&instance->internal->mutex); - return acquired; -} - -void service_instance_put(struct ServiceInstance* instance) { - mutex_lock(&instance->internal->mutex); - if (instance->internal->use_count > 0) { - instance->internal->use_count--; - } - mutex_unlock(&instance->internal->mutex); -} - } // extern "C" diff --git a/Tests/TactilityKernel/Source/ServiceTest.cpp b/Tests/TactilityKernel/Source/ServiceTest.cpp index 96e6456c8..5917e8cd2 100644 --- a/Tests/TactilityKernel/Source/ServiceTest.cpp +++ b/Tests/TactilityKernel/Source/ServiceTest.cpp @@ -68,35 +68,6 @@ TEST_CASE("ServiceInstance construction and destruction") { CHECK_EQ(last_destroy_context, &context_marker); } -TEST_CASE("service_instance_try_get/put reference counting") { - reset_counters(); - - static const ServiceManifest manifest = { - .id = "refcount-test", - .create_service = test_create_service, - .destroy_service = test_destroy_service - }; - - ServiceInstance instance = { .manifest = nullptr, .data = nullptr, .on_start = nullptr, .on_stop = nullptr, .internal = nullptr }; - CHECK_EQ(service_instance_construct(&instance, &manifest), ERROR_NONE); - - // Not started yet: claiming usage should fail - CHECK_FALSE(service_instance_try_get(&instance)); - - service_instance_set_state(&instance, SERVICE_STATE_STARTED); - - CHECK(service_instance_try_get(&instance)); - CHECK(service_instance_try_get(&instance)); - service_instance_put(&instance); - service_instance_put(&instance); - - // Extra put beyond the claimed count should not underflow - service_instance_put(&instance); - - service_instance_set_state(&instance, SERVICE_STATE_STOPPED); - CHECK_EQ(service_instance_destruct(&instance), ERROR_NONE); -} - TEST_CASE("service_manager_add rejects duplicate ids") { reset_counters(); From e3bd87a528973e9aa79c32100dbec5c276833f97 Mon Sep 17 00:00:00 2001 From: Ken Van Hoeylandt Date: Tue, 7 Jul 2026 21:07:12 +0200 Subject: [PATCH 6/7] Remove ServiceInstance --- .../Tactility/service/ServiceContext.h | 18 +++++--- .../Tactility/service/ServiceInstance.h | 43 ------------------- Tactility/Source/service/ServiceContext.cpp | 23 ++++++++++ .../Source/service/ServiceRegistration.cpp | 8 ++-- 4 files changed, 38 insertions(+), 54 deletions(-) delete mode 100644 Tactility/Private/Tactility/service/ServiceInstance.h create mode 100644 Tactility/Source/service/ServiceContext.cpp diff --git a/Tactility/Include/Tactility/service/ServiceContext.h b/Tactility/Include/Tactility/service/ServiceContext.h index 7362ebe13..553219c4f 100644 --- a/Tactility/Include/Tactility/service/ServiceContext.h +++ b/Tactility/Include/Tactility/service/ServiceContext.h @@ -2,29 +2,33 @@ #include +struct ServiceInstance; + namespace tt::service { struct ServiceManifest; class ServicePaths; /** - * The public representation of a service instance. + * Thin, non-owning wrapper around the running TactilityKernel service instance + * (::ServiceInstance, which the kernel API also calls ::ServiceContext). * @warning Do not store references or pointers to these! You can retrieve them via the Loader service. */ -class ServiceContext { +class ServiceContext final { -protected: + ::ServiceInstance* cContext; - virtual ~ServiceContext() = default; + std::shared_ptr& getCppManifest() const; public: + explicit ServiceContext(::ServiceInstance* cContext) : cContext(cContext) {} + /** @return a reference to the service's manifest */ - virtual const ServiceManifest& getManifest() const = 0; + const ServiceManifest& getManifest() const; /** Retrieve the paths that are relevant to this service */ - virtual std::unique_ptr getPaths() const = 0; + std::unique_ptr getPaths() const; }; - } // namespace diff --git a/Tactility/Private/Tactility/service/ServiceInstance.h b/Tactility/Private/Tactility/service/ServiceInstance.h deleted file mode 100644 index 7d99f5243..000000000 --- a/Tactility/Private/Tactility/service/ServiceInstance.h +++ /dev/null @@ -1,43 +0,0 @@ -#pragma once - -#include -#include -#include - -#include - -#include - -namespace tt::service { - -/** - * Thin wrapper around the running TactilityKernel service context (::ServiceContext, - * which is the same object as ::ServiceInstance in the C API). Non-owning: valid only - * for as long as the underlying kernel instance is running. - */ -class ServiceInstance final : public ServiceContext { - - ::ServiceContext* cContext; - - std::shared_ptr& getCppManifest(::ServiceContext* cContext) const { - const auto* cManifest = service_context_get_manifest(cContext); - return *static_cast*>(cManifest->context); - } - -public: - - explicit ServiceInstance(::ServiceContext* cContext) : cContext(cContext) {} - ~ServiceInstance() override = default; - - /** @return a reference to the service's manifest */ - const ServiceManifest& getManifest() const override { - return *getCppManifest(cContext); - } - - /** Retrieve the paths that are relevant to this service */ - std::unique_ptr getPaths() const override { - return std::make_unique(getCppManifest(cContext)); - } -}; - -} diff --git a/Tactility/Source/service/ServiceContext.cpp b/Tactility/Source/service/ServiceContext.cpp new file mode 100644 index 000000000..25eba862b --- /dev/null +++ b/Tactility/Source/service/ServiceContext.cpp @@ -0,0 +1,23 @@ +#include + +#include +#include + +#include + +namespace tt::service { + +std::shared_ptr& ServiceContext::getCppManifest() const { + const auto* cManifest = service_context_get_manifest(cContext); + return *static_cast*>(cManifest->context); +} + +const ServiceManifest& ServiceContext::getManifest() const { + return *getCppManifest(); +} + +std::unique_ptr ServiceContext::getPaths() const { + return std::make_unique(getCppManifest()); +} + +} // namespace diff --git a/Tactility/Source/service/ServiceRegistration.cpp b/Tactility/Source/service/ServiceRegistration.cpp index f7a56a49f..b0e33dcd4 100644 --- a/Tactility/Source/service/ServiceRegistration.cpp +++ b/Tactility/Source/service/ServiceRegistration.cpp @@ -1,6 +1,6 @@ #include -#include +#include #include #include @@ -20,13 +20,13 @@ extern "C" { static error_t cppOnStartTrampoline(::ServiceInstance* cContext) { auto& servicePtr = *static_cast*>(cContext->data); - ServiceInstance context(cContext); + ServiceContext context(cContext); return servicePtr->onStart(context) ? ERROR_NONE : ERROR_RESOURCE; } static void cppOnStopTrampoline(::ServiceInstance* cContext) { auto& servicePtr = *static_cast*>(cContext->data); - ServiceInstance context(cContext); + ServiceContext context(cContext); servicePtr->onStop(context); } @@ -100,7 +100,7 @@ std::shared_ptr findServiceContextById(const std::string& id) { if (cContext == nullptr) { return nullptr; } - return std::make_shared(cContext); + return std::make_shared(cContext); } std::shared_ptr findServiceById(const std::string& id) { From affdd61836af735ef5fcb469b3ad385833dba153 Mon Sep 17 00:00:00 2001 From: Ken Van Hoeylandt Date: Tue, 7 Jul 2026 22:17:42 +0200 Subject: [PATCH 7/7] Simplify --- Tactility/Include/Tactility/service/Service.h | 2 +- .../Tactility/service/ServiceContext.h | 11 ++- .../Tactility/service/ServiceManifest.h | 2 +- .../Include/Tactility/service/ServicePaths.h | 18 +++-- .../Tactility/service/ServiceRegistration.h | 2 +- Tactility/Source/service/ServiceContext.cpp | 14 ++-- .../Source/service/ServiceRegistration.cpp | 72 ++++++++----------- .../tactility/service/service_context.h | 28 -------- .../tactility/service/service_instance.h | 17 +---- .../tactility/service/service_manager.h | 7 +- .../tactility/service/service_manifest.h | 52 +++++++++----- TactilityKernel/source/kernel_symbols.c | 22 ++++++ .../source/service/service_instance.cpp | 14 +--- .../source/service/service_manager.cpp | 14 ++-- Tests/TactilityKernel/Source/ServiceTest.cpp | 66 +++++++++-------- 15 files changed, 156 insertions(+), 185 deletions(-) delete mode 100644 TactilityKernel/include/tactility/service/service_context.h diff --git a/Tactility/Include/Tactility/service/Service.h b/Tactility/Include/Tactility/service/Service.h index 12aef3587..670090a96 100644 --- a/Tactility/Include/Tactility/service/Service.h +++ b/Tactility/Include/Tactility/service/Service.h @@ -23,6 +23,6 @@ class Service { }; template -std::shared_ptr create() { return std::shared_ptr(new T); } +void* create(const ::ServiceManifest* /*manifest*/) { return new std::shared_ptr(std::shared_ptr(new T)); } } \ No newline at end of file diff --git a/Tactility/Include/Tactility/service/ServiceContext.h b/Tactility/Include/Tactility/service/ServiceContext.h index 553219c4f..3f9555ec9 100644 --- a/Tactility/Include/Tactility/service/ServiceContext.h +++ b/Tactility/Include/Tactility/service/ServiceContext.h @@ -1,6 +1,7 @@ #pragma once #include +#include struct ServiceInstance; @@ -11,21 +12,19 @@ class ServicePaths; /** * Thin, non-owning wrapper around the running TactilityKernel service instance - * (::ServiceInstance, which the kernel API also calls ::ServiceContext). + * (ServiceInstance, which the kernel API also calls ServiceContext). * @warning Do not store references or pointers to these! You can retrieve them via the Loader service. */ class ServiceContext final { - ::ServiceInstance* cContext; - - std::shared_ptr& getCppManifest() const; + ::ServiceInstance* service_instance; public: - explicit ServiceContext(::ServiceInstance* cContext) : cContext(cContext) {} + explicit ServiceContext(::ServiceInstance* instance) : service_instance(instance) {} /** @return a reference to the service's manifest */ - const ServiceManifest& getManifest() const; + const ::ServiceManifest* getManifest() const; /** Retrieve the paths that are relevant to this service */ std::unique_ptr getPaths() const; diff --git a/Tactility/Include/Tactility/service/ServiceManifest.h b/Tactility/Include/Tactility/service/ServiceManifest.h index 484855364..61b61a9b8 100644 --- a/Tactility/Include/Tactility/service/ServiceManifest.h +++ b/Tactility/Include/Tactility/service/ServiceManifest.h @@ -9,7 +9,7 @@ namespace tt::service { // Forward declarations class ServiceContext; -typedef std::shared_ptr(*CreateService)(); +using CreateService = ::ServiceCreate; /** A ledger that describes the main parts of a service. */ struct ServiceManifest { diff --git a/Tactility/Include/Tactility/service/ServicePaths.h b/Tactility/Include/Tactility/service/ServicePaths.h index a219b613b..04be7db87 100644 --- a/Tactility/Include/Tactility/service/ServicePaths.h +++ b/Tactility/Include/Tactility/service/ServicePaths.h @@ -1,14 +1,12 @@ #pragma once -#include - #include #include +#include #include #include #include -#include namespace tt::service { @@ -16,11 +14,11 @@ constexpr size_t PATH_BUFFER_SIZE = 255; class ServicePaths { - std::shared_ptr manifest; + const ::ServiceManifest* manifest; public: - explicit ServicePaths(std::shared_ptr manifest) : manifest(std::move(manifest)) {} + explicit ServicePaths(const ::ServiceManifest* manifest) : manifest(manifest) {} /** * The user data directory is intended to survive OS upgrades. @@ -28,7 +26,7 @@ class ServicePaths { */ std::string getUserDataDirectory() const { char buffer[PATH_BUFFER_SIZE]; - check(service_paths_get_user_data_directory(manifest->id.c_str(), buffer, sizeof(buffer)) == ERROR_NONE); + check(service_paths_get_user_data_directory(manifest->id, buffer, sizeof(buffer)) == ERROR_NONE); return buffer; } @@ -40,7 +38,7 @@ class ServicePaths { std::string getUserDataPath(const std::string& childPath) const { assert(!childPath.starts_with('/')); char buffer[PATH_BUFFER_SIZE]; - check(service_paths_get_user_data_path(manifest->id.c_str(), childPath.c_str(), buffer, sizeof(buffer)) == ERROR_NONE); + check(service_paths_get_user_data_path(manifest->id, childPath.c_str(), buffer, sizeof(buffer)) == ERROR_NONE); return buffer; } @@ -50,7 +48,7 @@ class ServicePaths { */ std::string getAssetsDirectory() const { char buffer[PATH_BUFFER_SIZE]; - check(service_paths_get_assets_directory(manifest->id.c_str(), buffer, sizeof(buffer)) == ERROR_NONE); + check(service_paths_get_assets_directory(manifest->id, buffer, sizeof(buffer)) == ERROR_NONE); return buffer; } @@ -61,9 +59,9 @@ class ServicePaths { std::string getAssetsPath(const std::string& childPath) const { assert(!childPath.starts_with('/')); char buffer[PATH_BUFFER_SIZE]; - check(service_paths_get_assets_path(manifest->id.c_str(), childPath.c_str(), buffer, sizeof(buffer)) == ERROR_NONE); + check(service_paths_get_assets_path(manifest->id, childPath.c_str(), buffer, sizeof(buffer)) == ERROR_NONE); return buffer; } }; -} \ No newline at end of file +} diff --git a/Tactility/Include/Tactility/service/ServiceRegistration.h b/Tactility/Include/Tactility/service/ServiceRegistration.h index a23a882ac..fe85c87b9 100644 --- a/Tactility/Include/Tactility/service/ServiceRegistration.h +++ b/Tactility/Include/Tactility/service/ServiceRegistration.h @@ -40,7 +40,7 @@ State getState(const std::string& id); * @param[in] id the id as defined in the manifest * @return the matching manifest or nullptr when it wasn't found */ -std::shared_ptr findManifestById(const std::string& id); +const ::ServiceManifest* findManifestById(const std::string& id); /** Find a ServiceContext by its manifest id. * @param[in] id the id as defined in the manifest diff --git a/Tactility/Source/service/ServiceContext.cpp b/Tactility/Source/service/ServiceContext.cpp index 25eba862b..b500f3c37 100644 --- a/Tactility/Source/service/ServiceContext.cpp +++ b/Tactility/Source/service/ServiceContext.cpp @@ -1,23 +1,17 @@ #include -#include #include -#include +#include namespace tt::service { -std::shared_ptr& ServiceContext::getCppManifest() const { - const auto* cManifest = service_context_get_manifest(cContext); - return *static_cast*>(cManifest->context); -} - -const ServiceManifest& ServiceContext::getManifest() const { - return *getCppManifest(); +const ::ServiceManifest* ServiceContext::getManifest() const { + return service_instance_get_manifest(service_instance); } std::unique_ptr ServiceContext::getPaths() const { - return std::make_unique(getCppManifest()); + return std::make_unique(getManifest()); } } // namespace diff --git a/Tactility/Source/service/ServiceRegistration.cpp b/Tactility/Source/service/ServiceRegistration.cpp index b0e33dcd4..02cc3e915 100644 --- a/Tactility/Source/service/ServiceRegistration.cpp +++ b/Tactility/Source/service/ServiceRegistration.cpp @@ -8,37 +8,31 @@ #include #include +#include namespace tt::service { constexpr auto* TAG = "ServiceRegistration"; -// Bridges the kernel's context-free C ServiceManifest/Service callbacks to the -// C++ Service instances they wrap. Declared extern "C" to match the linkage of -// the C function-pointer types they're assigned to (see e.g. gpio_controller.cpp). +// Bridges the kernel's C ServiceManifest/Service callbacks to the C++ Service +// instances they wrap. Declared extern "C" to match the linkage of the C +// function-pointer types they're assigned to (see e.g. gpio_controller.cpp). extern "C" { -static error_t cppOnStartTrampoline(::ServiceInstance* cContext) { - auto& servicePtr = *static_cast*>(cContext->data); - ServiceContext context(cContext); - return servicePtr->onStart(context) ? ERROR_NONE : ERROR_RESOURCE; -} - -static void cppOnStopTrampoline(::ServiceInstance* cContext) { - auto& servicePtr = *static_cast*>(cContext->data); - ServiceContext context(cContext); - servicePtr->onStop(context); +static void cppDestroyServiceTrampoline(const ::ServiceManifest* /*manifest*/, void* data) { + delete static_cast*>(data); } -static void cppCreateServiceTrampoline(::ServiceInstance* cContext, void* context) { - auto& cppManifest = *static_cast*>(context); - cContext->data = new std::shared_ptr(cppManifest->createService()); - cContext->on_start = cppOnStartTrampoline; - cContext->on_stop = cppOnStopTrampoline; +static error_t cppOnStartTrampoline(::ServiceInstance* instance, void* data) { + auto& servicePtr = *static_cast*>(data); + ServiceContext context(instance); + return servicePtr->onStart(context) ? ERROR_NONE : ERROR_RESOURCE; } -static void cppDestroyServiceTrampoline(::ServiceInstance* cContext, void* /*context*/) { - delete static_cast*>(cContext->data); +static void cppOnStopTrampoline(::ServiceInstance* instance, void* data) { + auto& servicePtr = *static_cast*>(data); + ServiceContext context(instance); + servicePtr->onStop(context); } } // extern "C" @@ -54,16 +48,16 @@ void addService(std::shared_ptr manifest, bool autoStart) return; } - // The bridging C manifest and the C++ manifest it carries in .context are - // intentionally never freed: services are registered once and live for the - // process lifetime (there is no removeService()), matching the previous - // implementation's static, never-erased manifest map. - auto* cppManifestPtr = new std::shared_ptr(manifest); + // 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. + auto* persistentManifest = new std::shared_ptr(manifest); auto* cManifest = new ::ServiceManifest { - .id = (*cppManifestPtr)->id.c_str(), - .create_service = cppCreateServiceTrampoline, + .id = (*persistentManifest)->id.c_str(), + .create_service = (*persistentManifest)->createService, .destroy_service = cppDestroyServiceTrampoline, - .context = cppManifestPtr + .on_start = cppOnStartTrampoline, + .on_stop = cppOnStopTrampoline, }; error_t error = service_manager_add(cManifest, autoStart); @@ -76,17 +70,13 @@ void addService(const ServiceManifest& manifest, bool autoStart) { addService(std::make_shared(manifest), autoStart); } -std::shared_ptr findManifestById(const std::string& id) { - const auto* cManifest = service_manager_find_manifest(id.c_str()); - if (cManifest == nullptr) { - return nullptr; - } - return *static_cast*>(cManifest->context); +const ::ServiceManifest* findManifestById(const std::string& id) { + return service_manager_find_manifest(id.c_str()); } bool startService(const std::string& id) { LOG_I(TAG, "Starting %s", id.c_str()); - error_t error = service_manager_start(id.c_str()); + const error_t error = service_manager_start(id.c_str()); if (error != ERROR_NONE) { LOG_E(TAG, "Starting %s failed: %s", id.c_str(), error_to_string(error)); return false; @@ -96,19 +86,19 @@ bool startService(const std::string& id) { } std::shared_ptr findServiceContextById(const std::string& id) { - auto* cContext = service_manager_find_context(id.c_str()); - if (cContext == nullptr) { + auto* instance = service_manager_find_instance(id.c_str()); + if (instance == nullptr) { return nullptr; } - return std::make_shared(cContext); + return std::make_shared(instance); } std::shared_ptr findServiceById(const std::string& id) { - auto* cContext = service_manager_find_context(id.c_str()); - if (cContext == nullptr) { + auto* instance = service_manager_find_instance(id.c_str()); + if (instance == nullptr) { return nullptr; } - return *static_cast*>(cContext->data); + return *static_cast*>(instance->data); } bool stopService(const std::string& id) { diff --git a/TactilityKernel/include/tactility/service/service_context.h b/TactilityKernel/include/tactility/service/service_context.h deleted file mode 100644 index 681886392..000000000 --- a/TactilityKernel/include/tactility/service/service_context.h +++ /dev/null @@ -1,28 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 - -#pragma once - -#include - -#ifdef __cplusplus -extern "C" { -#endif - -/** - * The context handed to a service's on_start/on_stop callbacks. - * This is the same object as the ServiceInstance that owns the service (C has no - * interface/inheritance to distinguish a restricted "context" view from the full - * instance, so the two are deliberately the same type under different names). - */ -typedef struct ServiceInstance ServiceContext; - -/** - * @brief Get the manifest of the running service. - * @param[in] context non-null service context pointer - * @return the manifest - */ -const struct ServiceManifest* service_context_get_manifest(ServiceContext* context); - -#ifdef __cplusplus -} -#endif diff --git a/TactilityKernel/include/tactility/service/service_instance.h b/TactilityKernel/include/tactility/service/service_instance.h index 87dc1d681..9a1adb678 100644 --- a/TactilityKernel/include/tactility/service/service_instance.h +++ b/TactilityKernel/include/tactility/service/service_instance.h @@ -25,19 +25,6 @@ struct ServiceInstance { const struct ServiceManifest* manifest; /** Service-specific data, owned by the service implementation. Can be NULL. */ void* data; - /** - * Called when the service is starting. - * Can be NULL, in which case starting always succeeds. - * @param[in,out] instance this service instance (also its own ServiceContext) - * @return ERROR_NONE if the service started successfully - */ - error_t (*on_start)(struct ServiceInstance* instance); - /** - * Called when the service is stopping. - * Can be NULL, in which case stopping is a no-op. - * @param[in,out] instance this service instance (also its own ServiceContext) - */ - void (*on_stop)(struct ServiceInstance* instance); /** * Internal state managed by the kernel. * ServiceInstance implementers should initialize this to NULL. @@ -48,7 +35,7 @@ struct ServiceInstance { /** * @brief Construct a service instance. - * @details This calls manifest->create_service() to build the service. + * @details This calls manifest->create_service() to build the instance's data. * @param[in,out] instance a service instance with manifest set and internal set to NULL * @param[in] manifest non-null manifest to construct the instance from * @retval ERROR_OUT_OF_MEMORY if internal data allocation failed @@ -58,7 +45,7 @@ error_t service_instance_construct(struct ServiceInstance* instance, const struc /** * @brief Destruct a service instance. - * @details This calls manifest->destroy_service() on the service. + * @details This calls manifest->destroy_service() on the instance's data. * @param[in,out] instance non-null service instance pointer * @retval ERROR_INVALID_STATE if the instance is not stopped * @retval ERROR_NONE on success diff --git a/TactilityKernel/include/tactility/service/service_manager.h b/TactilityKernel/include/tactility/service/service_manager.h index d4cfc74b7..299dcfbf4 100644 --- a/TactilityKernel/include/tactility/service/service_manager.h +++ b/TactilityKernel/include/tactility/service/service_manager.h @@ -4,7 +4,6 @@ #include #include -#include #include #include @@ -64,11 +63,11 @@ error_t service_manager_stop(const char* id); ServiceState service_manager_get_state(const char* id); /** - * @brief Find the context of a running service by id. + * @brief Find the service instance * @param[in] id non-null service id - * @return the context, or NULL if the service isn't running + * @return the instance when found, otherwise return NULL */ -ServiceContext* service_manager_find_context(const char* id); +struct ServiceInstance* service_manager_find_instance(const char* id); #ifdef __cplusplus } diff --git a/TactilityKernel/include/tactility/service/service_manifest.h b/TactilityKernel/include/tactility/service/service_manifest.h index 989b8e115..7a0cd67c2 100644 --- a/TactilityKernel/include/tactility/service/service_manifest.h +++ b/TactilityKernel/include/tactility/service/service_manifest.h @@ -9,28 +9,40 @@ extern "C" { #endif // ServiceContext (declared in service_context.h) is a typedef alias of ServiceInstance, -// so ServiceCreate/ServiceDestroy below are declared directly in terms of ServiceInstance -// to avoid a conflicting forward-declaration of an unrelated "ServiceContext" struct tag. +// so the callback types below are declared directly in terms of ServiceInstance to avoid +// a conflicting forward-declaration of an unrelated "ServiceContext" struct tag. struct ServiceInstance; /** - * Initializes a newly-registered service instance in place. - * Implementations should set instance->data, instance->on_start and instance->on_stop - * as needed; all three may be left at their zeroed defaults (NULL) if the service has - * no state or lifecycle callbacks. - * @param[in,out] instance the instance to populate (manifest and internal are already set) - * @param[in] context the manifest's context (ServiceManifest::context) + * Allocates and initializes the service's custom data. + * @param[in] manifest the manifest that owns this callback + * @return the service's custom data, or NULL if it has none */ -typedef void (*ServiceCreate)(struct ServiceInstance* instance, void* context); +typedef void* (*ServiceCreate)(const struct ServiceManifest* manifest); /** - * Tears down state that was set up by the matching ServiceCreate function - * (e.g. frees instance->data). Should not clear instance->data/on_start/on_stop; - * the caller does that. - * @param[in,out] instance the instance to tear down - * @param[in] context the manifest's context (ServiceManifest::context) + * Frees custom data that was set up by the matching ServiceCreate function. + * @param[in] data the custom data returned by create_service + * @param[in] manifest the manifest that owns this callback */ -typedef void (*ServiceDestroy)(struct ServiceInstance* instance, void* context); +typedef void (*ServiceDestroy)(const struct ServiceManifest* manifest, void* data); + +/** + * Called when a service instance is starting. + * Can be NULL, in which case starting always succeeds. + * @param[in,out] instance the starting service instance (also its own ServiceContext) + * @param[in] data the custom data returned by create_service + * @return ERROR_NONE if the service started successfully + */ +typedef error_t (*ServiceOnStart)(struct ServiceInstance* instance, void* data); + +/** + * Called when a service instance is stopping. + * Can be NULL, in which case stopping is a no-op. + * @param[in,out] instance the stopping service instance (also its own ServiceContext) + * @param[in] data the custom data returned by create_service + */ +typedef void (*ServiceOnStop)(struct ServiceInstance* instance, void* data); /** * Describes a registrable service type. @@ -39,12 +51,14 @@ typedef void (*ServiceDestroy)(struct ServiceInstance* instance, void* context); struct ServiceManifest { /** Unique service identifier. Should never be NULL. */ const char* id; - /** Allocates and initializes a new Service instance. Should never be NULL. */ + /** Allocates the service's custom data. Should never be NULL. */ ServiceCreate create_service; - /** Frees a Service instance created by create_service. Should never be NULL. */ + /** Frees the service's custom data. Should never be NULL. */ ServiceDestroy destroy_service; - /** Opaque context passed to create_service and destroy_service. Can be NULL. */ - void* context; + /** Called when a service instance starts. Can be NULL. */ + ServiceOnStart on_start; + /** Called when a service instance stops. Can be NULL. */ + ServiceOnStop on_stop; }; #ifdef __cplusplus diff --git a/TactilityKernel/source/kernel_symbols.c b/TactilityKernel/source/kernel_symbols.c index e06bef4aa..49e688443 100644 --- a/TactilityKernel/source/kernel_symbols.c +++ b/TactilityKernel/source/kernel_symbols.c @@ -21,6 +21,9 @@ #include #include #include +#include +#include +#include #ifndef ESP_PLATFORM #include @@ -247,6 +250,25 @@ const struct ModuleSymbol KERNEL_SYMBOLS[] = { DEFINE_MODULE_SYMBOL(module_is_started), DEFINE_MODULE_SYMBOL(module_resolve_symbol), DEFINE_MODULE_SYMBOL(module_resolve_symbol_global), + // service/service_instance + DEFINE_MODULE_SYMBOL(service_instance_construct), + DEFINE_MODULE_SYMBOL(service_instance_destruct), + DEFINE_MODULE_SYMBOL(service_instance_get_manifest), + DEFINE_MODULE_SYMBOL(service_instance_get_data), + DEFINE_MODULE_SYMBOL(service_instance_get_state), + // service/service_manager + DEFINE_MODULE_SYMBOL(service_manager_add), + DEFINE_MODULE_SYMBOL(service_manager_remove), + DEFINE_MODULE_SYMBOL(service_manager_find_manifest), + DEFINE_MODULE_SYMBOL(service_manager_start), + DEFINE_MODULE_SYMBOL(service_manager_stop), + DEFINE_MODULE_SYMBOL(service_manager_get_state), + DEFINE_MODULE_SYMBOL(service_manager_find_instance), + // service/service_paths + DEFINE_MODULE_SYMBOL(service_paths_get_user_data_directory), + DEFINE_MODULE_SYMBOL(service_paths_get_user_data_path), + DEFINE_MODULE_SYMBOL(service_paths_get_assets_directory), + DEFINE_MODULE_SYMBOL(service_paths_get_assets_path), // terminator MODULE_SYMBOL_TERMINATOR }; diff --git a/TactilityKernel/source/service/service_instance.cpp b/TactilityKernel/source/service/service_instance.cpp index 768c78f32..525d30e19 100644 --- a/TactilityKernel/source/service/service_instance.cpp +++ b/TactilityKernel/source/service/service_instance.cpp @@ -1,7 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 #include -#include #include #include @@ -24,10 +23,7 @@ error_t service_instance_construct(ServiceInstance* instance, const ServiceManif mutex_construct(&instance->internal->mutex); instance->manifest = manifest; - instance->data = nullptr; - instance->on_start = nullptr; - instance->on_stop = nullptr; - manifest->create_service(instance, manifest->context); + instance->data = manifest->create_service(manifest); LOG_D(TAG, "construct %s", manifest->id); return ERROR_NONE; @@ -45,10 +41,8 @@ error_t service_instance_destruct(ServiceInstance* instance) { LOG_D(TAG, "destruct %s", instance->manifest->id); - instance->manifest->destroy_service(instance, instance->manifest->context); + instance->manifest->destroy_service(instance->manifest, instance->data); instance->data = nullptr; - instance->on_start = nullptr; - instance->on_stop = nullptr; instance->internal = nullptr; mutex_destruct(&internal->mutex); @@ -79,8 +73,4 @@ void service_instance_set_state(ServiceInstance* instance, ServiceState state) { mutex_unlock(&instance->internal->mutex); } -const ServiceManifest* service_context_get_manifest(ServiceContext* context) { - return service_instance_get_manifest(context); -} - } // extern "C" diff --git a/TactilityKernel/source/service/service_manager.cpp b/TactilityKernel/source/service/service_manager.cpp index 35d921314..e0bb58186 100644 --- a/TactilityKernel/source/service/service_manager.cpp +++ b/TactilityKernel/source/service/service_manager.cpp @@ -65,7 +65,7 @@ error_t service_manager_add(const ServiceManifest* manifest, bool auto_start) { } error_t service_manager_remove(const char* id) { - if (service_manager_find_context(id) != nullptr) { + if (service_manager_get_state(id) != SERVICE_STATE_STOPPED) { return ERROR_INVALID_STATE; } @@ -98,7 +98,7 @@ error_t service_manager_start(const char* id) { return ERROR_INVALID_STATE; } - auto* instance = new(std::nothrow) ServiceInstance { .manifest = nullptr, .data = nullptr, .on_start = nullptr, .on_stop = nullptr, .internal = nullptr }; + auto* instance = new(std::nothrow) ServiceInstance { .manifest = nullptr, .data = nullptr, .internal = nullptr }; if (instance == nullptr) { mutex_unlock(&instance_ledger.mutex); return ERROR_OUT_OF_MEMORY; @@ -118,7 +118,7 @@ error_t service_manager_start(const char* id) { service_instance_set_state(instance, SERVICE_STATE_STARTING); LOG_I(TAG, "start %s", id); - error = (instance->on_start != nullptr) ? instance->on_start(instance) : ERROR_NONE; + error = (manifest->on_start != nullptr) ? manifest->on_start(instance, instance->data) : ERROR_NONE; if (error == ERROR_NONE) { service_instance_set_state(instance, SERVICE_STATE_STARTED); @@ -152,8 +152,8 @@ error_t service_manager_stop(const char* id) { service_instance_set_state(instance, SERVICE_STATE_STOPPING); - if (instance->on_stop != nullptr) { - instance->on_stop(instance); + if (instance->manifest->on_stop != nullptr) { + instance->manifest->on_stop(instance, instance->data); } service_instance_set_state(instance, SERVICE_STATE_STOPPED); @@ -189,10 +189,10 @@ const ServiceManifest* service_manager_find_manifest(const char* id) { return manifest; } -ServiceContext* service_manager_find_context(const char* id) { +ServiceInstance* service_manager_find_instance(const char* id) { mutex_lock(&instance_ledger.mutex); const auto iterator = instance_ledger.instances.find(id); - ServiceInstance* instance = (iterator != instance_ledger.instances.end()) ? iterator->second : nullptr; + auto* instance = (iterator != instance_ledger.instances.end()) ? iterator->second : nullptr; mutex_unlock(&instance_ledger.mutex); return instance; } diff --git a/Tests/TactilityKernel/Source/ServiceTest.cpp b/Tests/TactilityKernel/Source/ServiceTest.cpp index 5917e8cd2..ec8797dc2 100644 --- a/Tests/TactilityKernel/Source/ServiceTest.cpp +++ b/Tests/TactilityKernel/Source/ServiceTest.cpp @@ -9,25 +9,27 @@ static int destroy_called = 0; static int on_start_called = 0; static int on_stop_called = 0; static error_t on_start_result = ERROR_NONE; -static void* last_create_context = nullptr; -static void* last_destroy_context = nullptr; +static const ServiceManifest* last_create_manifest = nullptr; +static const ServiceManifest* last_destroy_manifest = nullptr; -static void test_create_service(ServiceInstance* instance, void* context) { +static void* test_create_service(const ServiceManifest* manifest) { create_called++; - last_create_context = context; - instance->data = nullptr; - instance->on_start = [](ServiceInstance*) -> error_t { - on_start_called++; - return on_start_result; - }; - instance->on_stop = [](ServiceInstance*) { - on_stop_called++; - }; + last_create_manifest = manifest; + return nullptr; } -static void test_destroy_service(ServiceInstance*, void* context) { +static void test_destroy_service(const ServiceManifest* manifest, void*) { destroy_called++; - last_destroy_context = context; + last_destroy_manifest = manifest; +} + +static error_t test_on_start(ServiceInstance*, void*) { + on_start_called++; + return on_start_result; +} + +static void test_on_stop(ServiceInstance*, void*) { + on_stop_called++; } static void reset_counters() { @@ -36,36 +38,34 @@ static void reset_counters() { on_start_called = 0; on_stop_called = 0; on_start_result = ERROR_NONE; - last_create_context = nullptr; - last_destroy_context = nullptr; + last_create_manifest = nullptr; + last_destroy_manifest = nullptr; } TEST_CASE("ServiceInstance construction and destruction") { reset_counters(); - static int context_marker = 0; static const ServiceManifest manifest = { .id = "instance-test", .create_service = test_create_service, .destroy_service = test_destroy_service, - .context = &context_marker + .on_start = test_on_start, + .on_stop = test_on_stop }; - ServiceInstance instance = { .manifest = nullptr, .data = nullptr, .on_start = nullptr, .on_stop = nullptr, .internal = nullptr }; + ServiceInstance instance = { .manifest = nullptr, .data = nullptr, .internal = nullptr }; CHECK_EQ(service_instance_construct(&instance, &manifest), ERROR_NONE); CHECK_NE(instance.internal, nullptr); CHECK_EQ(instance.manifest, &manifest); - CHECK_NE(instance.on_start, nullptr); - CHECK_NE(instance.on_stop, nullptr); CHECK_EQ(create_called, 1); - CHECK_EQ(last_create_context, &context_marker); + CHECK_EQ(last_create_manifest, &manifest); CHECK_EQ(service_instance_get_state(&instance), SERVICE_STATE_STOPPED); CHECK_EQ(service_instance_destruct(&instance), ERROR_NONE); CHECK_EQ(instance.internal, nullptr); CHECK_EQ(destroy_called, 1); - CHECK_EQ(last_destroy_context, &context_marker); + CHECK_EQ(last_destroy_manifest, &manifest); } TEST_CASE("service_manager_add rejects duplicate ids") { @@ -89,7 +89,9 @@ TEST_CASE("service_registration start/stop lifecycle") { static const ServiceManifest manifest = { .id = "lifecycle-test", .create_service = test_create_service, - .destroy_service = test_destroy_service + .destroy_service = test_destroy_service, + .on_start = test_on_start, + .on_stop = test_on_stop }; CHECK_EQ(service_manager_add(&manifest, false), ERROR_NONE); @@ -98,7 +100,7 @@ TEST_CASE("service_registration start/stop lifecycle") { CHECK_EQ(service_manager_start("lifecycle-test"), ERROR_NONE); CHECK_EQ(on_start_called, 1); CHECK_EQ(service_manager_get_state("lifecycle-test"), SERVICE_STATE_STARTED); - CHECK_NE(service_manager_find_context("lifecycle-test"), nullptr); + CHECK_NE(service_manager_find_instance("lifecycle-test"), nullptr); // Starting again while already started should fail CHECK_EQ(service_manager_start("lifecycle-test"), ERROR_INVALID_STATE); @@ -109,7 +111,7 @@ TEST_CASE("service_registration start/stop lifecycle") { CHECK_EQ(service_manager_stop("lifecycle-test"), ERROR_NONE); CHECK_EQ(on_stop_called, 1); CHECK_EQ(service_manager_get_state("lifecycle-test"), SERVICE_STATE_STOPPED); - CHECK_EQ(service_manager_find_context("lifecycle-test"), nullptr); + CHECK_EQ(service_manager_find_instance("lifecycle-test"), nullptr); // Stopping again while already stopped should fail CHECK_EQ(service_manager_stop("lifecycle-test"), ERROR_NOT_FOUND); @@ -123,7 +125,9 @@ TEST_CASE("service_manager_add with auto_start") { static const ServiceManifest manifest = { .id = "auto-start-test", .create_service = test_create_service, - .destroy_service = test_destroy_service + .destroy_service = test_destroy_service, + .on_start = test_on_start, + .on_stop = test_on_stop }; CHECK_EQ(service_manager_add(&manifest, true), ERROR_NONE); @@ -141,13 +145,15 @@ TEST_CASE("service_manager_start failure leaves service stopped") { static const ServiceManifest manifest = { .id = "failing-start-test", .create_service = test_create_service, - .destroy_service = test_destroy_service + .destroy_service = test_destroy_service, + .on_start = test_on_start, + .on_stop = test_on_stop }; CHECK_EQ(service_manager_add(&manifest, false), ERROR_NONE); CHECK_EQ(service_manager_start("failing-start-test"), ERROR_RESOURCE); CHECK_EQ(service_manager_get_state("failing-start-test"), SERVICE_STATE_STOPPED); - CHECK_EQ(service_manager_find_context("failing-start-test"), nullptr); + CHECK_EQ(service_manager_find_instance("failing-start-test"), nullptr); CHECK_EQ(service_manager_remove("failing-start-test"), ERROR_NONE); } @@ -155,7 +161,7 @@ TEST_CASE("service_manager_start failure leaves service stopped") { TEST_CASE("service_registration lookup functions with unknown id") { CHECK_EQ(service_manager_get_state("unknown-service-id"), SERVICE_STATE_STOPPED); CHECK_EQ(service_manager_find_manifest("unknown-service-id"), nullptr); - CHECK_EQ(service_manager_find_context("unknown-service-id"), nullptr); + CHECK_EQ(service_manager_find_instance("unknown-service-id"), nullptr); CHECK_EQ(service_manager_start("unknown-service-id"), ERROR_NOT_FOUND); CHECK_EQ(service_manager_stop("unknown-service-id"), ERROR_NOT_FOUND); CHECK_EQ(service_manager_remove("unknown-service-id"), ERROR_NOT_FOUND);