Some small cleanups

rzblue · Nov 1, 2024 · b638835 · b638835
1 parent 1786f15
commit b638835
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 29 deletions.
diff --git a/wpilibc/src/main/native/cpp/Alert.cpp b/wpilibc/src/main/native/cpp/Alert.cpp
@@ -159,6 +159,6 @@ std::string frc::format_as(Alert::AlertType type) {
     case Alert::AlertType::kError:
       return "kError";
     default:
-      return fmt::format("{}", fmt::underlying(type));
+      return std::to_string(static_cast<int>(type));
   }
 }
diff --git a/wpilibc/src/main/native/include/frc/Alert.h b/wpilibc/src/main/native/include/frc/Alert.h
@@ -127,11 +127,10 @@ class Alert {
  private:
   class PublishedAlert {
    public:
-    PublishedAlert(uint64_t timestamp, const std::string_view text)
+    PublishedAlert(uint64_t timestamp, std::string_view text)
         : timestamp{timestamp}, text{text} {}
     uint64_t timestamp;
-    std::string text;  // todo: This could be a string_view since deleting the
-                       // Alert will erase this from the set
+    std::string text;
     auto operator<=>(const PublishedAlert&) const = default;
   };
 
@@ -142,8 +141,6 @@ class Alert {
     void InitSendable(nt::NTSendableBuilder& builder) override;
 
     std::set<PublishedAlert>& GetSetForType(AlertType type);
-    // todo: not sure why i needed this, maybe when i was testing different
-    // container types?
     const std::set<PublishedAlert>& GetSetForType(AlertType type) const;
 
    private:

diff --git a/wpilibc/src/test/native/cpp/AlertTest.cpp b/wpilibc/src/test/native/cpp/AlertTest.cpp
@@ -28,20 +28,17 @@ class AlertsTest : public ::testing::Test {
     EXPECT_EQ(GetSubscriberForType(kInfo).Get().size(), 0ul);
   }
 
-  template <typename... Args>
-  Alert MakeAlert(Args&&... args) {
-    return Alert(GetGroupName(), std::forward<Args>(args)...);
-  }
-
   std::string GetGroupName() {
     const ::testing::TestInfo* testInfo =
         ::testing::UnitTest::GetInstance()->current_test_info();
     return fmt::format("{}_{}", testInfo->test_suite_name(),
                        testInfo->test_case_name());
   }
 
-  // todo: this
-  void ExpectAlertsState() {}
+  template <typename... Args>
+  Alert MakeAlert(Args&&... args) {
+    return Alert(GetGroupName(), std::forward<Args>(args)...);
+  }
 
   bool IsAlertActive(std::string_view text, Alert::AlertType type) {
     Update();
@@ -66,20 +63,11 @@ class AlertsTest : public ::testing::Test {
     }
   }
 
-  std::map<Alert::AlertType, nt::StringArraySubscriber> m_subs;
-  const nt::StringArraySubscriber& GetSubscriberForType(Alert::AlertType type) {
-    if (m_subs.contains(type)) {
-      return m_subs[type];
-    } else {
-      return m_subs
-          .emplace(std::make_pair(
-              type, nt::NetworkTableInstance::GetDefault()
-                        .GetStringArrayTopic(
-                            fmt::format("/SmartDashboard/{}/{}", GetGroupName(),
-                                        GetSubtableName(type)))
-                        .Subscribe({})))
-          .first->second;
-    }
+  const nt::StringArraySubscriber GetSubscriberForType(Alert::AlertType type) {
+    return nt::NetworkTableInstance::GetDefault()
+        .GetStringArrayTopic(fmt::format("/SmartDashboard/{}/{}",
+                                         GetGroupName(), GetSubtableName(type)))
+        .Subscribe({});
   }
 };
 

diff --git a/wpilibj/src/main/java/edu/wpi/first/wpilibj/Alert.java b/wpilibj/src/main/java/edu/wpi/first/wpilibj/Alert.java
@@ -147,7 +147,7 @@ public void setText(String text) {
     if (m_active) {
       var set = m_group.getSetForType(m_type);
       set.remove(
-          new PublishedAlert(m_activeStartTime, oldText)); // TODO: cache instead of constructing
+          new PublishedAlert(m_activeStartTime, oldText));
       set.add(new PublishedAlert(m_activeStartTime, m_text));
     }
   }
@@ -195,7 +195,7 @@ public void initSendable(SendableBuilder builder) {
   }
 
   @Override
-  public void close() throws Exception {
+  public void close() {
     set(false);
   }