Skip to content

Commit

Permalink
[wpilibc] Remove Alert magic static with map lookup
Browse files Browse the repository at this point in the history
The magic static adds yet another thing that needs to be reset if you
want to run a unit test with a completely reset wpilib state. Use the
existing Sendable static map to store the data instead.

This leaks memory if ResetSmartDashboardInstance is called.
  • Loading branch information
virtuald committed Jan 19, 2025
1 parent 00445f4 commit feaf3a5
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 13 deletions.
21 changes: 11 additions & 10 deletions wpilibc/src/main/native/cpp/Alert.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,17 +82,18 @@ class Alert::SendableAlerts : public nt::NTSendable,
* @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<Alert::SendableAlerts> groups;

auto [iter, exists] = groups.try_emplace(group);
SendableAlerts& sendable = iter->second;
if (!exists) {
frc::SmartDashboard::PutData(group, &iter->second);
SendableAlerts* salert = nullptr;
try {
auto* sendable = frc::SmartDashboard::GetData(group);
salert = dynamic_cast<SendableAlerts*>(sendable);
} catch (frc::RuntimeError&) {
}
return sendable;
if (!salert) {
// this leaks if ResetSmartDashboardInstance is called, but that's fine
salert = new Alert::SendableAlerts;
frc::SmartDashboard::PutData(group, salert);
}
return *salert;
}

private:
Expand Down
14 changes: 11 additions & 3 deletions wpilibc/src/test/native/cpp/AlertTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ class AlertsTest : public ::testing::Test {
std::string GetGroupName() {
const ::testing::TestInfo* testInfo =
::testing::UnitTest::GetInstance()->current_test_info();
return fmt::format("{}_{}", testInfo->test_suite_name(),
testInfo->test_case_name());
return fmt::format("{}_{}", testInfo->test_suite_name(), testInfo->name());
}

template <typename... Args>
Expand Down Expand Up @@ -80,7 +79,16 @@ class AlertsTest : public ::testing::Test {
#define EXPECT_STATE(type, ...) \
EXPECT_EQ(GetActiveAlerts(type), (std::vector<std::string>{__VA_ARGS__}))

TEST_F(AlertsTest, SetUnset) {
TEST_F(AlertsTest, SetUnsetSingle) {
auto one = MakeAlert("one", kInfo);
EXPECT_FALSE(IsAlertActive("one", kInfo));
one.Set(true);
EXPECT_TRUE(IsAlertActive("one", kInfo));
one.Set(false);
EXPECT_FALSE(IsAlertActive("one", kInfo));
}

TEST_F(AlertsTest, SetUnsetMultiple) {
auto one = MakeAlert("one", kError);
auto two = MakeAlert("two", kInfo);
EXPECT_FALSE(IsAlertActive("one", kError));
Expand Down

0 comments on commit feaf3a5

Please sign in to comment.