From 827f4711da288f51be5f7e0e4c0bec8b9f025aca Mon Sep 17 00:00:00 2001
From: Ilya Zhuravlev <whatever@xyz.is>
Date: Tue, 6 Jul 2021 00:40:33 -0400
Subject: [PATCH] qmk_settings: refactor to keep integers internally

---
 src/main/python/keyboard_comm.py | 14 ++++----
 src/main/python/qmk_settings.py  | 57 +++++++++++++++++++-------------
 2 files changed, 41 insertions(+), 30 deletions(-)

diff --git a/src/main/python/keyboard_comm.py b/src/main/python/keyboard_comm.py
index 5420e9f..0d875ac 100644
--- a/src/main/python/keyboard_comm.py
+++ b/src/main/python/keyboard_comm.py
@@ -401,16 +401,14 @@ class Keyboard:
 
         for qsid in self.supported_settings:
             from qmk_settings import QmkSettings
-            width = QmkSettings.setting_width(qsid)
-            if width is None:
+
+            if not QmkSettings.is_qsid_supported(qsid):
                 continue
 
             data = self.usb_send(self.dev, struct.pack("<BBH", CMD_VIA_VIAL_PREFIX, CMD_VIAL_QMK_SETTINGS_GET, qsid),
                                  retries=20)
-            if data[0] != 0:
-                self.settings[qsid] = None
-            else:
-                self.settings[qsid] = data[1:1+width]
+            if data[0] == 0:
+                self.settings[qsid] = QmkSettings.qsid_deserialize(qsid, data[1:])
 
     def reload_dynamic(self):
         if self.vial_protocol < 4:
@@ -720,8 +718,10 @@ class Keyboard:
         return [self.macro_deserialize(x) for x in macros]
 
     def qmk_settings_set(self, qsid, value):
+        from qmk_settings import QmkSettings
         self.settings[qsid] = value
-        data = self.usb_send(self.dev, struct.pack("<BBH", CMD_VIA_VIAL_PREFIX, CMD_VIAL_QMK_SETTINGS_SET, qsid) + value,
+        data = self.usb_send(self.dev, struct.pack("<BBH", CMD_VIA_VIAL_PREFIX, CMD_VIAL_QMK_SETTINGS_SET, qsid)
+                             + QmkSettings.qsid_serialize(qsid, value),
                              retries=20)
         return data[0]
 
diff --git a/src/main/python/qmk_settings.py b/src/main/python/qmk_settings.py
index 5252743..f0ac6c0 100644
--- a/src/main/python/qmk_settings.py
+++ b/src/main/python/qmk_settings.py
@@ -41,8 +41,8 @@ class BooleanOption(GenericOption):
         self.container.addWidget(self.checkbox, self.row, 1)
 
     def reload(self, keyboard):
-        data = super().reload(keyboard)
-        checked = data[0] & (1 << self.qsid_bit)
+        value = super().reload(keyboard)
+        checked = value & (1 << self.qsid_bit)
 
         self.checkbox.blockSignals(True)
         self.checkbox.setChecked(checked != 0)
@@ -69,11 +69,10 @@ class IntegerOption(GenericOption):
         self.container.addWidget(self.spinbox, self.row, 1)
 
     def reload(self, keyboard):
-        data = super().reload(keyboard)[0:self.option["width"]]
-        self.spinbox.setValue(int.from_bytes(data, byteorder="little"))
+        self.spinbox.setValue(super().reload(keyboard))
 
     def value(self):
-        return self.spinbox.value().to_bytes(self.option["width"], byteorder="little")
+        return self.spinbox.value()
 
     def delete(self):
         super().delete()
@@ -173,15 +172,8 @@ class QmkSettings(BasicEditor):
         qsid_values = defaultdict(int)
         for tab in self.tabs:
             for field in tab:
-                # hack for boolean options - we pack several booleans into a single byte
-                if isinstance(field, BooleanOption):
-                    qsid_values[field.qsid] |= field.value()
-                else:
-                    qsid_values[field.qsid] = field.value()
-
+                qsid_values[field.qsid] |= field.value()
         for qsid, value in qsid_values.items():
-            if isinstance(value, int):
-                value = value.to_bytes(1, byteorder="little")
             self.keyboard.qmk_settings_set(qsid, value)
 
     def reset_settings(self):
@@ -198,19 +190,38 @@ class QmkSettings(BasicEditor):
 
     @classmethod
     def initialize(cls, appctx):
-        cls.settings_widths = dict()
+        cls.qsid_fields = defaultdict(list)
         with open(appctx.get_resource("qmk_settings.json"), "r") as inf:
             cls.settings_defs = json.load(inf)
         for tab in cls.settings_defs["tabs"]:
             for field in tab["fields"]:
-                if field["type"] == "boolean":
-                    width = 1
-                elif field["type"] == "integer":
-                    width = field["width"]
-                else:
-                    raise RuntimeError("unsupported field type: {}".format(field))
-                cls.settings_widths[field["qsid"]] = width
+                cls.qsid_fields[field["qsid"]].append(field)
 
     @classmethod
-    def setting_width(cls, qsid):
-        return cls.settings_widths.get(qsid)
+    def is_qsid_supported(cls, qsid):
+        """ Return whether this qsid is supported by the settings editor """
+        return qsid in cls.qsid_fields
+
+    @classmethod
+    def qsid_serialize(cls, qsid, data):
+        """ Serialize from internal representation into binary that can be sent to the firmware """
+        fields = cls.qsid_fields[qsid]
+        if fields[0]["type"] == "boolean":
+            assert isinstance(data, int)
+            return data.to_bytes(1, byteorder="little")
+        elif fields[0]["type"] == "integer":
+            assert isinstance(data, int)
+            assert len(fields) == 1
+            return data.to_bytes(fields[0]["width"], byteorder="little")
+
+    @classmethod
+    def qsid_deserialize(cls, qsid, data):
+        """ Deserialize from binary received from firmware into internal representation """
+        fields = cls.qsid_fields[qsid]
+        if fields[0]["type"] == "boolean":
+            return data[0]
+        elif fields[0]["type"] == "integer":
+            assert len(fields) == 1
+            return int.from_bytes(data[0:fields[0]["width"]], byteorder="little")
+        else:
+            raise RuntimeError("unsupported field")