Move packet building and sending logic out of main.cpp

.gitignore

@@ -3,3 +3,4 @@
 .vscode/c_cpp_properties.json
 .vscode/launch.json
 .vscode/ipch
+.vscode/settings.json

config.example.json

@@ -0,0 +1,9 @@
+{
+    "shades": [
+        {
+            "radioId": "13AC",
+            "mqttId": "example_blind",
+            "friendly_name": "Example Blind"
+        }
+    ]
+}

lib/configurator/Configurator.cpp

@@ -1,62 +1,68 @@
 #include "Configurator.h"
 #include <cctype>
+#include <ArduinoJson.h>
 
 Configurator::Configurator() {
     // TODO: Should be using MQTT directly here (and methods for handling methods should be private)
 }
 
-std::string Configurator::extractKey(std::string topic) {
+uint16_t Configurator::parseRadioId(const char* rawRadioId) {
-    int startIndex = topic.find("/") + 1;
+    if (!rawRadioId) {
-    int endIndex = topic.find("/", startIndex);
+        return 0; // Or another appropriate error value/exception
-    // TODO: need to verify that startIndex and endIndex are valid before substr()
-    return topic.substr(startIndex, endIndex - startIndex);
-}
-
-void Configurator::processIdMessage(std::string topic, std::string payload) {
-    auto key = extractKey(topic);
-
-    if (payload.length() != 4) {
-        // Ignore payloads that aren't exactly 4 characters
-        return;
     }
 
     uint16_t id = 0;
-    for (char c : payload) {
+    std::string radioId = std::string(rawRadioId);
-        // Check if valid hex digit (0-9, A-F, a-f)
+
+    if (radioId.size() != 4) {
+        return 0; // Invalid length
+    }
+
+    for (char c : radioId) {
         if (!std::isxdigit(c)) {
-            return; // Invalid character, not a hex digit
+            return 0; // Invalid character, not a hex digit
         }
 
-        // Convert hex digit to numerical value (0-9, A-B=10-11, ...)
         int digit = std::toupper(c) - (c <= '9' ? '0' : 'A' - 10);
-
         id = (id << 4) | digit;
     }
 
-    discoveredIds[key] = id;
+    return id;
-    tryBuild(key);
 }
 
-void Configurator::processFriendlyNameMessage(std::string topic, std::string friendlyName) {
+void Configurator::processJson(std::string json) {
-    auto key = extractKey(topic);
+  JsonDocument doc;
 
-    discoveredFriendlyNames[key] = friendlyName;
+  DeserializationError error = deserializeJson(doc, json);
-    tryBuild(key);
+
+  if (error) {
+    return;
+  }
+
+  JsonArray shades = doc["shades"];
+
+  for (JsonObject shade : shades) {
+    const char* rawRadioId = shade["radioId"];
+    const char* mqttId = shade["mqttId"];
+    const char* friendlyName = shade["friendly_name"];
+
+    if (rawRadioId && mqttId && friendlyName && std::char_traits<char>::length(mqttId) > 0) {
+        uint16_t id = parseRadioId(rawRadioId);
+
+        if (id != 0) {
+            Shade s{id, mqttId, friendlyName, "stopped", -1, -1};
+            for (const auto& callback : shadeConfiguredCallbacks) {
+                callback(s);
+            }
+        }
+    }
+    else
+    {
+      // Invalid JSON - missing radioId, mqttId, or friendly_name"
+    }
+  }
 }
 
 void Configurator::addShadeConfiguredCallback(std::function<void (Shade)> callback) {
     shadeConfiguredCallbacks.push_back(callback);
 }
-
-bool Configurator::tryBuild(std::string key) {
-    if (auto id = discoveredIds.find(key); id != discoveredIds.end()) {
-        if (auto friendlyName = discoveredFriendlyNames.find(key); friendlyName != discoveredFriendlyNames.end()) {
-            auto shade = Shade{id->second, key, friendlyName->second, -1, -1, 0, 0, nullptr};
-            for (size_t i = 0; i < shadeConfiguredCallbacks.size(); i++) {
-                shadeConfiguredCallbacks[i](shade);
-            }
-            return true;
-        }
-    }
-    return false;
-}

lib/configurator/Configurator.h

@@ -12,19 +12,15 @@ class Configurator
 {
 private:
     std::vector<std::function<void (Shade)>> shadeConfiguredCallbacks;
-    std::map<std::string, uint16_t> discoveredIds;
-    std::map<std::string, std::string> discoveredFriendlyNames;
 
-    std::string extractKey(std::string topic);
+    uint16_t parseRadioId(const char* rawRadioId);
-    bool tryBuild(std::string topic);
 
 public:
     Configurator();
 
     void addShadeConfiguredCallback(std::function<void (Shade)> callback);
 
-    void processIdMessage(std::string topic, std::string id);
+    void processJson(std::string json);
-    void processFriendlyNameMessage(std::string topic, std::string friendlyName);
 };
 
 #endif // CONFIGURATOR_H

lib/packet_builder/PacketBuilder.cpp

@@ -0,0 +1,84 @@
+#include "PacketBuilder.h"
+
+PacketBuilder::PacketBuilder()
+{
+}
+
+Packet PacketBuilder::buildOpenPacket(uint16_t destination)
+{
+    Packet packet;
+    auto header = UnicastHeader{};
+    header.destination = destination;
+    header.source = 0x0000;
+    packet.header = header;
+    packet.type = PacketType::OPEN;
+
+    return packet;
+}
+
+Packet PacketBuilder::buildClosePacket(uint16_t destination)
+{
+    Packet packet;
+    auto header = UnicastHeader{};
+    header.destination = destination;
+    header.source = 0x0000;
+    packet.header = header;
+    packet.type = PacketType::CLOSE;
+
+    return packet;
+}
+
+Packet PacketBuilder::buildStopPacket(uint16_t destination)
+{
+    Packet packet;
+    auto header = UnicastHeader{};
+    header.destination = destination;
+    header.source = 0x0000;
+    packet.header = header;
+    packet.type = PacketType::STOP;
+
+    return packet;
+}
+
+Packet PacketBuilder::buildSetPositionPacket(uint16_t destination, float percentage)
+{
+    Packet packet;
+    auto header = UnicastHeader{};
+    header.destination = destination;
+    header.source = 0x0000;
+    packet.header = header;
+    packet.type = PacketType::FIELD_COMMAND;
+
+    std::vector<Field> fields;
+
+    uint8_t identifier = 0x50; // position
+    FieldType type = FieldType::SET;
+    uint16_t position = (uint16_t)(0xFFFF * percentage);
+    fields.push_back(Field{identifier, type, true, position});
+
+    packet.parameters = FieldsParameters{fields};
+
+    return packet;
+}
+
+Packet PacketBuilder::buildFetchPositionPacket(uint16_t destination)
+{
+    Packet packet;
+    auto header = UnicastHeader{};
+    header.destination = destination;
+    header.source = 0x0000;
+    packet.header = header;
+    packet.type = PacketType::FIELD_COMMAND;
+
+    std::vector<Field> fields;
+
+    // position
+    fields.push_back(Field{0x50, FieldType::FETCH, false, std::monostate{}});
+
+    // battery
+    fields.push_back(Field{0x42, FieldType::FETCH, false, std::monostate{}});
+
+    packet.parameters = FieldsParameters{fields};
+
+    return packet;
+}

lib/packet_builder/PacketBuilder.h

@@ -0,0 +1,17 @@
+#ifndef PACKET_BUILDER_H
+#define PACKET_BUILDER_H
+
+#include <PacketTypes.h>
+
+class PacketBuilder {
+public:
+    PacketBuilder();
+
+    Packet buildOpenPacket(uint16_t destination);
+    Packet buildClosePacket(uint16_t destination);
+    Packet buildStopPacket(uint16_t destination);
+    Packet buildSetPositionPacket(uint16_t destination, float percentage);
+    Packet buildFetchPositionPacket(uint16_t destination);
+};
+
+#endif // PACKET_BUILDER_H

lib/packet_sender/PacketSender.cpp

@@ -0,0 +1,27 @@
+#include "PacketSender.h"
+
+PacketSender::PacketSender(RFPowerView& powerView) : powerView(powerView)
+{
+}
+
+bool PacketSender::send(Packet packet)
+{
+    Serial.println("Attempting to send a packet");
+    packet.rollingCode1 = lastRollingCode1 + 1;
+    packet.rollingCode2 = lastRollingCode2 + 1;
+
+    bool didSend = powerView.sendPacket(&packet);
+    if (!didSend) {
+        Serial.println("Failed to send");
+        return false;
+    } else {
+        lastRollingCode1++;
+        lastRollingCode2++;
+        return true;
+    }
+}
+
+void PacketSender::setLastRollingCodes(uint8_t code1, uint8_t code2) {
+    lastRollingCode1 = code1;
+    lastRollingCode2 = code2;
+}

lib/packet_sender/PacketSender.h

@@ -0,0 +1,20 @@
+#ifndef PACKET_SENDER_H
+#define PACKET_SENDER_H
+
+#include <RFPowerView.h>
+#include <PacketTypes.h>
+
+class PacketSender {
+public:
+    PacketSender(RFPowerView& powerView);
+
+    bool send(Packet packet);
+    void setLastRollingCodes(uint8_t code1, uint8_t code2);
+private:
+    RFPowerView& powerView;
+
+    uint8_t lastRollingCode1 = 0x3D;
+    uint8_t lastRollingCode2 = 0x96;
+};
+
+#endif // PACKET_SENDER_H

lib/position_watcher/PositionWatcher.cpp

@@ -0,0 +1,53 @@
+#include "PositionWatcher.h"
+#include <Arduino.h>
+
+void PositionWatcher::start(int8_t newTargetValue)
+{
+    targetValue = newTargetValue;
+    lastPollTime = 0;
+    pollingInterval = 2000;
+    maxAttempts = 3;
+    maxConsecutiveDuplicates = 2;
+    consecutiveDuplicates = 0;
+    currentAttempt = 0;
+    isActive = true;
+}
+
+bool PositionWatcher::shouldFetch()
+{
+    if (isWatching() && millis() - lastPollTime > pollingInterval) {
+        if (currentAttempt >= maxAttempts) {
+            isActive = false;
+            return false;
+        }
+        return true;
+    }
+    return false;
+}
+
+bool PositionWatcher::isWatching()
+{
+    return isActive;
+}
+
+void PositionWatcher::fetchQueued()
+{
+    lastPollTime = millis();
+    currentAttempt++;
+}
+
+void PositionWatcher::fetchReceived(int8_t value)
+{
+    currentAttempt = 0;
+    if (value == targetValue) {
+        isActive = false;
+    } else if (lastValue == value) {
+        consecutiveDuplicates++;
+        if (consecutiveDuplicates >= maxConsecutiveDuplicates) {
+            isActive = false;
+        }
+    } else {
+        consecutiveDuplicates = 1;
+    }
+    lastValue = value;
+}

lib/position_watcher/PositionWatcher.h

@@ -0,0 +1,31 @@
+#ifndef POSITION_WATCHER_H
+#define POSITION_WATCHER_H
+
+#include <stdint.h>
+
+class PositionWatcher
+{
+private:
+    bool isActive;
+
+    int8_t targetValue;
+    int8_t lastValue;
+    int8_t consecutiveDuplicates;
+
+    uint32_t pollingInterval;
+
+    int8_t maxAttempts;
+    int8_t maxConsecutiveDuplicates;
+
+    int8_t currentAttempt;
+    uint32_t lastPollTime;
+
+public:
+    void start(int8_t newTargetValue);
+    bool shouldFetch();
+    bool isWatching();
+    void fetchQueued();
+    void fetchReceived(int8_t value);
+};
+
+#endif // POSITION_WATCHER_H

lib/shade/Shade.h

@@ -8,11 +8,13 @@ struct Shade {
   uint16_t ID;
   std::string key;
   std::string friendlyName;
-  int8_t lastTargetPosition;
+
+  std::string state;
+  
   int8_t lastPosition;
-  uint8_t samePositionCount;
+  int8_t lastBattery;
-  uint8_t positionFetchCount;
+
-  void* timer;
+  bool modified;
 };
 
 #endif // SHADE_H

lib/shade_command/ShadeCommand.h

@@ -0,0 +1,33 @@
+#ifndef SHADE_COMMAND_H
+#define SHADE_COMMAND_H
+
+#include <variant>
+#include <stdint.h>
+#include <typeindex>
+
+struct OpenCommand {
+  uint16_t shadeID;
+};
+
+struct CloseCommand {
+  uint16_t shadeID;
+};
+
+struct StopCommand {
+  uint16_t shadeID;
+};
+
+struct SetPositionCommand {
+  uint16_t shadeID;
+  float percentage;
+};
+
+struct RefreshCommand {
+  uint16_t shadeID;
+};
+
+
+using ShadeCommand = std::variant<OpenCommand, CloseCommand, StopCommand, SetPositionCommand, RefreshCommand>;
+
+
+#endif // SHADE_COMMAND_H

platformio.ini

@@ -24,8 +24,6 @@ lib_deps =
 
     plapointe6/EspMQTTClient@^1.13.3
 
-    contrem/arduino-timer@^3.0.1
-
     RFPowerView=https://git.mattway.com.au/matt/RFPowerView.git#v0.0.3
 
 [env:test_embedded]

src/.gitignore

@@ -1 +1,2 @@
-secrets.h
+secrets.h
+config.h

src/main.cpp

@@ -1,23 +1,25 @@
 #include <Arduino.h>
 #include <EspMqttClient.h>
-#include <arduino-timer.h>
 #include <ArduinoJson.h>
 #include <RFPowerView.h>
 #include <random>
 #include <chrono>
+#include "config.h"
 #include "Shade.h"
 #include "Configurator.h"
 #include "ShadeRepository.h"
 #include "HADiscovery.h"
 #include "secrets.h"
+#include "ShadeCommand.h"
+#include "PositionWatcher.h"
+#include "PacketBuilder.h"
+#include "PacketSender.h"
+#include <queue>
 
 #define SER_BAUDRATE                   (115200)
 
 RFPowerView powerView(RF_CE_PIN, RF_CS_PIN, RF_IRQ_PIN, RF_ID);
 
-uint8_t lastRollingCode1 = 0x3D;
-uint8_t lastRollingCode2 = 0x96;
-
 static std::mt19937 generator(std::chrono::system_clock::now().time_since_epoch().count());
 
 std::string generate_client_id_suffix(int length) {
@@ -49,31 +51,29 @@ Configurator configurator = Configurator();
 
 ShadeRepository shadeRepository = ShadeRepository();
 
+using Command = std::variant<OpenCommand, CloseCommand, StopCommand, SetPositionCommand, RefreshCommand>;
+std::queue<Command> commands;
+
+std::map<uint16_t, PositionWatcher> positionWatchers;
+
 HADiscovery haDiscovery(topic_prefix.c_str(), [] (const char* topic, const char* message) {
   client.publish(topic, message);
 });
 
-#define MAX_FETCH_COUNT               (20)
+PacketBuilder packetBuilder;
-auto timer = Timer<10, millis, uint16_t>();
+
+PacketSender packetSender(powerView);
 
 void processPacket(const Packet*);
 void processCommandMessage(const String& topic, const String &payload);
 void processSetPositionMessage(const String& topic, const String &payload);
 
-bool sendOpenPacket(uint16_t destination);
-bool sendClosePacket(uint16_t destination);
-bool sendStopPacket(uint16_t destination);
-bool sendSetPosition(uint16_t destination, float percentage);
-bool sendFetchPosition(uint16_t destination);
-bool sendPacket(Packet *packet);
-
-bool checkPosition(uint16_t shadeID);
-void startFetchingPosition(uint16_t shadeID, int8_t targetPosition);
-
 void publishPosition(const std::string& shadeKey, const uint8_t position);
 void publishState(const std::string& shadeKey, const String& state);
 void publishBattery(const std::string& shadeKey, const uint8_t battery);
 
+void startPositionWatcher(uint16_t shadeID, uint8_t targetPosition);
+
 void publishDiscoveryTopics();
 
 void setup() {
@@ -110,6 +110,8 @@ void setup() {
     publishDiscoveryTopics();
   });
 
+  configurator.processJson(config_json);
+
   delay(100);
 
   Serial.println("Ready");
@@ -118,7 +120,97 @@ void setup() {
 void loop() {
   powerView.loop();
   client.loop();
-  timer.tick();
+
+  if (client.isConnected()) {
+    if (!commands.empty()) {
+      auto command = commands.front();
+      commands.pop();
+
+      if (std::holds_alternative<OpenCommand>(command)) {
+        auto openCommand = std::get<OpenCommand>(command);
+        Serial.printf("Open command received for shade 0x%04x\n", openCommand.shadeID);
+        if (packetSender.send(packetBuilder.buildOpenPacket(openCommand.shadeID))) {
+          auto shade = shadeRepository.findById(openCommand.shadeID);
+          if (shade != nullptr) {
+            shade->state = "opening";
+            shade->modified = true;
+          }
+          startPositionWatcher(openCommand.shadeID, 100);
+        }
+        
+      } else if (std::holds_alternative<CloseCommand>(command)) {
+        auto closeCommand = std::get<CloseCommand>(command);
+        Serial.printf("Close command received for shade 0x%04x\n", closeCommand.shadeID);
+        if (packetSender.send(packetBuilder.buildClosePacket(closeCommand.shadeID))) {
+          auto shade = shadeRepository.findById(closeCommand.shadeID);
+          if (shade != nullptr) {
+            shade->state = "closing";
+            shade->modified = true;
+          }
+          startPositionWatcher(closeCommand.shadeID, 0);
+        }
+      } else if (std::holds_alternative<StopCommand>(command)) {
+        auto stopCommand = std::get<StopCommand>(command);
+        Serial.printf("Stop command received for shade 0x%04x\n", stopCommand.shadeID);
+        packetSender.send(packetBuilder.buildStopPacket(stopCommand.shadeID));
+        startPositionWatcher(stopCommand.shadeID, -1);
+        
+      } else if (std::holds_alternative<SetPositionCommand>(command)) {
+        auto setPositionCommand = std::get<SetPositionCommand>(command);
+        Serial.printf("Set Position command received for shade 0x%04x (%.2f)\n", setPositionCommand.shadeID, setPositionCommand.percentage);
+        if (packetSender.send(packetBuilder.buildSetPositionPacket(setPositionCommand.shadeID, setPositionCommand.percentage))) {
+          auto shade = shadeRepository.findById(setPositionCommand.shadeID);
+          if (shade != nullptr) {
+            if (setPositionCommand.percentage > shade->lastPosition / 100.0f) {
+              shade->state = "opening";
+              shade->modified = true;
+            } else {
+              shade->state = "closing";
+              shade->modified = true;
+            }
+          }
+          startPositionWatcher(setPositionCommand.shadeID, setPositionCommand.percentage * 100);
+        }
+      } else if (std::holds_alternative<RefreshCommand>(command)) {
+        auto refreshCommand = std::get<RefreshCommand>(command);
+        Serial.printf("Refresh command received for shade 0x%04x\n", refreshCommand.shadeID);
+        startPositionWatcher(refreshCommand.shadeID, -1);
+      }
+    }
+
+    for (auto& [shadeID, watcher] : positionWatchers) {
+      if (watcher.shouldFetch()) {
+        if (packetSender.send(packetBuilder.buildFetchPositionPacket(shadeID))) {
+          watcher.fetchQueued();
+        }
+      } else if (!watcher.isWatching()) {
+        positionWatchers.erase(shadeID);
+        auto shade = shadeRepository.findById(shadeID);
+        if (shade != nullptr) {
+          if (shade->lastPosition == 0) {
+            shade->state = "closed";
+            shade->modified = true;
+          } else {
+            shade->state = "open";
+            shade->modified = true;
+          }
+        }
+      }
+    }
+
+    for (auto shade = shadeRepository.begin(); shade != shadeRepository.end(); shade++) {
+      if (shade->modified) {
+        if (shade->lastPosition != -1) {
+          publishPosition(shade->key, shade->lastPosition);
+        }
+        if (shade->lastBattery != -1) {
+          publishBattery(shade->key, shade->lastBattery);
+        }
+        publishState(shade->key, shade->state.c_str());
+        shade->modified = false;
+      }
+    }
+  }
 }
 
 void processPacket(const Packet *packet) {
@@ -143,36 +235,37 @@ void processPacket(const Packet *packet) {
   // Update last rolling codes each time a packet from a real hub is detected
   if (source == 0x0000) {
     Serial.println("Updating rolling codes");
-    lastRollingCode1 = packet->rollingCode1;
+    packetSender.setLastRollingCodes(packet->rollingCode1, packet->rollingCode2);
-    lastRollingCode2 = packet->rollingCode2;
   }
 
   if (packet->type == PacketType::FIELDS) {
-    FieldsParameters parameters = std::get<FieldsParameters>(packet->parameters);
+    auto shade = shadeRepository.findById(source);
-    for (size_t i = 0; i < parameters.fields.size(); i++) {
+    if (shade != nullptr) {
-      Field field = parameters.fields[i];
+      FieldsParameters parameters = std::get<FieldsParameters>(packet->parameters);
-      if (field.identifier == 0x50) {
+      for (size_t i = 0; i < parameters.fields.size(); i++) {
-        auto shade = shadeRepository.findById(source);
+        Field field = parameters.fields[i];
-        if (shade != nullptr) {
+        if (field.identifier == 0x50) {
           uint16_t value = std::get<uint16_t>(field.value);
           uint8_t position = (uint8_t)std::round(((float)value / 0xFFFF) * 100);
 
-          if (shade->lastPosition == position) {
-            shade->samePositionCount++;
-          } else {
-            shade->samePositionCount = 1;
-          }
           shade->lastPosition = position;
+          shade->modified = true;
 
-          publishPosition(shade->key, position);
+          auto it = positionWatchers.find(shade->ID);
-        }
+          if (it != positionWatchers.end()) {
-      } else if (field.identifier == 0x42) {
+            auto& watcher = it->second;
-        auto shade = shadeRepository.findById(source);
+            watcher.fetchReceived(position);
-        if (shade != nullptr) {
+          }
+
+          // TODO: set updated flag?
+        } else if (field.identifier == 0x42) {
           uint8_t value = std::get<uint8_t>(field.value);
           uint8_t battery = uint8_t(((float)value / 200) * 100);
 
-          publishBattery(shade->key, battery);
+          shade->lastBattery = battery;
+          shade->modified = true;
+
+          // TODO: set updated flag?
         }
       }
     }
@@ -198,182 +291,45 @@ void onConnectionEstablished() {
   });
 }
 
-bool sendOpenPacket(uint16_t destination) {
+void startPositionWatcher(uint16_t shadeID, uint8_t targetValue) {
-  Packet packet;
+  auto& watcher = positionWatchers.try_emplace(shadeID).first->second;
-  auto header = UnicastHeader {};
+  watcher.start(targetValue);
-  header.destination = destination;
-  header.source = 0x0000;
-  packet.header = header;
-  packet.type = PacketType::OPEN;
-  
-  return sendPacket(&packet);
-}
-
-bool sendClosePacket(uint16_t destination) {
-  Packet packet;
-  auto header = UnicastHeader {};
-  header.destination = destination;
-  header.source = 0x0000;
-  packet.header = header;
-  packet.type = PacketType::CLOSE;
-  
-  return sendPacket(&packet);
-}
-
-bool sendStopPacket(uint16_t destination) {
-  Packet packet;
-  auto header = UnicastHeader {};
-  header.destination = destination;
-  header.source = 0x0000;
-  packet.header = header;
-  packet.type = PacketType::STOP;
-  
-  return sendPacket(&packet);
-}
-
-bool sendSetPosition(uint16_t destination, float percentage) {
-  Packet packet;
-  auto header = UnicastHeader {};
-  header.destination = destination;
-  header.source = 0x0000;
-  packet.header = header;
-  packet.type = PacketType::FIELD_COMMAND;
-  
-  std::vector<Field> fields;
-  
-  uint8_t identifier = 0x50; // position
-  FieldType type = FieldType::SET;
-  uint16_t position = (uint16_t)(0xFFFF * percentage);
-  fields.push_back(Field{identifier, type, true, position});
-  
-  packet.parameters = FieldsParameters {fields};
-  
-  return sendPacket(&packet);
-}
-
-bool sendFetchPosition(uint16_t destination) {
-  Packet packet;
-  auto header = UnicastHeader {};
-  header.destination = destination;
-  header.source = 0x0000;
-  packet.header = header;
-  packet.type = PacketType::FIELD_COMMAND;
-  
-  std::vector<Field> fields;
-  
-  // position
-  fields.push_back(Field{0x50, FieldType::FETCH, false, std::monostate{}});
-
-  // battery
-  fields.push_back(Field{0x42, FieldType::FETCH, false, std::monostate{}});
-  
-  packet.parameters = FieldsParameters {fields};
-  
-  return sendPacket(&packet);
-}
-
-bool sendPacket(Packet *packet) {
-  Serial.println("Attempting to send a packet");
-  packet->rollingCode1 = lastRollingCode1 + 1;
-  packet->rollingCode2 = lastRollingCode2 + 1;
-
-  bool didSend = powerView.sendPacket(packet);
-  if (!didSend) {
-    Serial.println("Failed to send");
-    return false;
-  } else {
-    lastRollingCode1++;
-    lastRollingCode2++;
-    return true;
-  }
 }
 
 void processCommandMessage(const String &topic, const String &payload) {
   int startIndex = topic.indexOf("/") + 1;
   int endIndex = topic.indexOf("/", startIndex);
-  auto key = topic.substring(startIndex, endIndex).c_str();
+  String key = topic.substring(startIndex, endIndex);
 
-  auto shade = shadeRepository.findByKey(key);
+  Serial.printf("Received command '%s' for shade '%s'\n", payload.c_str(), key.c_str());
+
+  auto shade = shadeRepository.findByKey(key.c_str());
   if (shade != nullptr) {
     if (payload == "OPEN") {
-      sendOpenPacket(shade->ID);
+      commands.push(OpenCommand {shade->ID});
-
-      startFetchingPosition(shade->ID, 100);
-
-      publishState(shade->key, "opening");
     } else if (payload == "CLOSE") {
-      sendClosePacket(shade->ID);
+      commands.push(CloseCommand {shade->ID});
-
-      startFetchingPosition(shade->ID, 0);
-
-      publishState(shade->key, "closing");
     } else if (payload == "STOP") {
-      sendStopPacket(shade->ID);
+      commands.push(StopCommand {shade->ID});
-
-      startFetchingPosition(shade->ID, -1);
-      timer.in(100, sendFetchPosition, shade->ID);
     } else if (payload == "REFRESH") {
-      startFetchingPosition(shade->ID, -1);
+      commands.push(RefreshCommand {shade->ID});
     }
+  } else {
+    Serial.println("Failed to find shade");
   }
 }
 
 void processSetPositionMessage(const String& topic, const String &payload)  {
   int startIndex = topic.indexOf("/") + 1;
   int endIndex = topic.indexOf("/", startIndex);
-  auto key = topic.substring(startIndex, endIndex).c_str();
+  String key = topic.substring(startIndex, endIndex);
 
-  auto shade = shadeRepository.findByKey(key);
+  Serial.printf("Received set position command for shade '%s' (%s)\n", key.c_str(), payload.c_str());
+
+  auto shade = shadeRepository.findByKey(key.c_str());
   if (shade != nullptr) {
     float percentage = payload.toInt() / 100.0f;
-    sendSetPosition(shade->ID, percentage);
+    commands.push(SetPositionCommand {shade->ID, percentage});
-
-    if (payload.toInt() > shade->lastPosition) {
-      publishState(shade->key, "opening");
-    } else if (payload.toInt() < shade->lastPosition) {
-      publishState(shade->key, "closing");
-    }
-
-    startFetchingPosition(shade->ID, payload.toInt());
-  }
-}
-
-bool checkPosition(uint16_t shadeID) {
-  auto shade = shadeRepository.findById(shadeID);
-  if (shade != nullptr) {
-    // Keep fetching position if:
-    // - the last reported position doesn't match the target position
-    // - a position hasn't been reported yet
-    // - there is no target position
-    if (shade->lastTargetPosition != shade->lastPosition || shade->lastPosition == -1 || shade->lastTargetPosition == -1) {
-      // Keep fetching position if the count limits have not been reached
-      if (shade->positionFetchCount < MAX_FETCH_COUNT && shade->samePositionCount < 2) {
-        shade->positionFetchCount++;
-        sendFetchPosition(shadeID);
-        return true;
-      }
-    }
-
-    publishState(shade->key, shade->lastPosition > 0 ? "open" : "closed");
-    return false;
-  }
-  return false;
-}
-
-void startFetchingPosition(uint16_t shadeID, int8_t targetPosition) {
-  auto shade = shadeRepository.findById(shadeID);
-  if (shade != nullptr) {
-    // Cancel any existing timer
-    if (shade->timer != nullptr) {
-      timer.cancel(shade->timer);
-      shade->timer = nullptr;
-    }
-
-    shade->lastTargetPosition = targetPosition;
-    shade->positionFetchCount = 0;
-    shade->samePositionCount = 1;
-
-    shade->timer = timer.every(2000, checkPosition, shade->ID);
   }
 }
 
@@ -393,4 +349,4 @@ void publishDiscoveryTopics() {
   for (auto shade = shadeRepository.begin(); shade != shadeRepository.end(); shade++) {
     haDiscovery.publish(*shade);
   }
-}
+}

test/test_configurator/test_configurator.cpp

@@ -10,6 +10,11 @@ void tearDown()
 {
 }
 
+std::string singleBlindJson = R"({"shades": [{"radioId": "ABCD","mqttId": "test_shade","friendly_name": "Test Shade"}]})";
+std::string doubleBlindJson = R"({"shades": [{"radioId": "ABCD","mqttId": "test_shade","friendly_name": "Test Shade"}, {"radioId": "1234","mqttId": "test_shade2","friendly_name": "Test Shade 2"}]})";
+std::string invalidRadioIdBlindJson = R"({"shades": [{"radioId": "XYZ9","mqttId": "test_shade","friendly_name": "Test Shade"}, {"mqttId": "test_shade","friendly_name": "Test Shade"}, {"radioId": "","mqttId": "test_shade","friendly_name": "Test Shade"}, {"radioId": "ABCDE","mqttId": "test_shade","friendly_name": "Test Shade"}]})";
+std::string invalidMqttIdBlindJson = R"({"shades": [{"radioId": "ABCD","mqttId": "","friendly_name": "Test Shade"}, {"radioId": "ABCD","friendly_name": "Test Shade"}]})";
+
 void test_shade_is_configured_with_id_and_friendly_name()
 {
   int callbackInvokedCount = 0;
@@ -23,71 +28,26 @@ void test_shade_is_configured_with_id_and_friendly_name()
     TEST_ASSERT_EQUAL_STRING("test_shade", shade.key.c_str());
   });
 
-  configurator.processIdMessage("hotdog/test_shade/id", "ABCD");
+  configurator.processJson(singleBlindJson);
-  configurator.processFriendlyNameMessage("hotdog/test_shade/friendly_name", "Test Shade");
 
   TEST_ASSERT_EQUAL_INT(1, callbackInvokedCount);
 }
 
-void test_shade_is_configured_again_with_new_friendly_name()
+void test_multiple_shades_are_configured()
 {
   int callbackInvokedCount = 0;
 
   Configurator configurator = Configurator();
   configurator.addShadeConfiguredCallback([&] (Shade shade) {
     callbackInvokedCount++;
-    
-    if (callbackInvokedCount == 1) {
-      TEST_ASSERT_EQUAL_STRING("Test Shade", shade.friendlyName.c_str());
-    } else if (callbackInvokedCount == 2) {
-      TEST_ASSERT_EQUAL_STRING("Updated Test Shade", shade.friendlyName.c_str());
-    } else {
-      TEST_ABORT();
-    }
-    TEST_ASSERT_EQUAL_HEX16(0xABCD, shade.ID);
-    TEST_ASSERT_EQUAL_STRING("test_shade", shade.key.c_str());
   });
 
-  configurator.processIdMessage("hotdog/test_shade/id", "ABCD");
+  configurator.processJson(doubleBlindJson);
-  configurator.processFriendlyNameMessage("hotdog/test_shade/friendly_name", "Test Shade");
-
-  TEST_ASSERT_EQUAL_INT(1, callbackInvokedCount);
-
-  configurator.processFriendlyNameMessage("hotdog/test_shade/friendly_name", "Updated Test Shade");
 
   TEST_ASSERT_EQUAL_INT(2, callbackInvokedCount);
 }
 
-void test_shade_is_configured_again_with_new_id()
+void test_invalid_radio_id_is_ignored()
-{
-  int callbackInvokedCount = 0;
-
-  Configurator configurator = Configurator();
-  configurator.addShadeConfiguredCallback([&] (Shade shade) {
-    callbackInvokedCount++;
-    
-    TEST_ASSERT_EQUAL_STRING("Test Shade", shade.friendlyName.c_str());
-    if (callbackInvokedCount == 1) {
-      TEST_ASSERT_EQUAL_HEX16(0xABCD, shade.ID);
-    } else if (callbackInvokedCount == 2) {
-      TEST_ASSERT_EQUAL_HEX16(0x9876, shade.ID);
-    } else {
-      TEST_ABORT();
-    }
-    TEST_ASSERT_EQUAL_STRING("test_shade", shade.key.c_str());
-  });
-
-  configurator.processIdMessage("hotdog/test_shade/id", "ABCD");
-  configurator.processFriendlyNameMessage("hotdog/test_shade/friendly_name", "Test Shade");
-
-  TEST_ASSERT_EQUAL_INT(1, callbackInvokedCount);
-
-  configurator.processIdMessage("hotdog/test_shade/id", "9876");
-
-  TEST_ASSERT_EQUAL_INT(2, callbackInvokedCount);
-}
-
-void test_shade_is_not_configured_without_friendly_name()
 {
   int callbackInvokedCount = 0;
 
@@ -96,12 +56,12 @@ void test_shade_is_not_configured_without_friendly_name()
     callbackInvokedCount++;
   });
 
-  configurator.processIdMessage("hotdog/test_shade/id", "ABCD");
+  configurator.processJson(invalidRadioIdBlindJson);
 
   TEST_ASSERT_EQUAL_INT(0, callbackInvokedCount);
 }
 
-void test_shade_is_not_configured_without_id()
+void test_invalid_mqtt_id_is_ignored()
 {
   int callbackInvokedCount = 0;
 
@@ -110,35 +70,18 @@ void test_shade_is_not_configured_without_id()
     callbackInvokedCount++;
   });
 
-  configurator.processFriendlyNameMessage("hotdog/test_shade/friendly_name", "Test Shade");
+  configurator.processJson(invalidMqttIdBlindJson);
 
   TEST_ASSERT_EQUAL_INT(0, callbackInvokedCount);
 }
 
-void test_shade_is_not_configured_with_invalid_id()
-{
-  int callbackInvokedCount = 0;
-
-  Configurator configurator = Configurator();
-  configurator.addShadeConfiguredCallback([&] (Shade shade) {
-    callbackInvokedCount++;
-  });
-
-  configurator.processIdMessage("hotdog/test_shade/id", "ZZZZ");
-  configurator.processFriendlyNameMessage("hotdog/test_shade/friendly_name", "Test Shade");
-  
-  TEST_ASSERT_EQUAL_INT(0, callbackInvokedCount);
-}
-
 int runUnityTests(void)
 {
   UNITY_BEGIN();
   RUN_TEST(test_shade_is_configured_with_id_and_friendly_name);
-  RUN_TEST(test_shade_is_configured_again_with_new_friendly_name);
+  RUN_TEST(test_multiple_shades_are_configured);
-  RUN_TEST(test_shade_is_configured_again_with_new_id);
+  RUN_TEST(test_invalid_radio_id_is_ignored);
-  RUN_TEST(test_shade_is_not_configured_without_friendly_name);
+  RUN_TEST(test_invalid_mqtt_id_is_ignored);
-  RUN_TEST(test_shade_is_not_configured_without_id);
-  RUN_TEST(test_shade_is_not_configured_with_invalid_id);
   return UNITY_END();
 }
 

test/test_ha_discovery/test_ha_discovery.cpp

@@ -23,7 +23,7 @@ void test_publish_message_count()
 
     HADiscovery haDiscovery = HADiscovery("my_prefix/", callback);
 
-    haDiscovery.publish(Shade{0xABCD, "test_shade", "Test Shade", -1, -1, 0, 0, nullptr});
+    haDiscovery.publish(Shade{0xABCD, "test_shade", "Test Shade", "stopped", -1, -1});
 
     TEST_ASSERT_EQUAL_INT(3, callbackInvokedCount);
 }
@@ -47,7 +47,7 @@ void test_publish_cover_message()
 
     HADiscovery haDiscovery = HADiscovery("my_prefix/", callback);
 
-    haDiscovery.publish(Shade{0xABCD, "test_shade", "Test Shade", -1, -1, 0, 0, nullptr});
+    haDiscovery.publish(Shade{0xABCD, "test_shade", "Test Shade", "stopped", -1, -1});
 
     TEST_ASSERT_EQUAL_INT(1, messageAssertedCount);
 }
@@ -71,7 +71,7 @@ void test_publish_battery_message()
 
     HADiscovery haDiscovery = HADiscovery("my_prefix/", callback);
 
-    haDiscovery.publish(Shade{0xABCD, "test_shade", "Test Shade", -1, -1, 0, 0, nullptr});
+    haDiscovery.publish(Shade{0xABCD, "test_shade", "Test Shade", "stopped", -1, -1});
 
     TEST_ASSERT_EQUAL_INT(1, messageAssertedCount);
 }
@@ -95,7 +95,7 @@ void test_publish_button_message()
 
     HADiscovery haDiscovery = HADiscovery("my_prefix/", callback);
 
-    haDiscovery.publish(Shade{0xABCD, "test_shade", "Test Shade", -1, -1, 0, 0, nullptr});
+    haDiscovery.publish(Shade{0xABCD, "test_shade", "Test Shade", "stopped", -1, -1});
 
     TEST_ASSERT_EQUAL_INT(1, messageAssertedCount);
 }

