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

Add option to repeat latched messages at the start of bag splits #1850

Merged
merged 5 commits into from
May 15, 2020

Conversation

tappan-at-git
Copy link
Contributor

Split bagfiles lose all latched data. Many long-running ROS applications will include some kind of latched data that's only published once on launch (e.g. maps, static transforms), but prefer split bagfiles for ease of use, meaning they will have a difficult time actually using latched data without playing back the entire series of bagfiles.

This PR adds the --repeat-latched option to rosbag record. When this option is enabled, rosbag::Recorder remembers up to one message for each <topic, publisher> pair and prepends each latched message to each new bag file.

@mikepurvis
Copy link
Member

This looks like a great addition; would you consider adding a simple test for the functionality, however? See several other bits of latch-related functional tests here:

https://github.com/ros/ros_comm/tree/melodic-devel/test/test_rosbag/test

@dirk-thomas
Copy link
Member

@tappan-at-git Friendly ping.

@@ -368,7 +370,7 @@ def eval_fn(topic, m, t):
else:
print('NO MATCH', verbose_pattern(topic, msg, t))

total_bytes += len(serialized_bytes)
total_bytes += len(serialized_bytes)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please revert unrelated whitespace changes.

Same below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really thought I caught all of the EOL changes my editor snuck in. Will address along with the requested tests when I have the cycles.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tappan-at-git Please follow up on this soon if you want this patch to be considered for Noetic.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please follow up on this soon if you want this patch to be considered for Noetic.

@paulbovbel FYI

Copy link
Member

@dirk-thomas dirk-thomas May 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tappan-at-git Friendly ping.

@dirk-thomas
Copy link
Member

Since this patch changes ABI I will mark it to be considered for a future distro only.

@oceanusxiv
Copy link

This resolves #706 if I recall correctly.

@paulbovbel paulbovbel force-pushed the add-repeat-latched branch from a19d6eb to d318d8c Compare March 9, 2020 19:36
@dirk-thomas dirk-thomas changed the base branch from melodic-devel to noetic-devel April 20, 2020 22:01
@tappan-at-git tappan-at-git force-pushed the add-repeat-latched branch 4 times, most recently from e268344 to 7751cef Compare May 14, 2020 13:36
(cherry picked from commit 836a4e5)
@tappan-at-git
Copy link
Contributor Author

Sorry about the force-push spam. Github was still showing one of the whitespace changes after I had pushed a fix. Unless Github is seriously messing with me, all the unrelated whitespace changes should be squashed at this point.

I can't commit to adding full testing for this feature. Unless I'm missing something rosbag's existing latched message test doesn't seem to check if its latched messages were recorded correctly, and I don't see any tests for splits. I just don't have time to add those.

@dirk-thomas
Copy link
Member

Thank you for the patch.

It is unfortunate that it doesn't come with any kind of test coverage. The fact that some other parts of existing code aren't covered is not a reason to not implement tests for new features.

Since it is ABI breaking I will go ahead and merge it for the upcoming Noetic release anyway sice otherwise this feature couldn't land afterwards.

@dirk-thomas
Copy link
Member

@ros-pull-request-builder retest this please

@dirk-thomas dirk-thomas merged commit 19b90c2 into ros:noetic-devel May 15, 2020
nim65s added a commit to nim65s/robotpkg that referenced this pull request Mar 12, 2021
Because DEPEND_ABI.ros-comm.noetic?= ros-comm>=1.15

