Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[noetic] Fix install and improve Python 3 support + python2.7 StringIO fix #97

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open
4 changes: 2 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ add_service_files(FILES

generate_messages(DEPENDENCIES std_msgs std_srvs)

catkin_package()
catkin_package(CATKIN_DEPENDS message_runtime std_msgs std_srvs)

## Install scripts
install(PROGRAMS scripts/capability_server
catkin_install_python(PROGRAMS scripts/capability_server
DESTINATION ${CATKIN_PACKAGE_BIN_DESTINATION})

if(CATKIN_ENABLE_TESTING)
Expand Down
2 changes: 2 additions & 0 deletions package.xml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
<build_depend>std_srvs</build_depend>

<build_export_depend>message_runtime</build_export_depend>
<build_export_depend>std_msgs</build_export_depend>
<build_export_depend>std_srvs</build_export_depend>

<exec_depend>bondpy</exec_depend>
<exec_depend>message_runtime</exec_depend>
Expand Down
2 changes: 1 addition & 1 deletion src/capabilities/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion src/capabilities/discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
35 changes: 22 additions & 13 deletions src/capabilities/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -380,8 +380,11 @@ def spin(self):
self.handle_get_remappings)

rospy.loginfo("Capability Server Ready")
rospy.Publisher("~events", CapabilityEvent, queue_size=1000).publish(
CapabilityEvent(type=CapabilityEvent.SERVER_READY))
heartbeat_interval = rospy.get_param('~heartbeat_interval', 1.0)
rospy.Timer(
rospy.Duration(heartbeat_interval),
lambda event: rospy.Publisher("~events", CapabilityEvent, queue_size=1000).publish(
CapabilityEvent(type=CapabilityEvent.SERVER_READY)))

rospy.spin()

Expand All @@ -398,7 +401,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]
Expand Down Expand Up @@ -434,9 +437,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]
Expand Down Expand Up @@ -579,13 +582,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) +
Expand Down Expand Up @@ -636,7 +639,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
Expand Down Expand Up @@ -685,7 +688,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]
Expand Down Expand Up @@ -909,9 +914,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
Expand Down Expand Up @@ -985,7 +994,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]
Expand Down
7 changes: 6 additions & 1 deletion src/capabilities/specs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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))
Expand Down
16 changes: 8 additions & 8 deletions src/capabilities/specs/interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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>'):
Expand All @@ -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>'):
Expand Down Expand Up @@ -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])
Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand All @@ -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:
Expand All @@ -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:
Expand Down
8 changes: 4 additions & 4 deletions src/capabilities/specs/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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>'):
Expand All @@ -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>'):
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions src/capabilities/specs/semantic_interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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>'):
Expand All @@ -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>'):
Expand Down
2 changes: 1 addition & 1 deletion test/rostest/test_server/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def wait_for_result_to_happen(expected, initial_result, tries=10, sleep_period=1

class Test(unittest.TestCase):
def test_use_and_free_capability(self):
assert wait_for_capability_server(10)
assert wait_for_capability_server(30)
c = CapabilitiesClient()
c.wait_for_services(timeout=3.0)
# Give invalid bond id to use_capability
Expand Down
2 changes: 1 addition & 1 deletion test/rostest/test_server/test_default_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

class Test(unittest.TestCase):
def test_default_provider(self):
assert wait_for_capability_server(10)
assert wait_for_capability_server(30)
call_service('/capability_server/start_capability', 'no_default_provider_pkg/Minimal', '')
rospy.sleep(1) # Wait for the system to settle
resp = call_service('/capability_server/get_running_capabilities')
Expand Down
2 changes: 1 addition & 1 deletion test/rostest/test_server/test_ros_services.test
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,5 @@
'invalid'
</rosparam>
</node>
<test test-name="ros_services" pkg="capabilities" type="test_ros_services.py"/>
<test test-name="ros_services" pkg="capabilities" type="test_ros_services.py" time-limit="120.0" />
</launch>
7 changes: 6 additions & 1 deletion test/unit/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@
import re
import sys

from StringIO import StringIO
try:
# Python 2
from cStringIO import StringIO
except ImportError:
# Python 3
from io import StringIO


def assert_raises(exception_classes, callable_obj=None, *args, **kwargs):
Expand Down
4 changes: 2 additions & 2 deletions test/unit/specs/test_interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion test/unit/specs/test_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion test/unit/specs/test_semantic_interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion test/unit/test_client.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from common import assert_raises
from .common import assert_raises

from capabilities.client import CapabilitiesClient
from capabilities.client import ServiceNotAvailableException
Expand Down
2 changes: 1 addition & 1 deletion test/unit/test_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down