Fix handling of designator None types/values (#924)

Fix handling of designator None types/values

robot_smach_states/src/robot_smach_states/util/designators/core.py

@@ -12,8 +12,6 @@ class Designator(object):
     define Designators that take a goal specification, like a query to a world
     model.
 
-    current is therefore a property with only a getter.
-
     >>> d = Designator("Initial value", name="tester")
     >>> d.resolve()
     'Initial value'
@@ -27,48 +25,85 @@ def __init__(self, initial_value=None, resolve_type=None, name=None):
         """
         Initialization method
 
-        :param initial_value: () initial value
-        :param resolve_type: (type) type to which this designator should resolve
-        :param name: (str) name used for debugging purposes
+        :param initial_value: Initial value, should match the given resolve_type.
+                              None is allowed if a resolve_type is provided.
+        :param resolve_type: Type to which this designator should resolve.
+                             If None, use the type of the initial value. Value
+                             must be not None in that case.
+        :vartype resolve_type: Type or a list with a single type (the element type).
+                               The latter represents list of values where each
+                               value has the element type.
+
+        :param name: name used for debugging purposes
+        :vartype name: str
         """
         super(Designator, self).__init__()
 
         self._name = name
-
         self.__initial_value = initial_value
-        if not resolve_type:
+        if resolve_type:
+            if isinstance(resolve_type, list):
+                # Value is a list of items with each item of type resolve_type[0] .
+                if len(resolve_type) != 1:
+                    msg = "The resolve_type must be a single-item list, found {}."
+                    raise TypeError(msg.format(resolve_type))
+
+                if resolve_type[0] is None:
+                    raise TypeError("The resolve_type may not be [None]")
+            self._resolve_type = resolve_type
+        else:
+            # No resolve type provided, use the inital value to derive a type.
+            if self.__initial_value is None:
+                raise TypeError("No resolve_type provided and type could not be inferred "
+                                "from the initial_value (found 'None').")
+
             if isinstance(self.__initial_value, list):
-                if self.__initial_value:
+                if len(self.__initial_value) > 0:
                     self._resolve_type = [type(self.__initial_value[0])]
                 else:
-                    raise TypeError("resolve_type could not be inferred from empty list.")
+                    raise TypeError("No resolve_type provided and type could not be inferred "
+                                    "from the initial_value (found empty list).")
             else:
                 self._resolve_type = type(self.__initial_value)
-        else:
-            self._resolve_type = resolve_type
 
         Designator.instances.add(self)
 
     def resolve(self):
-        """Selects a new goal and sets it as the current value."""
+        """
+        Selects a new goal and sets it as the current value.
+
+        Don't override this method, override self._resolve() instead.
+        """
         result = self._resolve()
         result_type = type(result)
         resolve_type = self.resolve_type
 
-        if isinstance(result, list):
-            if not result:
-                return []
-            result_type = type(result[0])
+        if result is None: # No data available for checking the type.
+            return result
+
+        if isinstance(self._resolve_type, list):
+            # Not None, list expected instead.
+            if not isinstance(result, list):
+                self.fail_with_type_error(result_type, resolve_type)
+
+            if len(result) == 0: # Empty list, cannot check element type.
+                return result
+
+            if not isinstance(result[0], resolve_type[0]): # Element type check.
+                self.fail_with_type_error(result_type, resolve_type)
 
-        if isinstance(self.resolve_type, list):
-            resolve_type = self.resolve_type[0]
+            return result
+
+        # Normal type.
+        if not issubclass(result_type, resolve_type):
+            self.fail_with_type_error(result_type, resolve_type)
 
-        if result is not None and not issubclass(result_type, resolve_type):
-            raise TypeError("{} resolved to a '{}' instead of expected '{}'. "
-                            "Originals: result type: {}, resolve type: {}".format(self, result_type, resolve_type,
-                                                                                  type(result), self.resolve_type))
         return result
 
+    def fail_with_type_error(self, result_type, resolve_type):
+            msg = "{} resolved to a '{}' instead of expected '{}'."
+            raise TypeError(msg.format(self, result_type, resolve_type))
+
     def _resolve(self):
         return self.__initial_value
 
@@ -131,17 +166,11 @@ def __init__(self, initial_value=None, resolve_type=None, name=None):
                             "can use it")
 
         super(VariableDesignator, self).__init__(initial_value, resolve_type, name=name)
-
         self._current = initial_value
-
         self.writeable = VariableWriter(self)
 
     def _set_current_protected(self, value):
         resolve_type = self.resolve_type
-        try:
-            resolve_type = tuple(self.resolve_type)
-        except TypeError:
-            pass
 
         if isinstance(value, list) and isinstance(self.resolve_type, list):
             if value and not issubclass(type(value[0]), resolve_type[0]):

robot_smach_states/src/robot_smach_states/util/designators/introspection.py

@@ -110,8 +110,8 @@ def make_legend():
     legend = Digraph('cluster_1')
 
     state = smach.State()
-    parent_desig = Designator(name="parent_designator")
-    desig = Designator(name="designator")
+    parent_desig = Designator(name="parent_designator", resolve_type=[str])  # No resolve_type really needed but required
+    desig = Designator(name="designator", resolve_type=[str])  # No resolve_type really needed but required
     vardesig = VariableDesignator(resolve_type=str, name="variable").writeable
 
     used_in_state = DesignatorUsedInState(state, desig, "role_resolved_from")

robot_smach_states/test/util/designators/test_core.py

@@ -46,6 +46,55 @@ def test_list(self):
         self.assertEqual(v.resolve_type, [str])
         self.assertListEqual(v.resolve(), ['a', 'b', 'c'])
 
+    def test_none_value(self):
+        # Resolve a [object] to a None value
+        class MyVariableDesignator(VariableDesignator):
+            def __init__(self):
+                super(MyVariableDesignator, self).__init__(resolve_type=[object])
+            def _resolve(self):
+                return None
+
+        v = MyVariableDesignator()
+        self.assertEquals(v.resolve(), None)
+
+    def test_empty_list_value(self):
+        # Resolve a [object] to an empty list value.
+        class MyVariableDesignator(VariableDesignator):
+            def __init__(self):
+                super(MyVariableDesignator, self).__init__(resolve_type=[str])
+            def _resolve(self):
+                return []
+
+        v = MyVariableDesignator()
+        self.assertEquals(v.resolve(), [])
+
+    def test_none_list_value(self):
+        # Resolve a [object] to a [None] list value.
+        class MyVariableDesignator(VariableDesignator):
+            def __init__(self):
+                super(MyVariableDesignator, self).__init__(resolve_type=[str])
+            def _resolve(self):
+                return [None]
+
+        v = MyVariableDesignator()
+        with self.assertRaises(TypeError):
+            self.assertEquals(v.resolve(), [None])
+
+    def test_empty_list_type(self):
+        with self.assertRaises(TypeError):
+            v = VariableDesignator(resolve_type=[])
+
+    def test_none_type_in_list(self):
+        with self.assertRaises(TypeError):
+            v = VariableDesignator(resolve_type=[None])
+
+    def test_multi_element_type(self):
+        with self.assertRaises(TypeError):
+            v = VariableDesignator(resolve_type=[str, bool])
+
+    def test_none_type_singular(self):
+        with self.assertRaises(TypeError):
+            v = VariableDesignator(resolve_type=None)
 
 class TestVariableWriter(unittest.TestCase):
     def test_basics(self):