pre-process keyboard auto detection steps into API friendly format

Continuing the theme of previous work, this branch parses pc105.tree
into API-friendly attr-based classes at snap build time (which also lets
us check some constraints then and not at run time).

po/POTFILES.in

@@ -39,6 +39,7 @@ subiquity/common/serialize.py
 subiquity/common/tests/__init__.py
 subiquity/common/tests/test_filesystem.py
 subiquity/common/tests/test_keyboard.py
+subiquity/common/tests/test_serialization.py
 subiquity/common/types.py
 subiquitycore/async_helpers.py
 subiquitycore/contextlib38.py
@@ -165,7 +166,6 @@ subiquity/ui/views/__init__.py
 subiquity/ui/views/installprogress.py
 subiquity/ui/views/keyboard.py
 subiquity/ui/views/mirror.py
-subiquity/ui/views/pc105.py
 subiquity/ui/views/proxy.py
 subiquity/ui/views/refresh.py
 subiquity/ui/views/snaplist.py

scripts/make-kbd-info.py

@@ -4,13 +4,23 @@ from collections import defaultdict
 import os
 import shutil
 import subprocess
+import sys
+from typing import Dict
 
 from subiquity.common.serialize import Serializer
 from subiquity.common.types import (
+    AnyStep,
     KeyboardLayout,
     KeyboardVariant,
+    StepPressKey,
+    StepKeyPresent,
     )
 
+sys.path.insert(0, os.path.dirname(__file__))
+
+import pc105 # noqa
+
+
 tdir = os.path.join(os.environ.get('SNAPCRAFT_PART_INSTALL', '.'), 'kbds')
 if os.path.exists(tdir):
     shutil.rmtree(tdir)
@@ -57,3 +67,27 @@ for lang, layouts in lang_to_layouts.items():
                     "subiquity assumes all keyboard layouts have at least one "
                     "variant!")
             out.write(s.to_json(KeyboardLayout, layout) + "\n")
+
+
+pc105tree = pc105.PC105Tree()
+pc105tree.read_steps()
+
+if '0' not in pc105tree.steps:
+    raise Exception("subiquity assumes there is a step '0' in the pc105 steps")
+
+for index, step in pc105tree.steps.items():
+    if isinstance(step, StepPressKey):
+        if len(step.symbols) == 0 or len(step.keycodes) == 0:
+            raise Exception(f"step {index} {step} appears to be incomplete")
+        for v in step.keycodes.values():
+            if v not in pc105tree.steps:
+                raise Exception(
+                    f"step {index} {step} references missing step {v}")
+    elif isinstance(step, StepKeyPresent):
+        for v in step.yes, step.no:
+            if v not in pc105tree.steps:
+                raise Exception(
+                    f"step {index} {step} references missing step {v}")
+
+with open(os.path.join(tdir, 'pc105.json'), 'w') as out:
+    out.write(s.to_json(Dict[str, AnyStep], pc105tree.steps))

scripts/pc105.py

@@ -17,113 +17,94 @@
 # /usr/share/console-setup/pc105.tree. This code parses that data into
 # subclasses of Step.
 
+"""Parses the pc105.tree file into Steps"""
 
-class Step:
+from subiquity.common.types import (
-    def __repr__(self):
+    StepKeyPresent,
-        kvs = []
+    StepPressKey,
-        for k, v in self.__dict__.items():
+    StepResult,
-            kvs.append("%s=%r" % (k, v))
+    )
-        return "%s(%s)" % (self.__class__.__name__, ", ".join(sorted(kvs)))
-
-    def check(self):
-        pass
-
-
-class StepPressKey(Step):
-    # "Press one of the following keys"
-    def __init__(self):
-        self.symbols = []
-        self.keycodes = {}
-
-    def check(self):
-        if len(self.symbols) == 0 or len(self.keycodes) == 0:
-            raise Exception
-
-
-class StepKeyPresent(Step):
-    # "Is this symbol present on your keyboard"
-    def __init__(self, symbol):
-        self.symbol = symbol
-        self.yes = None
-        self.no = None
-
-    def check(self):
-        if self.yes is None or self.no is None:
-            raise Exception
-
-
-class StepResult(Step):
-    # "This is the autodetected layout"
-    def __init__(self, result):
-        self.result = result
 
 
 class PC105Tree:
-    """Parses the pc105.tree file into subclasses of Step"""
     # This is adapted (quite heavily) from the code in ubiquity.
 
     def __init__(self):
         self.steps = {}
 
     def _add_step_from_lines(self, lines):
-        step = None
+        step_cls = None
-        step_index = -1
+        args = None
+        step_index = None
         for line in lines:
             if line.startswith('STEP '):
