Merge pull request #531 from mwhudson/controller-refactors

refactor some controller stuff

console_conf/controllers/identity.py

@@ -198,7 +198,7 @@ class IdentityController(BaseController):
         super().__init__(app)
         self.model = app.base_model.identity
 
-    def default(self):
+    def start_ui(self):
         footer = ""
         self.ui.set_footer(footer)
         self.ui.set_body(IdentityView(self.model, self))

console_conf/controllers/welcome.py

@@ -20,7 +20,7 @@ from subiquitycore.controller import BaseController
 
 class WelcomeController(BaseController):
 
-    def default(self):
+    def start_ui(self):
         view = WelcomeView(self)
         self.ui.set_body(view)
 

subiquity/controllers/filesystem.py

@@ -65,7 +65,6 @@ class FilesystemController(BaseController):
         self.answers.setdefault('guided', False)
         self.answers.setdefault('guided-index', 0)
         self.answers.setdefault('manual', [])
-        self.showing = False
         self._probe_state = ProbeState.NOT_STARTED
 
     def start(self):
@@ -125,8 +124,7 @@ class FilesystemController(BaseController):
             lambda fut: self._probed(fut, True),
             )
 
-    def default(self):
-        self.showing = True
+    def start_ui(self):
         if self._probe_state in [ProbeState.PROBING,
                                  ProbeState.REPROBING]:
             self.ui.set_body(SlowProbing(self))
@@ -254,11 +252,9 @@ class FilesystemController(BaseController):
         self.manual()
 
     def cancel(self):
-        self.showing = False
         self.signal.emit_signal('prev-screen')
 
     def finish(self):
-        self.showing = False
         log.debug("FilesystemController.finish next-screen")
         # start curtin install in background
         self.signal.emit_signal('installprogress:filesystem-config-done')

subiquity/controllers/identity.py

@@ -28,7 +28,7 @@ class IdentityController(BaseController):
         super().__init__(app)
         self.model = app.base_model.identity
 
-    def default(self):
+    def start_ui(self):
         self.ui.set_body(IdentityView(self.model, self))
         if all(elem in self.answers for elem in
                ['realname', 'username', 'password', 'hostname']):

subiquity/controllers/installprogress.py

@@ -211,7 +211,6 @@ class InstallProgressController(BaseController):
         self.model = app.base_model
         self.answers.setdefault('reboot', False)
         self.progress_view = None
