[tools] Add drake_lcm_cc_library opt-in for new lcm_gen (RobotLocomot…

…ion#20903)

Manual testing of flipping the default for this flag pointed out some
embarrassing problems with the new tool, which are also fixed here:

- Messages with sub-structs must include the headers for those structs.

- The getTypeName function is actually required downstream.

- Generated filenames must be `*.hpp` not `*.h`. Ideally we would have
  started a new filename convention with the new tool, but so much old
  code expects the messages to keep their old names that renaming is
  intractable in practice.

lcmtypes/BUILD.bazel

@@ -3,6 +3,7 @@ package(default_visibility = ["//visibility:public"])
 load("//tools/install:install.bzl", "install")
 load(
     "//tools/skylark:drake_cc.bzl",
+    "drake_cc_googletest",
     "drake_cc_library",
     "drake_transitive_installed_hdrs_filegroup",
 )
@@ -21,6 +22,7 @@ load(
     "drake_lcm_java_library",
     "drake_lcm_py_library",
 )
+load("//tools/workspace:generate_file.bzl", "generate_file")
 load(":defs.bzl", "ALL_LCM_SRCS")
 load("//tools/lint:lint.bzl", "add_lint_tests")
 
@@ -389,6 +391,34 @@ install(
 )
 
 # === test/ ===
+
+# TODO(jwnimmer-tri) We use this to test the `use_new_lcm_gen = True` flag.
+# Once we turn on that flag by default, we can probably ditch this test.
+generate_file(
+    name = "test_message.lcm",
+    content = """
+package drake;
+struct test_message { double value; }
+""",
+    visibility = ["//visibility:private"],
+)
+
+drake_lcm_cc_library(
+    name = "test_message",
+    lcm_package = "drake",
+    lcm_srcs = ["test_message.lcm"],
+    use_new_lcm_gen = True,
+    visibility = ["//visibility:private"],
+)
+
+drake_cc_googletest(
+    name = "test_message_test",
+    deps = [
+        ":test_message",
+        "//lcm:lcm_messages",
+    ],
+)
+
 drake_py_test(
     name = "nested_types_test",
     deps = [

lcmtypes/test/test_message_test.cc

@@ -0,0 +1,20 @@
+#include "drake/test_message.hpp"
+
+#include <gtest/gtest.h>
+
+#include "drake/lcm/lcm_messages.h"
+
+namespace drake {
+namespace lcm {
+namespace {
+
+GTEST_TEST(LcmTestMessageTest, Smoke) {
+  test_message foo{.value = 22.0};
+  const std::vector<uint8_t> bytes = EncodeLcmMessage(foo);
+  auto readback = DecodeLcmMessage<test_message>(bytes);
+  EXPECT_EQ(readback.value, foo.value);
+}
+
+}  // namespace
+}  // namespace lcm
+}  // namespace drake

tools/lcm_gen/BUILD.bazel

@@ -27,15 +27,16 @@ drake_py_binary(
         # it here to avoid duplicate linter complaints.
         "nolint",
     ],
+    visibility = ["//:__subpackages__"],
     deps = [":module_py"],
 )
 
 drake_py_unittest(
     name = "lcm_gen_test",
     data = [
-        "test/goal/lima.h",
-        "test/goal/mike.h",
-        "test/goal/november.h",
+        "test/goal/lima.hpp",
+        "test/goal/mike.hpp",
+        "test/goal/november.hpp",
         "test/lima.lcm",
         "test/mike.lcm",
         "test/november.lcm",
@@ -133,9 +134,9 @@ cc_library(
     name = "papa",
     testonly = True,
     hdrs = [
-        "test/goal/lima.h",
-        "test/goal/mike.h",
-        "test/goal/november.h",
+        "test/goal/lima.hpp",
+        "test/goal/mike.hpp",
+        "test/goal/november.hpp",
     ],
     include_prefix = "papa",
     strip_include_prefix = "test/goal",

tools/lcm_gen/__init__.py

@@ -351,6 +351,8 @@ def _qualified_identifier(self):
 #include <utility>
 #include <vector>
 
+@@SUBSTRUCT_INCLUDES@@
+
 @@NAMESPACE_BEGIN@@
 
 class @@STRUCT_NAME@@ {
@@ -360,6 +362,7 @@ class @@STRUCT_NAME@@ {
   // These functions match the expected API from the legacy lcm-gen tool,
   // but note that we use `int64_t` instead of `int` for byte counts.
   //@{
+  static const char* getTypeName() { return "@@STRUCT_NAME@@"; }
   int64_t getEncodedSize() const { return 8 + _getEncodedSizeNoHash(); }
   int64_t _getEncodedSizeNoHash() const {
     int64_t _result = 0;
@@ -614,6 +617,7 @@ def __init__(self, struct):
     def generate(self):
         """Returns the C++ text for the message provided in the constructor."""
         self._result = _CPP_TEMPLATE
+        self._fill_includes()
         self._fill_names()
         self._fill_member_constants()
         self._fill_member_fields()
@@ -629,6 +633,20 @@ def _replace(self, old, new):
         assert updated != self._result
         self._result = updated
 
+    def _fill_includes(self):
+        filenames = [
+            f"{field.typ.package}/{field.typ.name}.hpp"
+            for field in self._struct.fields
+            if isinstance(field.typ, UserType)
+        ]
+        includes = "\n".join([
+            f'#include "{filename}"\n'
+            for filename in sorted(set(filenames))
+        ])
+        if includes:
+            includes += "\n"
+        self._replace("@@SUBSTRUCT_INCLUDES@@\n\n", includes)
+
     def _fill_names(self):
         """Updates the namespace and struct names for this message."""
         namespace_begin, namespace_end = self._namespace_begin_end()
@@ -902,7 +920,7 @@ def main():
         struct = Parser.parse(filename=src)
         generator = CppGen(struct=struct)
         content = generator.generate()
-        path = args.outdir / f"{struct.typ.name}.h"
+        path = args.outdir / f"{struct.typ.name}.hpp"
         path.write_text(content, encoding="utf-8")
 
 

tools/lcm_gen/test/functional_test.cc

@@ -8,9 +8,9 @@
 // lcm-gen tool. Here, we use those as an "oracle" to compare against.
 
 // clang-format off
-#include "papa/lima.h"
-#include "papa/mike.h"
-#include "papa/november.h"
+#include "papa/lima.hpp"
+#include "papa/mike.hpp"
+#include "papa/november.hpp"
 #include "romeo/lima.hpp"
 #include "romeo/mike.hpp"
 #include "romeo/november.hpp"

tools/lcm_gen/test/goal/lima.h → tools/lcm_gen/test/goal/lima.hpp

@@ -32,6 +32,7 @@ class lima {
   // These functions match the expected API from the legacy lcm-gen tool,
   // but note that we use `int64_t` instead of `int` for byte counts.
   //@{
+  static const char* getTypeName() { return "lima"; }
   int64_t getEncodedSize() const { return 8 + _getEncodedSizeNoHash(); }
   int64_t _getEncodedSizeNoHash() const {
     int64_t _result = 0;

tools/lcm_gen/test/goal/mike.h → tools/lcm_gen/test/goal/mike.hpp

@@ -9,6 +9,8 @@
 #include <utility>
 #include <vector>
 
+#include "papa/lima.hpp"
+
 namespace papa {
 
 class mike {
@@ -30,6 +32,7 @@ class mike {
   // These functions match the expected API from the legacy lcm-gen tool,
   // but note that we use `int64_t` instead of `int` for byte counts.
   //@{
+  static const char* getTypeName() { return "mike"; }
   int64_t getEncodedSize() const { return 8 + _getEncodedSizeNoHash(); }
   int64_t _getEncodedSizeNoHash() const {
     int64_t _result = 0;

tools/lcm_gen/test/goal/november.h → tools/lcm_gen/test/goal/november.hpp

@@ -9,6 +9,8 @@
 #include <utility>
 #include <vector>
 
+#include "papa/lima.hpp"
+
 namespace papa {
 
 class november {
@@ -20,6 +22,7 @@ class november {
   // These functions match the expected API from the legacy lcm-gen tool,
   // but note that we use `int64_t` instead of `int` for byte counts.
   //@{
+  static const char* getTypeName() { return "november"; }
   int64_t getEncodedSize() const { return 8 + _getEncodedSizeNoHash(); }
   int64_t _getEncodedSizeNoHash() const {
     int64_t _result = 0;

tools/lcm_gen/test/lcm_gen_test.py

@@ -21,23 +21,23 @@ def setUp(self):
 
         self._lima_path = Path(self._manifest.Rlocation(
             "drake/tools/lcm_gen/test/lima.lcm"))
-        self._lima_h_path = Path(self._manifest.Rlocation(
-            "drake/tools/lcm_gen/test/goal/lima.h"))
+        self._lima_hpp_path = Path(self._manifest.Rlocation(
+            "drake/tools/lcm_gen/test/goal/lima.hpp"))
         self._mike_path = Path(self._manifest.Rlocation(
             "drake/tools/lcm_gen/test/mike.lcm"))
-        self._mike_h_path = Path(self._manifest.Rlocation(
-            "drake/tools/lcm_gen/test/goal/mike.h"))
+        self._mike_hpp_path = Path(self._manifest.Rlocation(
+            "drake/tools/lcm_gen/test/goal/mike.hpp"))
         self._november_path = Path(self._manifest.Rlocation(
             "drake/tools/lcm_gen/test/november.lcm"))
-        self._november_h_path = Path(self._manifest.Rlocation(
-            "drake/tools/lcm_gen/test/goal/november.h"))
+        self._november_hpp_path = Path(self._manifest.Rlocation(
+            "drake/tools/lcm_gen/test/goal/november.hpp"))
 
         assert self._lima_path.exists()
-        assert self._lima_h_path.exists()
+        assert self._lima_hpp_path.exists()
         assert self._mike_path.exists()
-        assert self._mike_h_path.exists()
+        assert self._mike_hpp_path.exists()
         assert self._november_path.exists()
-        assert self._november_h_path.exists()
+        assert self._november_hpp_path.exists()
 
 
 class TestParser(BaseTest):
@@ -216,21 +216,21 @@ class TestCppGen(BaseTest):
     def test_lima_text(self):
         """The generated text for lima.h exactly matches the goal file."""
         lima = Parser.parse(filename=self._lima_path)
-        expected_text = self._lima_h_path.read_text(encoding="utf-8")
+        expected_text = self._lima_hpp_path.read_text(encoding="utf-8")
         actual_text = CppGen(struct=lima).generate()
         self.assertMultiLineEqual(expected_text, actual_text, self._HELP)
 
     def test_mike_text(self):
         """The generated text for mike.h exactly matches the goal file."""
         mike = Parser.parse(filename=self._mike_path)
-        expected_text = self._mike_h_path.read_text(encoding="utf-8")
+        expected_text = self._mike_hpp_path.read_text(encoding="utf-8")
         actual_text = CppGen(struct=mike).generate()
         self.assertMultiLineEqual(expected_text, actual_text, self._HELP)
 
     def test_november_text(self):
         """The generated text for november.h exactly matches the goal file."""
         november = Parser.parse(filename=self._november_path)
-        expected_text = self._november_h_path.read_text(encoding="utf-8")
+        expected_text = self._november_hpp_path.read_text(encoding="utf-8")
         actual_text = CppGen(struct=november).generate()
         self.assertMultiLineEqual(expected_text, actual_text, self._HELP)
 

tools/skylark/drake_lcm.bzl

@@ -15,8 +15,14 @@ def drake_lcm_cc_library(
         deps = [],
         tags = [],
         strip_include_prefix = None,
+        use_new_lcm_gen = False,
         **kwargs):
-    """A wrapper to insert Drake-specific customizations."""
+    """A wrapper to insert Drake-specific customizations.
+
+    The use_new_lcm_gen flag is an instruction to use Drake's customized
+    lcm_gen tool (@drake//tools/lcm_gen), instead LCM's upstream lcm-gen.
+    Refer to //tools/lcm_gen for details.
+    """
 
     # Drake's *.lcm files all live in our //drake/lcmtypes package.  Per LCM
     # upstream convention, the include directory for generated code should
@@ -32,6 +38,7 @@ def drake_lcm_cc_library(
         deps = deps,
         tags = tags + ["nolint"],
         strip_include_prefix = strip_include_prefix,
+        _use_new_lcm_gen = use_new_lcm_gen,
         **kwargs
     )
     drake_installed_headers(

tools/workspace/lcm/lcm.bzl

@@ -142,6 +142,7 @@ def lcm_cc_library(
         deprecation = None,
         aggregate_hdr = None,
         aggregate_hdr_strip_prefix = ["**/include/"],
+        _use_new_lcm_gen = False,
         **kwargs):
     """Declares a cc_library on message classes generated from `*.lcm` files.
 
@@ -177,15 +178,33 @@ def lcm_cc_library(
             helper_tags.append(sticky_tag)
 
     outs = _lcm_outs(lcm_srcs, lcm_package, lcm_structs, ".hpp")
-    _lcm_library_gen(
-        name = name + "_lcm_library_gen",
-        language = "cc",
-        lcm_srcs = lcm_srcs,
-        lcm_package = lcm_package,
-        outs = outs,
-        deprecation = deprecation,
-        tags = helper_tags,
-    )
+    if _use_new_lcm_gen:
+        # The _use_new_lcm_gen flag is private, for the exclusive use of
+        # Drake's drake_lcm_cc_library wrapper. It's an instruction to use
+        # Drake's customized lcm_gen tool, instead LCM's upstream lcm-gen.
+        tool = "@drake//tools/lcm_gen"
+        native.genrule(
+            name = name + "_lcm_library_gen",
+            srcs = lcm_srcs,
+            outs = outs,
+            tools = [tool],
+            cmd = " ".join([
+                "$(execpath {})".format(tool),
+                "$(SRCS)",
+                "--outdir=$(RULEDIR)/$$(dirname {})".format(outs[0]),
+            ]),
+            tags = helper_tags,
+        )
+    else:
+        _lcm_library_gen(
+            name = name + "_lcm_library_gen",
+            language = "cc",
+            lcm_srcs = lcm_srcs,
+            lcm_package = lcm_package,
+            outs = outs,
+            deprecation = deprecation,
+            tags = helper_tags,
+        )
 
     if aggregate_hdr:
         outs += _lcm_aggregate_hdr(
@@ -200,8 +219,9 @@ def lcm_cc_library(
         )
 
     deps = kwargs.pop("deps", [])
-    if "@lcm//:lcm_coretypes" not in deps:
-        deps = deps + ["@lcm//:lcm_coretypes"]
+    if not _use_new_lcm_gen:
+        if "@lcm//:lcm_coretypes" not in deps:
+            deps = deps + ["@lcm//:lcm_coretypes"]
 
     includes = kwargs.pop("includes", [])
     if "." not in includes: