refactored managercontroller and runnablecontroller tests plus relate…

…d code fixes

fix tes_parts __init__

docs/arch/statemachine.rst

@@ -78,8 +78,9 @@ will allow saving and reverting these changes to take it back to a Ready state.
         Ready : Rest state
 
         Ready -up-> Editing : Edit
-        Editing -down-> Saving : Save
-        Editing -down-> Reverting : Revert
+        Editing -up-> Editable
+        Editable -down-> Saving : Save
+        Editable -down-> Reverting : Revert
         Saving -down-> Ready
         Reverting -down-> Ready
     }
@@ -150,8 +151,9 @@ become paused.
         Aborted -up-> Resetting : Reset
 
         Idle -up-> Editing : Edit
-        Editing -down-> Saving : Save
-        Editing -down-> Reverting : Revert
+        Editing -up-> Editable
+        Editable -down-> Saving : Save
+        Editable -down-> Reverting : Revert
         Saving -down-> Idle
         Reverting -down-> Idle
     }

malcolm/blocks/builtin/__init__.py

@@ -0,0 +1,6 @@
+# Find all subpackages, MethodMeta decorated callables, and YAML files
+from malcolm.packageutil import prepare_package
+
+__all__ = prepare_package(globals(), __name__)
+
+del prepare_package

malcolm/blocks/builtin/clientblock.py

@@ -0,0 +1,6 @@
+from malcolm.core import method_takes, REQUIRED
+from malcolm.core.vmetas import StringMeta
+
+@method_takes("mri", StringMeta("MRI for the client block"), REQUIRED)
+def ClientBlock(process, params):
+    return process.make_client_block(params.mri)

malcolm/controllers/defaultcontroller.py

@@ -39,8 +39,9 @@ def do_reset(self):
         self.run_hook(self.Reset, self.create_part_tasks())
 
     def go_to_error_state(self, exception):
-        self.log_exception("Fault occurred while running stateful function")
-        self.transition(sm.FAULT, str(exception))
+        if self.state.value != sm.FAULT:
+            self.log_exception("Fault occurred while running stateful function")
+            self.transition(sm.FAULT, str(exception))
 
     def try_stateful_function(self, start_state, end_state, func, *args,
                               **kwargs):

malcolm/controllers/managercontroller.py

@@ -139,7 +139,7 @@ def do_save(self, layout_name=None):
             layout_name = self.layout_name.value
         structure = self._save_to_structure()
         filename = "/tmp/" + layout_name + ".json"
-        text = json.dumps(structure, indent="  ")
+        text = json.dumps(structure, indent=2)
         open(filename, "w").write(text)
         self.layout_name.set_value(layout_name)
 
@@ -164,6 +164,7 @@ def _save_to_structure(self):
         return structure
 
     def _load_from_structure(self, structure):
-        self.set_layout(structure["layout"])
+        table = self.layout.meta.validate(structure["layout"])
+        self.set_layout(table)
         self.run_hook(self.Load, self.create_part_tasks(), structure)
 

malcolm/controllers/runnablecontroller.py

@@ -150,13 +150,6 @@ class RunnableController(ManagerController):
     # {part: completed_steps for that part}
     progress_reporting = None
 
-    def go_to_error_state(self, exception):
-        if isinstance(exception, StopIteration):
-            # Don't need to transition to aborted, we're already there
-            self.log_warning("Abort occurred while running stateful function")
-        else:
-            super(RunnableController, self).go_to_error_state(exception)
-
     @method_writeable_in(sm.IDLE)
     def edit(self):
         # Override edit to only work from Idle
@@ -221,7 +214,7 @@ def do_validate(self, params):
         # Get any status from all parts
         part_info = self.run_hook(self.ReportStatus, part_tasks)
         # Validate the params with all the parts
-        self.run_hook(self.Validate, part_tasks, part_info, params)
+        self.run_hook(self.Validate, part_tasks, part_info, **params)
 
     @method_takes(*configure_args)
     @method_writeable_in(sm.IDLE)
@@ -299,8 +292,10 @@ def _call_do_run(self):
                 self.do_run(resume=True)
             else:
                 # just drop out
-                self.log_debug("We were aborted")
-                raise
+                self.log_debug("We were aborted by unexpected StopIteration")
+                e = ValueError("unexpected StopIteration")
+                self.go_to_error_state(e)
+                raise e
 
     def do_run(self, resume=False):
         if resume:
@@ -313,13 +308,16 @@ def do_run(self, resume=False):
         if completed_steps < self.total_steps.value:
             steps_to_do = self.steps_per_run
             part_info = self.run_hook(self.ReportStatus, self.part_tasks)
+            self.completed_steps.set_value(completed_steps)
             self.run_hook(
                 self.PostRunReady, self.part_tasks, completed_steps,
                 steps_to_do, part_info, self.configure_params)
