TactilityCore improvements (#187)

FreeRTOS handles were stored plainly and they were deleted in the destructor of classes.
This meant that if a class were to be copied, the destructor would be called twice on the same handles and lead to double-free.

Seha on Discord suggested to fix this by using `std::unique_ptr` with a custom deletion function.

The changes affect:
- Thread
- Semaphore
- Mutex
- StreamBuffer
- Timer
- MessageQueue
- EventFlag

Thread  changes:
- Removal of the hack with the `Data` struct
- Thread's main body is now just a private static function inside the class.
- The C functions were relocated to static class members

PubSub changes:
- Refactored pubsub into class
- Renamed files to `PubSub` instead of `Pubsub`
- `PubSubSubscription` is now a private inner struct and `PubSub` only exposes `SubscriptionHandle`

Lockable, ScopedLockable, Mutex:
- Added `lock()` method that locks indefinitely
- Remove deprecated `acquire()` and `release()` methods
- Removed `TtWaitForever` in favour of `portMAX_DELAY`
This commit is contained in:
Ken Van Hoeylandt
2025-01-25 17:29:11 +01:00
committed by GitHub
parent c2edbad0fb
commit 686f7cce83
60 changed files with 711 additions and 831 deletions
+4 -5
View File
@@ -1,7 +1,6 @@
#pragma once
#include "I2cCompat.h"
#include "CoreTypes.h"
#include "RtosCompat.h"
#include <climits>
#include <string>
@@ -29,10 +28,10 @@ struct Configuration {
i2c_config_t config;
};
enum Status {
STARTED,
STOPPED,
UNKNOWN
enum class Status {
Started,
Stopped,
Unknown
};
bool init(const std::vector<i2c::Configuration>& configurations);
@@ -1,4 +1,5 @@
#include "I2cDevice.h"
#include <cstdint>
bool I2cDevice::readRegister12(uint8_t reg, float& out) const {
std::uint8_t data[2] = {0};
@@ -25,7 +25,7 @@ void addService(std::shared_ptr<const ServiceManifest> manifest, bool autoStart)
TT_LOG_I(TAG, "Adding %s", id.c_str());
manifest_mutex.lock(TtWaitForever);
manifest_mutex.lock();
if (service_manifest_map[id] == nullptr) {
service_manifest_map[id] = std::move(manifest);
} else {
@@ -43,18 +43,18 @@ void addService(const ServiceManifest& manifest, bool autoStart) {
}
std::shared_ptr<const ServiceManifest> _Nullable findManifestId(const std::string& id) {
manifest_mutex.acquire(TtWaitForever);
manifest_mutex.lock();
auto iterator = service_manifest_map.find(id);
auto manifest = iterator != service_manifest_map.end() ? iterator->second : nullptr;
manifest_mutex.release();
manifest_mutex.unlock();
return manifest;
}
static std::shared_ptr<ServiceInstance> _Nullable findServiceInstanceById(const std::string& id) {
manifest_mutex.acquire(TtWaitForever);
manifest_mutex.lock();
auto iterator = service_instance_map.find(id);
auto service = iterator != service_instance_map.end() ? iterator->second : nullptr;
manifest_mutex.release();
manifest_mutex.unlock();
return service;
}
@@ -70,9 +70,9 @@ bool startService(const std::string& id) {
auto service_instance = std::make_shared<ServiceInstance>(manifest);
// Register first, so that a service can retrieve itself during onStart()
instance_mutex.acquire(TtWaitForever);
instance_mutex.lock();
service_instance_map[manifest->id] = service_instance;
instance_mutex.release();
instance_mutex.unlock();
service_instance->getService()->onStart(*service_instance);
@@ -100,9 +100,9 @@ bool stopService(const std::string& id) {
service_instance->getService()->onStop(*service_instance);
instance_mutex.acquire(TtWaitForever);
instance_mutex.lock();
service_instance_map.erase(id);
instance_mutex.release();
instance_mutex.unlock();
if (service_instance.use_count() > 1) {
TT_LOG_W(TAG, "Possible memory leak: service %s still has %ld references", service_instance->getManifest().id.c_str(), service_instance.use_count() - 1);
@@ -20,11 +20,11 @@ private:
hal::SdCard::State lastState = hal::SdCard::State::Unmounted;
bool lock(TickType_t timeout) const {
return mutex.acquire(timeout) == TtStatusOk;
return mutex.lock(timeout);
}
void unlock() const {
tt_check(mutex.release() == TtStatusOk);
mutex.unlock();
}
void update() {
+1 -1
View File
@@ -1,6 +1,6 @@
#pragma once
#include "Pubsub.h"
#include "PubSub.h"
#include "WifiGlobals.h"
#include "WifiSettings.h"
#include <cstdio>
@@ -73,53 +73,53 @@ public:
RadioState getRadioState() const {
auto lockable = dataMutex.scoped();
lockable->lock(TtWaitForever);
lockable->lock();
// TODO: Handle lock failure
return radio_state;
}
void setRadioState(RadioState newState) {
auto lockable = dataMutex.scoped();
lockable->lock(TtWaitForever);
lockable->lock();
// TODO: Handle lock failure
radio_state = newState;
}
bool isScanning() const {
auto lockable = dataMutex.scoped();
lockable->lock(TtWaitForever);
lockable->lock();
// TODO: Handle lock failure
return scan_active;
}
void setScanning(bool newState) {
auto lockable = dataMutex.scoped();
lockable->lock(TtWaitForever);
lockable->lock();
// TODO: Handle lock failure
scan_active = newState;
}
bool isScanActive() const {
auto lcokable = dataMutex.scoped();
lcokable->lock(TtWaitForever);
lcokable->lock();
return scan_active;
}
void setScanActive(bool newState) {
auto lockable = dataMutex.scoped();
lockable->lock(TtWaitForever);
lockable->lock();
scan_active = newState;
}
bool isSecureConnection() const {
auto lockable = dataMutex.scoped();
lockable->lock(TtWaitForever);
lockable->lock();
return secure_connection;
}
void setSecureConnection(bool newState) {
auto lockable = dataMutex.scoped();
lockable->lock(TtWaitForever);
lockable->lock();
secure_connection = newState;
}
};
@@ -325,7 +325,7 @@ int getRssi() {
static void scan_list_alloc(std::shared_ptr<Wifi> wifi) {
auto lockable = wifi->dataMutex.scoped();
if (lockable->lock(TtWaitForever)) {
if (lockable->lock()) {
assert(wifi->scan_list == nullptr);
wifi->scan_list = static_cast<wifi_ap_record_t*>(malloc(sizeof(wifi_ap_record_t) * wifi->scan_list_limit));
wifi->scan_list_count = 0;
@@ -334,7 +334,7 @@ static void scan_list_alloc(std::shared_ptr<Wifi> wifi) {
static void scan_list_alloc_safely(std::shared_ptr<Wifi> wifi) {
auto lockable = wifi->dataMutex.scoped();
if (lockable->lock(TtWaitForever)) {
if (lockable->lock()) {
if (wifi->scan_list == nullptr) {
scan_list_alloc(wifi);
}
@@ -343,7 +343,7 @@ static void scan_list_alloc_safely(std::shared_ptr<Wifi> wifi) {
static void scan_list_free(std::shared_ptr<Wifi> wifi) {
auto lockable = wifi->dataMutex.scoped();
if (lockable->lock(TtWaitForever)) {
if (lockable->lock()) {
assert(wifi->scan_list != nullptr);
free(wifi->scan_list);
wifi->scan_list = nullptr;
@@ -353,7 +353,7 @@ static void scan_list_free(std::shared_ptr<Wifi> wifi) {
static void scan_list_free_safely(std::shared_ptr<Wifi> wifi) {
auto lockable = wifi->dataMutex.scoped();
if (lockable->lock(TtWaitForever)) {
if (lockable->lock()) {
if (wifi->scan_list != nullptr) {
scan_list_free(wifi);
}
@@ -362,9 +362,9 @@ static void scan_list_free_safely(std::shared_ptr<Wifi> wifi) {
static void publish_event_simple(std::shared_ptr<Wifi> wifi, EventType type) {
auto lockable = wifi->dataMutex.scoped();
if (lockable->lock(TtWaitForever)) {
if (lockable->lock()) {
Event turning_on_event = {.type = type};
tt_pubsub_publish(wifi->pubsub, &turning_on_event);
wifi->pubsub->publish(&turning_on_event);
}
}
@@ -379,7 +379,7 @@ static bool copy_scan_list(std::shared_ptr<Wifi> wifi) {
}
auto lockable = wifi->dataMutex.scoped();
if (!lockable->lock(TtWaitForever)) {
if (!lockable->lock()) {
return false;
}
@@ -449,7 +449,7 @@ static void eventHandler(TT_UNUSED void* arg, esp_event_base_t event_base, int32
if (event_base == WIFI_EVENT) {
TT_LOG_I(TAG, "eventHandler: WIFI_EVENT (%ld)", event_id);
} else if (event_base == IP_EVENT) {
TT_LOG_W(TAG, "eventHandler: IP_EVENT (%ld)", event_id);
TT_LOG_I(TAG, "eventHandler: IP_EVENT (%ld)", event_id);
}
if (event_base == WIFI_EVENT && event_id == WIFI_EVENT_STA_START) {
@@ -967,15 +967,15 @@ public:
wifi->autoConnectTimer = nullptr; // Must release as it holds a reference to this Wifi instance
// Acquire all mutexes
wifi->dataMutex.acquire(TtWaitForever);
wifi->radioMutex.acquire(TtWaitForever);
wifi->dataMutex.lock();
wifi->radioMutex.lock();
// Detach
wifi_singleton = nullptr;
// Release mutexes
wifi->dataMutex.release();
wifi->radioMutex.release();
wifi->dataMutex.unlock();
wifi->radioMutex.unlock();
// Release (hopefully) last Wifi instance by scope
}
@@ -6,7 +6,7 @@
#include "Log.h"
#include "MessageQueue.h"
#include "Mutex.h"
#include "Pubsub.h"
#include "PubSub.h"
#include "service/ServiceContext.h"
namespace tt::service::wifi {
@@ -36,7 +36,7 @@ static Wifi* wifi = nullptr;
static void publish_event_simple(Wifi* wifi, EventType type) {
Event turning_on_event = { .type = type };
tt_pubsub_publish(wifi->pubsub, &turning_on_event);
wifi->pubsub->publish(&turning_on_event);
}
// endregion Static