-        self.progress_view_showing = False
         self.install_state = InstallState.NOT_STARTED
         self.journal_listener_handle = None
         self._postinstall_prerequisites = {
@@ -373,7 +372,7 @@ class InstallProgressController(BaseController):
         self.install_state = InstallState.DONE
         log.debug('After curtin install OK')
         self.ui.progress_current += 1
-        if not self.progress_view_showing:
+        if not self.showing:
             self.ui.set_footer(_("Install complete"))
         else:
             # Re-set footer so progress bar updates.
@@ -564,8 +563,7 @@ class InstallProgressController(BaseController):
             utils.disable_subiquity()
         self.signal.emit_signal('quit')
 
-    def default(self):
-        self.progress_view_showing = True
+    def start_ui(self):
         if self.install_state == InstallState.RUNNING:
             self.progress_view.title = _("Installing system")
             footer = _("Thank you for using Ubuntu!")

subiquity/controllers/keyboard.py

@@ -42,7 +42,7 @@ class KeyboardController(BaseController):
         log.debug("loading launguage %s", code)
         self.model.load_language(code)
 
-    def default(self):
+    def start_ui(self):
         if self.model.current_lang is None:
             self.model.load_language('C')
         view = KeyboardView(self.model, self, self.opts)

subiquity/controllers/mirror.py

@@ -82,7 +82,7 @@ class MirrorController(BaseController):
         self.check_state = CheckState.DONE
         self.model.set_country(cc)
 
-    def default(self):
+    def start_ui(self):
         self.check_state = CheckState.DONE
         self.ui.set_body(MirrorView(self.model, self))
         if 'mirror' in self.answers:

subiquity/controllers/proxy.py

@@ -29,7 +29,7 @@ class ProxyController(BaseController):
         super().__init__(app)
         self.model = app.base_model.proxy
 
-    def default(self):
+    def start_ui(self):
         self.ui.set_body(ProxyView(self.model, self))
         if 'proxy' in self.answers:
             self.done(self.answers['proxy'])

subiquity/controllers/refresh.py

@@ -60,7 +60,6 @@ class RefreshController(BaseController):
         self.current_snap_version = "unknown"
         self.new_snap_version = ""
 
-        self.view = None
         self.offered_first_time = False
 
     def start(self):
@@ -201,8 +200,8 @@ class RefreshController(BaseController):
                     break
             else:
                 self.check_state = CheckState.UNAVAILABLE
-        if self.view:
-            self.view.update_check_state()
+        if self.showing:
+            self.ui.body.update_check_state()
 
     def start_update(self, callback):
         update_marker = os.path.join(self.app.state_dir, 'updating')
@@ -248,7 +247,7 @@ class RefreshController(BaseController):
         result = response.json()
         callback(result['result'])
 
-    def default(self, index=1):
+    def start_ui(self, index=1):
         from subiquity.ui.views.refresh import RefreshView
         if self.app.updated:
             raise Skip()
@@ -265,11 +264,10 @@ class RefreshController(BaseController):
         else:
             raise AssertionError("unexpected index {}".format(index))
         if show:
-            self.view = RefreshView(self)
-            self.ui.set_body(self.view)
+            self.ui.set_body(RefreshView(self))
             if 'update' in self.answers:
                 if self.answers['update']:
-                    self.view.update()
+                    self.ui.body.update()
                 else:
                     self.done()
         else:
@@ -277,9 +275,7 @@ class RefreshController(BaseController):
 
     def done(self, sender=None):
         log.debug("RefreshController.done next-screen")
-        self.view = None
         self.signal.emit_signal('next-screen')
 
     def cancel(self, sender=None):
-        self.view = None
         self.signal.emit_signal('prev-screen')

subiquity/controllers/snaplist.py

@@ -154,7 +154,7 @@ class SnapListController(BaseController):
         self.loader = self._make_loader()
         self.loader.start()
 
-    def default(self):
+    def start_ui(self):
         if self.loader.failed or not self.app.base_model.network.has_network:
             # If loading snaps failed or the network is disabled, skip the
             # screen.

subiquity/controllers/ssh.py

@@ -35,7 +35,7 @@ class SSHController(BaseController):
         super().__init__(app)
         self.model = app.base_model.ssh
 
-    def default(self):
+    def start_ui(self):
         self.ui.set_body(SSHView(self.model, self))
         if self.answers:
             d = {

subiquity/controllers/welcome.py

@@ -39,7 +39,7 @@ class WelcomeController(BaseController):
             if code == lang:
                 self.model.switch_language(code)
 
-    def default(self):
+    def start_ui(self):
         view = WelcomeView(self.model, self)
         self.ui.set_body(view)
         if 'lang' in self.answers:

subiquity/controllers/zdev.py

@@ -640,7 +640,7 @@ class ZdevController(BaseController):
                 zdevinfos = [ZdevInfo.from_row(row) for row in devices]
             self.zdevinfos = OrderedDict([(i.id, i) for i in zdevinfos])
 
-    def default(self):
+    def start_ui(self):
         if 'accept-default' in self.answers:
             self.done()
         self.ui.set_body(ZdevView(self))

subiquitycore/controller.py

@@ -46,15 +46,41 @@ class BaseController(ABC):
         self.signal.connect_signals(signals)
 
     def start(self):
+        """Called just before the main loop is started.
+
+        At the time this is called, all controllers and models and so on
+        have been created. This is when the controller should start
+        interacting with the outside world, e.g. probing for network
+        devices or start making connections to the snap store.
+        """
         pass
 
     @abstractmethod
     def cancel(self):
         pass
 
+    @property
+    def showing(self):
+        cur_controller = self.app.cur_controller
+        while isinstance(cur_controller, RepeatedController):
+            cur_controller = cur_controller.orig
+        return cur_controller is self
+
     @abstractmethod
-    def default(self):
-        pass
+    def start_ui(self):
+        """Start running this controller's UI.
+
+        This method should call self.ui.set_body.
+        """
+
+    def end_ui(self):
+        """Stop running this controller's UI.
+
+        This method doesn't actually need to remove this controller's UI
+        as the next one is about to replace it, it's more of a hook to
+        stop any background tasks that can be stopped when the UI is not
+        running.
+        """
 
     def serialize(self):
         return None
@@ -113,8 +139,8 @@ class RepeatedController(BaseController):
     def register_signals(self):
         pass
 
-    def default(self):
-        self.orig.default(self.index)
+    def start_ui(self):
+        self.orig.start_ui(self.index)
 
     def cancel(self):
         self.orig.cancel()

subiquitycore/controllers/network.py

@@ -244,13 +244,11 @@ class NetworkController(BaseController):
 
     def done(self):
         log.debug("NetworkController.done next-screen")
-        self.view = None
         self.model.has_network = bool(
             self.network_event_receiver.default_routes)
         self.signal.emit_signal('next-screen')
 
     def cancel(self):
-        self.view = None
         self.signal.emit_signal('prev-screen')
 
     def _action_get(self, id):
@@ -339,7 +337,7 @@ class NetworkController(BaseController):
                 dev.set_dhcp_state(v, "TIMEDOUT")
                 self.network_event_receiver.update_link(dev.ifindex)
 
-    def default(self):
+    def start_ui(self):
         if not self.view_shown:
             self.update_initial_configs()
         self.view = NetworkView(self.model, self)
@@ -349,6 +347,9 @@ class NetworkController(BaseController):
         self.network_event_receiver.view = self.view
         self.ui.set_body(self.view)
 
+    def end_ui(self):
+        self.view = self.network_event_receiver.view = None
+
     @property
     def netplan_path(self):
         if self.opts.project == "subiquity":

subiquitycore/core.py

@@ -278,6 +278,13 @@ class Application:
         self.controller_instances = dict.fromkeys(self.controllers)
         self.controller_index = -1
 
+    @property
+    def cur_controller(self):
+        if self.controller_index < 0:
+            return None
+        controller_name = self.controllers[self.controller_index]
+        return self.controller_instances[controller_name]
+
     def run_in_bg(self, func, callback):
         """Run func() in a thread and call callback on UI thread.
 
@@ -339,25 +346,25 @@ class Application:
         log.debug(self.signal)
 
     def save_state(self):
-        if self.controller_index < 0:
+        cur_controller = self.cur_controller
+        if cur_controller is None:
             return
-        cur_controller_name = self.controllers[self.controller_index]
-        cur_controller = self.controller_instances[cur_controller_name]
         state_path = os.path.join(
-            self.state_dir, 'states', cur_controller_name)
+            self.state_dir, 'states', cur_controller._controller_name())
         with open(state_path, 'w') as fp:
             json.dump(cur_controller.serialize(), fp)
 
     def select_screen(self, index):
+        if self.cur_controller is not None:
+            self.cur_controller.end_ui()
         self.controller_index = index
         self.ui.progress_current = index
-        controller_name = self.controllers[self.controller_index]
-        log.debug("moving to screen %s", controller_name)
-        controller = self.controller_instances[controller_name]
-        controller.default()
+        log.debug(
+            "moving to screen %s", self.cur_controller._controller_name())
+        self.cur_controller.start_ui()
         state_path = os.path.join(self.state_dir, 'last-screen')
         with open(state_path, 'w') as fp:
-            fp.write(controller_name)
+            fp.write(self.cur_controller._controller_name())
 
     def next_screen(self, *args):
         self.save_state()
@@ -466,6 +473,46 @@ class Application:
         if key == 'ctrl x':
             self.signal.emit_signal('control-x-quit')
 
+    def load_controllers(self):
+        controllers_mod = __import__(
+            '{}.controllers'.format(self.project), None, None, [''])
+        for i, k in enumerate(self.controllers):
+            if self.controller_instances[k] is None:
+                log.debug("Importing controller: {}".format(k))
+                klass = getattr(controllers_mod, k+"Controller")
+                self.controller_instances[k] = klass(self)
+            else:
+                count = 1
+                for k2 in self.controllers[:i]:
+                    if k2 == k or k2.startswith(k + '-'):
+                        count += 1
+                orig = self.controller_instances[k]
+                k += '-' + str(count)
+                self.controllers[i] = k
+                self.controller_instances[k] = RepeatedController(
+                    orig, count)
+        log.debug("*** %s", self.controller_instances)
+
+    def load_serialized_state(self):
+        for k in self.controllers:
+            state_path = os.path.join(self.state_dir, 'states', k)
+            if not os.path.exists(state_path):
+                continue
+            with open(state_path) as fp:
+                self.controller_instances[k].deserialize(
+                    json.load(fp))
+
+        last_screen = None
+        state_path = os.path.join(self.state_dir, 'last-screen')
+        if os.path.exists(state_path):
+            with open(state_path) as fp:
+                last_screen = fp.read().strip()
+
+        if last_screen in self.controllers:
+            return self.controllers.index(last_screen)
+        else:
+            return 0
+
     def run(self):
         if (self.opts.run_on_serial and
                 os.ttyname(0) != "/dev/ttysclp0"):
@@ -486,45 +533,13 @@ class Application:
         try:
             if self.opts.scripts:
                 self.run_scripts(self.opts.scripts)
-            controllers_mod = __import__('%s.controllers' % self.project,
-                                         None, None, [''])
-            for i, k in enumerate(self.controllers):
-                if self.controller_instances[k] is None:
-                    log.debug("Importing controller: {}".format(k))
-                    klass = getattr(controllers_mod, k+"Controller")
-                    self.controller_instances[k] = klass(self)
-                else:
-                    count = 1
-                    for k2 in self.controllers[:i]:
-                        if k2 == k or k2.startswith(k + '-'):
-                            count += 1
-                    orig = self.controller_instances[k]
-                    k += '-' + str(count)
-                    self.controllers[i] = k
-                    self.controller_instances[k] = RepeatedController(
-                        orig, count)
-            log.debug("*** %s", self.controller_instances)
+
+            self.load_controllers()
 
             initial_controller_index = 0
 
             if self.updated:
-                for k in self.controllers:
-                    state_path = os.path.join(self.state_dir, 'states', k)
-                    if not os.path.exists(state_path):
-                        continue
-                    with open(state_path) as fp:
-                        self.controller_instances[k].deserialize(
-                            json.load(fp))
-
-                last_screen = None
-                state_path = os.path.join(self.state_dir, 'last-screen')
-                if os.path.exists(state_path):
-                    with open(state_path) as fp:
-                        last_screen = fp.read().strip()
-
-                if last_screen in self.controllers:
-                    initial_controller_index = self.controllers.index(
-                        last_screen)
+                initial_controller_index = self.load_serialized_state()
 
             def select_initial_screen(loop, index):
                 try: