From ad4e4434e441298e680ce6e6273af74aed71a94b Mon Sep 17 00:00:00 2001 From: Wyatt Alt Date: Fri, 6 May 2022 09:17:11 -0700 Subject: [PATCH] CLI: Fix zero-valued frequencies (#381) Fixes a bug introduced recently that caused frequencies in `info` output to be zero due to unintentional shadowing of a global variable used for the cat command. Use of global variables is an artifact of cobra's CLI command code generation. We can get away from that with a bit of refactoring but haven't done so yet. As a stopgap, this commit also introduces a convention of prefixing the globals with the name of the related command. --- go/cli/mcap/cmd/cat.go | 28 ++++++++++++++-------------- go/cli/mcap/cmd/convert.go | 32 +++++++++++++++++--------------- go/cli/mcap/cmd/info.go | 5 +++-- 3 files changed, 34 insertions(+), 31 deletions(-) diff --git a/go/cli/mcap/cmd/cat.go b/go/cli/mcap/cmd/cat.go index c8eeba92be..58f0a4ef6b 100644 --- a/go/cli/mcap/cmd/cat.go +++ b/go/cli/mcap/cmd/cat.go @@ -26,10 +26,10 @@ import ( ) var ( - topics string - start int64 - end int64 - formatJSON bool + catTopics string + catStart int64 + catEnd int64 + catFormatJSON bool ) type DecimalTime uint64 @@ -175,12 +175,12 @@ var catCmd = &cobra.Command{ if err != nil { log.Fatalf("Failed to create reader: %s", err) } - topics := strings.FieldsFunc(topics, func(c rune) bool { return c == ',' }) - it, err := reader.Messages(start*1e9, end*1e9, topics, false) + topics := strings.FieldsFunc(catTopics, func(c rune) bool { return c == ',' }) + it, err := reader.Messages(catStart*1e9, catEnd*1e9, topics, false) if err != nil { log.Fatalf("Failed to read messages: %s", err) } - err = printMessages(ctx, os.Stdout, it, formatJSON) + err = printMessages(ctx, os.Stdout, it, catFormatJSON) if err != nil { log.Fatalf("Failed to print messages: %s", err) } @@ -197,12 +197,12 @@ var catCmd = &cobra.Command{ if err != nil { return fmt.Errorf("failed to create reader: %w", err) } - topics := strings.FieldsFunc(topics, func(c rune) bool { return c == ',' }) - it, err := reader.Messages(start*1e9, end*1e9, topics, true) + topics := strings.FieldsFunc(catTopics, func(c rune) bool { return c == ',' }) + it, err := reader.Messages(catStart*1e9, catEnd*1e9, topics, true) if err != nil { return fmt.Errorf("failed to read messages: %w", err) } - err = printMessages(ctx, os.Stdout, it, formatJSON) + err = printMessages(ctx, os.Stdout, it, catFormatJSON) if err != nil { return fmt.Errorf("failed to print messages: %w", err) } @@ -217,8 +217,8 @@ var catCmd = &cobra.Command{ func init() { rootCmd.AddCommand(catCmd) - catCmd.PersistentFlags().Int64VarP(&start, "start-secs", "", 0, "start time") - catCmd.PersistentFlags().Int64VarP(&end, "end-secs", "", math.MaxInt64, "end time") - catCmd.PersistentFlags().StringVarP(&topics, "topics", "", "", "comma-separated list of topics") - catCmd.PersistentFlags().BoolVarP(&formatJSON, "json", "", false, "print messages as JSON") + catCmd.PersistentFlags().Int64VarP(&catStart, "start-secs", "", 0, "start time") + catCmd.PersistentFlags().Int64VarP(&catEnd, "end-secs", "", math.MaxInt64, "end time") + catCmd.PersistentFlags().StringVarP(&catTopics, "topics", "", "", "comma-separated list of topics") + catCmd.PersistentFlags().BoolVarP(&catFormatJSON, "json", "", false, "print messages as JSON") } diff --git a/go/cli/mcap/cmd/convert.go b/go/cli/mcap/cmd/convert.go index d9a8ccd67f..9c84d95cfc 100644 --- a/go/cli/mcap/cmd/convert.go +++ b/go/cli/mcap/cmd/convert.go @@ -20,11 +20,13 @@ var ( db3Magic = []byte{0x53, 0x51, 0x4c, 0x69, 0x74, 0x65, 0x20, 0x66, 0x6f, 0x72, 0x6d, 0x61, 0x74, 0x20, 0x33, 0x00} ) -var amentPrefixPath string -var compression string -var chunkSize int64 -var includeCRC bool -var chunked bool +var ( + convertAmentPrefixPath string + convertCompression string + convertChunkSize int64 + convertIncludeCRC bool + convertChunked bool +) type FileType string @@ -83,7 +85,7 @@ var convertCmd = &cobra.Command{ defer w.Close() var compressionFormat mcap.CompressionFormat - switch compression { + switch convertCompression { case "lz4": compressionFormat = mcap.CompressionLZ4 case "zstd": @@ -93,9 +95,9 @@ var convertCmd = &cobra.Command{ } opts := &mcap.WriterOptions{ - IncludeCRC: includeCRC, - Chunked: chunked, - ChunkSize: chunkSize, + IncludeCRC: convertIncludeCRC, + Chunked: convertChunked, + ChunkSize: convertChunkSize, Compression: compressionFormat, } @@ -112,7 +114,7 @@ var convertCmd = &cobra.Command{ die("failed to open sqlite3: %s", err) } - amentPath := amentPrefixPath + amentPath := convertAmentPrefixPath prefixPath := os.Getenv("AMENT_PREFIX_PATH") if prefixPath != "" { amentPath += ":" + prefixPath @@ -131,35 +133,35 @@ var convertCmd = &cobra.Command{ func init() { rootCmd.AddCommand(convertCmd) convertCmd.PersistentFlags().StringVarP( - &amentPrefixPath, + &convertAmentPrefixPath, "ament-prefix-path", "", "", "(ros2 only) colon-separated list of directories to search for message definitions (e.g /opt/ros/galactic:/opt/ros/noetic)", ) convertCmd.PersistentFlags().StringVarP( - &compression, + &convertCompression, "compression", "", "zstd", "chunk compression algorithm (supported: zstd, lz4, none)", ) convertCmd.PersistentFlags().Int64VarP( - &chunkSize, + &convertChunkSize, "chunk-size", "", 8*1024*1024, "chunk size to target", ) convertCmd.PersistentFlags().BoolVarP( - &includeCRC, + &convertIncludeCRC, "include-crc", "", true, "include chunk CRC checksums in output", ) convertCmd.PersistentFlags().BoolVarP( - &chunked, + &convertChunked, "chunked", "", true, diff --git a/go/cli/mcap/cmd/info.go b/go/cli/mcap/cmd/info.go index 26b976eb68..467066120d 100644 --- a/go/cli/mcap/cmd/info.go +++ b/go/cli/mcap/cmd/info.go @@ -31,10 +31,11 @@ func printInfo(w io.Writer, info *mcap.Info) error { buf := &bytes.Buffer{} fmt.Fprintf(buf, "library: %s\n", info.Header.Library) fmt.Fprintf(buf, "profile: %s\n", info.Header.Profile) + var start, end uint64 if info.Statistics != nil { fmt.Fprintf(buf, "messages: %d\n", info.Statistics.MessageCount) - start := info.Statistics.MessageStartTime - end := info.Statistics.MessageEndTime + start = info.Statistics.MessageStartTime + end = info.Statistics.MessageEndTime starttime := time.Unix(int64(start/1e9), int64(start%1e9)) endtime := time.Unix(int64(end/1e9), int64(end%1e9)) fmt.Fprintf(buf, "duration: %s\n", endtime.Sub(starttime))