From 469cf0d2756ec533fc09dfd5169b9b0444b12631 Mon Sep 17 00:00:00 2001 From: Ken Van Hoeylandt Date: Sun, 5 Jul 2026 22:34:15 +0200 Subject: [PATCH] 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 | 30 +++-- .../source/service/service_instance.cpp | 34 ++++- ...e_registration.cpp => service_manager.cpp} | 35 +++--- Tests/TactilityKernel/Source/ServiceTest.cpp | 119 +++++++++++------- 8 files changed, 196 insertions(+), 166 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 -/** - * 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); +// 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; /** - * Frees a Service instance that was created by the matching ServiceCreate function. - * @param[in] service the service to free + * 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) */ -typedef void (*ServiceDestroy)(struct Service* service, void* context); +typedef void (*ServiceCreate)(struct ServiceInstance* instance, void* context); + +/** + * 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 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); }