diff --git a/clients/roscpp/src/libros/callback_queue.cpp b/clients/roscpp/src/libros/callback_queue.cpp index f9312f5ca1..cd2f4f8a6c 100644 --- a/clients/roscpp/src/libros/callback_queue.cpp +++ b/clients/roscpp/src/libros/callback_queue.cpp @@ -34,6 +34,7 @@ #include "ros/callback_queue.h" #include "ros/assert.h" +#include namespace ros { @@ -371,7 +372,17 @@ CallbackQueue::CallOneResult CallbackQueue::callOneCB(TLS* tls) tls->calling_in_this_thread = id_info->id; CallbackInterface::CallResult result = CallbackInterface::Invalid; - try { + + { + // Ensure that thread id gets restored, even if callback throws. + // This is done with RAII rather than try-catch so that the source + // of the original exception is not masked in a crash report. + BOOST_SCOPE_EXIT(&tls, &last_calling) + { + tls->calling_in_this_thread = last_calling; + } + BOOST_SCOPE_EXIT_END + if (info.marked_for_removal) { tls->cb_it = tls->callbacks.erase(tls->cb_it); @@ -382,14 +393,6 @@ CallbackQueue::CallOneResult CallbackQueue::callOneCB(TLS* tls) result = cb->call(); } } - catch (std::exception&) - { - // ensure that thread id gets restored, even in case of an exception - tls->calling_in_this_thread = last_calling; - throw; - } - - tls->calling_in_this_thread = last_calling; // Push TryAgain callbacks to the back of the shared queue if (result == CallbackInterface::TryAgain && !info.marked_for_removal) diff --git a/clients/roscpp/src/libros/transport/transport_udp.cpp b/clients/roscpp/src/libros/transport/transport_udp.cpp index 848893b2ce..c7d8298f98 100644 --- a/clients/roscpp/src/libros/transport/transport_udp.cpp +++ b/clients/roscpp/src/libros/transport/transport_udp.cpp @@ -590,6 +590,7 @@ int32_t TransportUDP::write(uint8_t* buffer, uint32_t size) else { num_bytes = 0; + --this_block; } } else if (num_bytes < (unsigned) sizeof(header)) diff --git a/clients/rospy/src/rospy/__init__.py b/clients/rospy/src/rospy/__init__.py index 3a432e0b68..337e8f780d 100644 --- a/clients/rospy/src/rospy/__init__.py +++ b/clients/rospy/src/rospy/__init__.py @@ -57,6 +57,7 @@ from .core import is_shutdown, signal_shutdown, \ get_node_uri, get_ros_root, \ logdebug, logwarn, loginfo, logout, logerr, logfatal, \ + logdebug_throttle, logwarn_throttle, loginfo_throttle, logerr_throttle, logfatal_throttle, \ parse_rosrpc_uri from .exceptions import * from .msg import AnyMsg @@ -101,6 +102,9 @@ 'logdebug', 'logwarn', 'loginfo', 'logout', 'logerr', 'logfatal', + 'logdebug_throttle', + 'logwarn_throttle', 'loginfo_throttle', + 'logerr_throttle', 'logfatal_throttle', 'parse_rosrpc_uri', 'MasterProxy', 'NodeProxy', diff --git a/clients/rospy/src/rospy/core.py b/clients/rospy/src/rospy/core.py index 08affe88dc..57c07b8367 100644 --- a/clients/rospy/src/rospy/core.py +++ b/clients/rospy/src/rospy/core.py @@ -37,6 +37,11 @@ import atexit +try: + import cPickle as pickle +except ImportError: + import pickle +import inspect import logging import os import signal @@ -149,6 +154,67 @@ def rospywarn(msg, *args): logerror = logerr # alias logerr logfatal = logging.getLogger('rosout').critical + + +class LoggingThrottle(object): + + last_logging_time_table = {} + + def __call__(self, caller_id, logging_func, period, msg): + """Do logging specified message periodically. + + - caller_id (str): Id to identify the caller + - logging_func (function): Function to do logging. + - period (float): Period to do logging in second unit. + - msg (object): Message to do logging. + """ + now = rospy.Time.now() + + last_logging_time = self.last_logging_time_table.get(caller_id) + + if (last_logging_time is None or + (now - last_logging_time) > rospy.Duration(period)): + logging_func(msg) + self.last_logging_time_table[caller_id] = now + + +_logging_throttle = LoggingThrottle() + + +def _frame_record_to_caller_id(frame_record): + frame, _, lineno, _, code, _ = frame_record + caller_id = ( + inspect.getabsfile(frame), + lineno, + frame.f_lasti, + ) + return pickle.dumps(caller_id) + + +def logdebug_throttle(period, msg): + caller_id = _frame_record_to_caller_id(inspect.stack()[1]) + _logging_throttle(caller_id, logdebug, period, msg) + + +def loginfo_throttle(period, msg): + caller_id = _frame_record_to_caller_id(inspect.stack()[1]) + _logging_throttle(caller_id, loginfo, period, msg) + + +def logwarn_throttle(period, msg): + caller_id = _frame_record_to_caller_id(inspect.stack()[1]) + _logging_throttle(caller_id, logwarn, period, msg) + + +def logerr_throttle(period, msg): + caller_id = _frame_record_to_caller_id(inspect.stack()[1]) + _logging_throttle(caller_id, logerr, period, msg) + + +def logfatal_throttle(period, msg): + caller_id = _frame_record_to_caller_id(inspect.stack()[1]) + _logging_throttle(caller_id, logfatal, period, msg) + ######################################################### # CONSTANTS diff --git a/clients/rospy/src/rospy/msproxy.py b/clients/rospy/src/rospy/msproxy.py index c44176c8c1..936f2215b6 100644 --- a/clients/rospy/src/rospy/msproxy.py +++ b/clients/rospy/src/rospy/msproxy.py @@ -90,8 +90,6 @@ def __init__(self, uri): self._lock = Lock() def __getattr__(self, key): #forward api calls to target - with self._lock: - f = getattr(self.target, key) if key in _master_arg_remap: remappings = _master_arg_remap[key] else: @@ -103,7 +101,9 @@ def wrappedF(*args, **kwds): i = i + 1 #callerId does not count #print "Remap %s => %s"%(args[i], rospy.names.resolve_name(args[i])) args[i] = rospy.names.resolve_name(args[i]) - return f(*args, **kwds) + with self._lock: + f = getattr(self.target, key) + return f(*args, **kwds) return wrappedF def __getitem__(self, key): diff --git a/clients/rospy/src/rospy/timer.py b/clients/rospy/src/rospy/timer.py index 4ddf49117a..b587426302 100644 --- a/clients/rospy/src/rospy/timer.py +++ b/clients/rospy/src/rospy/timer.py @@ -185,7 +185,7 @@ def __init__(self, period, callback, oneshot=False): """ Constructor. @param period: desired period between callbacks - @type period: rospy.Time + @type period: rospy.Duration @param callback: callback to be called @type callback: function taking rospy.TimerEvent @param oneshot: if True, fire only once, otherwise fire continuously until shutdown is called [default: False] diff --git a/test/test_rosbag/CMakeLists.txt b/test/test_rosbag/CMakeLists.txt index 41f60b427e..534403f7ca 100644 --- a/test/test_rosbag/CMakeLists.txt +++ b/test/test_rosbag/CMakeLists.txt @@ -54,7 +54,7 @@ if(CATKIN_ENABLE_TESTING) include_directories(${GTEST_INCLUDE_DIRS}) add_executable(double_pub EXCLUDE_FROM_ALL test/double_pub.cpp) target_link_libraries(double_pub ${GTEST_LIBRARIES} ${catkin_LIBRARIES}) - add_dependencies(double_pub rosgraph_msgs_genpy) + add_dependencies(double_pub ${rosgraph_msgs_EXPORTED_TARGETS}) if(TARGET tests) add_dependencies(tests double_pub) endif() diff --git a/test/test_roscpp/CMakeLists.txt b/test/test_roscpp/CMakeLists.txt index 7dd052a03b..41ed980f49 100644 --- a/test/test_roscpp/CMakeLists.txt +++ b/test/test_roscpp/CMakeLists.txt @@ -85,7 +85,6 @@ endif() catkin_package( CATKIN_DEPENDS message_runtime rosconsole rosgraph_msgs std_msgs xmlrpcpp - DEPENDS Boost ) if(CATKIN_ENABLE_TESTING) diff --git a/test/test_roscpp/test/src/latching_publisher.cpp b/test/test_roscpp/test/src/latching_publisher.cpp index f24b1f3f95..cd4beeb467 100644 --- a/test/test_roscpp/test/src/latching_publisher.cpp +++ b/test/test_roscpp/test/src/latching_publisher.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2NULLNULL8, Willow Garage, Inc. + * Copyright (c) 2008, Willow Garage, Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without diff --git a/test/test_roscpp/test/src/timer_callbacks.cpp b/test/test_roscpp/test/src/timer_callbacks.cpp index b016ee5d60..9f2bc7ec5e 100644 --- a/test/test_roscpp/test/src/timer_callbacks.cpp +++ b/test/test_roscpp/test/src/timer_callbacks.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2NULLNULL8, Willow Garage, Inc. + * Copyright (c) 2008, Willow Garage, Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without diff --git a/test/test_roscpp/test/src/wait_for_message.cpp b/test/test_roscpp/test/src/wait_for_message.cpp index 672cdae67b..d18b025b3d 100644 --- a/test/test_roscpp/test/src/wait_for_message.cpp +++ b/test/test_roscpp/test/src/wait_for_message.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2NULLNULL8, Willow Garage, Inc. + * Copyright (c) 2008, Willow Garage, Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without diff --git a/test/test_rospy/test/rostest/test_rospy_client_online.py b/test/test_rospy/test/rostest/test_rospy_client_online.py index 7944a68bac..028313de83 100755 --- a/test/test_rospy/test/rostest/test_rospy_client_online.py +++ b/test/test_rospy/test/rostest/test_rospy_client_online.py @@ -100,6 +100,55 @@ def test_log(self): rospy.logfatal("test 4") self.assert_("[FATAL]" in sys.stderr.getvalue()) self.assert_("test 4" in sys.stderr.getvalue()) + + # logXXX_throttle + for i in range(3): + sys.stdout = StringIO() + rospy.loginfo_throttle(3, "test 1") + if i == 0: + self.assert_("test 1" in sys.stdout.getvalue()) + rospy.sleep(rospy.Duration(1)) + elif i == 1: + self.assert_("" == sys.stdout.getvalue()) + rospy.sleep(rospy.Duration(2)) + else: + self.assert_("test 1" in sys.stdout.getvalue()) + + for i in range(3): + sys.stderr = StringIO() + rospy.logwarn_throttle(3, "test 2") + if i == 0: + self.assert_("test 2" in sys.stderr.getvalue()) + rospy.sleep(rospy.Duration(1)) + elif i == 1: + self.assert_("" == sys.stderr.getvalue()) + rospy.sleep(rospy.Duration(2)) + else: + self.assert_("test 2" in sys.stderr.getvalue()) + + for i in range(3): + sys.stderr = StringIO() + rospy.logerr_throttle(3, "test 3") + if i == 0: + self.assert_("test 3" in sys.stderr.getvalue()) + rospy.sleep(rospy.Duration(1)) + elif i == 1: + self.assert_("" == sys.stderr.getvalue()) + rospy.sleep(rospy.Duration(2)) + else: + self.assert_("test 3" in sys.stderr.getvalue()) + + for i in range(3): + sys.stderr = StringIO() + rospy.logfatal_throttle(3, "test 4") + if i == 0: + self.assert_("test 4" in sys.stderr.getvalue()) + rospy.sleep(rospy.Duration(1)) + elif i == 1: + self.assert_("" == sys.stderr.getvalue()) + rospy.sleep(rospy.Duration(2)) + else: + self.assert_("test 4" in sys.stderr.getvalue()) finally: sys.stdout = real_stdout sys.stderr = real_stderr diff --git a/test/test_rospy/test/unit/test_rospy_api.py b/test/test_rospy/test/unit/test_rospy_api.py index 2c4e37b41c..fe541d8765 100644 --- a/test/test_rospy/test/unit/test_rospy_api.py +++ b/test/test_rospy/test/unit/test_rospy_api.py @@ -118,6 +118,11 @@ def test_rospy_api(self): rospy.loginfo rospy.logout #deprecated rospy.logwarn + rospy.logdebug_throttle + rospy.logerr_throttle + rospy.logfatal_throttle + rospy.loginfo_throttle + rospy.logwarn_throttle rospy.myargv rospy.on_shutdown rospy.parse_rosrpc_uri diff --git a/tools/rosbag/src/recorder.cpp b/tools/rosbag/src/recorder.cpp index a6b7e26dd3..8e6e33b606 100644 --- a/tools/rosbag/src/recorder.cpp +++ b/tools/rosbag/src/recorder.cpp @@ -109,7 +109,9 @@ RecorderOptions::RecorderOptions() : split(false), max_size(0), max_duration(-1.0), - node("") + node(""), + min_space(1024 * 1024 * 1024), + min_space_str("1G") { } @@ -346,6 +348,11 @@ void Recorder::updateFilenames() { if (options_.split) parts.push_back(boost::lexical_cast(split_count_)); + if (parts.size() == 0) + { + throw BagException("Bag filename is empty (neither of these was specified: prefix, append_date, split)"); + } + target_filename_ = parts[0]; for (unsigned int i = 1; i < parts.size(); i++) target_filename_ += string("_") + parts[i]; diff --git a/tools/rosbag/src/rosbag/migration.py b/tools/rosbag/src/rosbag/migration.py index 840c29252f..19ea6b1b2a 100644 --- a/tools/rosbag/src/rosbag/migration.py +++ b/tools/rosbag/src/rosbag/migration.py @@ -1039,7 +1039,7 @@ def migrate_raw(self, msg_from, msg_to): path = self.find_path(msg_from[4], msg_to[4]) if False in [sn.rule.valid for sn in path]: - raise BagMigrationException("Migrate called, but no valid migration path from [%s] to [%s]"%(msg_from._type, msg_to._type)) + raise BagMigrationException("Migrate called, but no valid migration path from [%s] to [%s]"%(msg_from[0], msg_to[0])) # Short cut to speed up case of matching md5sum: if path == [] or msg_from[2] == msg_to[2]: diff --git a/tools/rosbag/src/rosbag/rosbag_main.py b/tools/rosbag/src/rosbag/rosbag_main.py index f75a49602b..0136e98cd7 100644 --- a/tools/rosbag/src/rosbag/rosbag_main.py +++ b/tools/rosbag/src/rosbag/rosbag_main.py @@ -65,6 +65,13 @@ def handle_split(option, opt_str, value, parser): print("Use of \"--split \" has been deprecated. Please use --split --size or --split --duration ", file=sys.stderr) parser.values.size = int(parser.rargs.pop(0)) + +def _stop_process(signum, frame, old_handler, process): + process.terminate() + if old_handler: + old_handler(signum, frame) + + def record_cmd(argv): parser = optparse.OptionParser(usage="rosbag record TOPIC1 [TOPIC2 TOPIC3 ...]", description="Record a bag file with the contents of specified topics.", @@ -121,9 +128,15 @@ def record_cmd(argv): cmd.extend(args) + old_handler = signal.signal( + signal.SIGTERM, + lambda signum, frame: _stop_process(signum, frame, old_handler, process) + ) # Better way of handling it than os.execv # This makes sure stdin handles are passed to the process. - subprocess.call(cmd) + process = subprocess.Popen(cmd) + process.wait() + def info_cmd(argv): parser = optparse.OptionParser(usage='rosbag info [options] BAGFILE1 [BAGFILE2 BAGFILE3 ...]', @@ -252,9 +265,16 @@ def play_cmd(argv): cmd.extend(['--bags']) cmd.extend(args) + + old_handler = signal.signal( + signal.SIGTERM, + lambda signum, frame: _stop_process(signum, frame, old_handler, process) + ) # Better way of handling it than os.execv # This makes sure stdin handles are passed to the process. - subprocess.call(cmd) + process = subprocess.Popen(cmd) + process.wait() + def filter_cmd(argv): def expr_eval(expr): @@ -304,7 +324,7 @@ def eval_fn(topic, m, t): return try: - meter = ProgressMeter(outbag_filename, inbag.size) + meter = ProgressMeter(outbag_filename, inbag._uncompressed_size) total_bytes = 0 if options.verbose_pattern: diff --git a/tools/rosconsole/include/ros/console.h b/tools/rosconsole/include/ros/console.h index 793e64dc18..7924a327c6 100644 --- a/tools/rosconsole/include/ros/console.h +++ b/tools/rosconsole/include/ros/console.h @@ -40,6 +40,7 @@ #include #include #include +#include #ifdef ROSCONSOLE_BACKEND_LOG4CXX #include "log4cxx/level.h" diff --git a/tools/rosgraph/src/rosgraph/xmlrpc.py b/tools/rosgraph/src/rosgraph/xmlrpc.py index 7d9aad8ed4..2cf043f76e 100644 --- a/tools/rosgraph/src/rosgraph/xmlrpc.py +++ b/tools/rosgraph/src/rosgraph/xmlrpc.py @@ -93,6 +93,9 @@ def __init__(self, addr, log_requests=1): # to True to allow quick restart on the same port. This is equivalent # to calling setsockopt(SOL_SOCKET,SO_REUSEADDR,1) self.allow_reuse_address = True + # Increase request_queue_size to handle issues with many simultaneous + # connections in OSX 10.11 + self.request_queue_size = min(socket.SOMAXCONN, 128) if rosgraph.network.use_ipv6(): logger = logging.getLogger('xmlrpc') # The XMLRPC library does not support IPv6 out of the box diff --git a/tools/roslaunch/src/roslaunch/substitution_args.py b/tools/roslaunch/src/roslaunch/substitution_args.py index 1ad0185dc5..cbf9dd9f16 100644 --- a/tools/roslaunch/src/roslaunch/substitution_args.py +++ b/tools/roslaunch/src/roslaunch/substitution_args.py @@ -135,21 +135,25 @@ def _find(resolved, a, args, context): rp = _get_rospack() if path: source_path_to_packages = rp.get_custom_cache('source_path_to_packages', {}) + res = None try: - return _find_executable( - resolve_without_path, a, [args[0], path], context, - source_path_to_packages=source_path_to_packages) - except SubstitutionException: - pass - try: - return _find_resource( + res = _find_executable( resolve_without_path, a, [args[0], path], context, source_path_to_packages=source_path_to_packages) except SubstitutionException: pass + if res is None: + try: + res = _find_resource( + resolve_without_path, a, [args[0], path], context, + source_path_to_packages=source_path_to_packages) + except SubstitutionException: + pass # persist mapping of packages in rospack instance if source_path_to_packages: rp.set_custom_cache('source_path_to_packages', source_path_to_packages) + if res is not None: + return res pkg_path = rp.get_path(args[0]) if path: pkg_path = os.path.join(pkg_path, path) diff --git a/tools/roslaunch/src/roslaunch/xmlloader.py b/tools/roslaunch/src/roslaunch/xmlloader.py index ff07ff4463..6bb752b6fc 100644 --- a/tools/roslaunch/src/roslaunch/xmlloader.py +++ b/tools/roslaunch/src/roslaunch/xmlloader.py @@ -409,7 +409,7 @@ def _node_tag(self, tag, context, ros_config, default_machine, is_test=False, ve elif tag_name == 'env': self._env_tag(t, env_context, ros_config) else: - ros_config.add_config_error("WARN: unrecognized '%s' tag in tag. Node xml is %s"%(t.tagName, tag.toxml())) + ros_config.add_config_error("WARN: unrecognized '%s' child tag in the parent tag element: %s"%(t.tagName, tag.toxml())) # #1036 evaluate all ~params in context # TODO: can we get rid of force_local (above), remove this for loop, and just rely on param_tag logic instead? diff --git a/tools/rostest/cmake/rostest-extras.cmake.em b/tools/rostest/cmake/rostest-extras.cmake.em index 6c13124a00..324f3b5f3d 100644 --- a/tools/rostest/cmake/rostest-extras.cmake.em +++ b/tools/rostest/cmake/rostest-extras.cmake.em @@ -83,7 +83,7 @@ function(add_rostest_gtest target launch_file) if(TARGET tests) add_dependencies(tests ${target}) endif() - add_rostest(${launch_file}) + add_rostest(${launch_file} DEPENDENCIES ${target}) endif() endfunction() diff --git a/tools/rostest/nodes/hztest b/tools/rostest/nodes/hztest index d8e5f9d48a..f8c3c28317 100755 --- a/tools/rostest/nodes/hztest +++ b/tools/rostest/nodes/hztest @@ -120,7 +120,7 @@ Test Duration: %s"""%(hz, hzerror, topic, test_duration)) self.assert_(hz >= 0.0, "bad parameter (hz)") self.assert_(hzerror >= 0.0, "bad parameter (hzerror)") self.assert_(test_duration > 0.0, "bad parameter (test_duration)") - self.assert_(len(topic), "bad parameter (topic") + self.assert_(len(topic), "bad parameter (topic)") if hz == 0: self.min_rate = 0.0 diff --git a/tools/rostest/src/rostest/__init__.py b/tools/rostest/src/rostest/__init__.py index 5f9e6da5b4..260e68e080 100644 --- a/tools/rostest/src/rostest/__init__.py +++ b/tools/rostest/src/rostest/__init__.py @@ -134,10 +134,11 @@ def rosrun(package, test_name, test, sysargs=None): import rospy suite = None - if issubclass(test, unittest.TestCase): - suite = unittest.TestLoader().loadTestsFromTestCase(test) - else: + if isinstance(test, str): suite = unittest.TestLoader().loadTestsFromName(test) + else: + # some callers pass a TestCase type (instead of an instance) + suite = unittest.TestLoader().loadTestsFromTestCase(test) if text_mode: result = unittest.TextTestRunner(verbosity=2).run(suite) diff --git a/tools/rostopic/src/rostopic/__init__.py b/tools/rostopic/src/rostopic/__init__.py index fe329140af..0fcafcb570 100644 --- a/tools/rostopic/src/rostopic/__init__.py +++ b/tools/rostopic/src/rostopic/__init__.py @@ -713,7 +713,7 @@ def __init__(self, topic, msg_eval, plot=False, filter_fn=None, offset_time=False, count=None, field_filter_fn=None, fixed_numeric_width=None): """ - :param plot: if ``True``, echo in plotting-friendly format, ``bool`` + :param plot: if ``True``, echo in plotting-friendly format (csv), ``bool`` :param filter_fn: function that evaluates to ``True`` if message is to be echo'd, ``fn(topic, msg)`` :param echo_all_topics: (optional) if ``True``, echo all messages in bag, ``bool`` :param offset_time: (optional) if ``True``, display time as offset from current time, ``bool``