-                step_index = int(line[5:])
+                step_index = line[5:].strip()
             elif line.startswith('PRESS '):
                 # Ask the user to press a character on the keyboard.
-                if step is None:
+                if step_cls is None:
-                    step = StepPressKey()
+                    step_cls = StepPressKey
-                elif not isinstance(step, StepPressKey):
+                    args = {
+                        'symbols': [],
+                        'keycodes': {},
+                        }
+                elif step_cls is not StepPressKey:
                     raise Exception
-                step.symbols.append(line[6:].strip())
+                args['symbols'].append(line[6:].strip())
             elif line.startswith('CODE '):
                 # Direct the evaluating code to process step ## next if the
                 # user has pressed a key which returned that keycode.
-                if not isinstance(step, StepPressKey):
+                if step_cls is not StepPressKey:
                     raise Exception
                 keycode = int(line[5:line.find(' ', 5)])
-                s = int(line[line.find(' ', 5) + 1:])
+                s = line[line.find(' ', 5) + 1:].strip()
-                step.keycodes[keycode] = s
+                args['keycodes'][keycode] = s
             elif line.startswith('FIND '):
                 # Ask the user whether that character is present on their
                 # keyboard.
-                if step is None:
+                if step_cls is None:
-                    step = StepKeyPresent(line[5:].strip())
+                    step_cls = StepKeyPresent
+                    args = {
+                        'symbol': line[5:].strip(),
+                        }
                 else:
                     raise Exception
             elif line.startswith('FINDP '):
                 # Equivalent to FIND, except that the user is asked to consider
                 # only the primary symbols (i.e. Plain and Shift).
-                if step is None:
+                if step_cls is None:
-                    step = StepKeyPresent(line[6:].strip())
+                    step_cls = StepKeyPresent
+                    args = {'symbol': line[5:].strip()}
                 else:
                     raise Exception
             elif line.startswith('YES '):
                 # Direct the evaluating code to process step ## next if the
                 # user does have this key.
-                if not isinstance(step, StepKeyPresent):
+                if step_cls is not StepKeyPresent:
                     raise Exception
-                step.yes = int(line[4:].strip())
+                args['yes'] = line[4:].strip()
             elif line.startswith('NO '):
                 # Direct the evaluating code to process step ## next if the
                 # user does not have this key.
-                if not isinstance(step, StepKeyPresent):
+                if step_cls is not StepKeyPresent:
                     raise Exception
-                step.no = int(line[3:].strip())
+                args['no'] = line[3:].strip()
             elif line.startswith('MAP '):
                 # This step uniquely identifies a keymap.
-                if step is None:
+                if step_cls is None:
-                    step = StepResult(line[4:].strip())
+                    step_cls = StepResult
+                    arg = line[4:].strip()
+                    if ':' in arg:
+                        layout, variant = arg.split(':')
+                    else:
+                        layout, variant = arg, ''
+                    args = {'layout': layout, 'variant': variant}
                 else:
                     raise Exception
             else:
                 raise Exception
-        if step is None or step_index == -1:
+        if step_cls is None or step_index is None:
             raise Exception
-        step.check()
+        self.steps[step_index] = step_cls(**args)
-        self.steps[step_index] = step
 
     def read_steps(self):
         cur_step_lines = []

subiquity/client/keyboard.py

@@ -14,9 +14,14 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import os
+from typing import Dict
 
 from subiquity.common.serialize import Serializer
-from subiquity.common.types import KeyboardLayout, KeyboardSetting
+from subiquity.common.types import (
+    AnyStep,
+    KeyboardLayout,
+    KeyboardSetting,
+    )
 
 
 # Non-latin keyboard layouts that are handled in a uniform way
@@ -133,3 +138,8 @@ class KeyboardList:
     def _clear(self):
         self.current_lang = None
         self.layouts = []
+
+    def load_pc105(self):
+        path = os.path.join(self._kbnames_dir, 'pc105.json')
+        with open(path) as fp:
+            return self.serializer.from_json(Dict[str, AnyStep], fp.read())

subiquity/common/types.py

@@ -20,7 +20,7 @@
 import datetime
 import enum
 import shlex
-from typing import List, Optional
+from typing import Dict, List, Optional, Union
 
 import attr
 
@@ -92,6 +92,31 @@ class RefreshStatus:
     new_snap_version: str = ''
 
 