+            self.configured_steps.set_value(completed_steps + steps_to_do)
         else:
             self.run_hook(self.PostRunIdle, self.part_tasks)
 
     def update_completed_steps(self, completed_steps, part):
+        # This is run in the child thread, so make sure it is thread safe
         self.progress_reporting[part] = completed_steps
         min_completed_steps = min(self.progress_reporting.values())
         if min_completed_steps > self.completed_steps.value:
@@ -367,9 +365,11 @@ def seek(self, params):
     def do_seek(self, completed_steps):
         steps_to_do = completed_steps % self.steps_per_run
         part_info = self.run_hook(self.ReportStatus, self.part_tasks)
+        self.completed_steps.set_value(completed_steps)
         self.run_hook(
             self.Seek, self.part_tasks, completed_steps,
-            steps_to_do, part_info, self.configure_params)
+            steps_to_do, part_info, **self.configure_params)
+        self.configured_steps.set_value(completed_steps + steps_to_do)
 
     @method_writeable_in(sm.PAUSED)
     def resume(self):

malcolm/core/process.py

@@ -246,10 +246,16 @@ def get_block(self, block_name):
         try:
             return self._blocks[block_name]
         except KeyError:
-            params = ClientController.MethodMeta.prepare_input_map(
-                mri=block_name)
-            controller = ClientController(self, {}, params)
-            return controller.block
+            if block_name in self.process_block.remoteBlocks:
+                return self.make_client_block(block_name)
+            else:
+                raise
+
+    def make_client_block(self, block_name):
+        params = ClientController.MethodMeta.prepare_input_map(
+            mri=block_name)
+        controller = ClientController(self, {}, params)
+        return controller.block
 
     def get_controller(self, block_name):
         return self._controllers[block_name]

malcolm/core/statemachine.py

@@ -148,7 +148,7 @@ def create_states(self):
         self.set_allowed(self.RUNNING, [self.POSTRUN, self.SEEKING])
         self.set_allowed(self.POSTRUN, [self.IDLE, self.READY])
         self.set_allowed(self.PAUSED, [self.SEEKING, self.RUNNING])
-        self.set_allowed(self.SEEKING, self.PAUSED)
+        self.set_allowed(self.SEEKING, [self.READY, self.PAUSED])
 
         # Add Abort to all normal states
         normal_states = [

malcolm/core/syncfactory.py

@@ -39,6 +39,11 @@ def create_lock(self):
         """Creates a new simple Lock object"""
         return Lock()
 
+    def __del__(self):
+        """When we get garbage collected, clean up the threads we created"""
+        self.pool.close()
+        self.pool.join()
+
 
 class InterruptableQueue(queue.Queue):
     # horrible horrible

malcolm/parts/builtin/runnablechildpart.py

@@ -63,8 +63,10 @@ def pause(self, task):
         task.post(self.child["pause"])
 
     @RunnableController.Seek
-    def seek(self, task, completed_steps, steps_to_do, part_info, params):
-        task.post(self.child["seek"])
+    def seek(self, task, completed_steps, steps_to_do, part_info):
+        params = self.child["seek"].prepare_input_map(
+            completedSteps=completed_steps)
+        task.post(self.child["seek"], params)
 
     @RunnableController.Resume
     def resume(self, task, update_completed_steps):

tests/test_comms/test_websocket/test_system_websocket.py

@@ -5,8 +5,8 @@
 
 import time
 # logging
-# import logging
-# logging.basicConfig(level=logging.DEBUG)
+import logging
+logging.basicConfig(level=logging.DEBUG)
 
 import unittest
 
@@ -91,15 +91,15 @@ def tearDown(self):
         self.process2.stop()
 
     def test_server_hello_with_malcolm_client(self):
-        block2 = self.process2.get_block("hello")
+        block2 = self.process2.make_client_block("hello")
         task = Task("task", self.process2)
         futures = task.when_matches_async(block2["state"], "Ready")
         task.wait_all(futures, timeout=1)
         ret = block2.greet("me2")
         self.assertEqual(ret, dict(greeting="Hello me2"))
 
     def test_server_counter_with_malcolm_client(self):
-        block2 = self.process2.get_block("counter")
+        block2 = self.process2.make_client_block("counter")
         task = Task("task", self.process2)
         futures = task.when_matches_async(block2["state"], "Ready")
         task.wait_all(futures, timeout=1)

tests/test_controllers/test_managercontroller.py

@@ -1,10 +1,11 @@
 import os
 import sys
-sys.path.append(os.path.join(os.path.dirname(__file__), "..", ".."))
+sys.path.append(os.path.join(os.path.dirname(__file__), ".."))
 import setup_malcolm_paths
 
 import unittest
 from mock import MagicMock, call
+from time import sleep
 
 # logging
 # import logging
@@ -13,17 +14,121 @@
 # module imports
 from malcolm.controllers.managercontroller import ManagerController
 from malcolm.core import method_writeable_in, method_takes, DefaultStateMachine
+from malcolm.core import Process, Part, Table
+from malcolm.core.syncfactory import SyncFactory
+from malcolm.parts.builtin.childpart import ChildPart
 
 
 class TestManagerController(unittest.TestCase):
-    maxDiff = None
+
+    def checkState(self, state, child=True, parent=True):
+        if child:
+            self.assertEqual(self.c_child.state.value, state)
+        if parent:
+            self.assertEqual(self.c.state.value, state)
 
     def setUp(self):
-        self.c = ManagerController('block', MagicMock())
+        self.s = SyncFactory('threading')
+        self.p = Process('process1', self.s)
+
+        # create a child ManagerController block
+        params = ManagerController.MethodMeta.\
+            prepare_input_map(mri='childBlock')
+        self.c_child = ManagerController(self.p, [], params)
+        self.b_child = self.c_child.block
+
+        self.sm = self.c_child.stateMachine
+
+        params = Part.MethodMeta.prepare_input_map(name='part1')
+        part1 = Part(self.p, params)
+        params = {'name': 'part2', 'mri': 'childBlock'}
+        params = ChildPart.MethodMeta.prepare_input_map(**params)
+        part2 = ChildPart(self.p, params)
+
+        # create a root block for the ManagerController block to reside in
+        parts = [part1, part2]
+        params = {'mri': 'mainBlock'}
+        params = ManagerController.MethodMeta.prepare_input_map(**params)
+        self.c = ManagerController(self.p, parts, params)
         self.b = self.c.block
 
+        # check that do_initial_reset works asynchronously
+        self.checkState(self.sm.DISABLED)
+        self.p.start()
+
+        retry = 0
+        while retry < 20 and self.c.state.value != self.sm.READY:
+            sleep(.1)
+            retry += 1
+        self.checkState(self.sm.READY)
+
+    def test_init(self):
+
+        # the following block attributes should be created by a call to
+        # set_attributes via _set_block_children in __init__
+        self.assertEqual(self.b['layout'].meta.typeid,
+                         'malcolm:core/TableMeta:1.0')
+        self.assertEqual(self.b['layoutName'].meta.typeid,
+                         'malcolm:core/StringMeta:1.0')
+
+        # the following hooks should be created via _find_hooks in __init__
+        self.assertEqual(self.c.hook_names, {
+            self.c.Reset: "Reset",
+            self.c.Disable: "Disable",
+            self.c.Layout: "Layout",
+            self.c.ReportOutports: "ReportOutports",
+            self.c.Load: "Load",
+            self.c.Save: "Save",
+        })
+
+        # check instantiation of object tree via logger names
+        self.assertEqual(self.c._logger.name,
+                         'ManagerController(mainBlock)')
+        self.assertEqual(self.c.parts['part1']._logger.name,
+                         'ManagerController(mainBlock).part1')
+        self.assertEqual(self.c.parts['part2']._logger.name,
+                         'ManagerController(mainBlock).part2')
+        self.assertEqual(self.c_child._logger.name,
+                         'ManagerController(childBlock)')
+
+    def test_edit(self):
+        self.c.edit()
+        # editing only affects one level
+        self.checkState(self.sm.EDITABLE, child=False)
+        self.assertEqual(self.c.revert_structure, self.c._save_to_structure())
+
+    def test_edit_exception(self):
+        self.c.edit()
+        with self.assertRaises(Exception):
+            self.c.edit()
+
+    def test_save(self):
+        self.c.edit()
+        params = {'layoutName': 'testSaveLayout'}
+        params = ManagerController.save.MethodMeta.prepare_input_map(**params)
+        self.c.save(params)
+        self.checkState(self.sm.AFTER_RESETTING, child=False)
+        self.assertEqual(self.c.layout_name.value, 'testSaveLayout')
 
+    def test_revert(self):
+        self.c.edit()
+        self.c.revert()
+        self.checkState(self.sm.AFTER_RESETTING, child=False)
 
+    def test_load_layout(self):
+        self.c.edit()
+        self.checkState(self.sm.EDITABLE, child=False)
+        # self.b.layoutName = 'testSaveLayout'
+        new_layout = Table(self.c.layout.meta)
+        new_layout.name = ["part2"]
+        new_layout.mri = ["P45-MRI"]
+        new_layout.x = [10]
+        new_layout.y = [20]
+        new_layout.visible = [True]
+        self.b.layout = new_layout
+        self.assertEqual(self.c.parts['part2'].x, 10)
+        self.assertEqual(self.c.parts['part2'].y, 20)
+        self.assertEqual(self.c.parts['part2'].visible, True)
 
 if __name__ == "__main__":
     unittest.main(verbosity=2)