Add lots of TODO comments

Sts1CobcSw/Periphery/FramRingArray.hpp

@@ -16,22 +16,31 @@
 
 namespace sts1cobcsw::fram
 {
+// TODO: Rename size to capacity
+// TODO: Maybe rename RingArray to RingVector
 template<typename T, std::size_t size, Address startAddress>
+// TODO: require serialSize<T> > 0, rename size to nElements
 class RingArray
 {
 public:
+    // TODO: Use serialSize<std::size_t> instead of sizeof
+    // TODO: buffersize_ and offset_ can be a static constexpr member
+    // TODO: Why is bufferSize_ size + 1U?
     RingArray() : bufferSize_(size + 1U), offset_(sizeof(std::size_t) * 2)
     {
+        // TODO: We can't initialize it here because the RingArray will be a global variable
         Initialize();
     };
 
     auto Push(T const & newData) -> void;
 
+    // TODO: All functions returning something should be marked as [[nodiscard]]
     auto operator[](std::size_t index) -> T;
 
     auto Front() -> T;
     auto Back() -> T;
 
+    // TODO: We don't need those useless comments
     //! @brief Returns the current size of the ring array
     auto Size() -> std::size_t;
 

Sts1CobcSw/Periphery/FramRingArray.ipp

@@ -6,13 +6,21 @@
 
 namespace sts1cobcsw::fram
 {
+// TODO: Use an SPI timeout > 0 everywhere. (with 6 MHz SPI clock, 750 bytes take 1 ms, so that
+// should be enough)
+// TODO: Drop the fram:: everywhere. We are already in the fram namespace
+// TODO: Rename this to PushBack(). It's not consistent with the ETL but with the STL, and a bit
+// more descriptive.
 template<typename T, std::size_t size, Address startAddress>
 void RingArray<T, size, startAddress>::Push(T const & newData)
 {
     auto const rawAddress = offset_ + value_of(startAddress) + (iEnd_ * serialSize<T>);
     fram::WriteTo(fram::Address(rawAddress), Span(Serialize(newData)), 0);
 
     ++iEnd_;
+    // TODO: Move this check to the beginning of the function because as it is right now iEnd_ is
+    // never equal to iBegin_ and we loose one element. If this is at the beginning, we can use the
+    // full capacity of the ring.
     if(iEnd_ == iBegin_)
     {
         iBegin_++;
@@ -25,6 +33,9 @@ void RingArray<T, size, startAddress>::Push(T const & newData)
 template<typename T, std::size_t size, Address startAddress>
 auto RingArray<T, size, startAddress>::Front() -> T
 {
+    // TODO: This function should just be `return *this[0]`;
+    // TODO: Put this into a function IndexToAddress(std::size_t index) or, ToAddress(), or
+    // AddressOfIndex(), or AddressOf(), ...
     auto const rawAddress = offset_ + value_of(startAddress) + (iBegin_ * serialSize<T>);
     auto readData = fram::ReadFrom<serialSize<T>>(fram::Address(rawAddress), 0);
     auto fromRing = Deserialize<T>(std::span(readData));
@@ -36,6 +47,8 @@ auto RingArray<T, size, startAddress>::Front() -> T
 template<typename T, std::size_t size, Address startAddress>
 auto RingArray<T, size, startAddress>::Back() -> T
 {
+    // TODO: Just make readIndex a cyclic_value, copy constructed from iEnd_ and decremented it
+    // instead of reinventing its logic here
     std::uint32_t readIndex = 0;
     if(iEnd_ == 0)
     {
@@ -57,6 +70,9 @@ auto RingArray<T, size, startAddress>::Back() -> T
 template<typename T, std::size_t size, Address startAddress>
 auto RingArray<T, size, startAddress>::operator[](std::size_t index) -> T
 {
+    // TODO: Again, copy iBegin_ and use .advance(index) on it
+    // TODO: Check if index is in bounds, i.e., index < Size(). If the index is out of bounds, we
+    // should DEBUG_PRINT() and do something predictable like returning the last element.
     auto const rawAddress =
         offset_ + value_of(startAddress) + ((iBegin_ + index) % bufferSize_) * serialSize<T>;
     auto readData = fram::ReadFrom<serialSize<T>>(fram::Address(rawAddress), 0);
@@ -73,25 +89,31 @@ auto RingArray<T, size, startAddress>::Size() -> std::size_t
     {
         return (iEnd_ - iBegin_);
     }
+    // TODO: If iEnd_ < iBegin_, the ring is full and (iBegin_ - iEnd_) is always 0
     return (bufferSize_ - (iBegin_ - iEnd_));
 }
 
 template<typename T, std::size_t size, Address startAddress>
 auto RingArray<T, size, startAddress>::Capacity() -> std::size_t
 {
+    // TODO: This function should be a static constexpr member and return size (= capacity after
+    // renaming)
     return (bufferSize_ - 1U);
 }
 
 template<typename T, std::size_t size, Address startAddress>
 auto RingArray<T, size, startAddress>::Initialize() -> void
 {
+    // TODO: We should always read the indexes from the FRAM, not just when calling Initialize()
     ReadIndices();
 }
 
 template<typename T, std::size_t size, Address startAddress>
 auto RingArray<T, size, startAddress>::WriteIndices() -> void
 {
+    // TODO: beginAddress and endAddress can be static constexpr members
     auto beginAddress = value_of(startAddress);
+    // TODO: Use serialSize<std::size_t> instead of sizeof
     auto endAddress = beginAddress + sizeof(std::size_t);
 
     fram::WriteTo(fram::Address(beginAddress), Span(Serialize(iBegin_.get())), 0);
@@ -101,7 +123,9 @@ auto RingArray<T, size, startAddress>::WriteIndices() -> void
 template<typename T, std::size_t size, Address startAddress>
 auto RingArray<T, size, startAddress>::ReadIndices() -> void
 {
+    // TODO: beginAddress and endAddress can be static constexpr members
     auto beginAddress = value_of(startAddress);
+    // TODO: Use serialSize<std::size_t> instead of sizeof
     auto endAddress = beginAddress + sizeof(std::size_t);
 
     auto beginData = fram::ReadFrom<sizeof(std::size_t)>(fram::Address(beginAddress), 0);