PR feedback and improvements

This commit is contained in:
Ken Van Hoeylandt 2026-07-25 14:27:41 +02:00
parent 311c3325ff
commit fb541795ec
18 changed files with 223 additions and 97 deletions

View File

@ -20,7 +20,7 @@ jobs:
- name: "Build Tests"
run: cmake --build build --target build-tests
- name: "Run Tests"
run: ctest --build-dir build/Tests
run: ctest --test-dir build/Tests
DevicetreeTests:
runs-on: ubuntu-latest
steps:

View File

@ -282,7 +282,7 @@ def write_device_structs(file, device: Device, parent_device: Device, bindings:
file.write(f"\t.address = {address_value},\n")
file.write(f"\t.name = \"{device.node_name}\",\n") # Use original name
file.write(f"\t.config = &{config_variable_name},\n")
file.write(f"\t.flags = DEVICE_FLAG_DTS,\n")
file.write("\t.flags = DEVICE_FLAG_DTS,\n")
file.write(f"\t.parent = {parent_value},\n")
file.write("\t.internal = NULL\n")
file.write("};\n\n")

View File

@ -1,9 +1,10 @@
# gps-generic-module
Kernel driver implementing the `GPS_TYPE`/`GpsApi` interface (declared by `Drivers/gps-module`,
Apache-2.0) for generic UART-connected GPS/GNSS receivers: chipset probing, init sequences, and
Kernel driver implementing the `GPS_TYPE`/`GpsApi` interface with for generic UART-connected GPS/GNSS receivers:
NMEA parsing for MTK, Airoha/AG33xx, ATGM336H/CASIC, Unicore UC6580 and u-blox 6/7/8/9/10 modules.
It is ported from [Meshtastic Firmware](https://github.com/MeshTastic/firmware), so it has a GPL v3.0 license.
## License
This module is licensed under **GPL-3.0-or-later** (see `LICENSE-GPL-3.0.md`), separately from

View File

@ -315,7 +315,7 @@ static GpsState gps_api_get_state(Device* device) {
static error_t gps_api_get_model_name(Device* device, char* model_name, size_t buffer_size) {
const auto* config = GET_CONFIG(device);
const char* name_to_set = gpsModelToString(config->model);
strncpy(model_name, name_to_set, buffer_size);
snprintf(model_name, buffer_size, "%s", name_to_set);
return ERROR_NONE;
}

View File

@ -7,13 +7,15 @@ These applications are not part of the Tactility operating system's main firmwar
"end-users" refers to people who install and/or use Tactility software on their devices.
## Past & Present
## Summary
Formerly, there was a mixed usage of [GPL v3.0](Documentation/LICENSE-GPL-3.0.md) for internal subprojects
and [Apache License v2.0](Documentation/LICENSE-Apache-2.0.md) for subprojects that would be used in external apps.
The main firmware projects (`Firmware/`, `Tactility/`) are licensed under [GPL v3.0](Documentation/LICENSE-GPL-3.0.md)
Most drivers have an [Apache License v2.0](Documentation/LICENSE-Apache-2.0.md). There are exceptions, such as `Drivers/gps-generic-module/`.
and for subprojects that would be used in external apps.
All projects under `Modules/` also have an [Apache License v2.0](Documentation/LICENSE-Apache-2.0.md).
For future subprojects, [Apache License v2.0](Documentation/LICENSE-Apache-2.0.md) will be chosen for internal subproject.
Existing GPL-licensed projects will retain this license, as it cannot be changed to a more permissive license.
## Overview
@ -22,13 +24,13 @@ Below is an overview of the licenses of some of the subprojects.
| Project | License |
|--------------------|-------------------------|
| Tactility | GNU Public License v3.0 |
| TactilityCore | GNU Public License v3.0 |
| TactilityC | Apache License v2.0 |
| TactilityFreeRTOS | Apache License v2.0 |
| TactilityKernel | Apache License v2.0 |
| Tests | GNU Public License v3.0 |
| Devices/* | GNU Public License v3.0 |
| Drivers/* | (varies) |
| Modules/* | Apache License v2.0 |
| DevicetreeCompiler | Apache License v2.0 |
| Platforms/* | Apache License v2.0 |

View File

@ -76,10 +76,27 @@ struct GpsSubscription {
* @brief API for GPS/GNSS receiver drivers.
*/
struct GpsApi {
/**
* @brief Registers a subscriber for GPS events (e.g. RMC/GGA sentences).
* @param[in] device the GPS device
* @param[in,out] sub subscription to register; caller owns the storage and must keep it alive until unsubscribed
*/
error_t (*event_subscribe)(struct Device* device, struct GpsSubscription* sub);
/**
* @brief Removes a previously registered subscription.
* @param[in] device the GPS device
* @param[in] sub subscription to remove, as passed to event_subscribe
*/
error_t (*event_unsubscribe)(struct Device* device, struct GpsSubscription* sub);
/**
* @brief Blocks the calling task until a new event arrives for the subscription, or timeout elapses.
* @param[in] device the GPS device
* @param[in,out] sub subscription to wait on
* @param[in] timeout max ticks to wait
* @return ERROR_NONE if an event arrived, ERROR_TIMEOUT if the timeout elapsed
*/
error_t (*event_await)(struct Device* device, struct GpsSubscription* sub, TickType_t timeout);
/**
@ -88,6 +105,13 @@ struct GpsApi {
*/
enum GpsState (*get_state)(struct Device* device);
/**
* @brief Gets a human-readable model name for the device, e.g. "UBLOX8".
* @param[in] device the GPS device
* @param[out] model_name buffer to receive the NUL-terminated name
* @param[in] buffer_size size of model_name in bytes
* @return ERROR_NONE on success
*/
error_t (*get_model_name)(struct Device* device, char* model_name, size_t buffer_size);
};
@ -103,7 +127,7 @@ error_t gps_event_await(struct Device* device, struct GpsSubscription* sub, Tick
/** @copydoc GpsApi::get_state */
enum GpsState gps_get_state(struct Device* device);
/** @copydoc GpsApi::get_state */
/** @copydoc GpsApi::get_model_name */
error_t gps_get_model_name(struct Device* device, char* model_name, size_t buffer_size);
extern const struct DeviceType GPS_TYPE;

View File

@ -14,15 +14,7 @@
constexpr auto* TAG = "gps_ledger";
/**
* @brief Configuration for a GPS_TYPE device.
* @warning Mirrors gps-generic-module's own (GPL) GpsConfig field-for-field - this module can't
* include that header (Apache/GPL boundary), so the layout has to be kept in sync by hand. uart is
* left NULL here: this path uses device_set_parent() to the UART_CONTROLLER_TYPE device instead,
* which the driver falls back to when config->uart is NULL.
*/
struct GpsConfig {
Device* uart = nullptr;
uint32_t baud_rate;
enum GpsModel model;
};

View File

@ -2,7 +2,7 @@
#include <gps/gps_settings.h>
#include <gps/private/gps_ledger.h>
#include <tactility/filesystem/file_lock.h>
#include <tactility/filesystem/file_mutex.h>
#include <tactility/log.h>
#include <tactility/service/service_paths.h>
@ -44,14 +44,23 @@ static bool get_configuration_path(char* out_path, size_t out_path_size) {
// Holds the lock (if any) that `path` needs for the lifetime of the guard - see file_find_lock().
class FileLockGuard {
FileMutex mutex;
bool locked;
public:
explicit FileLockGuard(const char* path) {
file_get_mutex(path, &mutex);
file_lock(&mutex);
file_mutex_get(&mutex, path);
file_mutex_lock(&mutex);
locked = true;
}
~FileLockGuard() {
file_unlock(&mutex);
unlock();
}
void unlock() {
if (locked) {
file_mutex_unlock(&mutex);
locked = false;
}
}
};
@ -120,6 +129,7 @@ static error_t write_configurations(const std::vector<GpsConfiguration>& configu
return ERROR_RESOURCE;
}
lock.unlock();
gps_ledger_sync();
return ERROR_NONE;

View File

@ -14,11 +14,11 @@ constexpr TickType_t defaultLockTime = 500 / portTICK_PERIOD_MS;
* @warning when passing zero, we wait forever, as this is the default behaviour for esp_lvgl_port, and we want it to remain consistent
* @deprecated Use lvgl_lock() or lvgl_try_lock() from lvgl-module instead.
*/
bool lock(TickType_t timeout = portMAX_DELAY) __attribute__((deprecated("Use file_get_mutex() from TactilityKernel")));
bool lock(TickType_t timeout = portMAX_DELAY) __attribute__((deprecated("Use lvgl_lock() from lvgl-module")));
/** @deprecated Use lvgl_unlock() from lvgl-module instead. */
void unlock() __attribute__((deprecated("Use file_get_mutex() from TactilityKernel")));
void unlock() __attribute__((deprecated("Use lvgl_unlock() from lvgl-module")));
std::shared_ptr<Lock> getSyncLock() __attribute__((deprecated("Use file_get_mutex() from TactilityKernel")));
std::shared_ptr<Lock> getSyncLock() __attribute__((deprecated("Use lvgl locking functions from lvgl-module")));
} // namespace

View File

@ -11,7 +11,7 @@ namespace tt::hal::sdcard {
* Attempt to find an SD card that the specified belongs to,
* and returns its lock if the SD card is mounted. Otherwise it returns nullptr.
* @deprecated
* @param[in] a path on a file system (e.g. file, directory, etc.)
* @param[in] path a path on a file system (e.g. file, directory, etc.)
* @return the lock of a mounted SD card or otherwise null
*/
std::shared_ptr<Lock> findSdCardLock(const std::string& path) __attribute__((deprecated("Use file_get_mutex() from TactilityKernel")));

View File

@ -53,7 +53,7 @@ constexpr auto* TAG = "Tactility";
static DispatcherHandle_t mainDispatcherHandle = dispatcher_alloc();
void initFileLvglLock();
void initFileMutexForLvgl();
namespace {
@ -374,16 +374,9 @@ void run(Module* dtsModules[], DtsDevice dtsDevices[]) {
return;
}
initFileLvglLock();
// crypt-module
check(module_construct_add_start(&crypt_module) == ERROR_NONE);
// gps-module
check(module_construct_add_start(&gps_module) == ERROR_NONE);
// gps-generic-module
check(module_construct_add_start(&gps_generic_module) == ERROR_NONE);
check(module_ensure_started(&crypt_module) == ERROR_NONE);
check(module_ensure_started(&gps_module) == ERROR_NONE);
check(module_ensure_started(&gps_generic_module) == ERROR_NONE);
#ifdef ESP_PLATFORM
initEsp();
@ -402,6 +395,9 @@ void run(Module* dtsModules[], DtsDevice dtsDevices[]) {
registerAndStartPrimaryServices();
// Must start right before LVGL
initFileMutexForLvgl();
lvgl_module_configure((LvglModuleConfig) {
.on_start = nullptr,
.on_stop = nullptr,
@ -414,8 +410,7 @@ void run(Module* dtsModules[], DtsDevice dtsDevices[]) {
.task_affinity = getCpuAffinityConfiguration().graphics
#endif
});
check(module_construct_add_start(&lvgl_module) == ERROR_NONE);
check(module_construct_add_start(&gps_module) == ERROR_NONE);
check(module_ensure_started(&lvgl_module) == ERROR_NONE);
registerAndStartSecondaryServices();

View File

@ -274,14 +274,13 @@ public:
// thread). Take the same lock updateDeviceStates() uses and hold it across the
// free below, so the timer can never observe pendingDeleteDevice as a dangling
// pointer in deviceRows.
auto lock = lvgl::getSyncLock()->asScopedLock();
lock.lock();
lvgl_lock();
// Drop the stale row unconditionally (cheap vector op, no LVGL calls) - this is
// what keeps the timer safe regardless of whether onShow() has run yet this cycle.
std::erase_if(deviceRows, [this](const DeviceRow& row) {
return row.device == pendingDeleteDevice;
});
lvgl_unlock();
// gps_settings_remove_configuration_at() frees the underlying Device synchronously -
// do this only after the dangling pointer is already out of deviceRows.
@ -291,9 +290,11 @@ public:
// Only safe to touch deviceListWrapper if onShow() already built it for this show
// cycle - it may not have run yet, in which case it'll rebuild fresh (post-deletion,
// deviceRows already correct) when it does.
lvgl_lock();
if (isShown) {
rebuildDeviceList();
}
lvgl_unlock();
}
};

View File

@ -1,10 +1,11 @@
#include <tactility/device.h>
#include <tactility/drivers/display.h>
#include <tactility/drivers/spi_controller.h>
#include <tactility/filesystem/file_lock.h>
#include <tactility/filesystem/file_mutex.h>
#include <tactility/filesystem/file_system.h>
#include <tactility/lvgl_module.h>
constexpr auto* TAG = "file_mutex_lvgl";
struct Device;
namespace tt {
@ -19,28 +20,36 @@ static const FileMutex lvgl_mutex = {
* Finds file systems with a device (e.g. sd card) that is owned by a SPI controller.
* If the SPI controller has a display on the bus, we create an LVGL lock for the file system path.
*/
void initFileLvglLock() {
void initFileMutexForLvgl() {
file_system_for_each(nullptr, [](FileSystem* fs, void* context) {
char mount_path[64];
if (file_system_get_path(fs, mount_path, sizeof(mount_path)) != ERROR_NONE) {
return true;
}
LOG_D(TAG, "Mount path %s", mount_path);
// We only care about file system with a Device (owner)
auto* owner = file_system_get_owner(fs);
if (owner == nullptr) {
LOG_D(TAG, "Owner: none");
return true;
}
LOG_D(TAG, "Owner: %s", owner->name);
// Ignore devices without a parent (root)
auto* parent = device_get_parent(owner);
if (parent == nullptr) {
LOG_D(TAG, "Owner: no parent");
return true;
}
LOG_D(TAG, "Owner: parent %s", parent->name);
// If the FileSystem is on a SPI bus and there's more than 1 device, we assume the other one is the display.
auto* type = device_get_type(parent);
if (type != &SPI_CONTROLLER_TYPE || device_get_child_count(parent) <= 1) {
LOG_D(TAG, "Owner parent not SPI controller or not enough children");
return true;
}
@ -49,21 +58,23 @@ void initFileLvglLock() {
};
Context ctx = { .mountPath = mount_path };
device_for_each_child(parent, &ctx, [](Device* child, void* context) {
device_for_each_child(parent, &ctx, [](Device* child, void* context) -> bool {
Context* ctx = static_cast<Context*>(context);
if (device_get_type(child) == &DISPLAY_TYPE) {
file_register_mutex(
ctx->mountPath,
&lvgl_mutex
LOG_I(TAG, "Adding file mutex for %s as it shares a bus with a display", ctx->mountPath);
file_mutex_register(
&lvgl_mutex,
ctx->mountPath
);
return false;
} else {
LOG_D(TAG, "child of parent, %s: not DISPLAY_TYPE", child->name);
}
return true;
});
return true;
});
}
}

View File

@ -1,28 +0,0 @@
// SPDX-License-Identifier: Apache-2.0
#pragma once
#include <tactility/freertos/freertos.h>
#ifdef __cplusplus
extern "C" {
#endif
struct FileMutex {
void (*lock)();
bool (*try_lock)(TickType_t timeout);
void (*unlock)();
};
void file_register_mutex(const char* path, const FileMutex* mutex);
void file_get_mutex(const char* path, struct FileMutex* mutex);
void file_lock(struct FileMutex* mutex);
bool file_try_lock(struct FileMutex* mutex, TickType_t timeout);
void file_unlock(struct FileMutex* mutex);
#ifdef __cplusplus
}
#endif

View File

@ -0,0 +1,51 @@
// SPDX-License-Identifier: Apache-2.0
#pragma once
#include <tactility/freertos/freertos.h>
#include <stdbool.h>
#ifdef __cplusplus
extern "C" {
#endif
/**
* @brief Set of lock/try_lock/unlock callbacks backing a filesystem mount's mutex.
* Any field left null is treated as a no-op by file_mutex_lock/try_lock/unlock.
*/
struct FileMutex {
void (*lock)();
bool (*try_lock)(TickType_t timeout);
void (*unlock)();
};
/**
* @brief Registers a mutex for a mount path (e.g. "/sdcard") and its descendants.
* @param[in] mutex callbacks to associate with the path; a copy is stored
* @param[in] path mount path this mutex serializes access to
* @note No-op if a mutex is already registered for this exact path.
*/
void file_mutex_register(const struct FileMutex* mutex, const char* path);
/**
* @brief Looks up the mutex registered for path or one of its ancestor mount paths.
* @param[out] mutex receives the matching mutex, or an all-null (no-op) mutex if none matches
* @param[in] path file or directory path to look up
*/
void file_mutex_get(struct FileMutex* mutex, const char* path);
/** @brief Locks mutex. No-op if mutex->lock is null. */
void file_mutex_lock(struct FileMutex* mutex);
/**
* @brief Attempts to lock mutex within timeout.
* @return true if locked (or mutex->try_lock is null), false on timeout
*/
bool file_mutex_try_lock(struct FileMutex* mutex, TickType_t timeout);
/** @brief Unlocks mutex. No-op if mutex->unlock is null. */
void file_mutex_unlock(struct FileMutex* mutex);
#ifdef __cplusplus
}
#endif

View File

@ -122,6 +122,22 @@ error_t module_stop(struct Module* module);
*/
error_t module_construct_add_start(struct Module* module);
/**
* @brief Tries to ensure the module is in a started state.
* Calls module_construct if needed, calls module_start if needed.
* @param module the module
* @return ERROR_NONE if module is in a started state
*/
error_t module_ensure_started(struct Module* module);
/**
* @brief Tries to ensure the module is in a started state.
* Calls module_stop if needed, calls module_destruct if needed.
* @param module the module
* @return ERROR_NONE if module is in a destructed state
*/
error_t module_ensure_destructed(struct Module* module);
/**
* @brief Check if the module is started.
* Can be used when module isn't constructed yet.

View File

@ -1,5 +1,5 @@
// SPDX-License-Identifier: Apache-2.0
#include <tactility/filesystem/file_lock.h>
#include <tactility/filesystem/file_mutex.h>
#include <cstring>
#include <string>
@ -19,7 +19,8 @@ struct FileMutexEntry {
static std::vector<FileMutexEntry> mutex_entries;
extern "C" {
void file_register_mutex(const char* path, const FileMutex* mutex) {
void file_mutex_register(const FileMutex* mutex, const char* path) {
// Skip if entry for path exists
for (auto& entry : mutex_entries) {
if (entry.path == path) {
@ -34,31 +35,35 @@ void file_register_mutex(const char* path, const FileMutex* mutex) {
});
}
void file_get_mutex(const char* path, FileMutex* mutex) {
void file_mutex_get(FileMutex* mutex, const char* path) {
std::string path_string = path;
for (auto& entry : mutex_entries) {
if (entry.path.rfind(path) == 0) {
memcpy(mutex, &entry.mutex, sizeof(FileMutexEntry));
// Match the mount path itself, or a descendant (e.g. "/sdcard" registered, "/sdcard/config.json" requested).
bool is_match = path_string == entry.path ||
(path_string.rfind(entry.path, 0) == 0 && path_string[entry.path.size()] == '/');
if (is_match) {
memcpy(mutex, &entry.mutex, sizeof(FileMutex));
return;
}
}
memcpy(mutex, &no_mutex, sizeof(FileMutex));
*mutex = no_mutex;
}
void file_lock(FileMutex* mutex) {
void file_mutex_lock(FileMutex* mutex) {
if (mutex->lock) {
mutex->lock();
}
}
bool file_try_lock(FileMutex* mutex, TickType_t timeout) {
bool file_mutex_try_lock(FileMutex* mutex, TickType_t timeout) {
if (mutex->try_lock) {
return mutex->try_lock(timeout);
}
return true;
}
void file_unlock(FileMutex* mutex) {
void file_mutex_unlock(FileMutex* mutex) {
if (mutex->unlock) {
mutex->unlock();
}

View File

@ -28,22 +28,39 @@ static ModuleLedger ledger;
extern "C" {
error_t module_construct(Module* module) {
if (module->internal != nullptr) {
LOG_E(TAG, "Module %s was already constructed", module->name);
return ERROR_INVALID_STATE;
}
module->internal = new (std::nothrow) ModuleInternal();
if (module->internal == nullptr) return ERROR_OUT_OF_MEMORY;
return ERROR_NONE;
}
error_t module_destruct(Module* module) {
delete static_cast<ModuleInternal*>(module->internal);
if (module->internal == nullptr) {
LOG_E(TAG, "Module %s was already destructed", module->name);
return ERROR_INVALID_STATE;
}
delete module->internal;
module->internal = nullptr;
return ERROR_NONE;
}
error_t module_add(Module* module) {
mutex_lock(&ledger.mutex);
ledger.modules.push_back(module);
bool exists = false;
for (auto* ledger_module : ledger.modules) {
if (ledger_module == module) {
exists = true;
break;
}
}
if (!exists) {
ledger.modules.push_back(module);
}
mutex_unlock(&ledger.mutex);
return ERROR_NONE;
return exists ? ERROR_INVALID_STATE : ERROR_NONE;
}
error_t module_remove(Module* module) {
@ -57,8 +74,8 @@ error_t module_start(Module* module) {
LOG_I(TAG, "start %s", module->name);
auto* internal = module->internal;
if (internal == nullptr) return ERROR_INVALID_STATE;
if (internal->started) return ERROR_NONE;
if (internal == nullptr) { return ERROR_INVALID_STATE; }
if (internal->started) { return ERROR_NONE; }
if (module->start != nullptr) {
auto error = module->start();
@ -90,8 +107,8 @@ error_t module_stop(Module* module) {
LOG_I(TAG, "stop %s", module->name);
auto* internal = module->internal;
if (internal == nullptr) return ERROR_INVALID_STATE;
if (!internal->started) return ERROR_NONE;
if (internal == nullptr) { return ERROR_INVALID_STATE; }
if (!internal->started) { return ERROR_NONE; }
if (module->drivers != nullptr && internal->drivers_ready) {
size_t count = 0;
@ -117,12 +134,41 @@ error_t module_stop(Module* module) {
error_t module_construct_add_start(Module* module) {
error_t error = module_construct(module);
if (error != ERROR_NONE) return error;
if (error != ERROR_NONE) { return error; }
error = module_add(module);
if (error != ERROR_NONE) return error;
if (error != ERROR_NONE) { return error; }
return module_start(module);
}
error_t module_ensure_started(Module* module) {
if (module->internal == nullptr) {
error_t result = module_construct(module);
if (result != ERROR_NONE) { return result; }
}
if (!module->internal->started) {
error_t result = module_start(module);
if (result != ERROR_NONE) { return result; }
}
return ERROR_NONE;
}
error_t module_ensure_destructed(Module* module) {
if (module->internal != nullptr) {
error_t result;
if (module->internal->started) {
result = module_stop(module);
if (result != ERROR_NONE) { return result; }
}
result = module_destruct(module);
if (result != ERROR_NONE) { return result; }
}
return ERROR_NONE;
}
bool module_resolve_symbol(Module* module, const char* symbol_name, uintptr_t* symbol_address) {
if (!module_is_started(module)) return false;
auto* symbol_ptr = module->symbols;