Spelling, comments, formating, and todos

Os/Generic/DefaultPriorityQueue.cpp

@@ -1,13 +1,14 @@
 // ======================================================================
 // \title Os/Posix/DefaultFile.cpp
-// \brief sets default Os::File to posix implementation via linker
+// \brief sets default Os::Queue to generic priority queue implementation via linker
 // ======================================================================
-#include "Os/Queue.hpp"
-#include "Os/Generic/PriorityQueue.hpp"
 #include "Os/Delegate.hpp"
+#include "Os/Generic/PriorityQueue.hpp"
+#include "Os/Queue.hpp"
 
 namespace Os {
 QueueInterface* QueueInterface::getDelegate(QueueHandleStorage& aligned_new_memory) {
-    return Os::Delegate::makeDelegate<QueueInterface, Os::Generic::PriorityQueue, QueueHandleStorage>(aligned_new_memory);
-}
+    return Os::Delegate::makeDelegate<QueueInterface, Os::Generic::PriorityQueue, QueueHandleStorage>(
+        aligned_new_memory);
 }
+}  // namespace Os

Os/Generic/PriorityQueue.cpp

@@ -1,42 +1,41 @@
-//
-// Created by Michael Starch on 9/17/24.
-//
+// ======================================================================
+// \title Os/Generic/PriorityQueue.cpp
+// \brief priority queue implementation for Os::Queue
+// ======================================================================
 
 #include "PriorityQueue.hpp"
 #include <Fw/Types/Assert.hpp>
 #include <cstring>
-#include <new>
 #include <mutex>
