-
Notifications
You must be signed in to change notification settings - Fork 252
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
Added support for filtering topics via regular expressions #1034
Conversation
bc3e1a2
to
fd879f7
Compare
I'm debugging an issue with the player and regular expressions. The following causes $ ros2 bag play . --topics-regex /lidar_front/points_raw
[INFO] [1656517413.919186963] [rosbag2_storage]: Opened database './rosbag2_2020_09_23-15_58_07.db3' for READ_ONLY.
[INFO] [1656517413.919316194] [rosbag2_player]: Set rate to 1
[INFO] [1656517413.925218047] [rosbag2_player]: Adding keyboard callbacks.
[INFO] [1656517413.925259647] [rosbag2_player]: Press SPACE for Pause/Resume
[INFO] [1656517413.925286814] [rosbag2_player]: Press CURSOR_RIGHT for Play Next Message
[INFO] [1656517413.925312879] [rosbag2_player]: Press CURSOR_UP for Increase Rate 10%
[INFO] [1656517413.925341094] [rosbag2_player]: Press CURSOR_DOWN for Decrease Rate 10%
[INFO] [1656517413.925446615] [rosbag2_player]: Playback until timestamp: -1
[INFO] [1656517413.925790959] [rosbag2_storage]: Opened database './rosbag2_2020_09_23-15_58_07.db3' for READ_ONLY.
[ERROR] [1656517414.026479569] [rosbag2_player]: Failed to play: Error when preparing SQL statement 'SELECT data, timestamp, topics.name, messages.id FROM messages JOIN topics ON messages.topic_id = topics.id WHERE ((topics.name REGEXP '/lidar_front/points_raw')) AND (((timestamp = 1600901887524769241) AND (messages.id >= 0)) OR (timestamp > 1600901887524769241)) ORDER BY messages.timestamp, messages.id;'. SQLite error: (1): SQL logic error however, when issuing that same query directly via the sqlite3 command line interface, I get the correct results. |
It seems that for the |
This now works, you can pass multiple arguments to |
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.
This looks okay to me, though I wouldn't mind a second opinion from another reviewer.
I will review it in a couple days. |
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.
@esteve Overall looks good to me.
Among one nitpick with pointers initialization.
Also could you please write a few unit tests to cover new topics_regex
API ?
rosbag2_storage_default_plugins/src/rosbag2_storage_default_plugins/sqlite/sqlite_wrapper.cpp
Outdated
Show resolved
Hide resolved
@MichaelOrlov I've added a test based on the the existing |
Signed-off-by: Esteve Fernandez <esteve.fernandez@tier4.jp>
…thin sqlite3 Signed-off-by: Esteve Fernandez <esteve.fernandez@tier4.jp>
Signed-off-by: Esteve Fernandez <esteve.fernandez@tier4.jp>
Signed-off-by: Esteve Fernandez <esteve.fernandez@tier4.jp>
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.
@esteve Thank you for your contribution!
Now it looks good. Approving.
Running CI: |
* Added support for filtering topics via regular expressions Signed-off-by: Esteve Fernandez <esteve.fernandez@tier4.jp> * Added application function for evaluating regular expressions from within sqlite3 Signed-off-by: Esteve Fernandez <esteve.fernandez@tier4.jp> * Use nullptr for null pointers Signed-off-by: Esteve Fernandez <esteve.fernandez@tier4.jp> * Added tests for topics-regex API Signed-off-by: Esteve Fernandez <esteve.fernandez@tier4.jp>
…ressions (#1034)- (#1039) * Added support for filtering topics via regular expressions (#1034) * Added support for filtering topics via regular expressions Signed-off-by: Esteve Fernandez <esteve.fernandez@tier4.jp> * Added application function for evaluating regular expressions from within sqlite3 Signed-off-by: Esteve Fernandez <esteve.fernandez@tier4.jp> * Use nullptr for null pointers Signed-off-by: Esteve Fernandez <esteve.fernandez@tier4.jp> * Added tests for topics-regex API Signed-off-by: Esteve Fernandez <esteve.fernandez@tier4.jp> * Renamed --topics-regex to --regex and -e Signed-off-by: Esteve Fernandez <esteve.fernandez@tier4.jp>
This pull request has been mentioned on ROS Discourse. There might be relevant details there: https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-8-18-2022/27050/1 |
This PR adds a new option
--topics-regex
that allows filtering topics via regular expressions.