Open Ryanf55 opened 4 months ago
Hello @afrixs, can I get a review and approval is this solves your issue?
Hello @Ryanf55, thank you for looking into this! I'll review and test it on Monday
Hello @Ryanf55, thank you for looking into this! I'll review and test it on Monday
Thanks. If you have any bag files you can share to reproduce your failure, it would be much appreciated. I could cut them down and add a specific test for it.
Hello,
the fix looks good, I'm sending a minimalistic bag file which contains an empty grid map + chatter topic (unfortunately I cannot post the actual bag where I found the problem, since it contains map of a private property of a third person), along with tests to check for a valid and invalid requested topic. Note that it is a good practice to add a succeeding test as well to check that the test is well-written (in your case the chatter1
topic was missing /
at the beginning, so it would fail even if there was a GridMap message)
TEST(RosbagHandling, checkTopicTypes)
{
// This test validates loadFromBag will reject a topic of the wrong type or
// non-existing topic and accept a correct topic.
std::string pathToBag = "test_multitopic";
string topic_wrong = "/chatter";
string topic_correct = "/grid_map";
string topic_non_existing = "/grid_map_non_existing";
GridMap gridMapOut;
rcpputils::fs::path dir(pathToBag);
EXPECT_FALSE(GridMapRosConverter::loadFromBag(pathToBag, topic_wrong, gridMapOut));
EXPECT_TRUE(GridMapRosConverter::loadFromBag(pathToBag, topic_correct, gridMapOut));
EXPECT_FALSE(GridMapRosConverter::loadFromBag(pathToBag, topic_non_existing, gridMapOut));
}
Also, there are still some linting/uncrustify errors to fix in your code
1: /__w/grid_map/grid_map/src/grid_map/grid_map_ros/src/GridMapRosConverter.cpp:724: Lines should be <= 100 characters long [whitespace/line_length] [2]
1: /__w/grid_map/grid_map/src/grid_map/grid_map_ros/src/GridMapRosConverter.cpp:727: Missing space around colon in range-based for loop [whitespace/forcolon] [2]
1: /__w/grid_map/grid_map/src/grid_map/grid_map_ros/src/GridMapRosConverter.cpp:735: Lines should be <= 100 characters long [whitespace/line_length] [2]
1: /__w/grid_map/grid_map/src/grid_map/grid_map_ros/src/GridMapRosConverter.cpp:746: Line ends in whitespace. Consider deleting these extra spaces. [whitespace/end_of_line] [4]
5: @@ -727 +727 @@
5: - for (const auto& m: topic_metadata) {
5: + for (const auto & m: topic_metadata) {
5: @@ -735 +735,2 @@
5: - "loadFromBag"), "Bagfile does not contain a GridMap message on the expected topic '%s'", topic.c_str());
5: + "loadFromBag"), "Bagfile does not contain a GridMap message on the expected topic '%s'",
5: + topic.c_str());
5: @@ -746 +747 @@
5: - // https://github.com/ANYbotics/grid_map/issues/401
5: + // https://github.com/ANYbotics/grid_map/issues/401
Description
Demo
Details
I uploaded a super small bagfile of two chatter topics. It's very small:
It might be better to construct the bag file on the fly in the test case, but that would take much more time than what I did here.
Issue
Solves #401