rfbridge: handle rfb{ON,OFF} just like any other setting

removes the extra 'save' button from the ui and the code that handles it
also make sure to use the updated foreach_prefix when looking for the
match in the settings

resolve xoseperez#2502

code/espurna/rfbridge.cpp

@@ -331,6 +331,60 @@ void _rfbLearnImpl();
 void _rfbReceiveImpl();
 void _rfbSendImpl(const RfbMessage& message);
 
+#if RELAY_SUPPORT
+
+namespace rfbridge {
+namespace settings {
+namespace keys {
+
+alignas(4) static constexpr char On[] PROGMEM = "rfbON";
+alignas(4) static constexpr char Off[] PROGMEM = "rfbOFF";
+
+} // namespace keys
+
+String off(size_t id) {
+    return getSetting({FPSTR(keys::Off), id});
+}
+
+String on(size_t id) {
+    return getSetting({FPSTR(keys::On), id});
+}
+
+void store(const __FlashStringHelper* prefix, size_t id, const String& value) {
+    SettingsKey key { prefix, id };
+    setSetting(key, value);
+    DEBUG_MSG_P(PSTR("[RF] Saved %s => \"%s\"\n"), key.c_str(), value.c_str());
+}
+
+void off(size_t id, const String& value) {
+    store(FPSTR(keys::Off), id, value);
+}
+
+void on(size_t id, const String& value) {
+    store(FPSTR(keys::On), id, value);
+}
+
+} // namespace settings
+} // namespace rfbridge
+
+void _rfbStore(size_t id, bool status, const String& code) {
+    if (status) {
+        rfbridge::settings::on(id, code);
+    } else {
+        rfbridge::settings::off(id, code);
+    }
+}
+
+String _rfbRetrieve(size_t id, bool status) {
+    if (status) {
+        return rfbridge::settings::on(id);
+    } else {
+        return rfbridge::settings::off(id);
+    }
+}
+
+#endif
+
 // -----------------------------------------------------------------------------
 // WEBUI INTEGRATION
 // -----------------------------------------------------------------------------
@@ -354,8 +408,8 @@ void _rfbWebSocketSendCodeArray(JsonObject& root, size_t start, size_t size) {
 
     for (auto id = start; id < (start + size); ++id) {
         JsonArray& pair = codes.createNestedArray();
-        pair.add(rfbRetrieve(id, false));
-        pair.add(rfbRetrieve(id, true));
+        pair.add(rfbridge::settings::off(id));
+        pair.add(rfbridge::settings::on(id));
     }
 }
 
@@ -376,11 +430,30 @@ void _rfbWebSocketOnConnected(JsonObject& root) {
 #endif
 }
 
