Log cat prints valid json with arrays

When used with the JSON flag, it is made sure that we make it array of
jsons so that it is parsable using json tools like python. jq tells that
{}{} is valid which ideally is not. Valid jsons should look like [{},{}]
so that none of the json parsers complain.

Change-Id: Idfadbccfb3977125778fd14932b67e01a852bc70
Signed-off-by: Naman Gupta <naman.gupta@bluerivertech.com>
Signed-off-by: Austin Schuh <austin.schuh@bluerivertech.com>
diff --git a/aos/aos_cli_utils.cc b/aos/aos_cli_utils.cc
index 2016a71..b930af5 100644
--- a/aos/aos_cli_utils.cc
+++ b/aos/aos_cli_utils.cc
@@ -249,7 +249,7 @@
 
     std::cout << "\"channel\": "
               << aos::configuration::StrippedChannelToString(channel)
-              << ", \"data\": " << *builder << "}\n";
+              << ", \"data\": " << *builder << "}";
   } else {
     if (!node_name.empty()) {
       std::cout << node_name << " ";
@@ -262,15 +262,15 @@
                   << context.realtime_remote_time << " ("
                   << context.monotonic_remote_time << ") "
                   << channel->name()->c_str() << ' ' << channel->type()->c_str()
-                  << ": " << *builder << "\n";
+                  << ": " << *builder;
       } else {
         std::cout << context.realtime_event_time << " ("
                   << context.monotonic_event_time << ") "
                   << channel->name()->c_str() << ' ' << channel->type()->c_str()
-                  << ": " << *builder << "\n";
+                  << ": " << *builder;
       }
     } else {
-      std::cout << *builder << '\n';
+      std::cout << *builder;
     }
   }
 }
@@ -291,4 +291,68 @@
   PrintMessage(node_name, channel, context, builder, options);
 }
 
+Printer::Printer(PrintOptions options, bool flush)
+    : options_(options), flush_(flush) {
+  if (options_.json) {
+    std::cout << "[";
+  }
+}
+
+Printer::~Printer() {
+  if (options_.json) {
+    if (message_count_ > 0) {
+      std::cout << "\n]\n";
+    } else {
+      std::cout << "]\n";
+    }
+  }
+}
+
+void Printer::PrintMessage(const std::string_view node_name,
+                           aos::NodeEventLoopFactory *node_factory,
+                           const aos::Channel *channel,
+                           const aos::Context &context) {
+  if (options_.json) {
+    if (message_count_ != 0) {
+      std::cout << ",\n  ";
+    } else {
+      std::cout << "\n  ";
+    }
+  }
+
+  aos::PrintMessage(node_name, node_factory, channel, context, &str_builder_,
+                    options_);
+
+  if (!options_.json) {
+    if (flush_) {
+      std::cout << std::endl;
+    } else {
+      std::cout << "\n";
+    }
+  }
+  ++message_count_;
+}
+
+void Printer::PrintMessage(const aos::Channel *channel,
+                           const aos::Context &context) {
+  if (options_.json) {
+    if (message_count_ != 0) {
+      std::cout << ",\n  ";
+    } else {
+      std::cout << "\n  ";
+    }
+  }
+
+  aos::PrintMessage(channel, context, &str_builder_, options_);
+
+  if (!options_.json) {
+    if (flush_) {
+      std::cout << std::endl;
+    } else {
+      std::cout << "\n";
+    }
+  }
+  ++message_count_;
+}
+
 }  // namespace aos
diff --git a/aos/aos_cli_utils.h b/aos/aos_cli_utils.h
index 7e143dd..9082188 100644
--- a/aos/aos_cli_utils.h
+++ b/aos/aos_cli_utils.h
@@ -37,6 +37,36 @@
 void PrintMessage(const aos::Channel *channel, const aos::Context &context,
                   aos::FastStringBuilder *builder, PrintOptions options);
 
