[hal] Use atomic rather then mutex for DS Data updates (#4787)

Using an atomic here means we are never going against a lock that is touchable from user code. That should make reading the DS data from the DS callback even safer.
This commit is contained in:
Thad House
2022-12-15 18:27:52 -08:00
committed by GitHub
parent 701995d6cc
commit 7713f68772
2 changed files with 34 additions and 18 deletions

View File

@@ -43,7 +43,6 @@ struct JoystickDataCache {
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>);
@@ -114,7 +113,9 @@ void JoystickDataCache::Update() {
static HAL_ControlWord newestControlWord;
static JoystickDataCache caches[3];
static JoystickDataCache* currentRead = &caches[0];
static JoystickDataCache* currentCache = &caches[1];
static JoystickDataCache* currentReadLocal = &caches[0];
static std::atomic<JoystickDataCache*> currentCache{&caches[1]};
static JoystickDataCache* lastGiven = &caches[1];
static JoystickDataCache* cacheToUpdate = &caches[2];
static wpi::mutex cacheMutex;
@@ -476,11 +477,18 @@ static void tcpOccur(void) {
static void udpOccur(void) {
cacheToUpdate->Update();
{
std::scoped_lock lock{cacheMutex};
std::swap(currentCache, cacheToUpdate);
currentCache->updated = true;
JoystickDataCache* given = cacheToUpdate;
JoystickDataCache* prev = currentCache.exchange(cacheToUpdate);
if (prev == nullptr) {
cacheToUpdate = currentReadLocal;
currentReadLocal = lastGiven;
} else {
// Current read local does not update
cacheToUpdate = prev;
}
lastGiven = given;
driverStation->newDataEvents.Wakeup();
}
@@ -506,9 +514,9 @@ void HAL_RefreshDSData(void) {
FRC_NetworkCommunication_getControlWord(
reinterpret_cast<ControlWord_t*>(&controlWord));
std::scoped_lock lock{cacheMutex};
if (currentCache->updated) {
std::swap(currentCache, currentRead);
currentCache->updated = false;
JoystickDataCache* prev = currentCache.exchange(nullptr);
if (prev != nullptr) {
currentRead = prev;
}
newestControlWord = controlWord;

View File

@@ -44,7 +44,6 @@ struct JoystickDataCache {
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>);
@@ -75,7 +74,9 @@ void JoystickDataCache::Update() {
static HAL_ControlWord newestControlWord;
static JoystickDataCache caches[3];
static JoystickDataCache* currentRead = &caches[0];
static JoystickDataCache* currentCache = &caches[1];
static JoystickDataCache* currentReadLocal = &caches[0];
static std::atomic<JoystickDataCache*> currentCache{&caches[1]};
static JoystickDataCache* lastGiven = &caches[1];
static JoystickDataCache* cacheToUpdate = &caches[2];
static ::FRCDriverStation* driverStation;
@@ -330,9 +331,9 @@ void HAL_RefreshDSData(void) {
controlWord.fmsAttached = SimDriverStationData->fmsAttached;
controlWord.dsAttached = SimDriverStationData->dsAttached;
std::scoped_lock lock{driverStation->cacheMutex};
if (currentCache->updated) {
std::swap(currentCache, currentRead);
currentCache->updated = false;
JoystickDataCache* prev = currentCache.exchange(nullptr);
if (prev != nullptr) {
currentRead = prev;
}
newestControlWord = controlWord;
}
@@ -368,11 +369,18 @@ void NewDriverStationData() {
return;
}
cacheToUpdate->Update();
{
std::scoped_lock lock{driverStation->cacheMutex};
std::swap(currentCache, cacheToUpdate);
currentCache->updated = true;
JoystickDataCache* given = cacheToUpdate;
JoystickDataCache* prev = currentCache.exchange(cacheToUpdate);
if (prev == nullptr) {
cacheToUpdate = currentReadLocal;
currentReadLocal = lastGiven;
} else {
// Current read local does not update
cacheToUpdate = prev;
}
lastGiven = given;
driverStation->newDataEvents.Wakeup();
SimDriverStationData->CallNewDataCallbacks();
}