+@attr.s(auto_attribs=True)
+class StepPressKey:
+    # "Press a key with one of the following symbols"
+    symbols: List[str]
+    keycodes: Dict[int, str]
+
+
+@attr.s(auto_attribs=True)
+class StepKeyPresent:
+    # "Is this symbol present on your keyboard"
+    symbol: str
+    yes: str
+    no: str
+
+
+@attr.s(auto_attribs=True)
+class StepResult:
+    # "This is the autodetected layout"
+    layout: str
+    variant: str
+
+
+AnyStep = Union[StepPressKey, StepKeyPresent, StepResult]
+
+
 @attr.s(auto_attribs=True)
 class KeyboardSetting:
     layout: str

subiquity/ui/views/keyboard.py

@@ -46,15 +46,18 @@ from subiquitycore.ui.utils import button_pile, Color, Padding, screen
 from subiquitycore.view import BaseView
 
 from subiquity.client.keyboard import for_ui, latinizable
-from subiquity.common.types import KeyboardSetting
+from subiquity.common.types import (
-from subiquity.ui.views import pc105
+    KeyboardSetting,
+    StepKeyPresent,
+    StepPressKey,
+    StepResult,
+    )
 
 log = logging.getLogger("subiquity.ui.views.keyboard")
 
 
 class AutoDetectBase(WidgetWrap):
     def __init__(self, keyboard_detector, step):
-        # step is an instance of pc105.Step
         self.keyboard_detector = keyboard_detector
         self.step = step
         lb = LineBox(
@@ -82,7 +85,7 @@ class AutoDetectBase(WidgetWrap):
 class AutoDetectIntro(AutoDetectBase):
 
     def ok(self, sender):
-        self.keyboard_detector.do_step(0)
+        self.keyboard_detector.do_step("0")
 
     def cancel(self, sender):
         self.keyboard_detector.abort()
@@ -100,19 +103,6 @@ class AutoDetectIntro(AutoDetectBase):
                 ])
 
 
-class AutoDetectFailed(AutoDetectBase):
-
-    def ok(self, sender):
-        self.keyboard_detector.abort()
-
-    def make_body(self):
-        return Pile([
-                Text(_("Keyboard auto detection failed, sorry")),
-                Text(""),
-                button_pile([ok_btn(label="OK", on_press=self.ok)]),
-                ])
-
-
 class AutoDetectResult(AutoDetectBase):
 
     preamble = _("""\
@@ -127,17 +117,16 @@ another layout or run the automated detection again.
 
 """)
 
+    @property
+    def _kview(self):
+        return self.keyboard_detector.keyboard_view
+
     def ok(self, sender):
-        self.keyboard_detector.keyboard_view.found_layout(
+        self._kview.found_layout(self.layout, self.variant)
-            self.layout, self.variant)
 
     def make_body(self):
-        if ':' in self.step.result:
+        self.layout, self.variant = self._kview.lookup(
-            layout_code, variant_code = self.step.result.split(':')
+            self.step.layout, self.step.variant)
-        else:
-            layout_code, variant_code = self.step.result, ""
-        view = self.keyboard_detector.keyboard_view
-        self.layout, self.variant = view.lookup(layout_code, variant_code)
         layout_text = _("Layout")
         var_text = _("Variant")
         width = max(len(layout_text), len(var_text), 12)
@@ -239,8 +228,7 @@ class Detector:
 
     def __init__(self, kview):
         self.keyboard_view = kview
-        self.pc105tree = pc105.PC105Tree()
+        self.pc105_steps = kview.keyboard_list.load_pc105()
-        self.pc105tree.read_steps()
         self.seen_steps = []
 
     def start(self):
@@ -252,9 +240,9 @@ class Detector:
         self.keyboard_view.remove_overlay()
 
     step_cls_to_view_cls = {
-        pc105.StepResult: AutoDetectResult,
+        StepResult: AutoDetectResult,
-        pc105.StepPressKey: AutoDetectPressKey,
+        StepPressKey: AutoDetectPressKey,
-        pc105.StepKeyPresent: AutoDetectKeyPresent,
+        StepKeyPresent: AutoDetectKeyPresent,
         }
 
     def backup(self):
@@ -275,14 +263,10 @@ class Detector:
         self.abort()
 
         log.debug("moving to step %s", step_index)
-        try:
+        step = self.pc105_steps[step_index]
-            step = self.pc105tree.steps[step_index]
+        self.seen_steps.append(step_index)
-        except KeyError:
+        log.debug("step: %s", repr(step))
-            self.overlay = AutoDetectFailed(self, None)
+        self.overlay = self.step_cls_to_view_cls[type(step)](self, step)
-        else:
-            self.seen_steps.append(step_index)
-            log.debug("step: %s", repr(step))
-            self.overlay = self.step_cls_to_view_cls[type(step)](self, step)
 
         self.overlay.start()
         self.keyboard_view.show_overlay(self.overlay)