settings: clean-up helper classes & functions

Move inline classes into a separate file.
Make serialize() into a basic function, support numeric conversions with base option
Refactor numeric conversions and add some more helper functions for the
build flags.

code/espurna/button.cpp

@@ -48,8 +48,7 @@ debounce_event::types::Mode convert(const String& value) {
     }
 }
 
-template<>
-String serialize(const debounce_event::types::Mode& value) {
+String serialize(debounce_event::types::Mode value) {
     String result;
     switch (value) {
         case debounce_event::types::Mode::Switch:
@@ -76,8 +75,7 @@ debounce_event::types::PinValue convert(const String& value) {
     }
 }
 
-template<>
-String serialize(const debounce_event::types::PinValue& value) {
+String serialize(debounce_event::types::PinValue value) {
     String result;
     switch (value) {
         case debounce_event::types::PinValue::Low:
@@ -106,8 +104,7 @@ debounce_event::types::PinMode convert(const String& value) {
     }
 }
 
-template<>
-String serialize(const debounce_event::types::PinMode& mode) {
+String serialize(debounce_event::types::PinMode mode) {
     String result;
     switch (mode) {
         case debounce_event::types::PinMode::InputPullup:

code/espurna/debug.cpp

@@ -315,8 +315,7 @@ void debugSetup() {
 namespace settings {
 namespace internal {
 
-template<>
-String serialize(const DebugLogMode& value) {
+String serialize(DebugLogMode value) {
     String result;
     switch (value) {
         case DebugLogMode::Disabled:

code/espurna/relay.cpp

@@ -12,6 +12,7 @@ Copyright (C) 2016-2019 by Xose Pérez <xose dot perez at gmail dot com>
 
 #include <Ticker.h>
 #include <ArduinoJson.h>
+
 #include <bitset>
 #include <cstring>
 #include <functional>
@@ -30,30 +31,8 @@ Copyright (C) 2016-2019 by Xose Pérez <xose dot perez at gmail dot com>
 #include "libs/BasePin.h"
 #include "relay_config.h"
 
-// Relay statuses are kept in a mutable bitmask struct
-// TODO: u32toString should be convert(...) ?
-
 namespace {
 
-String u32toString(uint32_t value, int base) {
-    String result;
-    result.reserve(32 + 2);
-
-    if (base == 2) {
-        result += "0b";
-    } else if (base == 8) {
-        result += "0o";
-    } else if (base == 16) {
-        result += "0x";
-    }
-
-    char buffer[33] = {0};
-    ultoa(value, buffer, base);
-    result += buffer;
-
-    return result;
-}
-
 using RelayMask = std::bitset<RelaysMax>;
 
 struct RelayMaskHelper {
@@ -72,7 +51,7 @@ struct RelayMaskHelper {
     }
 
     String toString() const {
-        return u32toString(toUnsigned(), 2);
+        return settings::internal::serialize(toUnsigned(), 2);
     }
 
     const RelayMask& mask() const {
@@ -95,8 +74,6 @@ private:
     RelayMask _mask { 0ul };
 };
 
-} // namespace
-
 template <typename T>
 T _relayPayloadToTristate(const char* payload) {
     auto len = strlen(payload);
@@ -146,6 +123,8 @@ const char* _relayLockToPayload(RelayLock lock) {
     return _relayTristateToPayload(lock);
 }
 
+} // namespace
+
 namespace settings {
 namespace internal {
 
@@ -219,8 +198,7 @@ RelayMaskHelper convert(const String& value) {
     return RelayMaskHelper(convert<unsigned long>(value));
 }
 
-template <>
-String serialize(const RelayMaskHelper& mask) {
+String serialize(RelayMaskHelper mask) {
     return mask.toString();
 }
 
@@ -274,15 +252,15 @@ public:
 };
 
 std::vector<relay_t> _relays;
-bool _relayRecursive = false;
-size_t _relayDummy = 0;
+bool _relayRecursive { false };
+size_t _relayDummy { 0ul };
 
-unsigned long _relay_flood_window = (1000 * RELAY_FLOOD_WINDOW);
-unsigned long _relay_flood_changes = RELAY_FLOOD_CHANGES;
+unsigned long _relay_flood_window { _relayFloodWindowMs() };
+unsigned long _relay_flood_changes { _relayFloodChanges() };
 
 unsigned long _relay_delay_interlock;
-unsigned char _relay_sync_mode = RELAY_SYNC_ANY;
-bool _relay_sync_locked = false;
+int _relay_sync_mode { RELAY_SYNC_ANY };
+bool _relay_sync_locked { false };
 
 Ticker _relay_save_timer;
 Ticker _relay_sync_timer;
@@ -637,10 +615,10 @@ void _relayHandleStatus(unsigned char id, PayloadStatus status) {
     }
 }
 
-bool _relayHandlePayload(unsigned char relayID, const char* payload) {
+bool _relayHandlePayload(unsigned char id, const char* payload) {
     auto status = relayParsePayload(payload);
     if (status != PayloadStatus::Unknown) {
-        _relayHandleStatus(relayID, status);
+        _relayHandleStatus(id, status);
         return true;
     }
 
@@ -648,8 +626,8 @@ bool _relayHandlePayload(unsigned char relayID, const char* payload) {
     return false;
 }
 
-bool _relayHandlePayload(unsigned char relayID, const String& payload) {
-    return _relayHandlePayload(relayID, payload.c_str());
+bool _relayHandlePayload(unsigned char id, const String& payload) {
+    return _relayHandlePayload(id, payload.c_str());
 }
 
 bool _relayHandlePulsePayload(unsigned char id, const char* payload) {
@@ -837,7 +815,7 @@ void relayPulse(unsigned char id) {
         relay.pulseTicker->once_ms(ms, relayToggle, id);
         // Reconfigure after dynamic pulse
         relay.pulse = getSetting({"relayPulse", id}, _relayPulseMode(id));
-        relay.pulse_ms = static_cast<unsigned long>(1000.0 * getSetting({"relayTime", id}, _relayPulseTime(id)));
+        relay.pulse_ms = static_cast<unsigned long>(1000.0f * getSetting({"relayTime", id}, _relayPulseTime(id)));
     }
 
 }
@@ -926,11 +904,11 @@ bool relayStatus(unsigned char id, bool status, bool report, bool group_report)
 }
 
 bool relayStatus(unsigned char id, bool status) {
-    #if MQTT_SUPPORT
-        return relayStatus(id, status, mqttForward(), true);
-    #else
-        return relayStatus(id, status, false, true);
-    #endif
+#if MQTT_SUPPORT
+    return relayStatus(id, status, mqttForward(), true);
+#else
+    return relayStatus(id, status, false, true);
+#endif
 }
 
 bool relayStatus(unsigned char id) {
@@ -1031,16 +1009,17 @@ void relaySave() {
 }
 
 void relayToggle(unsigned char id, bool report, bool group_report) {
-    if (id >= _relays.size()) return;
-    relayStatus(id, !relayStatus(id), report, group_report);
+    if (id < _relays.size()) {
+        relayStatus(id, !relayStatus(id), report, group_report);
+    }
 }
 
 void relayToggle(unsigned char id) {
-    #if MQTT_SUPPORT
-        relayToggle(id, mqttForward(), true);
-    #else
-        relayToggle(id, false, true);
-    #endif
+#if MQTT_SUPPORT
+    relayToggle(id, mqttForward(), true);
+#else
+    relayToggle(id, false, true);
+#endif
 }
 
 size_t relayCount() {
@@ -1153,14 +1132,15 @@ void _relayBootAll() {
 
     bool once { true };
     static RelayMask done;
-    for (unsigned char id = 0; id < relayCount(); ++id) {
+    auto relays = relayCount();
+    for (decltype(relays) id = 0; id < relays; ++id) {
         if (done[id]) {
             continue;
         }
 
         if (once) {
             DEBUG_MSG_P(PSTR("[RELAY] Number of relays: %u, boot mask: %s\n"),
-                _relays.size(), mask.toString().c_str());
+                relays, mask.toString().c_str());
             once = false;
         }
 
@@ -1172,25 +1152,26 @@ void _relayBootAll() {
 }
 
 void _relayConfigure() {
-    for (unsigned char i = 0, relays = _relays.size() ; (i < relays); ++i) {
-        _relays[i].pulse = getSetting({"relayPulse", i}, _relayPulseMode(i));
-        _relays[i].pulse_ms = static_cast<unsigned long>(1000.0 * getSetting({"relayTime", i}, _relayPulseTime(i)));
+    auto relays = _relays.size();
+    for (decltype(relays) id = 0; id < relays; ++id) {
+        _relays[id].pulse = getSetting({"relayPulse", id}, _relayPulseMode(id));
+        _relays[id].pulse_ms = static_cast<unsigned long>(1000.0f * getSetting({"relayTime", i}, _relayPulseTime(i)));
 
-        _relays[i].delay_on = getSetting({"relayDelayOn", i}, _relayDelayOn(i));
-        _relays[i].delay_off = getSetting({"relayDelayOff", i}, _relayDelayOff(i));
+        _relays[id].delay_on = getSetting({"relayDelayOn", id}, _relayDelayOn(id));
+        _relays[id].delay_off = getSetting({"relayDelayOff", id}, _relayDelayOff(id));
     }
 
-    _relay_flood_window = (1000 * getSetting("relayFloodTime", RELAY_FLOOD_WINDOW));
-    _relay_flood_changes = getSetting("relayFloodChanges", RELAY_FLOOD_CHANGES);
+    _relay_flood_window = (1000.0f * getSetting("relayFloodTime", _relayFloodWindow()));
+    _relay_flood_changes = getSetting("relayFloodChanges", _relayFloodChanges());
 
-    _relay_delay_interlock = getSetting("relayIlkDelay", RELAY_DELAY_INTERLOCK);
-    _relay_sync_mode = getSetting("relaySync", RELAY_SYNC);
+    _relay_delay_interlock = getSetting("relayIlkDelay", _relayInterlockDelay());
+    _relay_sync_mode = getSetting("relaySync", _relaySyncMode());
 
     #if MQTT_SUPPORT || API_SUPPORT
         settingsProcessConfig({
-            {_relay_rpc_payload_on,     "relayPayloadOn",     RELAY_MQTT_ON},
-            {_relay_rpc_payload_off,    "relayPayloadOff",    RELAY_MQTT_OFF},
-            {_relay_rpc_payload_toggle, "relayPayloadToggle", RELAY_MQTT_TOGGLE},
+            {_relay_rpc_payload_on,     "relayPayloadOn",     _relayMqttPayloadOn()},
+            {_relay_rpc_payload_off,    "relayPayloadOff",    _relayMqttPayloadOff()},
+            {_relay_rpc_payload_toggle, "relayPayloadToggle", _relayMqttPayloadToggle()},
         });
     #endif // MQTT_SUPPORT
 }
@@ -1213,9 +1194,10 @@ void _relayWebSocketUpdate(JsonObject& root) {
     JsonArray& lock = state.createNestedArray("lock");
 
     // Note: we use byte instead of bool to ever so slightly compress json output
-    for (unsigned char i=0; i<relayCount(); i++) {
-        status.add<uint8_t>(_relays[i].target_status);
-        lock.add(static_cast<uint8_t>(_relays[i].lock));
+    auto relays = relayCount();
+    for (decltype(relays) id = 0; id < relays; ++id) {
+        status.add(_relays[id].target_status ? 1 : 0);
+        lock.add(static_cast<uint8_t>(_relays[id].lock));
     }
 }
 
@@ -1284,8 +1266,8 @@ void _relayWebSocketOnVisible(JsonObject& root) {
 
     if (relayCount() > 1) {
         root["multirelayVisible"] = 1;
-        root["relaySync"] = getSetting("relaySync", RELAY_SYNC);
-        root["relayIlkDelay"] = getSetting("relayIlkDelay", RELAY_DELAY_INTERLOCK);
+        root["relaySync"] = static_cast<uint8_t>(getSetting("relaySync", _relaySyncMode()));
+        root["relayIlkDelay"] = getSetting("relayIlkDelay", _relayInterlockDelay());
     }
 
     root["relayVisible"] = 1;
@@ -1860,7 +1842,8 @@ void _relayProcess(bool mode) {
 
     bool changed = false;
 
-    for (unsigned char id = 0; id < _relays.size(); id++) {
+    auto relays = _relays.size();
+    for (decltype(relays) id = 0; id < relays; ++id) {
 
         bool target = _relays[id].target_status;
 
@@ -1902,9 +1885,17 @@ void _relayProcess(bool mode) {
     }
 
     // Whenever we are using sync modes and any relay had changed the state, check if we can unlock
-    const bool needs_unlock = ((_relay_sync_mode == RELAY_SYNC_NONE_OR_ONE) || (_relay_sync_mode == RELAY_SYNC_ONE));
-    if (_relay_sync_locked && needs_unlock && changed) {
-        _relaySyncUnlock();
+    switch (_relay_sync_mode) {
+    case RELAY_SYNC_ONE:
+    case RELAY_SYNC_NONE_OR_ONE
+        if (_relay_sync_locked && changed) {
+            _relaySyncUnlock();
+        }
+        break;
+    case RELAY_SYNC_ANY:
+    case RELAY_SYNC_SAME:
+    case RELAY_SYNC_FIRST:
+        break;
     }
 }
 

code/espurna/relay_config.h

@@ -8,7 +8,29 @@ RELAY MODULE
 
 #include "espurna.h"
 
-constexpr double _relayPulseTime(unsigned char index) {
+constexpr int _relaySyncMode() {
+    return RELAY_SYNC;
+}
+
+constexpr float _relayFloodWindow() {
+    return RELAY_FLOOD_WINDOW;
+}
+
+static_assert(_relayFloodWindow() >= 0.0f, "");
+
+constexpr unsigned long _relayFloodWindowMs() {
+    return static_cast<unsigned long>(_relayFloodWindow() * 1000.0f);
+}
+
+constexpr unsigned long _relayFloodChanges() {
+    return RELAY_FLOOD_CHANGES;
+}
+
+constexpr unsigned long _relayInterlockDelay() {
+    return RELAY_DELAY_INTERLOCK;
+}
+
+constexpr float _relayPulseTime(unsigned char index) {
     return (
         (index == 0) ? RELAY1_PULSE_TIME :
         (index == 1) ? RELAY2_PULSE_TIME :
@@ -21,6 +43,15 @@ constexpr double _relayPulseTime(unsigned char index) {
     );
 }
 
+static_assert(_relayPulseTime(0) >= 0.0f, "");
+static_assert(_relayPulseTime(1) >= 0.0f, "");
+static_assert(_relayPulseTime(2) >= 0.0f, "");
+static_assert(_relayPulseTime(3) >= 0.0f, "");
+static_assert(_relayPulseTime(4) >= 0.0f, "");
+static_assert(_relayPulseTime(5) >= 0.0f, "");
+static_assert(_relayPulseTime(6) >= 0.0f, "");
+static_assert(_relayPulseTime(7) >= 0.0f, "");
+
 constexpr RelayPulse _relayPulseMode(unsigned char index) {
     return (
         (index == 0) ? RELAY1_PULSE_MODE :
@@ -43,7 +74,7 @@ constexpr unsigned long _relayDelayOn(unsigned char index) {
         (index == 4) ? RELAY5_DELAY_ON :
         (index == 5) ? RELAY6_DELAY_ON :
         (index == 6) ? RELAY7_DELAY_ON :
-        (index == 7) ? RELAY8_DELAY_ON : 0
+        (index == 7) ? RELAY8_DELAY_ON : 0ul
     );
 }
 
@@ -56,7 +87,7 @@ constexpr unsigned long _relayDelayOff(unsigned char index) {
         (index == 4) ? RELAY5_DELAY_OFF :
         (index == 5) ? RELAY6_DELAY_OFF :
         (index == 6) ? RELAY7_DELAY_OFF :
-        (index == 7) ? RELAY8_DELAY_OFF : 0
+        (index == 7) ? RELAY8_DELAY_OFF : 0ul
     );
 }
 
@@ -121,7 +152,7 @@ constexpr int _relayBootMode(unsigned char index) {
         (index == 4) ? RELAY5_BOOT_MODE :
         (index == 5) ? RELAY6_BOOT_MODE :
         (index == 6) ? RELAY7_BOOT_MODE :
-        (index == 7) ? RELAY8_BOOT_MODE : GPIO_NONE
+        (index == 7) ? RELAY8_BOOT_MODE : RELAY_BOOT_OFF
     );
 }
 
@@ -151,7 +182,19 @@ constexpr RelayMqttTopicMode _relayMqttTopicMode(unsigned char index) {
     );
 }
 
-constexpr const char* _relayMqttTopicSub(unsigned char index) {
+constexpr const char* const _relayMqttPayloadOn() {
+    return RELAY_MQTT_ON;
+}
+
+constexpr const char* const _relayMqttPayloadOff() {
+    return RELAY_MQTT_OFF;
+}
+
+constexpr const char* const _relayMqttPayloadToggle() {
+    return RELAY_MQTT_TOGGLE;
+}
+
+constexpr const char* const _relayMqttTopicSub(unsigned char index) {
     return (
         (index == 0) ? (RELAY1_MQTT_TOPIC_SUB) :
         (index == 1) ? (RELAY2_MQTT_TOPIC_SUB) :
@@ -164,7 +207,7 @@ constexpr const char* _relayMqttTopicSub(unsigned char index) {
     );
 }
 
-constexpr const char* _relayMqttTopicPub(unsigned char index) {
+constexpr const char* const _relayMqttTopicPub(unsigned char index) {
     return (
         (index == 0) ? (RELAY1_MQTT_TOPIC_PUB) :
         (index == 1) ? (RELAY2_MQTT_TOPIC_PUB) :

code/espurna/rfbridge.cpp

@@ -1088,7 +1088,6 @@ void _rfbInitCommands() {
 
 #if RELAY_SUPPORT
     terminalRegisterCommand(F("RFB.LEARN"), [](const terminal::CommandContext& ctx) {
-
         if (ctx.argc != 3) {
             terminalError(ctx, F("RFB.LEARN <ID> <STATUS>"));
             return;
@@ -1229,13 +1228,14 @@ void _rfbSettingsMigrate(int version) {
 
     String buffer;
 
-    for (unsigned char index = 0; index < relayCount(); ++index) {
-        SettingsKey on_key {F("rfbON"), index};
+    auto relays = relayCount();
+    for (decltype(relays) id = 0; id < relays; ++id) {
+        SettingsKey on_key {F("rfbON"), id};
         if (migrate_code(buffer, getSetting(on_key))) {
             setSetting(on_key, buffer);
         }
 
-        SettingsKey off_key {F("rfbOFF"), index};
+        SettingsKey off_key {F("rfbOFF"), id};
         if (migrate_code(buffer, getSetting(off_key))) {
             setSetting(off_key, buffer);
         }

code/espurna/sensor.cpp

@@ -2365,21 +2365,25 @@ namespace internal {
 
 template <>
 sensor::Unit convert(const String& string) {
-    const int value = string.toInt();
-    if ((value > static_cast<int>(sensor::Unit::Min_)) && (value < static_cast<int>(sensor::Unit::Max_))) {
-        return static_cast<sensor::Unit>(value);
+    auto len = value.length();
+    if (len && isNumber(value)) {
+        constexpr int Min { static_cast<int>(sensor::Unit::Min_) };
+        constexpr int Max { static_cast<int>(sensor::Unit::Max_) };
+        auto num = convert<int>(value);
+        if ((Min < num) && (num < Max)) {
+            return static_cast<sensor::Unit>(num);
+        }
     }
 
     return sensor::Unit::None;
 }
 
-template <>
-String serialize(const sensor::Unit& unit) {
-    return String(static_cast<int>(unit));
+String serialize(sensor::Unit unit) {
+    return serialize(static_cast<int>(unit));
 }
 
-} // ns settings::internal
-} // ns settings
+} // namespace internal
+} // namespace settings
 
 void _sensorConfigure() {
 
@@ -2654,36 +2658,32 @@ String magnitudeTopicIndex(unsigned char index) {
 
 // -----------------------------------------------------------------------------
 
-void _sensorBackwards() {
-
+void _sensorBackwards(int version) {
     // Some keys from older versions were longer
-    moveSetting("powerUnits", "pwrUnits");
-    moveSetting("energyUnits", "eneUnits");
+    if (version < 3) {
+        moveSetting("powerUnits", "pwrUnits");
+        moveSetting("energyUnits", "eneUnits");
+    }
 
     // Energy is now indexed (based on magnitude.index_global)
-    moveSetting("eneTotal", "eneTotal0");
-
-	// Update PZEM004T energy total across multiple devices
-    moveSettings("pzEneTotal", "eneTotal");
+	// Also update PZEM004T energy total across multiple devices
+    if (version < 5) {
+        moveSetting("eneTotal", "eneTotal0");
+        moveSettings("pzEneTotal", "eneTotal");
+    }
 
     // Unit ID is no longer shared, drop when equal to Min_ or None
-    const char *keys[3] = {
-        "pwrUnits", "eneUnits", "tmpUnits"
-    };
-
-    for (auto* key : keys) {
-        const auto units = getSetting(key);
-        if (units.length() && (units.equals("0") || units.equals("1"))) {
-            delSetting(key);
-        }
+    if (version < 5) {
+        delSetting("pwrUnits");
+        delSetting("eneUnits");
+        delSetting("tmpUnits");
     }
-
 }
 
 void sensorSetup() {
 
     // Settings backwards compatibility
-    _sensorBackwards();
+    _sensorBackwards(migrateVersion());
 
     // Load configured sensors and set up all of magnitudes
     _sensorLoad();

code/espurna/settings.cpp

@@ -73,6 +73,16 @@ double convert(const String& value) {
     return atof(value.c_str());
 }
 
+template <>
+signed char convert(const String& value) {
+    return value.toInt();
+}
+
+template <>
+short convert(const String& value) {
+    return value.toInt();
+}
+
 template <>
 int convert(const String& value) {
     return value.toInt();
@@ -105,28 +115,57 @@ bool convert(const String& value) {
 }
 
 template <>
-unsigned long convert(const String& value) {
+uint32_t convert(const String& value) {
     if (!value.length()) {
         return 0;
     }
 
     int base = 10;
     if (value.length() > 2) {
-        if (value.startsWith("0b")) {
-            base = 2;
-        } else if (value.startsWith("0o")) {
-            base = 8;
-        } else if (value.startsWith("0x")) {
-            base = 16;
+        auto* ptr = value.c_str();
+        if (*ptr == '0') {
+            switch (*(ptr + 1)) {
+            case 'b':
+                base = 2;
+                break;
+            case 'o':
+                base = 8;
+                break;
+            case 'x':
+                base = 16;
+                break;
+            }
         }
     }
 
     return u32fromString((base == 10) ? value : value.substring(2), base);
 }
 
+String serialize(uint32_t value, int base) {
+    constexpr size_t Size { 4 * sizeof(decltype(value)) };
+    constexpr size_t Length { Size - 1 };
+
+    String result;
+    result.reserve(Length);
+
+    if (base == 2) {
+        result += "0b";
+    } else if (base == 8) {
+        result += "0o";
+    } else if (base == 16) {
+        result += "0x";
+    }
+
+    char buffer[Size] = {0};
+    ultoa(value, buffer, base);
+    result += buffer;
+
+    return result;
+}
+
 template <>
-unsigned int convert(const String& value) {
-    return convert<unsigned long>(value);
+unsigned long convert(const String& value) {
+    return convert<unsigned int>(value);
 }
 
 template <>
@@ -142,68 +181,11 @@ unsigned char convert(const String& value) {
 } // namespace settings::internal
 } // namespace settings
 
+// -----------------------------------------------------------------------------
+// Key-value API
 // -----------------------------------------------------------------------------
 
-/*
-struct SettingsKeys {
-
-    struct iterator {
-        iterator(size_t total) :
-            total(total)
-        {}
-
-        iterator& operator++() {
-            if (total && (current_index < (total - 1))) {
-                ++current_index
-                current_value = settingsKeyName(current_index);
-                return *this;
-            }
-            return end();
-        }
-
-        iterator operator++(int) {
-            iterator val = *this;
-            ++(*this);
-            return val;
-        }
-
-        operator String() {
-            return (current_index < total) ? current_value : empty_value;
-        }
-
-        bool operator ==(iterator& const other) const {
-            return (total == other.total) && (current_index == other.current_index);
-        }
-
-        bool operator !=(iterator& const other) const {
-            return !(*this == other);
-        }
-
-        using difference_type = size_t;
-        using value_type = size_t;
-        using pointer = const size_t*;
-        using reference = const size_t&;
-        using iterator_category = std::forward_iterator_tag;
-
-        const size_t total;
-
-        String empty_value;
-        String current_value;
-        size_t current_index = 0;
-    };
-
-    iterator begin() {
-        return iterator {total};
-    }
-
-    iterator end() {
-        return iterator {0};
-    }
-
-};
-*/
-
-// Note: we prefer things sorted via this function, not kv_store.keys() directly
+// TODO: UI needs this to avoid showing keys in storage order
 std::vector<String> settingsKeys() {
     auto keys = settings::kv_store.keys();
     std::sort(keys.begin(), keys.end(), [](const String& rhs, const String& lhs) -> bool {
@@ -229,10 +211,6 @@ String settingsQueryDefaults(const String& key) {
     return String();
 }
 
-// -----------------------------------------------------------------------------
-// Key-value API
-// -----------------------------------------------------------------------------
-
 settings_move_key_t _moveKeys(const String& from, const String& to, unsigned char index) {
     return settings_move_key_t {{from, index}, {to, index}};
 }
@@ -291,7 +269,7 @@ template
 double getSetting(const SettingsKey& key, double defaultValue);
 
 String getSetting(const String& key) {
-    return settings::kv_store.get(key).value;
+    return std::move(settings::kv_store.get(key)).get();
 }
 
 String getSetting(const __FlashStringHelper* key) {
@@ -316,21 +294,21 @@ String getSetting(const SettingsKey& key, const __FlashStringHelper* defaultValu
 }
 
 String getSetting(const SettingsKey& key, const String& defaultValue) {
-    auto result = settings::kv_store.get(key.toString());
-    if (!result) {
-        result.value = defaultValue;
+    auto result = settings::kv_store.get(key.value());
+    if (result) {
+        return std::move(result).get();
     }
 
-    return result.value;
+    return defaultValue;
 }
 
 String getSetting(const SettingsKey& key, String&& defaultValue) {
-    auto result = settings::kv_store.get(key.toString());
-    if (!result) {
-        result.value = std::move(defaultValue);
+    auto result = settings::kv_store.get(key.value());
+    if (result) {
+        return std::move(result).get();
     }
 
-    return result.value;
+    return std::move(defaultValue);
 }
 
 bool delSetting(const String& key) {
@@ -338,7 +316,7 @@ bool delSetting(const String& key) {
 }
 
 bool delSetting(const SettingsKey& key) {
-    return delSetting(key.toString());
+    return delSetting(key.value());
 }
 
 bool delSetting(const char* key) {
@@ -354,7 +332,7 @@ bool hasSetting(const String& key) {
 }
 
 bool hasSetting(const SettingsKey& key) {
-    return hasSetting(key.toString());
+    return hasSetting(key.value());
 }
 
 bool hasSetting(const char* key) {
@@ -459,8 +437,9 @@ void settingsProcessConfig(const settings_cfg_list_t& config, settings_filter_t
 // Initialization
 // -----------------------------------------------------------------------------
 
-void settingsSetup() {
+#if TERMINAL_SUPPORT
 
+void _settingsInitCommands() {
     terminalRegisterCommand(F("CONFIG"), [](const terminal::CommandContext& ctx) {
         // TODO: enough of a buffer?
         DynamicJsonBuffer jsonBuffer(1024);
@@ -559,5 +538,12 @@ void settingsSetup() {
         terminalOK(ctx);
     });
 #endif
+}
+
+#endif
 
+void settingsSetup() {
+#if TERMINAL_SUPPORT
+    _settingsInitCommands();
+#endif
 }

code/espurna/settings.h

@@ -3,6 +3,7 @@
 SETTINGS MODULE
 
 Copyright (C) 2016-2019 by Xose Pérez <xose dot perez at gmail dot com>
+Copyright (C) 2020-2021 by Maxim Prokhorov <prokhorov dot max at outlook dot com>
 
 */
 
@@ -18,7 +19,10 @@ Copyright (C) 2016-2019 by Xose Pérez <xose dot perez at gmail dot com>
 #include <ArduinoJson.h>
 
 #include "storage_eeprom.h"
+
+#include "settings_helpers.h"
 #include "settings_embedis.h"
+#include "terminal.h"
 
 // --------------------------------------------------------------------------
 
@@ -53,67 +57,9 @@ extern kvs_type kv_store;
 
 // --------------------------------------------------------------------------
 
-class SettingsKey {
-public:
-    SettingsKey(const char* key) :
-        _key(key)
-    {}
-
-    SettingsKey(const String& key) :
-        _key(key)
-    {}
-
-    SettingsKey(String&& key) :
-        _key(std::move(key))
-    {}
-
-    SettingsKey(const String& prefix, unsigned char index) {
-        _key.reserve(prefix.length());
-        _key += prefix;
-        _key += index;
-    }
-
-    SettingsKey(String&& prefix, unsigned char index) :
-        _key(std::move(prefix))
-    {
-        _key += index;
-    }
-
-    SettingsKey(const char* prefix, unsigned char index) :
-        _key(prefix)
-    {
-        _key += index;
-    }
-
-    bool operator==(const char* other) const {
-        return _key == other;
-    }
-
-    bool operator==(const String& other) const {
-        return _key == other;
-    }
-
-    const String& toString() const {
-        return _key;
-    }
-
-    explicit operator String() const & {
-        return _key;
-    }
-
-    explicit operator String() && {
-        return std::move(_key);
-    }
-
-private:
-    String _key;
-};
-
 using settings_move_key_t = std::pair<SettingsKey, SettingsKey>;
 using settings_filter_t = std::function<String(String& value)>;
 
-// --------------------------------------------------------------------------
-
 struct settings_cfg_t {
     String& setting;
     const char* key;
@@ -138,19 +84,21 @@ using enable_if_not_arduino_string = std::enable_if<!is_arduino_string<T>::value
 
 // --------------------------------------------------------------------------
 
-uint32_t u32fromString(const String& string, int base);
-
 template <typename T>
 T convert(const String& value);
 
-// --------------------------------------------------------------------------
-
 template <>
 float convert(const String& value);
 
 template <>
 double convert(const String& value);
 
+template <>
+signed char convert(const String& value);
+
+template <>
+short convert(const String& value);
+
 template <>
 int convert(const String& value);
 
@@ -172,8 +120,51 @@ unsigned short convert(const String& value);
 template <>
 unsigned char convert(const String& value);
 
-template<typename T>
-String serialize(const T& value);
+inline String serialize(uint8_t value, int base = 10) {
+    return String(value, base);
+}
+
+inline String serialize(uint16_t value, int base = 10) {
+    return String(value, base);
+}
+
+String serialize(uint32_t value, int base = 10);
+
+inline String serialize(unsigned long value, int base = 10) {
+    static_assert(sizeof(unsigned long) == sizeof(uint32_t), "");
+    static_assert(sizeof(unsigned int) == sizeof(unsigned long), "");
+    return serialize(static_cast<unsigned int>(value), base);
+}
+
+inline String serialize(int16_t value, int base = 10) {
+    return String(value, base);
+}
+
+inline String serialize(int8_t value, int base = 10) {
+    return serialize(static_cast<int16_t>(value), base);
+}
+
+inline String serialize(long value, int base = 10) {
+    return String(value, base);
+}
+
+inline String serialize(int value, int base = 10) {
+    static_assert(sizeof(long) == sizeof(int32_t), "");
+    static_assert(sizeof(int) == sizeof(long), "");
+    return serialize(static_cast<long>(value), base);
+}
+
+inline String serialize(float value) {
+    return String(value, 3);
+}
+
+inline String serialize(double value) {
+    return String(value, 3);
+}
+
+inline String serialize(bool value) {
+    return value ? "true" : "false";
+}
 
 } // namespace internal
 } // namespace settings
@@ -202,11 +193,11 @@ T getSetting(const SettingsKey& key, T defaultValue) __attribute__((noinline));
 
 template <typename T, typename = typename settings::internal::enable_if_not_arduino_string<T>::type>
 T getSetting(const SettingsKey& key, T defaultValue) {
-    auto result = settings::kv_store.get(key.toString());
-    if (!result) {
-        return defaultValue;
+    auto result = settings::kv_store.get(key.value());
+    if (result) {
+        return settings::internal::convert<T>(result.ref());
     }
-    return settings::internal::convert<T>(result.value);
+    return defaultValue;
 }
 
 String getSetting(const char* key);
@@ -222,7 +213,7 @@ String getSetting(const SettingsKey& key, String&& defaultValue);
 
 template<typename T, typename = typename settings::internal::enable_if_arduino_string<T>::type>
 bool setSetting(const SettingsKey& key, T&& value) {
-    return settings::kv_store.set(key.toString(), value);
+    return settings::kv_store.set(key.value(), value);
 }
 
 template<typename T, typename = typename settings::internal::enable_if_not_arduino_string<T>::type>

code/espurna/settings_embedis.h

@@ -17,20 +17,12 @@ Reimplementation of the Embedis storage format:
 #include <memory>
 #include <vector>
 
+#include "settings_helpers.h"
 #include "libs/TypeChecks.h"
 
 namespace settings {
 namespace embedis {
 
-// 'optional' type for byte range
-struct ValueResult {
-    operator bool() {
-        return result;
-    }
-    bool result { false };
-    String value;
-};
-
 // Sum total is calculated from:
 // - 4 bytes to store length of 2 values (stored as big-endian)
 // - N bytes of values themselves
@@ -260,7 +252,7 @@ class KeyValueStore {
 
     // Internal storage consists of sequences of <byte-range><length>
     struct KeyValueResult {
-        operator bool() {
+        explicit operator bool() {
             return (key) && (value) && (key.length > 0);
         }
 
@@ -498,9 +490,10 @@ class KeyValueStore {
             auto key_result = kv.key.read();
             if (key_result == key) {
                 if (read_value) {
-                    out.value = kv.value.read();
+                    out = std::move(kv.value.read();
+                } else {
+                    out = String();
                 }
-                out.result = true;
                 break;
             }
         } while (_state != State::End);

code/espurna/settings_helpers.h

@@ -0,0 +1,186 @@
+/*
+
+Part of the SETTINGS module
+
+Copyright (C) 2016-2019 by Xose Pérez <xose dot perez at gmail dot com>
+Copyright (C) 2020-2021 by Maxim Prokhorov <prokhorov dot max at outlook dot com>
+
+*/
+
+#pragma once
+
+#include <Arduino.h>
+
+#include <utility>
+
+// --------------------------------------------------------------------------
+
+class SettingsKey {
+public:
+    SettingsKey(const char* key) :
+        _key(key)
+    {}
+
+    SettingsKey(const String& key) :
+        _key(key)
+    {}
+
+    SettingsKey(String&& key) :
+        _key(std::move(key))
+    {}
+
+    SettingsKey(const String& prefix, size_t index) {
+        _key.reserve(prefix.length() + 4);
+        _key += prefix;
+        _key += index;
+    }
+
+    SettingsKey(String&& prefix, size_t index) :
+        _key(std::move(prefix))
+    {
+        _key += index;
+    }
+
+    SettingsKey(const char* prefix, size_t index) :
+        _key(prefix)
+    {
+        _key += index;
+    }
+
+    const char* c_str() const {
+        return _key.c_str();
+    }
+
+    size_t length() const {
+        return _key.length();
+    }
+
+    bool operator==(const char* other) const {
+        return _key == other;
+    }
+
+    bool operator==(const String& other) const {
+        return _key == other;
+    }
+
+    const String& value() const {
+        return _key;
+    }
+
+    explicit operator String() const & {
+        return _key;
+    }
+
+    explicit operator String() && {
+        return std::move(_key);
+    }
+
+private:
+    String _key;
+};
+
+// --------------------------------------------------------------------------
+
+namespace settings {
+namespace internal {
+
+struct BasicSetting {
+    using Get = String(*)();
+
+    BasicSetting() = delete;
+    constexpr BasicSetting(const char* const key, Get get) :
+        _key(key),
+        _get(get)
+    {}
+
+    constexpr const char* const key() const {
+        return _key;
+    }
+
+    String get() const {
+        return _get();
+    }
+
+private:
+    const char* const _key;
+    Get _get;
+};
+
+struct IndexedSetting {
+    using Get = String(*)(size_t);
+
+    IndexedSetting() = delete;
+    constexpr IndexedSetting(const char* const prefix, Get get) :
+        _prefix(prefix),
+        _get(get)
+    {}
+
+    constexpr const char* const prefix() const {
+        return _prefix;
+    }
+
+    String get(size_t index) const {
+        return _get(index);
+    }
+
+private:
+    const char* const _prefix;
+    Get _get;
+};
+
+} // namespace internal
+
+// 'optional' type for byte range
+struct ValueResult {
+    ValueResult() = default;
+    ValueResult(const ValueResult&) = default;
+    ValueResult(ValueResult&&) = default;
+
+    explicit ValueResult(const String& value) :
+        _result(true),
+        _value(value)
+    {}
+
+    explicit ValueResult(String&& value) :
+        _result(true),
+        _value(std::move(value))
+    {}
+
+    template <typename T>
+    ValueResult& operator=(T&& value) {
+        if (!_result) {
+            _result = true;
+            _value = std::forward<T>(value);
+        }
+
+        return *this;
+    }
+
+    explicit operator bool() const {
+        return _result;
+    }
+
+    String get() && {
+        auto moved = std::move(_value);
+        _result = false;
+        return moved;
+    }
+
+    const String& ref() const {
+        return _value;
+    }
+
+    const char* c_str() const {
+        return _value.c_str();
+    }
+
+    size_t length() const {
+        return _value.length();
+    }
+
+private:
+    bool _result { false };
+    String _value;
+};
+
+} // namespace settings

code/test/unit/settings/main.cpp

@@ -133,8 +133,8 @@ template <typename T>
 void check_kv(T& instance, const String& key, const String& value) {
     auto result = instance.kvs.get(key);
     TEST_ASSERT_MESSAGE(static_cast<bool>(result), key.c_str());
-    TEST_ASSERT(result.value.length());
-    TEST_ASSERT_EQUAL_STRING(value.c_str(), result.value.c_str());
+    TEST_ASSERT(result.length());
+    TEST_ASSERT_EQUAL_STRING(value.c_str(), result.c_str());
 };
 
 void test_sizes() {
@@ -274,7 +274,7 @@ void test_small_gaps() {
     auto check_empty = [&instance](const String& key) {
         auto result = instance.kvs.get(key);
         TEST_ASSERT(static_cast<bool>(result));
-        TEST_ASSERT_FALSE(result.value.length());
+        TEST_ASSERT_FALSE(result.length());
     };
 
     check_empty("empty_again");
@@ -285,8 +285,8 @@ void test_small_gaps() {
     auto check_value = [&instance](const String& key, const String& value) {
         auto result = instance.kvs.get(key);
         TEST_ASSERT(static_cast<bool>(result));
-        TEST_ASSERT(result.value.length());
-        TEST_ASSERT_EQUAL_STRING(value.c_str(), result.value.c_str());
+        TEST_ASSERT(result.length());
+        TEST_ASSERT_EQUAL_STRING(value.c_str(), result.c_str());
     };
 
     check_value("finally", "avalue");
@@ -354,7 +354,7 @@ void test_basic() {
     for (auto& kv : kvs) {
         auto result = instance.kvs.get(kv.first);
         TEST_ASSERT(static_cast<bool>(result));
-        TEST_ASSERT_EQUAL_STRING(kv.second.c_str(), result.value.c_str());
+        TEST_ASSERT_EQUAL_STRING(kv.second.c_str(), result.c_str());
     }
 
 }
@@ -403,7 +403,7 @@ void test_storage() {
         TEST_ASSERT_EQUAL(0, slice.available());
         auto result = slice.get("key1");
         TEST_ASSERT(static_cast<bool>(result));
-        TEST_ASSERT_EQUAL_STRING("value1", result.value.c_str());
+        TEST_ASSERT_EQUAL_STRING("value1", result.c_str());
     }
 
     // ensure that right offset also works
@@ -414,7 +414,7 @@ void test_storage() {
         TEST_ASSERT_EQUAL((Size - kvsize - kvsize), slice.available());
         auto result = slice.get("key2");
         TEST_ASSERT(static_cast<bool>(result));
-        TEST_ASSERT_EQUAL_STRING("value2", result.value.c_str());
+        TEST_ASSERT_EQUAL_STRING("value2", result.c_str());
     }
 
     // ensure offset does not introduce offset bugs
@@ -431,8 +431,8 @@ void test_storage() {
         auto key1 = slice.get("key1");
         TEST_ASSERT(static_cast<bool>(key1));
 
-        String updated(key1.value);
-        for (size_t index = 0; index < key1.value.length(); ++index) {
+        String updated(key1.ref());
+        for (size_t index = 0; index < key1.length(); ++index) {
             updated[index] = 'A';
         }
 
@@ -443,11 +443,11 @@ void test_storage() {
 
         auto check_key1 = slice.get("key1");
         TEST_ASSERT(static_cast<bool>(check_key1));
-        TEST_ASSERT_EQUAL_STRING(updated.c_str(), check_key1.value.c_str());
+        TEST_ASSERT_EQUAL_STRING(updated.c_str(), check_key1.c_str());
 
         auto check_key2 = slice.get("key2");
         TEST_ASSERT(static_cast<bool>(check_key2));
-        TEST_ASSERT_EQUAL_STRING(updated.c_str(), check_key2.value.c_str());
+        TEST_ASSERT_EQUAL_STRING(updated.c_str(), check_key2.c_str());
 
         TEST_ASSERT_EQUAL(available - offset, slice.available());
     }