[noetic] Fix install and improve Python 3 support

CMakeLists.txt

@@ -34,7 +34,7 @@ generate_messages(DEPENDENCIES std_msgs std_srvs)
 catkin_package()
 
 ## Install scripts
-install(PROGRAMS scripts/capability_server
+catkin_install_python(PROGRAMS scripts/capability_server
   DESTINATION ${CATKIN_PACKAGE_BIN_DESTINATION})
 
 if(CATKIN_ENABLE_TESTING)

src/capabilities/client.py

@@ -134,7 +134,7 @@ def wait_for_services(self, timeout=None, services=None):
         :returns: :py:obj:`True` is the services are available, :py:obj:`False` otherwise (timeout)
         :rtype: :py:obj:`bool`
         """
-        services = self._services.keys() if services is None else services
+        services = list(self._services.keys()) if services is None else services
         assert isinstance(services, list), services
         for service in services:
             if service not in self._services:

src/capabilities/discovery.py

@@ -436,7 +436,7 @@ def names(self):
         :rtype: :py:obj:`list` (:py:obj:`str`)
         """
         return list(itertools.chain(
-            self.interfaces.keys(), self.semantic_interfaces.keys(), self.providers.keys()))
+            list(self.interfaces.keys()), list(self.semantic_interfaces.keys()), list(self.providers.keys())))
 
     @property
     def specs(self):

src/capabilities/server.py

@@ -398,7 +398,7 @@ def __load_capabilities(self):
         package_index = package_index_from_package_path(self.__package_paths)
         self.spec_file_index = spec_file_index_from_package_index(package_index)
         # Prune packages by black and white list
-        for package in self.spec_file_index.keys():
+        for package in list(self.spec_file_index.keys()):
             if self.__package_whitelist and package not in self.__package_whitelist:
                 rospy.loginfo("Package '{0}' not in whitelist, skipping.".format(package))
                 del self.spec_file_index[package]
@@ -434,9 +434,9 @@ def __load_capabilities(self):
                     spec_index.remove_provider(provider.name)
         self.__spec_index = spec_index
         # Prune spec_file_index
-        spec_paths = spec_index.interface_paths.values() + \
-            spec_index.semantic_interface_paths.values() + \
-            spec_index.provider_paths.values()
+        spec_paths = list(spec_index.interface_paths.values()) + \
+            list(spec_index.semantic_interface_paths.values()) + \
+            list(spec_index.provider_paths.values())
         for package_name, package_dict in self.spec_file_index.items():
             for spec_type in ['capability_interface', 'semantic_capability_interface', 'capability_provider']:
                 package_dict[spec_type][:] = [path for path in package_dict[spec_type] if path in spec_paths]
@@ -579,13 +579,13 @@ def __cleanup_graph(self):
         """
         # Collect all running capabilities
         running_capabilities = [x
-                                for x in self.__capability_instances.values()
+                                for x in list(self.__capability_instances.values())
                                 if x.state == 'running']
         for cap in running_capabilities:
             if cap.started_by == USER_SERVICE_REASON:
                 # Started by user, do not garbage collect this
                 continue
-            rdepends = get_reverse_depends(cap.interface, self.__capability_instances.values())
+            rdepends = get_reverse_depends(cap.interface, list(self.__capability_instances.values()))
             if rdepends:
                 # Someone depends on me, do not garbage collect this
                 rospy.logdebug("Keeping the '{0}' provider of the '{1}' interface, ".format(cap.name, cap.interface) +
@@ -636,7 +636,7 @@ def __stop_capability(self, name):
                          "which is not in the list of capability instances.")
             return
         capability = self.__capability_instances[name]
-        rdepends = get_reverse_depends(name, self.__capability_instances.values())
+        rdepends = get_reverse_depends(name, list(self.__capability_instances.values()))
         for cap in rdepends:
             if cap.state in ['stopping', 'terminated']:  # pragma: no cover
                 # It is possible that this cap was stopped by another cap in this list
@@ -685,7 +685,9 @@ def __get_providers_for_interface(self, interface, allow_semantic=False):
         return providers  # Could be empty
 
     def __start_capability(self, capability, preferred_provider):
-        if capability not in self.__spec_index.interfaces.keys() + self.__spec_index.semantic_interfaces.keys():
+        if capability not in (
+                list(self.__spec_index.interfaces.keys()) +
+                list(self.__spec_index.semantic_interfaces.keys())):
             raise RuntimeError("Capability '{0}' not found.".format(capability))
         # If no preferred provider is given, use the default
         preferred_provider = preferred_provider or self.__default_providers[capability]
@@ -909,9 +911,13 @@ def handle_get_providers(self, req):
 
     def _handle_get_providers(self, req):
         if req.interface:
-            if req.interface not in self.__spec_index.interfaces.keys() + self.__spec_index.semantic_interfaces.keys():
+            if req.interface not in (
+                    list(self.__spec_index.interfaces.keys()) +
+                    list(self.__spec_index.semantic_interfaces.keys())):
                 raise RuntimeError("Capability Interface '{0}' not found.".format(req.interface))
-            providers = self.__get_providers_for_interface(req.interface, allow_semantic=req.include_semantic).keys()
+            providers = list(
+                self.__get_providers_for_interface(
+                    req.interface, allow_semantic=req.include_semantic).keys())
             default_provider = self.__default_providers[req.interface]
         else:
             providers = self.__spec_index.provider_names
@@ -985,7 +991,7 @@ def _handle_get_remappings(self, req):
                 remappings[map_type].update(mapping)
             # Collapse remapping chains
             for mapping in remappings.values():
-                for key, value in mapping.items():
+                for key, value in list(mapping.items()):
                     if value in mapping:
                         mapping[key] = mapping[value]
                         del mapping[value]

src/capabilities/specs/common.py

@@ -38,6 +38,11 @@
 
 from __future__ import print_function
 
+try:
+    __basestring = basestring
+except NameError:  # pragma: no cover
+    __basestring = str
+
 
 def validate_spec_name(name):
     """Validates a given spec name follows the 'package/spec_name' format
@@ -60,7 +65,7 @@ def split_spec_name(name):
     :raises: AssertionError if spec name is not a str
     :raises: ValueError if spec name is invalid
     """
-    assert isinstance(name, basestring), "a spec name must be a string"
+    assert isinstance(name, __basestring), "a spec name must be a string"
     split_name = name.split('/')
     if len(split_name) != 2 or not split_name[0] or not split_name[1]:
         raise ValueError("Invalid spec name '{0}', it should be of the form 'package/spec_name'".format(name))

src/capabilities/specs/interface.py

@@ -122,7 +122,7 @@ def capability_interface_from_file_path(file_path):
     :raises: :py:exc:`OSError` if the given file does not exist
     """
     with open(os.path.abspath(file_path), 'r') as f:
-        return capability_interface_from_dict(yaml.load(f), file_path)
+        return capability_interface_from_dict(yaml.safe_load(f), file_path)
 
 
 def capability_interface_from_file(file_handle):
@@ -136,7 +136,7 @@ def capability_interface_from_file(file_handle):
     :rtype: :py:class:`CapabilityInterface`
     :raises: :py:exc:`OSError` if the given file does not exist
     """
-    return capability_interface_from_dict(yaml.load(file_handle.read()), file_handle.name)
+    return capability_interface_from_dict(yaml.safe_load(file_handle.read()), file_handle.name)
 
 
 def capability_interface_from_string(string, file_name='<string>'):
@@ -152,7 +152,7 @@ def capability_interface_from_string(string, file_name='<string>'):
     :rtype: :py:class:`CapabilityInterface`
     :raises: :py:exc:`AttributeError` if the given value for string is not a str
     """
-    return capability_interface_from_dict(yaml.load(string), file_name)
+    return capability_interface_from_dict(yaml.safe_load(string), file_name)
 
 
 def capability_interface_from_dict(spec, file_name='<dict>'):
@@ -226,7 +226,7 @@ def __add_element_to_interface(name, element, group_name=None):
     for group in element_groups:
         if group in ['requires', 'provides']:
             elements = element_groups[group] or {}
-            for name, element in elements.iteritems():
+            for name, element in elements.items():
                 __add_element_to_interface(name, element, group)
         else:
             __add_element_to_interface(group, element_groups[group])
