More refactorings, cleanup

components/ratgdo/common.h

@@ -10,22 +10,26 @@
 namespace esphome {
 namespace ratgdo {
 
+namespace protocol {
+
 struct SetRollingCodeCounter { uint32_t counter; };
 struct GetRollingCodeCounter {};
-struct RollingCodeCounter { observable<uint32_t>* counter; };
 struct SetClientID { uint64_t client_id; };
+struct QueryStatus{};
+struct QueryOpenings{};
 struct ActivateLearn {};
 struct InactivateLearn {};
 
 
 // a poor man's sum-type, because C++
-class ProtocolArgs {
+class Args {
 public:
     union {
        SetRollingCodeCounter set_rolling_code_counter;
        GetRollingCodeCounter get_rolling_code_counter;
-       RollingCodeCounter rolling_code_counter;
        SetClientID set_client_id;
+       QueryStatus query_status;
+       QueryOpenings query_openings;
        ActivateLearn activate_learn;
        InactivateLearn inactivate_learn;
     } value;
@@ -33,36 +37,57 @@ public:
     enum class Tag {
         set_rolling_code_counter,
         get_rolling_code_counter,
-        rolling_code_counter,
         set_client_id,
+        query_status,
+        query_openings,
         activate_learn,
         inactivate_learn,
-        void_,
     } tag;
 
-    ProtocolArgs(): tag(Tag::void_) {
+    Args(GetRollingCodeCounter&& arg): tag(Tag::get_rolling_code_counter) {
-    }
-
-    ProtocolArgs(GetRollingCodeCounter&& arg): tag(Tag::get_rolling_code_counter) {
         value.get_rolling_code_counter = std::move(arg);
     }
-    ProtocolArgs(SetRollingCodeCounter&& arg): tag(Tag::set_rolling_code_counter) {
+    Args(SetRollingCodeCounter&& arg): tag(Tag::set_rolling_code_counter) {
         value.set_rolling_code_counter = std::move(arg);
     }
-    ProtocolArgs(RollingCodeCounter&& arg): tag(Tag::rolling_code_counter) {
+    Args(SetClientID&& arg): tag(Tag::set_client_id) {
-        value.rolling_code_counter = std::move(arg);
-    }
-    ProtocolArgs(SetClientID&& arg): tag(Tag::set_client_id) {
         value.set_client_id = std::move(arg);
     }
-    ProtocolArgs(ActivateLearn&& arg): tag(Tag::activate_learn) {
+    Args(QueryStatus&& arg): tag(Tag::query_status) {
+        value.query_status = std::move(arg);
+    }
+    Args(QueryOpenings&& arg): tag(Tag::query_openings) {
+        value.query_openings = std::move(arg);
+    }
+    Args(ActivateLearn&& arg): tag(Tag::activate_learn) {
         value.activate_learn = std::move(arg);
     }
-    ProtocolArgs(InactivateLearn&& arg): tag(Tag::inactivate_learn) {
+    Args(InactivateLearn&& arg): tag(Tag::inactivate_learn) {
         value.inactivate_learn = std::move(arg);
     }
 };
 
 
+struct RollingCodeCounter { observable<uint32_t>* value; };
+
+class Result {
+public:
+    union {
+       RollingCodeCounter rolling_code_counter;
+    } value;
+
+    enum class Tag {
+        rolling_code_counter,
+        void_,
+    } tag;
+
+    Result(): tag(Tag::void_) {
+    }
+    Result(RollingCodeCounter&& arg): tag(Tag::rolling_code_counter) {
+        value.rolling_code_counter = std::move(arg);
+    }
+};
+
+} // namespace protocol
 } // namespace ratgdo
 } // namespace esphome

components/ratgdo/protocol.h

@@ -23,9 +23,8 @@ namespace ratgdo {
         virtual void light_action(LightAction action);
         virtual void lock_action(LockAction action);
         virtual void door_action(DoorAction action);
-        virtual void query_action(QueryAction action);
 
-        virtual ProtocolArgs call(ProtocolArgs args);
+        virtual protocol::Result call(protocol::Args args);
     };
     
 } // namespace ratgdo

components/ratgdo/ratgdo.cpp

@@ -25,18 +25,10 @@
 namespace esphome {
 namespace ratgdo {
 
+    using namespace protocol;
+
     static const char* const TAG = "ratgdo";
     static const int SYNC_DELAY = 1000;
-    //
-    // MAX_CODES_WITHOUT_FLASH_WRITE is a bit of a guess
-    // since we write the flash at most every every 5s
-    //
-    // We want the rolling counter to be high enough that the
-    // GDO will accept the command after an unexpected reboot
-    // that did not save the counter to flash in time which
-    // results in the rolling counter being behind what the GDO
-    // expects.
-
 
     void RATGDOComponent::setup()
     {
@@ -64,8 +56,7 @@ namespace ratgdo {
 
 
     // initializing protocol, this gets called before setup() because 
-    // the protocol_ member must be initialized before setup() because it children
+    // its children components might require that
-    // components might require that
     void RATGDOComponent::init_protocol()
     {
 #ifdef PROTOCOL_SECPLUSV2
@@ -380,12 +371,12 @@ namespace ratgdo {
     void RATGDOComponent::query_status()
     {
         ESP_LOG2(TAG, "Query status action");
-        this->protocol_->query_action(QueryAction::STATUS);
+        this->protocol_->call(QueryStatus{});
     }
 
     void RATGDOComponent::query_openings()
     {
-        this->protocol_->query_action(QueryAction::OPENINGS);
+        this->protocol_->call(QueryOpenings{});
     }
 
     void RATGDOComponent::sync()
@@ -570,8 +561,8 @@ namespace ratgdo {
         // change update to children is defered until after component loop
         // if multiple changes occur during component loop, only the last one is notified
         auto counter = this->protocol_->call(GetRollingCodeCounter{});
-        if (counter.tag==ProtocolArgs::Tag::rolling_code_counter) {
+        if (counter.tag==Result::Tag::rolling_code_counter) {
-            counter.value.rolling_code_counter.counter->subscribe([=](uint32_t state) { defer("rolling_code_counter", [=] { f(state); }); });
+            counter.value.rolling_code_counter.value->subscribe([=](uint32_t state) { defer("rolling_code_counter", [=] { f(state); }); });
         }
     }
     void RATGDOComponent::subscribe_opening_duration(std::function<void(float)>&& f)

components/ratgdo/ratgdo_state.h

@@ -99,11 +99,6 @@ namespace ratgdo {
         (STOP, 3),
         (UNKNOWN, 4))
 
-    ENUM(QueryAction, uint8_t,
-        (STATUS, 0),
-        (OPENINGS, 1),
-        (UNKNOWN, 2))
-
     struct Openings {
         uint16_t count;
         uint8_t flag;

components/ratgdo/secplus1.cpp

@@ -134,12 +134,8 @@ namespace secplus1 {
         // }
     }
 
-    void Secplus1::query_action(QueryAction action)
-    {
-    }
 
-
+    Result Secplus1::call(Args args) 
-    ProtocolArgs Secplus1::call(ProtocolArgs args) 
     {
         return {};
     }

components/ratgdo/secplus1.h

@@ -16,6 +16,8 @@ namespace esphome {
 namespace ratgdo {
 namespace secplus1 {
 
+    using namespace esphome::ratgdo::protocol;
+
     static const uint8_t RX_LENGTH = 2;
     typedef uint8_t RxPacket[RX_LENGTH];
 
@@ -60,9 +62,8 @@ namespace secplus1 {
         void light_action(LightAction action);
         void lock_action(LockAction action);
         void door_action(DoorAction action);
-        void query_action(QueryAction action);
 
-        ProtocolArgs call(ProtocolArgs args);
+        Result call(Args args);
 
     protected:
         void wall_panel_emulation(size_t index);

components/ratgdo/secplus2.cpp

@@ -14,6 +14,14 @@ namespace esphome {
 namespace ratgdo {
 namespace secplus2 {
 
+    // MAX_CODES_WITHOUT_FLASH_WRITE is a bit of a guess
+    // since we write the flash at most every every 5s
+    //
+    // We want the rolling counter to be high enough that the
+    // GDO will accept the command after an unexpected reboot
+    // that did not save the counter to flash in time which
+    // results in the rolling counter being behind what the GDO
+    // expects.
     static const uint8_t MAX_CODES_WITHOUT_FLASH_WRITE = 10;
 
     static const char* const TAG = "ratgdo_secplus2";
@@ -132,22 +140,16 @@ namespace secplus2 {
         this->door_command(action);
     }
 
-    void Secplus2::query_action(QueryAction action)
+
+    Result Secplus2::call(Args args)
     {
-        if (action == QueryAction::STATUS) {
+        using Tag = Args::Tag;
+        if (args.tag == Tag::query_status) {
             this->send_command(CommandType::GET_STATUS);
-        } else if (action == QueryAction::OPENINGS) {
+        } else if (args.tag == Tag::query_openings) {
             this->send_command(CommandType::GET_OPENINGS);
-        }
+        } else if (args.tag == Tag::get_rolling_code_counter) {
-    }
+            return Result(RollingCodeCounter{std::addressof(this->rolling_code_counter_)});
-
-
-    ProtocolArgs Secplus2::call(ProtocolArgs args)
-    {
-        using Tag = ProtocolArgs::Tag;
-
-        if (args.tag == Tag::get_rolling_code_counter) {
-            return ProtocolArgs(RollingCodeCounter{std::addressof(this->rolling_code_counter_)});
         } else if (args.tag == Tag::set_rolling_code_counter) {
             this->set_rolling_code_counter(args.value.set_rolling_code_counter.counter);
         } else if (args.tag == Tag::set_client_id) {

components/ratgdo/secplus2.h

@@ -19,6 +19,8 @@ namespace ratgdo {
 
 namespace secplus2 {
 
+    using namespace esphome::ratgdo::protocol;
+
     static const uint8_t PACKET_LENGTH = 19;
     typedef uint8_t WirePacket[PACKET_LENGTH];
 
@@ -80,9 +82,8 @@ namespace secplus2 {
         void light_action(LightAction action);
         void lock_action(LockAction action);
         void door_action(DoorAction action);
-        void query_action(QueryAction action);
 
-        ProtocolArgs call(ProtocolArgs args);
+        Result call(Args args);
 
         void increment_rolling_code_counter(int delta = 1);
         void set_rolling_code_counter(uint32_t counter);