Skip to content

Commit

Permalink
export: fix bag reindexing (#141)
Browse files Browse the repository at this point in the history
### Changelog

- Fixed: previously ROS 1 bag files downloaded using the foxglove CLI
would fail with an "unindexed bag" error. These files should now
download correctly.

### Docs


### Description

Since #124, the code to reindex exported bag files has been broken - it
first tries to use a seeking reader to read the unindexed file (which
will not work), then it fails to close the reindexing writer in the
happy path. This PR fixes that and adds a test.
  • Loading branch information
james-rms authored May 8, 2024
1 parent fc12837 commit bfd0eec
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 2 deletions.
4 changes: 2 additions & 2 deletions foxglove/cmd/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ func reindexBagFile(w io.Writer, r io.Reader) error {
if err != nil {
return fmt.Errorf("failed to construct bag reader: %w", err)
}
it, err := reader.Messages()
it, err := reader.Messages(rosbag.ScanLinear(true))
if err != nil {
return fmt.Errorf("failed to construct bag iterator: %w", err)
}
Expand Down Expand Up @@ -233,7 +233,7 @@ func reindexBagFile(w io.Writer, r io.Reader) error {
return err
}
}
return nil
return writer.Close()
}

// reindexMCAPFile rewrites an MCAP file to a new output location, and properly
Expand Down
38 changes: 38 additions & 0 deletions foxglove/cmd/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"
"io"
"os"
"path/filepath"
"testing"
"time"

Expand All @@ -16,6 +17,7 @@ import (
"github.com/foxglove/go-rosbag"
"github.com/foxglove/mcap/go/mcap"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func withStdoutRedirected(output io.Writer, f func()) error {
Expand Down Expand Up @@ -407,3 +409,39 @@ func TestExportCommand(t *testing.T) {
assert.Equal(t, 30445, count)
})
}

func copyTo(t *testing.T, from, to string) {
infile, err := os.Open(from)
require.NoError(t, err)
defer func() {
require.NoError(t, infile.Close())
}()
outfile, err := os.Create(to)
require.NoError(t, err)
defer func() {
require.NoError(t, outfile.Close())
}()
_, err = io.Copy(outfile, infile)
require.NoError(t, err)
}

func TestReindexBag(t *testing.T) {
workingPath := filepath.Join(t.TempDir(), "gps.bag.active")
copyTo(t, "../testdata/gps.bag.active", workingPath)
didReindex, info, err := reindex(t.TempDir(), workingPath, "bag1")
require.NoError(t, err)
require.True(t, didReindex)
require.Equal(t, 30445, int(info.messageCount))

// check that the new bag is indexed by retrieving Info() from it
fd, err := os.Open(workingPath)
require.NoError(t, err)
defer func() {
require.NoError(t, fd.Close())
}()
reader, err := rosbag.NewReader(fd)
require.NoError(t, err)
bagInfo, err := reader.Info()
require.NoError(t, err)
require.Equal(t, 30445, int(bagInfo.MessageCount))
}
Binary file added foxglove/testdata/gps.bag.active
Binary file not shown.

0 comments on commit bfd0eec

Please sign in to comment.