test/test_position_watcher/test_position_watcher.cpp

@@ -0,0 +1,161 @@
+#include <unity.h>
+#include <Arduino.h>
+#include <ArduinoFake.h>
+#include "PositionWatcher.h"
+
+using namespace fakeit;
+
+void setUp()
+{
+}
+
+void tearDown()
+{
+}
+
+void test_position_watcher_should_not_fetch_initially()
+{
+  PositionWatcher positionWatcher = PositionWatcher();
+  
+  TEST_ASSERT_FALSE(positionWatcher.shouldFetch());
+}
+
+void test_position_watcher_should_fetch_after_start()
+{
+  When(Method(ArduinoFake(), millis)).Return(10000);
+
+  PositionWatcher positionWatcher = PositionWatcher();
+  
+  TEST_ASSERT_FALSE(positionWatcher.shouldFetch());
+  positionWatcher.start(75);
+  TEST_ASSERT_TRUE(positionWatcher.shouldFetch());
+}
+
+void test_position_watcher_should_not_fetch_before_polling_interval()
+{
+  When(Method(ArduinoFake(), millis)).AlwaysReturn(10000);
+
+  PositionWatcher positionWatcher = PositionWatcher();
+  
+  TEST_ASSERT_FALSE(positionWatcher.shouldFetch());
+  positionWatcher.start(75);
+  TEST_ASSERT_TRUE(positionWatcher.shouldFetch());
+  positionWatcher.fetchQueued();
+  TEST_ASSERT_FALSE(positionWatcher.shouldFetch());
+}
+
+void test_position_watcher_should_fetch_after_polling_interval()
+{
+  When(Method(ArduinoFake(), millis)).AlwaysReturn(10000);
+
+  PositionWatcher positionWatcher = PositionWatcher();
+  
+  TEST_ASSERT_FALSE(positionWatcher.shouldFetch());
+  positionWatcher.start(75);
+  TEST_ASSERT_TRUE(positionWatcher.shouldFetch());
+  positionWatcher.fetchQueued();
+  TEST_ASSERT_FALSE(positionWatcher.shouldFetch());
+
+  When(Method(ArduinoFake(), millis)).AlwaysReturn(20000);
+
+  TEST_ASSERT_TRUE(positionWatcher.shouldFetch());
+}
+
+void test_position_watcher_should_not_fetch_after_max_attempts()
+{
+  When(Method(ArduinoFake(), millis)).AlwaysReturn(10000);
+
+  PositionWatcher positionWatcher = PositionWatcher();
+  
+  TEST_ASSERT_FALSE(positionWatcher.shouldFetch());
+  positionWatcher.start(75);
+  TEST_ASSERT_TRUE(positionWatcher.shouldFetch());
+  positionWatcher.fetchQueued();
+  TEST_ASSERT_FALSE(positionWatcher.shouldFetch());
+
+  When(Method(ArduinoFake(), millis)).AlwaysReturn(20000);
+
+  TEST_ASSERT_TRUE(positionWatcher.shouldFetch());
+  positionWatcher.fetchQueued();
+
+  When(Method(ArduinoFake(), millis)).AlwaysReturn(30000);
+
+  TEST_ASSERT_TRUE(positionWatcher.shouldFetch());
+  positionWatcher.fetchQueued();
+
+  When(Method(ArduinoFake(), millis)).AlwaysReturn(40000);
+
+  TEST_ASSERT_FALSE(positionWatcher.shouldFetch());
+}
+
+void test_position_watcher_should_stop_after_enough_duplicates()
+{
+  When(Method(ArduinoFake(), millis)).AlwaysReturn(10000);
+
+  PositionWatcher positionWatcher = PositionWatcher();
+  
+  TEST_ASSERT_FALSE(positionWatcher.shouldFetch());
+  positionWatcher.start(75);
+  TEST_ASSERT_TRUE(positionWatcher.shouldFetch());
+  positionWatcher.fetchQueued();
+  positionWatcher.fetchReceived(50);
+  TEST_ASSERT_TRUE(positionWatcher.isWatching());
+  positionWatcher.fetchQueued();
+  positionWatcher.fetchReceived(60);
+  TEST_ASSERT_TRUE(positionWatcher.isWatching());
+  positionWatcher.fetchQueued();
+  positionWatcher.fetchReceived(60);
+  TEST_ASSERT_FALSE(positionWatcher.isWatching());
+}
+
+void test_position_watcher_should_stop_after_target_value()
+{
+  When(Method(ArduinoFake(), millis)).AlwaysReturn(10000);
+
+  PositionWatcher positionWatcher = PositionWatcher();
+  
+  TEST_ASSERT_FALSE(positionWatcher.shouldFetch());
+  positionWatcher.start(75);
+  TEST_ASSERT_TRUE(positionWatcher.shouldFetch());
+  positionWatcher.fetchQueued();
+  positionWatcher.fetchReceived(50);
+  TEST_ASSERT_TRUE(positionWatcher.isWatching());
+  positionWatcher.fetchQueued();
+  positionWatcher.fetchReceived(60);
+  TEST_ASSERT_TRUE(positionWatcher.isWatching());
+  positionWatcher.fetchQueued();
+  positionWatcher.fetchReceived(75);
+  TEST_ASSERT_FALSE(positionWatcher.isWatching());
+}
+
+int runUnityTests(void)
+{
+  UNITY_BEGIN();
+  RUN_TEST(test_position_watcher_should_not_fetch_initially);
+  RUN_TEST(test_position_watcher_should_fetch_after_start);
+  RUN_TEST(test_position_watcher_should_not_fetch_before_polling_interval);
+  RUN_TEST(test_position_watcher_should_fetch_after_polling_interval);
+  RUN_TEST(test_position_watcher_should_not_fetch_after_max_attempts);
+  RUN_TEST(test_position_watcher_should_stop_after_enough_duplicates);
+  RUN_TEST(test_position_watcher_should_stop_after_target_value);
+  return UNITY_END();
+}
+
+/**
+ * For Arduino framework
+ */
+
+void setup()
+{
+  // Wait ~2 seconds before the Unity test runner
+  // establishes connection with a board Serial interface
+  delay(2000);
+
+  runUnityTests();
+}
+void loop() {}
+
+int main(void)
+{
+  return runUnityTests();
+}

