Add topics with zero message counts to the SQLiteStorage::get_metadata(). #1725

fujitatomoya · 2024-06-22T20:35:00Z

Address Ros bag reindex failed to generate topic inside metadata.yaml if the message_count is 0 #1719 for rolling.
I guess we can backport this to jazzy.

Signed-off-by: Tomoya Fujita <[email protected]>

rosbag2_storage_sqlite3/src/rosbag2_storage_sqlite3/sqlite_storage.cpp

MichaelOrlov

@fujitatomoya Honestly, I am not a fan of overcomplicated SQL requests.
I found a more simple and elegant solution. See my implementation for Humble #1722

fujitatomoya · 2024-06-23T04:13:36Z

@fujitatomoya Honestly, I am not a fan of overcomplicated SQL requests. I found a more simple and elegant solution. See my implementation for Humble #1722

@MichaelOrlov hmmm, i prefer to let the SQL do the job since it is meant to be queried those index data... calling SQL query multiple times and then constructing the data in the program would be more complicated IMO... i guess current SQL query is not complicated at all, seems to me really straight forward.

but if you want to take your fix, that is fine too.

besides, i think there are slightly different from humble and rolling, right? so if you want to take this PR, i can manage rolling and jazzy on my side. what do you think?

MichaelOrlov · 2024-06-23T06:40:41Z

@fujitatomoya As regards the extra SQL request, we actually have to do this extra SQL request, SqliteStorage::fill_topics_and_types(), anyway at the very beginning, on construction or during storage opening.

Yes. the implementation is slightly different on Rolling and Jazzy, this is why I started from Humble.
However, I would prefer my version of the fix. I would appreciate it if you would take it for rolling and jazzy.

fujitatomoya · 2024-06-23T23:06:15Z

@MichaelOrlov i do not have strong opinion on this, so okay to take your fix. do you want me to work on rolling and jazzy? or we should wait #1722 and backport it to rolling and jazzy?

MichaelOrlov · 2024-06-24T07:08:49Z

@fujitatomoya I would appreciate it if you could prepare PR for Rolling, and then we can backport it to Jazzy.

fujitatomoya · 2024-06-24T17:21:10Z

@MichaelOrlov okay, i will borrow your code and adjust it to rolling, then backport to jazzy!

…y messages." This reverts commit a842689. Signed-off-by: Tomoya Fujita <[email protected]>

Signed-off-by: Tomoya Fujita <[email protected]>

Signed-off-by: Tomoya Fujita <[email protected]> Co-authored-by: Michael Orlov <[email protected]>

fujitatomoya · 2024-06-24T23:58:03Z

@MichaelOrlov it now aligns with #1722, CI is passing on my local env. can you take a look? just a slightly structure is different from humble one but complicated. (note, added you as co-author since i borrowed your code.)

MichaelOrlov

@fujitatomoya Thanks, overall looks good.
Only one concern about failure on the Build_and_Test job. It seems we missed dependencies from the std_msgs package for test.

Signed-off-by: Tomoya Fujita <[email protected]>

fujitatomoya · 2024-06-25T20:22:44Z

@MichaelOrlov https://build.ros2.org/job/Rpr__rosbag2__ubuntu_noble_amd64/235/ seems unrelated to this change, everything else is green.

MichaelOrlov

LGTM with green CI from buildfarm.

MichaelOrlov

@fujitatomoya Sorry for the late call - but I found a bug in the implementation.
When creating a TopicMetadata object, we need to use mapping from the inner_topic_id to the extern_topic_id for further comparison with an existing object.

rosbag2_storage_sqlite3/src/rosbag2_storage_sqlite3/sqlite_storage.cpp

Signed-off-by: Tomoya Fujita <[email protected]>

fujitatomoya · 2024-06-25T23:50:53Z

CI:

Linux
Linux-aarch64
Linux-rhel
Windows

MichaelOrlov

LGTM.

fujitatomoya · 2024-06-26T16:06:33Z

@MichaelOrlov can you merge this?

MichaelOrlov · 2024-06-26T17:51:51Z

https://github.com/Mergifyio backport jazzy

mergify · 2024-06-26T17:51:59Z

backport jazzy

✅ Backports have been created

#1731 Add topics with zero message counts to the SQLiteStorage::get_metadata(). (backport #1725) has been created for branch jazzy

…a(). (#1725) * storage_sqlite3 constructs metadata with topics don't have any messages. Signed-off-by: Tomoya Fujita <[email protected]> * Revert "storage_sqlite3 constructs metadata with topics don't have any messages." This reverts commit a842689. Signed-off-by: Tomoya Fujita <[email protected]> * update metadata list topics that does not have any messages. Signed-off-by: Tomoya Fujita <[email protected]> * add get_metadata_include_topics_with_zero_messages test Signed-off-by: Tomoya Fujita <[email protected]> Co-authored-by: Michael Orlov <[email protected]> * add std_msgs to rosbag2_cpp for test build. Signed-off-by: Tomoya Fujita <[email protected]> * call get_or_generate_extern_topic_id when updating metadata. Signed-off-by: Tomoya Fujita <[email protected]> --------- Signed-off-by: Tomoya Fujita <[email protected]> Co-authored-by: Michael Orlov <[email protected]> (cherry picked from commit ff829e8)

…a(). (#1725) (#1731) * storage_sqlite3 constructs metadata with topics don't have any messages. Signed-off-by: Tomoya Fujita <[email protected]> * Revert "storage_sqlite3 constructs metadata with topics don't have any messages." This reverts commit a842689. Signed-off-by: Tomoya Fujita <[email protected]> * update metadata list topics that does not have any messages. Signed-off-by: Tomoya Fujita <[email protected]> * add get_metadata_include_topics_with_zero_messages test Signed-off-by: Tomoya Fujita <[email protected]> Co-authored-by: Michael Orlov <[email protected]> * add std_msgs to rosbag2_cpp for test build. Signed-off-by: Tomoya Fujita <[email protected]> * call get_or_generate_extern_topic_id when updating metadata. Signed-off-by: Tomoya Fujita <[email protected]> --------- Signed-off-by: Tomoya Fujita <[email protected]> Co-authored-by: Michael Orlov <[email protected]> (cherry picked from commit ff829e8) Co-authored-by: Tomoya Fujita <[email protected]>

…a(). (#1725) (#1731) * storage_sqlite3 constructs metadata with topics don't have any messages. Signed-off-by: Tomoya Fujita <[email protected]> * Revert "storage_sqlite3 constructs metadata with topics don't have any messages." This reverts commit a842689. Signed-off-by: Tomoya Fujita <[email protected]> * update metadata list topics that does not have any messages. Signed-off-by: Tomoya Fujita <[email protected]> * add get_metadata_include_topics_with_zero_messages test Signed-off-by: Tomoya Fujita <[email protected]> Co-authored-by: Michael Orlov <[email protected]> * add std_msgs to rosbag2_cpp for test build. Signed-off-by: Tomoya Fujita <[email protected]> * call get_or_generate_extern_topic_id when updating metadata. Signed-off-by: Tomoya Fujita <[email protected]> --------- Signed-off-by: Tomoya Fujita <[email protected]> Co-authored-by: Michael Orlov <[email protected]> (cherry picked from commit ff829e8) Co-authored-by: Tomoya Fujita <[email protected]> (cherry picked from commit 2a627c9) # Conflicts: # rosbag2_cpp/CMakeLists.txt # rosbag2_storage_sqlite3/src/rosbag2_storage_sqlite3/sqlite_storage.cpp

storage_sqlite3 constructs metadata with topics don't have any messages.

a842689

Signed-off-by: Tomoya Fujita <[email protected]>

fujitatomoya requested a review from a team as a code owner June 22, 2024 20:35

fujitatomoya requested review from MichaelOrlov and jhdcs and removed request for a team June 22, 2024 20:35

fujitatomoya mentioned this pull request Jun 22, 2024

[Humble] Add topics with zero message counts to the SQLiteStorage::get_metadata(). #1722

Merged

fujitatomoya commented Jun 22, 2024

View reviewed changes

rosbag2_storage_sqlite3/src/rosbag2_storage_sqlite3/sqlite_storage.cpp Outdated Show resolved Hide resolved

MichaelOrlov requested changes Jun 22, 2024

View reviewed changes

fujitatomoya and others added 3 commits June 24, 2024 14:59

Revert "storage_sqlite3 constructs metadata with topics don't have an…

1bb606e

…y messages." This reverts commit a842689. Signed-off-by: Tomoya Fujita <[email protected]>

update metadata list topics that does not have any messages.

1ab0cc2

Signed-off-by: Tomoya Fujita <[email protected]>

add get_metadata_include_topics_with_zero_messages test

5e6843a

Signed-off-by: Tomoya Fujita <[email protected]> Co-authored-by: Michael Orlov <[email protected]>

MichaelOrlov reviewed Jun 25, 2024

View reviewed changes

add std_msgs to rosbag2_cpp for test build.

9272e5b

Signed-off-by: Tomoya Fujita <[email protected]>

MichaelOrlov approved these changes Jun 25, 2024

View reviewed changes

MichaelOrlov requested changes Jun 25, 2024

View reviewed changes

rosbag2_storage_sqlite3/src/rosbag2_storage_sqlite3/sqlite_storage.cpp Outdated Show resolved Hide resolved

call get_or_generate_extern_topic_id when updating metadata.

1708c92

Signed-off-by: Tomoya Fujita <[email protected]>

MichaelOrlov approved these changes Jun 26, 2024

View reviewed changes

MichaelOrlov changed the title ~~storage_sqlite3 constructs metadata with topics don't have any messages.~~ Add topics with zero messages to the SQLiteStorage::get_metadata() Jun 26, 2024

MichaelOrlov changed the title ~~Add topics with zero messages to the SQLiteStorage::get_metadata()~~ Add topics with zero message counts to the SQLiteStorage::get_metadata(). Jun 26, 2024

MichaelOrlov merged commit ff829e8 into ros2:rolling Jun 26, 2024
13 of 14 checks passed

mergify bot mentioned this pull request Jun 26, 2024

[jazzy] Add topics with zero message counts to the SQLiteStorage::get_metadata(). (backport #1725) #1731

Merged

mergify bot mentioned this pull request Jul 27, 2024

[Iron] Add topics with zero message counts to the SQLiteStorage::get_metadata(). (backport #1722) #1766

Merged

mergify bot mentioned this pull request Aug 9, 2024

[iron] Add topics with zero message counts to the SQLiteStorage::get_metadata(). (backport #1725) (backport #1731) #1783

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add topics with zero message counts to the SQLiteStorage::get_metadata(). #1725

Add topics with zero message counts to the SQLiteStorage::get_metadata(). #1725

fujitatomoya commented Jun 22, 2024 •

edited by MichaelOrlov

Loading

MichaelOrlov left a comment

fujitatomoya commented Jun 23, 2024

MichaelOrlov commented Jun 23, 2024 •

edited

Loading

fujitatomoya commented Jun 23, 2024

MichaelOrlov commented Jun 24, 2024

fujitatomoya commented Jun 24, 2024

fujitatomoya commented Jun 24, 2024

MichaelOrlov left a comment

fujitatomoya commented Jun 25, 2024

MichaelOrlov left a comment

MichaelOrlov left a comment

fujitatomoya commented Jun 25, 2024 •

edited

Loading

MichaelOrlov left a comment

fujitatomoya commented Jun 26, 2024

MichaelOrlov commented Jun 26, 2024

mergify bot commented Jun 26, 2024 •

edited

Loading

Add topics with zero message counts to the SQLiteStorage::get_metadata(). #1725

Add topics with zero message counts to the SQLiteStorage::get_metadata(). #1725

Conversation

fujitatomoya commented Jun 22, 2024 • edited by MichaelOrlov Loading

MichaelOrlov left a comment

Choose a reason for hiding this comment

fujitatomoya commented Jun 23, 2024

MichaelOrlov commented Jun 23, 2024 • edited Loading

fujitatomoya commented Jun 23, 2024

MichaelOrlov commented Jun 24, 2024

fujitatomoya commented Jun 24, 2024

fujitatomoya commented Jun 24, 2024

MichaelOrlov left a comment

Choose a reason for hiding this comment

fujitatomoya commented Jun 25, 2024

MichaelOrlov left a comment

Choose a reason for hiding this comment

MichaelOrlov left a comment

Choose a reason for hiding this comment

fujitatomoya commented Jun 25, 2024 • edited Loading

MichaelOrlov left a comment

Choose a reason for hiding this comment

fujitatomoya commented Jun 26, 2024

MichaelOrlov commented Jun 26, 2024

mergify bot commented Jun 26, 2024 • edited Loading

✅ Backports have been created

fujitatomoya commented Jun 22, 2024 •

edited by MichaelOrlov

Loading

MichaelOrlov commented Jun 23, 2024 •

edited

Loading

fujitatomoya commented Jun 25, 2024 •

edited

Loading

mergify bot commented Jun 26, 2024 •

edited

Loading