-void _rfbWebSocketOnAction(uint32_t client_id, const char * action, JsonObject& data) {
+void _rfbWebSocketOnAction(uint32_t client_id, const char* action, JsonObject& data) {
 #if RELAY_SUPPORT
-    if (strcmp(action, "rfblearn") == 0) rfbLearn(data["id"], data["status"]);
-    if (strcmp(action, "rfbforget") == 0) rfbForget(data["id"], data["status"]);
-    if (strcmp(action, "rfbsend") == 0) rfbStore(data["id"], data["status"], data["data"].as<const char*>());
+    if (strncmp(action, "rfb", 3) != 0) {
+        return;
+    }
+
+    auto idValue = data[F("id")];
+    if (!idValue.success()) {
+        return;
+    }
+
+    auto statusValue = data[F("status")];
+    if (!statusValue.success()) {
+        return;
+    }
+
+    const size_t id { idValue.as<size_t>() };
+    const size_t status { statusValue.as<bool>() };
+
+    if (STRING_VIEW("rfblearn") == action) {
+        rfbLearn(id, status);
+    } else if (STRING_VIEW("rfbforget") == action) {
+        rfbForget(id, status);
+    }
 #endif
 }
 
@@ -421,67 +494,66 @@ bool _rfbCompare(const char* lhs, const char* rhs, size_t length) {
 // previous implementation tried to help MQTT / API requests to match based on the saved code,
 // thus requiring us to 'return' value from settings as the real code, replacing input
 RfbRelayMatch _rfbMatch(const char* code) {
+    RfbRelayMatch matched;
 
     if (!relayCount()) {
-        return {};
+        return matched;
     }
 
-    const auto len = strlen(code);
+    ::settings::StringView codeView { code };
 
     // we gather all available options, as the kv store might be defined in any order
     // scan kvs only once, since we want both ON and OFF options and don't want to depend on the relayCount()
-    RfbRelayMatch matched;
-
-    settings::internal::foreach([code, len, &matched](settings::kvs_type::KeyValueResult&& kv) {
-        const auto key = kv.key.read();
-        PayloadStatus status = key.startsWith(F("rfbON"))
-            ? PayloadStatus::On : key.startsWith(F("rfbOFF"))
-            ? PayloadStatus::Off : PayloadStatus::Unknown;
-
-        if (PayloadStatus::Unknown == status) {
-            return;
-        }
+    settings::internal::foreach_prefix(
+        [codeView, &matched](settings::StringView prefix, String key, const settings::kvs_type::ReadResult& value) {
+            if (codeView.length() != value.length()) {
+                return;
+            }
 
-        const auto value = kv.value.read();
-        if (len != value.length()) {
-            return;
-        }
+            PayloadStatus status {
+                (prefix.c_str() == &rfbridge::settings::keys::On[0]) ? PayloadStatus::On :
+                (prefix.c_str() == &rfbridge::settings::keys::Off[0]) ? PayloadStatus::Off :
+                PayloadStatus::Unknown };
 
-        if (!_rfbCompare(code, value.c_str(), len)) {
-            return;
-        }
+            if (PayloadStatus::Unknown == status) {
+                return;
+            }
 
-        // note: strlen is constexpr here
-        const char* id_ptr = key.c_str() + (
-            (PayloadStatus::On == status) ? strlen("rfbON") : strlen("rfbOFF"));
-        if (*id_ptr == '\0') {
-            return;
-        }
+            if (!_rfbCompare(codeView.c_str(), value.read().c_str(), codeView.length())) {
+                return;
+            }
 
-        size_t id;
-        if (!tryParseId(id_ptr, relayCount, id)) {
-            return;
-        }
+            const char* id_ptr = key.c_str() + prefix.length();
+            if (*id_ptr == '\0') {
+                return;
+            }
 
-        // when we see the same id twice, we match the opposite statuses
-        if (matched && (id == matched.id())) {
-            matched.reset(matched.id(), PayloadStatus::Toggle);
-            return;
-        }
+            size_t id;
+            if (!tryParseId(id_ptr, relayCount, id)) {
+                return;
+            }
 
-        matched.reset(matched ? std::min(id, matched.id()) : id, status);
-    });
+            // when we see the same id twice, we match the opposite statuses
+            if (matched && (id == matched.id())) {
+                matched.reset(matched.id(), PayloadStatus::Toggle);
+                return;
+            }
 
+            matched.reset(matched ? std::min(id, matched.id()) : id, status);
+        },
+        {
+            rfbridge::settings::keys::On,
+            rfbridge::settings::keys::Off
+        });
 
     return matched;
-
 }
 
 void _rfbLearnFromString(std::unique_ptr<RfbLearn>& learn, const char* buffer) {
     if (!learn) return;
 
     DEBUG_MSG_P(PSTR("[RF] Learned relay ID %u after %u ms\n"), learn->id, millis() - learn->ts);
-    rfbStore(learn->id, learn->status, buffer);
+    _rfbStore(learn->id, learn->status, buffer);
 
     // Websocket update needs to happen right here, since the only time
     // we send these in bulk is at the very start of the connection
@@ -1161,26 +1233,24 @@ void _rfbInitCommands() {
 // PUBLIC
 // -----------------------------------------------------------------------------
 
-void rfbStore(size_t id, bool status, const char * code) {
-    SettingsKey key { status ? F("rfbON") : F("rfbOFF"), id };
-    setSetting(key, code);
-    DEBUG_MSG_P(PSTR("[RF] Saved %s => \"%s\"\n"), key.c_str(), code);
+#if RELAY_SUPPORT
+
+void rfbStore(size_t id, bool status, String code) {
+    _rfbStore(id, status, std::move(code));
 }
 
 String rfbRetrieve(size_t id, bool status) {
-    return getSetting({ status ? F("rfbON") : F("rfbOFF"), id });
+    return _rfbRetrieve(id, status);
 }
 
-#if RELAY_SUPPORT
-
 void rfbStatus(size_t id, bool status) {
     // TODO: This is a left-over from the old implementation. Right now we set this lock when relay handler
     //       is called within the receiver, while this is called from either relayStatus or relay loop calling
     //       this via provider callback. This prevents us from re-sending the code we just received.
     // TODO: Consider having 'origin' of the relay change. Either supply relayStatus with an additional arg,
     //       or track these statuses directly.
     if (!_rfb_relay_status_lock[id]) {
-        rfbSend(rfbRetrieve(id, status));
+        rfbSend(_rfbRetrieve(id, status));
     }
 
     _rfb_relay_status_lock[id] = false;

code/espurna/rfbridge.h

@@ -21,7 +21,7 @@ void rfbStatus(size_t id, bool status);
 void rfbLearn(size_t id, bool status);
 
 String rfbRetrieve(size_t id, bool status);
-void rfbStore(size_t id, bool status, const char* code);
+void rfbStore(size_t id, bool status, String code);
 
 void rfbForget(size_t id, bool status);
 void rfbSetup();