From 5a16b0e108ca0daedcc520bf4015dae39796fe5f Mon Sep 17 00:00:00 2001 From: Ryan Blue Date: Wed, 6 Nov 2024 18:09:06 -0500 Subject: [PATCH] [wpilib] Refactor Alert (#7279) This refactors Alert in both c++ and java to fix the issues with the current c++ implementation and improve performance. Currently, constructing an Alert adds it to a list of Alerts with the same group and type. Activating an alert sets a flag on the alert. When the SendableAlerts is polled (GetStrings), the entire list is iterated over, filtered, and the filtered list is sorted by timestamp. This leads to a worst case O(m + nlog(n)) time complexity for GetStrings, where m and n are the count of total constructed alerts active alerts respectively. It also allocates intermediate data structures to hold the active alert strings for sorting. This changes the implementation to improve the performance of GetStrings, by shifting the sorting overhead to Alert.Set Constructing the Alert only initializes the alert's initial state, and initializes the SendableAlerts for the group if it is not already initialized. Activating or deactivating an alert sets an internal flag for state tracking, and inserts or removes a structure containing the timestamp and text into a self-sorting data structure (std::set, TreeSet) containing other active alerts with the same group and type. (worst case O(log(n)) Now, SendableAlerts.GetStrings only has to iterate over the structure and copy the strings to the returned array. (amortized O(n)) This also fixes the c++ implementation by removing the need for SendableAlerts to directly access the Alert. This also adds a helper method to SendableRegistry to force initialization of the instance to prevent static initialization ordering issues. --- wpilibc/src/main/native/cpp/Alert.cpp | 192 +++++++++++--- wpilibc/src/main/native/include/frc/Alert.h | 61 +++-- wpilibc/src/test/native/cpp/AlertTest.cpp | 242 +++++++++++++++++ .../java/edu/wpi/first/wpilibj/Alert.java | 249 ++++++++++++------ .../java/edu/wpi/first/wpilibj/AlertTest.java | 225 ++++++++++++++++ .../native/cpp/sendable/SendableRegistry.cpp | 4 + .../include/wpi/sendable/SendableRegistry.h | 6 + 7 files changed, 832 insertions(+), 147 deletions(-) create mode 100644 wpilibc/src/test/native/cpp/AlertTest.cpp create mode 100644 wpilibj/src/test/java/edu/wpi/first/wpilibj/AlertTest.java diff --git a/wpilibc/src/main/native/cpp/Alert.cpp b/wpilibc/src/main/native/cpp/Alert.cpp index c9523fab55..2df38bcd9f 100644 --- a/wpilibc/src/main/native/cpp/Alert.cpp +++ b/wpilibc/src/main/native/cpp/Alert.cpp @@ -4,69 +4,183 @@ #include "frc/Alert.h" -#include +#include #include +#include #include +#include +#include #include +#include +#include +#include -#include "frc/Timer.h" +#include "frc/Errors.h" +#include "frc/RobotController.h" #include "frc/smartdashboard/SmartDashboard.h" using namespace frc; +class Alert::PublishedAlert { + public: + PublishedAlert(uint64_t timestamp, std::string_view text) + : timestamp{timestamp}, text{text} {} + uint64_t timestamp; + std::string text; + auto operator<=>(const PublishedAlert& other) const { + if (timestamp != other.timestamp) { + return other.timestamp <=> timestamp; + } else { + return text <=> other.text; + } + } +}; + +class Alert::SendableAlerts : public nt::NTSendable, + public wpi::SendableHelper { + public: + SendableAlerts() { m_alerts.fill({}); } + + void InitSendable(nt::NTSendableBuilder& builder) override { + builder.SetSmartDashboardType("Alerts"); + builder.AddStringArrayProperty( + "errors", [this]() { return GetStrings(AlertType::kError); }, nullptr); + builder.AddStringArrayProperty( + "warnings", [this]() { return GetStrings(AlertType::kWarning); }, + nullptr); + builder.AddStringArrayProperty( + "infos", [this]() { return GetStrings(AlertType::kInfo); }, nullptr); + } + + /** + * Returns a reference to the set of active alerts for the given type. + * @param type the type + * @return reference to the set of active alerts for the type + */ + std::set& GetActiveAlertsStorage(AlertType type) { + return const_cast&>( + std::as_const(*this).GetActiveAlertsStorage(type)); + } + + const std::set& GetActiveAlertsStorage(AlertType type) const { + switch (type) { + case AlertType::kInfo: + case AlertType::kWarning: + case AlertType::kError: + return m_alerts[static_cast(type)]; + default: + throw FRC_MakeError(frc::err::InvalidParameter, + "Invalid Alert Type: {}", type); + } + } + + /** + * Returns the SendableAlerts for a given group, initializing and publishing + * if it does not already exist. + * @param group the group name + * @return the SendableAlerts for the group + */ + static SendableAlerts& ForGroup(std::string_view group) { + // Force initialization of SendableRegistry before our magic static to + // prevent incorrect destruction order. + wpi::SendableRegistry::EnsureInitialized(); + static wpi::StringMap groups; + + auto [iter, exists] = groups.try_emplace(group); + SendableAlerts& sendable = iter->second; + if (!exists) { + frc::SmartDashboard::PutData(group, &iter->second); + } + return sendable; + } + + private: + std::vector GetStrings(AlertType type) const { + auto& set = GetActiveAlertsStorage(type); + std::vector output; + output.reserve(set.size()); + for (auto& alert : set) { + output.emplace_back(alert.text); + } + return output; + } + + std::array, 3> m_alerts; +}; + Alert::Alert(std::string_view text, AlertType type) : Alert("Alerts", text, type) {} -wpi::StringMap Alert::groups; - Alert::Alert(std::string_view group, std::string_view text, AlertType type) - : m_type(type), m_text(text) { - if (!groups.contains(group)) { - groups[group] = SendableAlerts(); - frc::SmartDashboard::PutData(group, &groups[group]); + : m_type(type), + m_text(text), + m_activeAlerts{ + &SendableAlerts::ForGroup(group).GetActiveAlertsStorage(m_type)} {} + +Alert::Alert(Alert&& other) + : m_type{other.m_type}, + m_text{std::move(other.m_text)}, + m_activeAlerts{std::exchange(other.m_activeAlerts, nullptr)}, + m_active{std::exchange(other.m_active, false)}, + m_activeStartTime{other.m_activeStartTime} {} + +Alert& Alert::operator=(Alert&& other) { + if (&other != this) { + // We want to destroy current state after the move is done + Alert tmp{std::move(*this)}; + // Now, swap moved-from state with other state + std::swap(m_type, other.m_type); + std::swap(m_text, other.m_text); + std::swap(m_activeAlerts, other.m_activeAlerts); + std::swap(m_active, other.m_active); + std::swap(m_activeStartTime, other.m_activeStartTime); } - groups[group].m_alerts.push_back(std::shared_ptr(this)); + return *this; +} + +Alert::~Alert() { + Set(false); } void Alert::Set(bool active) { - if (active && !m_active) { - m_activeStartTime = frc::Timer::GetFPGATimestamp(); + if (active == m_active) { + return; + } + + if (active) { + m_activeStartTime = frc::RobotController::GetFPGATime(); + m_activeAlerts->emplace(m_activeStartTime, m_text); + } else { + m_activeAlerts->erase({m_activeStartTime, m_text}); } m_active = active; } void Alert::SetText(std::string_view text) { + if (text == m_text) { + return; + } + + std::string oldText = std::move(m_text); m_text = text; + + if (m_active) { + auto iter = m_activeAlerts->find({m_activeStartTime, oldText}); + auto hint = m_activeAlerts->erase(iter); + m_activeAlerts->emplace_hint(hint, m_activeStartTime, m_text); + } } -void Alert::SendableAlerts::InitSendable(nt::NTSendableBuilder& builder) { - builder.SetSmartDashboardType("Alerts"); - builder.AddStringArrayProperty( - "errors", [this]() { return GetStrings(AlertType::kError); }, nullptr); - builder.AddStringArrayProperty( - "warnings", [this]() { return GetStrings(AlertType::kWarning); }, - nullptr); - builder.AddStringArrayProperty( - "infos", [this]() { return GetStrings(AlertType::kInfo); }, nullptr); -} - -std::vector Alert::SendableAlerts::GetStrings( - AlertType type) const { - wpi::SmallVector> alerts; - alerts.reserve(m_alerts.size()); - for (auto alert : m_alerts) { - if (alert->m_active && alert->m_type == type) { - alerts.push_back(alert); - } +std::string frc::format_as(Alert::AlertType type) { + switch (type) { + case Alert::AlertType::kInfo: + return "kInfo"; + case Alert::AlertType::kWarning: + return "kWarning"; + case Alert::AlertType::kError: + return "kError"; + default: + return std::to_string(static_cast(type)); } - std::sort(alerts.begin(), alerts.end(), [](const auto a, const auto b) { - return a->m_activeStartTime > b->m_activeStartTime; - }); - std::vector output{alerts.size()}; - for (unsigned int i = 0; i < alerts.size(); ++i) { - std::string text{alerts[i]->m_text}; - output[i] = text; - } - return output; } diff --git a/wpilibc/src/main/native/include/frc/Alert.h b/wpilibc/src/main/native/include/frc/Alert.h index dde95306e0..d51eba2b76 100644 --- a/wpilibc/src/main/native/include/frc/Alert.h +++ b/wpilibc/src/main/native/include/frc/Alert.h @@ -4,15 +4,10 @@ #pragma once -#include -#include -#include +#include -#include -#include -#include -#include -#include +#include +#include namespace frc { @@ -21,7 +16,7 @@ namespace frc { * of kError, kWarning, or kInfo to denote urgency. See Alert::AlertType for * suggested usage of each type. Alerts can be displayed on supported * dashboards, and are shown in a priority order based on type and recency of - * activation. + * activation, with newly activated alerts first. * * Alerts should be created once and stored persistently, then updated to * "active" or "inactive" as necessary. Set(bool) can be safely called @@ -88,6 +83,14 @@ class Alert { */ Alert(std::string_view group, std::string_view text, AlertType type); + Alert(Alert&&); + Alert& operator=(Alert&&); + + Alert(const Alert&) = default; + Alert& operator=(const Alert&) = default; + + ~Alert(); + /** * Sets whether the alert should currently be displayed. This method can be * safely called periodically. @@ -96,6 +99,12 @@ class Alert { */ void Set(bool active); + /** + * Gets whether the alert is active. + * @return whether the alert is active. + */ + bool Get() const { return m_active; } + /** * Updates current alert text. Use this method to dynamically change the * displayed alert, such as including more details about the detected problem. @@ -104,23 +113,29 @@ class Alert { */ void SetText(std::string_view text); + /** + * Gets the current alert text. + * @return the current text. + */ + std::string GetText() const { return m_text; } + + /** + * Get the type of this alert. + * @return the type + */ + AlertType GetType() const { return m_type; } + private: + class PublishedAlert; + class SendableAlerts; + AlertType m_type; + std::string m_text; + std::set* m_activeAlerts; bool m_active = false; - units::second_t m_activeStartTime; - std::string_view m_text; - - class SendableAlerts : public nt::NTSendable, - public wpi::SendableHelper { - public: - wpi::SmallVector> m_alerts; - void InitSendable(nt::NTSendableBuilder& builder) override; - - private: - std::vector GetStrings(AlertType type) const; - }; - - static wpi::StringMap groups; + uint64_t m_activeStartTime; }; +std::string format_as(Alert::AlertType type); + } // namespace frc diff --git a/wpilibc/src/test/native/cpp/AlertTest.cpp b/wpilibc/src/test/native/cpp/AlertTest.cpp new file mode 100644 index 0000000000..a02e0166a8 --- /dev/null +++ b/wpilibc/src/test/native/cpp/AlertTest.cpp @@ -0,0 +1,242 @@ +// 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 +#include +#include +#include +#include +#include + +#include +#include +#include +#include + +#include "frc/Alert.h" +#include "frc/simulation/SimHooks.h" +#include "frc/smartdashboard/SmartDashboard.h" + +using namespace frc; +using enum Alert::AlertType; +class AlertsTest : public ::testing::Test { + public: + ~AlertsTest() { + // test all destructors + Update(); + EXPECT_EQ(GetSubscriberForType(kError).Get().size(), 0ul); + EXPECT_EQ(GetSubscriberForType(kWarning).Get().size(), 0ul); + EXPECT_EQ(GetSubscriberForType(kInfo).Get().size(), 0ul); + } + + std::string GetGroupName() { + const ::testing::TestInfo* testInfo = + ::testing::UnitTest::GetInstance()->current_test_info(); + return fmt::format("{}_{}", testInfo->test_suite_name(), + testInfo->test_case_name()); + } + + template + Alert MakeAlert(Args&&... args) { + return Alert(GetGroupName(), std::forward(args)...); + } + + std::vector GetActiveAlerts(Alert::AlertType type) { + Update(); + return GetSubscriberForType(type).Get(); + } + + bool IsAlertActive(std::string_view text, Alert::AlertType type) { + auto activeAlerts = GetActiveAlerts(type); + return std::find(activeAlerts.begin(), activeAlerts.end(), text) != + activeAlerts.end(); + } + + void Update() { frc::SmartDashboard::UpdateValues(); } + + private: + std::string GetSubtableName(Alert::AlertType type) { + switch (type) { + case kError: + return "errors"; + case kWarning: + return "warnings"; + case kInfo: + return "infos"; + default: + return "unknown"; + } + } + + const nt::StringArraySubscriber GetSubscriberForType(Alert::AlertType type) { + return nt::NetworkTableInstance::GetDefault() + .GetStringArrayTopic(fmt::format("/SmartDashboard/{}/{}", + GetGroupName(), GetSubtableName(type))) + .Subscribe({}); + } +}; + +#define EXPECT_STATE(type, ...) \ + EXPECT_EQ(GetActiveAlerts(type), (std::vector{__VA_ARGS__})) + +TEST_F(AlertsTest, SetUnset) { + auto one = MakeAlert("one", kError); + auto two = MakeAlert("two", kInfo); + EXPECT_FALSE(IsAlertActive("one", kError)); + EXPECT_FALSE(IsAlertActive("two", kInfo)); + one.Set(true); + EXPECT_TRUE(IsAlertActive("one", kError)); + EXPECT_FALSE(IsAlertActive("two", kInfo)); + one.Set(true); + two.Set(true); + EXPECT_TRUE(IsAlertActive("one", kError)); + EXPECT_TRUE(IsAlertActive("two", kInfo)); + one.Set(false); + EXPECT_FALSE(IsAlertActive("one", kError)); + EXPECT_TRUE(IsAlertActive("two", kInfo)); +} + +TEST_F(AlertsTest, SetIsIdempotent) { + auto a = MakeAlert("A", kInfo); + auto b = MakeAlert("B", kInfo); + auto c = MakeAlert("C", kInfo); + a.Set(true); + + b.Set(true); + c.Set(true); + + const auto startState = GetActiveAlerts(kInfo); + + b.Set(true); + EXPECT_STATE(kInfo, startState); + + a.Set(true); + EXPECT_STATE(kInfo, startState); +} + +TEST_F(AlertsTest, DestructorUnsetsAlert) { + { + auto alert = MakeAlert("alert", kWarning); + alert.Set(true); + EXPECT_TRUE(IsAlertActive("alert", kWarning)); + } + EXPECT_FALSE(IsAlertActive("alert", kWarning)); +} + +TEST_F(AlertsTest, SetTextWhileUnset) { + auto alert = MakeAlert("BEFORE", kInfo); + EXPECT_EQ("BEFORE", alert.GetText()); + alert.Set(true); + EXPECT_TRUE(IsAlertActive("BEFORE", kInfo)); + alert.Set(false); + EXPECT_FALSE(IsAlertActive("BEFORE", kInfo)); + alert.SetText("AFTER"); + EXPECT_EQ("AFTER", alert.GetText()); + alert.Set(true); + EXPECT_FALSE(IsAlertActive("BEFORE", kInfo)); + EXPECT_TRUE(IsAlertActive("AFTER", kInfo)); +} + +TEST_F(AlertsTest, SetTextWhileSet) { + auto alert = MakeAlert("BEFORE", kInfo); + EXPECT_EQ("BEFORE", alert.GetText()); + alert.Set(true); + EXPECT_TRUE(IsAlertActive("BEFORE", kInfo)); + alert.SetText("AFTER"); + EXPECT_EQ("AFTER", alert.GetText()); + EXPECT_FALSE(IsAlertActive("BEFORE", kInfo)); + EXPECT_TRUE(IsAlertActive("AFTER", kInfo)); +} + +TEST_F(AlertsTest, SetTextDoesNotAffectFirstOrderSort) { + frc::sim::PauseTiming(); + + auto a = MakeAlert("A", kError); + auto b = MakeAlert("B", kError); + auto c = MakeAlert("C", kError); + + a.Set(true); + frc::sim::StepTiming(1_s); + b.Set(true); + frc::sim::StepTiming(1_s); + c.Set(true); + + auto expectedEndState = GetActiveAlerts(kError); + std::replace(expectedEndState.begin(), expectedEndState.end(), + std::string("B"), std::string("AFTER")); + b.SetText("AFTER"); + + EXPECT_STATE(kError, expectedEndState); + frc::sim::ResumeTiming(); +} + +TEST_F(AlertsTest, MoveAssign) { + auto outer = MakeAlert("outer", kInfo); + outer.Set(true); + EXPECT_TRUE(IsAlertActive("outer", kInfo)); + + { + auto inner = MakeAlert("inner", kWarning); + inner.Set(true); + EXPECT_TRUE(IsAlertActive("inner", kWarning)); + outer = std::move(inner); + // Assignment target should be unset and invalidated as part of move, before + // destruction + EXPECT_FALSE(IsAlertActive("outer", kInfo)); + } + EXPECT_TRUE(IsAlertActive("inner", kWarning)); +} + +TEST_F(AlertsTest, MoveConstruct) { + auto a = MakeAlert("A", kInfo); + a.Set(true); + EXPECT_TRUE(IsAlertActive("A", kInfo)); + Alert b{std::move(a)}; + EXPECT_TRUE(IsAlertActive("A", kInfo)); + b.Set(false); + EXPECT_FALSE(IsAlertActive("A", kInfo)); + b.Set(true); + EXPECT_TRUE(IsAlertActive("A", kInfo)); +} + +TEST_F(AlertsTest, SortOrder) { + frc::sim::PauseTiming(); + auto a = MakeAlert("A", kInfo); + auto b = MakeAlert("B", kInfo); + auto c = MakeAlert("C", kInfo); + a.Set(true); + EXPECT_STATE(kInfo, "A"); + frc::sim::StepTiming(1_s); + b.Set(true); + EXPECT_STATE(kInfo, "B", "A"); + frc::sim::StepTiming(1_s); + c.Set(true); + EXPECT_STATE(kInfo, "C", "B", "A"); + + frc::sim::StepTiming(1_s); + c.Set(false); + EXPECT_STATE(kInfo, "B", "A"); + + frc::sim::StepTiming(1_s); + c.Set(true); + EXPECT_STATE(kInfo, "C", "B", "A"); + + frc::sim::StepTiming(1_s); + a.Set(false); + EXPECT_STATE(kInfo, "C", "B"); + + frc::sim::StepTiming(1_s); + b.Set(false); + EXPECT_STATE(kInfo, "C"); + + frc::sim::StepTiming(1_s); + b.Set(true); + EXPECT_STATE(kInfo, "B", "C"); + + frc::sim::StepTiming(1_s); + a.Set(true); + EXPECT_STATE(kInfo, "A", "B", "C"); + + frc::sim::ResumeTiming(); +} diff --git a/wpilibj/src/main/java/edu/wpi/first/wpilibj/Alert.java b/wpilibj/src/main/java/edu/wpi/first/wpilibj/Alert.java index 1e57a993c9..faeda9aaf7 100644 --- a/wpilibj/src/main/java/edu/wpi/first/wpilibj/Alert.java +++ b/wpilibj/src/main/java/edu/wpi/first/wpilibj/Alert.java @@ -7,17 +7,18 @@ package edu.wpi.first.wpilibj; import edu.wpi.first.util.sendable.Sendable; import edu.wpi.first.util.sendable.SendableBuilder; import edu.wpi.first.wpilibj.smartdashboard.SmartDashboard; -import java.util.Collection; +import java.util.Comparator; import java.util.HashMap; -import java.util.HashSet; import java.util.Map; +import java.util.Set; +import java.util.TreeSet; /** * Persistent alert to be sent via NetworkTables. Alerts are tagged with a type of {@code kError}, * {@code kWarning}, or {@code kInfo} to denote urgency. See {@link * edu.wpi.first.wpilibj.Alert.AlertType AlertType} for suggested usage of each type. Alerts can be * displayed on supported dashboards, and are shown in a priority order based on type and recency of - * activation. + * activation, with newly activated alerts first. * *

Alerts should be created once and stored persistently, then updated to "active" or "inactive" * as necessary. {@link #set(boolean)} can be safely called periodically. @@ -43,88 +44,7 @@ import java.util.Map; * } * */ -public class Alert { - private static Map groups = new HashMap(); - - private final AlertType m_type; - private boolean m_active; - private double m_activeStartTime; - private String m_text; - - /** - * Creates a new alert in the default group - "Alerts". If this is the first to be instantiated, - * the appropriate entries will be added to NetworkTables. - * - * @param text Text to be displayed when the alert is active. - * @param type Alert urgency level. - */ - public Alert(String text, AlertType type) { - this("Alerts", text, type); - } - - /** - * Creates a new alert. If this is the first to be instantiated in its group, the appropriate - * entries will be added to NetworkTables. - * - * @param group Group identifier, used as the entry name in NetworkTables. - * @param text Text to be displayed when the alert is active. - * @param type Alert urgency level. - */ - @SuppressWarnings("this-escape") - public Alert(String group, String text, AlertType type) { - if (!groups.containsKey(group)) { - groups.put(group, new SendableAlerts()); - SmartDashboard.putData(group, groups.get(group)); - } - - m_text = text; - m_type = type; - groups.get(group).m_alerts.add(this); - } - - /** - * Sets whether the alert should currently be displayed. This method can be safely called - * periodically. - * - * @param active Whether to display the alert. - */ - public void set(boolean active) { - if (active && !m_active) { - m_activeStartTime = Timer.getFPGATimestamp(); - } - m_active = active; - } - - /** - * Updates current alert text. Use this method to dynamically change the displayed alert, such as - * including more details about the detected problem. - * - * @param text Text to be displayed when the alert is active. - */ - public void setText(String text) { - m_text = text; - } - - private static final class SendableAlerts implements Sendable { - private final Collection m_alerts = new HashSet<>(); - - private String[] getStrings(AlertType type) { - return m_alerts.stream() - .filter((Alert a) -> a.m_active && a.m_type == type) - .sorted((Alert a, Alert b) -> Double.compare(b.m_activeStartTime, a.m_activeStartTime)) - .map((Alert a) -> a.m_text) - .toArray(String[]::new); - } - - @Override - public void initSendable(SendableBuilder builder) { - builder.setSmartDashboardType("Alerts"); - builder.addStringArrayProperty("errors", () -> getStrings(AlertType.kError), null); - builder.addStringArrayProperty("warnings", () -> getStrings(AlertType.kWarning), null); - builder.addStringArrayProperty("infos", () -> getStrings(AlertType.kInfo), null); - } - } - +public class Alert implements AutoCloseable { /** Represents an alert's level of urgency. */ public enum AlertType { /** @@ -148,4 +68,163 @@ public class Alert { */ kInfo } + + private final AlertType m_type; + private boolean m_active; + private long m_activeStartTime; + private String m_text; + private Set m_activeAlerts; + + /** + * Creates a new alert in the default group - "Alerts". If this is the first to be instantiated, + * the appropriate entries will be added to NetworkTables. + * + * @param text Text to be displayed when the alert is active. + * @param type Alert urgency level. + */ + public Alert(String text, AlertType type) { + this("Alerts", text, type); + } + + /** + * Creates a new alert. If this is the first to be instantiated in its group, the appropriate + * entries will be added to NetworkTables. + * + * @param group Group identifier, used as the entry name in NetworkTables. + * @param text Text to be displayed when the alert is active. + * @param type Alert urgency level. + */ + @SuppressWarnings("this-escape") + public Alert(String group, String text, AlertType type) { + m_type = type; + m_text = text; + m_activeAlerts = SendableAlerts.forGroup(group).getActiveAlertsStorage(type); + } + + /** + * Sets whether the alert should currently be displayed. This method can be safely called + * periodically. + * + * @param active Whether to display the alert. + */ + public void set(boolean active) { + if (active == m_active) { + return; + } + + if (active) { + m_activeStartTime = RobotController.getFPGATime(); + m_activeAlerts.add(new PublishedAlert(m_activeStartTime, m_text)); + } else { + m_activeAlerts.remove(new PublishedAlert(m_activeStartTime, m_text)); + } + m_active = active; + } + + /** + * Gets whether the alert is active. + * + * @return whether the alert is active. + */ + public boolean get() { + return m_active; + } + + /** + * Updates current alert text. Use this method to dynamically change the displayed alert, such as + * including more details about the detected problem. + * + * @param text Text to be displayed when the alert is active. + */ + public void setText(String text) { + if (text.equals(m_text)) { + return; + } + var oldText = m_text; + m_text = text; + if (m_active) { + m_activeAlerts.remove(new PublishedAlert(m_activeStartTime, oldText)); + m_activeAlerts.add(new PublishedAlert(m_activeStartTime, m_text)); + } + } + + /** + * Gets the current alert text. + * + * @return the current text. + */ + public String getText() { + return m_text; + } + + /** + * Get the type of this alert. + * + * @return the type + */ + public AlertType getType() { + return m_type; + } + + @Override + public void close() { + set(false); + } + + private record PublishedAlert(long timestamp, String text) implements Comparable { + private static final Comparator comparator = + Comparator.comparingLong((PublishedAlert alert) -> alert.timestamp()) + .reversed() + .thenComparing(Comparator.comparing((PublishedAlert alert) -> alert.text())); + + @Override + public int compareTo(PublishedAlert o) { + return comparator.compare(this, o); + } + } + + private static final class SendableAlerts implements Sendable { + private static final Map groups = new HashMap(); + + private final Map> m_alerts = new HashMap<>(); + + /** + * Returns a reference to the set of active alerts for the given type. + * + * @param type the type + * @return reference to the set of active alerts for the type + */ + public Set getActiveAlertsStorage(AlertType type) { + return m_alerts.computeIfAbsent(type, _type -> new TreeSet<>()); + } + + private String[] getStrings(AlertType type) { + return getActiveAlertsStorage(type).stream().map(a -> a.text()).toArray(String[]::new); + } + + @Override + public void initSendable(SendableBuilder builder) { + builder.setSmartDashboardType("Alerts"); + builder.addStringArrayProperty("errors", () -> getStrings(AlertType.kError), null); + builder.addStringArrayProperty("warnings", () -> getStrings(AlertType.kWarning), null); + builder.addStringArrayProperty("infos", () -> getStrings(AlertType.kInfo), null); + } + + /** + * Returns the SendableAlerts for a given group, initializing and publishing if it does not + * already exist. + * + * @param group the group name + * @return the SendableAlerts for the group + */ + private static SendableAlerts forGroup(String group) { + return groups.computeIfAbsent( + group, + _group -> { + var sendable = new SendableAlerts(); + SmartDashboard.putData(_group, sendable); + return sendable; + }); + } + } } diff --git a/wpilibj/src/test/java/edu/wpi/first/wpilibj/AlertTest.java b/wpilibj/src/test/java/edu/wpi/first/wpilibj/AlertTest.java new file mode 100644 index 0000000000..3e1f9b3abd --- /dev/null +++ b/wpilibj/src/test/java/edu/wpi/first/wpilibj/AlertTest.java @@ -0,0 +1,225 @@ +// 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.wpilibj; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import edu.wpi.first.networktables.NetworkTableInstance; +import edu.wpi.first.networktables.StringArraySubscriber; +import edu.wpi.first.wpilibj.Alert.AlertType; +import edu.wpi.first.wpilibj.simulation.SimHooks; +import edu.wpi.first.wpilibj.smartdashboard.SmartDashboard; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInfo; +import org.junit.jupiter.api.parallel.ResourceLock; + +class AlertTest { + String m_groupName; + + @BeforeEach + void setup(TestInfo info) { + m_groupName = "AlertTest_" + info.getDisplayName(); + } + + @AfterEach + void checkClean() { + update(); + assertEquals(0, getActiveAlerts(AlertType.kError).length); + assertEquals(0, getActiveAlerts(AlertType.kWarning).length); + assertEquals(0, getActiveAlerts(AlertType.kInfo).length); + } + + private String getSubtableName(Alert.AlertType type) { + switch (type) { + case kError: + return "errors"; + case kWarning: + return "warnings"; + case kInfo: + return "infos"; + default: + return "unknown"; + } + } + + private StringArraySubscriber getSubscriberForType(Alert.AlertType type) { + return NetworkTableInstance.getDefault() + .getStringArrayTopic("/SmartDashboard/" + m_groupName + "/" + getSubtableName(type)) + .subscribe(new String[] {}); + } + + private String[] getActiveAlerts(AlertType type) { + update(); + try (var sub = getSubscriberForType(type)) { + return sub.get(); + } + } + + private void update() { + SmartDashboard.updateValues(); + } + + private boolean isAlertActive(String text, Alert.AlertType type) { + return Arrays.asList(getActiveAlerts(type)).contains(text); + } + + private void assertState(Alert.AlertType type, List state) { + assertEquals(state, Arrays.asList(getActiveAlerts(type))); + } + + private Alert makeAlert(String text, Alert.AlertType type) { + return new Alert(m_groupName, text, type); + } + + @Test + void setUnset() { + try (var one = makeAlert("one", AlertType.kError); + var two = makeAlert("two", AlertType.kInfo)) { + assertFalse(isAlertActive("one", AlertType.kError)); + assertFalse(isAlertActive("two", AlertType.kInfo)); + one.set(true); + assertTrue(isAlertActive("one", AlertType.kError)); + assertFalse(isAlertActive("two", AlertType.kInfo)); + one.set(true); + two.set(true); + assertTrue(isAlertActive("one", AlertType.kError)); + assertTrue(isAlertActive("two", AlertType.kInfo)); + one.set(false); + assertFalse(isAlertActive("one", AlertType.kError)); + assertTrue(isAlertActive("two", AlertType.kInfo)); + } + } + + @Test + void setIsIdempotent() { + try (var a = makeAlert("A", AlertType.kInfo); + var b = makeAlert("B", AlertType.kInfo); + var c = makeAlert("C", AlertType.kInfo)) { + a.set(true); + b.set(true); + c.set(true); + + var startState = List.of(getActiveAlerts(AlertType.kInfo)); + + b.set(true); + assertState(AlertType.kInfo, startState); + + a.set(true); + assertState(AlertType.kInfo, startState); + } + } + + @Test + void closeUnsetsAlert() { + try (var alert = makeAlert("alert", AlertType.kWarning)) { + alert.set(true); + assertTrue(isAlertActive("alert", AlertType.kWarning)); + } + assertFalse(isAlertActive("alert", AlertType.kWarning)); + } + + @Test + void setTextWhileUnset() { + try (var alert = makeAlert("BEFORE", AlertType.kInfo)) { + assertEquals("BEFORE", alert.getText()); + alert.set(true); + assertTrue(isAlertActive("BEFORE", AlertType.kInfo)); + alert.set(false); + assertFalse(isAlertActive("BEFORE", AlertType.kInfo)); + alert.setText("AFTER"); + assertEquals("AFTER", alert.getText()); + alert.set(true); + assertFalse(isAlertActive("BEFORE", AlertType.kInfo)); + assertTrue(isAlertActive("AFTER", AlertType.kInfo)); + } + } + + @Test + void setTextWhileSet() { + try (var alert = makeAlert("BEFORE", AlertType.kInfo)) { + assertEquals("BEFORE", alert.getText()); + alert.set(true); + assertTrue(isAlertActive("BEFORE", AlertType.kInfo)); + alert.setText("AFTER"); + assertEquals("AFTER", alert.getText()); + assertFalse(isAlertActive("BEFORE", AlertType.kInfo)); + assertTrue(isAlertActive("AFTER", AlertType.kInfo)); + } + } + + @ResourceLock("timing") + @Test + void setTextDoesNotAffectFirstOrderSort() { + SimHooks.pauseTiming(); + try (var a = makeAlert("A", AlertType.kInfo); + var b = makeAlert("B", AlertType.kInfo); + var c = makeAlert("C", AlertType.kInfo)) { + a.set(true); + SimHooks.stepTiming(1); + b.set(true); + SimHooks.stepTiming(1); + c.set(true); + + var expectedEndState = new ArrayList<>(List.of(getActiveAlerts(AlertType.kInfo))); + expectedEndState.replaceAll(s -> "B".equals(s) ? "AFTER" : s); + b.setText("AFTER"); + + assertState(AlertType.kInfo, expectedEndState); + } finally { + SimHooks.resumeTiming(); + } + } + + @ResourceLock("timing") + @Test + void sortOrder() { + SimHooks.pauseTiming(); + try (var a = makeAlert("A", AlertType.kInfo); + var b = makeAlert("B", AlertType.kInfo); + var c = makeAlert("C", AlertType.kInfo)) { + a.set(true); + assertState(AlertType.kInfo, List.of("A")); + SimHooks.stepTiming(1); + b.set(true); + assertState(AlertType.kInfo, List.of("B", "A")); + SimHooks.stepTiming(1); + c.set(true); + assertState(AlertType.kInfo, List.of("C", "B", "A")); + + SimHooks.stepTiming(1); + c.set(false); + assertState(AlertType.kInfo, List.of("B", "A")); + + SimHooks.stepTiming(1); + c.set(true); + assertState(AlertType.kInfo, List.of("C", "B", "A")); + + SimHooks.stepTiming(1); + a.set(false); + assertState(AlertType.kInfo, List.of("C", "B")); + + SimHooks.stepTiming(1); + b.set(false); + assertState(AlertType.kInfo, List.of("C")); + + SimHooks.stepTiming(1); + b.set(true); + assertState(AlertType.kInfo, List.of("B", "C")); + + SimHooks.stepTiming(1); + a.set(true); + assertState(AlertType.kInfo, List.of("A", "B", "C")); + } finally { + SimHooks.resumeTiming(); + } + } +} diff --git a/wpiutil/src/main/native/cpp/sendable/SendableRegistry.cpp b/wpiutil/src/main/native/cpp/sendable/SendableRegistry.cpp index c2b6b8ede7..99ff7f5e1a 100644 --- a/wpiutil/src/main/native/cpp/sendable/SendableRegistry.cpp +++ b/wpiutil/src/main/native/cpp/sendable/SendableRegistry.cpp @@ -81,6 +81,10 @@ void ResetSendableRegistry() { } // namespace wpi::impl #endif +void SendableRegistry::EnsureInitialized() { + GetInstance(); +} + void SendableRegistry::SetLiveWindowBuilderFactory( std::function()> factory) { GetInstance().liveWindowFactory = std::move(factory); diff --git a/wpiutil/src/main/native/include/wpi/sendable/SendableRegistry.h b/wpiutil/src/main/native/include/wpi/sendable/SendableRegistry.h index 463974930b..37ed45fe19 100644 --- a/wpiutil/src/main/native/include/wpi/sendable/SendableRegistry.h +++ b/wpiutil/src/main/native/include/wpi/sendable/SendableRegistry.h @@ -26,6 +26,12 @@ class SendableRegistry final { using UID = size_t; + /** + * Initializes the SendableRegistry. This is used to ensure initialization and + * destruction order relative to Sendables. + */ + static void EnsureInitialized(); + /** * Sets the factory for LiveWindow builders. *