Tidying

src/EventManager/EventListener.hpp

@@ -53,6 +53,9 @@ namespace Bloom
 
         /**
          * Each event listener is supplied an ID upon registering with the EventManager.
+         *
+         * @TODO: It might be a better idea to use an std::atomic to generate this auto-incremented ID. Like we do
+         *        with events.
          */
         size_t id = 0;
 

src/EventManager/EventManager.cpp

@@ -19,7 +19,7 @@ void EventManager::triggerEvent(std::shared_ptr<const Events::Event> event) {
     auto registerListenersLock = std::unique_lock(this->registerListenerMutex);
     for(auto const& [listenerId, listener] : this->registeredListeners) {
         auto registeredEventTypes = listener->getRegisteredEventTypeNames();
-        if (registeredEventTypes.find(event->getName()) != registeredEventTypes.end()) {
+        if (registeredEventTypes.contains(event->getName())) {
             listener->registerEvent(event);
         }
     }

src/EventManager/EventManager.hpp

@@ -1,13 +1,22 @@
 #pragma once
 
-#include <string>
 #include <map>
 #include <mutex>
+
 #include "Events/Events.hpp"
 #include "EventListener.hpp"
 
 namespace Bloom
 {
+    /**
+     * The EventManager class provides a method of dispatching events to a set of listeners.
+     * A single instance of this class is created in Application class. That instance is then passed by references to
+     * all other components in Bloom, that require the ability to trigger events.
+     *
+     * @TODO: Should this be a static class? As in, all methods and variables declared static. We seem to be
+     *        using it in that way. It would save us from having to pass around that single instance by reference.
+     *        Something to consider.
+     */
     class EventManager
     {
     private:
@@ -19,13 +28,31 @@ namespace Bloom
 
     public:
         /**
-         * Generates a new registered listener.
+         * Registers an EventListener instance with this manager.
+         *
+         * All EventListener instances must be registered with the EventManager before any events can
+         * be dispatched to them.
+         *
+         * The EventManager possesses partial ownership of the EventListener. This is why we use a shared_ptr here.
          *
          * @param listenerName
          */
         void registerListener(std::shared_ptr<EventListener> listener);
+
+        /**
+         * Deregister an EventListener instance.
+         *
+         * @param listenerId
+         *  The ID of the EventListener to deregister. See EventListener::getId();
+         */
         void deregisterListener(size_t listenerId);
 
+        /**
+         * Dispatches an event to all registered listeners, if they have registered an interest in the event type.
+         * See EventListener::registeredEventTypes for more.
+         *
+         * @param event
+         */
         void triggerEvent(GenericEventPointer event);
     };
 

src/TargetController/TargetController.hpp

@@ -27,7 +27,7 @@ namespace Bloom
     /**
      * The TargetController possesses full control of the debugging target and the debug tool.
      *
-`    * The TargetController runs on a dedicated thread. Its sole purpose is to handle communication to  &from the
+`    * The TargetController runs on a dedicated thread. Its sole purpose is to handle communication to & from the
      * debug tool and target.
      *
      * The TargetController should be oblivious to any manufacture/device specific functionality. It should
@@ -73,19 +73,19 @@ namespace Bloom
             return std::map<std::string, std::function<std::unique_ptr<DebugTool>()>> {
                 {
                     "atmel-ice",
-                    []() -> std::unique_ptr<DebugTool> {
+                    []() {
                         return std::make_unique<AtmelIce>();
                     }
                 },
                 {
                     "power-debugger",
-                    []() -> std::unique_ptr<DebugTool> {
+                    []() {
                         return std::make_unique<PowerDebugger>();
                     }
                 },
                 {
                     "snap",
-                    []() -> std::unique_ptr<DebugTool> {
+                    []() {
                         return std::make_unique<MplabSnap>();
                     }
                 },
@@ -102,7 +102,7 @@ namespace Bloom
             auto mapping = std::map<std::string, std::function<std::unique_ptr<Targets::Target>()>> {
                 {
                     "avr8",
-                    []() -> std::unique_ptr<Targets::Target> {
+                    []() {
                         return std::make_unique<Avr8>();
                     }
                 },
@@ -119,10 +119,10 @@ namespace Bloom
                     auto targetName = targetIt->toObject().find("targetName").value().toString()
                         .toLower().toStdString();
 
-                    if (mapping.find(targetName) == mapping.end()) {
+                    if (!mapping.contains(targetName)) {
                         mapping.insert({
                            targetName,
-                           [targetName]() -> std::unique_ptr<Targets::Target> {
+                           [targetName]() {
                                return std::make_unique<Avr8>(targetName);
                            }
                        });

src/Targets/Microchip/AVR/AVR8/Avr8.hpp

@@ -37,21 +37,62 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit
         std::map<std::string, PadDescriptor> padDescriptorsByName;
         std::map<int, TargetVariant> targetVariantsById;
 
+        /**
+         * Extracts the ID from the target's memory.
+         *
+         * This function will cache the ID value and use the cached version for any subsequent calls.
+         *
+         * @return
+         */
+        TargetSignature getId() override;
+
+        /**
+         * Extracts the AVR8 target parameters from the loaded part description file.
+         *
+         * @return
+         */
         virtual TargetParameters& getTargetParameters();
 
+        /**
+         * Generates a collection of PadDescriptor object from data in the loaded part description file and
+         * populates this->padDescriptorsByName.
+         */
         virtual void loadPadDescriptors();
+
+        /**
+         * Extracts target variant information from the loaded part description file and generates a collection
+         * of TargetVariant objects.
+         *
+         * @return
+         */
+        virtual std::vector<TargetVariant> generateVariantsFromPartDescription();
+
+        /**
+         * Populates this->targetVariantsById using this->generateVariantsFromPartDescription() and data from
+         * this->padDescriptorsByName.
+         */
         virtual void loadTargetVariants();
 
     public:
         explicit Avr8() = default;
         Avr8(const std::string& name): name(name) {};
 
-        [[nodiscard]] std::string getName() const override {
+        /*
-            return this->name;
+         * The functions below implement the Target interface for AVR8 targets.
-        }
+         *
+         * See the Bloom::Targets::Target interface class for documentation on the expected behaviour of
+         * each function.
+         */
+
+        void preActivationConfigure(const TargetConfig& targetConfig) override;
+        void postActivationConfigure() override;
+        virtual void postPromotionConfigure() override;
+
+        void activate() override;
+        void deactivate() override;
 
         /**
-         * Checks if DebugTool is compatible with AVR8 targets.
+         * All AVR8 compatible debug tools must provide a valid Avr8Interface.
          *
          * @param debugTool
          * @return
@@ -64,31 +105,8 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit
             this->avr8Interface = debugTool->getAvr8Interface();
         };
 
-        void preActivationConfigure(const TargetConfig& targetConfig) override;
-        void postActivationConfigure() override;
-
-        virtual void postPromotionConfigure() override;
-
-        void activate() override;
-
-        void deactivate() override;
-
-        TargetSignature getId() override;
-
-        void run() override;
-
-        void stop() override;
-
-        void step() override;
-
-        void reset() override;
-
-        void setBreakpoint(std::uint32_t address) override;
-        void removeBreakpoint(std::uint32_t address) override;
-        void clearAllBreakpoints() override;
-
         /**
-         * AVR8 targets are promotable. See promote() method for more.
+         * Instances to this target class can be promoted. See Avr8::promote() method for more.
          *
          * @return
          */
@@ -96,25 +114,52 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit
             return true;
         }
 
-        virtual std::vector<TargetVariant> generateVariantsFromPartDescription();
+        /**
+         * Instances of this generic Avr8 target class will be promoted to a family specific class (see the Mega, Xmega
+         * and Tiny classes for more).
+         *
+         * @return
+         */
+        virtual std::unique_ptr<Targets::Target> promote() override;
+
+        std::string getName() const override {
+            return this->name;
+        }
 
         virtual TargetDescriptor getDescriptor() override;
 
-        virtual std::unique_ptr<Targets::Target> promote() override;
+        void run() override;
+        void stop() override;
+        void step() override;
+        void reset() override;
+
+        void setBreakpoint(std::uint32_t address) override;
+        void removeBreakpoint(std::uint32_t address) override;
+        void clearAllBreakpoints() override;
 
         virtual TargetRegisters readGeneralPurposeRegisters(std::set<std::size_t> registerIds) override;
         virtual void writeRegisters(const TargetRegisters& registers) override;
         virtual TargetRegisters readRegisters(const TargetRegisterDescriptors& descriptors) override;
-        virtual TargetMemoryBuffer readMemory(TargetMemoryType memoryType, std::uint32_t startAddress, std::uint32_t bytes) override;
+
-        virtual void writeMemory(TargetMemoryType memoryType, std::uint32_t startAddress, const TargetMemoryBuffer& buffer) override;
+        virtual TargetMemoryBuffer readMemory(
+            TargetMemoryType memoryType,
+            std::uint32_t startAddress,
+            std::uint32_t bytes
+        ) override;
+        virtual void writeMemory(
+            TargetMemoryType memoryType,
+            std::uint32_t startAddress,
+            const TargetMemoryBuffer& buffer
+        ) override;
 
         virtual TargetState getState() override;
 
         virtual std::uint32_t getProgramCounter() override;
         virtual TargetRegister getProgramCounterRegister() override;
+        virtual void setProgramCounter(std::uint32_t programCounter) override;
+
         virtual TargetRegister getStackPointerRegister() override;
         virtual TargetRegister getStatusRegister() override;
-        virtual void setProgramCounter(std::uint32_t programCounter) override;
 
         virtual std::map<int, TargetPinState> getPinStates(int variantId) override;
         virtual void setPinState(

src/Targets/Target.hpp

@@ -30,14 +30,14 @@ namespace Bloom::Targets
     class Target
     {
     protected:
-        bool activated = false;
-
         /**
          * Target related configuration provided by the user. This is passed in via the first stage of target
          * configuration. See Target::preActivationConfigure() for more.
          */
         TargetConfig config;
 
+        bool activated = false;
+
     public:
         explicit Target() {}
 
@@ -54,15 +54,15 @@ namespace Bloom::Targets
          * that is required in order for us to successfully activate the target. For an example, we use this method in
          * the Avr8 target class to configure the debug tool with the correct physical interface and config variant
          * parameters (taken from the user's settings, via the TargetConfig instance). Without these being configured,
-         * the debug tool would not be able to interface with the target, and thus target activation would fail.
+         * the debug tool would not be able to interface with the AVR8 target, and thus target activation would fail.
          *
          * postActivationConfigure() - The second stage is right after target activation (successful invocation of
          * Target::activate()). At this point, we will have established a connection with the target and so interaction
          * with the target is permitted here. We use this method in the Avr8 target class to extract the target signature
          * from the target's memory, which we then use to find & load the correct part description file.
          *
-         * postPromotionConfigure() - The final stage of configuration occurs just after the target has been promoted
+         * postPromotionConfigure() - The final stage of configuration occurs just after the target instance has been
-         * to its final form. See the Target::promote() method for more in this.
+         * promoted to a different class. See the Target::promote() method for more in this.
          *
          * If any of the three configuration methods throw an exception, the exception will be treated as a fatal error.
          * In response, the TargetController will shutdown, along with the rest of Bloom.
@@ -98,13 +98,15 @@ namespace Bloom::Targets
 
         /**
          * Should check if the given debugTool is compatible with the target. Returning false in this method will
-         * prevent Bloom from attempting to use the selected debug tool with the selected target.
+         * prevent Bloom from attempting to use the selected debug tool with the selected target. An InvalidConfig
+         * exception will be raised and Bloom will shutdown.
          *
          * For AVR8 targets, we simply check if the debug tool returns a valid Avr8Interface
          * (via DebugTool::getAvr8Interface()). If it fails to do so, it would mean that the debug tool, or more so our
          * debug tool driver, does not support AVR8 targets.
          *
          * @param debugTool
+         *
          * @return
          */
         virtual bool isDebugToolSupported(DebugTool* debugTool) = 0;
@@ -158,6 +160,9 @@ namespace Bloom::Targets
          *   - The call to Target::promote() on the current target instance returns a target class type that is equal
          *     to the type of the current target instance (promotion failed).
          *
+         * Once at least one of the above conditions are met, the TargetController will break out of the loop and use
+         * the last promoted target instance from there onwards.
+         *
          * See TargetController::startup() for more on this.
          *
          * @return
@@ -220,9 +225,10 @@ namespace Bloom::Targets
         virtual void clearAllBreakpoints() = 0;
 
         /**
-         * Should read general purpose register values, for the given general prupose register ids.
+         * Should read general purpose register values, for the given general purpose register ids.
          *
          * @param registerIds
+         *
          * @return
          */
         virtual TargetRegisters readGeneralPurposeRegisters(std::set<std::size_t> registerIds) = 0;
@@ -238,6 +244,7 @@ namespace Bloom::Targets
          * Should read register values of the registers described by the given descriptors.
          *
          * @param descriptors
+         *
          * @return
          */
         virtual TargetRegisters readRegisters(const TargetRegisterDescriptors& descriptors) = 0;
@@ -248,6 +255,7 @@ namespace Bloom::Targets
          * @param memoryType
          * @param startAddress
          * @param bytes
+         *
          * @return
          */
         virtual TargetMemoryBuffer readMemory(TargetMemoryType memoryType, std::uint32_t startAddress, std::uint32_t bytes) = 0;
@@ -282,6 +290,13 @@ namespace Bloom::Targets
          */
         virtual TargetRegister getProgramCounterRegister() = 0;
 
+        /**
+         * Should update the program counter on the target.
+         *
+         * @param programCounter
+         */
+        virtual void setProgramCounter(std::uint32_t programCounter) = 0;
+
         /**
          * Should fetch the status register value.
          *
@@ -296,17 +311,11 @@ namespace Bloom::Targets
          */
         virtual TargetRegister getStackPointerRegister() = 0;
 
-        /**
-         * Should update the program counter on the target.
-         *
-         * @param programCounter
-         */
-        virtual void setProgramCounter(std::uint32_t programCounter) = 0;
-
         /**
          * Should get the current pin states for each pin on the target, mapped by pin number
          *
          * @param variantId
+         *
          * @return
          */
         virtual std::map<int, TargetPinState> getPinStates(int variantId) = 0;