[ntcore] Various NT4 fixes (#4474)

* TopicListener: Fix Add() return values
* Update PubSubOption poll storage documentation
* Update NetworkTableEntry::GetValue() doc
* Add documentation regarding asynchronous callbacks
* Unpublish entry: set publisher to nullptr
* Implement ValueListenerPoller default constructor
* Remove SetNetworkIdentity, make parameter to StartClient
* URI-escape client ID, improve error message
* Add connected message with client id; also improve disconnected message a bit
* Handle SetServers either before or after StartClient
* Fix client use-after-free; also delay reconnect after disconnect to rate limit
* Don't re-announce to already subscribed client; we especially don't want to send the last value again
* Always accept in-order sets, only use timestamp for tiebreak
* Fix LocalStorage::StartNetwork race
* Remove unused/unimplemented function

Also:
* [glass] Remove debug print
* [glass] Fix mpack string decoding
* [cameraserver] Fix up startclient

CMakeLists.txt

@@ -36,22 +36,23 @@ SET(CMAKE_SKIP_BUILD_RPATH  FALSE)
 # (but later on when installing)
 SET(CMAKE_BUILD_WITH_INSTALL_RPATH FALSE)
 
-SET(CMAKE_INSTALL_RPATH "${CMAKE_INSTALL_PREFIX}/wpilib/lib")
+SET(CMAKE_INSTALL_RPATH "${CMAKE_INSTALL_PREFIX}/lib")
 
 # add the automatically determined parts of the RPATH
 # which point to directories outside the build tree to the install RPATH
 SET(CMAKE_INSTALL_RPATH_USE_LINK_PATH TRUE)
 
 # the RPATH to be used when installing, but only if it's not a system directory
-LIST(FIND CMAKE_PLATFORM_IMPLICIT_LINK_DIRECTORIES "${CMAKE_INSTALL_PREFIX}/wpilib/lib" isSystemDir)
+LIST(FIND CMAKE_PLATFORM_IMPLICIT_LINK_DIRECTORIES "${CMAKE_INSTALL_PREFIX}/lib" isSystemDir)
 IF("${isSystemDir}" STREQUAL "-1")
-   SET(CMAKE_INSTALL_RPATH "${CMAKE_INSTALL_PREFIX}/wpilib/lib")
+   SET(CMAKE_INSTALL_RPATH "${CMAKE_INSTALL_PREFIX}/lib")
 ENDIF("${isSystemDir}" STREQUAL "-1")
 
 # Options for building certain parts of the repo. Everything is built by default.
 option(BUILD_SHARED_LIBS "Build with shared libs (needed for JNI)" ON)
 option(WITH_JAVA "Include java and JNI in the build" ON)
 option(WITH_CSCORE "Build cscore (needs OpenCV)" ON)
+option(WITH_NTCORE "Build ntcore" ON)
 option(WITH_WPIMATH "Build wpimath" ON)
 option(WITH_WPILIB "Build hal, wpilibc/j, and myRobot (needs OpenCV)" ON)
 option(WITH_EXAMPLES "Build examples" OFF)
@@ -63,10 +64,10 @@ option(WITH_SIMULATION_MODULES "Build simulation modules" ON)
 option(WITH_EXTERNAL_HAL "Use a separately built HAL" OFF)
 set(EXTERNAL_HAL_FILE "" CACHE FILEPATH "Location to look for an external HAL CMake File")
 
-# Options for using a package manager (vcpkg) for certain dependencies.
-option(USE_VCPKG_FMTLIB "Use vcpkg fmtlib" OFF)
-option(USE_VCPKG_LIBUV "Use vcpkg libuv" OFF)
-option(USE_VCPKG_EIGEN "Use vcpkg eigen" OFF)
+# Options for using a package manager (e.g., vcpkg) for certain dependencies.
+option(USE_SYSTEM_FMTLIB "Use system fmtlib" OFF)
+option(USE_SYSTEM_LIBUV "Use system libuv" OFF)
+option(USE_SYSTEM_EIGEN "Use system eigen" OFF)
 
 # Options for installation.
 option(WITH_FLAT_INSTALL "Use a flat install directory" OFF)
@@ -117,6 +118,34 @@ FATAL: Cannot build simulation modules with wpilib disabled.
 ")
 endif()
 
+if (NOT WITH_NTCORE AND WITH_CSCORE)
+    message(FATAL_ERROR "
+FATAL: Cannot build cameraserver without ntcore.
+       Enable ntcore by setting WITH_NTCORE=ON
+")
+endif()
+
+if (NOT WITH_NTCORE AND WITH_GUI)
+    message(FATAL_ERROR "
+FATAL: Cannot build GUI modules without ntcore.
+       Enable ntcore by setting WITH_NTCORE=ON
+")
+endif()
+
+if (NOT WITH_NTCORE AND WITH_SIMULATION_MODULES)
+    message(FATAL_ERROR "
+FATAL: Cannot build simulation modules without ntcore.
+       Enable ntcore by setting WITH_NTCORE=ON
+")
+endif()
+
+if (NOT WITH_NTCORE AND WITH_WPILIB)
+    message(FATAL_ERROR "
+FATAL: Cannot build wpilib without ntcore.
+       Enable ntcore by setting WITH_NTCORE=ON
+")
+endif()
+
 if (NOT WITH_WPIMATH AND WITH_WPILIB)
     message(FATAL_ERROR "
 FATAL: Cannot build wpilib without wpimath.
@@ -124,11 +153,11 @@ FATAL: Cannot build wpilib without wpimath.
 ")
 endif()
 
-set( wpilib_dest wpilib)
-set( include_dest wpilib/include )
-set( main_lib_dest wpilib/lib )
-set( java_lib_dest wpilib/java )
-set( jni_lib_dest wpilib/jni )
+set( wpilib_dest "")
+set( include_dest include )
+set( main_lib_dest lib )
+set( java_lib_dest java )
+set( jni_lib_dest jni )
 
 if (WITH_FLAT_INSTALL)
     set (wpilib_config_dir ${wpilib_dest})
@@ -136,12 +165,15 @@ else()
     set (wpilib_config_dir share/wpilib)
 endif()
 
-if (USE_VCPKG_LIBUV)
-set (LIBUV_VCPKG_REPLACE "find_package(unofficial-libuv CONFIG)")
+if (USE_SYSTEM_LIBUV)
+set (LIBUV_SYSTEM_REPLACE "
+find_package(PkgConfig REQUIRED)
+pkg_check_modules(libuv REQUIRED IMPORTED_TARGET libuv)
+")
 endif()
 
-if (USE_VCPKG_EIGEN)
-set (EIGEN_VCPKG_REPLACE "find_package(Eigen3 CONFIG)")
+if (USE_SYSTEM_EIGEN)
+set (EIGEN_SYSTEM_REPLACE "find_package(Eigen3 CONFIG)")
 endif()
 
 find_package(LIBSSH 0.7.1)
@@ -247,8 +279,11 @@ if (WITH_TESTS)
 endif()
 
 add_subdirectory(wpiutil)
-add_subdirectory(wpinet)
-add_subdirectory(ntcore)
+
+if (WITH_NTCORE)
+    add_subdirectory(wpinet)
+    add_subdirectory(ntcore)
+endif()
 
 if (WITH_WPIMATH)
     add_subdirectory(wpimath)

README-CMAKE.md

@@ -31,14 +31,20 @@ The following build options are available:
   * This option will enable Java and JNI builds. If this is on, `WITH_SHARED_LIBS` must be on. Otherwise CMake will error.
 * `WITH_SHARED_LIBS` (ON Default)
   * This option will cause cmake to build static libraries instead of shared libraries. If this is off, `WITH_JAVA` must be off. Otherwise CMake will error.
-* `WITH_TESTS` (ON Default)
-  * This option will build C++ unit tests. These can be run via `make test`.
 * `WITH_CSCORE` (ON Default)
   * This option will cause cscore to be built. Turning this off will implicitly disable cameraserver, the hal and wpilib as well, irrespective of their specific options. If this is off, the OpenCV build requirement is removed.
+* `WITH_NTCORE` (ON Default)
+  * This option will cause ntcore to be built. Turning this off will implicitly disable wpinet and wpilib as well, irrespective of their specific options.
 * `WITH_WPIMATH` (ON Default)
   * This option will build the wpimath library. This option must be on to build wpilib.
 * `WITH_WPILIB` (ON Default)
   * This option will build the hal and wpilibc/j during the build. The HAL is the simulator hal, unless the external hal options are used. The cmake build has no capability to build for the RoboRIO.
+* `WITH_EXAMPLES` (ON Default)
+  * This option will build C++ examples.
+* `WITH_TESTS` (ON Default)
+  * This option will build C++ unit tests. These can be run via `make test`.
+* `WITH_GUI` (ON Default)
+  * This option will build GUI items.
 * `WITH_SIMULATION_MODULES` (ON Default)
   * This option will build simulation modules, including wpigui and the HALSim plugins.
 * `WITH_EXTERNAL_HAL` (OFF Default)

build.gradle

@@ -124,6 +124,10 @@ subprojects {
         }
     }
 
+    tasks.withType(JavaCompile) {
+        options.compilerArgs.add '-XDstringConcat=inline'
+    }
+
     // Enables UTF-8 support in Javadoc
     tasks.withType(Javadoc) {
         options.addStringOption("charset", "utf-8")

cameraserver/multiCameraServer/src/main/java/edu/wpi/Main.java

@@ -176,7 +176,7 @@ public final class Main {
     } else {
       System.out.println("Setting up NetworkTables client for team " + team);
       ntinst.setServerTeam(team);
-      ntinst.startClient4();
+      ntinst.startClient4("multicameraserver");
     }
 
     // start cameras

cameraserver/multiCameraServer/src/main/native/cpp/main.cpp

@@ -183,7 +183,7 @@ int main(int argc, char* argv[]) {
     ntinst.StartServer();
   } else {
     fmt::print("Setting up NetworkTables client for team {}\n", team);
-    ntinst.StartClient4();
+    ntinst.StartClient4("multicameraserver");
     ntinst.SetServerTeam(team);
   }
 

crossConnIntegrationTests/src/main/native/cpp/TestEnvironment.cpp

@@ -49,6 +49,7 @@ class TestEnvironment : public testing::Environment {
       HAL_GetControlWord(&controlWord);
       return controlWord.enabled && controlWord.dsAttached;
     };
+    HAL_RefreshDSData();
     while (!checkEnabled()) {
       if (enableCounter > 50) {
         // Robot did not enable properly after 5 seconds.
@@ -60,6 +61,7 @@ class TestEnvironment : public testing::Environment {
       std::this_thread::sleep_for(100ms);
 
       fmt::print("Waiting for enable: {}\n", enableCounter++);
+      HAL_RefreshDSData();
     }
     std::this_thread::sleep_for(500ms);
   }

cscore/src/main/native/cpp/CvSinkImpl.cpp

@@ -110,7 +110,7 @@ void CvSinkImpl::ThreadMain() {
       std::this_thread::sleep_for(std::chrono::seconds(1));
       continue;
     }
-    SDEBUG4("{}", "waiting for frame");
+    SDEBUG4("waiting for frame");
     Frame frame = source->GetNextFrame();  // blocks
     if (!m_active) {
       break;

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

@@ -85,7 +85,7 @@ void HttpCameraImpl::MonitorThreadMain() {
     // (this will result in an error at the read point, and ultimately
     // a reconnect attempt)
     if (m_streamConn && m_frameCount == 0) {
-      SWARNING("{}", "Monitor detected stream hung, disconnecting");
+      SWARNING("Monitor detected stream hung, disconnecting");
       m_streamConn->stream->close();
     }
 
@@ -93,7 +93,7 @@ void HttpCameraImpl::MonitorThreadMain() {
     m_frameCount = 0;
   }
 
-  SDEBUG("{}", "Monitor Thread exiting");
+  SDEBUG("Monitor Thread exiting");
 }
 
 void HttpCameraImpl::StreamThreadMain() {
@@ -141,7 +141,7 @@ void HttpCameraImpl::StreamThreadMain() {
     }
   }
 
-  SDEBUG("{}", "Camera Thread exiting");
+  SDEBUG("Camera Thread exiting");
   SetConnected(false);
 }
 
@@ -152,7 +152,7 @@ wpi::HttpConnection* HttpCameraImpl::DeviceStreamConnect(
   {
     std::scoped_lock lock(m_mutex);
     if (m_locations.empty()) {
-      SERROR("{}", "locations array is empty!?");
+      SERROR("locations array is empty!?");
       std::this_thread::sleep_for(std::chrono::seconds(1));
       return nullptr;
     }
@@ -273,7 +273,7 @@ bool HttpCameraImpl::DeviceStreamFrame(wpi::raw_istream& is,
   wpi::SmallString<64> contentTypeBuf;
   wpi::SmallString<64> contentLengthBuf;
   if (!ParseHttpHeaders(is, &contentTypeBuf, &contentLengthBuf)) {
-    SWARNING("{}", "disconnected during headers");
+    SWARNING("disconnected during headers");
     PutError("disconnected during headers", wpi::Now());
     return false;
   }
@@ -295,7 +295,7 @@ bool HttpCameraImpl::DeviceStreamFrame(wpi::raw_istream& is,
     // Ugh, no Content-Length?  Read the blocks of the JPEG file.
     int width, height;
     if (!ReadJpeg(is, imageBuf, &width, &height)) {
-      SWARNING("{}", "did not receive a JPEG image");
+      SWARNING("did not receive a JPEG image");
       PutError("did not receive a JPEG image", wpi::Now());
       return false;
     }
@@ -314,7 +314,7 @@ bool HttpCameraImpl::DeviceStreamFrame(wpi::raw_istream& is,
   }
   int width, height;
   if (!GetJpegSize(image->str(), &width, &height)) {
-    SWARNING("{}", "did not receive a JPEG image");
+    SWARNING("did not receive a JPEG image");
     PutError("did not receive a JPEG image", wpi::Now());
     return false;
   }
@@ -344,7 +344,7 @@ void HttpCameraImpl::SettingsThreadMain() {
     DeviceSendSettings(req);
   }
 
-  SDEBUG("{}", "Settings Thread exiting");
+  SDEBUG("Settings Thread exiting");
 }
 
 void HttpCameraImpl::DeviceSendSettings(wpi::HttpRequest& req) {

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

@@ -27,26 +27,37 @@ inline void NamedLog(wpi::Logger& logger, unsigned int level, const char* file,
 
 }  // namespace cs
 
-#define LOG(level, format, ...) WPI_LOG(m_logger, level, format, __VA_ARGS__)
+#define LOG(level, format, ...) \
+  WPI_LOG(m_logger, level, format __VA_OPT__(, ) __VA_ARGS__)
 
 #undef ERROR
-#define ERROR(format, ...) WPI_ERROR(m_logger, format, __VA_ARGS__)
-#define WARNING(format, ...) WPI_WARNING(m_logger, format, __VA_ARGS__)
-#define INFO(format, ...) WPI_INFO(m_logger, format, __VA_ARGS__)
+#define ERROR(format, ...) \
+  WPI_ERROR(m_logger, format __VA_OPT__(, ) __VA_ARGS__)
+#define WARNING(format, ...) \
+  WPI_WARNING(m_logger, format __VA_OPT__(, ) __VA_ARGS__)
+#define INFO(format, ...) WPI_INFO(m_logger, format __VA_OPT__(, ) __VA_ARGS__)
 
-#define DEBUG0(format, ...) WPI_DEBUG(m_logger, format, __VA_ARGS__)
-#define DEBUG1(format, ...) WPI_DEBUG1(m_logger, format, __VA_ARGS__)
-#define DEBUG2(format, ...) WPI_DEBUG2(m_logger, format, __VA_ARGS__)
-#define DEBUG3(format, ...) WPI_DEBUG3(m_logger, format, __VA_ARGS__)
-#define DEBUG4(format, ...) WPI_DEBUG4(m_logger, format, __VA_ARGS__)
+#define DEBUG0(format, ...) \
+  WPI_DEBUG(m_logger, format __VA_OPT__(, ) __VA_ARGS__)
+#define DEBUG1(format, ...) \
+  WPI_DEBUG1(m_logger, format __VA_OPT__(, ) __VA_ARGS__)
+#define DEBUG2(format, ...) \
+  WPI_DEBUG2(m_logger, format __VA_OPT__(, ) __VA_ARGS__)
+#define DEBUG3(format, ...) \
+  WPI_DEBUG3(m_logger, format __VA_OPT__(, ) __VA_ARGS__)
+#define DEBUG4(format, ...) \
+  WPI_DEBUG4(m_logger, format __VA_OPT__(, ) __VA_ARGS__)
 
-#define SLOG(level, format, ...)                                               \
-  NamedLog(m_logger, level, __FILE__, __LINE__, GetName(), FMT_STRING(format), \
-           __VA_ARGS__)
+#define SLOG(level, format, ...)                           \
+  NamedLog(m_logger, level, __FILE__, __LINE__, GetName(), \
+           FMT_STRING(format) __VA_OPT__(, ) __VA_ARGS__)
 
-#define SERROR(format, ...) SLOG(::wpi::WPI_LOG_ERROR, format, __VA_ARGS__)
-#define SWARNING(format, ...) SLOG(::wpi::WPI_LOG_WARNING, format, __VA_ARGS__)
-#define SINFO(format, ...) SLOG(::wpi::WPI_LOG_INFO, format, __VA_ARGS__)
+#define SERROR(format, ...) \
+  SLOG(::wpi::WPI_LOG_ERROR, format __VA_OPT__(, ) __VA_ARGS__)
+#define SWARNING(format, ...) \
+  SLOG(::wpi::WPI_LOG_WARNING, format __VA_OPT__(, ) __VA_ARGS__)
+#define SINFO(format, ...) \
+  SLOG(::wpi::WPI_LOG_INFO, format __VA_OPT__(, ) __VA_ARGS__)
 
 #ifdef NDEBUG
 #define SDEBUG(format, ...) \
@@ -65,11 +76,16 @@ inline void NamedLog(wpi::Logger& logger, unsigned int level, const char* file,
   do {                       \
   } while (0)
 #else
-#define SDEBUG(format, ...) SLOG(::wpi::WPI_LOG_DEBUG, format, __VA_ARGS__)
-#define SDEBUG1(format, ...) SLOG(::wpi::WPI_LOG_DEBUG1, format, __VA_ARGS__)
-#define SDEBUG2(format, ...) SLOG(::wpi::WPI_LOG_DEBUG2, format, __VA_ARGS__)
-#define SDEBUG3(format, ...) SLOG(::wpi::WPI_LOG_DEBUG3, format, __VA_ARGS__)
-#define SDEBUG4(format, ...) SLOG(::wpi::WPI_LOG_DEBUG4, format, __VA_ARGS__)
+#define SDEBUG(format, ...) \
+  SLOG(::wpi::WPI_LOG_DEBUG, format __VA_OPT__(, ) __VA_ARGS__)
+#define SDEBUG1(format, ...) \
+  SLOG(::wpi::WPI_LOG_DEBUG1, format __VA_OPT__(, ) __VA_ARGS__)
+#define SDEBUG2(format, ...) \
+  SLOG(::wpi::WPI_LOG_DEBUG2, format __VA_OPT__(, ) __VA_ARGS__)
+#define SDEBUG3(format, ...) \
+  SLOG(::wpi::WPI_LOG_DEBUG3, format __VA_OPT__(, ) __VA_ARGS__)
+#define SDEBUG4(format, ...) \
+  SLOG(::wpi::WPI_LOG_DEBUG4, format __VA_OPT__(, ) __VA_ARGS__)
 #endif
 
 #endif  // CSCORE_LOG_H_

cscore/src/main/native/cpp/MjpegServerImpl.cpp

@@ -650,7 +650,7 @@ void MjpegServerImpl::Stop() {
 // Send HTTP response and a stream of JPG-frames
 void MjpegServerImpl::ConnThread::SendStream(wpi::raw_socket_ostream& os) {
   if (m_noStreaming) {
-    SERROR("{}", "Too many simultaneous client streams");
+    SERROR("Too many simultaneous client streams");
     SendError(os, 503, "Too many simultaneous streams");
     return;
   }
@@ -663,7 +663,7 @@ void MjpegServerImpl::ConnThread::SendStream(wpi::raw_socket_ostream& os) {
   SendHeader(oss, 200, "OK", "multipart/x-mixed-replace;boundary=" BOUNDARY);
   os << oss.str();
 
-  SDEBUG("{}", "Headers send, sending stream now");
+  SDEBUG("Headers send, sending stream now");
 
   Frame::Time lastFrameTime = 0;
   Frame::Time timePerFrame = 0;
@@ -685,7 +685,7 @@ void MjpegServerImpl::ConnThread::SendStream(wpi::raw_socket_ostream& os) {
       std::this_thread::sleep_for(std::chrono::milliseconds(200));
       continue;
     }
-    SDEBUG4("{}", "waiting for frame");
+    SDEBUG4("waiting for frame");
     Frame frame = source->GetNextFrame(0.225);  // blocks
     if (!m_active) {
       break;
@@ -783,7 +783,7 @@ void MjpegServerImpl::ConnThread::ProcessRequest() {
   wpi::SmallString<128> reqBuf;
   std::string_view req = is.getline(reqBuf, 4096);
   if (is.has_error()) {
-    SDEBUG("{}", "error getting request string");
+    SDEBUG("error getting request string");
     return;
   }
 
@@ -824,7 +824,7 @@ void MjpegServerImpl::ConnThread::ProcessRequest() {
   } else if (req.find("GET / ") != std::string_view::npos || req == "GET /\n") {
     kind = kRootPage;
   } else {
-    SDEBUG("{}", "HTTP request resource not found");
+    SDEBUG("HTTP request resource not found");
     SendError(os, 404, "Resource not found");
     return;
   }
@@ -866,11 +866,11 @@ void MjpegServerImpl::ConnThread::ProcessRequest() {
         SendHeader(os, 200, "OK", "text/plain");
         os << "Ignored due to no connected source."
            << "\r\n";
-        SDEBUG("{}", "Ignored due to no connected source.");
+        SDEBUG("Ignored due to no connected source.");
       }
       break;
     case kGetSettings:
-      SDEBUG("{}", "request for JSON file");
+      SDEBUG("request for JSON file");
       if (auto source = GetSource()) {
         SendJSON(os, *source, true);
       } else {
@@ -878,7 +878,7 @@ void MjpegServerImpl::ConnThread::ProcessRequest() {
       }
       break;
     case kGetSourceConfig:
-      SDEBUG("{}", "request for JSON file");
+      SDEBUG("request for JSON file");
       if (auto source = GetSource()) {
         SendHeader(os, 200, "OK", "application/json");
         CS_Status status = CS_OK;
@@ -889,7 +889,7 @@ void MjpegServerImpl::ConnThread::ProcessRequest() {
       }
       break;
     case kRootPage:
-      SDEBUG("{}", "request for root page");
+      SDEBUG("request for root page");
       SendHeader(os, 200, "OK", "text/html");
       if (auto source = GetSource()) {
         SendHTML(os, *source, false);
@@ -900,7 +900,7 @@ void MjpegServerImpl::ConnThread::ProcessRequest() {
       break;
   }
 
-  SDEBUG("{}", "leaving HTTP client thread");
+  SDEBUG("leaving HTTP client thread");
 }
 
 // worker thread for clients that connected to this server
@@ -927,7 +927,7 @@ void MjpegServerImpl::ServerThreadMain() {
     return;
   }
 
-  SDEBUG("{}", "waiting for clients to connect");
+  SDEBUG("waiting for clients to connect");
   while (m_active) {
     auto stream = m_acceptor->accept();
     if (!stream) {
@@ -977,7 +977,7 @@ void MjpegServerImpl::ServerThreadMain() {
     thr->m_cond.notify_one();
   }
 
-  SDEBUG("{}", "leaving server thread");
+  SDEBUG("leaving server thread");
 }
 
 void MjpegServerImpl::SetSourceImpl(std::shared_ptr<SourceImpl> source) {

cscore/src/main/native/cpp/RawSinkImpl.cpp

@@ -127,7 +127,7 @@ void RawSinkImpl::ThreadMain() {
       std::this_thread::sleep_for(std::chrono::seconds(1));
       continue;
     }
-    SDEBUG4("{}", "waiting for frame");
+    SDEBUG4("waiting for frame");
     Frame frame = source->GetNextFrame();  // blocks
     if (!m_active) {
       break;

cscore/src/main/native/linux/UsbCameraImpl.cpp

@@ -454,7 +454,7 @@ void UsbCameraImpl::CameraThreadMain() {
 
     // Handle notify events
     if (notify_fd >= 0 && FD_ISSET(notify_fd, &readfds)) {
-      SDEBUG4("{}", "notify event");
+      SDEBUG4("notify event");
       struct inotify_event event;
       do {
         // Read the event structure
@@ -483,7 +483,7 @@ void UsbCameraImpl::CameraThreadMain() {
 
     // Handle commands
     if (command_fd >= 0 && FD_ISSET(command_fd, &readfds)) {
-      SDEBUG4("{}", "got command");
+      SDEBUG4("got command");
       // Read it to clear
       eventfd_t val;
       eventfd_read(command_fd, &val);
@@ -493,7 +493,7 @@ void UsbCameraImpl::CameraThreadMain() {
 
     // Handle frames
     if (m_streaming && fd >= 0 && FD_ISSET(fd, &readfds)) {
-      SDEBUG4("{}", "grabbing image");
+      SDEBUG4("grabbing image");
 
       // Dequeue buffer
       struct v4l2_buffer buf;
@@ -501,7 +501,7 @@ void UsbCameraImpl::CameraThreadMain() {
       buf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
       buf.memory = V4L2_MEMORY_MMAP;
       if (DoIoctl(fd, VIDIOC_DQBUF, &buf) != 0) {
-        SWARNING("{}", "could not dequeue buffer");
+        SWARNING("could not dequeue buffer");
         wasStreaming = m_streaming;
         DeviceStreamOff();
         DeviceDisconnect();
@@ -525,7 +525,7 @@ void UsbCameraImpl::CameraThreadMain() {
         bool good = true;
         if (m_mode.pixelFormat == VideoMode::kMJPEG &&
             !GetJpegSize(image, &width, &height)) {
-          SWARNING("{}", "invalid JPEG image received from camera");
+          SWARNING("invalid JPEG image received from camera");
           good = false;
         }
         if (good) {
@@ -536,7 +536,7 @@ void UsbCameraImpl::CameraThreadMain() {
 
       // Requeue buffer
       if (DoIoctl(fd, VIDIOC_QBUF, &buf) != 0) {
-        SWARNING("{}", "could not requeue buffer");
+        SWARNING("could not requeue buffer");
         wasStreaming = m_streaming;
         DeviceStreamOff();
         DeviceDisconnect();
@@ -579,7 +579,7 @@ void UsbCameraImpl::DeviceConnect() {
   }
 
   // Try to open the device
-  SDEBUG3("{}", "opening device");
+  SDEBUG3("opening device");
   int fd = open(m_path.c_str(), O_RDWR);
   if (fd < 0) {
     return;
@@ -587,7 +587,7 @@ void UsbCameraImpl::DeviceConnect() {
   m_fd = fd;
 
   // Get capabilities
-  SDEBUG3("{}", "getting capabilities");
+  SDEBUG3("getting capabilities");
   struct v4l2_capability vcap;
   std::memset(&vcap, 0, sizeof(vcap));
   if (DoIoctl(fd, VIDIOC_QUERYCAP, &vcap) >= 0) {
@@ -599,18 +599,18 @@ void UsbCameraImpl::DeviceConnect() {
 
   // Get or restore video mode
   if (!m_properties_cached) {
-    SDEBUG3("{}", "caching properties");
+    SDEBUG3("caching properties");
     DeviceCacheProperties();
     DeviceCacheVideoModes();
     DeviceCacheMode();
     m_properties_cached = true;
   } else {
-    SDEBUG3("{}", "restoring video mode");
+    SDEBUG3("restoring video mode");
     DeviceSetMode();
     DeviceSetFPS();
 
     // Restore settings
-    SDEBUG3("{}", "restoring settings");
+    SDEBUG3("restoring settings");
     std::unique_lock lock2(m_mutex);
     for (size_t i = 0; i < m_propertyData.size(); ++i) {
       const auto prop =
@@ -625,21 +625,21 @@ void UsbCameraImpl::DeviceConnect() {
   }
 
   // Request buffers
-  SDEBUG3("{}", "allocating buffers");
+  SDEBUG3("allocating buffers");
   struct v4l2_requestbuffers rb;
   std::memset(&rb, 0, sizeof(rb));
   rb.count = kNumBuffers;
   rb.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
   rb.memory = V4L2_MEMORY_MMAP;
   if (DoIoctl(fd, VIDIOC_REQBUFS, &rb) != 0) {
-    SWARNING("{}", "could not allocate buffers");
+    SWARNING("could not allocate buffers");
     close(fd);
     m_fd = -1;
     return;
   }
 
   // Map buffers
-  SDEBUG3("{}", "mapping buffers");
+  SDEBUG3("mapping buffers");
   for (int i = 0; i < kNumBuffers; ++i) {
     struct v4l2_buffer buf;
     std::memset(&buf, 0, sizeof(buf));
@@ -689,7 +689,7 @@ bool UsbCameraImpl::DeviceStreamOn() {
   }
 
   // Queue buffers
-  SDEBUG3("{}", "queuing buffers");
+  SDEBUG3("queuing buffers");
   for (int i = 0; i < kNumBuffers; ++i) {
     struct v4l2_buffer buf;
     std::memset(&buf, 0, sizeof(buf));
@@ -708,7 +708,6 @@ bool UsbCameraImpl::DeviceStreamOn() {
     if (errno == ENOSPC) {
       // indicates too much USB bandwidth requested
       SERROR(
-          "{}",
           "could not start streaming due to USB bandwidth limitations; try a "
           "lower resolution or a different pixel format (VIDIOC_STREAMON: "
           "No space left on device)");
@@ -718,7 +717,7 @@ bool UsbCameraImpl::DeviceStreamOn() {
     }
     return false;
   }
-  SDEBUG4("{}", "enabled streaming");
+  SDEBUG4("enabled streaming");
   m_streaming = true;
   return true;
 }
@@ -735,7 +734,7 @@ bool UsbCameraImpl::DeviceStreamOff() {
   if (DoIoctl(fd, VIDIOC_STREAMOFF, &type) != 0) {
     return false;
   }
-  SDEBUG4("{}", "disabled streaming");
+  SDEBUG4("disabled streaming");
   m_streaming = false;
   return true;
 }
@@ -1000,7 +999,7 @@ void UsbCameraImpl::DeviceCacheMode() {
 #endif
   vfmt.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
   if (DoIoctl(fd, VIDIOC_G_FMT, &vfmt) != 0) {
-    SERROR("{}", "could not read current video mode");
+    SERROR("could not read current video mode");
     std::scoped_lock lock(m_mutex);
     m_mode = VideoMode{VideoMode::kMJPEG, 320, 240, 30};
     return;
@@ -1668,7 +1667,7 @@ std::vector<UsbCameraInfo> EnumerateUsbCameras(CS_Status* status) {
     ::closedir(dp);
   } else {
     // *status = ;
-    WPI_ERROR(Instance::GetInstance().logger, "{}", "Could not open /dev");
+    WPI_ERROR(Instance::GetInstance().logger, "Could not open /dev");
     return retval;
   }
 

cscore/src/main/native/windows/UsbCameraImpl.cpp

@@ -486,7 +486,7 @@ bool UsbCameraImpl::DeviceConnect() {
     SINFO("Connecting to USB camera on {}", m_path);
   }
 
-  SDEBUG3("{}", "opening device");
+  SDEBUG3("opening device");
 
   const wchar_t* path = m_widePath.c_str();
   m_mediaSource = CreateVideoCaptureDevice(path);
@@ -520,13 +520,13 @@ bool UsbCameraImpl::DeviceConnect() {
   }
 
   if (!m_properties_cached) {
-    SDEBUG3("{}", "caching properties");
+    SDEBUG3("caching properties");
     DeviceCacheProperties();
     DeviceCacheVideoModes();
     DeviceCacheMode();
     m_properties_cached = true;
   } else {
-    SDEBUG3("{}", "restoring video mode");
+    SDEBUG3("restoring video mode");
     DeviceSetMode();
   }
 

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

@@ -176,7 +176,6 @@ static void UpdateMsgpackValueSource(NetworkTablesModel::ValueSource* out,
     case mpack::mpack_type_str: {
       std::string str;
       mpack_read_str(&r, &tag, &str);
-      mpack_done_str(&r);
       out->UpdateFromValue(nt::Value::MakeString(std::move(str), time), name,
                            "");
       break;
@@ -461,7 +460,6 @@ void NetworkTablesModel::Update() {
                              entry->info.type_str);
       if (wpi::starts_with(entry->info.name, '$') && entry->value.IsRaw() &&
           entry->info.type_str == "msgpack") {
-        fmt::print(stderr, "Updating meta-topic {}\n", entry->info.name);
         // meta topic handling
         if (entry->info.name == "$clients") {
           UpdateClients(entry->value.GetRaw());

glass/src/libnt/native/cpp/NetworkTablesSettings.cpp

@@ -61,11 +61,10 @@ void NetworkTablesSettings::Thread::Main() {
     if (m_mode == 1 || m_mode == 2) {
       std::string_view serverTeam{m_serverTeam};
       std::optional<unsigned int> team;
-      nt::SetNetworkIdentity(m_inst, m_clientName);
       if (m_mode == 1) {
-        nt::StartClient4(m_inst);
+        nt::StartClient4(m_inst, m_clientName);
       } else if (m_mode == 2) {
-        nt::StartClient3(m_inst);
+        nt::StartClient3(m_inst, m_clientName);
       }
       if (!wpi::contains(serverTeam, '.') &&
           (team = wpi::parse_integer<unsigned int>(serverTeam, 10))) {

hal/src/main/java/edu/wpi/first/hal/DriverStationJNI.java

@@ -0,0 +1,137 @@
+// Copyright (c) FIRST and other WPILib contributors.
+// Open Source Software; you can modify and/or share it under the terms of
+// the WPILib BSD license file in the root directory of this project.
+
+package edu.wpi.first.hal;
+
+import java.nio.ByteBuffer;
+
+public class DriverStationJNI extends JNIWrapper {
+  public static native void observeUserProgramStarting();
+
+  public static native void observeUserProgramDisabled();
+
+  public static native void observeUserProgramAutonomous();
+
+  public static native void observeUserProgramTeleop();
+
+  public static native void observeUserProgramTest();
+
+  public static void report(int resource, int instanceNumber) {
+    report(resource, instanceNumber, 0, "");
+  }
+
+  public static void report(int resource, int instanceNumber, int context) {
+    report(resource, instanceNumber, context, "");
+  }
+
+  /**
+   * Report the usage of a resource of interest.
+   *
+   * <p>Original signature: <code>uint32_t report(tResourceType, uint8_t, uint8_t, const
+   * char*)</code>
+   *
+   * @param resource one of the values in the tResourceType above (max value 51).
+   * @param instanceNumber an index that identifies the resource instance.
+   * @param context an optional additional context number for some cases (such as module number).
+   *     Set to 0 to omit.
+   * @param feature a string to be included describing features in use on a specific resource.
+   *     Setting the same resource more than once allows you to change the feature string.
+   * @return TODO
+   */
+  public static native int report(int resource, int instanceNumber, int context, String feature);
+
+  public static native int nativeGetControlWord();
+
+  @SuppressWarnings("MissingJavadocMethod")
+  public static void getControlWord(ControlWord controlWord) {
+    int word = nativeGetControlWord();
+    controlWord.update(
+        (word & 1) != 0,
+        ((word >> 1) & 1) != 0,
+        ((word >> 2) & 1) != 0,
+        ((word >> 3) & 1) != 0,
+        ((word >> 4) & 1) != 0,
+        ((word >> 5) & 1) != 0);
+  }
+
+  private static native int nativeGetAllianceStation();
+
+  public static final int kRed1AllianceStation = 0;
+  public static final int kRed2AllianceStation = 1;
+  public static final int kRed3AllianceStation = 2;
+  public static final int kBlue1AllianceStation = 3;
+  public static final int kBlue2AllianceStation = 4;
+  public static final int kBlue3AllianceStation = 5;
+
+  @SuppressWarnings("MissingJavadocMethod")
+  public static AllianceStationID getAllianceStation() {
+    switch (nativeGetAllianceStation()) {
+      case kRed1AllianceStation:
+        return AllianceStationID.Red1;
+      case kRed2AllianceStation:
+        return AllianceStationID.Red2;
+      case kRed3AllianceStation:
+        return AllianceStationID.Red3;
+      case kBlue1AllianceStation:
+        return AllianceStationID.Blue1;
+      case kBlue2AllianceStation:
+        return AllianceStationID.Blue2;
+      case kBlue3AllianceStation:
+        return AllianceStationID.Blue3;
+      default:
+        return null;
+    }
+  }
+
+  public static final int kMaxJoystickAxes = 12;
+  public static final int kMaxJoystickPOVs = 12;
+  public static final int kMaxJoysticks = 6;
+
+  public static native int getJoystickAxes(byte joystickNum, float[] axesArray);
+
+  public static native int getJoystickAxesRaw(byte joystickNum, int[] rawAxesArray);
+
+  public static native int getJoystickPOVs(byte joystickNum, short[] povsArray);
+
+  public static native int getJoystickButtons(byte joystickNum, ByteBuffer count);
+
+  public static native void getAllJoystickData(
+      float[] axesArray, byte[] rawAxesArray, short[] povsArray, long[] buttonsAndMetadata);
+
+  public static native int setJoystickOutputs(
+      byte joystickNum, int outputs, short leftRumble, short rightRumble);
+
+  public static native int getJoystickIsXbox(byte joystickNum);
+
+  public static native int getJoystickType(byte joystickNum);
+
+  public static native String getJoystickName(byte joystickNum);
+
+  public static native int getJoystickAxisType(byte joystickNum, byte axis);
+
+  public static native double getMatchTime();
+
+  public static native int getMatchInfo(MatchInfoData info);
+
+  public static native int sendError(
+      boolean isError,
+      int errorCode,
+      boolean isLVCode,
+      String details,
+      String location,
+      String callStack,
+      boolean printMsg);
+
+  public static native int sendConsoleLine(String line);
+
+  public static native void refreshDSData();
+
+  public static native void provideNewDataEventHandle(int handle);
+
+  public static native void removeNewDataEventHandle(int handle);
+
+  public static native boolean getOutputsActive();
+
+  private DriverStationJNI() {}
+}

hal/src/main/java/edu/wpi/first/hal/HAL.java

@@ -4,7 +4,6 @@
 
 package edu.wpi.first.hal;
 
-import java.nio.ByteBuffer;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -13,8 +12,6 @@ import java.util.List;
  * .
  */
 public final class HAL extends JNIWrapper {
-  public static native void waitForDSData();
-
   public static native boolean initialize(int timeout, int mode);
 
   public static native void shutdown();
@@ -116,15 +113,13 @@ public final class HAL extends JNIWrapper {
     }
   }
 
-  public static native void observeUserProgramStarting();
+  public static native boolean getBrownedOut();
 
-  public static native void observeUserProgramDisabled();
+  public static native boolean getSystemActive();
 
-  public static native void observeUserProgramAutonomous();
+  public static native int getPortWithModule(byte module, byte channel);
 
-  public static native void observeUserProgramTeleop();
-
-  public static native void observeUserProgramTest();
+  public static native int getPort(byte channel);
 
   public static void report(int resource, int instanceNumber) {
     report(resource, instanceNumber, 0, "");
@@ -134,114 +129,9 @@ public final class HAL extends JNIWrapper {
     report(resource, instanceNumber, context, "");
   }
 
-  /**
-   * Report the usage of a resource of interest.
-   *
-   * <p>Original signature: <code>uint32_t report(tResourceType, uint8_t, uint8_t, const
-   * char*)</code>
-   *
-   * @param resource one of the values in the tResourceType above (max value 51).
-   * @param instanceNumber an index that identifies the resource instance.
-   * @param context an optional additional context number for some cases (such as module number).
-   *     Set to 0 to omit.
-   * @param feature a string to be included describing features in use on a specific resource.
-   *     Setting the same resource more than once allows you to change the feature string.
-   * @return TODO
-   */
-  public static native int report(int resource, int instanceNumber, int context, String feature);
-
-  public static native int nativeGetControlWord();
-
-  /**
-   * Get the current DriverStation control word.
-   *
-   * @param controlWord Storage for control word.
-   */
-  public static void getControlWord(ControlWord controlWord) {
-    int word = nativeGetControlWord();
-    controlWord.update(
-        (word & 1) != 0,
-        ((word >> 1) & 1) != 0,
-        ((word >> 2) & 1) != 0,
-        ((word >> 3) & 1) != 0,
-        ((word >> 4) & 1) != 0,
-        ((word >> 5) & 1) != 0);
+  public static int report(int resource, int instanceNumber, int context, String feature) {
+    return DriverStationJNI.report(resource, instanceNumber, context, feature);
   }
 
-  private static native int nativeGetAllianceStation();
-
-  /**
-   * Get the alliance station.
-   *
-   * @return The alliance station.
-   */
-  public static AllianceStationID getAllianceStation() {
-    switch (nativeGetAllianceStation()) {
-      case 0:
-        return AllianceStationID.Red1;
-      case 1:
-        return AllianceStationID.Red2;
-      case 2:
-        return AllianceStationID.Red3;
-      case 3:
-        return AllianceStationID.Blue1;
-      case 4:
-        return AllianceStationID.Blue2;
-      case 5:
-        return AllianceStationID.Blue3;
-      default:
-        return null;
-    }
-  }
-
-  public static native boolean isNewControlData();
-
-  public static native void releaseDSMutex();
-
-  public static native boolean waitForDSDataTimeout(double timeout);
-
-  public static final int kMaxJoystickAxes = 12;
-  public static final int kMaxJoystickPOVs = 12;
-
-  public static native short getJoystickAxes(byte joystickNum, float[] axesArray);
-
-  public static native short getJoystickPOVs(byte joystickNum, short[] povsArray);
-
-  public static native int getJoystickButtons(byte joystickNum, ByteBuffer count);
-
-  public static native int setJoystickOutputs(
-      byte joystickNum, int outputs, short leftRumble, short rightRumble);
-
-  public static native int getJoystickIsXbox(byte joystickNum);
-
-  public static native int getJoystickType(byte joystickNum);
-
-  public static native String getJoystickName(byte joystickNum);
-
-  public static native int getJoystickAxisType(byte joystickNum, byte axis);
-
-  public static native double getMatchTime();
-
-  public static native boolean getSystemActive();
-
-  public static native boolean getBrownedOut();
-
-  public static native int getMatchInfo(MatchInfoData info);
-
-  public static native int sendError(
-      boolean isError,
-      int errorCode,
-      boolean isLVCode,
-      String details,
-      String location,
-      String callStack,
-      boolean printMsg);
-
-  public static native int sendConsoleLine(String line);
-
-  public static native int getPortWithModule(byte module, byte channel);
-
-  public static native int getPort(byte channel);
-
   private HAL() {}
 }

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

@@ -13,11 +13,14 @@
 #include <FRC_NetworkCommunication/FRCComm.h>
 #include <FRC_NetworkCommunication/NetCommRPCProxy_Occur.h>
 #include <fmt/format.h>
+#include <wpi/EventVector.h>
 #include <wpi/SafeThread.h>
+#include <wpi/SmallVector.h>
 #include <wpi/condition_variable.h>
 #include <wpi/mutex.h>
 
 #include "hal/DriverStation.h"
+#include "hal/Errors.h"
 
 static_assert(sizeof(int32_t) >= sizeof(int),
               "FRC_NetworkComm status variable is larger than 32 bits");
@@ -27,25 +30,45 @@ struct HAL_JoystickAxesInt {
   int16_t axes[HAL_kMaxJoystickAxes];
 };
 
-static constexpr int kJoystickPorts = 6;
+namespace {
+struct JoystickDataCache {
+  JoystickDataCache() { std::memset(this, 0, sizeof(*this)); }
+  void Update();
+
+  HAL_JoystickAxes axes[HAL_kMaxJoysticks];
+  HAL_JoystickPOVs povs[HAL_kMaxJoysticks];
+  HAL_JoystickButtons buttons[HAL_kMaxJoysticks];
+  HAL_AllianceStationID allianceStation;
+  float matchTime;
+  bool updated;
+};
+static_assert(std::is_standard_layout_v<JoystickDataCache>);
+// static_assert(std::is_trivial_v<JoystickDataCache>);
+
+struct FRCDriverStation {
+  wpi::EventVector newDataEvents;
+};
+}  // namespace
+
+static ::FRCDriverStation* driverStation;
 
 // Message and Data variables
 static wpi::mutex msgMutex;
 
 static int32_t HAL_GetJoystickAxesInternal(int32_t joystickNum,
                                            HAL_JoystickAxes* axes) {
-  HAL_JoystickAxesInt axesInt;
+  JoystickAxes_t netcommAxes;
 
   int retVal = FRC_NetworkCommunication_getJoystickAxes(
-      joystickNum, reinterpret_cast<JoystickAxes_t*>(&axesInt),
-      HAL_kMaxJoystickAxes);
+      joystickNum, &netcommAxes, HAL_kMaxJoystickAxes);
 
   // copy integer values to double values
-  axes->count = axesInt.count;
+  axes->count = netcommAxes.count;
   // current scaling is -128 to 127, can easily be patched in the future by
   // changing this function.
-  for (int32_t i = 0; i < axesInt.count; i++) {
-    int8_t value = axesInt.axes[i];
+  for (int32_t i = 0; i < netcommAxes.count; i++) {
+    int8_t value = netcommAxes.axes[i];
+    axes->raw[i] = value;
     if (value < 0) {
       axes->axes[i] = value / 128.0;
     } else {
@@ -68,6 +91,30 @@ static int32_t HAL_GetJoystickButtonsInternal(int32_t joystickNum,
   return FRC_NetworkCommunication_getJoystickButtons(
       joystickNum, &buttons->buttons, &buttons->count);
 }
+
+void JoystickDataCache::Update() {
+  for (int i = 0; i < HAL_kMaxJoysticks; i++) {
+    HAL_GetJoystickAxesInternal(i, &axes[i]);
+    HAL_GetJoystickPOVsInternal(i, &povs[i]);
+    HAL_GetJoystickButtonsInternal(i, &buttons[i]);
+  }
+  FRC_NetworkCommunication_getAllianceStation(
+      reinterpret_cast<AllianceStationID_t*>(&allianceStation));
+  FRC_NetworkCommunication_getMatchTime(&matchTime);
+}
+
+#define CHECK_JOYSTICK_NUMBER(stickNum)                  \
+  if ((stickNum) < 0 || (stickNum) >= HAL_kMaxJoysticks) \
+  return PARAMETER_OUT_OF_RANGE
+
+static HAL_ControlWord newestControlWord;
+static JoystickDataCache caches[3];
+static JoystickDataCache* currentRead = &caches[0];
+static JoystickDataCache* currentCache = &caches[1];
+static JoystickDataCache* cacheToUpdate = &caches[2];
+
+static wpi::mutex cacheMutex;
+
 /**
  * Retrieve the Joystick Descriptor for particular slot.
  *
@@ -102,12 +149,6 @@ static int32_t HAL_GetJoystickDescriptorInternal(int32_t joystickNum,
   return retval;
 }
 
-static int32_t HAL_GetControlWordInternal(HAL_ControlWord* controlWord) {
-  std::memset(controlWord, 0, sizeof(HAL_ControlWord));
-  return FRC_NetworkCommunication_getControlWord(
-      reinterpret_cast<ControlWord_t*>(controlWord));
-}
-
 static int32_t HAL_GetMatchInfoInternal(HAL_MatchInfo* info) {
   MatchType_t matchType = MatchType_t::kMatchType_none;
   info->gameSpecificMessageSize = sizeof(info->gameSpecificMessage);
@@ -126,16 +167,11 @@ static int32_t HAL_GetMatchInfoInternal(HAL_MatchInfo* info) {
   return status;
 }
 
-static wpi::mutex* newDSDataAvailableMutex;
-static wpi::condition_variable* newDSDataAvailableCond;
-static int newDSDataAvailableCounter{0};
-
 namespace hal::init {
 void InitializeFRCDriverStation() {
-  static wpi::mutex newMutex;
-  newDSDataAvailableMutex = &newMutex;
-  static wpi::condition_variable newCond;
-  newDSDataAvailableCond = &newCond;
+  std::memset(&newestControlWord, 0, sizeof(newestControlWord));
+  static FRCDriverStation ds;
+  driverStation = &ds;
 }
 }  // namespace hal::init
 
@@ -248,20 +284,39 @@ int32_t HAL_SendConsoleLine(const char* line) {
 }
 
 int32_t HAL_GetControlWord(HAL_ControlWord* controlWord) {
-  return HAL_GetControlWordInternal(controlWord);
+  std::scoped_lock lock{cacheMutex};
+  *controlWord = newestControlWord;
+  return 0;
 }
 
 int32_t HAL_GetJoystickAxes(int32_t joystickNum, HAL_JoystickAxes* axes) {
-  return HAL_GetJoystickAxesInternal(joystickNum, axes);
+  CHECK_JOYSTICK_NUMBER(joystickNum);
+  std::scoped_lock lock{cacheMutex};
+  *axes = currentRead->axes[joystickNum];
+  return 0;
 }
 
 int32_t HAL_GetJoystickPOVs(int32_t joystickNum, HAL_JoystickPOVs* povs) {
-  return HAL_GetJoystickPOVsInternal(joystickNum, povs);
+  CHECK_JOYSTICK_NUMBER(joystickNum);
+  std::scoped_lock lock{cacheMutex};
+  *povs = currentRead->povs[joystickNum];
+  return 0;
 }
 
 int32_t HAL_GetJoystickButtons(int32_t joystickNum,
                                HAL_JoystickButtons* buttons) {
-  return HAL_GetJoystickButtonsInternal(joystickNum, buttons);
+  CHECK_JOYSTICK_NUMBER(joystickNum);
+  std::scoped_lock lock{cacheMutex};
+  *buttons = currentRead->buttons[joystickNum];
+  return 0;
+}
+
+void HAL_GetAllJoystickData(HAL_JoystickAxes* axes, HAL_JoystickPOVs* povs,
+                            HAL_JoystickButtons* buttons) {
+  std::scoped_lock lock{cacheMutex};
+  std::memcpy(axes, currentRead->axes, sizeof(currentRead->axes));
+  std::memcpy(povs, currentRead->povs, sizeof(currentRead->povs));
+  std::memcpy(buttons, currentRead->buttons, sizeof(currentRead->buttons));
 }
 
 int32_t HAL_GetJoystickDescriptor(int32_t joystickNum,
@@ -274,10 +329,8 @@ int32_t HAL_GetMatchInfo(HAL_MatchInfo* info) {
 }
 
 HAL_AllianceStationID HAL_GetAllianceStation(int32_t* status) {
-  HAL_AllianceStationID allianceStation;
-  *status = FRC_NetworkCommunication_getAllianceStation(
-      reinterpret_cast<AllianceStationID_t*>(&allianceStation));
-  return allianceStation;
+  std::scoped_lock lock{cacheMutex};
+  return currentRead->allianceStation;
 }
 
 HAL_Bool HAL_GetJoystickIsXbox(int32_t joystickNum) {
@@ -317,6 +370,7 @@ void HAL_FreeJoystickName(char* name) {
 }
 
 int32_t HAL_GetJoystickAxisType(int32_t joystickNum, int32_t axis) {
+  CHECK_JOYSTICK_NUMBER(joystickNum);
   HAL_JoystickDescriptor joystickDesc;
   if (HAL_GetJoystickDescriptor(joystickNum, &joystickDesc) < 0) {
     return -1;
@@ -327,14 +381,14 @@ int32_t HAL_GetJoystickAxisType(int32_t joystickNum, int32_t axis) {
 
 int32_t HAL_SetJoystickOutputs(int32_t joystickNum, int64_t outputs,
                                int32_t leftRumble, int32_t rightRumble) {
+  CHECK_JOYSTICK_NUMBER(joystickNum);
   return FRC_NetworkCommunication_setJoystickOutputs(joystickNum, outputs,
                                                      leftRumble, rightRumble);
 }
 
 double HAL_GetMatchTime(int32_t* status) {
-  float matchTime;
-  *status = FRC_NetworkCommunication_getMatchTime(&matchTime);
-  return matchTime;
+  std::scoped_lock lock{cacheMutex};
+  return currentRead->matchTime;
 }
 
 void HAL_ObserveUserProgramStarting(void) {
@@ -357,55 +411,6 @@ void HAL_ObserveUserProgramTest(void) {
   FRC_NetworkCommunication_observeUserProgramTest();
 }
 
-static int& GetThreadLocalLastCount() {
-  // There is a rollover error condition here. At Packet# = n * (uintmax), this
-  // will return false when instead it should return true. However, this at a
-  // 20ms rate occurs once every 2.7 years of DS connected runtime, so not
-  // worth the cycles to check.
-  thread_local int lastCount{0};
-  return lastCount;
-}
-
-HAL_Bool HAL_IsNewControlData(void) {
-  std::scoped_lock lock{*newDSDataAvailableMutex};
-  int& lastCount = GetThreadLocalLastCount();
-  int currentCount = newDSDataAvailableCounter;
-  if (lastCount == currentCount) {
-    return false;
-  }
-  lastCount = currentCount;
-  return true;
-}
-
-void HAL_WaitForDSData(void) {
-  HAL_WaitForDSDataTimeout(0);
-}
-
-HAL_Bool HAL_WaitForDSDataTimeout(double timeout) {
-  std::unique_lock lock{*newDSDataAvailableMutex};
-  int& lastCount = GetThreadLocalLastCount();
-  int currentCount = newDSDataAvailableCounter;
-  if (lastCount != currentCount) {
-    lastCount = currentCount;
-    return true;
-  }
-  auto timeoutTime =
-      std::chrono::steady_clock::now() + std::chrono::duration<double>(timeout);
-
-  while (newDSDataAvailableCounter == currentCount) {
-    if (timeout > 0) {
-      auto timedOut = newDSDataAvailableCond->wait_until(lock, timeoutTime);
-      if (timedOut == std::cv_status::timeout) {
-        return false;
-      }
-    } else {
-      newDSDataAvailableCond->wait(lock);
-    }
-  }
-  lastCount = newDSDataAvailableCounter;
-  return true;
-}
-
 // Constant number to be used for our occur handle
 constexpr int32_t refNumber = 42;
 
@@ -415,45 +420,47 @@ static void newDataOccur(uint32_t refNum) {
   if (refNum != refNumber) {
     return;
   }
-  std::scoped_lock lock{*newDSDataAvailableMutex};
-  // Notify all threads
-  ++newDSDataAvailableCounter;
-  newDSDataAvailableCond->notify_all();
+  cacheToUpdate->Update();
+  {
+    std::scoped_lock lock{cacheMutex};
+    std::swap(currentCache, cacheToUpdate);
+    currentCache->updated = true;
+  }
+  driverStation->newDataEvents.Wakeup();
 }
 
-/*
- * Call this to initialize the driver station communication. This will properly
- * handle multiple calls. However note that this CANNOT be called from a library
- * that interfaces with LabVIEW.
- */
-void HAL_InitializeDriverStation(void) {
-  static std::atomic_bool initialized{false};
-  static wpi::mutex initializeMutex;
-  // Initial check, as if it's true initialization has finished
-  if (initialized) {
-    return;
+void HAL_RefreshDSData(void) {
+  HAL_ControlWord controlWord;
+  std::memset(&controlWord, 0, sizeof(controlWord));
+  FRC_NetworkCommunication_getControlWord(
+      reinterpret_cast<ControlWord_t*>(&controlWord));
+  std::scoped_lock lock{cacheMutex};
+  if (currentCache->updated) {
+    std::swap(currentCache, currentRead);
+    currentCache->updated = false;
   }
+  newestControlWord = controlWord;
+}
 
-  std::scoped_lock lock(initializeMutex);
-  // Second check in case another thread was waiting
-  if (initialized) {
-    return;
-  }
+void HAL_ProvideNewDataEventHandle(WPI_EventHandle handle) {
+  driverStation->newDataEvents.Add(handle);
+}
 
+void HAL_RemoveNewDataEventHandle(WPI_EventHandle handle) {
+  driverStation->newDataEvents.Remove(handle);
+}
+
+HAL_Bool HAL_GetOutputsEnabled(void) {
+  return FRC_NetworkCommunication_getWatchdogActive();
+}
+
+}  // extern "C"
+
+namespace hal {
+void InitializeDriverStation() {
   // Set up the occur function internally with NetComm
   NetCommRPCProxy_SetOccurFuncPointer(newDataOccur);
   // Set up our occur reference number
   setNewDataOccurRef(refNumber);
-
-  initialized = true;
 }
-
-/*
- * Releases the DS Mutex to allow proper shutdown of any threads that are
- * waiting on it.
- */
-void HAL_ReleaseDSMutex(void) {
-  newDataOccur(refNumber);
-}
-
-}  // extern "C"
+}  // namespace hal

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

@@ -40,6 +40,7 @@ static uint64_t dsStartTime;
 using namespace hal;
 
 namespace hal {
+void InitializeDriverStation();
 namespace init {
 void InitializeHAL() {
   InitializeCTREPCM();
@@ -431,7 +432,7 @@ HAL_Bool HAL_Initialize(int32_t timeout, int32_t mode) {
     return false;
   }
 
-  HAL_InitializeDriverStation();
+  hal::InitializeDriverStation();
 
   dsStartTime = HAL_GetFPGATime(&status);
   if (status != 0) {

hal/src/main/native/cpp/jni/DriverStationJNI.cpp

@@ -0,0 +1,450 @@
+// Copyright (c) FIRST and other WPILib contributors.
+// Open Source Software; you can modify and/or share it under the terms of
+// the WPILib BSD license file in the root directory of this project.
+
+#include <jni.h>
+
+#include <cassert>
+
+#include <fmt/format.h>
+#include <wpi/jni_util.h>
+
+#include "HALUtil.h"
+#include "edu_wpi_first_hal_DriverStationJNI.h"
+#include "hal/DriverStation.h"
+#include "hal/FRCUsageReporting.h"
+#include "hal/HALBase.h"
+
+// TODO Static asserts
+
+using namespace hal;
+using namespace wpi::java;
+
+extern "C" {
+
+/*
+ * Class:     edu_wpi_first_hal_DriverStationJNI
+ * Method:    observeUserProgramStarting
+ * Signature: ()V
+ */
+JNIEXPORT void JNICALL
+Java_edu_wpi_first_hal_DriverStationJNI_observeUserProgramStarting
+  (JNIEnv*, jclass)
+{
+  HAL_ObserveUserProgramStarting();
+}
+
+/*
+ * Class:     edu_wpi_first_hal_DriverStationJNI
+ * Method:    observeUserProgramDisabled
+ * Signature: ()V
+ */
+JNIEXPORT void JNICALL
+Java_edu_wpi_first_hal_DriverStationJNI_observeUserProgramDisabled
+  (JNIEnv*, jclass)
+{
+  HAL_ObserveUserProgramDisabled();
+}
+
+/*
+ * Class:     edu_wpi_first_hal_DriverStationJNI
+ * Method:    observeUserProgramAutonomous
+ * Signature: ()V
+ */
+JNIEXPORT void JNICALL
+Java_edu_wpi_first_hal_DriverStationJNI_observeUserProgramAutonomous
+  (JNIEnv*, jclass)
+{
+  HAL_ObserveUserProgramAutonomous();
+}
+
+/*
+ * Class:     edu_wpi_first_hal_DriverStationJNI
+ * Method:    observeUserProgramTeleop
+ * Signature: ()V
+ */
+JNIEXPORT void JNICALL
+Java_edu_wpi_first_hal_DriverStationJNI_observeUserProgramTeleop
+  (JNIEnv*, jclass)
+{
+  HAL_ObserveUserProgramTeleop();
+}
+
+/*
+ * Class:     edu_wpi_first_hal_DriverStationJNI
+ * Method:    observeUserProgramTest
+ * Signature: ()V
+ */
+JNIEXPORT void JNICALL
+Java_edu_wpi_first_hal_DriverStationJNI_observeUserProgramTest
+  (JNIEnv*, jclass)
+{
+  HAL_ObserveUserProgramTest();
+}
+
+/*
+ * Class:     edu_wpi_first_hal_DriverStationJNI
+ * Method:    report
+ * Signature: (IIILjava/lang/String;)I
+ */
+JNIEXPORT jint JNICALL
+Java_edu_wpi_first_hal_DriverStationJNI_report
+  (JNIEnv* paramEnv, jclass, jint paramResource, jint paramInstanceNumber,
+   jint paramContext, jstring paramFeature)
+{
+  JStringRef featureStr{paramEnv, paramFeature};
+  jint returnValue = HAL_Report(paramResource, paramInstanceNumber,
+                                paramContext, featureStr.c_str());
+  return returnValue;
+}
+
+/*
+ * Class:     edu_wpi_first_hal_DriverStationJNI
+ * Method:    nativeGetControlWord
+ * Signature: ()I
+ */
+JNIEXPORT jint JNICALL
+Java_edu_wpi_first_hal_DriverStationJNI_nativeGetControlWord
+  (JNIEnv*, jclass)
+{
+  static_assert(sizeof(HAL_ControlWord) == sizeof(jint),
+                "Java int must match the size of control word");
+  HAL_ControlWord controlWord;
+  HAL_GetControlWord(&controlWord);
+  jint retVal = 0;
+  std::memcpy(&retVal, &controlWord, sizeof(HAL_ControlWord));
+  return retVal;
+}
+
+/*
+ * Class:     edu_wpi_first_hal_DriverStationJNI
+ * Method:    nativeGetAllianceStation
+ * Signature: ()I
+ */
+JNIEXPORT jint JNICALL
+Java_edu_wpi_first_hal_DriverStationJNI_nativeGetAllianceStation
+  (JNIEnv*, jclass)
+{
+  int32_t status = 0;
+  auto allianceStation = HAL_GetAllianceStation(&status);
+  return static_cast<jint>(allianceStation);
+}
+
+/*
+ * Class:     edu_wpi_first_hal_DriverStationJNI
+ * Method:    getJoystickAxesRaw
+ * Signature: (B[I)I
+ */
+JNIEXPORT jint JNICALL
+Java_edu_wpi_first_hal_DriverStationJNI_getJoystickAxesRaw
+  (JNIEnv* env, jclass, jbyte joystickNum, jintArray axesRawArray)
+{
+  HAL_JoystickAxes axes;
+  HAL_GetJoystickAxes(joystickNum, &axes);
+
+  jsize javaSize = env->GetArrayLength(axesRawArray);
+  if (axes.count > javaSize) {
+    ThrowIllegalArgumentException(
+        env,
+        fmt::format("Native array size larger then passed in java array "
+                    "size\nNative Size: {} Java Size: {}",
+                    static_cast<int>(axes.count), static_cast<int>(javaSize)));
+    return 0;
+  }
+
+  jint raw[HAL_kMaxJoystickAxes];
+  for (int16_t i = 0; i < axes.count; i++) {
+    raw[i] = axes.raw[i];
+  }
+  env->SetIntArrayRegion(axesRawArray, 0, axes.count, raw);
+
+  return axes.count;
+}
+
+/*
+ * Class:     edu_wpi_first_hal_DriverStationJNI
+ * Method:    getJoystickAxes
+ * Signature: (B[F)I
+ */
+JNIEXPORT jint JNICALL
+Java_edu_wpi_first_hal_DriverStationJNI_getJoystickAxes
+  (JNIEnv* env, jclass, jbyte joystickNum, jfloatArray axesArray)
+{
+  HAL_JoystickAxes axes;
+  HAL_GetJoystickAxes(joystickNum, &axes);
+
+  jsize javaSize = env->GetArrayLength(axesArray);
+  if (axes.count > javaSize) {
+    ThrowIllegalArgumentException(
+        env,
+        fmt::format("Native array size larger then passed in java array "
+                    "size\nNative Size: {} Java Size: {}",
+                    static_cast<int>(axes.count), static_cast<int>(javaSize)));
+    return 0;
+  }
+
+  env->SetFloatArrayRegion(axesArray, 0, axes.count, axes.axes);
+
+  return axes.count;
+}
+
+/*
+ * Class:     edu_wpi_first_hal_DriverStationJNI
+ * Method:    getJoystickPOVs
+ * Signature: (B[S)I
+ */
+JNIEXPORT jint JNICALL
+Java_edu_wpi_first_hal_DriverStationJNI_getJoystickPOVs
+  (JNIEnv* env, jclass, jbyte joystickNum, jshortArray povsArray)
+{
+  HAL_JoystickPOVs povs;
+  HAL_GetJoystickPOVs(joystickNum, &povs);
+
+  jsize javaSize = env->GetArrayLength(povsArray);
+  if (povs.count > javaSize) {
+    ThrowIllegalArgumentException(
+        env,
+        fmt::format("Native array size larger then passed in java array "
+                    "size\nNative Size: {} Java Size: {}",
+                    static_cast<int>(povs.count), static_cast<int>(javaSize)));
+    return 0;
+  }
+
+  env->SetShortArrayRegion(povsArray, 0, povs.count, povs.povs);
+
+  return povs.count;
+}
+
+/*
+ * Class:     edu_wpi_first_hal_DriverStationJNI
+ * Method:    getAllJoystickData
+ * Signature: ([F[B[S[J)V
+ */
+JNIEXPORT void JNICALL
+Java_edu_wpi_first_hal_DriverStationJNI_getAllJoystickData
+  (JNIEnv* env, jclass cls, jfloatArray axesArray, jbyteArray rawAxesArray,
+   jshortArray povsArray, jlongArray buttonsAndMetadataArray)
+{
+  HAL_JoystickAxes axes[HAL_kMaxJoysticks];
+  HAL_JoystickPOVs povs[HAL_kMaxJoysticks];
+  HAL_JoystickButtons buttons[HAL_kMaxJoysticks];
+
+  HAL_GetAllJoystickData(axes, povs, buttons);
+
+  CriticalJFloatArrayRef jAxes(env, axesArray);
+  CriticalJByteArrayRef jRawAxes(env, rawAxesArray);
+  CriticalJShortArrayRef jPovs(env, povsArray);
+  CriticalJLongArrayRef jButtons(env, buttonsAndMetadataArray);
+
+  static_assert(sizeof(jAxes[0]) == sizeof(axes[0].axes[0]));
+  static_assert(sizeof(jRawAxes[0]) == sizeof(axes[0].raw[0]));
+  static_assert(sizeof(jPovs[0]) == sizeof(povs[0].povs[0]));
+
+  for (size_t i = 0; i < HAL_kMaxJoysticks; i++) {
+    std::memcpy(&jAxes[i * HAL_kMaxJoystickAxes], axes[i].axes,
+                sizeof(axes[i].axes));
+    std::memcpy(&jRawAxes[i * HAL_kMaxJoystickAxes], axes[i].raw,
+                sizeof(axes[i].raw));
+    std::memcpy(&jPovs[i * HAL_kMaxJoystickPOVs], povs[i].povs,
+                sizeof(povs[i].povs));
+    jButtons[i * 4] = axes[i].count;
+    jButtons[(i * 4) + 1] = povs[i].count;
+    jButtons[(i * 4) + 2] = buttons[i].count;
+    jButtons[(i * 4) + 3] = buttons[i].buttons;
+  }
+}
+
+/*
+ * Class:     edu_wpi_first_hal_DriverStationJNI
+ * Method:    getJoystickButtons
+ * Signature: (BLjava/lang/Object;)I
+ */
+JNIEXPORT jint JNICALL
+Java_edu_wpi_first_hal_DriverStationJNI_getJoystickButtons
+  (JNIEnv* env, jclass, jbyte joystickNum, jobject count)
+{
+  HAL_JoystickButtons joystickButtons;
+  HAL_GetJoystickButtons(joystickNum, &joystickButtons);
+  jbyte* countPtr =
+      reinterpret_cast<jbyte*>(env->GetDirectBufferAddress(count));
+  *countPtr = joystickButtons.count;
+  return joystickButtons.buttons;
+}
+
+/*
+ * Class:     edu_wpi_first_hal_DriverStationJNI
+ * Method:    setJoystickOutputs
+ * Signature: (BISS)I
+ */
+JNIEXPORT jint JNICALL
+Java_edu_wpi_first_hal_DriverStationJNI_setJoystickOutputs
+  (JNIEnv*, jclass, jbyte port, jint outputs, jshort leftRumble,
+   jshort rightRumble)
+{
+  return HAL_SetJoystickOutputs(port, outputs, leftRumble, rightRumble);
+}
+
+/*
+ * Class:     edu_wpi_first_hal_DriverStationJNI
+ * Method:    getJoystickIsXbox
+ * Signature: (B)I
+ */
+JNIEXPORT jint JNICALL
+Java_edu_wpi_first_hal_DriverStationJNI_getJoystickIsXbox
+  (JNIEnv*, jclass, jbyte port)
+{
+  return HAL_GetJoystickIsXbox(port);
+}
+
+/*
+ * Class:     edu_wpi_first_hal_DriverStationJNI
+ * Method:    getJoystickType
+ * Signature: (B)I
+ */
+JNIEXPORT jint JNICALL
+Java_edu_wpi_first_hal_DriverStationJNI_getJoystickType
+  (JNIEnv*, jclass, jbyte port)
+{
+  return HAL_GetJoystickType(port);
+}
+
+/*
+ * Class:     edu_wpi_first_hal_DriverStationJNI
+ * Method:    getJoystickName
+ * Signature: (B)Ljava/lang/String;
+ */
+JNIEXPORT jstring JNICALL
+Java_edu_wpi_first_hal_DriverStationJNI_getJoystickName
+  (JNIEnv* env, jclass, jbyte port)
+{
+  char* joystickName = HAL_GetJoystickName(port);
+  jstring str = MakeJString(env, joystickName);
+  HAL_FreeJoystickName(joystickName);
+  return str;
+}
+
+/*
+ * Class:     edu_wpi_first_hal_DriverStationJNI
+ * Method:    getJoystickAxisType
+ * Signature: (BB)I
+ */
+JNIEXPORT jint JNICALL
+Java_edu_wpi_first_hal_DriverStationJNI_getJoystickAxisType
+  (JNIEnv*, jclass, jbyte joystickNum, jbyte axis)
+{
+  return HAL_GetJoystickAxisType(joystickNum, axis);
+}
+
+/*
+ * Class:     edu_wpi_first_hal_DriverStationJNI
+ * Method:    getMatchTime
+ * Signature: ()D
+ */
+JNIEXPORT jdouble JNICALL
+Java_edu_wpi_first_hal_DriverStationJNI_getMatchTime
+  (JNIEnv* env, jclass)
+{
+  int32_t status = 0;
+  return HAL_GetMatchTime(&status);
+}
+
+/*
+ * Class:     edu_wpi_first_hal_DriverStationJNI
+ * Method:    getMatchInfo
+ * Signature: (Ljava/lang/Object;)I
+ */
+JNIEXPORT jint JNICALL
+Java_edu_wpi_first_hal_DriverStationJNI_getMatchInfo
+  (JNIEnv* env, jclass, jobject info)
+{
+  HAL_MatchInfo matchInfo;
+  auto status = HAL_GetMatchInfo(&matchInfo);
+  if (status == 0) {
+    SetMatchInfoObject(env, info, matchInfo);
+  }
+  return status;
+}
+
+/*
+ * Class:     edu_wpi_first_hal_DriverStationJNI
+ * Method:    sendError
+ * Signature: (ZIZLjava/lang/String;Ljava/lang/String;Ljava/lang/String;Z)I
+ */
+JNIEXPORT jint JNICALL
+Java_edu_wpi_first_hal_DriverStationJNI_sendError
+  (JNIEnv* env, jclass, jboolean isError, jint errorCode, jboolean isLVCode,
+   jstring details, jstring location, jstring callStack, jboolean printMsg)
+{
+  JStringRef detailsStr{env, details};
+  JStringRef locationStr{env, location};
+  JStringRef callStackStr{env, callStack};
+
+  jint returnValue =
+      HAL_SendError(isError, errorCode, isLVCode, detailsStr.c_str(),
+                    locationStr.c_str(), callStackStr.c_str(), printMsg);
+  return returnValue;
+}
+
+/*
+ * Class:     edu_wpi_first_hal_DriverStationJNI
+ * Method:    sendConsoleLine
+ * Signature: (Ljava/lang/String;)I
+ */
+JNIEXPORT jint JNICALL
+Java_edu_wpi_first_hal_DriverStationJNI_sendConsoleLine
+  (JNIEnv* env, jclass, jstring line)
+{
+  JStringRef lineStr{env, line};
+
+  jint returnValue = HAL_SendConsoleLine(lineStr.c_str());
+  return returnValue;
+}
+
+/*
+ * Class:     edu_wpi_first_hal_DriverStationJNI
+ * Method:    refreshDSData
+ * Signature: ()V
+ */
+JNIEXPORT void JNICALL
+Java_edu_wpi_first_hal_DriverStationJNI_refreshDSData
+  (JNIEnv*, jclass)
+{
+  HAL_RefreshDSData();
+}
+
+/*
+ * Class:     edu_wpi_first_hal_DriverStationJNI
+ * Method:    provideNewDataEventHandle
+ * Signature: (I)V
+ */
+JNIEXPORT void JNICALL
+Java_edu_wpi_first_hal_DriverStationJNI_provideNewDataEventHandle
+  (JNIEnv*, jclass, jint handle)
+{
+  HAL_ProvideNewDataEventHandle(handle);
+}
+
+/*
+ * Class:     edu_wpi_first_hal_DriverStationJNI
+ * Method:    removeNewDataEventHandle
+ * Signature: (I)V
+ */
+JNIEXPORT void JNICALL
+Java_edu_wpi_first_hal_DriverStationJNI_removeNewDataEventHandle
+  (JNIEnv*, jclass, jint handle)
+{
+  HAL_RemoveNewDataEventHandle(handle);
+}
+
+/*
+ * Class:     edu_wpi_first_hal_DriverStationJNI
+ * Method:    getOutputsActive
+ * Signature: ()Z
+ */
+JNIEXPORT jboolean JNICALL
+Java_edu_wpi_first_hal_DriverStationJNI_getOutputsActive
+  (JNIEnv*, jclass)
+{
+  return HAL_GetOutputsEnabled();
+}
+}  // extern "C"

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

@@ -106,310 +106,6 @@ Java_edu_wpi_first_hal_HAL_simPeriodicAfterNative
   HAL_SimPeriodicAfter();
 }
 
-/*
- * Class:     edu_wpi_first_hal_HAL
- * Method:    observeUserProgramStarting
- * Signature: ()V
- */
-JNIEXPORT void JNICALL
-Java_edu_wpi_first_hal_HAL_observeUserProgramStarting
-  (JNIEnv*, jclass)
-{
-  HAL_ObserveUserProgramStarting();
-}
-
-/*
- * Class:     edu_wpi_first_hal_HAL
- * Method:    observeUserProgramDisabled
- * Signature: ()V
- */
-JNIEXPORT void JNICALL
-Java_edu_wpi_first_hal_HAL_observeUserProgramDisabled
-  (JNIEnv*, jclass)
-{
-  HAL_ObserveUserProgramDisabled();
-}
-
-/*
- * Class:     edu_wpi_first_hal_HAL
- * Method:    observeUserProgramAutonomous
- * Signature: ()V
- */
-JNIEXPORT void JNICALL
-Java_edu_wpi_first_hal_HAL_observeUserProgramAutonomous
-  (JNIEnv*, jclass)
-{
-  HAL_ObserveUserProgramAutonomous();
-}
-
-/*
- * Class:     edu_wpi_first_hal_HAL
- * Method:    observeUserProgramTeleop
- * Signature: ()V
- */
-JNIEXPORT void JNICALL
-Java_edu_wpi_first_hal_HAL_observeUserProgramTeleop
-  (JNIEnv*, jclass)
-{
-  HAL_ObserveUserProgramTeleop();
-}
-
-/*
- * Class:     edu_wpi_first_hal_HAL
- * Method:    observeUserProgramTest
- * Signature: ()V
- */
-JNIEXPORT void JNICALL
-Java_edu_wpi_first_hal_HAL_observeUserProgramTest
-  (JNIEnv*, jclass)
-{
-  HAL_ObserveUserProgramTest();
-}
-
-/*
- * Class:     edu_wpi_first_hal_HAL
- * Method:    report
- * Signature: (IIILjava/lang/String;)I
- */
-JNIEXPORT jint JNICALL
-Java_edu_wpi_first_hal_HAL_report
-  (JNIEnv* paramEnv, jclass, jint paramResource, jint paramInstanceNumber,
-   jint paramContext, jstring paramFeature)
-{
-  JStringRef featureStr{paramEnv, paramFeature};
-  jint returnValue = HAL_Report(paramResource, paramInstanceNumber,
-                                paramContext, featureStr.c_str());
-  return returnValue;
-}
-
-/*
- * Class:     edu_wpi_first_hal_HAL
- * Method:    nativeGetControlWord
- * Signature: ()I
- */
-JNIEXPORT jint JNICALL
-Java_edu_wpi_first_hal_HAL_nativeGetControlWord
-  (JNIEnv*, jclass)
-{
-  static_assert(sizeof(HAL_ControlWord) == sizeof(jint),
-                "Java int must match the size of control word");
-  HAL_ControlWord controlWord;
-  HAL_GetControlWord(&controlWord);
-  jint retVal = 0;
-  std::memcpy(&retVal, &controlWord, sizeof(HAL_ControlWord));
-  return retVal;
-}
-
-/*
- * Class:     edu_wpi_first_hal_HAL
- * Method:    nativeGetAllianceStation
- * Signature: ()I
- */
-JNIEXPORT jint JNICALL
-Java_edu_wpi_first_hal_HAL_nativeGetAllianceStation
-  (JNIEnv*, jclass)
-{
-  int32_t status = 0;
-  auto allianceStation = HAL_GetAllianceStation(&status);
-  return static_cast<jint>(allianceStation);
-}
-
-/*
- * Class:     edu_wpi_first_hal_HAL
- * Method:    getJoystickAxes
- * Signature: (B[F)S
- */
-JNIEXPORT jshort JNICALL
-Java_edu_wpi_first_hal_HAL_getJoystickAxes
-  (JNIEnv* env, jclass, jbyte joystickNum, jfloatArray axesArray)
-{
-  HAL_JoystickAxes axes;
-  HAL_GetJoystickAxes(joystickNum, &axes);
-
-  jsize javaSize = env->GetArrayLength(axesArray);
-  if (axes.count > javaSize) {
-    ThrowIllegalArgumentException(
-        env,
-        fmt::format("Native array size larger then passed in java array "
-                    "size\nNative Size: {} Java Size: {}",
-                    static_cast<int>(axes.count), static_cast<int>(javaSize)));
-    return 0;
-  }
-
-  env->SetFloatArrayRegion(axesArray, 0, axes.count, axes.axes);
-
-  return axes.count;
-}
-
-/*
- * Class:     edu_wpi_first_hal_HAL
- * Method:    getJoystickPOVs
- * Signature: (B[S)S
- */
-JNIEXPORT jshort JNICALL
-Java_edu_wpi_first_hal_HAL_getJoystickPOVs
-  (JNIEnv* env, jclass, jbyte joystickNum, jshortArray povsArray)
-{
-  HAL_JoystickPOVs povs;
-  HAL_GetJoystickPOVs(joystickNum, &povs);
-
-  jsize javaSize = env->GetArrayLength(povsArray);
-  if (povs.count > javaSize) {
-    ThrowIllegalArgumentException(
-        env,
-        fmt::format("Native array size larger then passed in java array "
-                    "size\nNative Size: {} Java Size: {}",
-                    static_cast<int>(povs.count), static_cast<int>(javaSize)));
-    return 0;
-  }
-
-  env->SetShortArrayRegion(povsArray, 0, povs.count, povs.povs);
-
-  return povs.count;
-}
-
-/*
- * Class:     edu_wpi_first_hal_HAL
- * Method:    getJoystickButtons
- * Signature: (BLjava/lang/Object;)I
- */
-JNIEXPORT jint JNICALL
-Java_edu_wpi_first_hal_HAL_getJoystickButtons
-  (JNIEnv* env, jclass, jbyte joystickNum, jobject count)
-{
-  HAL_JoystickButtons joystickButtons;
-  HAL_GetJoystickButtons(joystickNum, &joystickButtons);
-  jbyte* countPtr =
-      reinterpret_cast<jbyte*>(env->GetDirectBufferAddress(count));
-  *countPtr = joystickButtons.count;
-  return joystickButtons.buttons;
-}
-
-/*
- * Class:     edu_wpi_first_hal_HAL
- * Method:    setJoystickOutputs
- * Signature: (BISS)I
- */
-JNIEXPORT jint JNICALL
-Java_edu_wpi_first_hal_HAL_setJoystickOutputs
-  (JNIEnv*, jclass, jbyte port, jint outputs, jshort leftRumble,
-   jshort rightRumble)
-{
-  return HAL_SetJoystickOutputs(port, outputs, leftRumble, rightRumble);
-}
-
-/*
- * Class:     edu_wpi_first_hal_HAL
- * Method:    getJoystickIsXbox
- * Signature: (B)I
- */
-JNIEXPORT jint JNICALL
-Java_edu_wpi_first_hal_HAL_getJoystickIsXbox
-  (JNIEnv*, jclass, jbyte port)
-{
-  return HAL_GetJoystickIsXbox(port);
-}
-
-/*
- * Class:     edu_wpi_first_hal_HAL
- * Method:    getJoystickType
- * Signature: (B)I
- */
-JNIEXPORT jint JNICALL
-Java_edu_wpi_first_hal_HAL_getJoystickType
-  (JNIEnv*, jclass, jbyte port)
-{
-  return HAL_GetJoystickType(port);
-}
-
-/*
- * Class:     edu_wpi_first_hal_HAL
- * Method:    getJoystickName
- * Signature: (B)Ljava/lang/String;
- */
-JNIEXPORT jstring JNICALL
-Java_edu_wpi_first_hal_HAL_getJoystickName
-  (JNIEnv* env, jclass, jbyte port)
-{
-  char* joystickName = HAL_GetJoystickName(port);
-  jstring str = MakeJString(env, joystickName);
-  HAL_FreeJoystickName(joystickName);
-  return str;
-}
-
-/*
- * Class:     edu_wpi_first_hal_HAL
- * Method:    getJoystickAxisType
- * Signature: (BB)I
- */
-JNIEXPORT jint JNICALL
-Java_edu_wpi_first_hal_HAL_getJoystickAxisType
-  (JNIEnv*, jclass, jbyte joystickNum, jbyte axis)
-{
-  return HAL_GetJoystickAxisType(joystickNum, axis);
-}
-
-/*
- * Class:     edu_wpi_first_hal_HAL
- * Method:    isNewControlData
- * Signature: ()Z
- */
-JNIEXPORT jboolean JNICALL
-Java_edu_wpi_first_hal_HAL_isNewControlData
-  (JNIEnv*, jclass)
-{
-  return static_cast<jboolean>(HAL_IsNewControlData());
-}
-
-/*
- * Class:     edu_wpi_first_hal_HAL
- * Method:    waitForDSData
- * Signature: ()V
- */
-JNIEXPORT void JNICALL
-Java_edu_wpi_first_hal_HAL_waitForDSData
-  (JNIEnv* env, jclass)
-{
-  HAL_WaitForDSData();
-}
-
-/*
- * Class:     edu_wpi_first_hal_HAL
- * Method:    releaseDSMutex
- * Signature: ()V
- */
-JNIEXPORT void JNICALL
-Java_edu_wpi_first_hal_HAL_releaseDSMutex
-  (JNIEnv* env, jclass)
-{
-  HAL_ReleaseDSMutex();
-}
-
-/*
- * Class:     edu_wpi_first_hal_HAL
- * Method:    waitForDSDataTimeout
- * Signature: (D)Z
- */
-JNIEXPORT jboolean JNICALL
-Java_edu_wpi_first_hal_HAL_waitForDSDataTimeout
-  (JNIEnv*, jclass, jdouble timeout)
-{
-  return static_cast<jboolean>(HAL_WaitForDSDataTimeout(timeout));
-}
-
-/*
- * Class:     edu_wpi_first_hal_HAL
- * Method:    getMatchTime
- * Signature: ()D
- */
-JNIEXPORT jdouble JNICALL
-Java_edu_wpi_first_hal_HAL_getMatchTime
-  (JNIEnv* env, jclass)
-{
-  int32_t status = 0;
-  return HAL_GetMatchTime(&status);
-}
-
 /*
  * Class:     edu_wpi_first_hal_HAL
  * Method:    getSystemActive
@@ -440,58 +136,6 @@ Java_edu_wpi_first_hal_HAL_getBrownedOut
   return val;
 }
 
-/*
- * Class:     edu_wpi_first_hal_HAL
- * Method:    getMatchInfo
- * Signature: (Ljava/lang/Object;)I
- */
-JNIEXPORT jint JNICALL
-Java_edu_wpi_first_hal_HAL_getMatchInfo
-  (JNIEnv* env, jclass, jobject info)
-{
-  HAL_MatchInfo matchInfo;
-  auto status = HAL_GetMatchInfo(&matchInfo);
-  if (status == 0) {
-    SetMatchInfoObject(env, info, matchInfo);
-  }
-  return status;
-}
-
-/*
- * Class:     edu_wpi_first_hal_HAL
- * Method:    sendError
- * Signature: (ZIZLjava/lang/String;Ljava/lang/String;Ljava/lang/String;Z)I
- */
-JNIEXPORT jint JNICALL
-Java_edu_wpi_first_hal_HAL_sendError
-  (JNIEnv* env, jclass, jboolean isError, jint errorCode, jboolean isLVCode,
-   jstring details, jstring location, jstring callStack, jboolean printMsg)
-{
-  JStringRef detailsStr{env, details};
-  JStringRef locationStr{env, location};
-  JStringRef callStackStr{env, callStack};
-
-  jint returnValue =
-      HAL_SendError(isError, errorCode, isLVCode, detailsStr.c_str(),
-                    locationStr.c_str(), callStackStr.c_str(), printMsg);
-  return returnValue;
-}
-
-/*
- * Class:     edu_wpi_first_hal_HAL
- * Method:    sendConsoleLine
- * Signature: (Ljava/lang/String;)I
- */
-JNIEXPORT jint JNICALL
-Java_edu_wpi_first_hal_HAL_sendConsoleLine
-  (JNIEnv* env, jclass, jstring line)
-{
-  JStringRef lineStr{env, line};
-
-  jint returnValue = HAL_SendConsoleLine(lineStr.c_str());
-  return returnValue;
-}
-
 /*
  * Class:     edu_wpi_first_hal_HAL
  * Method:    getPortWithModule

hal/src/main/native/include/hal/DriverStation.h

@@ -6,6 +6,8 @@
 
 #include <stdint.h>
 
+#include <wpi/Synchronization.h>
+
 #include "hal/DriverStationTypes.h"
 #include "hal/Types.h"
 
@@ -87,6 +89,9 @@ int32_t HAL_GetJoystickPOVs(int32_t joystickNum, HAL_JoystickPOVs* povs);
 int32_t HAL_GetJoystickButtons(int32_t joystickNum,
                                HAL_JoystickButtons* buttons);
 
+void HAL_GetAllJoystickData(HAL_JoystickAxes* axes, HAL_JoystickPOVs* povs,
+                            HAL_JoystickButtons* buttons);
+
 /**
  * Retrieves the Joystick Descriptor for particular slot.
  *
@@ -183,6 +188,11 @@ int32_t HAL_SetJoystickOutputs(int32_t joystickNum, int64_t outputs,
  */
 double HAL_GetMatchTime(int32_t* status);
 
+/**
+ * Gets if outputs are enabled by the control system.
+ */
+HAL_Bool HAL_GetOutputsEnabled(void);
+
 /**
  * Gets info about a specific match.
  *
@@ -191,44 +201,10 @@ double HAL_GetMatchTime(int32_t* status);
  */
 int32_t HAL_GetMatchInfo(HAL_MatchInfo* info);
 
-/**
- * Releases the DS Mutex to allow proper shutdown of any threads that are
- * waiting on it.
- */
-void HAL_ReleaseDSMutex(void);
+void HAL_RefreshDSData(void);
 
-/**
- * Has a new control packet from the driver station arrived since the last
- * time this function was called?
- *
- * @return true if the control data has been updated since the last call
- */
-HAL_Bool HAL_IsNewControlData(void);
-
-/**
- * Waits for the newest DS packet to arrive. Note that this is a blocking call.
- * Checks if new control data has arrived since the last HAL_WaitForDSData or
- * HAL_IsNewControlData call. If new data has not arrived, waits for new data
- * to arrive. Otherwise, returns immediately.
- */
-void HAL_WaitForDSData(void);
-
-/**
- * Waits for the newest DS packet to arrive. If timeout is <= 0, this will wait
- * forever. Otherwise, it will wait until either a new packet, or the timeout
- * time has passed.
- *
- * @param[in] timeout timeout in seconds
- * @return true for new data, false for timeout
- */
-HAL_Bool HAL_WaitForDSDataTimeout(double timeout);
-
-/**
- * Initializes the driver station communication. This will properly
- * handle multiple calls. However note that this CANNOT be called from a library
- * that interfaces with LabVIEW.
- */
-void HAL_InitializeDriverStation(void);
+void HAL_ProvideNewDataEventHandle(WPI_EventHandle handle);
+void HAL_RemoveNewDataEventHandle(WPI_EventHandle handle);
 
 /**
  * Sets the program starting flag in the DS.

hal/src/main/native/include/hal/DriverStationTypes.h

@@ -69,6 +69,7 @@ HAL_ENUM(HAL_MatchType) {
 struct HAL_JoystickAxes {
   int16_t count;
   float axes[HAL_kMaxJoystickAxes];
+  uint8_t raw[HAL_kMaxJoystickAxes];
 };
 typedef struct HAL_JoystickAxes HAL_JoystickAxes;
 

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

@@ -14,32 +14,78 @@
 #include <string>
 
 #include <fmt/format.h>
+#include <wpi/EventVector.h>
 #include <wpi/condition_variable.h>
 #include <wpi/mutex.h>
 
 #include "HALInitializer.h"
+#include "hal/Errors.h"
 #include "hal/cpp/fpga_clock.h"
 #include "hal/simulation/MockHooks.h"
 #include "mockdata/DriverStationDataInternal.h"
 
 static wpi::mutex msgMutex;
-static wpi::condition_variable* newDSDataAvailableCond;
-static wpi::mutex newDSDataAvailableMutex;
-static int newDSDataAvailableCounter{0};
-static std::atomic_bool isFinalized{false};
 static std::atomic<HALSIM_SendErrorHandler> sendErrorHandler{nullptr};
 static std::atomic<HALSIM_SendConsoleLineHandler> sendConsoleLineHandler{
     nullptr};
 
+using namespace hal;
+
+static constexpr int kJoystickPorts = 6;
+
+namespace {
+struct JoystickDataCache {
+  JoystickDataCache() { std::memset(this, 0, sizeof(*this)); }
+  void Update();
+
+  HAL_JoystickAxes axes[kJoystickPorts];
+  HAL_JoystickPOVs povs[kJoystickPorts];
+  HAL_JoystickButtons buttons[kJoystickPorts];
+  HAL_AllianceStationID allianceStation;
+  double matchTime;
+  bool updated;
+};
+static_assert(std::is_standard_layout_v<JoystickDataCache>);
+// static_assert(std::is_trivial_v<JoystickDataCache>);
+
+static std::atomic_bool gShutdown{false};
+
+struct FRCDriverStation {
+  ~FRCDriverStation() { gShutdown = true; }
+  wpi::EventVector newDataEvents;
+  wpi::mutex cacheMutex;
+};
+}  // namespace
+
+void JoystickDataCache::Update() {
+  for (int i = 0; i < kJoystickPorts; i++) {
+    SimDriverStationData->GetJoystickAxes(i, &axes[i]);
+    SimDriverStationData->GetJoystickPOVs(i, &povs[i]);
+    SimDriverStationData->GetJoystickButtons(i, &buttons[i]);
+  }
+  allianceStation = SimDriverStationData->allianceStationId;
+  matchTime = SimDriverStationData->matchTime;
+}
+
+#define CHECK_JOYSTICK_NUMBER(stickNum)                  \
+  if ((stickNum) < 0 || (stickNum) >= HAL_kMaxJoysticks) \
+  return PARAMETER_OUT_OF_RANGE
+
+static HAL_ControlWord newestControlWord;
+static JoystickDataCache caches[3];
+static JoystickDataCache* currentRead = &caches[0];
+static JoystickDataCache* currentCache = &caches[1];
+static JoystickDataCache* cacheToUpdate = &caches[2];
+
+static ::FRCDriverStation* driverStation;
+
 namespace hal::init {
 void InitializeDriverStation() {
-  static wpi::condition_variable nddaC;
-  newDSDataAvailableCond = &nddaC;
+  static FRCDriverStation ds;
+  driverStation = &ds;
 }
 }  // namespace hal::init
 
-using namespace hal;
-
 extern "C" {
 
 void HALSIM_SetSendError(HALSIM_SendErrorHandler handler) {
@@ -122,39 +168,49 @@ int32_t HAL_SendConsoleLine(const char* line) {
 }
 
 int32_t HAL_GetControlWord(HAL_ControlWord* controlWord) {
-  std::memset(controlWord, 0, sizeof(HAL_ControlWord));
-  controlWord->enabled = SimDriverStationData->enabled;
-  controlWord->autonomous = SimDriverStationData->autonomous;
-  controlWord->test = SimDriverStationData->test;
-  controlWord->eStop = SimDriverStationData->eStop;
-  controlWord->fmsAttached = SimDriverStationData->fmsAttached;
-  controlWord->dsAttached = SimDriverStationData->dsAttached;
+  std::scoped_lock lock{driverStation->cacheMutex};
+  *controlWord = newestControlWord;
   return 0;
 }
 
 HAL_AllianceStationID HAL_GetAllianceStation(int32_t* status) {
-  *status = 0;
-  return SimDriverStationData->allianceStationId;
+  std::scoped_lock lock{driverStation->cacheMutex};
+  return currentRead->allianceStation;
 }
 
 int32_t HAL_GetJoystickAxes(int32_t joystickNum, HAL_JoystickAxes* axes) {
-  SimDriverStationData->GetJoystickAxes(joystickNum, axes);
+  CHECK_JOYSTICK_NUMBER(joystickNum);
+  std::scoped_lock lock{driverStation->cacheMutex};
+  *axes = currentRead->axes[joystickNum];
   return 0;
 }
 
 int32_t HAL_GetJoystickPOVs(int32_t joystickNum, HAL_JoystickPOVs* povs) {
-  SimDriverStationData->GetJoystickPOVs(joystickNum, povs);
+  CHECK_JOYSTICK_NUMBER(joystickNum);
+  std::scoped_lock lock{driverStation->cacheMutex};
+  *povs = currentRead->povs[joystickNum];
   return 0;
 }
 
 int32_t HAL_GetJoystickButtons(int32_t joystickNum,
                                HAL_JoystickButtons* buttons) {
-  SimDriverStationData->GetJoystickButtons(joystickNum, buttons);
+  CHECK_JOYSTICK_NUMBER(joystickNum);
+  std::scoped_lock lock{driverStation->cacheMutex};
+  *buttons = currentRead->buttons[joystickNum];
   return 0;
 }
 
+void HAL_GetAllJoystickData(HAL_JoystickAxes* axes, HAL_JoystickPOVs* povs,
+                            HAL_JoystickButtons* buttons) {
+  std::scoped_lock lock{driverStation->cacheMutex};
+  std::memcpy(axes, currentRead->axes, sizeof(currentRead->axes));
+  std::memcpy(povs, currentRead->povs, sizeof(currentRead->povs));
+  std::memcpy(buttons, currentRead->buttons, sizeof(currentRead->buttons));
+}
+
 int32_t HAL_GetJoystickDescriptor(int32_t joystickNum,
                                   HAL_JoystickDescriptor* desc) {
+  CHECK_JOYSTICK_NUMBER(joystickNum);
   SimDriverStationData->GetJoystickDescriptor(joystickNum, desc);
   return 0;
 }
@@ -196,7 +252,8 @@ int32_t HAL_SetJoystickOutputs(int32_t joystickNum, int64_t outputs,
 }
 
 double HAL_GetMatchTime(int32_t* status) {
-  return SimDriverStationData->matchTime;
+  std::scoped_lock lock{driverStation->cacheMutex};
+  return currentRead->matchTime;
 }
 
 int32_t HAL_GetMatchInfo(HAL_MatchInfo* info) {
@@ -224,103 +281,57 @@ void HAL_ObserveUserProgramTest(void) {
   // TODO
 }
 
-static int& GetThreadLocalLastCount() {
-  // There is a rollover error condition here. At Packet# = n * (uintmax), this
-  // will return false when instead it should return true. However, this at a
-  // 20ms rate occurs once every 2.7 years of DS connected runtime, so not
-  // worth the cycles to check.
-  thread_local int lastCount{0};
-  return lastCount;
+void HAL_RefreshDSData(void) {
+  HAL_ControlWord controlWord;
+  std::memset(&controlWord, 0, sizeof(controlWord));
+  controlWord.enabled = SimDriverStationData->enabled;
+  controlWord.autonomous = SimDriverStationData->autonomous;
+  controlWord.test = SimDriverStationData->test;
+  controlWord.eStop = SimDriverStationData->eStop;
+  controlWord.fmsAttached = SimDriverStationData->fmsAttached;
+  controlWord.dsAttached = SimDriverStationData->dsAttached;
+  std::scoped_lock lock{driverStation->cacheMutex};
+  if (currentCache->updated) {
+    std::swap(currentCache, currentRead);
+    currentCache->updated = false;
+  }
+  newestControlWord = controlWord;
 }
 
-HAL_Bool HAL_IsNewControlData(void) {
-  std::scoped_lock lock(newDSDataAvailableMutex);
-  int& lastCount = GetThreadLocalLastCount();
-  int currentCount = newDSDataAvailableCounter;
-  if (lastCount == currentCount) {
-    return false;
-  }
-  lastCount = currentCount;
-  return true;
-}
-
-void HAL_WaitForDSData(void) {
-  HAL_WaitForDSDataTimeout(0);
-}
-
-HAL_Bool HAL_WaitForDSDataTimeout(double timeout) {
-  std::unique_lock lock(newDSDataAvailableMutex);
-  int& lastCount = GetThreadLocalLastCount();
-  int currentCount = newDSDataAvailableCounter;
-  if (lastCount != currentCount) {
-    lastCount = currentCount;
-    return true;
-  }
-
-  if (isFinalized.load()) {
-    return false;
-  }
-  auto timeoutTime =
-      std::chrono::steady_clock::now() + std::chrono::duration<double>(timeout);
-
-  while (newDSDataAvailableCounter == currentCount) {
-    if (timeout > 0) {
-      auto timedOut = newDSDataAvailableCond->wait_until(lock, timeoutTime);
-      if (timedOut == std::cv_status::timeout) {
-        return false;
-      }
-    } else {
-      newDSDataAvailableCond->wait(lock);
-    }
-  }
-  lastCount = newDSDataAvailableCounter;
-  return true;
-}
-
-// Constant number to be used for our occur handle
-constexpr int32_t refNumber = 42;
-
-static int32_t newDataOccur(uint32_t refNum) {
-  // Since we could get other values, require our specific handle
-  // to signal our threads
-  if (refNum != refNumber) {
-    return 0;
-  }
-  SimDriverStationData->CallNewDataCallbacks();
-  std::scoped_lock lock(newDSDataAvailableMutex);
-  // Nofify all threads
-  newDSDataAvailableCounter++;
-  newDSDataAvailableCond->notify_all();
-  return 0;
-}
-
-void HAL_InitializeDriverStation(void) {
-  hal::init::CheckInit();
-  static std::atomic_bool initialized{false};
-  static wpi::mutex initializeMutex;
-  // Initial check, as if it's true initialization has finished
-  if (initialized) {
+void HAL_ProvideNewDataEventHandle(WPI_EventHandle handle) {
+  if (gShutdown) {
     return;
   }
-
-  std::scoped_lock lock(initializeMutex);
-  // Second check in case another thread was waiting
-  if (initialized) {
-    return;
-  }
-
-  SimDriverStationData->ResetData();
-
-  std::atexit([]() {
-    isFinalized.store(true);
-    HAL_ReleaseDSMutex();
-  });
-
-  initialized = true;
+  driverStation->newDataEvents.Add(handle);
 }
 
-void HAL_ReleaseDSMutex(void) {
-  newDataOccur(refNumber);
+void HAL_RemoveNewDataEventHandle(WPI_EventHandle handle) {
+  if (gShutdown) {
+    return;
+  }
+  driverStation->newDataEvents.Remove(handle);
+}
+
+HAL_Bool HAL_GetOutputsEnabled(void) {
+  std::scoped_lock lock{driverStation->cacheMutex};
+  return newestControlWord.enabled && newestControlWord.dsAttached;
 }
 
 }  // extern "C"
+
+namespace hal {
+void NewDriverStationData() {
+  cacheToUpdate->Update();
+  {
+    std::scoped_lock lock{driverStation->cacheMutex};
+    std::swap(currentCache, cacheToUpdate);
+    currentCache->updated = true;
+  }
+  driverStation->newDataEvents.Wakeup();
+  SimDriverStationData->CallNewDataCallbacks();
+}
+
+void InitializeDriverStation() {
+  SimDriverStationData->ResetData();
+}
+}  // namespace hal

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

@@ -57,6 +57,10 @@ static std::vector<std::pair<void*, void (*)(void*)>> gOnShutdown;
 static SimPeriodicCallbackRegistry gSimPeriodicBefore;
 static SimPeriodicCallbackRegistry gSimPeriodicAfter;
 
+namespace hal {
+void InitializeDriverStation();
+}  // namespace hal
+
 namespace hal::init {
 void InitializeHAL() {
   InitializeAccelerometerData();
@@ -335,7 +339,7 @@ HAL_Bool HAL_Initialize(int32_t timeout, int32_t mode) {
   hal::init::HAL_IsInitialized.store(true);
 
   hal::RestartTiming();
-  HAL_InitializeDriverStation();
+  hal::InitializeDriverStation();
 
   initialized = true;
 

hal/src/main/native/sim/mockdata/DriverStationData.cpp

@@ -216,8 +216,12 @@ void DriverStationData::CallNewDataCallbacks() {
   m_newDataCallbacks(&empty);
 }
 
+namespace hal {
+void NewDriverStationData();
+}  // namespace hal
+
 void DriverStationData::NotifyNewData() {
-  HAL_ReleaseDSMutex();
+  hal::NewDriverStationData();
 }
 
 void DriverStationData::SetJoystickButton(int32_t stick, int32_t button,

hal/src/test/native/cpp/mockdata/DriverStationDataTest.cpp

@@ -61,6 +61,9 @@ TEST(DriverStationTest, Joystick) {
   HALSIM_SetJoystickPOVs(joystickUnderTest, &set_povs);
   HALSIM_SetJoystickButtons(joystickUnderTest, &set_buttons);
 
+  HALSIM_NotifyDriverStationNewData();
+  HAL_RefreshDSData();
+
   // Check the set values
   HAL_GetJoystickAxes(joystickUnderTest, &axes);
   HAL_GetJoystickPOVs(joystickUnderTest, &povs);
@@ -89,6 +92,9 @@ TEST(DriverStationTest, Joystick) {
 
   // Reset
   HALSIM_ResetDriverStationData();
+  HALSIM_NotifyDriverStationNewData();
+  HAL_RefreshDSData();
+
   for (int joystickNum = 0; joystickNum < 6; ++joystickNum) {
     HAL_GetJoystickAxes(joystickNum, &axes);
     HAL_GetJoystickPOVs(joystickNum, &povs);

ntcore/src/dev/native/cpp/main.cpp

@@ -51,12 +51,10 @@ void bench() {
   // set up instances
   auto client = nt::CreateInstance();
   auto server = nt::CreateInstance();
-  nt::SetNetworkIdentity(server, "server");
-  nt::SetNetworkIdentity(client, "client");
 
   // connect client and server
   nt::StartServer(server, "bench.json", "127.0.0.1", 0, 10000);
-  nt::StartClient4(client);
+  nt::StartClient4(client, "client");
   nt::SetServer(client, "127.0.0.1", 10000);
 
   using namespace std::chrono_literals;

ntcore/src/generate/java/NetworkTableInstance.java.jinja

@@ -450,16 +450,6 @@ public final class NetworkTableInstance implements AutoCloseable {
    * Client/Server Functions
    */
 
-  /**
-   * Set the network identity of this node. This is the name used during the initial connection
-   * handshake, and is visible through ConnectionInfo on the remote node.
-   *
-   * @param name identity to advertise
-   */
-  public void setNetworkIdentity(String name) {
-    NetworkTablesJNI.setNetworkIdentity(m_handle, name);
-  }
-
   /**
    * Get the current network mode.
    *
@@ -541,14 +531,22 @@ public final class NetworkTableInstance implements AutoCloseable {
     NetworkTablesJNI.stopServer(m_handle);
   }
 
-  /** Starts a NT3 client. Use SetServer or SetServerTeam to set the server name and port. */
-  public void startClient3() {
-    NetworkTablesJNI.startClient3(m_handle);
+  /**
+   * Starts a NT3 client. Use SetServer or SetServerTeam to set the server name and port.
+   *
+   * @param identity network identity to advertise (cannot be empty string)
+   */
+  public void startClient3(String identity) {
+    NetworkTablesJNI.startClient3(m_handle, identity);
   }
 
-  /** Starts a NT4 client. Use SetServer or SetServerTeam to set the server name and port. */
-  public void startClient4() {
-    NetworkTablesJNI.startClient4(m_handle);
+  /**
+   * Starts a NT4 client. Use SetServer or SetServerTeam to set the server name and port.
+   *
+   * @param identity network identity to advertise (cannot be empty string)
+   */
+  public void startClient4(String identity) {
+    NetworkTablesJNI.startClient4(m_handle, identity);
   }
 
   /** Stops the client if it is running. */

ntcore/src/generate/java/NetworkTablesJNI.java.jinja

@@ -234,8 +234,6 @@ public final class NetworkTablesJNI {
 
   public static native void removeConnectionListener(int connListener);
 
-  public static native void setNetworkIdentity(int inst, String name);
-
   public static native int getNetworkMode(int inst);
 
   public static native void startLocal(int inst);
@@ -247,9 +245,9 @@ public final class NetworkTablesJNI {
 
   public static native void stopServer(int inst);
 
-  public static native void startClient3(int inst);
+  public static native void startClient3(int inst, String identity);
 
-  public static native void startClient4(int inst);
+  public static native void startClient4(int inst, String identity);
 
   public static native void stopClient(int inst);
 

ntcore/src/main/java/edu/wpi/first/networktables/ConnectionInfo.java

@@ -9,7 +9,7 @@ package edu.wpi.first.networktables;
 public final class ConnectionInfo {
   /**
    * The remote identifier (as set on the remote node by {@link
-   * NetworkTableInstance#setNetworkIdentity(String)}).
+   * NetworkTableInstance#startClient4(String)}).
    */
   public final String remote_id;
 

ntcore/src/main/java/edu/wpi/first/networktables/ConnectionListener.java

@@ -11,7 +11,11 @@ import java.util.concurrent.locks.Condition;
 import java.util.concurrent.locks.ReentrantLock;
 import java.util.function.Consumer;
 
-/** Connection listener. This calls back to a callback function when a connection change occurs. */
+/**
+ * Connection listener. This calls back to a callback function when a connection change occurs. The
+ * callback function is called asynchronously on a separate thread, so it's important to use
+ * synchronization or atomics when accessing any shared state from the callback function.
+ */
 public final class ConnectionListener implements AutoCloseable {
   /**
    * Create a listener for connection changes.

ntcore/src/main/java/edu/wpi/first/networktables/PubSubOption.java

@@ -64,7 +64,8 @@ public class PubSubOption {
 
   /**
    * Polling storage for subscription. Specifies the maximum number of updates NetworkTables should
-   * store between calls to the subscriber's poll() function. Defaults to 1.
+   * store between calls to the subscriber's poll() function. Defaults to 1 if sendAll is false, 20
+   * if sendAll is true.
    *
    * @param depth number of entries to save for polling.
    * @return option

ntcore/src/main/java/edu/wpi/first/networktables/TopicListener.java

@@ -13,7 +13,9 @@ import java.util.function.Consumer;
 
 /**
  * Topic change listener. This calls back to a callback function when a topic change matching the
- * specified mask occurs.
+ * specified mask occurs. The callback function is called asynchronously on a separate thread, so
+ * it's important to use synchronization or atomics when accessing any shared state from the
+ * callback function.
  */
 public final class TopicListener implements AutoCloseable {
   /**

ntcore/src/main/java/edu/wpi/first/networktables/ValueListener.java

@@ -13,7 +13,9 @@ import java.util.function.Consumer;
 
 /**
  * Value change listener. This calls back to a callback function when a value change matching the
- * specified mask occurs.
+ * specified mask occurs. The callback function is called asynchronously on a separate thread, so
+ * it's important to use synchronization or atomics when accessing any shared state from the
+ * callback function.
  */
 public final class ValueListener implements AutoCloseable {
   /**

ntcore/src/main/native/cpp/InstanceImpl.cpp

@@ -120,23 +120,29 @@ void InstanceImpl::StopServer() {
   networkMode = NT_NET_MODE_NONE;
 }
 
-void InstanceImpl::StartClient3() {
+void InstanceImpl::StartClient3(std::string_view identity) {
   std::scoped_lock lock{m_mutex};
   if (networkMode != NT_NET_MODE_NONE) {
     return;
   }
   m_networkClient = std::make_shared<NetworkClient3>(
-      m_inst, m_identity, localStorage, connectionList, logger);
+      m_inst, identity, localStorage, connectionList, logger);
+  if (!m_servers.empty()) {
+    m_networkClient->SetServers(m_servers);
+  }
   networkMode = NT_NET_MODE_CLIENT3;
 }
 
-void InstanceImpl::StartClient4() {
+void InstanceImpl::StartClient4(std::string_view identity) {
   std::scoped_lock lock{m_mutex};
   if (networkMode != NT_NET_MODE_NONE) {
     return;
   }
   m_networkClient = std::make_shared<NetworkClient>(
-      m_inst, m_identity, localStorage, connectionList, logger);
+      m_inst, identity, localStorage, connectionList, logger);
+  if (!m_servers.empty()) {
+    m_networkClient->SetServers(m_servers);
+  }
   networkMode = NT_NET_MODE_CLIENT4;
 }
 
@@ -149,9 +155,13 @@ void InstanceImpl::StopClient() {
   networkMode = NT_NET_MODE_NONE;
 }
 
-void InstanceImpl::SetIdentity(std::string_view identity) {
+void InstanceImpl::SetServers(
+    std::span<const std::pair<std::string, unsigned int>> servers) {
   std::scoped_lock lock{m_mutex};
-  m_identity = identity;
+  m_servers = {servers.begin(), servers.end()};
+  if (m_networkClient) {
+    m_networkClient->SetServers(servers);
+  }
 }
 
 std::shared_ptr<NetworkServer> InstanceImpl::GetServer() {

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

@@ -8,6 +8,7 @@
 #include <memory>
 #include <string>
 #include <string_view>
+#include <utility>
 #include <vector>
 
 #include <wpi/mutex.h>
@@ -45,10 +46,11 @@ class InstanceImpl {
                    std::string_view listenAddress, unsigned int port3,
                    unsigned int port4);
   void StopServer();
-  void StartClient3();
-  void StartClient4();
+  void StartClient3(std::string_view identity);
+  void StartClient4(std::string_view identity);
   void StopClient();
-  void SetIdentity(std::string_view identity);
+  void SetServers(
+      std::span<const std::pair<std::string, unsigned int>> servers);
 
   std::shared_ptr<NetworkServer> GetServer();
   std::shared_ptr<INetworkClient> GetClient();
@@ -68,9 +70,9 @@ class InstanceImpl {
   static wpi::mutex s_mutex;
 
   wpi::mutex m_mutex;
-  std::string m_identity;
   std::shared_ptr<NetworkServer> m_networkServer;
   std::shared_ptr<INetworkClient> m_networkClient;
+  std::vector<std::pair<std::string, unsigned int>> m_servers;
   int m_inst;
 };
 

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

@@ -77,6 +77,7 @@ struct TopicData {
   std::string name;
 
   Value lastValue;  // also stores timestamp
+  bool lastValueNetwork{false};
   NT_Type type{NT_UNASSIGNED};
   std::string typeStr;
   unsigned int flags{0};            // for NT3 APIs
@@ -646,6 +647,7 @@ void LSImpl::CheckReset(TopicData* topic) {
     return;
   }
   topic->lastValue = {};
+  topic->lastValueNetwork = false;
   topic->type = NT_UNASSIGNED;
   topic->typeStr.clear();
   topic->flags = 0;
@@ -658,10 +660,13 @@ bool LSImpl::SetValue(TopicData* topic, const Value& value,
   if (topic->type != NT_UNASSIGNED && topic->type != value.type()) {
     return false;
   }
-  if (!topic->lastValue || value.time() >= topic->lastValue.time()) {
+  bool isNetwork = (eventFlags & NT_VALUE_NOTIFY_LOCAL) == 0;
+  if (!topic->lastValue || topic->lastValueNetwork == isNetwork ||
+      value.time() >= topic->lastValue.time()) {
     // TODO: notify option even if older value
     topic->type = value.type();
     topic->lastValue = value;
+    topic->lastValueNetwork = isNetwork;
     NotifyValue(topic, eventFlags);
   }
   if (topic->datalogType == value.type()) {
@@ -999,6 +1004,7 @@ std::unique_ptr<PublisherData> LSImpl::RemoveLocalPublisher(
 
 SubscriberData* LSImpl::AddLocalSubscriber(TopicData* topic,
                                            const PubSubConfig& config) {
+  DEBUG4("AddLocalSubscriber({})", topic->name);
   auto subscriber = m_subscribers.Add(m_inst, topic, config);
   topic->localSubscribers.Add(subscriber);
   // set subscriber to active if the type matches
@@ -1011,6 +1017,7 @@ SubscriberData* LSImpl::AddLocalSubscriber(TopicData* topic,
         topic->name, config.typeStr, topic->typeStr);
   }
   if (m_network) {
+    DEBUG4("-> NetworkSubscribe({})", topic->name);
     m_network->Subscribe(subscriber->handle, {{topic->name}}, config);
   }
   return subscriber;
@@ -1578,7 +1585,8 @@ void LocalStorage::NetworkSetValue(NT_Topic topicHandle, const Value& value) {
   }
 }
 
-void LocalStorage::StartNetwork(net::NetworkStartupInterface& startup) {
+void LocalStorage::StartNetwork(net::NetworkStartupInterface& startup,
+                                net::NetworkInterface* network) {
   std::scoped_lock lock{m_mutex};
   // publish all active publishers to the network and send last values
   // only send value once per topic
@@ -1596,19 +1604,13 @@ void LocalStorage::StartNetwork(net::NetworkStartupInterface& startup) {
     }
   }
   for (auto&& subscriber : m_impl->m_subscribers) {
-    if (subscriber->active) {
-      startup.Subscribe(subscriber->handle, {{subscriber->topic->name}},
-                        subscriber->config);
-    }
+    startup.Subscribe(subscriber->handle, {{subscriber->topic->name}},
+                      subscriber->config);
   }
   for (auto&& subscriber : m_impl->m_multiSubscribers) {
     startup.Subscribe(subscriber->handle, subscriber->prefixes,
                       subscriber->options);
   }
-}
-
-void LocalStorage::SetNetwork(net::NetworkInterface* network) {
-  std::scoped_lock lock{m_mutex};
   m_impl->m_network = network;
 }
 
@@ -1914,6 +1916,7 @@ void LocalStorage::Unpublish(NT_Handle pubentryHandle) {
   } else if (auto entry = m_impl->m_entries.Get(pubentryHandle)) {
     if (entry->publisher) {
       m_impl->RemoveLocalPublisher(entry->publisher->handle);
+      entry->publisher = nullptr;
     }
   } else {
     // TODO: report warning

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

@@ -41,8 +41,8 @@ class LocalStorage final : public net::ILocalStorage {
                                bool ack) final;
   void NetworkSetValue(NT_Topic topicHandle, const Value& value) final;
 
-  void StartNetwork(net::NetworkStartupInterface& startup) final;
-  void SetNetwork(net::NetworkInterface* network) final;
+  void StartNetwork(net::NetworkStartupInterface& startup,
+                    net::NetworkInterface* network) final;
   void ClearNetwork() final;
 
   // User functions.  These are the actual implementations of the corresponding

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

@@ -6,15 +6,23 @@
 
 #include <wpi/Logger.h>
 
-#define LOG(level, format, ...) WPI_LOG(m_logger, level, format, __VA_ARGS__)
+#define LOG(level, format, ...) \
+  WPI_LOG(m_logger, level, format __VA_OPT__(, ) __VA_ARGS__)
 
 #undef ERROR
-#define ERROR(format, ...) WPI_ERROR(m_logger, format, __VA_ARGS__)
-#define WARNING(format, ...) WPI_WARNING(m_logger, format, __VA_ARGS__)
-#define INFO(format, ...) WPI_INFO(m_logger, format, __VA_ARGS__)
+#define ERROR(format, ...) \
+  WPI_ERROR(m_logger, format __VA_OPT__(, ) __VA_ARGS__)
+#define WARNING(format, ...) \
+  WPI_WARNING(m_logger, format __VA_OPT__(, ) __VA_ARGS__)
+#define INFO(format, ...) WPI_INFO(m_logger, format __VA_OPT__(, ) __VA_ARGS__)
 
-#define DEBUG0(format, ...) WPI_DEBUG(m_logger, format, __VA_ARGS__)
-#define DEBUG1(format, ...) WPI_DEBUG1(m_logger, format, __VA_ARGS__)
-#define DEBUG2(format, ...) WPI_DEBUG2(m_logger, format, __VA_ARGS__)
-#define DEBUG3(format, ...) WPI_DEBUG3(m_logger, format, __VA_ARGS__)
-#define DEBUG4(format, ...) WPI_DEBUG4(m_logger, format, __VA_ARGS__)
+#define DEBUG0(format, ...) \
+  WPI_DEBUG(m_logger, format __VA_OPT__(, ) __VA_ARGS__)
+#define DEBUG1(format, ...) \
+  WPI_DEBUG1(m_logger, format __VA_OPT__(, ) __VA_ARGS__)
+#define DEBUG2(format, ...) \
+  WPI_DEBUG2(m_logger, format __VA_OPT__(, ) __VA_ARGS__)
+#define DEBUG3(format, ...) \
+  WPI_DEBUG3(m_logger, format __VA_OPT__(, ) __VA_ARGS__)
+#define DEBUG4(format, ...) \
+  WPI_DEBUG4(m_logger, format __VA_OPT__(, ) __VA_ARGS__)

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

@@ -11,9 +11,11 @@
 #include <vector>
 
 #include <fmt/format.h>
+#include <wpi/SmallString.h>
 #include <wpi/StringExtras.h>
 #include <wpinet/DsClient.h>
 #include <wpinet/EventLoopRunner.h>
+#include <wpinet/HttpUtil.h>
 #include <wpinet/ParallelTcpConnector.h>
 #include <wpinet/WebSocket.h>
 #include <wpinet/uv/Async.h>
@@ -114,7 +116,7 @@ class NCImpl4 : public NCImpl {
   void WsConnected(wpi::WebSocket& ws, uv::Tcp& tcp);
   void Disconnect(std::string_view reason) override;
 
-  std::unique_ptr<net::WebSocketConnection> m_wire;
+  std::shared_ptr<net::WebSocketConnection> m_wire;
   std::unique_ptr<net::ClientImpl> m_clientImpl;
 };
 
@@ -131,7 +133,7 @@ NCImpl::NCImpl(int inst, std::string_view id, net::ILocalStorage& localStorage,
       m_loop{*m_loopRunner.GetLoop()} {
   m_localMsgs.reserve(net::NetworkLoopQueue::kInitialQueueSize);
 
-  INFO("{}", "starting network client");
+  INFO("starting network client");
 }
 
 void NCImpl::SetServers(
@@ -191,7 +193,8 @@ void NCImpl::Disconnect(std::string_view reason) {
   m_connHandle = 0;
 
   // start trying to connect again
-  m_parallelConnect->Disconnected();
+  uv::Timer::SingleShot(m_loop, kReconnectRate,
+                        [this] { m_parallelConnect->Disconnected(); });
 }
 
 NCImpl3::NCImpl3(int inst, std::string_view id,
@@ -214,8 +217,10 @@ NCImpl3::NCImpl3(int inst, std::string_view id,
     // set up flush async
     m_flush = uv::Async<>::Create(m_loop);
     m_flush->wakeup.connect([this] {
-      HandleLocal();
-      m_clientImpl->SendPeriodic(m_loop.Now().count());
+      if (m_clientImpl) {
+        HandleLocal();
+        m_clientImpl->SendPeriodic(m_loop.Now().count());
+      }
     });
     m_flushAtomic = m_flush.get();
 
@@ -285,13 +290,13 @@ void NCImpl3::TcpConnected(uv::Tcp& tcp) {
           }
         });
         tcp.end.connect([this, &tcp] {
-          DEBUG3("{}", "NT3 TCP read ended");
+          DEBUG3("NT3 TCP read ended");
           if (!tcp.IsLoopClosing()) {
             Disconnect("remote end closed connection");
           }
         });
         tcp.closed.connect([this, &tcp] {
-          DEBUG3("{}", "NT3 TCP connection closed");
+          DEBUG3("NT3 TCP connection closed");
           if (!tcp.IsLoopClosing()) {
             Disconnect(m_wire->GetDisconnectReason());
           }
@@ -299,9 +304,8 @@ void NCImpl3::TcpConnected(uv::Tcp& tcp) {
 
         {
           net3::ClientStartup3 startup{*m_clientImpl};
-          m_localStorage.StartNetwork(startup);
+          m_localStorage.StartNetwork(startup, &m_localQueue);
         }
-        m_localStorage.SetNetwork(&m_localQueue);
         m_clientImpl->SetLocal(&m_localStorage);
       });
 
@@ -373,7 +377,9 @@ NCImpl4::~NCImpl4() {
 
 void NCImpl4::HandleLocal() {
   m_localQueue.ReadQueue(&m_localMsgs);
-  m_clientImpl->HandleLocal(std::move(m_localMsgs));
+  if (m_clientImpl) {
+    m_clientImpl->HandleLocal(std::move(m_localMsgs));
+  }
 }
 
 void NCImpl4::TcpConnected(uv::Tcp& tcp) {
@@ -387,9 +393,10 @@ void NCImpl4::TcpConnected(uv::Tcp& tcp) {
   }
   wpi::WebSocket::ClientOptions options;
   options.handshakeTimeout = kWebsocketHandshakeTimeout;
-  auto ws =
-      wpi::WebSocket::CreateClient(tcp, fmt::format("/nt/{}", m_id), "",
-                                   {{"networktables.first.wpi.edu"}}, options);
+  wpi::SmallString<128> idBuf;
+  auto ws = wpi::WebSocket::CreateClient(
+      tcp, fmt::format("/nt/{}", wpi::EscapeURI(m_id, idBuf)), "",
+      {{"networktables.first.wpi.edu"}}, options);
   ws->SetMaxMessageSize(kMaxMessageSize);
   ws->open.connect([this, &tcp, ws = ws.get()](std::string_view) {
     if (m_connList.IsConnected()) {
@@ -410,7 +417,7 @@ void NCImpl4::WsConnected(wpi::WebSocket& ws, uv::Tcp& tcp) {
   INFO("CONNECTED NT4 to {} port {}", connInfo.remote_ip, connInfo.remote_port);
   m_connHandle = m_connList.AddConnection(connInfo);
 
-  m_wire = std::make_unique<net::WebSocketConnection>(ws);
+  m_wire = std::make_shared<net::WebSocketConnection>(ws);
   m_clientImpl = std::make_unique<net::ClientImpl>(
       m_loop.Now().count(), m_inst, *m_wire, m_logger,
       [this](uint32_t repeatMs) {
@@ -420,9 +427,8 @@ void NCImpl4::WsConnected(wpi::WebSocket& ws, uv::Tcp& tcp) {
       });
   {
     net::ClientStartup startup{*m_clientImpl};
-    m_localStorage.StartNetwork(startup);
+    m_localStorage.StartNetwork(startup, &m_localQueue);
   }
-  m_localStorage.SetNetwork(&m_localQueue);
   m_clientImpl->SetLocal(&m_localStorage);
   ws.closed.connect([this, &ws](uint16_t, std::string_view reason) {
     if (!ws.GetStream().IsLoopClosing()) {
@@ -430,10 +436,14 @@ void NCImpl4::WsConnected(wpi::WebSocket& ws, uv::Tcp& tcp) {
     }
   });
   ws.text.connect([this](std::string_view data, bool) {
-    m_clientImpl->ProcessIncomingText(data);
+    if (m_clientImpl) {
+      m_clientImpl->ProcessIncomingText(data);
+    }
   });
   ws.binary.connect([this](std::span<const uint8_t> data, bool) {
-    m_clientImpl->ProcessIncomingBinary(data);
+    if (m_clientImpl) {
+      m_clientImpl->ProcessIncomingBinary(data);
+    }
   });
 }
 
@@ -482,7 +492,9 @@ void NetworkClient::FlushLocal() {
 void NetworkClient::Flush() {
   m_impl->m_loopRunner.ExecAsync([this](uv::Loop&) {
     m_impl->HandleLocal();
-    m_impl->m_clientImpl->SendValues(m_impl->m_loop.Now().count());
+    if (m_impl->m_clientImpl) {
+      m_impl->m_clientImpl->SendValues(m_impl->m_loop.Now().count());
+    }
   });
 }
 

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

@@ -11,12 +11,14 @@
 #include <system_error>
 #include <vector>
 
+#include <wpi/SmallString.h>
 #include <wpi/StringExtras.h>
 #include <wpi/fs.h>
 #include <wpi/mutex.h>
 #include <wpi/raw_istream.h>
 #include <wpi/raw_ostream.h>
 #include <wpinet/EventLoopRunner.h>
+#include <wpinet/HttpUtil.h>
 #include <wpinet/HttpWebSocketServerConnection.h>
 #include <wpinet/UrlParser.h>
 #include <wpinet/uv/Async.h>
@@ -87,7 +89,7 @@ class ServerConnection4 final
   void ProcessRequest() final;
   void ProcessWsUpgrade() final;
 
-  std::unique_ptr<net::WebSocketConnection> m_wire;
+  std::shared_ptr<net::WebSocketConnection> m_wire;
 };
 
 class ServerConnection3 : public ServerConnection {
@@ -97,7 +99,7 @@ class ServerConnection3 : public ServerConnection {
                     wpi::Logger& logger);
 
  private:
-  std::unique_ptr<net3::UvStreamConnection3> m_wire;
+  std::shared_ptr<net3::UvStreamConnection3> m_wire;
 };
 
 class NSImpl {
@@ -223,20 +225,24 @@ void ServerConnection4::ProcessWsUpgrade() {
   }
   DEBUG4("path: '{}'", path);
 
+  wpi::SmallString<128> nameBuf;
   std::string_view name;
+  bool err = false;
   if (wpi::starts_with(path, "/nt/")) {
-    name = wpi::drop_front(path, 4);
+    name = wpi::UnescapeURI(wpi::drop_front(path, 4), nameBuf, &err);
   }
-  if (name.empty()) {
-    INFO("invalid path '{}' (from {}), closing", path, m_connInfo);
-    m_websocket->Fail(404, fmt::format("invalid path '{}'", path));
+  if (err || name.empty()) {
+    INFO("invalid path '{}' (from {}), must match /nt/[clientId], closing",
+         path, m_connInfo);
+    m_websocket->Fail(
+        404, fmt::format("invalid path '{}', must match /nt/[clientId]", path));
     return;
   }
 
   m_websocket->SetMaxMessageSize(kMaxMessageSize);
 
   m_websocket->open.connect([this, name = std::string{name}](std::string_view) {
-    m_wire = std::make_unique<net::WebSocketConnection>(*m_websocket);
+    m_wire = std::make_shared<net::WebSocketConnection>(*m_websocket);
     // TODO: set local flag appropriately
     m_clientId = m_server.m_serverImpl.AddClient(
         name, m_connInfo, false, *m_wire,
@@ -247,11 +253,12 @@ void ServerConnection4::ProcessWsUpgrade() {
       m_websocket->Fail(409, fmt::format("duplicate name '{}'", name));
       return;
     }
+    INFO("CONNECTED NT4 client '{}' (from {})", name, m_connInfo);
     m_info.remote_id = name;
     m_server.AddConnection(this, m_info);
     m_websocket->closed.connect([this](uint16_t, std::string_view reason) {
-      INFO("NT4 connection '{}' closed (from {})", m_info.remote_id,
-           m_connInfo);
+      INFO("DISCONNECTED NT4 client '{}' (from {}): {}", m_info.remote_id,
+           m_connInfo, reason);
       ConnectionClosed();
     });
     m_websocket->text.connect([this](std::string_view data, bool) {
@@ -269,7 +276,7 @@ ServerConnection3::ServerConnection3(std::shared_ptr<uv::Stream> stream,
                                      NSImpl& server, std::string_view addr,
                                      unsigned int port, wpi::Logger& logger)
     : ServerConnection{server, addr, port, logger},
-      m_wire{std::make_unique<net3::UvStreamConnection3>(*stream)} {
+      m_wire{std::make_shared<net3::UvStreamConnection3>(*stream)} {
   m_info.remote_ip = addr;
   m_info.remote_port = port;
 
@@ -280,6 +287,7 @@ ServerConnection3::ServerConnection3(std::shared_ptr<uv::Stream> stream,
         m_info.remote_id = name;
         m_info.protocol_version = proto;
         m_server.AddConnection(this, m_info);
+        INFO("CONNECTED NT4 client '{}' (from {})", name, m_connInfo);
       },
       [this](uint32_t repeatMs) { UpdatePeriodicTimer(repeatMs); });
 
@@ -298,7 +306,7 @@ ServerConnection3::ServerConnection3(std::shared_ptr<uv::Stream> stream,
     m_wire->GetStream().Shutdown([this] { m_wire->GetStream().Close(); });
   });
   stream->closed.connect([this] {
-    INFO("NT3 connection '{}' closed (from {}): {}", m_info.remote_id,
+    INFO("DISCONNECTED NT3 client '{}' (from {}): {}", m_info.remote_id,
          m_connInfo, m_wire->GetDisconnectReason());
     ConnectionClosed();
   });
@@ -332,9 +340,8 @@ NSImpl::NSImpl(std::string_view persistentFilename,
     // connect local storage to server
     {
       net::ServerStartup startup{m_serverImpl};
-      m_localStorage.StartNetwork(startup);
+      m_localStorage.StartNetwork(startup, &m_localQueue);
     }
-    m_localStorage.SetNetwork(&m_localQueue);
     m_serverImpl.SetLocal(&m_localStorage);
 
     // load persistent file first, then initialize
@@ -360,7 +367,7 @@ void NSImpl::LoadPersistent() {
   is.readinto(m_persistentData, size);
   DEBUG4("read data: {}", m_persistentData);
   if (is.has_error()) {
-    WARNING("{}", "error reading persistent file");
+    WARNING("error reading persistent file");
     return;
   }
 }
@@ -453,7 +460,7 @@ void NSImpl::Init() {
       if (uv::AddrToName(tcp->GetPeer(), &peerAddr, &peerPort) == 0) {
         INFO("Got a NT3 connection from {} port {}", peerAddr, peerPort);
       } else {
-        INFO("{}", "Got a NT3 connection from unknown");
+        INFO("Got a NT3 connection from unknown");
       }
       auto conn = std::make_shared<ServerConnection3>(tcp, *this, peerAddr,
                                                       peerPort, m_logger);
@@ -485,7 +492,7 @@ void NSImpl::Init() {
       if (uv::AddrToName(tcp->GetPeer(), &peerAddr, &peerPort) == 0) {
         INFO("Got a NT4 connection from {} port {}", peerAddr, peerPort);
       } else {
-        INFO("{}", "Got a NT4 connection from unknown");
+        INFO("Got a NT4 connection from unknown");
       }
       auto conn = std::make_shared<ServerConnection4>(tcp, *this, peerAddr,
                                                       peerPort, m_logger);
@@ -496,7 +503,7 @@ void NSImpl::Init() {
   }
 
   if (m_initDone) {
-    DEBUG4("{}", "NetworkServer initDone()");
+    DEBUG4("NetworkServer initDone()");
     m_initDone();
     m_initDone = nullptr;
   }

ntcore/src/main/native/cpp/jni/NetworkTablesJNI.cpp

@@ -1225,22 +1225,6 @@ Java_edu_wpi_first_networktables_NetworkTablesJNI_removeConnectionListener
   nt::RemoveConnectionListener(connListenerUid);
 }
 
-/*
- * Class:     edu_wpi_first_networktables_NetworkTablesJNI
- * Method:    setNetworkIdentity
- * Signature: (ILjava/lang/String;)V
- */
-JNIEXPORT void JNICALL
-Java_edu_wpi_first_networktables_NetworkTablesJNI_setNetworkIdentity
-  (JNIEnv* env, jclass, jint inst, jstring name)
-{
-  if (!name) {
-    nullPointerEx.Throw(env, "name cannot be null");
-    return;
-  }
-  nt::SetNetworkIdentity(inst, JStringRef{env, name}.str());
-}
-
 /*
  * Class:     edu_wpi_first_networktables_NetworkTablesJNI
  * Method:    getNetworkMode
@@ -1314,25 +1298,33 @@ Java_edu_wpi_first_networktables_NetworkTablesJNI_stopServer
 /*
  * Class:     edu_wpi_first_networktables_NetworkTablesJNI
  * Method:    startClient3
- * Signature: (I)V
+ * Signature: (ILjava/lang/String;)V
  */
 JNIEXPORT void JNICALL
 Java_edu_wpi_first_networktables_NetworkTablesJNI_startClient3
-  (JNIEnv*, jclass, jint inst)
+  (JNIEnv* env, jclass, jint inst, jstring identity)
 {
-  nt::StartClient3(inst);
+  if (!identity) {
+    nullPointerEx.Throw(env, "identity cannot be null");
+    return;
+  }
+  nt::StartClient3(inst, JStringRef{env, identity}.str());
 }
 
 /*
  * Class:     edu_wpi_first_networktables_NetworkTablesJNI
  * Method:    startClient4
- * Signature: (I)V
+ * Signature: (ILjava/lang/String;)V
  */
 JNIEXPORT void JNICALL
 Java_edu_wpi_first_networktables_NetworkTablesJNI_startClient4
-  (JNIEnv*, jclass, jint inst)
+  (JNIEnv* env, jclass, jint inst, jstring identity)
 {
-  nt::StartClient4(inst);
+  if (!identity) {
+    nullPointerEx.Throw(env, "identity cannot be null");
+    return;
+  }
+  nt::StartClient4(inst, JStringRef{env, identity}.str());
 }
 
 /*

ntcore/src/main/native/cpp/net/ClientImpl.cpp

@@ -54,7 +54,6 @@ class CImpl : public ServerMessageHandler {
 
   void ProcessIncomingBinary(std::span<const uint8_t> data);
   void HandleLocal(std::vector<ClientMessage>&& msgs);
-  void SendOutgoing(std::span<const ClientMessage> msgs);
   bool SendControl(uint64_t curTimeMs);
   void SendValues(uint64_t curTimeMs);
   bool CheckNetworkReady();
@@ -171,7 +170,7 @@ void CImpl::ProcessIncomingBinary(std::span<const uint8_t> data) {
 }
 
 void CImpl::HandleLocal(std::vector<ClientMessage>&& msgs) {
-  DEBUG4("{}", "HandleLocal()");
+  DEBUG4("HandleLocal()");
   for (auto&& elem : msgs) {
     // common case is value
     if (auto msg = std::get_if<ClientValueMsg>(&elem.contents)) {

ntcore/src/main/native/cpp/net/NetworkInterface.h

@@ -59,8 +59,8 @@ class NetworkInterface : public NetworkStartupInterface {
 
 class ILocalStorage : public LocalInterface {
  public:
-  virtual void StartNetwork(NetworkStartupInterface& startup) = 0;
-  virtual void SetNetwork(NetworkInterface* network) = 0;
+  virtual void StartNetwork(NetworkStartupInterface& startup,
+                            NetworkInterface* network) = 0;
   virtual void ClearNetwork() = 0;
 };
 

ntcore/src/main/native/cpp/net/NetworkLoopQueue.cpp

@@ -45,7 +45,7 @@ void NetworkLoopQueue::SetValue(NT_Publisher pubHandle, const Value& value) {
   m_size += sizeof(ClientMessage);
   if (m_size > kMaxSize) {
     if (!m_sizeErrored) {
-      WPI_ERROR(m_logger, "{}", "NT: dropping value set due to memory limits");
+      WPI_ERROR(m_logger, "NT: dropping value set due to memory limits");
       m_sizeErrored = true;
     }
     return;  // avoid potential out of memory

ntcore/src/main/native/cpp/net/ServerImpl.cpp

@@ -325,6 +325,7 @@ struct TopicData {
   std::string name;
   unsigned int id;
   Value lastValue;
+  ClientData* lastValueClient = nullptr;
   std::string typeStr;
   wpi::json properties = wpi::json::object();
   bool persistent{false};
@@ -627,6 +628,15 @@ void ClientData4Base::ClientSubscribe(int64_t subuid,
       removed = topic->subscribers.Remove(sub.get());
     }
 
+    // is client already subscribed?
+    bool wasSubscribed = false;
+    for (auto subscriber : topic->subscribers) {
+      if (subscriber->client == this) {
+        wasSubscribed = true;
+        break;
+      }
+    }
+
     bool added = false;
     if (sub->Matches(topic->name, topic->special)) {
       topic->subscribers.Add(sub.get());
@@ -645,7 +655,7 @@ void ClientData4Base::ClientSubscribe(int64_t subuid,
       }
     }
 
-    if (added && !removed) {
+    if (!wasSubscribed && added && !removed) {
       // announce topic to client
       DEBUG4("client {}: announce {}", m_id, topic->name);
       SendAnnounce(topic.get(), std::nullopt);
@@ -758,7 +768,7 @@ void ClientDataLocal::SendPropertiesUpdate(TopicData* topic,
 }
 
 void ClientDataLocal::HandleLocal(std::span<const ClientMessage> msgs) {
-  DEBUG4("{}", "HandleLocal()");
+  DEBUG4("HandleLocal()");
   // just map as a normal client into client=0 calls
   for (const auto& elem : msgs) {  // NOLINT
     // common case is value, so check that first
@@ -2086,11 +2096,13 @@ void SImpl::SetFlags(ClientData* client, TopicData* topic, unsigned int flags) {
 }
 
 void SImpl::SetValue(ClientData* client, TopicData* topic, const Value& value) {
-  // update retained value if timestamp newer
-  if (!topic->lastValue || value.time() > topic->lastValue.time()) {
+  // update retained value if from same client or timestamp newer
+  if (!topic->lastValue || topic->lastValueClient == client ||
+      value.time() >= topic->lastValue.time()) {
     DEBUG4("updating '{}' last value (time was {} is {})", topic->name,
            topic->lastValue.time(), value.time());
     topic->lastValue = value;
+    topic->lastValueClient = client;
 
     // if persistent, update flag
     if (topic->persistent) {
@@ -2145,7 +2157,7 @@ void SImpl::UpdateMetaClients(const std::vector<ConnectionInfo>& conns) {
   if (mpack_writer_destroy(&w) == mpack_ok) {
     SetValue(nullptr, m_metaClients, Value::MakeRaw(std::move(w.bytes)));
   } else {
-    DEBUG4("{}", "failed to encode $clients");
+    DEBUG4("failed to encode $clients");
   }
 }
 

ntcore/src/main/native/cpp/net/WebSocketConnection.cpp

@@ -44,10 +44,12 @@ void WebSocketConnection::Flush() {
   }
 
   ++m_sendsActive;
-  m_ws.SendFrames(m_ws_frames, [this](auto bufs, auto) {
-    m_buf_pool.insert(m_buf_pool.end(), bufs.begin(), bufs.end());
-    if (m_sendsActive > 0) {
-      --m_sendsActive;
+  m_ws.SendFrames(m_ws_frames, [selfweak = weak_from_this()](auto bufs, auto) {
+    if (auto self = selfweak.lock()) {
+      self->m_buf_pool.insert(self->m_buf_pool.end(), bufs.begin(), bufs.end());
+      if (self->m_sendsActive > 0) {
+        --self->m_sendsActive;
+      }
     }
   });
   m_frames.clear();

ntcore/src/main/native/cpp/net/WebSocketConnection.h

@@ -4,6 +4,7 @@
 
 #pragma once
 
+#include <memory>
 #include <vector>
 
 #include <wpi/SmallVector.h>
@@ -15,7 +16,9 @@
 
 namespace nt::net {
 
-class WebSocketConnection final : public WireConnection {
+class WebSocketConnection final
+    : public WireConnection,
+      public std::enable_shared_from_this<WebSocketConnection> {
  public:
   explicit WebSocketConnection(wpi::WebSocket& ws);
   ~WebSocketConnection() override;

ntcore/src/main/native/cpp/net/WireDecoder.cpp

@@ -119,7 +119,7 @@ static void WireDecodeTextImpl(std::string_view in, T& out,
   }
 
   if (!j.is_array()) {
-    WPI_WARNING(logger, "{}", "expected JSON array at top level");
+    WPI_WARNING(logger, "expected JSON array at top level");
     return;
   }
 

ntcore/src/main/native/cpp/net3/ClientImpl3.cpp

@@ -238,7 +238,7 @@ void CImpl::SendPeriodic(uint64_t curTimeMs, bool initial) {
     if (!CheckNetworkReady()) {
       return;
     }
-    DEBUG4("{}", "Sending keep alive");
+    DEBUG4("Sending keep alive");
     WireEncodeKeepAlive(out.stream());
     // drift isn't critical here, so just go from current time
     m_nextKeepAliveTimeMs = curTimeMs + kKeepAliveIntervalMs;
@@ -274,7 +274,7 @@ void CImpl::SendPeriodic(uint64_t curTimeMs, bool initial) {
   }
 
   if (initial) {
-    DEBUG4("{}", "Sending ClientHelloDone");
+    DEBUG4("Sending ClientHelloDone");
     WireEncodeClientHelloDone(out.stream());
   }
 
@@ -403,7 +403,7 @@ void CImpl::SetValue(NT_Publisher pubHandle, const Value& value) {
 }
 
 void CImpl::KeepAlive() {
-  DEBUG4("{}", "KeepAlive()");
+  DEBUG4("KeepAlive()");
   if (m_state != kStateRunning && m_state != kStateInitialAssignments) {
     m_decoder.SetError("received unexpected KeepAlive message");
     return;
@@ -412,7 +412,7 @@ void CImpl::KeepAlive() {
 }
 
 void CImpl::ServerHelloDone() {
-  DEBUG4("{}", "ServerHelloDone()");
+  DEBUG4("ServerHelloDone()");
   if (m_state != kStateInitialAssignments) {
     m_decoder.SetError("received unexpected ServerHelloDone message");
     return;
@@ -426,7 +426,7 @@ void CImpl::ServerHelloDone() {
 }
 
 void CImpl::ClientHelloDone() {
-  DEBUG4("{}", "ClientHelloDone()");
+  DEBUG4("ClientHelloDone()");
   m_decoder.SetError("received unexpected ClientHelloDone message");
 }
 
@@ -572,7 +572,7 @@ void CImpl::EntryDelete(unsigned int id) {
 }
 
 void CImpl::ClearEntries() {
-  DEBUG4("{}", "ClearEntries()");
+  DEBUG4("ClearEntries()");
   if (m_state != kStateRunning) {
     m_decoder.SetError("received ClearEntries message before ServerHelloDone");
     return;
@@ -609,7 +609,7 @@ ClientImpl3::ClientImpl3(uint64_t curTimeMs, int inst, WireConnection3& wire,
                                     std::move(setPeriodic))} {}
 
 ClientImpl3::~ClientImpl3() {
-  WPI_DEBUG4(m_impl->m_logger, "{}", "NT3 ClientImpl destroyed");
+  WPI_DEBUG4(m_impl->m_logger, "NT3 ClientImpl destroyed");
 }
 
 void ClientImpl3::Start(std::string_view selfId,

ntcore/src/main/native/cpp/net3/UvStreamConnection3.cpp

@@ -23,10 +23,12 @@ void UvStreamConnection3::Flush() {
     return;
   }
   ++m_sendsActive;
-  m_stream.Write(m_buffers, [this](auto bufs, auto) {
-    m_buf_pool.insert(m_buf_pool.end(), bufs.begin(), bufs.end());
-    if (m_sendsActive > 0) {
-      --m_sendsActive;
+  m_stream.Write(m_buffers, [selfweak = weak_from_this()](auto bufs, auto) {
+    if (auto self = selfweak.lock()) {
+      self->m_buf_pool.insert(self->m_buf_pool.end(), bufs.begin(), bufs.end());
+      if (self->m_sendsActive > 0) {
+        --self->m_sendsActive;
+      }
     }
   });
   m_buffers.clear();

ntcore/src/main/native/cpp/net3/UvStreamConnection3.h

@@ -4,6 +4,7 @@
 
 #pragma once
 
+#include <memory>
 #include <string>
 #include <string_view>
 #include <vector>
@@ -20,7 +21,9 @@ class Stream;
 
 namespace nt::net3 {
 
-class UvStreamConnection3 final : public WireConnection3 {
+class UvStreamConnection3 final
+    : public WireConnection3,
+      public std::enable_shared_from_this<UvStreamConnection3> {
   static constexpr size_t kAllocSize = 4096;
 
  public:

ntcore/src/main/native/cpp/ntcore_c.cpp

@@ -542,10 +542,6 @@ void NT_RemoveConnectionListener(NT_ConnectionListener conn_listener) {
  * Client/Server Functions
  */
 
-void NT_SetNetworkIdentity(NT_Inst inst, const char* name, size_t name_len) {
-  nt::SetNetworkIdentity(inst, {name, name_len});
-}
-
 unsigned int NT_GetNetworkMode(NT_Inst inst) {
   return nt::GetNetworkMode(inst);
 }
@@ -568,12 +564,12 @@ void NT_StopServer(NT_Inst inst) {
   nt::StopServer(inst);
 }
 
-void NT_StartClient3(NT_Inst inst) {
-  nt::StartClient3(inst);
+void NT_StartClient3(NT_Inst inst, const char* identity) {
+  nt::StartClient3(inst, identity);
 }
 
-void NT_StartClient4(NT_Inst inst) {
-  nt::StartClient4(inst);
+void NT_StartClient4(NT_Inst inst, const char* identity) {
+  nt::StartClient4(inst, identity);
 }
 
 void NT_StopClient(NT_Inst inst) {

ntcore/src/main/native/cpp/ntcore_cpp.cpp

@@ -624,12 +624,6 @@ void StopConnectionDataLog(NT_ConnectionDataLogger logger) {
  * Client/Server Functions
  */
 
-void SetNetworkIdentity(NT_Inst inst, std::string_view name) {
-  if (auto ii = InstanceImpl::GetTyped(inst, Handle::kInstance)) {
-    ii->SetIdentity(name);
-  }
-}
-
 unsigned int GetNetworkMode(NT_Inst inst) {
   if (auto ii = InstanceImpl::GetTyped(inst, Handle::kInstance)) {
     return ii->networkMode;
@@ -664,15 +658,15 @@ void StopServer(NT_Inst inst) {
   }
 }
 
-void StartClient3(NT_Inst inst) {
+void StartClient3(NT_Inst inst, std::string_view identity) {
   if (auto ii = InstanceImpl::GetTyped(inst, Handle::kInstance)) {
-    ii->StartClient3();
+    ii->StartClient3(identity);
   }
 }
 
-void StartClient4(NT_Inst inst) {
+void StartClient4(NT_Inst inst, std::string_view identity) {
   if (auto ii = InstanceImpl::GetTyped(inst, Handle::kInstance)) {
-    ii->StartClient4();
+    ii->StartClient4(identity);
   }
 }
 
@@ -690,44 +684,39 @@ void SetServer(
     NT_Inst inst,
     std::span<const std::pair<std::string_view, unsigned int>> servers) {
   if (auto ii = InstanceImpl::GetTyped(inst, Handle::kInstance)) {
-    if (auto client = ii->GetClient()) {
-      std::vector<std::pair<std::string, unsigned int>> serversCopy;
-      serversCopy.reserve(servers.size());
-      for (auto&& server : servers) {
-        serversCopy.emplace_back(std::string{server.first}, server.second);
-      }
-      client->SetServers(serversCopy);
+    std::vector<std::pair<std::string, unsigned int>> serversCopy;
+    serversCopy.reserve(servers.size());
+    for (auto&& server : servers) {
+      serversCopy.emplace_back(std::string{server.first}, server.second);
     }
+    ii->SetServers(serversCopy);
   }
 }
 
 void SetServerTeam(NT_Inst inst, unsigned int team, unsigned int port) {
   if (auto ii = InstanceImpl::GetTyped(inst, Handle::kInstance)) {
-    if (auto client = ii->GetClient()) {
-      std::vector<std::pair<std::string, unsigned int>> servers;
-      servers.reserve(5);
+    std::vector<std::pair<std::string, unsigned int>> servers;
+    servers.reserve(5);
 
-      // 10.te.am.2
-      servers.emplace_back(
-          fmt::format("10.{}.{}.2", static_cast<int>(team / 100),
-                      static_cast<int>(team % 100)),
-          port);
+    // 10.te.am.2
+    servers.emplace_back(fmt::format("10.{}.{}.2", static_cast<int>(team / 100),
+                                     static_cast<int>(team % 100)),
+                         port);
 
-      // 172.22.11.2
-      servers.emplace_back("172.22.11.2", port);
+    // 172.22.11.2
+    servers.emplace_back("172.22.11.2", port);
 
-      // roboRIO-<team>-FRC.local
-      servers.emplace_back(fmt::format("roboRIO-{}-FRC.local", team), port);
+    // roboRIO-<team>-FRC.local
+    servers.emplace_back(fmt::format("roboRIO-{}-FRC.local", team), port);
 
-      // roboRIO-<team>-FRC.lan
-      servers.emplace_back(fmt::format("roboRIO-{}-FRC.lan", team), port);
+    // roboRIO-<team>-FRC.lan
+    servers.emplace_back(fmt::format("roboRIO-{}-FRC.lan", team), port);
 
-      // roboRIO-<team>-FRC.frc-field.local
-      servers.emplace_back(fmt::format("roboRIO-{}-FRC.frc-field.local", team),
-                           port);
+    // roboRIO-<team>-FRC.frc-field.local
+    servers.emplace_back(fmt::format("roboRIO-{}-FRC.frc-field.local", team),
+                         port);
 
-      client->SetServers(servers);
-    }
+    ii->SetServers(servers);
   }
 }
 

ntcore/src/main/native/include/networktables/ConnectionListener.h

@@ -15,7 +15,9 @@ class NetworkTableInstance;
 
 /**
  * Connection listener. This calls back to a callback function when a connection
- * change occurs.
+ * change occurs. The callback function is called asynchronously on a separate
+ * thread, so it's important to use synchronization or atomics when accessing
+ * any shared state from the callback function.
  */
 class ConnectionListener final {
  public:

ntcore/src/main/native/include/networktables/NetworkTableEntry.h

@@ -112,9 +112,10 @@ class NetworkTableEntry final {
   int64_t GetLastChange() const;
 
   /**
-   * Gets the entry's value. If the entry does not exist, returns nullptr.
+   * Gets the entry's value. If the entry does not exist, returns an empty
+   * value.
    *
-   * @return the entry's value or nullptr if it does not exist.
+   * @return the entry's value or an empty value if it does not exist.
    */
   Value GetValue() const;
 

ntcore/src/main/native/include/networktables/NetworkTableInstance.h

@@ -383,16 +383,6 @@ class NetworkTableInstance final {
    * @name Client/Server Functions
    */
 
-  /**
-   * Set the network identity of this node.
-   *
-   * This is the name used during the initial connection handshake, and is
-   * visible through ConnectionInfo on the remote node.
-   *
-   * @param name      identity to advertise
-   */
-  void SetNetworkIdentity(std::string_view name);
-
   /**
    * Get the current network mode.
    *
@@ -436,14 +426,18 @@ class NetworkTableInstance final {
   /**
    * Starts a NT3 client.  Use SetServer or SetServerTeam to set the server name
    * and port.
+   *
+   * @param identity  network identity to advertise (cannot be empty string)
    */
-  void StartClient3();
+  void StartClient3(std::string_view identity);
 
   /**
    * Starts a NT4 client.  Use SetServer or SetServerTeam to set the server name
    * and port.
+   *
+   * @param identity  network identity to advertise (cannot be empty string)
    */
-  void StartClient4();
+  void StartClient4(std::string_view identity);
 
   /**
    * Stops the client if it is running.

ntcore/src/main/native/include/networktables/NetworkTableInstance.inc

@@ -87,10 +87,6 @@ inline void NetworkTableInstance::RemoveConnectionListener(
   ::nt::RemoveConnectionListener(conn_listener);
 }
 
-inline void NetworkTableInstance::SetNetworkIdentity(std::string_view name) {
-  ::nt::SetNetworkIdentity(m_handle, name);
-}
-
 inline unsigned int NetworkTableInstance::GetNetworkMode() const {
   return ::nt::GetNetworkMode(m_handle);
 }
@@ -114,12 +110,12 @@ inline void NetworkTableInstance::StopServer() {
   ::nt::StopServer(m_handle);
 }
 
-inline void NetworkTableInstance::StartClient3() {
-  ::nt::StartClient3(m_handle);
+inline void NetworkTableInstance::StartClient3(std::string_view identity) {
+  ::nt::StartClient3(m_handle, identity);
 }
 
-inline void NetworkTableInstance::StartClient4() {
-  ::nt::StartClient4(m_handle);
+inline void NetworkTableInstance::StartClient4(std::string_view identity) {
+  ::nt::StartClient4(m_handle, identity);
 }
 
 inline void NetworkTableInstance::StopClient() {

ntcore/src/main/native/include/networktables/TopicListener.h

@@ -66,7 +66,9 @@ struct TopicListenerFlags {
 
 /**
  * Topic change listener. This calls back to a callback function when a topic
- * change matching the specified mask occurs.
+ * change matching the specified mask occurs. The callback function is called
+ * asynchronously on a separate thread, so it's important to use synchronization
+ * or atomics when accessing any shared state from the callback function.
  */
 class TopicListener final {
  public:
@@ -202,7 +204,7 @@ class TopicListenerPoller final {
    * @param mask Bitmask of TopicListenerFlags values
    * @return Listener handle
    */
-  NT_ValueListener Add(const Subscriber& subscriber, unsigned int mask);
+  NT_TopicListener Add(const Subscriber& subscriber, unsigned int mask);
 
   /**
    * Start listening to topic changes on a subscriber.
@@ -211,7 +213,7 @@ class TopicListenerPoller final {
    * @param mask Bitmask of TopicListenerFlags values
    * @return Listener handle
    */
-  NT_ValueListener Add(const MultiSubscriber& subscriber, unsigned int mask);
+  NT_TopicListener Add(const MultiSubscriber& subscriber, unsigned int mask);
 
   /**
    * Start listening to topic changes on an entry.
@@ -220,7 +222,7 @@ class TopicListenerPoller final {
    * @param mask Bitmask of TopicListenerFlags values
    * @return Listener handle
    */
-  NT_ValueListener Add(const NetworkTableEntry& entry, unsigned int mask);
+  NT_TopicListener Add(const NetworkTableEntry& entry, unsigned int mask);
 
   /**
    * Remove a listener.

ntcore/src/main/native/include/networktables/ValueListener.h

@@ -48,7 +48,9 @@ struct ValueListenerFlags {
 
 /**
  * Value change listener. This calls back to a callback function when a value
- * change matching the specified mask occurs.
+ * change matching the specified mask occurs. The callback function is called
+ * asynchronously on a separate thread, so it's important to use synchronization
+ * or atomics when accessing any shared state from the callback function.
  */
 class ValueListener final {
  public:
@@ -120,7 +122,7 @@ class ValueListener final {
  */
 class ValueListenerPoller final {
  public:
-  ValueListenerPoller();
+  ValueListenerPoller() = default;
 
   /**
    * Construct a value listener poller.
@@ -196,7 +198,7 @@ class ValueListenerPoller final {
   std::vector<ValueNotification> ReadQueue();
 
  private:
-  NT_ValueListenerPoller m_handle;
+  NT_ValueListenerPoller m_handle{0};
 };
 
 }  // namespace nt

ntcore/src/main/native/include/ntcore_c.h

@@ -198,8 +198,7 @@ struct NT_TopicInfo {
 /** NetworkTables Connection Information */
 struct NT_ConnectionInfo {
   /**
-   * The remote identifier (as set on the remote node by
-   * NetworkTableInstance::SetNetworkIdentity() or nt::SetNetworkIdentity()).
+   * The remote identifier (as set on the remote node by NT_StartClient4().
    */
   struct NT_String remote_id;
 
@@ -1114,17 +1113,6 @@ void NT_RemoveConnectionListener(NT_ConnectionListener conn_listener);
  * @{
  */
 
-/**
- * Set the network identity of this node.
- * This is the name used during the initial connection handshake, and is
- * visible through NT_ConnectionInfo on the remote node.
- *
- * @param inst      instance handle
- * @param name      identity to advertise
- * @param name_len  length of name in bytes
- */
-void NT_SetNetworkIdentity(NT_Inst inst, const char* name, size_t name_len);
-
 /**
  * Get the current network mode.
  *
@@ -1172,17 +1160,19 @@ void NT_StopServer(NT_Inst inst);
  * Starts a NT3 client.  Use NT_SetServer or NT_SetServerTeam to set the server
  * name and port.
  *
- * @param inst  instance handle
+ * @param inst      instance handle
+ * @param identity  network identity to advertise (cannot be empty string)
  */
-void NT_StartClient3(NT_Inst inst);
+void NT_StartClient3(NT_Inst inst, const char* identity);
 
 /**
  * Starts a NT4 client.  Use NT_SetServer or NT_SetServerTeam to set the server
  * name and port.
  *
- * @param inst  instance handle
+ * @param inst      instance handle
+ * @param identity  network identity to advertise (cannot be empty string)
  */
-void NT_StartClient4(NT_Inst inst);
+void NT_StartClient4(NT_Inst inst, const char* identity);
 
 /**
  * Stops the client if it is running.

ntcore/src/main/native/include/ntcore_cpp.h

@@ -74,7 +74,7 @@ struct TopicInfo {
 struct ConnectionInfo {
   /**
    * The remote identifier (as set on the remote node by
-   * NetworkTableInstance::SetNetworkIdentity() or nt::SetNetworkIdentity()).
+   * NetworkTableInstance::StartClient4() or nt::StartClient4()).
    */
   std::string remote_id;
 
@@ -292,7 +292,7 @@ class PubSubOption {
   /**
    * Polling storage for subscription. Specifies the maximum number of updates
    * NetworkTables should store between calls to the subscriber's poll()
-   * function. Defaults to 1 if logging is false, 20 if logging is true.
+   * function. Defaults to 1 if SendAll is false, 20 if SendAll is true.
    *
    * @param depth number of entries to save for polling.
    * @return option
@@ -1007,16 +1007,6 @@ void RemoveConnectionListener(NT_ConnectionListener conn_listener);
  * @{
  */
 
-/**
- * Set the network identity of this node.
- * This is the name used during the initial connection handshake, and is
- * visible through ConnectionInfo on the remote node.
- *
- * @param inst      instance handle
- * @param name      identity to advertise
- */
-void SetNetworkIdentity(NT_Inst inst, std::string_view name);
-
 /**
  * Get the current network mode.
  *
@@ -1064,17 +1054,19 @@ void StopServer(NT_Inst inst);
  * Starts a NT3 client.  Use SetServer or SetServerTeam to set the server name
  * and port.
  *
- * @param inst  instance handle
+ * @param inst      instance handle
+ * @param identity  network identity to advertise (cannot be empty string)
  */
-void StartClient3(NT_Inst inst);
+void StartClient3(NT_Inst inst, std::string_view identity);
 
 /**
  * Starts a NT4 client.  Use SetServer or SetServerTeam to set the server name
  * and port.
  *
- * @param inst  instance handle
+ * @param inst      instance handle
+ * @param identity  network identity to advertise (cannot be empty string)
  */
-void StartClient4(NT_Inst inst);
+void StartClient4(NT_Inst inst, std::string_view identity);
 
 /**
  * Stops the client if it is running.

ntcore/src/test/java/edu/wpi/first/networktables/ConnectionListenerTest.java

@@ -27,10 +27,7 @@ class ConnectionListenerTest {
   @BeforeEach
   void setUp() {
     m_serverInst = NetworkTableInstance.create();
-    m_serverInst.setNetworkIdentity("server");
-
     m_clientInst = NetworkTableInstance.create();
-    m_clientInst.setNetworkIdentity("client");
   }
 
   @AfterEach
@@ -42,7 +39,7 @@ class ConnectionListenerTest {
   /** Connect to the server. */
   private void connect(int port) {
     m_serverInst.startServer("connectionlistenertest.json", "127.0.0.1", 0, port);
-    m_clientInst.startClient4();
+    m_clientInst.startClient4("client");
     m_clientInst.setServer("127.0.0.1", port);
 
     // wait for client to report it's connected, then wait another 0.1 sec
@@ -125,7 +122,7 @@ class ConnectionListenerTest {
             false);
 
     // trigger a connect event
-    m_clientInst.startClient4();
+    m_clientInst.startClient4("client");
     m_clientInst.setServer(address, threadedPort);
     threadedPort++;
 

ntcore/src/test/java/edu/wpi/first/networktables/LoggerTest.java

@@ -31,7 +31,7 @@ class LoggerTest {
     List<LogMessage> msgs = new ArrayList<>();
     m_clientInst.addLogger(msgs::add, LogMessage.kInfo, 100);
 
-    m_clientInst.startClient4();
+    m_clientInst.startClient4("client");
     m_clientInst.setServer("127.0.0.1", 10000);
 
     // wait for client to report it's started, then wait another 0.1 sec

ntcore/src/test/java/edu/wpi/first/networktables/TopicListenerTest.java

@@ -20,10 +20,7 @@ class TopicListenerTest {
   @BeforeEach
   void setUp() {
     m_serverInst = NetworkTableInstance.create();
-    m_serverInst.setNetworkIdentity("server");
-
     m_clientInst = NetworkTableInstance.create();
-    m_clientInst.setNetworkIdentity("client");
   }
 
   @AfterEach
@@ -34,7 +31,7 @@ class TopicListenerTest {
 
   private void connect() {
     m_serverInst.startServer("topiclistenertest.json", "127.0.0.1", 0, 10010);
-    m_clientInst.startClient4();
+    m_clientInst.startClient4("client");
     m_clientInst.setServer("127.0.0.1", 10010);
 
     // Use connection listener to ensure we've connected

ntcore/src/test/native/cpp/ConnectionListenerTest.cpp

@@ -15,10 +15,7 @@
 class ConnectionListenerTest : public ::testing::Test {
  public:
   ConnectionListenerTest()
-      : server_inst(nt::CreateInstance()), client_inst(nt::CreateInstance()) {
-    nt::SetNetworkIdentity(server_inst, "server");
-    nt::SetNetworkIdentity(client_inst, "client");
-  }
+      : server_inst(nt::CreateInstance()), client_inst(nt::CreateInstance()) {}
 
   ~ConnectionListenerTest() override {
     nt::DestroyInstance(server_inst);
@@ -36,7 +33,7 @@ void ConnectionListenerTest::Connect(const char* address, unsigned int port3,
                                      unsigned int port4) {
   nt::StartServer(server_inst, "connectionlistenertest.ini", address, port3,
                   port4);
-  nt::StartClient4(client_inst);
+  nt::StartClient4(client_inst, "client");
   nt::SetServer(client_inst, address, port4);
 
   // wait for client to report it's connected, then wait another 0.1 sec

ntcore/src/test/native/cpp/LocalStorageTest.cpp

@@ -31,10 +31,7 @@ namespace nt {
 
 class LocalStorageTest : public ::testing::Test {
  public:
-  LocalStorageTest() {
-    storage.StartNetwork(startup);
-    storage.SetNetwork(&network);
-  }
+  LocalStorageTest() { storage.StartNetwork(startup, &network); }
 
   ::testing::StrictMock<net::MockNetworkStartupInterface> startup;
   ::testing::StrictMock<net::MockNetworkInterface> network;

ntcore/src/test/native/cpp/TopicListenerTest.cpp

@@ -18,8 +18,6 @@ class TopicListenerTest : public ::testing::Test {
  public:
   TopicListenerTest()
       : m_serverInst(nt::CreateInstance()), m_clientInst(nt::CreateInstance()) {
-    nt::SetNetworkIdentity(m_serverInst, "server");
-    nt::SetNetworkIdentity(m_clientInst, "client");
 #if 0
     nt::AddLogger(server_inst,
                   [](const nt::LogMessage& msg) {
@@ -52,7 +50,7 @@ class TopicListenerTest : public ::testing::Test {
 
 void TopicListenerTest::Connect(unsigned int port) {
   nt::StartServer(m_serverInst, "topiclistenertest.json", "127.0.0.1", 0, port);
-  nt::StartClient4(m_clientInst);
+  nt::StartClient4(m_clientInst, "client");
   nt::SetServer(m_clientInst, "127.0.0.1", port);
 
   // Use connection listener to ensure we've connected

ntcore/src/test/native/cpp/ValueListenerTest.cpp

@@ -21,9 +21,7 @@ namespace nt {
 // Test only local here; it's more reliable to mock the network
 class ValueListenerTest : public ::testing::Test {
  public:
-  ValueListenerTest() : m_inst{nt::CreateInstance()} {
-    nt::SetNetworkIdentity(m_inst, "server");
-  }
+  ValueListenerTest() : m_inst{nt::CreateInstance()} {}
 
   ~ValueListenerTest() override { nt::DestroyInstance(m_inst); }
 

ntcore/src/test/native/cpp/net/MockNetworkInterface.h

@@ -77,9 +77,9 @@ class MockLocalStorage : public ILocalStorage {
               (override));
   MOCK_METHOD(void, NetworkSetValue, (NT_Topic topicHandle, const Value& value),
               (override));
-  MOCK_METHOD(void, StartNetwork, (NetworkStartupInterface & startup),
+  MOCK_METHOD(void, StartNetwork,
+              (NetworkStartupInterface & startup, NetworkInterface* network),
               (override));
-  MOCK_METHOD(void, SetNetwork, (NetworkInterface * network), (override));
   MOCK_METHOD(void, ClearNetwork, (), (override));
 };
 

ntcoreffi/src/main/native/symbols.txt

@@ -197,7 +197,6 @@ NT_SetFloat
 NT_SetFloatArray
 NT_SetInteger
 NT_SetIntegerArray
-NT_SetNetworkIdentity
 NT_SetNow
 NT_SetRaw
 NT_SetServer

roborioteamnumbersetter/src/main/native/cpp/DeploySession.cpp

@@ -79,7 +79,7 @@ bool DeploySession::ChangeTeamNumber(const std::string& macAddress,
           session.Open();
           DEBUG("SSH connection to {} was successful.", ip);
 
-          SUCCESS("{}", "roboRIO Connected!");
+          SUCCESS("roboRIO Connected!");
 
           try {
             session.Execute(fmt::format(
@@ -123,7 +123,7 @@ bool DeploySession::Reboot(const std::string& macAddress,
           session.Open();
           DEBUG("SSH connection to {} was successful.", ip);
 
-          SUCCESS("{}", "roboRIO Connected!");
+          SUCCESS("roboRIO Connected!");
 
           try {
             session.Execute(fmt::format("sync ; shutdown -r now"));
@@ -162,7 +162,7 @@ bool DeploySession::Blink(const std::string& macAddress,
           session.Open();
           DEBUG("SSH connection to {} was successful.", ip);
 
-          SUCCESS("{}", "roboRIO Connected!");
+          SUCCESS("roboRIO Connected!");
 
           try {
             session.Execute(fmt::format(

roborioteamnumbersetter/src/main/native/cpp/SshSession.cpp

@@ -18,7 +18,7 @@
 
 using namespace sysid;
 
-#define INFO(fmt, ...) WPI_INFO(m_logger, fmt, __VA_ARGS__)
+#define INFO(fmt, ...) WPI_INFO(m_logger, fmt __VA_OPT__(, ) __VA_ARGS__)
 
 SshSession::SshSession(std::string_view host, int port, std::string_view user,
                        std::string_view pass, wpi::Logger& logger)

simulation/halsim_ws_server/src/test/native/cpp/main.cpp

@@ -157,6 +157,8 @@ TEST_F(WebServerIntegrationTest, DriverStation) {
   using namespace std::chrono_literals;
   std::this_thread::sleep_for(1s);
 
+  HAL_RefreshDSData();
+
   // Compare results
   HAL_ControlWord cw;
   HAL_GetControlWord(&cw);

upstream_utils/llvm_patches/0020-Prefer-fmtlib.patch

@@ -49,7 +49,7 @@ index ec1a1633a..8de7b726d 100644
 -    dbgs() << " at " << file << ":" << line;
 -  dbgs() << "!\n";
 +    fmt::print(stderr, " at {}:{}", file, line);
-+  fmt::print(stderr, "{}", "!\n");
++  fmt::print(stderr, "!\n");
    abort();
  #ifdef LLVM_BUILTIN_UNREACHABLE
    // Windows systems and possibly others don't declare abort() to be noreturn,

wpilib-config.cmake.in

@@ -2,8 +2,8 @@ include(CMakeFindDependencyMacro)
 @FILENAME_DEP_REPLACE@
 set(THREADS_PREFER_PTHREAD_FLAG ON)
 find_dependency(Threads)
-@LIBUV_VCPKG_REPLACE@
-@EIGEN_VCPKG_REPLACE@
+@LIBUV_SYSTEM_REPLACE@
+@EIGEN_SYSTEM_REPLACE@
 @WPIUTIL_DEP_REPLACE@
 @NTCORE_DEP_REPLACE@
 @CSCORE_DEP_REPLACE@

wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/Command.java

@@ -258,7 +258,10 @@ public interface Command {
    * decorated without issue.
    *
    * @return the decorated command
-   * @deprecated use {@link #endlessly()} instead.
+   * @deprecated PerpetualCommand violates the assumption that execute() doesn't get called after
+   *     isFinished() returns true -- an assumption that should be valid. This was unsafe/undefined
+   *     behavior from the start, and RepeatCommand provides an easy way to achieve similar end
+   *     results with slightly different (and safe) semantics.
    */
   @SuppressWarnings("removal") // PerpetualCommand
   @Deprecated(forRemoval = true, since = "2023")
@@ -266,22 +269,6 @@ public interface Command {
     return new PerpetualCommand(this);
   }
 
-  /**
-   * Decorates this command to run endlessly, ignoring its ordinary end conditions. The decorated
-   * command can still be interrupted or canceled.
-   *
-   * <p>Note: This decorator works by composing this command within a CommandGroup. The command
-   * cannot be used independently after being decorated, or be re-decorated with a different
-   * decorator, unless it is manually cleared from the list of grouped commands with {@link
-   * CommandGroupBase#clearGroupedCommand(Command)}. The decorated command can, however, be further
-   * decorated without issue.
-   *
-   * @return the decorated command
-   */
-  default EndlessCommand endlessly() {
-    return new EndlessCommand(this);
-  }
-
   /**
    * Decorates this command to run repeatedly, restarting it when it ends, until this command is
    * interrupted. The decorated command can still be canceled.

wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/EndlessCommand.java

@@ -1,55 +0,0 @@
-// Copyright (c) FIRST and other WPILib contributors.
-// Open Source Software; you can modify and/or share it under the terms of
-// the WPILib BSD license file in the root directory of this project.
-
-package edu.wpi.first.wpilibj2.command;
-
-import static edu.wpi.first.wpilibj2.command.CommandGroupBase.registerGroupedCommands;
-import static edu.wpi.first.wpilibj2.command.CommandGroupBase.requireUngrouped;
-
-/**
- * A command that runs another command endlessly, ignoring that command's end conditions. While this
- * class does not extend {@link CommandGroupBase}, it is still considered a CommandGroup, as it
- * allows one to compose another command within it; the command instances that are passed to it
- * cannot be added to any other groups, or scheduled individually.
- *
- * <p>As a rule, CommandGroups require the union of the requirements of their component commands.
- *
- * <p>This class is provided by the NewCommands VendorDep
- */
-public class EndlessCommand extends CommandBase {
-  protected final Command m_command;
-
-  /**
-   * Creates a new EndlessCommand. Will run another command endlessly, ignoring that command's end
-   * conditions, unless this command itself is interrupted.
-   *
-   * @param command the command to run endlessly
-   */
-  public EndlessCommand(Command command) {
-    requireUngrouped(command);
-    registerGroupedCommands(command);
-    m_command = command;
-    m_requirements.addAll(command.getRequirements());
-  }
-
-  @Override
-  public void initialize() {
-    m_command.initialize();
-  }
-
-  @Override
-  public void execute() {
-    m_command.execute();
-  }
-
-  @Override
-  public void end(boolean interrupted) {
-    m_command.end(interrupted);
-  }
-
-  @Override
-  public boolean runsWhenDisabled() {
-    return m_command.runsWhenDisabled();
-  }
-}

wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/PerpetualCommand.java

@@ -17,7 +17,10 @@ import static edu.wpi.first.wpilibj2.command.CommandGroupBase.requireUngrouped;
  *
  * <p>This class is provided by the NewCommands VendorDep
  *
- * @deprecated Replaced by {@link EndlessCommand}.
+ * @deprecated PerpetualCommand violates the assumption that execute() doesn't get called after
+ *     isFinished() returns true -- an assumption that should be valid. This was unsafe/undefined
+ *     behavior from the start, and RepeatCommand provides an easy way to achieve similar end
+ *     results with slightly different (and safe) semantics.
  */
 @Deprecated(forRemoval = true, since = "2023")
 public class PerpetualCommand extends CommandBase {

wpilibNewCommands/src/main/native/cpp/frc2/command/Command.cpp

@@ -7,7 +7,6 @@
 #include "frc2/command/CommandHelper.h"
 #include "frc2/command/CommandScheduler.h"
 #include "frc2/command/ConditionalCommand.h"
-#include "frc2/command/EndlessCommand.h"
 #include "frc2/command/InstantCommand.h"
 #include "frc2/command/ParallelCommandGroup.h"
 #include "frc2/command/ParallelDeadlineGroup.h"
@@ -91,10 +90,6 @@ PerpetualCommand Command::Perpetually() && {
   WPI_UNIGNORE_DEPRECATED
 }
 
-CommandPtr Command::Endlessly() && {
-  return CommandPtr(std::move(*this).TransferOwnership()).Endlessly();
-}
-
 CommandPtr Command::Repeatedly() && {
   return CommandPtr(std::move(*this).TransferOwnership()).Repeatedly();
 }

wpilibNewCommands/src/main/native/cpp/frc2/command/CommandGroupBase.cpp

@@ -9,7 +9,7 @@ using namespace frc2;
 bool CommandGroupBase::RequireUngrouped(const Command& command) {
   if (command.IsGrouped()) {
     throw FRC_MakeError(
-        frc::err::CommandIllegalUse, "{}",
+        frc::err::CommandIllegalUse,
         "Commands cannot be added to more than one CommandGroup");
   }
   return true;
@@ -27,7 +27,7 @@ bool CommandGroupBase::RequireUngrouped(
   }
   if (!allUngrouped) {
     throw FRC_MakeError(
-        frc::err::CommandIllegalUse, "{}",
+        frc::err::CommandIllegalUse,
         "Commands cannot be added to more than one CommandGroup");
   }
   return allUngrouped;
@@ -41,7 +41,7 @@ bool CommandGroupBase::RequireUngrouped(
   }
   if (!allUngrouped) {
     throw FRC_MakeError(
-        frc::err::CommandIllegalUse, "{}",
+        frc::err::CommandIllegalUse,
         "Commands cannot be added to more than one CommandGroup");
   }
   return allUngrouped;

wpilibNewCommands/src/main/native/cpp/frc2/command/CommandPtr.cpp

@@ -6,7 +6,6 @@
 
 #include "frc2/command/CommandScheduler.h"
 #include "frc2/command/ConditionalCommand.h"
-#include "frc2/command/EndlessCommand.h"
 #include "frc2/command/InstantCommand.h"
 #include "frc2/command/ParallelCommandGroup.h"
 #include "frc2/command/ParallelDeadlineGroup.h"
@@ -26,11 +25,6 @@ CommandPtr CommandPtr::Repeatedly() && {
   return std::move(*this);
 }
 
-CommandPtr CommandPtr::Endlessly() && {
-  m_ptr = std::make_unique<EndlessCommand>(std::move(m_ptr));
-  return std::move(*this);
-}
-
 CommandPtr CommandPtr::AsProxy() && {
   m_ptr = std::make_unique<ProxyScheduleCommand>(std::move(m_ptr));
   return std::move(*this);

wpilibNewCommands/src/main/native/cpp/frc2/command/CommandScheduler.cpp

@@ -119,7 +119,7 @@ void CommandScheduler::Schedule(Command* command) {
   }
 
   if (command->IsGrouped()) {
-    throw FRC_MakeError(frc::err::CommandIllegalUse, "{}",
+    throw FRC_MakeError(frc::err::CommandIllegalUse,
                         "A command that is part of a command group "
                         "cannot be independently scheduled");
     return;
@@ -296,6 +296,20 @@ void CommandScheduler::UnregisterSubsystem(
   }
 }
 
+void CommandScheduler::SetDefaultCommand(Subsystem* subsystem,
+                                         CommandPtr&& defaultCommand) {
+  if (!defaultCommand.get()->HasRequirement(subsystem)) {
+    throw FRC_MakeError(frc::err::CommandIllegalUse, "{}",
+                        "Default commands must require their subsystem!");
+  }
+  if (defaultCommand.get()->IsFinished()) {
+    throw FRC_MakeError(frc::err::CommandIllegalUse, "{}",
+                        "Default commands should not end!");
+  }
+
+  SetDefaultCommandImpl(subsystem, std::move(defaultCommand).Unwrap());
+}
+
 Command* CommandScheduler::GetDefaultCommand(const Subsystem* subsystem) const {
   auto&& find = m_impl->subsystems.find(subsystem);
   if (find != m_impl->subsystems.end()) {

wpilibNewCommands/src/main/native/cpp/frc2/command/EndlessCommand.cpp

@@ -1,28 +0,0 @@
-// Copyright (c) FIRST and other WPILib contributors.
-// Open Source Software; you can modify and/or share it under the terms of
-// the WPILib BSD license file in the root directory of this project.
-
-#include "frc2/command/EndlessCommand.h"
-
-using namespace frc2;
-
-EndlessCommand::EndlessCommand(std::unique_ptr<Command>&& command) {
-  if (!CommandGroupBase::RequireUngrouped(*command)) {
-    return;
-  }
-  m_command = std::move(command);
-  m_command->SetGrouped(true);
-  AddRequirements(m_command->GetRequirements());
-}
-
-void EndlessCommand::Initialize() {
-  m_command->Initialize();
-}
-
-void EndlessCommand::Execute() {
-  m_command->Execute();
-}
-
-void EndlessCommand::End(bool interrupted) {
-  m_command->End(interrupted);
-}

wpilibNewCommands/src/main/native/cpp/frc2/command/ParallelCommandGroup.cpp

@@ -65,7 +65,7 @@ void ParallelCommandGroup::AddCommands(
   }
 
   if (isRunning) {
-    throw FRC_MakeError(frc::err::CommandIllegalUse, "{}",
+    throw FRC_MakeError(frc::err::CommandIllegalUse,
                         "Commands cannot be added to a CommandGroup "
                         "while the group is running");
   }
@@ -77,7 +77,7 @@ void ParallelCommandGroup::AddCommands(
       m_runWhenDisabled &= command->RunsWhenDisabled();
       m_commands.emplace_back(std::move(command), false);
     } else {
-      throw FRC_MakeError(frc::err::CommandIllegalUse, "{}",
+      throw FRC_MakeError(frc::err::CommandIllegalUse,
                           "Multiple commands in a parallel group cannot "
                           "require the same subsystems");
     }

wpilibNewCommands/src/main/native/cpp/frc2/command/ParallelDeadlineGroup.cpp

@@ -60,7 +60,7 @@ void ParallelDeadlineGroup::AddCommands(
   }
 
   if (!m_finished) {
-    throw FRC_MakeError(frc::err::CommandIllegalUse, "{}",
+    throw FRC_MakeError(frc::err::CommandIllegalUse,
                         "Commands cannot be added to a CommandGroup "
                         "while the group is running");
   }
@@ -72,7 +72,7 @@ void ParallelDeadlineGroup::AddCommands(
       m_runWhenDisabled &= command->RunsWhenDisabled();
       m_commands.emplace_back(std::move(command), false);
     } else {
-      throw FRC_MakeError(frc::err::CommandIllegalUse, "{}",
+      throw FRC_MakeError(frc::err::CommandIllegalUse,
                           "Multiple commands in a parallel group cannot "
                           "require the same subsystems");
     }

wpilibNewCommands/src/main/native/cpp/frc2/command/ParallelRaceGroup.cpp

@@ -50,7 +50,7 @@ void ParallelRaceGroup::AddCommands(
   }
 
   if (isRunning) {
-    throw FRC_MakeError(frc::err::CommandIllegalUse, "{}",
+    throw FRC_MakeError(frc::err::CommandIllegalUse,
                         "Commands cannot be added to a CommandGroup "
                         "while the group is running");
   }
@@ -62,7 +62,7 @@ void ParallelRaceGroup::AddCommands(
       m_runWhenDisabled &= command->RunsWhenDisabled();
       m_commands.emplace_back(std::move(command));
     } else {
-      throw FRC_MakeError(frc::err::CommandIllegalUse, "{}",
+      throw FRC_MakeError(frc::err::CommandIllegalUse,
                           "Multiple commands in a parallel group cannot "
                           "require the same subsystems");
     }

wpilibNewCommands/src/main/native/cpp/frc2/command/SequentialCommandGroup.cpp

@@ -60,7 +60,7 @@ void SequentialCommandGroup::AddCommands(
   }
 
   if (m_currentCommandIndex != invalid_index) {
-    throw FRC_MakeError(frc::err::CommandIllegalUse, "{}",
+    throw FRC_MakeError(frc::err::CommandIllegalUse,
                         "Commands cannot be added to a CommandGroup "
                         "while the group is running");
   }

wpilibNewCommands/src/main/native/cpp/frc2/command/Subsystem.cpp

@@ -4,6 +4,8 @@
 
 #include "frc2/command/Subsystem.h"
 
+#include "frc2/command/CommandPtr.h"
+
 using namespace frc2;
 Subsystem::~Subsystem() {
   CommandScheduler::GetInstance().UnregisterSubsystem(this);
@@ -13,6 +15,11 @@ void Subsystem::Periodic() {}
 
 void Subsystem::SimulationPeriodic() {}
 
+void Subsystem::SetDefaultCommand(CommandPtr&& defaultCommand) {
+  CommandScheduler::GetInstance().SetDefaultCommand(this,
+                                                    std::move(defaultCommand));
+}
+
 Command* Subsystem::GetDefaultCommand() const {
   return CommandScheduler::GetInstance().GetDefaultCommand(this);
 }

wpilibNewCommands/src/main/native/include/frc2/command/Command.h

@@ -200,19 +200,21 @@ class Command {
    * conditions.  The decorated command can still be interrupted or canceled.
    *
    * @return the decorated command
-   * @deprecated replace with EndlessCommand
+   * @deprecated PerpetualCommand violates the assumption that execute() doesn't
+get called after isFinished() returns true -- an assumption that should be
+valid. This was unsafe/undefined behavior from the start, and RepeatCommand
+provides an easy way to achieve similar end results with slightly different (and
+safe) semantics.
    */
-  WPI_DEPRECATED("Replace with Endlessly()")
+  WPI_DEPRECATED(
+      "PerpetualCommand violates the assumption that execute() doesn't get "
+      "called after isFinished() returns true -- an assumption that should be "
+      "valid."
+      "This was unsafe/undefined behavior from the start, and RepeatCommand "
+      "provides an easy way to achieve similar end results with slightly "
+      "different (and safe) semantics.")
   PerpetualCommand Perpetually() &&;
 
-  /**
-   * Decorates this command to run endlessly, ignoring its ordinary end
-   * conditions. The decorated command can still be interrupted or canceled.
-   *
-   * @return the decorated command
-   */
-  [[nodiscard]] CommandPtr Endlessly() &&;
-
   /**
    * Decorates this command to run repeatedly, restarting it when it ends, until
    * this command is interrupted. The decorated command can still be canceled.

wpilibNewCommands/src/main/native/include/frc2/command/CommandPtr.h

@@ -46,14 +46,6 @@ class CommandPtr final {
    */
   [[nodiscard]] CommandPtr Repeatedly() &&;
 
-  /**
-   * Decorates this command to run endlessly, ignoring its ordinary end
-   * conditions. The decorated command can still be interrupted or canceled.
-   *
-   * @return the decorated command
-   */
-  [[nodiscard]] CommandPtr Endlessly() &&;
-
   /**
    * Decorates this command to run "by proxy" by wrapping it in a
    * ProxyScheduleCommand. This is useful for "forking off" from command groups

wpilibNewCommands/src/main/native/include/frc2/command/CommandScheduler.h

@@ -179,11 +179,11 @@ class CommandScheduler final : public nt::NTSendable,
                          Command, std::remove_reference_t<T>>>>
   void SetDefaultCommand(Subsystem* subsystem, T&& defaultCommand) {
     if (!defaultCommand.HasRequirement(subsystem)) {
-      throw FRC_MakeError(frc::err::CommandIllegalUse, "{}",
+      throw FRC_MakeError(frc::err::CommandIllegalUse,
                           "Default commands must require their subsystem!");
     }
     if (defaultCommand.IsFinished()) {
-      throw FRC_MakeError(frc::err::CommandIllegalUse, "{}",
+      throw FRC_MakeError(frc::err::CommandIllegalUse,
                           "Default commands should not end!");
     }
     SetDefaultCommandImpl(subsystem,
@@ -191,6 +191,19 @@ class CommandScheduler final : public nt::NTSendable,
                               std::forward<T>(defaultCommand)));
   }
 
+  /**
+   * Sets the default command for a subsystem.  Registers that subsystem if it
+   * is not already registered.  Default commands will run whenever there is no
+   * other command currently scheduled that requires the subsystem.  Default
+   * commands should be written to never end (i.e. their IsFinished() method
+   * should return false), as they would simply be re-scheduled if they do.
+   * Default commands must also require their subsystem.
+   *
+   * @param subsystem      the subsystem whose default command will be set
+   * @param defaultCommand the default command to associate with the subsystem
+   */
+  void SetDefaultCommand(Subsystem* subsystem, CommandPtr&& defaultCommand);
+
   /**
    * Gets the default command associated with this subsystem.  Null if this
    * subsystem has no default command associated with it.

wpilibNewCommands/src/main/native/include/frc2/command/EndlessCommand.h

@@ -1,77 +0,0 @@
-// Copyright (c) FIRST and other WPILib contributors.
-// Open Source Software; you can modify and/or share it under the terms of
-// the WPILib BSD license file in the root directory of this project.
-
-#pragma once
-
-#ifdef _WIN32
-#pragma warning(push)
-#pragma warning(disable : 4521)
-#endif
-
-#include <memory>
-#include <utility>
-
-#include "frc2/command/CommandBase.h"
-#include "frc2/command/CommandGroupBase.h"
-#include "frc2/command/CommandHelper.h"
-
-namespace frc2 {
-/**
- * A command that runs another command endlessly, ignoring that command's
- * end conditions.  While this class does not extend {@link CommandGroupBase},
- * it is still considered a CommandGroup, as it allows one to compose another
- * command within it; the command instances that are passed to it cannot be
- * added to any other groups, or scheduled individually.
- *
- * <p>As a rule, CommandGroups require the union of the requirements of their
- * component commands.
- *
- * This class is provided by the NewCommands VendorDep
- */
-class EndlessCommand : public CommandHelper<CommandBase, EndlessCommand> {
- public:
-  /**
-   * Creates a new EndlessCommand. Will run another command endlessly,
-   * ignoring that command's end conditions, unless this command itself is
-   * interrupted.
-   *
-   * @param command the command to run endlessly
-   */
-  explicit EndlessCommand(std::unique_ptr<Command>&& command);
-
-  /**
-   * Creates a new EndlessCommand. Will run another command endlessly,
-   * ignoring that command's end conditions, unless this command itself is
-   * interrupted.
-   *
-   * @param command the command to run endlessly
-   */
-  template <class T, typename = std::enable_if_t<std::is_base_of_v<
-                         Command, std::remove_reference_t<T>>>>
-  explicit EndlessCommand(T&& command)
-      : EndlessCommand(std::make_unique<std::remove_reference_t<T>>(
-            std::forward<T>(command))) {}
-
-  EndlessCommand(EndlessCommand&& other) = default;
-
-  // No copy constructors for command groups
-  EndlessCommand(const EndlessCommand& other) = delete;
-
-  // Prevent template expansion from emulating copy ctor
-  EndlessCommand(EndlessCommand&) = delete;
-
-  void Initialize() override;
-
-  void Execute() override;
-
-  void End(bool interrupted) override;
-
- private:
-  std::unique_ptr<Command> m_command;
-};
-}  // namespace frc2
-
-#ifdef _WIN32
-#pragma warning(pop)
-#endif

wpilibNewCommands/src/main/native/include/frc2/command/PerpetualCommand.h

@@ -29,7 +29,11 @@ namespace frc2 {
  *
  * This class is provided by the NewCommands VendorDep
  *
- * @deprecated replace with EndlessCommand
+ * @deprecated PerpetualCommand violates the assumption that execute() doesn't
+get called after isFinished() returns true -- an assumption that should be
+valid. This was unsafe/undefined behavior from the start, and RepeatCommand
+provides an easy way to achieve similar end results with slightly different (and
+safe) semantics.
  */
 class PerpetualCommand : public CommandHelper<CommandBase, PerpetualCommand> {
  public:
@@ -40,10 +44,16 @@ class PerpetualCommand : public CommandHelper<CommandBase, PerpetualCommand> {
    *
    * @param command the command to run perpetually
    */
-  WPI_DEPRECATED("Replace with EndlessCommand")
+  WPI_DEPRECATED(
+      "PerpetualCommand violates the assumption that execute() doesn't get "
+      "called after isFinished() returns true -- an assumption that should be "
+      "valid."
+      "This was unsafe/undefined behavior from the start, and RepeatCommand "
+      "provides an easy way to achieve similar end results with slightly "
+      "different (and safe) semantics.")
   explicit PerpetualCommand(std::unique_ptr<Command>&& command);
-
   WPI_IGNORE_DEPRECATED
+
   /**
    * Creates a new PerpetualCommand.  Will run another command in perpetuity,
    * ignoring that command's end conditions, unless this command itself is
@@ -53,7 +63,13 @@ class PerpetualCommand : public CommandHelper<CommandBase, PerpetualCommand> {
    */
   template <class T, typename = std::enable_if_t<std::is_base_of_v<
                          Command, std::remove_reference_t<T>>>>
-  WPI_DEPRECATED("Replace with EndlessCommand")
+  WPI_DEPRECATED(
+      "PerpetualCommand violates the assumption that execute() doesn't get "
+      "called after isFinished() returns true -- an assumption that should be "
+      "valid."
+      "This was unsafe/undefined behavior from the start, and RepeatCommand "
+      "provides an easy way to achieve similar end results with slightly "
+      "different (and safe) semantics.")
   explicit PerpetualCommand(T&& command)
       : PerpetualCommand(std::make_unique<std::remove_reference_t<T>>(
             std::forward<T>(command))) {}

wpilibNewCommands/src/main/native/include/frc2/command/Subsystem.h

@@ -11,6 +11,7 @@
 
 namespace frc2 {
 class Command;
+class CommandPtr;
 /**
  * A robot subsystem.  Subsystems are the basic unit of robot organization in
  * the Command-based framework; they encapsulate low-level hardware objects
@@ -71,6 +72,17 @@ class Subsystem {
         this, std::forward<T>(defaultCommand));
   }
 
+  /**
+   * Sets the default Command of the subsystem.  The default command will be
+   * automatically scheduled when no other commands are scheduled that require
+   * the subsystem. Default commands should generally not end on their own, i.e.
+   * their IsFinished() method should always return false.  Will automatically
+   * register this subsystem with the CommandScheduler.
+   *
+   * @param defaultCommand the default command to associate with this subsystem
+   */
+  void SetDefaultCommand(CommandPtr&& defaultCommand);
+
   /**
    * Gets the default command for this subsystem.  Returns null if no default
    * command is currently associated with the subsystem.

wpilibNewCommands/src/test/java/edu/wpi/first/wpilibj2/MockHardwareExtension.java

@@ -34,5 +34,6 @@ public final class MockHardwareExtension implements BeforeAllCallback {
     DriverStationSim.setAutonomous(false);
     DriverStationSim.setEnabled(true);
     DriverStationSim.setTest(true);
+    DriverStationSim.notifyNewData();
   }
 }

wpilibNewCommands/src/test/java/edu/wpi/first/wpilibj2/command/CommandDecoratorTest.java

@@ -201,22 +201,6 @@ class CommandDecoratorTest extends CommandTestBase {
     }
   }
 
-  @Test
-  void endlesslyTest() {
-    try (CommandScheduler scheduler = new CommandScheduler()) {
-      Command command = new InstantCommand();
-
-      Command perpetual = command.endlessly();
-
-      scheduler.schedule(perpetual);
-      scheduler.run();
-      scheduler.run();
-      scheduler.run();
-
-      assertTrue(scheduler.isScheduled(perpetual));
-    }
-  }
-
   @Test
   void unlessTest() {
     try (CommandScheduler scheduler = new CommandScheduler()) {

wpilibNewCommands/src/test/java/edu/wpi/first/wpilibj2/command/CommandTestBase.java

@@ -31,7 +31,6 @@ public class CommandTestBase {
 
     DriverStationSim.setEnabled(enabled);
     DriverStationSim.notifyNewData();
-    DriverStation.isNewControlData();
     while (DriverStation.isEnabled() != enabled) {
       try {
         Thread.sleep(1);

wpilibNewCommands/src/test/java/edu/wpi/first/wpilibj2/command/EndlessCommandTest.java

@@ -1,23 +0,0 @@
-// Copyright (c) FIRST and other WPILib contributors.
-// Open Source Software; you can modify and/or share it under the terms of
-// the WPILib BSD license file in the root directory of this project.
-
-package edu.wpi.first.wpilibj2.command;
-
-import static org.junit.jupiter.api.Assertions.assertTrue;
-
-import org.junit.jupiter.api.Test;
-
-class EndlessCommandTest extends CommandTestBase {
-  @Test
-  void endlessCommandScheduleTest() {
-    try (CommandScheduler scheduler = new CommandScheduler()) {
-      EndlessCommand command = new EndlessCommand(new InstantCommand());
-
-      scheduler.schedule(command);
-      scheduler.run();
-
-      assertTrue(scheduler.isScheduled(command));
-    }
-  }
-}