From a9535c4def5d419858a0f6b0599a8dc6346e5fca Mon Sep 17 00:00:00 2001 From: SimonKozik <244535158+SimonKozik@users.noreply.github.com> Date: Tue, 28 Jul 2026 15:33:23 +0100 Subject: [PATCH 1/6] First version of new API for Launch Manager --- .../src/lm_control/src/fixed_string.hpp | 304 ++++++++++++++++++ .../src/lm_control/src/ilm_control.hpp | 163 ++++++++++ .../src/run_target_activation_source.hpp | 40 +++ 3 files changed, 507 insertions(+) create mode 100644 score/launch_manager/src/lm_control/src/fixed_string.hpp create mode 100644 score/launch_manager/src/lm_control/src/ilm_control.hpp create mode 100644 score/launch_manager/src/lm_control/src/run_target_activation_source.hpp diff --git a/score/launch_manager/src/lm_control/src/fixed_string.hpp b/score/launch_manager/src/lm_control/src/fixed_string.hpp new file mode 100644 index 000000000..a28853a8d --- /dev/null +++ b/score/launch_manager/src/lm_control/src/fixed_string.hpp @@ -0,0 +1,304 @@ +/******************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Apache License Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + ********************************************************************************/ + +#ifndef SCORE_MW_LIFECYCLE_FIXED_STRING_HPP +#define SCORE_MW_LIFECYCLE_FIXED_STRING_HPP + +#include +#include +#include +#include + +namespace score::mw::lifecycle +{ + +/// @brief Allocation-free, fixed-capacity string with a null-termination invariant. +/// +/// Stores a null-terminated string in a plain `std::array` of size `MaxLength + 1`. +/// Designed for use across API boundaries where heap allocation is undesirable and +/// a bounded string length is acceptable. +/// +/// @tparam MaxLength Maximum number of usable characters, excluding the null terminator. +/// The internal storage is `MaxLength + 1` bytes so the terminator +/// always fits. +/// +/// @par Invariant +/// The stored string is always null-terminated. Every constructor and assignment path +/// enforces this. Characters beyond the null terminator are indeterminate and must +/// not be read directly — use `as_string_view()` or `data()` instead. +/// +/// @par Copying and moving +/// Copy and move are compiler-generated (rule of zero). The `std::array` storage +/// carries no heap resource, so the generated forms are correct and efficient. +/// +/// @par Ordering +/// No `operator<` is provided. `FixedString` values have no meaningful ordering. +/// +/// @par Cross-capacity comparison +/// `FixedString` and `FixedString` are distinct types, but free-function +/// `operator==` / `operator!=` overloads allow comparing them by string content. +template +struct FixedString +{ + /// @brief Construct an empty string (equivalent to ""). + /// + /// Only `storage_[0]` is set to `'\0'`; the rest of the array is indeterminate. + /// This is intentional — zeroing `MaxLength + 1` bytes on every default + /// construction would be wasteful when the value is immediately overwritten. + FixedString() noexcept + { + storage_[0] = '\0'; + } + + /// @brief Construct from a string_view, truncating if necessary. + /// + /// If `s.size() > MaxLength`, only the first `MaxLength` characters are stored. + /// No error is raised at this level — detection and logging of over-length + /// strings must happen at the user side where the source and context are + /// available. + /// + /// @param s Source string. Does not require to be null-terminated. + explicit FixedString(std::string_view s) noexcept + { + const auto len = std::min(s.size(), MaxLength); + std::memcpy(storage_.data(), s.data(), len); + storage_[len] = '\0'; + } + + /// @brief Assign from a string_view, truncating if necessary. + /// + /// Behaves identically to the string_view constructor: copies up to + /// `MaxLength` characters and always null-terminates. If `s.size() > + /// MaxLength`, only the first `MaxLength` characters are stored - no + /// error is raised at this level. + /// + /// Allows direct assignment from string literals and std::string_view + /// without constructing a temporary FixedString: + /// @code + /// FixedString name; + /// name = "Running"; + /// @endcode + /// + /// @param s Source string. Does not require to be null-terminated. + /// + /// @returns Reference to this instance. + FixedString& operator=(std::string_view s) noexcept + { + const auto len = std::min(s.size(), MaxLength); + std::memcpy(storage_.data(), s.data(), len); + storage_[len] = '\0'; + return *this; + } + + /// @brief Returns the fixed character capacity of the FixedString. + /// + /// The returned value is equal to the template parameter `MaxLength` and + /// represents the maximum number of characters that can be stored, excluding + /// the terminating null character. + /// + /// @return Maximum number of usable characters. + static constexpr std::size_t max_size() noexcept + { + return MaxLength; + } + + /// @brief Returns the current length of the FixedString. + /// + /// The returned value represents the number of characters currently stored, + /// excluding the terminating null character. Returns 0 for a + /// default-constructed or cleared instance. + /// + /// Equivalent to `as_string_view().size()`. + /// + /// @return Number of stored characters. + std::size_t size() const noexcept + { + return as_string_view().size(); + } + + /// @brief Checks whether the FixedString is empty. + /// + /// A FixedString is considered empty when it contains no characters. + /// Returns true for a default-constructed or cleared instance. + /// + /// Equivalent to `size() == 0`. + /// + /// @return true if the FixedString is empty; otherwise, false. + bool empty() const noexcept + { + return storage_[0] == '\0'; + } + + /// @brief Clears the contents of the FixedString. + /// + /// After this call, the FixedString is empty, `size()` returns 0, + /// and `as_string_view()` returns an empty string. + /// + /// Only the terminating null character at the beginning of the internal + /// storage is written; the remaining storage is left unchanged. + void clear() noexcept + { + storage_[0] = '\0'; + } + + /// @brief Returns the contents of the FixedString as a std::string_view. + /// + /// The returned view refers directly to the internal storage of this + /// FixedString and does not own the underlying character data. + /// + /// The view remains valid only while this FixedString instance exists and + /// its contents are not modified. + /// + /// @return A std::string_view representing the stored character sequence. + std::string_view as_string_view() const noexcept + { + return std::string_view{storage_.data()}; + } + + /// @brief Returns a pointer to the stored null-terminated character sequence. + /// + /// The returned pointer refers directly to the internal storage of this + /// FixedString and is suitable for use with APIs expecting a C-style string. + /// + /// The pointer remains valid only while this FixedString instance exists and + /// its contents are not modified. + /// + /// @return Pointer to the stored null-terminated character sequence. + const char* data() const noexcept + { + return storage_.data(); + } + + /// @brief Equality comparison with another FixedString of the same capacity. + /// + /// Equality is determined by comparing the string representations of + /// both instances. + /// + /// @param other The FixedString instance to compare against. + /// + /// @return true if both FixedString objects represent the same string; + /// otherwise, false. + bool operator==(const FixedString& other) const noexcept + { + return as_string_view() == other.as_string_view(); + } + + /// @brief Equality comparison with a string_view. + /// + /// Allows comparisons such as `fs == "Running"` without constructing + /// a temporary FixedString. + /// + /// @param other The string value to compare against. + /// + /// @return true if this FixedString represents the same string as + /// @p other; otherwise, false. + bool operator==(std::string_view other) const noexcept + { + return as_string_view() == other; + } + + /// @brief Inequality comparison with another FixedString of the same capacity. + /// + /// Inequality is determined by comparing the string representations of + /// both instances. + /// + /// @param other The FixedString instance to compare against. + /// + /// @return true if the FixedString objects represent different strings; + /// otherwise, false. + bool operator!=(const FixedString& other) const noexcept + { + return !(*this == other); + } + + /// @brief Inequality comparison with a string_view. + /// + /// Allows comparisons such as `fs != "Running"` without constructing + /// a temporary FixedString. + /// + /// @param other The string value to compare against. + /// + /// @return true if this FixedString represents a different string + /// than @p other; otherwise, false. + bool operator!=(std::string_view other) const noexcept + { + return !(*this == other); + } + + private: + std::array storage_; +}; + +/// @brief Equality comparison between FixedString instances of different capacities. +/// +/// Equality is determined by comparing the string representations of both +/// instances, regardless of their storage capacities. +/// +/// For example, a `FixedString<128>` and a `FixedString<64>` containing the +/// same character sequence compare equal. +/// +/// @tparam N Capacity of the left-hand FixedString. +/// @tparam M Capacity of the right-hand FixedString. +/// @param lhs The left-hand FixedString instance. +/// @param rhs The right-hand FixedString instance. +/// @return true if both FixedString objects represent the same string; +/// otherwise, false. +template +bool operator==(const FixedString& lhs, const FixedString& rhs) noexcept +{ + return lhs.as_string_view() == rhs.as_string_view(); +} + +/// @brief Inequality comparison between FixedString instances of different capacities. +/// +/// Inequality is determined by comparing the string representations of both +/// instances, regardless of their storage capacities. +/// +/// For example, a `FixedString<128>` and a `FixedString<64>` containing +/// different character sequences compare not equal. +/// +/// @tparam N Capacity of the left-hand FixedString. +/// @tparam M Capacity of the right-hand FixedString. +/// @param lhs The left-hand FixedString instance. +/// @param rhs The right-hand FixedString instance. +/// @return true if the FixedString objects represent different strings; +/// otherwise, false. +template +bool operator!=(const FixedString& lhs, const FixedString& rhs) noexcept +{ + return !(lhs == rhs); +} + +/// @brief Stream insertion operator for FixedString. +/// +/// Writes the string representation of the FixedString to the specified +/// output stream. +/// +/// The operator is templated on the stream type so it can be used with any +/// ostream-compatible stream (for example, `std::ostream` or mw::log streams) +/// without introducing additional dependencies in this header. +/// +/// @tparam Stream Type of the output stream. +/// @tparam MaxLength Maximum capacity of the FixedString. +/// @param os The output stream. +/// @param fs The FixedString instance to write. +/// @return A reference to the output stream. +template +Stream& operator<<(Stream& os, const FixedString& fs) +{ + return os << fs.data(); +} + +} // namespace score::mw::lifecycle + +#endif // SCORE_MW_LIFECYCLE_FIXED_STRING_HPP diff --git a/score/launch_manager/src/lm_control/src/ilm_control.hpp b/score/launch_manager/src/lm_control/src/ilm_control.hpp new file mode 100644 index 000000000..4df179093 --- /dev/null +++ b/score/launch_manager/src/lm_control/src/ilm_control.hpp @@ -0,0 +1,163 @@ +/******************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Apache License Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + ********************************************************************************/ +#ifndef SCORE_MW_LIFECYCLE_ILM_CONTROL_H_ +#define SCORE_MW_LIFECYCLE_ILM_CONTROL_H_ + +#include +#include +#include + +#include "score/mw/lifecycle/fixed_string.hpp" +#include "score/mw/lifecycle/run_target_activation_source.hpp" +#include "score/result/result.h" + +namespace score::mw::lifecycle +{ + +/// @brief Maximum number of usable characters in a RunTargetName, excluding the null terminator. +/// +/// Names longer than this are silently truncated. The internal storage is +/// `kMaxRunTargetNameLength + 1` bytes to always accommodate the terminator. +inline constexpr std::size_t kMaxRunTargetNameLength = 128U; + +/// @brief Alias for the Run Target name type used throughout the lifecycle API. +using RunTargetName = FixedString; + +/// @brief Public interface for controlling the Launch Manager from a State Manager. +/// +/// Callers obtain an instance via ILmControl::Create() and interact with the +/// Launch Manager exclusively through this interface. The concrete implementation +/// is an internal detail — it is not visible in this header. +/// +/// @par Usage +/// ```cpp +/// auto result = ILmControl::Create(); +/// if (!result.has_value()) { /* handle error */ } +/// std::unique_ptr lm = std::move(result).value(); +/// ``` +/// +/// @par Ownership +/// Instances are owned through `std::unique_ptr`. The interface is +/// non-copyable and non-movable to prevent slicing; transfer ownership via the +/// unique_ptr instead. +class ILmControl +{ + public: + /// @brief Callback fired when a Run Target activation settles. + /// + /// Activation cannot fail: it always resolves into some Run Target, + /// though not necessarily the one that was requested (e.g. a + /// recovery action may activate a different Run Target). + /// The subscriber sees every settling. + /// + /// @param[in] activationSource What caused the activation to occur. + /// @param[in] activatedRunTarget The Run Target that was activated — may + /// differ from the one originally requested. + using ActivationCallback = + std::function; + + /// @brief Factory method — create a connected ILmControl instance. + /// + /// Establishes the mw::com connection to the Launch Manager. + /// + /// @pre mw::com must be fully initialized by the caller before invoking + /// this function. Create() does not initialize mw::com internally. + /// + /// @param[in] instance_specifier The mw::com instance specifier identifying + /// the Launch Manager service. Must be a + /// non-empty, valid specifier string. An empty + /// or malformed value is treated as an error. + /// + /// @returns A unique_ptr to the ILmControl instance on success. + /// + /// @error kInvalidArguments instance_specifier is empty or malformed. + /// @error kNoConnection The Launch Manager service could not be reached. + static score::Result> Create(std::string_view instance_specifier); + + /// @brief Virtual destructor for safe deletion through this interface. + virtual ~ILmControl() noexcept = default; + + // Non-copyable and non-movable. Polymorphic types must not be copied or + // moved through the interface — it would slice the concrete implementation. + // Transfer ownership via std::unique_ptr instead. + ILmControl(const ILmControl&) = delete; + ILmControl& operator=(const ILmControl&) = delete; + ILmControl(ILmControl&&) = delete; + ILmControl& operator=(ILmControl&&) = delete; + + /// @brief Request Run Target activation. + /// + /// Posts the request into the Launch Manager's fixed-capacity FIFO queue + /// and returns as soon as the request is accepted. The Launch Manager + /// executes activations one at a time in FIFO order. Completion is + /// notified asynchronously via the callback registered with + /// register_run_target_activation_callback(). + /// If the queue is full, kRequestQueueIsFull is returned immediately + /// and the request is discarded. + /// + /// @param[in] runTargetName Name of a Run Target configured in the Launch Manager. + /// @param[in] force If false (default), the request is queued behind any + /// in-progress activation and executed afterwards. + /// If true, any in-progress activation is cancelled, the + /// queue is cleared, and this activation starts immediately. + /// + /// @returns void when the Launch Manager accepted the request. + /// + /// @error kRequestQueueIsFull Activation was rejected because Launch Manager cannot accept another activation + /// request. + /// @error kRunTargetDoesntExist Name of the requested Run Target does not exist in current configuration. + /// @error kNoConnection Connection with Launch Manager cannot be established and request cannot be sent. + virtual score::Result activate_run_target(std::string_view runTargetName, bool force = false) = 0; + + /// @brief Register a callback invoked whenever Launch Manager finishes a Run Target activation. + /// + /// Only a single subscriber is supported. The expected usage is + /// register-once during setup, leaving the callback in place until + /// the ILmControl instance is destroyed. Re-registration is + /// supported defensively (a second call overrides the previous + /// setting) but callers that re-register must be prepared to + /// handle kCallbackInProgress (transient, retry). There is no + /// un-register path; empty callbacks are rejected. + /// + /// @param[in] callback The callback to invoke when Run Target activation finishes. + /// + /// @returns void on success. + /// + /// @error kCallbackInProgress A new callback cannot be registered because the + /// current callback is still executing. Retry after it returns. + /// @error kInvalidArguments The callback is empty. + virtual score::Result register_run_target_activation_callback(ActivationCallback callback) = 0; + + /// @brief Query the Launch Manager for the currently active Run Target. + /// + /// Sends a synchronous request to the Launch Manager and returns the name of + /// the Run Target the Launch Manager has settled on. If an activation is in + /// progress, the Launch Manager has not settled on any single Run Target yet + /// and the call returns kActivationInProgress instead. In that case, the caller + /// should wait for the activation completion callback and retry if needed. + /// + /// @returns the name of the currently active Run Target. + /// + /// @error kActivationInProgress Launch Manager is currently executing a Run Target + /// activation and thus there is no single Run Target active. + /// @error kNoConnection Connection with Launch Manager cannot be established + /// and information about active Run Target cannot be retrieved. + virtual score::Result get_active_run_target() = 0; + + protected: + ILmControl() = default; +}; + +} // namespace score::mw::lifecycle + +#endif // SCORE_MW_LIFECYCLE_ILM_CONTROL_H_ diff --git a/score/launch_manager/src/lm_control/src/run_target_activation_source.hpp b/score/launch_manager/src/lm_control/src/run_target_activation_source.hpp new file mode 100644 index 000000000..7204c930e --- /dev/null +++ b/score/launch_manager/src/lm_control/src/run_target_activation_source.hpp @@ -0,0 +1,40 @@ +/******************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Apache License Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + ********************************************************************************/ + +#ifndef SCORE_MW_LIFECYCLE_RUN_TARGET_ACTIVATION_SOURCE_HPP +#define SCORE_MW_LIFECYCLE_RUN_TARGET_ACTIVATION_SOURCE_HPP + +namespace score::mw::lifecycle +{ + +/// @brief Describes what caused a Run Target activation to occur. +/// +/// Passed to the activation completion callback registered via +/// `ILmControl::register_run_target_activation_callback()`. +/// +/// @note Activation cannot fail and always resolves into some +/// Run Target. This enum describes *why* it resolved, +/// not whether it succeeded. +enum class RunTargetActivationSource +{ + /// @brief Activation was explicitly requested by a State Manager. + kStateManagerRequest = 0, + + /// @brief Activation happened automatically as part of a recovery action, + /// without an explicit State Manager request. + kRecoveryAction = 1 +}; + +} // namespace score::mw::lifecycle + +#endif // SCORE_MW_LIFECYCLE_RUN_TARGET_ACTIVATION_SOURCE_HPP From 8a3f84d7f434852c73819bdc1b0fb5ef509141b6 Mon Sep 17 00:00:00 2001 From: SimonKozik <244535158+SimonKozik@users.noreply.github.com> Date: Thu, 30 Jul 2026 14:40:57 +0100 Subject: [PATCH 2/6] Reusing existing errors --- score/launch_manager/src/lm_control/src/ilm_control.hpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/score/launch_manager/src/lm_control/src/ilm_control.hpp b/score/launch_manager/src/lm_control/src/ilm_control.hpp index 4df179093..afc124e67 100644 --- a/score/launch_manager/src/lm_control/src/ilm_control.hpp +++ b/score/launch_manager/src/lm_control/src/ilm_control.hpp @@ -17,6 +17,7 @@ #include #include +#include "score/mw/lifecycle/execution_error.h" #include "score/mw/lifecycle/fixed_string.hpp" #include "score/mw/lifecycle/run_target_activation_source.hpp" #include "score/result/result.h" @@ -81,7 +82,7 @@ class ILmControl /// @returns A unique_ptr to the ILmControl instance on success. /// /// @error kInvalidArguments instance_specifier is empty or malformed. - /// @error kNoConnection The Launch Manager service could not be reached. + /// @error kCommunicationError The Launch Manager service could not be reached. static score::Result> Create(std::string_view instance_specifier); /// @brief Virtual destructor for safe deletion through this interface. @@ -116,7 +117,8 @@ class ILmControl /// @error kRequestQueueIsFull Activation was rejected because Launch Manager cannot accept another activation /// request. /// @error kRunTargetDoesntExist Name of the requested Run Target does not exist in current configuration. - /// @error kNoConnection Connection with Launch Manager cannot be established and request cannot be sent. + /// @error kCommunicationError Connection with Launch Manager cannot be established and request cannot be + /// sent. virtual score::Result activate_run_target(std::string_view runTargetName, bool force = false) = 0; /// @brief Register a callback invoked whenever Launch Manager finishes a Run Target activation. @@ -150,7 +152,7 @@ class ILmControl /// /// @error kActivationInProgress Launch Manager is currently executing a Run Target /// activation and thus there is no single Run Target active. - /// @error kNoConnection Connection with Launch Manager cannot be established + /// @error kCommunicationError Connection with Launch Manager cannot be established /// and information about active Run Target cannot be retrieved. virtual score::Result get_active_run_target() = 0; From 1a8682c2780777f9746a23ea48700b77a8d240c3 Mon Sep 17 00:00:00 2001 From: SimonKozik <244535158+SimonKozik@users.noreply.github.com> Date: Fri, 31 Jul 2026 11:23:04 +0100 Subject: [PATCH 3/6] Applying changes resulted from the review comment --- .../src/lm_control/src/fixed_string.hpp | 28 +++++++++++++------ 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/score/launch_manager/src/lm_control/src/fixed_string.hpp b/score/launch_manager/src/lm_control/src/fixed_string.hpp index a28853a8d..a7f56078b 100644 --- a/score/launch_manager/src/lm_control/src/fixed_string.hpp +++ b/score/launch_manager/src/lm_control/src/fixed_string.hpp @@ -55,7 +55,7 @@ struct FixedString /// Only `storage_[0]` is set to `'\0'`; the rest of the array is indeterminate. /// This is intentional — zeroing `MaxLength + 1` bytes on every default /// construction would be wasteful when the value is immediately overwritten. - FixedString() noexcept + constexpr FixedString() noexcept { storage_[0] = '\0'; } @@ -68,10 +68,12 @@ struct FixedString /// available. /// /// @param s Source string. Does not require to be null-terminated. - explicit FixedString(std::string_view s) noexcept + constexpr explicit FixedString(std::string_view s) noexcept { const auto len = std::min(s.size(), MaxLength); - std::memcpy(storage_.data(), s.data(), len); + for (std::size_t i = 0; i < len; ++i) { + storage_[i] = s[i]; + } storage_[len] = '\0'; } @@ -92,10 +94,12 @@ struct FixedString /// @param s Source string. Does not require to be null-terminated. /// /// @returns Reference to this instance. - FixedString& operator=(std::string_view s) noexcept + constexpr FixedString& operator=(std::string_view s) noexcept { const auto len = std::min(s.size(), MaxLength); - std::memcpy(storage_.data(), s.data(), len); + for (std::size_t i = 0; i < len; ++i) { + storage_[i] = s[i]; + } storage_[len] = '\0'; return *this; } @@ -190,7 +194,9 @@ struct FixedString /// otherwise, false. bool operator==(const FixedString& other) const noexcept { - return as_string_view() == other.as_string_view(); + // strcmp does a single optimised pass — avoids the two strlen calls + // that constructing string_views would require. + return std::strcmp(storage_.data(), other.storage_.data()) == 0; } /// @brief Equality comparison with a string_view. @@ -204,7 +210,13 @@ struct FixedString /// @p other; otherwise, false. bool operator==(std::string_view other) const noexcept { - return as_string_view() == other; + // other.size() is already known — use it as a length guard and for + // memcmp, avoiding strlen on our storage. + if (other.size() > MaxLength) { + return false; + } + return std::strncmp(storage_.data(), other.data(), other.size()) == 0 + && storage_[other.size()] == '\0'; } /// @brief Inequality comparison with another FixedString of the same capacity. @@ -256,7 +268,7 @@ struct FixedString template bool operator==(const FixedString& lhs, const FixedString& rhs) noexcept { - return lhs.as_string_view() == rhs.as_string_view(); + return std::strcmp(lhs.data(), rhs.data()) == 0; } /// @brief Inequality comparison between FixedString instances of different capacities. From 41c20f0e077fc615ac7ec1f3fcd17c2e2afdfa92 Mon Sep 17 00:00:00 2001 From: SimonKozik <244535158+SimonKozik@users.noreply.github.com> Date: Wed, 5 Aug 2026 08:25:43 +0100 Subject: [PATCH 4/6] Applying review suggestion --- score/launch_manager/src/lm_control/src/ilm_control.hpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/score/launch_manager/src/lm_control/src/ilm_control.hpp b/score/launch_manager/src/lm_control/src/ilm_control.hpp index afc124e67..eca769f61 100644 --- a/score/launch_manager/src/lm_control/src/ilm_control.hpp +++ b/score/launch_manager/src/lm_control/src/ilm_control.hpp @@ -15,7 +15,6 @@ #include #include -#include #include "score/mw/lifecycle/execution_error.h" #include "score/mw/lifecycle/fixed_string.hpp" @@ -119,7 +118,7 @@ class ILmControl /// @error kRunTargetDoesntExist Name of the requested Run Target does not exist in current configuration. /// @error kCommunicationError Connection with Launch Manager cannot be established and request cannot be /// sent. - virtual score::Result activate_run_target(std::string_view runTargetName, bool force = false) = 0; + virtual score::Result activate_run_target(RunTargetName runTargetName, bool force = false) = 0; /// @brief Register a callback invoked whenever Launch Manager finishes a Run Target activation. /// From 1ac5c2fcc629ebecba5a6caf2cb91221e842ef5b Mon Sep 17 00:00:00 2001 From: SimonKozik <244535158+SimonKozik@users.noreply.github.com> Date: Wed, 5 Aug 2026 10:44:39 +0100 Subject: [PATCH 5/6] Addressing review comments --- .../src/lm_control/src/fixed_string.hpp | 12 +++++++----- .../src/lm_control/src/ilm_control.hpp | 3 +-- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/score/launch_manager/src/lm_control/src/fixed_string.hpp b/score/launch_manager/src/lm_control/src/fixed_string.hpp index a7f56078b..0368e0f27 100644 --- a/score/launch_manager/src/lm_control/src/fixed_string.hpp +++ b/score/launch_manager/src/lm_control/src/fixed_string.hpp @@ -71,7 +71,8 @@ struct FixedString constexpr explicit FixedString(std::string_view s) noexcept { const auto len = std::min(s.size(), MaxLength); - for (std::size_t i = 0; i < len; ++i) { + for (std::size_t i = 0; i < len; ++i) + { storage_[i] = s[i]; } storage_[len] = '\0'; @@ -97,7 +98,8 @@ struct FixedString constexpr FixedString& operator=(std::string_view s) noexcept { const auto len = std::min(s.size(), MaxLength); - for (std::size_t i = 0; i < len; ++i) { + for (std::size_t i = 0; i < len; ++i) + { storage_[i] = s[i]; } storage_[len] = '\0'; @@ -212,11 +214,11 @@ struct FixedString { // other.size() is already known — use it as a length guard and for // memcmp, avoiding strlen on our storage. - if (other.size() > MaxLength) { + if (other.size() > MaxLength) + { return false; } - return std::strncmp(storage_.data(), other.data(), other.size()) == 0 - && storage_[other.size()] == '\0'; + return std::strncmp(storage_.data(), other.data(), other.size()) == 0 && storage_[other.size()] == '\0'; } /// @brief Inequality comparison with another FixedString of the same capacity. diff --git a/score/launch_manager/src/lm_control/src/ilm_control.hpp b/score/launch_manager/src/lm_control/src/ilm_control.hpp index eca769f61..4dfab5028 100644 --- a/score/launch_manager/src/lm_control/src/ilm_control.hpp +++ b/score/launch_manager/src/lm_control/src/ilm_control.hpp @@ -87,8 +87,7 @@ class ILmControl /// @brief Virtual destructor for safe deletion through this interface. virtual ~ILmControl() noexcept = default; - // Non-copyable and non-movable. Polymorphic types must not be copied or - // moved through the interface — it would slice the concrete implementation. + // Non-copyable and non-movable. // Transfer ownership via std::unique_ptr instead. ILmControl(const ILmControl&) = delete; ILmControl& operator=(const ILmControl&) = delete; From 460b99dca9595dee7e1cd446adb636b359928ed9 Mon Sep 17 00:00:00 2001 From: SimonKozik <244535158+SimonKozik@users.noreply.github.com> Date: Thu, 6 Aug 2026 15:03:48 +0100 Subject: [PATCH 6/6] Marking Create method as work in progress, improving FixedString ux --- .../src/lm_control/src/fixed_string.hpp | 72 ++++++++++++++++--- .../src/lm_control/src/ilm_control.hpp | 14 +++- 2 files changed, 74 insertions(+), 12 deletions(-) diff --git a/score/launch_manager/src/lm_control/src/fixed_string.hpp b/score/launch_manager/src/lm_control/src/fixed_string.hpp index 0368e0f27..af67ca02f 100644 --- a/score/launch_manager/src/lm_control/src/fixed_string.hpp +++ b/score/launch_manager/src/lm_control/src/fixed_string.hpp @@ -35,7 +35,7 @@ namespace score::mw::lifecycle /// @par Invariant /// The stored string is always null-terminated. Every constructor and assignment path /// enforces this. Characters beyond the null terminator are indeterminate and must -/// not be read directly — use `as_string_view()` or `data()` instead. +/// not be read directly - use `as_string_view()` or `data()` instead. /// /// @par Copying and moving /// Copy and move are compiler-generated (rule of zero). The `std::array` storage @@ -53,17 +53,42 @@ struct FixedString /// @brief Construct an empty string (equivalent to ""). /// /// Only `storage_[0]` is set to `'\0'`; the rest of the array is indeterminate. - /// This is intentional — zeroing `MaxLength + 1` bytes on every default + /// This is intentional - zeroing `MaxLength + 1` bytes on every default /// construction would be wasteful when the value is immediately overwritten. constexpr FixedString() noexcept { storage_[0] = '\0'; } + /// @brief Construct from a string literal or compile-time char array. + /// + /// Accepts only string literals and `const char[N]` arrays - not runtime + /// `const char*` pointers. This prevents null pointer and dangling pointer + /// bugs that a `const char*` constructor would allow implicitly. + /// + /// If the literal is longer than `MaxLength` characters (excluding the null + /// terminator), the build fails with a `static_assert` - no silent truncation. + /// + /// @tparam N Size of the char array, including the null terminator. + /// For a string literal `"foo"`, N == 4. + /// @param s Source char array. Must be null-terminated (guaranteed for literals). + template + constexpr FixedString(const char (&s)[N]) noexcept + { + static_assert( + N - 1U <= MaxLength, + "String literal exceeds FixedString capacity - use a shorter literal or increase MaxLength"); + for (std::size_t i = 0U; i < N - 1U; ++i) + { + storage_[i] = s[i]; + } + storage_[N - 1U] = '\0'; + } + /// @brief Construct from a string_view, truncating if necessary. /// /// If `s.size() > MaxLength`, only the first `MaxLength` characters are stored. - /// No error is raised at this level — detection and logging of over-length + /// No error is raised at this level - detection and logging of over-length /// strings must happen at the user side where the source and context are /// available. /// @@ -196,24 +221,21 @@ struct FixedString /// otherwise, false. bool operator==(const FixedString& other) const noexcept { - // strcmp does a single optimised pass — avoids the two strlen calls + // strcmp does a single optimised pass - avoids the two strlen calls // that constructing string_views would require. return std::strcmp(storage_.data(), other.storage_.data()) == 0; } /// @brief Equality comparison with a string_view. /// - /// Allows comparisons such as `fs == "Running"` without constructing - /// a temporary FixedString. - /// /// @param other The string value to compare against. /// /// @return true if this FixedString represents the same string as /// @p other; otherwise, false. bool operator==(std::string_view other) const noexcept { - // other.size() is already known — use it as a length guard and for - // memcmp, avoiding strlen on our storage. + // other.size() is already known - use it as a length guard and for + // strncmp, avoiding strlen on our storage. if (other.size() > MaxLength) { return false; @@ -221,6 +243,25 @@ struct FixedString return std::strncmp(storage_.data(), other.data(), other.size()) == 0 && storage_[other.size()] == '\0'; } + /// @brief Equality comparison with a string literal or compile-time char array. + /// + /// This overload is an exact match for string literals, resolving the + /// ambiguity that would otherwise arise between operator==(const FixedString&) + /// and operator==(std::string_view) when both require one implicit conversion + /// from a char array. An exact-match template is always preferred. + /// + /// @tparam N Size of the char array including the null terminator. + /// @return true if this FixedString represents the same string; otherwise false. + template + bool operator==(const char (&other)[N]) const noexcept + { + if (N - 1U > MaxLength) + { + return false; + } + return std::strncmp(storage_.data(), other, N - 1U) == 0 && storage_[N - 1U] == '\0'; + } + /// @brief Inequality comparison with another FixedString of the same capacity. /// /// Inequality is determined by comparing the string representations of @@ -249,6 +290,19 @@ struct FixedString return !(*this == other); } + /// @brief Inequality comparison with a string literal or compile-time char array. + /// + /// Exact-match overload that mirrors operator==(const char(&)[N]) to avoid + /// ambiguity when comparing against string literals. + /// + /// @tparam N Size of the char array including the null terminator. + /// @return true if this FixedString represents a different string; otherwise false. + template + bool operator!=(const char (&other)[N]) const noexcept + { + return !(*this == other); + } + private: std::array storage_; }; diff --git a/score/launch_manager/src/lm_control/src/ilm_control.hpp b/score/launch_manager/src/lm_control/src/ilm_control.hpp index 4dfab5028..8427a088c 100644 --- a/score/launch_manager/src/lm_control/src/ilm_control.hpp +++ b/score/launch_manager/src/lm_control/src/ilm_control.hpp @@ -37,7 +37,7 @@ using RunTargetName = FixedString; /// /// Callers obtain an instance via ILmControl::Create() and interact with the /// Launch Manager exclusively through this interface. The concrete implementation -/// is an internal detail — it is not visible in this header. +/// is an internal detail - it is not visible in this header. /// /// @par Usage /// ```cpp @@ -61,12 +61,20 @@ class ILmControl /// The subscriber sees every settling. /// /// @param[in] activationSource What caused the activation to occur. - /// @param[in] activatedRunTarget The Run Target that was activated — may + /// @param[in] activatedRunTarget The Run Target that was activated - may /// differ from the one originally requested. using ActivationCallback = std::function; - /// @brief Factory method — create a connected ILmControl instance. + /// @brief Factory method - create a connected ILmControl instance. + /// + /// @warning **Work in progress API shape not yet finalised.** + /// The signature and behaviour of this method may change. Open + /// questions include how much of the mw::com initialisation and + /// configuration is the caller's responsibility versus handled + /// internally, and whether the instance specifier is the right + /// abstraction to expose at this level. Do not treat this interface + /// as stable until these decisions are resolved. /// /// Establishes the mw::com connection to the Launch Manager. ///