1.15.9 (2020-10-16)
-------------------
* Fix deadlock when service connection is dropped (ros/ros_comm#2074)
* Update maintainers (ros/ros_comm#2075)
* Fix case where accessing cached parameters shuts down another node (ros/ros_comm#2068)
* Fix spelling (ros/ros_comm#2066)
* Fix Lost Wake Bug in ROSOutAppender (ros/ros_comm#2033)
* Fix compatibility issue with boost 1.73 and above (ros/ros_comm#2023)
* Gracefully stop recording upon SIGTERM and SIGINT (ros/ros_comm#2038)
* Use heapq.merge instead of custom merge sort code (ros/ros_comm#2017)
* Fix handling of single quotes in command arguments on Windows (ros/ros_comm#2051)
* Clearer error message (ros/ros_comm#2035)
* Ignore underscores when parsing literal numeric values for Python 3 compatibility (ros/ros_comm#2022)
* Clear cached URI for a node that has gone offline (ros/ros_comm#2010)
* Add skip_cache parameter to rosnode_ping() (ros/ros_comm#2009)
* Install advertisetest (ros/ros_comm#2046)
* Use range instead of xrange for Python 3 compatibility (ros/ros_comm#2013)
* Fix to address CVE-2020-16124 (ros/ros_comm#2065)
* Fix XmlRpcValue::_doubleFormat being unused (ros/ros_comm#2003)

1.15.8 (2020-07-23)
-------------------
* change is_async_connected to use epoll when available (ros/ros_comm#1983)
* allow mixing latched and unlatched publishers (ros/ros_comm#1991)
* remove not existing NodeProxy from rospy __all_\_ (ros/ros_comm#2007)
* fix typo in topics.py (ros/ros_comm#1977)
* fix bad relative import (still Python 2 style) (ros/ros_comm#1973)
* improve shutdown message with duplicate node name (ros/ros_comm#1992)
* remove dependency on rostopic from rostest package (ros/ros_comm#2002)
* fix missing reload() function in Python 3 (ros/ros_comm#1968)
* add latch param to throttle (ros/ros_comm#1944)
* add const versions of XmlRpcValue converting operators (ros/ros_comm#1978)

1.15.7 (2020-05-28)
-------------------
* fix Windows build break (ros/ros_comm#1961)
* fix NameError in launch error handling (ros/ros_comm#1965)

1.15.6 (2020-05-21)
-------------------
* fix a bug that using a destroyed connection object (ros/ros_comm#1950)

1.15.5 (2020-05-15)
-------------------
* check if async socket connect is success or failure before TransportTCP::read() and TransportTCP::write() (ros/ros_comm#1954)
* fix bug that connection drop signal related funtion throw a bad_weak exception (ros/ros_comm#1940)
* multiple latched publishers per process on the same topic (ros/ros_comm#1544)
* fix negative numbers in ros statistics (ros/ros_comm#1531)
* remove extra \n in ROS_DEBUG (ros/ros_comm#1925)
* add option to repeat latched messages at the start of bag splits (ros/ros_comm#1850)
* fix bag migration failures caused by typo in connection_header assignment (ros/ros_comm#1952)
* fix brief description comments after members (ros/ros_comm#1920)
* add --sigint-timeout and --sigterm-timeout parameters (ros/ros_comm#1937)
* roslaunch-check: search dir recursively (ros/ros_comm#1914)
* sort printed nodes by namespace alphabetically (ros/ros_comm#1934)
* remove pycrypto import (not used) (ros/ros_comm#1922)
* avoid infinite recursion in rosrun tab completion when rosbash is not installed (ros/ros_comm#1948)
* fix bare pointer in topic_tools::ShapeShifter (ros/ros_comm#1722)
* clear message queue on simtime jumping back (ros/ros_comm#1518)
* use undefined dynamic_lookup on macOS (ros/ros_comm#1923)
* check if enough FDs are free, instead counting the total free FDs (ros/ros_comm#1929)

1.15.4 (2020-03-19)
-------------------
* restrict boost dependencies to components used (ros/ros_comm#1871)
* add exception for ConnectionAbortedError (ros/ros_comm#1908)
* fix mac trying to use epoll instead of kqueue (ros/ros_comm#1907)
* fix AttributeError: __exit__ (ros/ros_comm#1915, regression from 1.14.4)

1.15.3 (2020-02-28)
-------------------
* remove Boost version check since Noetic only targets platforms with 1.67+ (ros/ros_comm#1903)

1.15.2 (2020-02-25)
-------------------
* export missing Boost dependency (ros/ros_comm#1898)
* add timestamp formatting for rosconsole (ros/ros_comm#1892)

1.15.1 (2020-02-24)
-------------------
* fix missing boost dependencies (ros/ros_comm#1895)
* use setuptools instead of distutils (ros/ros_comm#1870)
* increase time limit of advertisetest/publishtest.test to reduce flakyness (ros/ros_comm#1897)

1.15.0 (2020-02-21)
-------------------
* fix dictionary changed size during iteration (ros/ros_comm#1894)
* update test to pass with old and new yaml (ros/ros_comm#1893)

Packaging changes:
- removed patch-an, as there are no more boost version checks
- updated patch-ao
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants