Revise encoders to use shared buffer and handle terminator encoding s…

…eperately

include/Common.h

@@ -3,12 +3,12 @@
 #include <cstdint>
 #include <string_view>
 
-#define DISABLE_COPY(TypeName)             \
-  TypeName(const TypeName&) = delete;      \
-  void operator=(const TypeName&) = delete
-#define DISABLE_MOVE(TypeName)             \
-  TypeName(TypeName&&) = delete;           \
-  void operator=(TypeName&&) = delete
+#define DISABLE_COPY(TypeName)                   \
+  TypeName(const TypeName&)            = delete; \
+  TypeName& operator=(const TypeName&) = delete
+#define DISABLE_MOVE(TypeName)              \
+  TypeName(TypeName&&)            = delete; \
+  TypeName& operator=(TypeName&&) = delete
 
 #ifndef OPENSHOCK_API_DOMAIN
 #error "OPENSHOCK_API_DOMAIN must be defined"

include/radio/rmt/CaiXianlinEncoder.h

@@ -1,12 +1,10 @@
 #pragma once
 
+#include "RmtSequence.h"
 #include "ShockerCommandType.h"
 
-#include <esp32-hal-rmt.h>
-
 #include <cstdint>
-#include <vector>
 
 namespace OpenShock::Rmt::CaiXianlinEncoder {
-  std::vector<rmt_data_t> GetSequence(uint16_t transmitterId, uint8_t channelId, OpenShock::ShockerCommandType type, uint8_t intensity);
+  RmtSequence GetSequence(uint16_t shockerId, uint8_t channelId, OpenShock::ShockerCommandType type, uint8_t intensity);
 }

include/radio/rmt/MainEncoder.h

@@ -1,17 +1,11 @@
 #pragma once
 
+#include "RmtSequence.h"
 #include "ShockerCommandType.h"
 #include "ShockerModelType.h"
 
-#include <esp32-hal-rmt.h>
-
 #include <cstdint>
-#include <memory>
-#include <vector>
 
 namespace OpenShock::Rmt {
-  std::vector<rmt_data_t> GetSequence(ShockerModelType model, uint16_t shockerId, OpenShock::ShockerCommandType type, uint8_t intensity);
-  inline std::vector<rmt_data_t> GetZeroSequence(ShockerModelType model, uint16_t shockerId) {
-    return GetSequence(model, shockerId, ShockerCommandType::Vibrate, 0);
-  }
-}
+  RmtSequence GetSequence(ShockerModelType model, uint16_t shockerId, OpenShock::ShockerCommandType type, uint8_t intensity);
+}  // namespace OpenShock::Rmt

include/radio/rmt/Petrainer998DREncoder.h

@@ -1,12 +1,10 @@
 #pragma once
 
+#include "RmtSequence.h"
 #include "ShockerCommandType.h"
 
-#include <esp32-hal-rmt.h>
-
 #include <cstdint>
-#include <vector>
 
 namespace OpenShock::Rmt::Petrainer998DREncoder {
-  std::vector<rmt_data_t> GetSequence(uint16_t shockerId, OpenShock::ShockerCommandType type, uint8_t intensity);
+  RmtSequence GetSequence(uint16_t shockerId, OpenShock::ShockerCommandType type, uint8_t intensity);
 }

include/radio/rmt/PetrainerEncoder.h

@@ -1,12 +1,10 @@
 #pragma once
 
+#include "RmtSequence.h"
 #include "ShockerCommandType.h"
 
-#include <esp32-hal-rmt.h>
-
 #include <cstdint>
-#include <vector>
 
 namespace OpenShock::Rmt::PetrainerEncoder {
-  std::vector<rmt_data_t> GetSequence(uint16_t shockerId, OpenShock::ShockerCommandType type, uint8_t intensity);
+  RmtSequence GetSequence(uint16_t shockerId, OpenShock::ShockerCommandType type, uint8_t intensity);
 }

include/radio/rmt/RmtSequence.h

@@ -0,0 +1,58 @@
+#pragma once
+
+#include "Common.h"
+
+#include <esp32-hal-rmt.h>
+
+#include <cstdint>
+
+namespace OpenShock::Rmt {
+  class RmtSequence {
+    DISABLE_COPY(RmtSequence);
+
+  public:
+    RmtSequence()
+      : m_data(nullptr)
+      , m_size(0)
+    {
+    }
+    RmtSequence(size_t size)
+      : m_data(reinterpret_cast<rmt_data_t*>(malloc(size * 2 * sizeof(rmt_data_t))))
+      , m_size(size)
+    {
+    }
+    RmtSequence(RmtSequence&& other) noexcept
+      : m_data(other.m_data)
+      , m_size(other.m_size)
+    {
+      other.m_data = nullptr;
+      other.m_size = 0;
+    }
+    ~RmtSequence() { free(m_data); }
+
+    constexpr bool is_valid() const noexcept { return m_data != nullptr && m_size != 0; }
+
+    constexpr rmt_data_t* payload() noexcept { return m_data; }
+    constexpr rmt_data_t* terminator() noexcept { return m_data + m_size; }
+    constexpr size_t size() const noexcept { return m_size; }
+
+    RmtSequence& operator=(RmtSequence&& other)
+    {
+      if (this == &other) return *this;
+
+      free(m_data);
+
+      m_data = other.m_data;
+      m_size = other.m_size;
+
+      other.m_data = nullptr;
+      other.m_size = 0;
+
+      return *this;
+    }
+
+  private:
+    rmt_data_t* m_data;
+    size_t m_size;
+  };
+}  // namespace OpenShock::Rmt

include/radio/rmt/T330Encoder.h

@@ -1,12 +1,10 @@
 #pragma once
 
+#include "RmtSequence.h"
 #include "ShockerCommandType.h"
 
-#include <esp32-hal-rmt.h>
-
 #include <cstdint>
-#include <vector>
 
 namespace OpenShock::Rmt::T330Encoder {
-  std::vector<rmt_data_t> GetSequence(uint16_t transmitterId, OpenShock::ShockerCommandType type, uint8_t intensity);
+  RmtSequence GetSequence(uint16_t shockerId, OpenShock::ShockerCommandType type, uint8_t intensity);
 }

include/radio/rmt/internal/Shared.h

@@ -5,18 +5,18 @@
 #include <cstdint>
 #include <limits>
 #include <type_traits>
-#include <vector>
 
 namespace OpenShock::Rmt::Internal {
-  template<std::size_t N, typename T>
-  inline void EncodeBits(std::vector<rmt_data_t>& pulses, T data, const rmt_data_t& rmtOne, const rmt_data_t& rmtZero) {
+  template<size_t N, typename T>
+  constexpr void EncodeBits(rmt_data_t* sequence, T data, const rmt_data_t& rmtOne, const rmt_data_t& rmtZero)
+  {
     static_assert(std::is_unsigned<T>::value, "T must be an unsigned integer");
     static_assert(N > 0, "N must be greater than 0");
     static_assert(N < std::numeric_limits<T>::digits, "N must be less or equal to the number of bits in T");
 
-    pulses.reserve(pulses.size() + N);
-    for (int64_t bit_pos = N - 1; bit_pos >= 0; --bit_pos) {
-      pulses.push_back((data >> bit_pos) & 1 ? rmtOne : rmtZero);
+    for (size_t i = 0; i < N; ++i) {
+      size_t bit_pos = N - (i + 1);
+      sequence[i]    = (data >> bit_pos) & 1 ? rmtOne : rmtZero;
     }
   }
-}
+}  // namespace OpenShock::Rmt::Internal

src/radio/RFTransmitter.cpp

@@ -5,9 +5,9 @@
 const char* const TAG = "RFTransmitter";
 
 #include "estop/EStopManager.h"
-
 #include "Logging.h"
 #include "radio/rmt/MainEncoder.h"
+#include "radio/rmt/RmtSequence.h"
 #include "Time.h"
 #include "util/FnProxy.h"
 #include "util/TaskUtils.h"
@@ -24,8 +24,7 @@ using namespace OpenShock;
 
 struct command_t {
   int64_t until;
-  std::vector<rmt_data_t> sequence;
-  std::vector<rmt_data_t> zeroSequence;
+  Rmt::RmtSequence sequence;
   uint16_t shockerId;
   bool overwrite;
 };
@@ -77,7 +76,13 @@ bool RFTransmitter::SendCommand(ShockerModelType model, uint16_t shockerId, Shoc
     return false;
   }
 
-  command_t* cmd = new command_t {.until = OpenShock::millis() + durationMs, .sequence = Rmt::GetSequence(model, shockerId, type, intensity), .zeroSequence = Rmt::GetZeroSequence(model, shockerId), .shockerId = shockerId, .overwrite = overwriteExisting};
+  auto sequence = Rmt::GetSequence(model, shockerId, type, intensity);
+  if (!sequence.is_valid()) {
+    OS_LOGE(TAG, "[pin-%hhi] Failed to create sequence for command, refusing to send", m_txPin);
+    return false;
+  }
+
+  command_t* cmd = new command_t {.until = OpenShock::millis() + durationMs, .sequence = std::move(sequence), .shockerId = shockerId, .overwrite = overwriteExisting};
 
   // We will use nullptr commands to end the task, if we got a nullptr here, we are out of memory... :(
   if (cmd == nullptr) {
@@ -202,15 +207,12 @@ void RFTransmitter::TransmitTask()
       cmd = *it;
 
       bool expired = cmd->until < OpenShock::millis();
-      bool empty   = cmd->sequence.empty();
 
-      // Remove expired or empty commands, else send the command.
+      // Remove expired commands, else send the command.
       // After sending/receiving a command, move to the next one.
-      if (expired || empty) {
-        // If the command is not empty, send the zero sequence to stop the shocker
-        if (!empty) {
-          rmtWriteBlocking(m_rmtHandle, cmd->zeroSequence.data(), cmd->zeroSequence.size());
-        }
+      if (expired) {
+        // Send the zero sequence to stop the shocker
+        rmtWriteBlocking(m_rmtHandle, cmd->sequence.terminator(), cmd->sequence.size());
 
         if (cmd->until + TRANSMIT_END_DURATION < OpenShock::millis()) {
           // Remove the command and move to the next one
@@ -222,7 +224,7 @@ void RFTransmitter::TransmitTask()
         }
       } else {
         // Send the command
-        rmtWriteBlocking(m_rmtHandle, cmd->sequence.data(), cmd->sequence.size());
+        rmtWriteBlocking(m_rmtHandle, cmd->sequence.payload(), cmd->sequence.size());
 
         // Move to the next command
         ++it;

src/radio/rmt/CaiXianlinEncoder.cpp

@@ -15,7 +15,7 @@ const rmt_data_t kRmtZero     = {250, 1, 750, 0};
 
 using namespace OpenShock;
 
-std::vector<rmt_data_t> Rmt::CaiXianlinEncoder::GetSequence(uint16_t transmitterId, uint8_t channelId, ShockerCommandType type, uint8_t intensity)
+static bool fillSequence(rmt_data_t* sequence, uint16_t shockerId, uint8_t channelId, ShockerCommandType type, uint8_t intensity)
 {
   // Intensity must be between 0 and 99
   intensity = std::min(intensity, static_cast<uint8_t>(99));
@@ -33,11 +33,11 @@ std::vector<rmt_data_t> Rmt::CaiXianlinEncoder::GetSequence(uint16_t transmitter
       intensity = 0;  // Sound intensity must be 0 for some shockers, otherwise it wont work, or they soft lock until restarted
       break;
     default:
-      return {};  // Invalid type
+      return false;  // Invalid type
   }
 
-  // Payload layout: [transmitterId:16][channelId:4][type:4][intensity:8]
-  uint32_t payload = (static_cast<uint32_t>(transmitterId) << 16) | (static_cast<uint32_t>(channelId & 0xF) << 12) | (static_cast<uint32_t>(typeVal & 0xF) << 8) | static_cast<uint32_t>(intensity);
+  // Payload layout: [shockerId:16][channelId:4][type:4][intensity:8]
+  uint32_t payload = (static_cast<uint32_t>(shockerId) << 16) | (static_cast<uint32_t>(channelId & 0xF) << 12) | (static_cast<uint32_t>(typeVal & 0xF) << 8) | static_cast<uint32_t>(intensity);
 
   // Calculate the checksum of the payload
   uint8_t checksum = Checksum::Sum8(payload);
@@ -48,12 +48,19 @@ std::vector<rmt_data_t> Rmt::CaiXianlinEncoder::GetSequence(uint16_t transmitter
   // Shift the data left by 3 bits to add the postamble (3 bits of 0)
   data <<= 3;
 
-  std::vector<rmt_data_t> pulses;
-  pulses.reserve(44);
-
   // Generate the sequence
-  pulses.push_back(kRmtPreamble);
-  Internal::EncodeBits<43>(pulses, data, kRmtOne, kRmtZero);
+  sequence[0] = kRmtPreamble;
+  Rmt::Internal::EncodeBits<43>(sequence + 1, data, kRmtOne, kRmtZero);
+
+  return true;
+}
+
+Rmt::RmtSequence Rmt::CaiXianlinEncoder::GetSequence(uint16_t shockerId, uint8_t channelId, ShockerCommandType type, uint8_t intensity)
+{
+  Rmt::RmtSequence sequence(44);
+
+  if (!fillSequence(sequence.payload(), shockerId, channelId, type, intensity)) return {};
+  if (!fillSequence(sequence.terminator(), shockerId, channelId, ShockerCommandType::Vibrate, 0)) return {};
 
-  return pulses;
+  return sequence;
 }

src/radio/rmt/MainEncoder.cpp

@@ -7,11 +7,10 @@ const char* const TAG = "RmtMainEncoder";
 #include "radio/rmt/Petrainer998DREncoder.h"
 #include "radio/rmt/PetrainerEncoder.h"
 
-#include <unordered_map>
-
 using namespace OpenShock;
 
-std::vector<rmt_data_t> Rmt::GetSequence(ShockerModelType model, uint16_t shockerId, ShockerCommandType type, uint8_t intensity) {
+Rmt::RmtSequence Rmt::GetSequence(ShockerModelType model, uint16_t shockerId, ShockerCommandType type, uint8_t intensity)
+{
   switch (model) {
     case ShockerModelType::Petrainer:
       return Rmt::PetrainerEncoder::GetSequence(shockerId, type, intensity);
@@ -24,32 +23,3 @@ std::vector<rmt_data_t> Rmt::GetSequence(ShockerModelType model, uint16_t shocke
       return {};
   }
 }
-/*
-std::shared_ptr<std::vector<rmt_data_t>> Rmt::GetZeroSequence(ShockerModelType model, uint16_t shockerId) {
-  static std::unordered_map<uint16_t, std::shared_ptr<std::vector<rmt_data_t>>> _sequences;
-
-  auto it = _sequences.find(shockerId);
-  if (it != _sequences.end()) return it->second; // FIXME: This is not thread-safe, and does not check if model is the same, causing a protocol mismatch
-
-  std::shared_ptr<std::vector<rmt_data_t>> sequence;
-  switch (model) {
-    case ShockerModelType::Petrainer:
-      sequence = std::make_shared<std::vector<rmt_data_t>>(Rmt::PetrainerEncoder::GetSequence(shockerId, ShockerCommandType::Vibrate, 0));
-      break;
-    case ShockerModelType::Petrainer998DR:
-      sequence = std::make_shared<std::vector<rmt_data_t>>(Rmt::Petrainer998DREncoder::GetSequence(shockerId, ShockerCommandType::Vibrate, 0));
-      break;
-    case ShockerModelType::CaiXianlin:
-      sequence = std::make_shared<std::vector<rmt_data_t>>(Rmt::CaiXianlinEncoder::GetSequence(shockerId, 0, ShockerCommandType::Vibrate, 0));
-      break;
-    default:
-      OS_LOGE(TAG, "Unknown shocker model: %u", model);
-      sequence = nullptr;
-      break;
-  }
-
-  _sequences[shockerId] = sequence;
-
-  return sequence;
-}
-*/

src/radio/rmt/Petrainer998DREncoder.cpp

@@ -11,7 +11,7 @@ const rmt_data_t kRmtPostamble = {250, 1, 3750, 0};  // Some subvariants expect
 
 using namespace OpenShock;
 
-std::vector<rmt_data_t> Rmt::Petrainer998DREncoder::GetSequence(uint16_t shockerId, ShockerCommandType type, uint8_t intensity)
+static bool fillSequence(rmt_data_t* sequence, uint16_t shockerId, ShockerCommandType type, uint8_t intensity)
 {
   // Intensity must be between 0 and 100
   intensity = std::min(intensity, static_cast<uint8_t>(100));
@@ -31,7 +31,7 @@ std::vector<rmt_data_t> Rmt::Petrainer998DREncoder::GetSequence(uint16_t shocker
       typeShift = 3;
       break;
     default:
-      return {};  // Invalid type
+      return false;  // Invalid type
   }
 
   uint8_t typeVal    = 1 << typeShift;
@@ -45,13 +45,20 @@ std::vector<rmt_data_t> Rmt::Petrainer998DREncoder::GetSequence(uint16_t shocker
   uint64_t data
     = (static_cast<uint64_t>(channel & 0xF) << 36 | static_cast<uint64_t>(typeVal & 0xF) << 32 | static_cast<uint64_t>(shockerId & 0xFFFF) << 16 | static_cast<uint64_t>(intensity & 0xFF) << 8 | static_cast<uint64_t>(typeInvert & 0xF) << 4 | static_cast<uint64_t>(channelInvert & 0xF));
 
-  std::vector<rmt_data_t> pulses;
-  pulses.reserve(42);
-
   // Generate the sequence
-  pulses.push_back(kRmtPreamble);
-  Internal::EncodeBits<40>(pulses, data, kRmtOne, kRmtZero);
-  pulses.push_back(kRmtPostamble);
+  sequence[0] = kRmtPreamble;
+  Rmt::Internal::EncodeBits<40>(sequence + 1, data, kRmtOne, kRmtZero);
+  sequence[41] = kRmtPostamble;
+
+  return true;
+}
+
+Rmt::RmtSequence Rmt::Petrainer998DREncoder::GetSequence(uint16_t shockerId, ShockerCommandType type, uint8_t intensity)
+{
+  Rmt::RmtSequence sequence(42);
+
+  if (!fillSequence(sequence.payload(), shockerId, type, intensity)) return {};
+  if (!fillSequence(sequence.terminator(), shockerId, ShockerCommandType::Vibrate, 0)) return {};
 
-  return pulses;
+  return sequence;
 }