+// RAII class to manage printing out sequences of messages, and to print them
+// all in a JSON array properly.
+class Printer {
+ public:
+  Printer(PrintOptions options, bool flush);
+  ~Printer();
+
+  // Number of messages that have been printed.
+  uint64_t message_count() const { return message_count_; }
+
+  // Prints a message.
+  void PrintMessage(const std::string_view node_name,
+                    aos::NodeEventLoopFactory *node_factory,
+                    const aos::Channel *channel, const aos::Context &context);
+  void PrintMessage(const aos::Channel *channel, const aos::Context &context);
+
+ private:
+  // Builder to make printing fast
+  aos::FastStringBuilder str_builder_;
+
+  // Number of messages
+  uint64_t message_count_ = 0;
+
+  // Options for printing.
+  const PrintOptions options_;
+
+  // If true, use std::endl to flush stdout, otherwise write a "\n"
+  const bool flush_;
+};
+
 // The information needed by the main function of a CLI tool.
 struct CliUtilInfo {
   // If this returns true, main should return immediately with 0.
diff --git a/aos/aos_dump.cc b/aos/aos_dump.cc
index 9f3d01f..573a06c 100644
--- a/aos/aos_dump.cc
+++ b/aos/aos_dump.cc
@@ -27,7 +27,8 @@
 DEFINE_int32(timeout, -1,
              "The max time in milliseconds to wait for messages before "
              "exiting.  -1 means forever, 0 means don't wait.");
-DEFINE_bool(use_hex, false, "Are integers in the messages printed in hex notation.");
+DEFINE_bool(use_hex, false,
+            "Are integers in the messages printed in hex notation.");
 
 int main(int argc, char **argv) {
   gflags::SetUsageMessage(
@@ -51,32 +52,32 @@
 
   uint64_t message_count = 0;
 
-  aos::FastStringBuilder str_builder;
-
   aos::monotonic_clock::time_point next_send_time =
       aos::monotonic_clock::min_time;
 
+  aos::Printer printer(
+      {
+          .pretty = FLAGS_pretty,
+          .max_vector_size = static_cast<size_t>(FLAGS_max_vector_size),
+          .pretty_max = FLAGS_pretty_max,
+          .print_timestamps = FLAGS_print_timestamps,
+          .json = FLAGS_json,
+          .distributed_clock = false,
+          .use_hex = FLAGS_use_hex,
+      },
+      /*flush*/ true);
+
   for (const aos::Channel *channel : cli_info.found_channels) {
     if (FLAGS_fetch) {
       const std::unique_ptr<aos::RawFetcher> fetcher =
           cli_info.event_loop->MakeRawFetcher(channel);
       if (fetcher->Fetch()) {
-        PrintMessage(
-            channel, fetcher->context(), &str_builder,
-            {
-                .pretty = FLAGS_pretty,
-                .max_vector_size = static_cast<size_t>(FLAGS_max_vector_size),
-                .pretty_max = FLAGS_pretty_max,
-                .print_timestamps = FLAGS_print_timestamps,
-                .json = FLAGS_json,
-                .distributed_clock = false,
-                .use_hex = FLAGS_use_hex,
-            });
+        printer.PrintMessage(channel, fetcher->context());
         ++message_count;
       }
     }
 
-    if (FLAGS_count > 0 && message_count >= FLAGS_count) {
+    if (FLAGS_count > 0 && printer.message_count() >= FLAGS_count) {
       return 0;
     }
 
@@ -85,28 +86,17 @@
     }
 
     cli_info.event_loop->MakeRawWatcher(
-        channel,
-        [channel, &str_builder, &cli_info, &message_count, &next_send_time](
-            const aos::Context &context, const void * /*message*/) {
+        channel, [channel, &printer, &cli_info, &next_send_time](
+                     const aos::Context &context, const void * /*message*/) {
           if (context.monotonic_event_time > next_send_time) {
-            if (FLAGS_count > 0 && message_count >= FLAGS_count) {
+            if (FLAGS_count > 0 && printer.message_count() >= FLAGS_count) {
               return;
             }
-            PrintMessage(channel, context, &str_builder,
-                         {
-                             .pretty = FLAGS_pretty,
-                             .max_vector_size =
-                                 static_cast<size_t>(FLAGS_max_vector_size),
-                             .pretty_max = FLAGS_pretty_max,
-                             .print_timestamps = FLAGS_print_timestamps,
-                             .json = FLAGS_json,
-                             .distributed_clock = false,
-                             .use_hex = FLAGS_use_hex,
-                         });
-            ++message_count;
+
+            printer.PrintMessage(channel, context);
             next_send_time = context.monotonic_event_time +
                              std::chrono::milliseconds(FLAGS_rate_limit);
-            if (FLAGS_count > 0 && message_count >= FLAGS_count) {
+            if (FLAGS_count > 0 && printer.message_count() >= FLAGS_count) {
               cli_info.event_loop->Exit();
             }
           }
diff --git a/aos/events/logging/log_cat.cc b/aos/events/logging/log_cat.cc
index f925651..da061ed 100644
--- a/aos/events/logging/log_cat.cc
+++ b/aos/events/logging/log_cat.cc
@@ -59,7 +59,8 @@
 DEFINE_double(monotonic_end_time, 0.0,
               "If set, only print messages sent at or before this many seconds "
               "after epoch.");
-DEFINE_bool(use_hex, false, "Are integers in the messages printed in hex notation.");
+DEFINE_bool(use_hex, false,
+            "Are integers in the messages printed in hex notation.");
 
 using aos::monotonic_clock;
 namespace chrono = std::chrono;
@@ -200,18 +201,16 @@
 // This class prints out all data from a node on a boot.
 class NodePrinter {
  public:
-  NodePrinter(aos::EventLoop *event_loop, uint64_t *message_print_counter,
-              aos::SimulatedEventLoopFactory *factory,
-              aos::FastStringBuilder *builder)
+  NodePrinter(aos::EventLoop *event_loop,
+              aos::SimulatedEventLoopFactory *factory, aos::Printer *printer)
       : factory_(factory),
         node_factory_(factory->GetNodeEventLoopFactory(event_loop->node())),
         event_loop_(event_loop),
-        message_print_counter_(message_print_counter),
         node_name_(
             event_loop_->node() == nullptr
                 ? ""
                 : std::string(event_loop->node()->name()->string_view())),
-        builder_(builder) {
+        printer_(printer) {
     event_loop_->SkipTimingReport();
     event_loop_->SkipAosLog();
 
@@ -253,6 +252,9 @@
           if (!FLAGS_print) {
             return;
           }
+          if (FLAGS_count > 0 && printer_->message_count() >= FLAGS_count) {
+            return;
+          }
 
           if (!FLAGS_fetch && !started_) {
             return;
@@ -263,19 +265,8 @@
             return;
           }
 
-          PrintMessage(
-              node_name_, node_factory_, channel, context, builder_,
-              {
-                  .pretty = FLAGS_pretty,
-                  .max_vector_size = static_cast<size_t>(FLAGS_max_vector_size),
-                  .pretty_max = FLAGS_pretty_max,
-                  .print_timestamps = FLAGS_print_timestamps,
-                  .json = FLAGS_json,
-                  .distributed_clock = FLAGS_distributed_clock,
-                  .use_hex = FLAGS_use_hex,
-              });
-          ++(*message_print_counter_);
-          if (FLAGS_count > 0 && *message_print_counter_ >= FLAGS_count) {
+          printer_->PrintMessage(node_name_, node_factory_, channel, context);
+          if (FLAGS_count > 0 && printer_->message_count() >= FLAGS_count) {
             factory_->Exit();
           }
         });
@@ -318,13 +309,11 @@
   aos::NodeEventLoopFactory *node_factory_;
   aos::EventLoop *event_loop_;
 
-  uint64_t *message_print_counter_ = nullptr;
-
   std::string node_name_;
 
   bool started_ = false;
 
-  aos::FastStringBuilder *builder_;
+  aos::Printer *printer_ = nullptr;
 };
 
 int main(int argc, char **argv) {
@@ -393,9 +382,17 @@
     }
   }
 
-  aos::FastStringBuilder builder;
-
-  uint64_t message_print_counter = 0;
+  aos::Printer printer(
+      {
+          .pretty = FLAGS_pretty,
+          .max_vector_size = static_cast<size_t>(FLAGS_max_vector_size),
+          .pretty_max = FLAGS_pretty_max,
+          .print_timestamps = FLAGS_print_timestamps,
+          .json = FLAGS_json,
+          .distributed_clock = FLAGS_distributed_clock,
+          .use_hex = FLAGS_use_hex,
+      },
+      false);
 
   std::vector<NodePrinter *> printers;
   printers.resize(aos::configuration::NodesCount(reader.configuration()),
@@ -413,12 +410,11 @@
     // notified when the log starts and stops.
     aos::NodeEventLoopFactory *node_factory =
         event_loop_factory.GetNodeEventLoopFactory(node);
-    node_factory->OnStartup([&event_loop_factory, node_factory,
-                             &message_print_counter, &builder, &printers,
-                             node_index]() {
-      printers[node_index] = node_factory->AlwaysStart<NodePrinter>(
-          "printer", &message_print_counter, &event_loop_factory, &builder);
-    });
+    node_factory->OnStartup(
+        [&event_loop_factory, node_factory, &printer, &printers, node_index]() {
+          printers[node_index] = node_factory->AlwaysStart<NodePrinter>(
+              "printer", &event_loop_factory, &printer);
+        });
     node_factory->OnShutdown(
         [&printers, node_index]() { printers[node_index] = nullptr; });