test/test_shade_repository/test_shade_repository.cpp

@@ -13,7 +13,7 @@ void tearDown()
 void test_shade_is_found_with_id()
 {
   ShadeRepository shadeRepository = ShadeRepository();
-  shadeRepository.upsert(Shade{0xABCD, "test_shade", "Test Shade", -1, -1, 0, 0, nullptr});
+  shadeRepository.upsert(Shade{0xABCD, "test_shade", "Test Shade", "stopped", -1, -1});
 
   auto shade = shadeRepository.findById(0xABCD);
   
@@ -26,7 +26,7 @@ void test_shade_is_found_with_id()
 void test_shade_is_found_with_key()
 {
   ShadeRepository shadeRepository = ShadeRepository();
-  shadeRepository.upsert(Shade{0xABCD, "test_shade", "Test Shade", -1, -1, 0, 0, nullptr});
+  shadeRepository.upsert(Shade{0xABCD, "test_shade", "Test Shade", "stopped", -1, -1});
 
   auto shade = shadeRepository.findByKey("test_shade");
   
@@ -39,8 +39,8 @@ void test_shade_is_found_with_key()
 void test_adding_shade_twice_only_added_once()
 {
   ShadeRepository shadeRepository = ShadeRepository();
-  shadeRepository.upsert(Shade{0xABCD, "test_shade", "Test Shade", -1, -1, 0, 0, nullptr});
+  shadeRepository.upsert(Shade{0xABCD, "test_shade", "Test Shade", "stopped", -1, -1});
-  shadeRepository.upsert(Shade{0xABCD, "test_shade", "Test Shade", -1, -1, 0, 0, nullptr});
+  shadeRepository.upsert(Shade{0xABCD, "test_shade", "Test Shade", "stopped", -1, -1});
 
   int count = 0;
 
@@ -54,13 +54,13 @@ void test_adding_shade_twice_only_added_once()
 void test_updating_shade_id()
 {
   ShadeRepository shadeRepository = ShadeRepository();
-  shadeRepository.upsert(Shade{0xABCD, "test_shade", "Test Shade", -1, -1, 0, 0, nullptr});
+  shadeRepository.upsert(Shade{0xABCD, "test_shade", "Test Shade", "stopped", -1, -1});
 
   auto shade1 = shadeRepository.findById(0xABCD);
 
   TEST_ASSERT_EQUAL_HEX16(0xABCD, shade1->ID);
 
-  shadeRepository.upsert(Shade{0x1234, "test_shade", "Test Shade", -1, -1, 0, 0, nullptr});
+  shadeRepository.upsert(Shade{0x1234, "test_shade", "Test Shade", "stopped", -1, -1});
 
   auto shade2 = shadeRepository.findById(0x1234);
 
@@ -74,13 +74,13 @@ void test_updating_shade_id()
 void test_updating_shade_friendly_name()
 {
   ShadeRepository shadeRepository = ShadeRepository();
-  shadeRepository.upsert(Shade{0xABCD, "test_shade", "Test Shade", -1, -1, 0, 0, nullptr});
+  shadeRepository.upsert(Shade{0xABCD, "test_shade", "Test Shade", "stopped", -1, -1});
 
   auto shade1 = shadeRepository.findByKey("test_shade");
 
   TEST_ASSERT_EQUAL_STRING("Test Shade", shade1->friendlyName.c_str());
 
-  shadeRepository.upsert(Shade{0xABCD, "test_shade", "Updated Test Shade", -1, -1, 0, 0, nullptr});
+  shadeRepository.upsert(Shade{0xABCD, "test_shade", "Updated Test Shade", "stopped", -1, -1});
 
   auto shade2 = shadeRepository.findByKey("test_shade");
 
@@ -101,7 +101,7 @@ void test_shade_added_callback()
     TEST_ASSERT_EQUAL_STRING("test_shade", shade.key.c_str());
   });
 
-  shadeRepository.upsert(Shade{0xABCD, "test_shade", "Test Shade", -1, -1, 0, 0, nullptr});
+  shadeRepository.upsert(Shade{0xABCD, "test_shade", "Test Shade", "stopped", -1, -1});
 
   TEST_ASSERT_EQUAL_INT(1, callbackInvokedCount);
 }
@@ -119,8 +119,8 @@ void test_shade_changed_callback()
     TEST_ASSERT_EQUAL_STRING("test_shade", shade.key.c_str());
   });
 
-  shadeRepository.upsert(Shade{0xABCD, "test_shade", "Test Shade", -1, -1, 0, 0, nullptr});
+  shadeRepository.upsert(Shade{0xABCD, "test_shade", "Test Shade", "stopped", -1, -1});
-  shadeRepository.upsert(Shade{0x1234, "test_shade", "Updated Test Shade", -1, -1, 0, 0, nullptr});
+  shadeRepository.upsert(Shade{0x1234, "test_shade", "Updated Test Shade", "stopped", -1, -1});
 
   TEST_ASSERT_EQUAL_INT(1, callbackInvokedCount);
 }