Merge branch 'wpilibsuite:main' into PhysicsSimTorqueCurrent

.github/workflows/upstream-utils.yml

@@ -40,11 +40,6 @@ jobs:
           cd upstream_utils
           ./argparse_lib.py clone
           ./argparse_lib.py copy-src
-      - name: Run concurrentqueue.py
-        run: |
-          cd upstream_utils
-          ./concurrentqueue.py clone
-          ./concurrentqueue.py copy-src
       - name: Run eigen.py
         run: |
           cd upstream_utils

GeneratedFiles.md

@@ -0,0 +1,25 @@
+# Maintaining Generated Files
+WPILib extensively uses [metaprogramming](https://en.wikipedia.org/wiki/Metaprogramming#Code_generation) to generate code that would otherwise be tedious and error-prone to maintain. We use [Jinja](https://jinja.palletsprojects.com), a templating engine with a Python API, alongside JSON files that contain data to generate code. This document explains how to maintain these generated files and create new ones.
+
+## File hierarchy
+The Python script used to generate a subproject's files will always be located in the subproject's directory, e.g. wpilibc. It will always be called `generate_<thing>.py` where `<thing>` is the name for what you're generating.
+
+The templates will be located under `subproject/src/generate/main`, and generated files will be located under `subproject/src/generated/main`.
+
+If the generated file is for C++, the hierarchy should be symmetrical, so if a generated header is located under `subproject/src/generated/main/native/include/frc/header.h`, the template to generate it should be located under `subproject/src/generate/main/native/include/frc/template.h.jinja`. You should pretend like `subproject/src/generate/main` is just like `subproject/src/main`, in that the file hierarchy must make sense if the files weren't generated, e.g, headers that would go in `subproject/src/main/native/include/blah` should be in `subproject/src/generated/main/native/include/blah`.
+
+If the generated file is for Java, templates should be located under `subproject/src/generate/main/java`, and the hierarchy for output files should reflect the declared package of the output Java files. For example, a Jinja template at `subproject/src/main/java/template.java.jinja` with the package `edu.wpi.first.wpilibj` would be used to generate Java files located at `subproject/src/generated/main/java/edu/wpi/first/wpilibj`
+
+The JSON files live under `subproject/src/generate` since they apply to both languages. One unique case is JSON files that are used by multiple subprojects, currently only JSON files shared by wpilibc and wpilibj. In that specific case, the JSON files will always be located in wpilibj since Java is the most used language.
+
+## Using code generation
+If you've identified a set of files which are extremely similar, one file with lots of repetitive code, or both, you can create Jinja templates, a JSON file, and a Python script to automatically generate the code instead.
+
+### Preparing files for codegen
+Once you've identified the files you want to codegen, you will need to identify parts of code that are similar, and extract the data that's different. Code needs to go into your Jinja template, while data that will be used to fill in the template goes into a JSON file. Using game controllers as an example, they have lots of similar methods to read the value of a button, check if a button has been pressed since the last check, and check if a button has been released since the last check. Those methods are code that goes in a Jinja template, with the specific button replaced with a Jinja expression. The buttons, both the name and value, go into a JSON file.
+
+### Writing a Python script
+To maintain consistency with other Python scripts, copy an existing `generate_*.py` script. [generate_pwm_motor_controllers.py](./wpilibj/generate_pwm_motor_controllers.py) is a good start, since it's relatively basic. Modify the script to reference your templates and JSON file, modify the paths so the files end up in the right place, and rename the functions so they match what you're generating. An important part of the script is to give the files the correct name. Depending on files you're generating, this could be the name of the template itself (see [ntcore/generate_topics.py](./ntcore/generate_topics.py)) or it could be part of the data in the JSON file.
+
+### (Re)Generating files and committing them
+Once your Python script is complete, you can run `python generate_<thing>.py` to generate the files. Once you're finished with your files, commit these files to Git. If you regenerated the files and Git indicates the files have changed, but the diff doesn't show any changes, only the line endings have changed. If you expected changes to the generated code, you didn't correctly make changes. If you didn't expect changes, you can ignore this and discard the changes. Also ensure that you've marked the Python script as executable, since this is necessary for CI workflows to run your scripts. To add your script to the CI workflows, edit [.github/workflows/pregen_all.py](.github/workflows/pregen_all.py), and add your script alongside the rest of the scripts.

README.md

@@ -145,7 +145,7 @@ If you only want to run the Java autoformatter, run `./gradlew spotlessApply`.
 
 ### Generated files
 
-Several files within WPILib are generated using Jinja. If a PR is opened that modifies these templates then the files can be generated through CI by commenting `/pregen` on the PR. A new commit will be pushed with the regenerated files.
+Several files within WPILib are generated using Jinja. If a PR is opened that modifies these templates then the files can be generated through CI by commenting `/pregen` on the PR. A new commit will be pushed with the regenerated files. See [GeneratedFiles.md](GeneratedFiles.md) for more information.
 
 ### CMake
 

ntcore/src/main/native/cpp/Handle.h

@@ -33,12 +33,12 @@ class Handle {
   static_assert(kTypeMax <= wpi::kHandleTypeHALBase);
   enum { kIndexMax = 0xfffff };
 
-  explicit Handle(NT_Handle handle) : m_handle(handle) {}
-  operator NT_Handle() const { return m_handle; }
+  constexpr explicit Handle(NT_Handle handle) : m_handle(handle) {}
+  constexpr operator NT_Handle() const { return m_handle; }
 
-  NT_Handle handle() const { return m_handle; }
+  constexpr NT_Handle handle() const { return m_handle; }
 
-  Handle(int inst, int index, Type type) {
+  constexpr Handle(int inst, int index, Type type) {
     if (inst < 0 || index < 0) {
       m_handle = 0;
       return;
@@ -47,16 +47,22 @@ class Handle {
                (index & 0xfffff);
   }
 
-  unsigned int GetIndex() const {
+  constexpr unsigned int GetIndex() const {
     return static_cast<unsigned int>(m_handle) & 0xfffff;
   }
-  Type GetType() const {
+  constexpr Type GetType() const {
     return static_cast<Type>((static_cast<int>(m_handle) >> 24) & 0x7f);
   }
-  int GetInst() const { return (static_cast<int>(m_handle) >> 20) & 0xf; }
-  bool IsType(Type type) const { return type == GetType(); }
-  int GetTypedIndex(Type type) const { return IsType(type) ? GetIndex() : -1; }
-  int GetTypedInst(Type type) const { return IsType(type) ? GetInst() : -1; }
+  constexpr int GetInst() const {
+    return (static_cast<int>(m_handle) >> 20) & 0xf;
+  }
+  constexpr bool IsType(Type type) const { return type == GetType(); }
+  constexpr int GetTypedIndex(Type type) const {
+    return IsType(type) ? GetIndex() : -1;
+  }
+  constexpr int GetTypedInst(Type type) const {
+    return IsType(type) ? GetInst() : -1;
+  }
 
  private:
   NT_Handle m_handle;

ntcore/src/main/native/cpp/LocalStorage.cpp

@@ -403,7 +403,7 @@ void LocalStorage::Impl::PropertiesUpdated(TopicData* topic,
   NotifyTopic(topic, eventFlags | NT_EVENT_PROPERTIES);
   // check local flag so we don't echo back received properties changes
   if (m_network && sendNetwork) {
-    m_network->SetProperties(topic->handle, topic->name, update);
+    m_network->SetProperties(topic->name, update);
   }
 }
 
@@ -427,10 +427,10 @@ void LocalStorage::Impl::RefreshPubSubActive(TopicData* topic,
 void LocalStorage::Impl::NetworkAnnounce(TopicData* topic,
                                          std::string_view typeStr,
                                          const wpi::json& properties,
-                                         NT_Publisher pubHandle) {
+                                         std::optional<int> pubuid) {
   DEBUG4("LS NetworkAnnounce({}, {}, {}, {})", topic->name, typeStr,
-         properties.dump(), pubHandle);
-  if (pubHandle != 0) {
+         properties.dump(), pubuid.value_or(-1));
+  if (pubuid.has_value()) {
     return;  // ack of our publish; ignore
   }
 
@@ -503,7 +503,7 @@ void LocalStorage::Impl::RemoveNetworkPublisher(TopicData* topic) {
       // this may result in a duplicate publish warning on the server side,
       // but send one anyway in this case just to be sure
       if (nextPub->active && m_network) {
-        m_network->Publish(nextPub->handle, topic->handle, topic->name,
+        m_network->Publish(Handle{nextPub->handle}.GetIndex(), topic->name,
                            topic->typeStr, topic->properties, nextPub->config);
       }
     }
@@ -561,7 +561,7 @@ LocalStorage::PublisherData* LocalStorage::Impl::AddLocalPublisher(
   }
 
   if (publisher->active && m_network) {
-    m_network->Publish(publisher->handle, topic->handle, topic->name,
+    m_network->Publish(Handle{publisher->handle}.GetIndex(), topic->name,
                        topic->typeStr, topic->properties, config);
   }
   return publisher;
@@ -580,7 +580,7 @@ LocalStorage::Impl::RemoveLocalPublisher(NT_Publisher pubHandle) {
     }
 
     if (publisher->active && m_network) {
-      m_network->Unpublish(publisher->handle, topic->handle);
+      m_network->Unpublish(Handle{publisher->handle}.GetIndex());
     }
 
     if (publisher->active && !topic->localPublishers.empty()) {
@@ -593,7 +593,7 @@ LocalStorage::Impl::RemoveLocalPublisher(NT_Publisher pubHandle) {
         topic->typeStr = nextPub->config.typeStr;
         RefreshPubSubActive(topic, false);
         if (nextPub->active && m_network) {
-          m_network->Publish(nextPub->handle, topic->handle, topic->name,
+          m_network->Publish(Handle{nextPub->handle}.GetIndex(), topic->name,
                              topic->typeStr, topic->properties,
                              nextPub->config);
         }
@@ -619,7 +619,8 @@ LocalStorage::SubscriberData* LocalStorage::Impl::AddLocalSubscriber(
   }
   if (m_network && !subscriber->config.hidden) {
     DEBUG4("-> NetworkSubscribe({})", topic->name);
-    m_network->Subscribe(subscriber->handle, {{topic->name}}, config);
+    m_network->Subscribe(Handle{subscriber->handle}.GetIndex(), {{topic->name}},
+                         config);
   }
 
   // queue current value
@@ -647,7 +648,7 @@ LocalStorage::Impl::RemoveLocalSubscriber(NT_Subscriber subHandle) {
       }
     }
     if (m_network && !subscriber->config.hidden) {
-      m_network->Unsubscribe(subscriber->handle);
+      m_network->Unsubscribe(Handle{subscriber->handle}.GetIndex());
     }
   }
   return subscriber;
@@ -684,8 +685,8 @@ LocalStorage::MultiSubscriberData* LocalStorage::Impl::AddMultiSubscriber(
   }
   if (m_network && !subscriber->options.hidden) {
     DEBUG4("-> NetworkSubscribe");
-    m_network->Subscribe(subscriber->handle, subscriber->prefixes,
-                         subscriber->options);
+    m_network->Subscribe(Handle{subscriber->handle}.GetIndex(),
+                         subscriber->prefixes, subscriber->options);
   }
   return subscriber;
 }
@@ -703,7 +704,7 @@ LocalStorage::Impl::RemoveMultiSubscriber(NT_MultiSubscriber subHandle) {
       }
     }
     if (m_network && !subscriber->options.hidden) {
-      m_network->Unsubscribe(subscriber->handle);
+      m_network->Unsubscribe(Handle{subscriber->handle}.GetIndex());
     }
   }
   return subscriber;
@@ -977,7 +978,7 @@ bool LocalStorage::Impl::PublishLocalValue(PublisherData* publisher,
       if (publisher->topic->IsCached()) {
         publisher->topic->lastValueNetwork = value;
       }
-      m_network->SetValue(publisher->handle, value);
+      m_network->SetValue(Handle{publisher->handle}.GetIndex(), value);
     }
     return SetValue(publisher->topic, value, NT_EVENT_VALUE_LOCAL,
                     suppressDuplicates, publisher);
@@ -1076,10 +1077,10 @@ LocalStorage::~LocalStorage() = default;
 NT_Topic LocalStorage::NetworkAnnounce(std::string_view name,
                                        std::string_view typeStr,
                                        const wpi::json& properties,
-                                       NT_Publisher pubHandle) {
+                                       std::optional<int> pubuid) {
   std::scoped_lock lock{m_mutex};
   auto topic = m_impl.GetOrCreateTopic(name);
-  m_impl.NetworkAnnounce(topic, typeStr, properties, pubHandle);
+  m_impl.NetworkAnnounce(topic, typeStr, properties, pubuid);
   return topic->handle;
 }
 
@@ -1124,25 +1125,26 @@ void LocalStorage::Impl::StartNetwork(net::NetworkInterface* network) {
     PublisherData* anyPublisher = nullptr;
     for (auto&& publisher : topic->localPublishers) {
       if (publisher->active) {
-        network->Publish(publisher->handle, topic->handle, topic->name,
+        network->Publish(Handle{publisher->handle}.GetIndex(), topic->name,
                          topic->typeStr, topic->properties, publisher->config);
         anyPublisher = publisher;
       }
     }
     if (anyPublisher && topic->lastValue) {
-      network->SetValue(anyPublisher->handle, topic->lastValue);
+      network->SetValue(Handle{anyPublisher->handle}.GetIndex(),
+                        topic->lastValue);
     }
   }
   for (auto&& subscriber : m_subscribers) {
     if (!subscriber->config.hidden) {
-      network->Subscribe(subscriber->handle, {{subscriber->topic->name}},
-                         subscriber->config);
+      network->Subscribe(Handle{subscriber->handle}.GetIndex(),
+                         {{subscriber->topic->name}}, subscriber->config);
     }
   }
   for (auto&& subscriber : m_multiSubscribers) {
     if (!subscriber->options.hidden) {
-      network->Subscribe(subscriber->handle, subscriber->prefixes,
-                         subscriber->options);
+      network->Subscribe(Handle{subscriber->handle}.GetIndex(),
+                         subscriber->prefixes, subscriber->options);
     }
   }
 }

ntcore/src/main/native/cpp/LocalStorage.h

@@ -48,7 +48,7 @@ class LocalStorage final : public net::ILocalStorage {
   // network interface functions
   NT_Topic NetworkAnnounce(std::string_view name, std::string_view typeStr,
                            const wpi::json& properties,
-                           NT_Publisher pubHandle) final;
+                           std::optional<int> pubuid) final;
   void NetworkUnannounce(std::string_view name) final;
   void NetworkPropertiesUpdate(std::string_view name, const wpi::json& update,
                                bool ack) final;
@@ -601,7 +601,8 @@ class LocalStorage final : public net::ILocalStorage {
     void RefreshPubSubActive(TopicData* topic, bool warnOnSubMismatch);
 
     void NetworkAnnounce(TopicData* topic, std::string_view typeStr,
-                         const wpi::json& properties, NT_Publisher pubHandle);
+                         const wpi::json& properties,
+                         std::optional<int> pubuid);
     void RemoveNetworkPublisher(TopicData* topic);
     void NetworkPropertiesUpdate(TopicData* topic, const wpi::json& update,
                                  bool ack);

ntcore/src/main/native/cpp/NetworkClient.cpp

@@ -407,7 +407,7 @@ void NetworkClient::WsConnected(wpi::WebSocket& ws, uv::Tcp& tcp,
   m_wire = std::make_shared<net::WebSocketConnection>(
       ws, connInfo.protocol_version, m_logger);
   m_clientImpl = std::make_unique<net::ClientImpl>(
-      m_loop.Now().count(), m_inst, *m_wire, m_logger, m_timeSyncUpdated,
+      m_loop.Now().count(), *m_wire, m_logger, m_timeSyncUpdated,
       [this](uint32_t repeatMs) {
         DEBUG4("Setting periodic timer to {}", repeatMs);
         if (m_sendOutgoingTimer &&

ntcore/src/main/native/cpp/NetworkServer.cpp

@@ -254,7 +254,7 @@ void NetworkServer::ServerConnection4::ProcessWsUpgrade() {
       m_websocket->binary.connect([this](std::span<const uint8_t> data, bool) {
         while (!data.empty()) {
           // decode message
-          int64_t pubuid;
+          int pubuid;
           Value value;
           std::string error;
           if (!net::WireDecodeBinary(&data, &pubuid, &value, &error, 0)) {