[WIP] The time bounded snapshot feature #1844
Conversation
MichaelOrlov
changed the title
MichaelOrlov
force-pushed
the
morlov/add_max_snapshot_duration_option
branch
4 times, most recently
from
0e076ec to
351b529
Compare
|
@reinzor I've made good progress on this PR. I would say I've made all the required changes for this feature. However, I didn't have time to test it and write unit tests. The help here would be really appreciated. Feel free to cherry-pick commits from this PR or create a fork above it. |
There was a problem hiding this comment.
@reinzor FYI, i left a few comments for your reference when you take this over.
rosbag2_cpp/include/rosbag2_cpp/cache/message_cache_circular_buffer.hpp
Outdated
Show resolved
Hide resolved
| ROSBAG2_CPP_LOG_ERROR_STREAM("Invalid arguments for the MessageCacheCircularBuffer. " | ||
| "Both max_bytes_size and max_cache_duration are zero."); |
There was a problem hiding this comment.
if this is debug message, it should include some extra debug information that the following exception cannot throw as in the message? otherwise, it is the same information with the following exception, so maybe debugging message here is not really useful since user can see the exact same message via exception?
There was a problem hiding this comment.
@fujitatomoya The reason why I added a "duplicated" logging message is that in most cases if it fails, it is going to be a process termination with an unhandled exception. In this case the body of the exception may not appear in the log or std::out/std::err.
|
Is this still in development? It seems close to finished, what is blocking its merge? |
|
Sorry, did not have time to work on tests and testing this feature. Afaik that is what still is missing. |
|
Understood, thanks for the quick reply. Do you approximately know if/when you will be able to get to it? |
|
When this gets on our radar again and I have time to work on this. I cannot commit to a specific date. |
- Added the "--snapshot-duration" CLI option. Default value 0 indicates that the snapshot will be limited by the --max-cache-size parameter only. If the value is more than 0, the cyclic buffer for the snapshot will be limited by both the series of messages duration and the maximum cache size parameter. - To override the upper bound by total messages size, the "--maximum-cache-size" CLI option can be settled to 0. Signed-off-by: Michael Orlov <[email protected]>
Signed-off-by: Michael Orlov <[email protected]>
Signed-off-by: Michael Orlov <[email protected]>
Signed-off-by: Michael Orlov <[email protected]>
Signed-off-by: Michael Orlov <[email protected]>
Signed-off-by: Michael Orlov <[email protected]>
MichaelOrlov
force-pushed
the
morlov/add_max_snapshot_duration_option
branch
from
351b529 to
f2c1095
Compare
Signed-off-by: Michael Orlov <[email protected]>
This PR adds the "--snapshot-duration" CLI option.
The snapshot duration CLI option is specified as a floating point value in seconds.
The default value 0.0 indicates that the snapshot will be limited by the
--max-cache-sizeparameter only. If the value is more than 0.0, the cyclic buffer for the snapshot will be limited by both the series of messages duration and the maximum cache size parameter. To override the upper bound by total messages size, the--maximum-cache-sizeCLI option can be settled to 0.