Skip to content

Commit

Permalink
Fix issues found in testing
Browse files Browse the repository at this point in the history
- move constructor now invalidates old state
- SetText gets a reference of the set, not a copy
  • Loading branch information
rzblue committed Oct 30, 2024
1 parent 7574bb7 commit 6d9b55e
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 5 deletions.
23 changes: 22 additions & 1 deletion wpilibc/src/main/native/cpp/Alert.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,27 @@ Alert::Alert(std::string_view text, AlertType type)
Alert::Alert(std::string_view group, std::string_view text, AlertType type)
: m_type(type), m_text(text), m_group{&GetGroupSendable(group)} {}

Alert::Alert(Alert&& other)
: m_type{other.m_type},
m_text{std::move(other.m_text)},
m_group{std::exchange(other.m_group, 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_group, other.m_group);
std::swap(m_active, other.m_active);
std::swap(m_activeStartTime, other.m_activeStartTime);
}
return *this;
}

Alert::~Alert() {
Set(false);
}
Expand Down Expand Up @@ -54,7 +75,7 @@ void Alert::SetText(std::string_view text) {
m_text = text;

if (m_active) {
auto set = m_group->GetSetForType(m_type);
auto& set = m_group->GetSetForType(m_type);
auto iter = set.find({m_activeStartTime, oldText});
auto hint = set.erase(iter);
set.emplace_hint(hint, m_activeStartTime, m_text);
Expand Down
15 changes: 11 additions & 4 deletions wpilibc/src/main/native/include/frc/Alert.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,8 @@ class Alert {
*/
Alert(std::string_view group, std::string_view text, AlertType type);

Alert(Alert&&) = default;
Alert& operator=(Alert&&) = default;
Alert(Alert&&);
Alert& operator=(Alert&&);

~Alert();

Expand Down Expand Up @@ -130,6 +130,8 @@ 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:
Expand All @@ -139,11 +141,16 @@ class Alert {

AlertType m_type;
std::string m_text;
SendableAlerts* m_group;

SendableAlerts* m_group; // could replace with pointer to type set
bool m_active = false;
uint64_t m_activeStartTime;

/**
* 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& GetGroupSendable(std::string_view group);
};

Expand Down

0 comments on commit 6d9b55e

Please sign in to comment.