Merge branch 'main' into 3d-pose-estimation

.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
 

cscore/src/main/native/cpp/HttpCameraImpl.cpp

@@ -458,11 +458,7 @@ std::unique_ptr<PropertyImpl> HttpCameraImpl::CreateEmptyProperty(
 }
 
 bool HttpCameraImpl::CacheProperties(CS_Status* status) const {
-#ifdef _MSC_VER  // work around VS2019 16.4.0 bug
-  std::scoped_lock<wpi::mutex> lock(m_mutex);
-#else
   std::scoped_lock lock(m_mutex);
-#endif
 
   // Pretty typical set of video modes
   m_videoModes.clear();

cscore/src/main/native/cpp/UnlimitedHandleResource.h

@@ -75,11 +75,7 @@ template <typename THandle, typename TStruct, int typeValue, typename TMutex>
 template <typename... Args>
 THandle UnlimitedHandleResource<THandle, TStruct, typeValue, TMutex>::Allocate(
     Args&&... args) {
-#ifdef _MSC_VER  // work around VS2019 16.4.0 bug
-  std::scoped_lock<TMutex> lock(m_handleMutex);
-#else
   std::scoped_lock sync(m_handleMutex);
-#endif
   size_t i;
   for (i = 0; i < m_structures.size(); i++) {
     if (m_structures[i] == nullptr) {

glass/src/libnt/native/cpp/NetworkTables.cpp

@@ -745,7 +745,7 @@ void NetworkTablesModel::ValueSource::UpdateFromValue(
           structName = *withoutArray;
         }
         auto desc = model.m_structDb.Find(structName);
-        if (desc && desc->IsValid()) {
+        if (desc && desc->IsValid() && desc->GetSize() != 0) {
           if (isArray) {
             // array of struct at top level
             if (valueChildrenMap) {

hal/src/main/native/athena/CTREPCM.cpp

@@ -185,7 +185,7 @@ HAL_CTREPCMHandle HAL_InitializeCTREPCM(int32_t module,
                                            pcm->previousAllocation);
     } else {
       hal::SetLastErrorIndexOutOfRange(status, "Invalid Index for CTRE PCM", 0,
-                                       kNumCTREPCMModules, module);
+                                       kNumCTREPCMModules - 1, module);
     }
     return HAL_kInvalidHandle;  // failed to allocate. Pass error back.
   }

hal/src/main/native/athena/CTREPDP.cpp

@@ -133,8 +133,9 @@ HAL_PDPHandle HAL_InitializePDP(int32_t module, const char* allocationLocation,
                                 int32_t* status) {
   hal::init::CheckInit();
   if (!HAL_CheckPDPModule(module)) {
-    *status = PARAMETER_OUT_OF_RANGE;
-    hal::SetLastError(status, fmt::format("Invalid pdp module {}", module));
+    *status = RESOURCE_OUT_OF_RANGE;
+    hal::SetLastErrorIndexOutOfRange(status, "Invalid Index for CTRE PDP", 0,
+                                     kNumCTREPDPModules - 1, module);
     return HAL_kInvalidHandle;
   }
 
@@ -147,7 +148,7 @@ HAL_PDPHandle HAL_InitializePDP(int32_t module, const char* allocationLocation,
                                            pdp->previousAllocation);
     } else {
       hal::SetLastErrorIndexOutOfRange(status, "Invalid Index for CTRE PDP", 0,
-                                       kNumCTREPDPModules, module);
+                                       kNumCTREPDPModules - 1, module);
     }
     return HAL_kInvalidHandle;  // failed to allocate. Pass error back.
   }

hal/src/main/native/athena/LEDs.cpp

@@ -6,10 +6,14 @@
 
 #include <unistd.h>
 
+#include <cstring>
 #include <fstream>
 
+#include <fmt/format.h>
+#include <fmt/std.h>
 #include <wpi/fs.h>
 
+#include "HALInternal.h"
 #include "hal/Errors.h"
 
 namespace hal::init {
@@ -28,19 +32,56 @@ static const fs::path radioLEDRedFilePath =
 static const char* onStr = "1";
 static const char* offStr = "0";
 
+static bool ReadStateFromFile(fs::path path, int32_t* status) {
+  std::error_code ec;
+  fs::file_t file = fs::OpenFileForRead(path, ec, fs::OF_Text);
+  if (ec) {
+    hal::SetLastError(status, fmt::format("Could not open '{}' for read: {}",
+                                          path, ec.message()));
+    *status = INCOMPATIBLE_STATE;
+    return false;
+  }
+  // We only need to read one byte because the file won't have leading zeros.
+  char buf[1]{};
+  ssize_t count = read(file, buf, 1);
+  // save errno, always close file.
+  int err = errno;
+  fs::CloseFile(file);
+  if (count <= 0) {
+    *status = INCOMPATIBLE_STATE;
+    if (count == 0) {
+      hal::SetLastError(status,
+                        fmt::format("Read from '{}' returned no data.", path));
+    } else {
+      hal::SetLastError(status, fmt::format("Failed to read from '{}': {}",
+                                            path, std::strerror(err)));
+    }
+    return false;
+  }
+  // If the brightness is not zero, the LED is on.
+  return buf[0] != '0';
+}
+
 extern "C" {
 void HAL_SetRadioLEDState(HAL_RadioLEDState state, int32_t* status) {
   std::error_code ec;
   fs::file_t greenFile = fs::OpenFileForWrite(radioLEDGreenFilePath, ec,
                                               fs::CD_OpenExisting, fs::OF_Text);
   if (ec) {
+    // not opened, nothing to clean up
     *status = INCOMPATIBLE_STATE;
+    hal::SetLastError(status, fmt::format("Could not open '{}' for write: {}",
+                                          greenFile, ec.message()));
     return;
   }
   fs::file_t redFile = fs::OpenFileForWrite(radioLEDRedFilePath, ec,
                                             fs::CD_OpenExisting, fs::OF_Text);
   if (ec) {
+    // green file opened successfully, need to close it
+    fs::CloseFile(greenFile);
     *status = INCOMPATIBLE_STATE;
+    hal::SetLastError(status, fmt::format("Could not open '{}' for write: {}",
+                                          greenFile, ec.message()));
     return;
   }
 
@@ -51,24 +92,6 @@ void HAL_SetRadioLEDState(HAL_RadioLEDState state, int32_t* status) {
   fs::CloseFile(redFile);
 }
 
-bool ReadStateFromFile(fs::path path, int32_t* status) {
-  std::error_code ec;
-  fs::file_t file = fs::OpenFileForRead(path, ec, fs::OF_Text);
-  if (ec) {
-    *status = INCOMPATIBLE_STATE;
-    return false;
-  }
-  // We only need to read one byte because the file won't have leading zeros.
-  char buf[1]{};
-  size_t count = read(file, buf, 1);
-  if (count == 0) {
-    *status = INCOMPATIBLE_STATE;
-    return false;
-  }
-  // If the brightness is not zero, the LED is on.
-  return buf[0] != '0';
-}
-
 HAL_RadioLEDState HAL_GetRadioLEDState(int32_t* status) {
   bool green = ReadStateFromFile(radioLEDGreenFilePath, status);
   bool red = ReadStateFromFile(radioLEDRedFilePath, status);

hal/src/main/native/athena/REVPDH.cpp

@@ -205,17 +205,20 @@ HAL_REVPDHHandle HAL_InitializeREVPDH(int32_t module,
   hal::init::CheckInit();
   if (!HAL_CheckREVPDHModuleNumber(module)) {
     *status = RESOURCE_OUT_OF_RANGE;
+    hal::SetLastErrorIndexOutOfRange(status, "Invalid Index for REV PDH", 1,
+                                     kNumREVPDHModules, module);
     return HAL_kInvalidHandle;
   }
 
   HAL_REVPDHHandle handle;
-  auto hpdh = REVPDHHandles->Allocate(module, &handle, status);
+  // Module starts at 1
+  auto hpdh = REVPDHHandles->Allocate(module - 1, &handle, status);
   if (*status != 0) {
     if (hpdh) {
       hal::SetLastErrorPreviouslyAllocated(status, "REV PDH", module,
                                            hpdh->previousAllocation);
     } else {
-      hal::SetLastErrorIndexOutOfRange(status, "Invalid Index for REV PDH", 0,
+      hal::SetLastErrorIndexOutOfRange(status, "Invalid Index for REV PDH", 1,
                                        kNumREVPDHModules, module);
     }
     return HAL_kInvalidHandle;  // failed to allocate. Pass error back.
@@ -253,7 +256,7 @@ int32_t HAL_GetREVPDHModuleNumber(HAL_REVPDHHandle handle, int32_t* status) {
 }
 
 HAL_Bool HAL_CheckREVPDHModuleNumber(int32_t module) {
-  return ((module >= 1) && (module < kNumREVPDHModules)) ? 1 : 0;
+  return ((module >= 1) && (module <= kNumREVPDHModules)) ? 1 : 0;
 }
 
 HAL_Bool HAL_CheckREVPDHChannelNumber(int32_t channel) {

hal/src/main/native/athena/REVPH.cpp

@@ -196,13 +196,15 @@ HAL_REVPHHandle HAL_InitializeREVPH(int32_t module,
                                     int32_t* status) {
   hal::init::CheckInit();
   if (!HAL_CheckREVPHModuleNumber(module)) {
+    *status = RESOURCE_OUT_OF_RANGE;
     hal::SetLastErrorIndexOutOfRange(status, "Invalid Index for REV PH", 1,
                                      kNumREVPHModules, module);
     return HAL_kInvalidHandle;
   }
 
   HAL_REVPHHandle handle;
-  auto hph = REVPHHandles->Allocate(module, &handle, status);
+  // Module starts at 1
+  auto hph = REVPHHandles->Allocate(module - 1, &handle, status);
   if (*status != 0) {
     if (hph) {
       hal::SetLastErrorPreviouslyAllocated(status, "REV PH", module,
@@ -247,7 +249,7 @@ void HAL_FreeREVPH(HAL_REVPHHandle handle) {
 }
 
 HAL_Bool HAL_CheckREVPHModuleNumber(int32_t module) {
-  return module >= 1 && module < kNumREVPDHModules;
+  return module >= 1 && module <= kNumREVPHModules;
 }
 
 HAL_Bool HAL_CheckREVPHSolenoidChannel(int32_t channel) {

hal/src/main/native/include/hal/simulation/SimCallbackRegistry.h

@@ -79,11 +79,7 @@ class SimCallbackRegistry : public impl::SimCallbackRegistryBase {
 
   template <typename... U>
   void Invoke(U&&... u) const {
-#ifdef _MSC_VER  // work around VS2019 16.4.0 bug
-    std::scoped_lock<wpi::recursive_spinlock> lock(m_mutex);
-#else
     std::scoped_lock lock(m_mutex);
-#endif
     if (m_callbacks) {
       const char* name = GetName();
       for (auto&& cb : *m_callbacks) {

hal/src/main/native/sim/CTREPCM.cpp

@@ -49,7 +49,7 @@ HAL_CTREPCMHandle HAL_InitializeCTREPCM(int32_t module,
                                            pcm->previousAllocation);
     } else {
       hal::SetLastErrorIndexOutOfRange(status, "Invalid Index for CTRE PCM", 0,
-                                       kNumCTREPCMModules, module);
+                                       kNumCTREPCMModules - 1, module);
     }
     return HAL_kInvalidHandle;  // failed to allocate. Pass error back.
   }

hal/src/main/native/sim/HAL.cpp

@@ -44,11 +44,7 @@ class SimPeriodicCallbackRegistry : public impl::SimCallbackRegistryBase {
   }
 
   void operator()() const {
-#ifdef _MSC_VER  // work around VS2019 16.4.0 bug
-    std::scoped_lock<wpi::recursive_spinlock> lock(m_mutex);
-#else
     std::scoped_lock lock(m_mutex);
-#endif
     if (m_callbacks) {
       for (auto&& cb : *m_callbacks) {
         reinterpret_cast<HALSIM_SimPeriodicCallback>(cb.callback)(cb.param);

hal/src/main/native/sim/PowerDistribution.cpp

@@ -44,8 +44,14 @@ HAL_PowerDistributionHandle HAL_InitializePowerDistribution(
   }
 
   if (!HAL_CheckPowerDistributionModule(module, type)) {
-    *status = PARAMETER_OUT_OF_RANGE;
-    hal::SetLastError(status, fmt::format("Invalid pdp module {}", module));
+    *status = RESOURCE_OUT_OF_RANGE;
+    if (type == HAL_PowerDistributionType::HAL_PowerDistributionType_kCTRE) {
+      hal::SetLastErrorIndexOutOfRange(status, "Invalid Index for CTRE PDP", 0,
+                                       kNumCTREPDPModules - 1, module);
+    } else {
+      hal::SetLastErrorIndexOutOfRange(status, "Invalid Index for REV PDH", 1,
+                                       kNumREVPDHModules, module);
+    }
     return HAL_kInvalidHandle;
   }
   hal::init::CheckInit();
@@ -74,7 +80,7 @@ HAL_Bool HAL_CheckPowerDistributionModule(int32_t module,
   if (type == HAL_PowerDistributionType::HAL_PowerDistributionType_kCTRE) {
     return module < kNumCTREPDPModules && module >= 0;
   } else {
-    return module < kNumREVPDHModules && module >= 1;
+    return module <= kNumREVPDHModules && module >= 1;
   }
 }
 

hal/src/main/native/sim/REVPH.cpp

@@ -40,14 +40,16 @@ HAL_REVPHHandle HAL_InitializeREVPH(int32_t module,
                                     int32_t* status) {
   hal::init::CheckInit();
 
-  if (module == 0) {
+  if (!HAL_CheckREVPHModuleNumber(module)) {
+    *status = RESOURCE_OUT_OF_RANGE;
     hal::SetLastErrorIndexOutOfRange(status, "Invalid Index for REV PH", 1,
                                      kNumREVPHModules, module);
     return HAL_kInvalidHandle;
   }
 
   HAL_REVPHHandle handle;
-  auto pcm = pcmHandles->Allocate(module, &handle, status);
+  // Module starts at 1
+  auto pcm = pcmHandles->Allocate(module - 1, &handle, status);
 
   if (*status != 0) {
     if (pcm) {
@@ -82,7 +84,7 @@ void HAL_FreeREVPH(HAL_REVPHHandle handle) {
 }
 
 HAL_Bool HAL_CheckREVPHModuleNumber(int32_t module) {
-  return module >= 1 && module < kNumREVPDHModules;
+  return module >= 1 && module <= kNumREVPHModules;
 }
 
 HAL_Bool HAL_CheckREVPHSolenoidChannel(int32_t channel) {