diff --git a/src/DebugServer/Gdb/CommandPackets/Detach.cpp b/src/DebugServer/Gdb/CommandPackets/Detach.cpp index 73ef16ed..043ab965 100644 --- a/src/DebugServer/Gdb/CommandPackets/Detach.cpp +++ b/src/DebugServer/Gdb/CommandPackets/Detach.cpp @@ -25,13 +25,13 @@ namespace Bloom::DebugServer::Gdb::CommandPackets try { if (Services::ProcessService::isManagedByClion()) { - targetControllerService.suspendTargetController(); + // TODO: Force the TC to shutdown. } debugSession.connection.writePacket(OkResponsePacket()); } catch (const Exception& exception) { - Logger::error("Failed to suspend TargetController - " + exception.getMessage()); + Logger::error("Failed to shut down TargetController - " + exception.getMessage()); debugSession.connection.writePacket(ErrorResponsePacket()); } } diff --git a/src/DebugServer/Gdb/GdbRspDebugServer.cpp b/src/DebugServer/Gdb/GdbRspDebugServer.cpp index 82502318..cfbc7ef4 100644 --- a/src/DebugServer/Gdb/GdbRspDebugServer.cpp +++ b/src/DebugServer/Gdb/GdbRspDebugServer.cpp @@ -48,8 +48,6 @@ namespace Bloom::DebugServer::Gdb using CommandPackets::CommandPacket; - using TargetController::TargetControllerState; - GdbRspDebugServer::GdbRspDebugServer( const DebugServerConfig& debugServerConfig, EventListener& eventListener, @@ -160,25 +158,6 @@ namespace Bloom::DebugServer::Gdb this->debugServerConfig ); - /* - * Before proceeding with a new debug session, we must ensure that the TargetController is able to - * service it. - */ - if (!this->targetControllerService.isTargetControllerInService()) { - // The TargetController is suspended - attempt to wake it up - try { - this->targetControllerService.resumeTargetController(); - - } catch (Bloom::Exceptions::Exception& exception) { - Logger::error("Failed to wake up TargetController - " + exception.getMessage()); - } - - if (!this->targetControllerService.isTargetControllerInService()) { - this->activeDebugSession.reset(); - throw DebugSessionInitialisationFailure("TargetController not in service"); - } - } - this->targetControllerService.stopTargetExecution(); this->targetControllerService.resetTarget(); } diff --git a/src/EventManager/Events/Event.hpp b/src/EventManager/Events/Event.hpp index ec597452..5ab41b82 100644 --- a/src/EventManager/Events/Event.hpp +++ b/src/EventManager/Events/Event.hpp @@ -18,7 +18,6 @@ namespace Bloom::Events DEBUG_SESSION_STARTED, DEBUG_SESSION_FINISHED, TARGET_CONTROLLER_THREAD_STATE_CHANGED, - TARGET_CONTROLLER_STATE_CHANGED, SHUTDOWN_TARGET_CONTROLLER, TARGET_CONTROLLER_ERROR_OCCURRED, SHUTDOWN_APPLICATION, diff --git a/src/EventManager/Events/Events.hpp b/src/EventManager/Events/Events.hpp index f3c28f47..1164f183 100644 --- a/src/EventManager/Events/Events.hpp +++ b/src/EventManager/Events/Events.hpp @@ -6,7 +6,6 @@ #include "DebugSessionStarted.hpp" #include "DebugSessionFinished.hpp" #include "TargetControllerThreadStateChanged.hpp" -#include "TargetControllerStateChanged.hpp" #include "ShutdownTargetController.hpp" #include "TargetControllerErrorOccurred.hpp" #include "ShutdownApplication.hpp" diff --git a/src/EventManager/Events/TargetControllerStateChanged.hpp b/src/EventManager/Events/TargetControllerStateChanged.hpp deleted file mode 100644 index c323ab63..00000000 --- a/src/EventManager/Events/TargetControllerStateChanged.hpp +++ /dev/null @@ -1,29 +0,0 @@ -#pragma once - -#include - -#include "Event.hpp" -#include "src/TargetController/TargetControllerState.hpp" - -namespace Bloom::Events -{ - class TargetControllerStateChanged: public Event - { - public: - static constexpr EventType type = EventType::TARGET_CONTROLLER_STATE_CHANGED; - static const inline std::string name = "TargetControllerStateChanged"; - - TargetController::TargetControllerState state; - explicit TargetControllerStateChanged(TargetController::TargetControllerState state) - : state(state) - {}; - - [[nodiscard]] EventType getType() const override { - return TargetControllerStateChanged::type; - } - - [[nodiscard]] std::string getName() const override { - return TargetControllerStateChanged::name; - } - }; -} diff --git a/src/Services/TargetControllerService.cpp b/src/Services/TargetControllerService.cpp index c137752d..7b9b501c 100644 --- a/src/Services/TargetControllerService.cpp +++ b/src/Services/TargetControllerService.cpp @@ -1,9 +1,6 @@ #include "TargetControllerService.hpp" // Commands -#include "src/TargetController/Commands/GetState.hpp" -#include "src/TargetController/Commands/Suspend.hpp" -#include "src/TargetController/Commands/Resume.hpp" #include "src/TargetController/Commands/GetTargetDescriptor.hpp" #include "src/TargetController/Commands/GetTargetState.hpp" #include "src/TargetController/Commands/StopTargetExecution.hpp" @@ -27,9 +24,6 @@ namespace Bloom::Services { - using TargetController::Commands::GetState; - using TargetController::Commands::Suspend; - using TargetController::Commands::Resume; using TargetController::Commands::GetTargetDescriptor; using TargetController::Commands::GetTargetState; using TargetController::Commands::StopTargetExecution; @@ -51,8 +45,6 @@ namespace Bloom::Services using TargetController::Commands::EnableProgrammingMode; using TargetController::Commands::DisableProgrammingMode; - using TargetController::TargetControllerState; - using Targets::TargetDescriptor; using Targets::TargetState; @@ -73,38 +65,6 @@ namespace Bloom::Services using Targets::TargetPinState; using Targets::TargetPinStateMapping; - TargetControllerState TargetControllerService::getTargetControllerState() const { - return this->commandManager.sendCommandAndWaitForResponse( - std::make_unique(), - this->defaultTimeout - )->state; - } - - bool TargetControllerService::isTargetControllerInService() const noexcept { - try { - return this->getTargetControllerState() == TargetControllerState::ACTIVE; - - } catch (...) { - return false; - } - } - - void TargetControllerService::resumeTargetController() const { - this->commandManager.sendCommandAndWaitForResponse( - std::make_unique(), - this->defaultTimeout - ); - return; - } - - void TargetControllerService::suspendTargetController() const { - this->commandManager.sendCommandAndWaitForResponse( - std::make_unique(), - this->defaultTimeout - ); - return; - } - const TargetDescriptor& TargetControllerService::getTargetDescriptor() const { return this->commandManager.sendCommandAndWaitForResponse( std::make_unique(), diff --git a/src/Services/TargetControllerService.hpp b/src/Services/TargetControllerService.hpp index 3c46ffc1..1b114676 100644 --- a/src/Services/TargetControllerService.hpp +++ b/src/Services/TargetControllerService.hpp @@ -5,7 +5,6 @@ #include #include "src/TargetController/CommandManager.hpp" -#include "src/TargetController/TargetControllerState.hpp" #include "src/Targets/TargetState.hpp" #include "src/Targets/TargetRegister.hpp" @@ -30,35 +29,6 @@ namespace Bloom::Services this->defaultTimeout = timeout; } - /** - * Requests the current TargetController state from the TargetController. The TargetController should always - * respond to such a request, even when it's in a suspended state. - * - * To check if the TargetController is in an active state, isTargetControllerInService() can be used for - * convenience. - * - * @return - */ - TargetController::TargetControllerState getTargetControllerState() const; - - /** - * Retrieves the TargetController state and checks if it's currently active. - * - * @return - * True if the TargetController is currently in an active state, otherwise false. - */ - bool isTargetControllerInService() const noexcept; - - /** - * Resumes the TargetController if it's suspended. Otherwise, this function does nothing. - */ - void resumeTargetController() const; - - /** - * Suspends the TargetController if it's active. Otherwise, this function does nothing. - */ - void suspendTargetController() const; - /** * Requests the TargetDescriptor from the TargetController * diff --git a/src/TargetController/Commands/Command.hpp b/src/TargetController/Commands/Command.hpp index b1d83c82..0623dc2d 100644 --- a/src/TargetController/Commands/Command.hpp +++ b/src/TargetController/Commands/Command.hpp @@ -36,10 +36,6 @@ namespace Bloom::TargetController::Commands return Command::type; } - [[nodiscard]] virtual bool requiresActiveState() const { - return true; - } - [[nodiscard]] virtual bool requiresStoppedTargetState() const { return false; } diff --git a/src/TargetController/Commands/GetState.hpp b/src/TargetController/Commands/GetState.hpp deleted file mode 100644 index 43fce95f..00000000 --- a/src/TargetController/Commands/GetState.hpp +++ /dev/null @@ -1,29 +0,0 @@ -#pragma once - -#include "Command.hpp" - -#include "src/TargetController/Responses/State.hpp" - -namespace Bloom::TargetController::Commands -{ - class GetState: public Command - { - public: - using SuccessResponseType = Responses::State; - - static constexpr CommandType type = CommandType::GET_STATE; - static const inline std::string name = "GetState"; - - [[nodiscard]] CommandType getType() const override { - return GetState::type; - } - - [[nodiscard]] bool requiresActiveState() const override { - return false; - } - - [[nodiscard]] bool requiresDebugMode() const override { - return false; - } - }; -} diff --git a/src/TargetController/README.md b/src/TargetController/README.md index fdf6e581..bbf51619 100644 --- a/src/TargetController/README.md +++ b/src/TargetController/README.md @@ -76,40 +76,6 @@ All components within Bloom should use the `TargetControllerService` class to in **should not** directly issue commands via the `Bloom::TargetController::CommandManager`, unless there is a very good reason to do so. -### TargetController suspension - -The TargetController possesses the ability to go into a suspended state. In this state, control of the connected -hardware is surrendered - Bloom will no longer be able to interact with the debug tool or target. The purpose of this -state is to allow other programs access to the hardware, without requiring the termination of the Bloom process. The -TargetController goes into a suspended state at the end of a debug session, if the user has enabled this via the -`releasePostDebugSession` debug tool parameter, in their project configuration file (bloom.yaml). See -`TargetControllerComponent::onDebugSessionFinishedEvent()` for more. - -When in a suspended state, the TargetController will reject most commands. More specifically, any command that -requires access to the debug tool or target. Issuing any of these commands whilst the TargetController is suspended -will result in an error response. - -In some cases, the TargetController may be forced to go into a suspended state. This could be in response to the user -disconnecting the debug tool, or from another program stealing control of the hardware. Actually, this is what led to -the introduction of TargetController suspension. See the corresponding -[GitHub issue](https://github.com/navnavnav/Bloom/issues/3) for more. - -Upon suspension, the TargetController will trigger a `Bloom::Events::TargetControllerStateChanged` event. Other -components listen for this event to promptly perform the necessary actions in response to the state change. For example, -the [GDB debug server implementation](../DebugServer/Gdb/README.md) will terminate any active debug session: - -```c++ -void GdbRspDebugServer::onTargetControllerStateChanged(const Events::TargetControllerStateChanged& event) { - if (event.state == TargetControllerState::SUSPENDED && this->activeDebugSession.has_value()) { - Logger::warning("TargetController suspended unexpectedly - terminating debug session"); - this->activeDebugSession.reset(); - } -} -``` - -For more on TargetController suspension, see `TargetControllerComponent::suspend()` and -`TargetControllerComponent::resume()`. - ### Programming mode When a component needs to write to the target's program memory, it must enable programming mode on the target. This can diff --git a/src/TargetController/Responses/State.hpp b/src/TargetController/Responses/State.hpp deleted file mode 100644 index f7c0495f..00000000 --- a/src/TargetController/Responses/State.hpp +++ /dev/null @@ -1,24 +0,0 @@ -#pragma once - -#include "Response.hpp" - -#include "src/TargetController/TargetControllerState.hpp" - -namespace Bloom::TargetController::Responses -{ - class State: public Response - { - public: - static constexpr ResponseType type = ResponseType::STATE; - - TargetControllerState state; - - explicit State(TargetControllerState state) - : state(state) - {} - - [[nodiscard]] ResponseType getType() const override { - return State::type; - } - }; -} diff --git a/src/TargetController/TargetControllerComponent.cpp b/src/TargetController/TargetControllerComponent.cpp index ec3e9be5..8fd2515c 100644 --- a/src/TargetController/TargetControllerComponent.cpp +++ b/src/TargetController/TargetControllerComponent.cpp @@ -20,9 +20,6 @@ namespace Bloom::TargetController using Commands::CommandIdType; using Commands::Command; - using Commands::GetState; - using Commands::Resume; - using Commands::Suspend; using Commands::GetTargetDescriptor; using Commands::GetTargetState; using Commands::StopTargetExecution; @@ -67,9 +64,7 @@ namespace Bloom::TargetController Logger::debug("TargetController ready and waiting for events."); while (this->getThreadState() == ThreadState::READY) { - if (this->state == TargetControllerState::ACTIVE) { - this->fireTargetEvents(); - } + this->fireTargetEvents(); TargetControllerComponent::notifier.waitForNotification(std::chrono::milliseconds(60)); @@ -86,6 +81,10 @@ namespace Bloom::TargetController } void TargetControllerComponent::registerCommand(std::unique_ptr command) { + if (TargetControllerComponent::state != TargetControllerState::ACTIVE) { + throw Exception("Command rejected - TargetController not in active state."); + } + auto commandQueueLock = TargetControllerComponent::commandQueue.acquireLock(); TargetControllerComponent::commandQueue.getValue().push(std::move(command)); TargetControllerComponent::notifier.notify(); @@ -140,18 +139,6 @@ namespace Bloom::TargetController EventManager::registerListener(this->eventListener); // Register command handlers - this->registerCommandHandler( - std::bind(&TargetControllerComponent::handleGetState, this, std::placeholders::_1) - ); - - this->registerCommandHandler( - std::bind(&TargetControllerComponent::handleResume, this, std::placeholders::_1) - ); - - this->registerCommandHandler( - std::bind(&TargetControllerComponent::handleSuspend, this, std::placeholders::_1) - ); - this->registerCommandHandler( std::bind(&TargetControllerComponent::handleGetTargetDescriptor, this, std::placeholders::_1) ); @@ -237,11 +224,48 @@ namespace Bloom::TargetController std::bind(&TargetControllerComponent::onShutdownTargetControllerEvent, this, std::placeholders::_1) ); - this->eventListener->registerCallbackForEventType( - std::bind(&TargetControllerComponent::onDebugSessionStartedEvent, this, std::placeholders::_1) + this->eventListener->registerCallbackForEventType( + std::bind(&TargetControllerComponent::onDebugSessionFinishedEvent, this, std::placeholders::_1) ); - this->resume(); + this->acquireHardware(); + this->loadRegisterDescriptors(); + + if (this->target->getState() != TargetState::RUNNING) { + this->target->run(); + this->lastTargetState = TargetState::RUNNING; + } + + this->state = TargetControllerState::ACTIVE; + } + + void TargetControllerComponent::shutdown() { + const auto threadState = this->getThreadState(); + if (threadState == ThreadState::SHUTDOWN_INITIATED || threadState == ThreadState::STOPPED) { + return; + } + + this->threadState = ThreadState::SHUTDOWN_INITIATED; + + try { + Logger::info("Shutting down TargetController"); + this->state = TargetControllerState::INACTIVE; + EventManager::deregisterListener(this->eventListener->getId()); + + // Reject any commands still waiting in the queue + this->processQueuedCommands(); + + this->releaseHardware(); + + } catch (const std::exception& exception) { + this->target.reset(); + this->debugTool.reset(); + Logger::error( + "Failed to properly shutdown TargetController. Error: " + std::string(exception.what()) + ); + } + + this->setThreadStateAndEmitEvent(ThreadState::STOPPED); } std::map< @@ -357,20 +381,18 @@ namespace Bloom::TargetController throw Exception("No handler registered for this command."); } - if (this->state != TargetControllerState::ACTIVE && command->requiresActiveState()) { + if (this->state != TargetControllerState::ACTIVE) { throw Exception("Command rejected - TargetController not in active state."); } - if (this->state == TargetControllerState::ACTIVE) { - if (command->requiresStoppedTargetState() && this->lastTargetState != TargetState::STOPPED) { - throw Exception("Command rejected - command requires target execution to be stopped."); - } + if (command->requiresStoppedTargetState() && this->lastTargetState != TargetState::STOPPED) { + throw Exception("Command rejected - command requires target execution to be stopped."); + } - if (this->target->programmingModeEnabled() && command->requiresDebugMode()) { - throw Exception( - "Command rejected - command cannot be serviced whilst the target is in programming mode." - ); - } + if (this->target->programmingModeEnabled() && command->requiresDebugMode()) { + throw Exception( + "Command rejected - command cannot be serviced whilst the target is in programming mode." + ); } this->registerCommandResponse(commandId, commandHandlerIt->second(*(command.get()))); @@ -403,74 +425,6 @@ namespace Bloom::TargetController TargetControllerComponent::responsesByCommandIdCv.notify_all(); } - void TargetControllerComponent::shutdown() { - if (this->getThreadState() == ThreadState::STOPPED) { - return; - } - - try { - Logger::info("Shutting down TargetController"); - EventManager::deregisterListener(this->eventListener->getId()); - this->releaseHardware(); - - } catch (const std::exception& exception) { - this->target.reset(); - this->debugTool.reset(); - Logger::error( - "Failed to properly shutdown TargetController. Error: " + std::string(exception.what()) - ); - } - - this->setThreadStateAndEmitEvent(ThreadState::STOPPED); - } - - void TargetControllerComponent::suspend() { - if (this->getThreadState() != ThreadState::READY) { - return; - } - - Logger::debug("Suspending TargetController"); - - try { - this->releaseHardware(); - - } catch (const std::exception& exception) { - Logger::error("Failed to release connected debug tool and target resources. Error: " - + std::string(exception.what())); - } - - this->eventListener->deregisterCallbacksForEventType(); - - this->lastTargetState = TargetState::UNKNOWN; - this->targetDescriptor = std::nullopt; - this->registerDescriptorsByMemoryType.clear(); - this->registerAddressRangeByMemoryType.clear(); - - TargetControllerComponent::state = TargetControllerState::SUSPENDED; - EventManager::triggerEvent(std::make_shared(TargetControllerComponent::state)); - - Logger::debug("TargetController suspended"); - } - - void TargetControllerComponent::resume() { - this->acquireHardware(); - this->loadRegisterDescriptors(); - - this->eventListener->registerCallbackForEventType( - std::bind(&TargetControllerComponent::onDebugSessionFinishedEvent, this, std::placeholders::_1) - ); - - TargetControllerComponent::state = TargetControllerState::ACTIVE; - EventManager::triggerEvent( - std::make_shared(TargetControllerComponent::state) - ); - - if (this->target->getState() != TargetState::RUNNING) { - this->target->run(); - this->lastTargetState = TargetState::RUNNING; - } - } - void TargetControllerComponent::acquireHardware() { auto debugToolName = this->environmentConfig.debugToolConfig.name; auto targetName = this->environmentConfig.targetConfig.name; @@ -675,42 +629,17 @@ namespace Bloom::TargetController this->shutdown(); } - void TargetControllerComponent::onDebugSessionStartedEvent(const Events::DebugSessionStarted&) { - if (TargetControllerComponent::state == TargetControllerState::SUSPENDED) { - Logger::debug("Waking TargetController"); - - this->resume(); - this->fireTargetEvents(); - } - } - void TargetControllerComponent::onDebugSessionFinishedEvent(const DebugSessionFinished&) { + if (this->state != TargetControllerState::ACTIVE) { + return; + } + if (this->target->getState() != TargetState::RUNNING) { this->target->run(); this->fireTargetEvents(); } } - std::unique_ptr TargetControllerComponent::handleGetState(GetState& command) { - return std::make_unique(this->state); - } - - std::unique_ptr TargetControllerComponent::handleResume(Resume& command) { - if (this->state != TargetControllerState::ACTIVE) { - this->resume(); - } - - return std::make_unique(); - } - - std::unique_ptr TargetControllerComponent::handleSuspend(Suspend& command) { - if (this->state != TargetControllerState::SUSPENDED) { - this->suspend(); - } - - return std::make_unique(); - } - std::unique_ptr TargetControllerComponent::handleGetTargetDescriptor( GetTargetDescriptor& command ) { diff --git a/src/TargetController/TargetControllerComponent.hpp b/src/TargetController/TargetControllerComponent.hpp index 2ed6c2ae..d1d1964b 100644 --- a/src/TargetController/TargetControllerComponent.hpp +++ b/src/TargetController/TargetControllerComponent.hpp @@ -20,9 +20,6 @@ // Commands #include "Commands/Command.hpp" -#include "Commands/GetState.hpp" -#include "Commands/Resume.hpp" -#include "Commands/Suspend.hpp" #include "Commands/GetTargetDescriptor.hpp" #include "Commands/GetTargetState.hpp" #include "Commands/StopTargetExecution.hpp" @@ -46,7 +43,6 @@ // Responses #include "Responses/Response.hpp" -#include "Responses/State.hpp" #include "Responses/TargetDescriptor.hpp" #include "Responses/TargetState.hpp" #include "Responses/TargetRegistersRead.hpp" @@ -110,11 +106,7 @@ namespace Bloom::TargetController static inline ConditionVariableNotifier notifier = ConditionVariableNotifier(); static inline std::condition_variable responsesByCommandIdCv = std::condition_variable(); - /** - * The TC starts off in a suspended state. TargetControllerComponent::resume() is invoked from the start up - * routine. - */ - TargetControllerState state = TargetControllerState::SUSPENDED; + static inline std::atomic state = TargetControllerState::INACTIVE; ProjectConfig projectConfig; EnvironmentConfig environmentConfig; @@ -200,6 +192,13 @@ namespace Bloom::TargetController */ void startup(); + /** + * Exit point - must be called before the TargetController thread is terminated. + * + * Handles releasing the hardware among other clean-up related things. + */ + void shutdown(); + /** * Constructs a mapping of supported debug tool names to lambdas. The lambdas should *only* instantiate * and return an instance to the derived DebugTool class. They should not attempt to establish @@ -231,25 +230,6 @@ namespace Bloom::TargetController */ void registerCommandResponse(Commands::CommandIdType commandId, std::unique_ptr response); - /** - * Exit point - must be called before the TargetController thread is terminated. - * - * Handles releasing the hardware among other clean-up related things. - */ - void shutdown(); - - /** - * Puts the TargetController into the suspended state. - * - * In this state, the hardware is released and the TargetController will only handle a subset of events. - */ - void suspend(); - - /** - * Wakes the TargetController from the suspended state. - */ - void resume(); - /** * Establishes a connection with the debug tool and target. Prepares the hardware for a debug session. */ @@ -315,13 +295,6 @@ namespace Bloom::TargetController */ void onShutdownTargetControllerEvent(const Events::ShutdownTargetController& event); - /** - * Will hold the target stopped at it's current state. - * - * @param event - */ - void onDebugSessionStartedEvent(const Events::DebugSessionStarted& event); - /** * Will simply kick off execution on the target. * @@ -330,9 +303,6 @@ namespace Bloom::TargetController void onDebugSessionFinishedEvent(const Events::DebugSessionFinished& event); // Command handlers - std::unique_ptr handleGetState(Commands::GetState& command); - std::unique_ptr handleSuspend(Commands::Suspend& command); - std::unique_ptr handleResume(Commands::Resume& command); std::unique_ptr handleGetTargetDescriptor(Commands::GetTargetDescriptor& command); std::unique_ptr handleGetTargetState(Commands::GetTargetState& command); std::unique_ptr handleStopTargetExecution(Commands::StopTargetExecution& command); diff --git a/src/TargetController/TargetControllerState.hpp b/src/TargetController/TargetControllerState.hpp index cd3a1a9a..a1e1fb77 100644 --- a/src/TargetController/TargetControllerState.hpp +++ b/src/TargetController/TargetControllerState.hpp @@ -7,6 +7,6 @@ namespace Bloom::TargetController enum class TargetControllerState: std::uint8_t { ACTIVE, - SUSPENDED, + INACTIVE, }; }