+#include <new>
 
 namespace Os {
 namespace Generic {
 
 FwSizeType PriorityQueueHandle ::find_index() {
+    // This function is predicated on the definition of unsigned "overflow"
+    static_assert(not std::numeric_limits<FwSizeType>::is_signed, "FwSizeType is presumed to be unsigned");
+    // This function will never be called when full, this assert ensures that the start index never surpasses the end
+    // index.
+    FW_ASSERT(this->m_startIndex != this->m_stopIndex);
     FwSizeType index = this->m_indices[this->m_startIndex % this->m_depth];
-    //TODO: handle overflow
+    // Overflow allowed here, as computations handle wrap-around when assuming unsigned integers
     this->m_startIndex++;
     FwSizeType diff = this->m_stopIndex - this->m_startIndex;
-    FW_ASSERT(
-        diff <= this->m_depth,
-        static_cast<FwAssertArgType>(diff),
-        static_cast<FwAssertArgType>(this->m_depth),
-        static_cast<FwAssertArgType>(this->m_stopIndex),
-        static_cast<FwAssertArgType>(this->m_startIndex));
+    FW_ASSERT(diff <= this->m_depth, static_cast<FwAssertArgType>(diff), static_cast<FwAssertArgType>(this->m_depth),
+              static_cast<FwAssertArgType>(this->m_stopIndex), static_cast<FwAssertArgType>(this->m_startIndex));
     return index;
 }
 
 void PriorityQueueHandle ::return_index(FwSizeType index) {
+    // This function is predicated on the definition of unsigned "overflow"
+    static_assert(not std::numeric_limits<FwSizeType>::is_signed, "FwSizeType is presumed to be unsigned");
     this->m_indices[this->m_stopIndex % this->m_depth] = index;
-    //TODO: handle overflow
+    // Overflow allowed here, as computations handle wrap-around when assuming unsigned integers
     this->m_stopIndex++;
     FwSizeType diff = this->m_stopIndex - this->m_startIndex;
-    FW_ASSERT(
-        diff <= this->m_depth,
-        static_cast<FwAssertArgType>(diff),
-        static_cast<FwAssertArgType>(this->m_depth),
-        static_cast<FwAssertArgType>(this->m_stopIndex),
-        static_cast<FwAssertArgType>(this->m_startIndex)
-    );
+    FW_ASSERT(diff <= this->m_depth, static_cast<FwAssertArgType>(diff), static_cast<FwAssertArgType>(this->m_depth),
+              static_cast<FwAssertArgType>(this->m_stopIndex), static_cast<FwAssertArgType>(this->m_startIndex));
 }
 
 void PriorityQueueHandle ::store_data(FwSizeType index, const U8* data, FwSizeType size) {
@@ -61,57 +60,60 @@ PriorityQueue::~PriorityQueue() {
     delete[] this->m_handle.m_sizes;
 }
 
-
-
 QueueInterface::Status PriorityQueue::create(const Fw::StringBase& name, FwSizeType depth, FwSizeType messageSize) {
-
-    //TODO: point this allocation to Os::Allocate implementation
+    // Allocate indices list
     FwSizeType* indices = new (std::nothrow) FwSizeType[depth];
     if (indices == nullptr) {
         return QueueInterface::Status::UNKNOWN_ERROR;
     }
+    // Allocate sizes list or clean-up
     FwSizeType* sizes = new (std::nothrow) FwSizeType[depth];
     if (sizes == nullptr) {
         delete[] indices;
         return QueueInterface::Status::UNKNOWN_ERROR;
     }
+    // Allocate sizes list or clean-up
     U8* data = new (std::nothrow) U8[depth * messageSize];
     if (data == nullptr) {
         delete[] indices;
         delete[] sizes;
         return QueueInterface::Status::UNKNOWN_ERROR;
     }
+    // Allocate max heap or clean-up
     bool created = this->m_handle.m_heap.create(depth);
     if (not created) {
         delete[] indices;
         delete[] sizes;
         delete[] data;
         return QueueInterface::Status::UNKNOWN_ERROR;
     }
-    // Assign initial indices
+    // Assign initial indices and sizes
     for (FwSizeType i = 0; i < depth; i++) {
         indices[i] = i;
         sizes[i] = 0;
     }
-
+    // Set local tracking variables
     this->m_handle.m_maxSize = messageSize;
     this->m_handle.m_indices = indices;
     this->m_handle.m_data = data;
     this->m_handle.m_sizes = sizes;
     this->m_handle.m_startIndex = 0;
     this->m_handle.m_stopIndex = depth;
     this->m_handle.m_depth = depth;
+    this->m_handle.m_highMark = 0;
 
     return QueueInterface::Status::OP_OK;
 }
 
 QueueInterface::Status PriorityQueue::send(const U8* buffer,
-                                       FwSizeType size,
-                                       FwQueuePriorityType priority,
-                                       QueueInterface::BlockingType blockType) {
+                                           FwSizeType size,
+                                           FwQueuePriorityType priority,
+                                           QueueInterface::BlockingType blockType) {
+    // Check for sizing problem before locking
     if (size > this->m_handle.m_maxSize) {
         return QueueInterface::Status::SIZE_MISMATCH;
     }
+    // Artificial block scope for scope lock ensuring an unlock in all cases and ensuring an unlock before notify
     {
         Os::ScopeLock lock(this->m_handle.m_data_lock);
         if (this->m_handle.m_heap.isFull() and blockType == BlockingType::NONBLOCKING) {
@@ -129,17 +131,17 @@ QueueInterface::Status PriorityQueue::send(const U8* buffer,
         }
         this->m_handle.store_data(index, buffer, size);
         this->m_handle.m_sizes[index] = size;
-        this->m_handle.m_hwm = FW_MAX(this->m_handle.m_hwm, this->getMessagesAvailable());
+        this->m_handle.m_highMark = FW_MAX(this->m_handle.m_highMark, this->getMessagesAvailable());
     }
     this->m_handle.m_empty.notify();
     return QueueInterface::Status::OP_OK;
 }
 
 QueueInterface::Status PriorityQueue::receive(U8* destination,
-                                          FwSizeType capacity,
-                                          QueueInterface::BlockingType blockType,
-                                          FwSizeType& actualSize,
-                                          FwQueuePriorityType& priority) {
+                                              FwSizeType capacity,
+                                              QueueInterface::BlockingType blockType,
+                                              FwSizeType& actualSize,
+                                              FwQueuePriorityType& priority) {
     {
         Os::ScopeLock lock(this->m_handle.m_data_lock);
         if (this->m_handle.m_heap.isEmpty() and blockType == BlockingType::NONBLOCKING) {
@@ -176,7 +178,9 @@ FwSizeType PriorityQueue::getMessagesAvailable() const {
 }
 
 FwSizeType PriorityQueue::getMessageHighWaterMark() const {
-    return this->m_handle.m_hwm;
-}
+    // Safe to cast away const in this context because scope lock will restore unlocked state on return
+    Os::ScopeLock lock(const_cast<Mutex&>(this->m_handle.m_data_lock));
+    return this->m_handle.m_highMark;
 }
+}  // namespace Generic
 }  // namespace Os

Os/Generic/PriorityQueue.hpp

@@ -1,43 +1,56 @@
-//
-// Created by Michael Starch on 9/17/24.
-//
-#include "Os/Queue.hpp"
-#include "Os/Mutex.hpp"
+// ======================================================================
+// \title Os/Generic/PriorityQueue.hpp
+// \brief priority queue implementation definitions for Os::Queue
+// ======================================================================
 #include "Os/Condition.hpp"
 #include "Os/Generic/Types/MaxHeap.hpp"
+#include "Os/Mutex.hpp"
+#include "Os/Queue.hpp"
 #ifndef OS_GENERIC_PRIORITYQUEUE_HPP
 #define OS_GENERIC_PRIORITYQUEUE_HPP
 
 namespace Os {
 namespace Generic {
 
-struct Metadata {
-
-};
-
+//! \brief critical data stored for priority queue
+//!
+//! The priority queue has two essential data structures: a block of unordered memory storing message data and size. The
+//! queue also stores a circular list of indices into that memory tracking which slots are free and which are taken.
+//! These indices are ordered by a max heap data structure projecting priority on to the otherwise unordered data. Both
+//! the data region and index list have queue depth number of entries.
 struct PriorityQueueHandle : public QueueHandle {
-    Types::MaxHeap m_heap;
-    U8* m_data = nullptr;
-    FwSizeType* m_indices = nullptr;
-    FwSizeType* m_sizes = nullptr;
-    FwSizeType m_depth = 0;
-    FwSizeType m_startIndex = 0;
-    FwSizeType m_stopIndex = 0;
-    FwSizeType m_maxSize = 0;
-    FwSizeType m_hwm = 0;
-    Os::Mutex m_data_lock;
-    Os::ConditionVariable m_full;
-    Os::ConditionVariable m_empty;
-
+    Types::MaxHeap m_heap;            //!< MaxHeap data store for tracking priority
+    U8* m_data = nullptr;             //!< Pointer to data allocation
+    FwSizeType* m_indices = nullptr;  //!< List of indices into data
+    FwSizeType* m_sizes = nullptr;    //!< Size store for each method
+    FwSizeType m_depth = 0;           //!< Depth of the queue
+    FwSizeType m_startIndex = 0;      //!< Start index of the circular data structure
+    FwSizeType m_stopIndex = 0;       //!< End index of the circular data structure
+    FwSizeType m_maxSize = 0;         //!< Maximum size allowed of a message
+    FwSizeType m_highMark = 0;        //!< Message count high water mark
+    Os::Mutex m_data_lock;            //!< Lock against data manipulation
+    Os::ConditionVariable m_full;     //!< Queue full condition variable to support blocking
+    Os::ConditionVariable m_empty;    //!< Queue empty condition variable to support blocking
+
+    //!\brief find an available index to store data from the list
     FwSizeType find_index();
 
+    //!\brief return index to the circular data structure
+    //!\param index: index to return to the list
     void return_index(FwSizeType index);
 
+    //!\brief store data into a set index in the data store
     void store_data(FwSizeType index, const U8* source, FwSizeType size);
 
+    //!\brief load data from a set index in the data store
     void load_data(FwSizeType index, U8* destination, FwSizeType capacity);
 };
-
+//! \brief generic priority queue implementation
+//!
+//! A generic implementation of a priority queue to support the Os::QueueInterface. This queue uses OSAL mutexes,
+//! and condition variables to provide for a task-safe blocking queue implementation. Data is stored in heap memory.
+//!
+//! \warning allocates memory on the heap
 class PriorityQueue : public Os::QueueInterface {
   public:
     //! \brief default queue interface constructor
@@ -58,10 +71,13 @@ class PriorityQueue : public Os::QueueInterface {
     //! \brief create queue storage
     //!
     //! Creates a queue ensuring sufficient storage to hold `depth` messages of `messageSize` size each.
+    //!
+    //! \warning allocates memory on the heap
+    //!
     //! \param name: name of queue
     //! \param depth: depth of queue in number of messages
     //! \param messageSize: size of an individual message
-    //! \return: stauts of the creation
+    //! \return: status of the creation
     Status create(const Fw::StringBase& name, FwSizeType depth, FwSizeType messageSize) override;
 
     //! \brief send a message into the queue
@@ -70,6 +86,9 @@ class PriorityQueue : public Os::QueueInterface {
     //! `blockType` is set to BLOCKING, this call will block on queue full. Otherwise, this will return an error
     //! status on queue full.
     //!
+    //! \warning It is invalid to send a null buffer
+    //! \warning This method will block if the queue is full and blockType is set to BLOCKING
+    //!
     //! \param buffer: message data
     //! \param size: size of message data
     //! \param priority: priority of the message
@@ -83,6 +102,9 @@ class PriorityQueue : public Os::QueueInterface {
     //! When `blockType` is set to BLOCKING, this call will block on queue empty. Otherwise, this will return an
     //! error status on queue empty. Actual size received and priority of message is set on success status.
     //!
+    //! \warning It is invalid to send a null buffer
+    //! \warning This method will block if the queue is full and blockType is set to BLOCKING
+    //!
     //! \param destination: destination for message data
     //! \param capacity: maximum size of message data
     //! \param blockType: BLOCKING to wait for message or NONBLOCKING to return error when queue is empty