-
Notifications
You must be signed in to change notification settings - Fork 914
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
changes between 1.11.20 and 1.12.7 for backporting #1008
Conversation
…ny to the created rostest
Thanks to Wookey
rospy.Time -> rospy.Duration for period.
Don't rely on transitive header inclusion to declare std::vector as building with GCC-6 fails due to no '#Include <vector>' statement.
Handles issues with many simultaneous connections to XmlRPC Server in OSX 10.11
Take the ._type using [0], because the arguments passed to migrate_raw are tuples. This was throwing another exception because of that bug: AttributeError, saying that tuple has no attribute _type.
… confusing. For example, [this warning msg](https://travis-ci.org/wg-perception/people/jobs/202019288#L3737) (in this [PR](wg-perception/people#49)): ``` * WARN: unrecognized 'group' tag in <node> tag. Node xml is <test name="$(arg testnode_name)" pkg="rostest" test-name="hztest_test" type="hztest"> <group unless="$(arg expected_success)"> <node args="-r 0.5 $(find face_detector)/test/face_detector_noface_test_diamondback.bag" name="play" pkg="rosbag" type="play"/> <param name="hz" value="0.0"/> </group> ``` This is confusing because the tag in question is actually `<test>`, not `<node>`. Although the warning message refers to the exact <test> tag, I didn't try to read it because I was told the issue is with <node>, not <test>. With this PR the message becomes a bit verbose but hope the readers get better idea.
* [rospy] made get_published_topics threadsafe
* fixed UDP block number when EAGAIN or EWOULDBLOCK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, I especially looked at the "low risk of regression?" labeled ones.
@wjwwood Thank you for going through this lengthy list as well as all the other backport PRs. 🙇♂️ |
@dirk-thomas: I found the list extremely informative - not only because it provides an overview of bugs that were fixed, but also because it shows what is not backported. +10. Do you intend to provide such as list for future backport PRs? Would it be an idea to link to these PRs somewhere or post the list to a more visible location? |
Due to the amount of patches to be considered and the advantage of having the backports tested by CI, yes, I think I will do it the same way in the future in this repo.
Why not. What do you have in mind? |
It would be great to queue #962 for backport. I have ros/geometry/pull/145 against geometry to use the new path. However since I haven't forked development I'm holding the PR until #962 is backported. Since the goal is for downstream packages to use the new path it would be great to let that happen sooner rather than waiting for indigo to EOL or development to fork. |
#962 is intentionally not backported as described above since it is not necessary for older distributions. So I don't think any of the rational had changed to justify backporting it. |
Indeed #962 is "not necessary" for older distributions. It's also "not necessary" on any distribution. It's a style cleanup flagged by @jspricke as a potential conflict on Debian. And the goal of fixing this way was that downstream code should be easy to fix. The benefit of this change is not achieved until everyone has switched. Looking at the comment stating that it should be removed in lunar @mightyCelu helpfully submitted ros/geometry#145 to update it, but I cannot merge it because the development is not forked for the new distro(s). And especially as geometry is now mostly a backwards compatibility place holder I don't plan on forking it since there's no new feature development going on. Not backporting this change would force forking the repo as soon as the old header locations are removed. I can see that it's the same situation for diagnostics. And if there is any other moderately stable package out there they will have the same issue. Thus I'm asking that this be queued as it will be convenient for downstream developers, such as myself, as well as anyone else using the xmlrpcpp headers directly in a package not under active development but still being maintained. Alternatively tocking it can wait until the Indigo rosdistro is EOL, but that's quite a long way out still. |
@dirk-thomas wrote:
I only stumbled on this PR and the description of the backports by accident. Perhaps we could create some page on the wiki where these kind of PRs are listed chronologically? I was first thinking of perhaps adding a link to the repo sync mails / posts that @tfoote sends out, but that would possibly be hard to correlate with a particular backport PR. |
In terms of overall visibility/discoverability, I'm not sure that another wiki page to have to maintain would be any better than linking to a Github search, eg: https://github.com/ros/ros_comm/pulls?q=backport%20in%3Atitle |
I don't really care how or where, as long these backport PRs receive some more attention. Keeping up-to-speed with all the changes in (core) packages is quite involved, so having a single overview of backports like this would seem like a valuable resource, that seems to deserve a bit more exposure. |
Is the concern primarily about breakages, or new features, or something else? These are intentionally selected on the basis of being low-risk changes/fixes that have been specifically requested by users of the LTS release. |
I'm not too worried about breakages: the really important pkgs seem to have sufficient unit tests to catch regressions most of the time. Keeping up with backports of new features is definitely nice, but knowing what has changed (even if it was supposed to be an orthogonal change that shouldn't have introduced any changes to existing behaviour) is invaluable to me when trying to debug something. Also: being able to point users on ROS Answers to PRs that have either changed something that they were not aware of or introduced features they thought / assumed they could not use on their version of ROS would be another rationale. |
The following list of changes has been integrated into ros_comm 1.12.7 (Kinetic) since the last Indigo / Jade release (1.11.20).
Backported: (these changes are part of this PR)
improve stacktrace Changed calling_in_this_thread restore mechanism in callOneCB from try-catch to RAII. #811
rospy.logXXX_throttle [rospy] Implement rospy.logXXX_throttle #812
rostopic doc More detailed help string for rostopic echo -p #816
rostest target dependency add_rostest_gtest does now add the created gtest-target as a depende… #830
fix copyright Fix confusing copyright messages/dates #844
rosbag filter progress update rosbag filter progress meter to use raw uncompressed input size #857
fix target dependency fix unknown msg-generation dependency #869
Timer doc [rospy] Fix wrong type in docstring for rospy.Timer #878
init values set default values for min_space and min_space_str #884
test type handling e341225
remove invalid export 4468f86
include for gcc6 Add '#Include <vector>' to fix building on GCC-6 #911
increase rpc queue size Problem with many simultaneous connects to rosmaster on OSX 10.11 #849
Fix BagMigrationException Fix BagMigrationException in migrate_raw #917
rospy param doc Add note for dict usage in rospy set_param #964
fix rospack caching in launch Issue947 #966
prevent segfault throw exception instead of accessing invalid memory #971
typo [rostest] trivial typo. #992
improve error message [roslaunch] Indicating <node> tag when it is actually a <test> tag is confusing. #989
get_published_topics threadsafe [rospy] made get_published_topics threadsafe #958
fix UDP block number fixed UDP block number when EAGAIN or EWOULDBLOCK #957
rosbag terminate subprocesses terminate underlying 'rosbag {play,record}' on SIGTERM #951
Not backported:
make destructor virtual make LogAppender destructor virtual #773
add parameter add missing parameter to AdvertiseOptions::createAdvertiseOptions #774
change return value change return value of param() to bool #775
remove global variables improve TopicManager::instance #776 fix static destruction order #871
reenable tests 33a5a27 22a7dc6
add test writing rosbags adds a newline #780
defused xml Use defusedxml in rosmaster #782
remove duplicate function Remove duplicated function, use decorator #783
roslaunch expressions python evaluation of expressions in roslaunch #784 added math symbols and some standard data types for python evaluation #793
rosnode info quiet Add a non verbose (quiet) mode for rosnode info #809
timestamp-less cache Allow saving timestamp-less messages to Cache (using rospy.Time.now() as their timestamp). #806
compiler warning remove extra semicolon #817
edge list contains add EdgeList.__contains__ #754
rostopic hz multiple [rostopic] Get multiple topics with rostopic hz #712 Fix rostopic hz for multiple topics without no message #843 [rostopic] Fix for no messages for one topic in two topics #886 fix rostopic hz to print no message when there are no message on any topic #888 restore API compatibility of rostopic.ROSTopicHz #896
abstract connection based transport [topic_tools] Abstract class to implement connection based transport #713
enable tests Enabled tests and then fixed them #826
change queue operation order to match msg Move queue operations to after debug message so that tenses of message match reality. #818
explicit constructor rosbag_storage: make Bag constructor explicit #835
compiler warning Drop strict-aliasing warnings from roslz4 file. #839 fix compiler warnings in release mode #842
Python 3 roslaunch: Fix <param command="..." /> for python3. #840 Fix
rosservice call
for python3. #847 Fixes for Python 3 compatibility. #978lz4_FOUND c14b942
logging formatter Use logging Formatter, enabling printing exception info with exc_info=1 #828
Darwin Add 'Darwin' to unix-like platforms (resolve #845) #846
rostopic echo --noarr/nostr [rostopic] Show stat when rostopic echo --noarr/nostr #724 [rostopic] Fix rostopic echo for non rosmsg field #909 [rostopic] Fix typo of arg for _str_plot function #916
fast approximate time synchronization [message_filters] Fast approximate time synchronization in message_filters (pure python) #802
64bit rosbag Allow 64-bit sizes to be passed to robag max_size #865
record max number of splits Record a maximum number of splits and then begin deleting old files #866
specific to kinetic only [rostopic] Fix rostopic echo for list of ROS message to fix #868 #872
example / comment fix order of init and publisher in example #873 [rostest] Refactor hztestX.test #862
rostopic type [rostopic] Show topic field type with rostopic type #860
CMake warning register nosetests only when testing is enabled #880
paramtest, publishtest [rostest] Add test node if topic message is published at least once #863 [rostest] Add reusable node to test parameters. #859
use_test_depends support use_test_depends option for roslaunch-check #887 Added missing arg to check_roslaunch_dir() definition #890 6606843
gennodejs #02794b1760aaaef8c7683615aa792b8d57d1956d
roslaunch include file empty [roslaunch] Added condition to skip empty ('') <include> paths #882
empty node name [roscpp] throw exception on ros::init with empty node name #894 [rospy] raise error on rospy.init_node with None or empty node name string #895
roslaunch Better naming for roslaunch check test XMLs. #897
fix multi-threaded spinning fix multi-threaded spinning #867
boost bind callback Use boost::bind to bind the callback function #906
roslaunch USE_TEST_DEPENDENCIES add USE_TEST_DEPENDENCIES option to roslaunch_add_file_check() #910
default rospy logging format [rosgraph] Fix rospy default rosconsole format for consistency with roscpp #879
rospy reconnect Indigo devel fix rospy reconnect #851
class -> struct Fix WallTimerEvent class -> struct #924
drop epydoc Drop epydoc from rosbag_storage #925
use poll in write_header In write_header() use poll if available. #929
change return value Return result of client execute #938
rosbag plat wait for subscribers rosbag play: added option wait-for-subscriber #959
rosbag player pause service Pause service for rosbag player #949
move xmlrpc headers move headers to include/xmlrpcpp #962
poll -> epoll Fix/107 #831
separate logging Add logger roscpp_internal.connections. #980
additional tests Add a very simple testcase #772 add test code for multibyte string #985
more logging Add more logging to publisher update calls. #979
rosmsg info [rosmsg] Rosmsg info implemented as alias of rosmsg show #941
rate control topic Implemented rate-control-topic and rate-control-max-delay. #947
@ros/ros_team @ros/ros_comm-maintainers Please comment on the decision which changes to (not) backport. Especially for the three backported changes marked with a "?" I wasn't sure about the regression risk.