@@ -485,7 +485,7 @@ def __init__(self, name, spec_version, description=None):
 
     def __str__(self):
         elements = "topics:\n"
-        required_and_provided = list(self.required_topics.keys() + self.provided_topics.keys())
+        required_and_provided = list(self.required_topics.keys()) + list(self.provided_topics.keys())
         both = [x for x in self.topics if x not in required_and_provided]
         for name, topic in self.topics.items():
             if name not in both:
@@ -504,7 +504,7 @@ def __str__(self):
             elements += "\n          ".join(str(topic).splitlines())
             elements += "\n"
         elements += "    services:\n"
-        required_and_provided = list(self.required_services.keys() + self.provided_services.keys())
+        required_and_provided = list(self.required_services.keys()) + list(self.provided_services.keys())
         both = [x for x in self.services if x not in required_and_provided]
         for name, service in self.services.items():
             if name not in both:
@@ -523,7 +523,7 @@ def __str__(self):
             elements += "\n          ".join(str(service).splitlines())
             elements += "\n"
         elements += "    actions:\n"
-        required_and_provided = list(self.required_actions.keys() + self.provided_actions.keys())
+        required_and_provided = list(self.required_actions.keys()) + list(self.provided_actions.keys())
         both = [x for x in self.actions if x not in required_and_provided]
         for name, action in self.actions.items():
             if name not in both:
@@ -542,7 +542,7 @@ def __str__(self):
             elements += "\n          ".join(str(action).splitlines())
             elements += "\n"
         elements += "    parameters:\n"
-        required_and_provided = list(self.required_parameters.keys() + self.provided_parameters.keys())
+        required_and_provided = list(self.required_parameters.keys()) + list(self.provided_parameters.keys())
         both = [x for x in self.parameters if x not in required_and_provided]
         for name, parameter in self.parameters.items():
             if name not in both:

src/capabilities/specs/provider.py

@@ -109,7 +109,7 @@ def capability_provider_from_file_path(file_path):
     :raises: :py:exc:`OSError` if the given file does not exist
     """
     with open(os.path.abspath(file_path), 'r') as f:
-        return capability_provider_from_dict(yaml.load(f.read()), file_path)
+        return capability_provider_from_dict(yaml.safe_load(f.read()), file_path)
 
 
 def capability_provider_from_file(file_handle):
@@ -123,7 +123,7 @@ def capability_provider_from_file(file_handle):
     :rtype: :py:class:`CapabilityProvider`
     :raises: :py:exc:`OSError` if the given file does not exist
     """
-    return capability_provider_from_dict(yaml.load(file_handle.read()), file_handle.name)
+    return capability_provider_from_dict(yaml.safe_load(file_handle.read()), file_handle.name)
 
 
 def capability_provider_from_string(string, file_name='<string>'):
@@ -139,7 +139,7 @@ def capability_provider_from_string(string, file_name='<string>'):
     :rtype: :py:class:`CapabilityProvider`
     :raises: :py:exc:`AttributeError` if the given value for string is not a str
     """
-    return capability_provider_from_dict(yaml.load(string), file_name)
+    return capability_provider_from_dict(yaml.safe_load(string), file_name)
 
 
 def capability_provider_from_dict(spec, file_name='<dict>'):
@@ -197,7 +197,7 @@ def capability_provider_from_dict(spec, file_name='<dict>'):
         raise InvalidProvider("Invalid depends_on section, expected dict got: '{0}'".format(type(depends_on)),
                               file_name)
     valid_conditionals = ['provider']
-    for interface, conditions in depends_on.iteritems():
+    for interface, conditions in depends_on.items():
         if not isinstance(conditions, dict):
             raise InvalidProvider("Invalid depends_on conditional section, expected dict got: '{0}'"
                                   .format(type(conditions)), file_name)

src/capabilities/specs/semantic_interface.py

@@ -116,7 +116,7 @@ def semantic_capability_interface_from_file_path(file_path):
     :raises: :py:exc:`OSError` if the given file does not exist
     """
     with open(os.path.abspath(file_path), 'r') as f:
-        return semantic_capability_interface_from_dict(yaml.load(f.read()), file_path)
+        return semantic_capability_interface_from_dict(yaml.safe_load(f.read()), file_path)
 
 
 def semantic_capability_interface_from_file(file_handle):
@@ -130,7 +130,7 @@ def semantic_capability_interface_from_file(file_handle):
     :rtype: :py:class:`SemanticCapabilityInterface`
     :raises: :py:exc:`OSError` if the given file does not exist
     """
-    return semantic_capability_interface_from_dict(yaml.load(file_handle.read()), file_handle.name)
+    return semantic_capability_interface_from_dict(yaml.safe_load(file_handle.read()), file_handle.name)
 
 
 def semantic_capability_interface_from_string(string, file_name='<string>'):
@@ -146,7 +146,7 @@ def semantic_capability_interface_from_string(string, file_name='<string>'):
     :rtype: :py:class:`SemanticCapabilityInterface`
     :raises: :py:exc:`AttributeError` if the given value for string is not a str
     """
-    return semantic_capability_interface_from_dict(yaml.load(string), file_name)
+    return semantic_capability_interface_from_dict(yaml.safe_load(string), file_name)
 
 
 def semantic_capability_interface_from_dict(spec, file_name='<dict>'):

test/unit/common.py

@@ -3,7 +3,7 @@
 import re
 import sys
 
-from StringIO import StringIO
+from io import StringIO
 
 
 def assert_raises(exception_classes, callable_obj=None, *args, **kwargs):

test/unit/specs/test_interface.py

@@ -32,7 +32,7 @@ def check_navigation(ci):
     ci.provided_parameters
     ci.required_parameters
     check_interface(ci)
-    str(ci.topics.values()[0])
+    str(list(ci.topics.values())[0])
 
 
 def check_all_interfaces(ci):
@@ -80,7 +80,7 @@ def check_minimal(ci):
 
 def test_capability_interface_from_file_path():
     default_checker = lambda x: None
-    for test_file, (checker, expected_exception, expected_exception_regex) in test_files_map.iteritems():
+    for test_file, (checker, expected_exception, expected_exception_regex) in test_files_map.items():
         checker = checker or default_checker
         print('running test on file ' + test_file)
         test_file_path = os.path.join(test_data_dir, test_file)

test/unit/specs/test_provider.py

@@ -58,7 +58,7 @@ def check_minimal(cp):
 def test_capability_provider_from_file_path():
     default_checker = lambda x: None
     print()  # Keeps the output clean when doing nosetests -s
-    for test_file, (checker, expected_exception, expected_exception_regex) in test_files_map.iteritems():
+    for test_file, (checker, expected_exception, expected_exception_regex) in test_files_map.items():
         checker = checker or default_checker
         print('running test on file ' + test_file)
         test_file_path = os.path.join(test_data_dir, test_file)

test/unit/specs/test_semantic_interface.py

@@ -48,7 +48,7 @@ def check_minimal(sci):
 def test_semantic_capability_interface_from_file_path():
     default_checker = lambda x: None
     print()  # Keeps the output clean when doing nosetests -s
-    for test_file, (checker, expected_exception, expected_exception_regex) in test_files_map.iteritems():
+    for test_file, (checker, expected_exception, expected_exception_regex) in test_files_map.items():
         checker = checker or default_checker
         print('running test on file ' + test_file)
         test_file_path = os.path.join(test_data_dir, test_file)

test/unit/test_client.py

@@ -1,4 +1,4 @@
-from common import assert_raises
+from .common import assert_raises
 
 from capabilities.client import CapabilitiesClient
 from capabilities.client import ServiceNotAvailableException

test/unit/test_server.py

@@ -60,7 +60,7 @@ def test_get_reverse_depends():
         instances['foo'].depends_on = ['bar']
         instances['bar'].depends_on = []
         instances['baz'].depends_on = ['bar']
-        result = [x.name for x in server.get_reverse_depends('bar', instances.values())]
+        result = [x.name for x in server.get_reverse_depends('bar', list(instances.values()))]
         assert sorted(['foo_pkg/foo', 'baz_pkg/baz']) == sorted(result), sorted(result)
 
 except ImportError as exc: