Skip to content

Commit

Permalink
Handle invalid metadata in bag files (#196)
Browse files Browse the repository at this point in the history
If the bag database was unable to parse the metadata in a bag file,
it would throw an exception and cause it to completely fail to read the
bag file.  This change will make it log an error but continue processing
the file.

Signed-off-by: P. J. Reed <phillipreed@hatchbed.com>

Signed-off-by: P. J. Reed <phillipreed@hatchbed.com>
  • Loading branch information
pjreed authored Sep 9, 2022
1 parent fee6c93 commit 518c602
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 6 deletions.
4 changes: 2 additions & 2 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -414,9 +414,9 @@

<!-- Reading and deserializing bag files -->
<dependency>
<groupId>com.github.swri-robotics</groupId>
<groupId>io.github.hatchbed</groupId>
<artifactId>bag-reader-java</artifactId>
<version>1.10.3</version>
<version>2.0.0</version>
</dependency>

</dependencies>
Expand Down
7 changes: 4 additions & 3 deletions src/main/java/com/github/swrirobotics/bags/BagService.java
Original file line number Diff line number Diff line change
Expand Up @@ -1292,7 +1292,7 @@ public String getVehicleName(BagFile bag) {
* @param bagFile The bag file to read metadata for.
* @return A map of all of the key:value metadata in the bag.
*/
private Map<String, String> getMetadata(BagFile bagFile) {
public Map<String, String> getMetadata(BagFile bagFile) {
final Map<String, String> tags = Maps.newHashMap();
String[] topics = myConfigService.getConfiguration().getMetadataTopics();
try {
Expand All @@ -1306,8 +1306,9 @@ private Map<String, String> getMetadata(BagFile bagFile) {
myLogger.debug("Examining message: " + data);
tags.putAll(lineSplitter.split(data));
}
catch (UninitializedFieldException e) {
// continue
catch (IllegalArgumentException | UninitializedFieldException e) {
reportStatus(Status.State.ERROR,
"Unable to parse metadata on topic " + topic + " in bag file " + bagFile.getPath());
}
return true;
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
import java.util.HashMap;

import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.springframework.restdocs.headers.HeaderDocumentation.headerWithName;
import static org.springframework.restdocs.headers.HeaderDocumentation.requestHeaders;
Expand All @@ -84,7 +85,7 @@ public void close() throws IOException {

@Override
public BagFile getBagFile() throws BagReaderException {
return new BagFile("/test.bag");
return mock(BagFile.class);
}

@Override
Expand Down
50 changes: 50 additions & 0 deletions src/test/java/com/github/swrirobotics/bags/BagServiceTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,68 @@

package com.github.swrirobotics.bags;

import com.github.swrirobotics.bags.reader.BagFile;
import com.github.swrirobotics.bags.reader.MessageHandler;
import com.github.swrirobotics.bags.reader.exceptions.BagReaderException;
import com.github.swrirobotics.bags.reader.messages.serialization.MessageCollection;
import com.github.swrirobotics.bags.reader.messages.serialization.MessageType;
import com.github.swrirobotics.bags.reader.messages.serialization.StringType;
import com.github.swrirobotics.config.ConfigService;
import com.github.swrirobotics.config.WebAppConfigurationAware;
import com.github.swrirobotics.support.web.Configuration;
import org.junit.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.mock.mockito.MockBean;

import java.awt.*;
import java.awt.image.BufferedImage;

import static org.junit.Assert.*;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.*;

public class BagServiceTest extends WebAppConfigurationAware {
@Autowired
BagService myBagService;
@MockBean
ConfigService myConfigService;

private static final Logger myLogger = LoggerFactory.getLogger(BagServiceTest.class);

@Test
public void testGetMetadata() throws BagReaderException {
// Set up a mock bag file to provide some test data to the service
BagFile mockBagFile = mock(BagFile.class);
myLogger.info("testGetMetadata");
Configuration tmpConfig = new Configuration();
// Set up one topic that has good data on it, and one topic that has bad data
tmpConfig.setMetadataTopics(new String[]{"good", "bad"});
when(myConfigService.getConfiguration()).thenReturn(tmpConfig);
doAnswer((i) -> {
// Create a standard ROS std_msgs/String message
MessageType msg = new MessageType("MSG: std_msgs/String\nstring data", new MessageCollection());
StringType str = msg.getField("data");
boolean isGood = i.getArgument(0, String.class).equals("good");
if (isGood) {
str.setValue("name: John Doe\nemail: jdoe@example.com\n");
}
else {
str.setValue("name: John Doe\nemail: jdoe@example.com\nbadvalue\n");
}
var handler = i.getArgument(1, MessageHandler.class);
handler.process(msg, null);
myLogger.info(i.getArgument(1).toString());
return null;
}).when(mockBagFile).forMessagesOnTopic(any(), any());

var metadata = myBagService.getMetadata(mockBagFile);

assertEquals(2, metadata.size());
assertEquals("John Doe", metadata.get("name"));
assertEquals("jdoe@example.com", metadata.get("email"));
}

@Test
public void testDecodeBgra() {
Expand Down

0 comments on commit 518c602

Please sign in to comment.