From cde16bad5612a981973bdac3a635013132cb4ce9 Mon Sep 17 00:00:00 2001 From: Daniel Thwaites Date: Wed, 9 Sep 2026 11:28:06 +0100 Subject: [PATCH] Split `activate_run_target` into two methods Forced activation will be implemented first, so splitting it makes it possible to just add the queue method later, rather than having certain combinations of arguments which cause an error. I think this will also make the usage more readable, as the behaviour is explained in the method name, rather than a boolean argument where you have to read the documentation to know what it does. --- .../src/details/lm_control_impl.hpp | 44 ++++++++-- .../src/details/lm_control_impl_UT.cpp | 82 ++++++++++++++----- .../src/lm_control/src/ilm_control.hpp | 35 ++++++-- 3 files changed, 127 insertions(+), 34 deletions(-) diff --git a/score/launch_manager/src/lm_control/src/details/lm_control_impl.hpp b/score/launch_manager/src/lm_control/src/details/lm_control_impl.hpp index ef9591342a..f617a2abf8 100644 --- a/score/launch_manager/src/lm_control/src/details/lm_control_impl.hpp +++ b/score/launch_manager/src/lm_control/src/details/lm_control_impl.hpp @@ -170,7 +170,40 @@ class BasicLmControlImpl final : public ILmControl } } - score::Result activate_run_target(RunTargetName runTargetName, bool force) override + /* + Not implemented on the daemon side yet. + + score::Result queue_run_target(RunTargetName runTargetName) override + { + auto* const proxy = connectedProxy(); + if (proxy == nullptr) + { + return score::MakeUnexpected(ExecErrc::kCommunicationError); + } + + LM_LOG_DEBUG() << "LmControl: queue_run_target:" << runTargetName; + + const ActivateRunTargetRequest request{runTargetName, ActivationMode::kQueued}; + + const score::Result> result = proxy->activate_run_target(request); + if (!result.has_value()) + { + LM_LOG_ERROR() << "LmControl: queue_run_target: proxy method call failed with error:" << result.error(); + return score::MakeUnexpected(ExecErrc::kCommunicationError); + } + + const auto& response = *result.value(); + if (response.status == RequestStatus::kRejected) + { + LM_LOG_DEBUG() << "LmControl: queue_run_target: rejected by LM with code" << response.rejection_reason; + return score::MakeUnexpected(response.rejection_reason); + } + + return {}; + } + */ + + score::Result force_run_target(RunTargetName runTargetName) override { auto* const proxy = connectedProxy(); if (proxy == nullptr) @@ -178,22 +211,21 @@ class BasicLmControlImpl final : public ILmControl return score::MakeUnexpected(ExecErrc::kCommunicationError); } - LM_LOG_DEBUG() << "LmControl: activate_run_target:" << runTargetName << " force=" << force; + LM_LOG_DEBUG() << "LmControl: force_run_target:" << runTargetName; - const ActivateRunTargetRequest request{ - runTargetName, force ? ActivationMode::kForced : ActivationMode::kQueued}; + const ActivateRunTargetRequest request{runTargetName, ActivationMode::kForced}; const score::Result> result = proxy->activate_run_target(request); if (!result.has_value()) { - LM_LOG_ERROR() << "LmControl: activate_run_target: proxy method call failed with error:" << result.error(); + LM_LOG_ERROR() << "LmControl: force_run_target: proxy method call failed with error:" << result.error(); return score::MakeUnexpected(ExecErrc::kCommunicationError); } const auto& response = *result.value(); if (response.status == RequestStatus::kRejected) { - LM_LOG_DEBUG() << "LmControl: activate_run_target: rejected by LM with code" << response.rejection_reason; + LM_LOG_DEBUG() << "LmControl: force_run_target: rejected by LM with code" << response.rejection_reason; return score::MakeUnexpected(response.rejection_reason); } diff --git a/score/launch_manager/src/lm_control/src/details/lm_control_impl_UT.cpp b/score/launch_manager/src/lm_control/src/details/lm_control_impl_UT.cpp index ec9d8a703e..66b0ed8bed 100644 --- a/score/launch_manager/src/lm_control/src/details/lm_control_impl_UT.cpp +++ b/score/launch_manager/src/lm_control/src/details/lm_control_impl_UT.cpp @@ -359,7 +359,7 @@ TEST_F(LmControlUT, MethodsFailWhileNotYetConnected) { RecordProperty( "Description", - "Before the service is discovered, activate_run_target and get_active_run_target return " + "Before the service is discovered, queue_run_target, force_run_target and get_active_run_target return " "kCommunicationError without issuing any proxy calls."); EXPECT_CALL(mock_, ActivateRunTarget(_)).Times(0); @@ -367,9 +367,15 @@ TEST_F(LmControlUT, MethodsFailWhileNotYetConnected) auto sut = MakeLmControl(); - auto activate = sut->activate_run_target("Running", false); - ASSERT_FALSE(activate.has_value()); - EXPECT_EQ(activate.error(), ExecErrc::kCommunicationError); + /* + auto queue = sut->queue_run_target("Running"); + ASSERT_FALSE(queue.has_value()); + EXPECT_EQ(queue.error(), ExecErrc::kCommunicationError); + */ + + auto force = sut->force_run_target("Running"); + ASSERT_FALSE(force.has_value()); + EXPECT_EQ(force.error(), ExecErrc::kCommunicationError); auto get = sut->get_active_run_target(); ASSERT_FALSE(get.has_value()); @@ -516,13 +522,13 @@ TEST_F(LmControlUT, StopFindServiceFailureOnDestructionIsTolerated) } // --------------------------------------------------------------------------- -// activate_run_target +// queue_run_target // --------------------------------------------------------------------------- -TEST_F(LmControlUT, ActivateForwardsNameAndQueuedModeByDefault) +/* +TEST_F(LmControlUT, QueueForwardsNameAndUsesQueue) { - RecordProperty( - "Description", "activate_run_target forwards the run target name and defaults to queued activation mode."); + RecordProperty("Description", "queue_run_target forwards the run target name and uses the queued activation mode."); auto sut = MakeConnected(); @@ -533,46 +539,82 @@ TEST_F(LmControlUT, ActivateForwardsNameAndQueuedModeByDefault) Field(&ActivateRunTargetRequest::mode, ActivationMode::kQueued)))) .WillOnce(Return(Accepted())); - // Through the interface: that is where the force=false default lives. ILmControl& lm_control = *sut; - EXPECT_TRUE(lm_control.activate_run_target("Driving").has_value()); + EXPECT_TRUE(lm_control.queue_run_target("Driving").has_value()); +} + +TEST_F(LmControlUT, QueueRejectionSurfacesRejectionReason) +{ + RecordProperty("Description", "A rejected queue_run_target surfaces the rejection reason as the returned error."); + + auto sut = MakeConnected(); + + EXPECT_CALL(mock_, ActivateRunTarget(_)) + .WillOnce(Return(ActivateRunTargetResponse{RequestStatus::kRejected, ExecErrc::kRequestQueueIsFull})); + + auto result = sut->queue_run_target("Driving"); + ASSERT_FALSE(result.has_value()); + EXPECT_EQ(result.error(), ExecErrc::kRequestQueueIsFull); } -TEST_F(LmControlUT, ActivateForcedMapsToForcedMode) +TEST_F(LmControlUT, QueueTransportFailureReturnsCommunicationError) { - RecordProperty("Description", "activate_run_target with force=true maps to forced activation mode."); + RecordProperty("Description", "A transport failure during queue_run_target is reported as kCommunicationError."); auto sut = MakeConnected(); - EXPECT_CALL(mock_, ActivateRunTarget(Field(&ActivateRunTargetRequest::mode, ActivationMode::kForced))) + EXPECT_CALL(mock_, ActivateRunTarget(_)).WillOnce(Return(score::MakeUnexpected(ExecErrc::kFailed))); + + auto result = sut->queue_run_target("Driving"); + ASSERT_FALSE(result.has_value()); + EXPECT_EQ(result.error(), ExecErrc::kCommunicationError); +} +*/ + +// --------------------------------------------------------------------------- +// force_run_target +// --------------------------------------------------------------------------- + +TEST_F(LmControlUT, ForceForwardsNameAndUsesForce) +{ + RecordProperty("Description", "force_run_target forwards the run target name and uses the forced activation mode."); + + auto sut = MakeConnected(); + + EXPECT_CALL( + mock_, + ActivateRunTarget(AllOf( + Field(&ActivateRunTargetRequest::run_target_name, RunTargetName{"Driving"}), + Field(&ActivateRunTargetRequest::mode, ActivationMode::kForced)))) .WillOnce(Return(Accepted())); - EXPECT_TRUE(sut->activate_run_target("Driving", /*force=*/true).has_value()); + ILmControl& lm_control = *sut; + EXPECT_TRUE(lm_control.force_run_target("Driving").has_value()); } -TEST_F(LmControlUT, ActivateRejectionSurfacesRejectionReason) +TEST_F(LmControlUT, ForceRejectionSurfacesRejectionReason) { - RecordProperty("Description", "A rejected activation surfaces the rejection reason as the returned error."); + RecordProperty("Description", "A rejected force_run_target surfaces the rejection reason as the returned error."); auto sut = MakeConnected(); EXPECT_CALL(mock_, ActivateRunTarget(_)) .WillOnce(Return(ActivateRunTargetResponse{RequestStatus::kRejected, ExecErrc::kRequestQueueIsFull})); - auto result = sut->activate_run_target("Driving", false); + auto result = sut->force_run_target("Driving"); ASSERT_FALSE(result.has_value()); EXPECT_EQ(result.error(), ExecErrc::kRequestQueueIsFull); } -TEST_F(LmControlUT, ActivateTransportFailureReturnsCommunicationError) +TEST_F(LmControlUT, ForceTransportFailureReturnsCommunicationError) { - RecordProperty("Description", "A transport failure during activate_run_target is reported as kCommunicationError."); + RecordProperty("Description", "A transport failure during force_run_target is reported as kCommunicationError."); auto sut = MakeConnected(); EXPECT_CALL(mock_, ActivateRunTarget(_)).WillOnce(Return(score::MakeUnexpected(ExecErrc::kFailed))); - auto result = sut->activate_run_target("Driving", false); + auto result = sut->force_run_target("Driving"); ASSERT_FALSE(result.has_value()); EXPECT_EQ(result.error(), ExecErrc::kCommunicationError); } 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 bf46c56fc7..6b0e357699 100644 --- a/score/launch_manager/src/lm_control/src/ilm_control.hpp +++ b/score/launch_manager/src/lm_control/src/ilm_control.hpp @@ -109,17 +109,15 @@ class ILmControl /// /// 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(). + /// executes activations one at a time in FIFO order. + /// /// If the queue is full, kRequestQueueIsFull is returned immediately /// and the request is discarded. /// + /// Completion is notified asynchronously via the callback registered with + /// register_run_target_activation_callback(). + /// /// @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. /// @@ -128,7 +126,28 @@ class ILmControl /// @error kRunTargetDoesntExist Name of the requested Run Target does not exist in current configuration. /// @error kCommunicationError Connection with Launch Manager could not be established and request cannot be /// sent. - virtual score::Result activate_run_target(RunTargetName runTargetName, bool force = false) = 0; + /* + Not implemented on the daemon side yet. + + virtual score::Result queue_run_target(RunTargetName runTargetName) = 0; + */ + + /// @brief Request Run Target activation. + /// + /// Cancels any previously queued activations, and immediately begins + /// switching to the specified run target. + /// + /// Completion is notified asynchronously via the callback registered with + /// register_run_target_activation_callback(). + /// + /// @param[in] runTargetName Name of a Run Target configured in the Launch Manager. + /// + /// @returns void when the Launch Manager accepted the request. + /// + /// @error kRunTargetDoesntExist Name of the requested Run Target does not exist in current configuration. + /// @error kCommunicationError Connection with Launch Manager could not be established and request cannot be + /// sent. + virtual score::Result force_run_target(RunTargetName runTargetName) = 0; /// @brief Register a callback invoked whenever Launch Manager finishes a Run Target activation. ///