Device cleanup (#592)

- Replaced device_find_\* usage by device_get_\* variants.
- Removed deprecated device_find_\* functions.
- Improved reliability of Bluetooth scanning, pairing, connection, and HID host lifecycle behavior, including peer auto-connect handling.
- Improved Bluetooth/USB status indicators and clarified “Eject failed” alerts with the affected mount path.
This commit is contained in:
Ken Van Hoeylandt 2026-07-27 17:29:12 +02:00 committed by GitHub
parent 3354924359
commit d1f06cb774
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
20 changed files with 328 additions and 204 deletions

View File

@ -1,16 +1,18 @@
#include "tab5_headphone_detect.h"
#include <tactility/error.h>
#include <tactility/device.h>
#include <tactility/drivers/gpio.h>
#include <tactility/drivers/gpio_controller.h>
#include <tactility/log.h>
#include <tactility/concurrent/mutex.h>
#include <freertos/FreeRTOS.h>
#include <freertos/timers.h>
#include <atomic>
#define TAG "Tab5"
constexpr auto* TAG = "Tab5";
// PI4IOE5V6408-0 (0x43) bit 1
constexpr auto GPIO_EXP0_PIN_SPEAKER_ENABLE = 1;
@ -20,18 +22,88 @@ constexpr auto GPIO_EXP0_PIN_HEADPHONE_DETECT = 7;
constexpr auto HP_DETECT_POLL_MS = 1000;
static TimerHandle_t hp_detect_timer = nullptr;
static std::atomic<Device*> io_expander0_cached { nullptr };
// Flags are written by the timer daemon task
static std::atomic hp_detect_last { false };
static std::atomic hp_detect_initialized { false };
static void headphone_detect_callback(TimerHandle_t /*timer*/) {
Device* cached = io_expander0_cached.load(std::memory_order_acquire);
if (!cached) {
cached = device_find_by_name("io_expander0");
io_expander0_cached.store(cached, std::memory_order_release);
// Owns the cached io_expander0 reference.
// Takes care of refcounting and concurrency.
struct HeadphoneDetectCache {
Mutex mutex {};
Device* io_expander0 = nullptr;
bool active = false;
HeadphoneDetectCache() {
mutex_construct(&mutex);
}
bool isActive() {
mutex_lock(&mutex);
bool result = active;
mutex_unlock(&mutex);
return result;
}
void setActive(bool value) {
mutex_lock(&mutex);
active = value;
mutex_unlock(&mutex);
}
Device* getIoExpander0() {
mutex_lock(&mutex);
Device* dev = io_expander0;
if (dev) {
device_get(dev);
}
mutex_unlock(&mutex);
return dev;
}
// Pass nullptr to clear/release the current entry - that always succeeds, regardless of
// `active`, since it's what stop() uses to tear the cache down.
bool setIoExpander0(Device* dev) {
mutex_lock(&mutex);
if (dev && !active) {
mutex_unlock(&mutex);
return false;
}
Device* old = io_expander0;
if (dev) {
device_get(dev);
}
io_expander0 = dev;
mutex_unlock(&mutex);
if (old) {
device_put(old);
}
return true;
}
};
static HeadphoneDetectCache& headphoneDetectCache() {
static HeadphoneDetectCache instance;
return instance;
}
static void headphone_detect_callback(TimerHandle_t /*timer*/) {
auto& cache = headphoneDetectCache();
if (!cache.isActive()) {
return; // Teardown is in progress or done - don't acquire/publish a new reference
}
Device* io_expander0 = cache.getIoExpander0();
if (!io_expander0) {
Device* dev = nullptr;
if (device_get_by_name("io_expander0", &dev) == ERROR_NONE) {
if (cache.setIoExpander0(dev)) {
device_put(dev); // Cache now holds its own reference
io_expander0 = cache.getIoExpander0();
} else {
io_expander0 = dev; // Deactivated concurrently - use our own reference just this once
}
}
}
auto* io_expander0 = cached;
if (!io_expander0) {
return; // Not ready yet, will retry on next tick
}
@ -39,6 +111,7 @@ static void headphone_detect_callback(TimerHandle_t /*timer*/) {
auto* hp_pin = gpio_descriptor_acquire(io_expander0, GPIO_EXP0_PIN_HEADPHONE_DETECT, GPIO_FLAG_DIRECTION_INPUT, GPIO_OWNER_GPIO);
if (!hp_pin) {
LOG_W(TAG, "hp_detect: HP_DET pin busy");
device_put(io_expander0);
return;
}
@ -48,6 +121,7 @@ static void headphone_detect_callback(TimerHandle_t /*timer*/) {
if (err != ERROR_NONE) {
LOG_W(TAG, "hp_detect: HP_DET read error: %s", error_to_string(err));
device_put(io_expander0);
return;
}
@ -57,18 +131,22 @@ static void headphone_detect_callback(TimerHandle_t /*timer*/) {
auto* spk_pin = gpio_descriptor_acquire(io_expander0, GPIO_EXP0_PIN_SPEAKER_ENABLE, GPIO_FLAG_DIRECTION_OUTPUT, GPIO_OWNER_GPIO);
if (!spk_pin) {
LOG_W(TAG, "hp_detect: SPK_EN pin busy, will retry");
device_put(io_expander0);
return;
}
error_t spk_err = gpio_descriptor_set_level(spk_pin, !hp);
gpio_descriptor_release(spk_pin);
if (spk_err != ERROR_NONE) {
LOG_W(TAG, "hp_detect: SPK_EN set error: %s, will retry", error_to_string(spk_err));
device_put(io_expander0);
return;
}
hp_detect_last = hp;
hp_detect_initialized = true;
LOG_I(TAG, "Headphones %s, speaker %s", hp ? "detected" : "removed", hp ? "disabled" : "enabled");
}
device_put(io_expander0);
}
void tab5_headphone_detect_start() {
@ -80,15 +158,20 @@ void tab5_headphone_detect_start() {
hp_detect_initialized = false;
hp_detect_last = false;
auto& cache = headphoneDetectCache();
cache.setActive(true);
hp_detect_timer = xTimerCreate("hp_detect", pdMS_TO_TICKS(HP_DETECT_POLL_MS), pdTRUE, nullptr, headphone_detect_callback);
if (!hp_detect_timer) {
LOG_E(TAG, "Failed to create hp_detect timer");
cache.setActive(false);
return;
}
if (xTimerStart(hp_detect_timer, pdMS_TO_TICKS(100)) != pdPASS) {
LOG_E(TAG, "Failed to start hp_detect timer");
xTimerDelete(hp_detect_timer, pdMS_TO_TICKS(100));
hp_detect_timer = nullptr;
cache.setActive(false);
}
}
@ -97,6 +180,10 @@ void tab5_headphone_detect_stop() {
return;
}
auto& cache = headphoneDetectCache();
// Block any callback invocation from this point on from installing a new reference.
cache.setActive(false);
if (xTimerStop(hp_detect_timer, pdMS_TO_TICKS(100)) != pdPASS) {
LOG_W(TAG, "Failed to stop hp_detect timer");
}
@ -106,5 +193,6 @@ void tab5_headphone_detect_stop() {
// Always clear the handle — stale non-null handle is worse than a resource leak, as it would
// cause tab5_headphone_detect_start() to silently skip re-creating the timer.
hp_detect_timer = nullptr;
io_expander0_cached.store(nullptr, std::memory_order_release);
cache.setIoExpander0(nullptr);
}

View File

@ -13,6 +13,9 @@
## Higher Priority
- Bluetooth app: when toggling BT on, it doesn't update the UI with discovered devices. It only works after re-opening the app.
- display.h API: get_backlight does not change ref counting, but it should
- bluetooth: various getters for child devices do not change ref counting, but they should
- Improve kernel_init.cpp (and other modules): create driver_ensure_added() and driver_ensure_destructed()
- Remove and migrate `Include/Tactility/kernel/Kernel.h` into `tactility/delay.h`
- Drivers/audio-codec-module is not a module. Move it somewhere else. Or make it an actual module.

View File

@ -76,8 +76,7 @@ lv_indev_t* init() {
return g_indev;
}
g_device = device_find_first_active_by_type(&TDECK_TRACKBALL_TYPE);
if (g_device == nullptr) {
if (device_get_first_active_by_type(&TDECK_TRACKBALL_TYPE, &g_device) != ERROR_NONE) {
LOG_E(TAG, "tdeck_trackball kernel device not found or not started");
return nullptr;
}
@ -88,6 +87,7 @@ lv_indev_t* init() {
g_indev = lv_indev_create();
if (g_indev == nullptr) {
LOG_E(TAG, "Failed to register LVGL input device");
device_put(g_device);
g_device = nullptr;
return nullptr;
}
@ -129,6 +129,8 @@ void deinit() {
lv_indev_delete(g_indev);
g_indev = nullptr;
device_put(g_device);
g_device = nullptr;
g_mode = Mode::Encoder;

View File

@ -17,7 +17,7 @@ class BtManage final : public App {
State state;
View view = View(&bindings, &state);
bool isViewEnabled = false;
struct Device* btDevice = nullptr;
Device* btDevice = nullptr;
public:

View File

@ -54,13 +54,8 @@ class BootApp : public App {
);
static void setupDisplay() {
auto* display = device_find_first_by_type(&DISPLAY_TYPE);
// Boards not yet migrated to the kernel display driver register a placeholder device (so
// the devicetree node resolves) with a NULL api - nothing for this function to act on.
if (display != nullptr && device_get_driver(display)->api == nullptr) {
display = nullptr;
}
if (display != nullptr) {
Device* display = nullptr;
if (device_get_first_by_type(&DISPLAY_TYPE, &display) == ERROR_NONE) {
Device* backlight;
if (display_get_backlight(display, &backlight) == ERROR_NONE) {
if (!device_is_ready(backlight)) {
@ -83,6 +78,7 @@ class BootApp : public App {
} else {
LOG_I(TAG, "No backlight for %s", display->name);
}
device_put(display);
} else {
LOG_I(TAG, "No kernel display");
}

View File

@ -18,25 +18,38 @@ extern const AppManifest manifest;
static void onBtToggled(bool requestOn) {
#if defined(CONFIG_BT_NIMBLE_ENABLED)
Device* dev = device_find_first_by_type(&BLUETOOTH_TYPE);
if (!dev) return;
bool radio_on = bluetooth::isRadioOnOrPending(dev);
if (requestOn && !radio_on) {
bluetooth::start(dev);
} else if (!requestOn && radio_on) {
bluetooth::stop(dev);
Device* dev;
if (device_get_first_by_type(&BLUETOOTH_TYPE, &dev) == ERROR_NONE) {
bool radio_on = bluetooth::isRadioOnOrPending(dev);
if (requestOn && !radio_on) {
LOG_I(TAG, "Turning on");
bluetooth::start(dev);
} else if (!requestOn && radio_on) {
LOG_I(TAG, "Turning off");
bluetooth::stop(dev);
}
device_put(dev);
} else {
LOG_W(TAG, "Toggle: No bluetooth device found");
}
#endif
}
static void onScanToggled(bool enabled) {
Device* dev = device_find_first_active_by_type(&BLUETOOTH_TYPE);
if (!dev) return;
Device* dev;
if (device_get_first_active_by_type(&BLUETOOTH_TYPE, &dev) != ERROR_NONE) {
LOG_W(TAG, "Scan: No bluetooth device found");
return;
}
if (enabled) {
bluetooth_scan_start(dev);
} else {
bluetooth_scan_stop(dev);
}
device_put(dev);
}
static void onConnectPeer(const std::array<uint8_t, 6>& addr, int profileId) {
@ -86,7 +99,7 @@ void BtManage::requestViewUpdate() {
unlock();
}
void BtManage::onBtEvent(const struct BtEvent& event) {
void BtManage::onBtEvent(const BtEvent& event) {
auto radio_state = bluetooth::getRadioState();
LOG_I(TAG, "Update with state %s", bluetooth::radioStateToString(radio_state));
getState().setRadioState(radio_state);
@ -112,10 +125,13 @@ void BtManage::onBtEvent(const struct BtEvent& event) {
case BT_EVENT_RADIO_STATE_CHANGED:
if (event.radio_state == BT_RADIO_STATE_ON) {
getState().updatePairedPeers();
Device* dev = device_find_first_active_by_type(&BLUETOOTH_TYPE);
if (dev && !bluetooth_is_scanning(dev)) {
Device* dev = nullptr;
if (device_get_first_active_by_type(&BLUETOOTH_TYPE, &dev) == ERROR_NONE && !bluetooth_is_scanning(dev)) {
bluetooth_scan_start(dev);
}
if (dev) {
device_put(dev);
}
}
break;
default:
@ -141,7 +157,9 @@ void BtManage::onShow(AppContext& app, lv_obj_t* parent) {
// Initialise state and view before subscribing to avoid incoming events
// racing with state initialisation.
state.setRadioState(bluetooth::getRadioState());
Device* dev = device_find_first_active_by_type(&BLUETOOTH_TYPE);
Device* dev = nullptr;
device_get_first_active_by_type(&BLUETOOTH_TYPE, &dev);
state.setScanning(dev ? bluetooth_is_scanning(dev) : false);
state.updateScanResults();
state.updatePairedPeers();
@ -152,6 +170,11 @@ void BtManage::onShow(AppContext& app, lv_obj_t* parent) {
view.update();
unlock();
if (btDevice) {
// Decrease refcount before re-ssignment
device_put(btDevice);
}
btDevice = dev;
if (btDevice) {
bluetooth_add_event_callback(btDevice, this, onKernelBtEvent);
@ -172,6 +195,7 @@ void BtManage::onHide(AppContext& app) {
lock();
if (btDevice) {
bluetooth_remove_event_callback(btDevice, onKernelBtEvent);
device_put(btDevice);
btDevice = nullptr;
}
isViewEnabled = false;

View File

@ -46,8 +46,12 @@ static void onEnableOnBootParentClicked(lv_event_t* event) {
static void onScanButtonClicked(lv_event_t* event) {
auto bt = std::static_pointer_cast<BtManage>(getCurrentApp());
Device* dev = device_find_first_active_by_type(&BLUETOOTH_TYPE);
Device* dev = nullptr;
device_get_first_active_by_type(&BLUETOOTH_TYPE, &dev);
bool scanning = dev ? bluetooth_is_scanning(dev) : false;
if (dev) {
device_put(dev);
}
bt->getBindings().onScanToggled(!scanning);
}

View File

@ -121,8 +121,12 @@ public:
}
void onShow(AppContext& app, lv_obj_t* parent) override {
if (Device* dev = device_find_first_active_by_type(&BLUETOOTH_TYPE)) {
bluetooth_add_event_callback(dev, this, onKernelBtEvent);
{
Device* dev;
if (device_get_first_active_by_type(&BLUETOOTH_TYPE, &dev) == ERROR_NONE) {
bluetooth_add_event_callback(dev, this, onKernelBtEvent);
device_put(dev);
}
}
// Load stored settings (name, autoConnect)
@ -189,8 +193,10 @@ public:
}
void onHide(AppContext& app) override {
if (Device* dev = device_find_first_active_by_type(&BLUETOOTH_TYPE)) {
Device* dev;
if (device_get_first_active_by_type(&BLUETOOTH_TYPE, &dev) == ERROR_NONE) {
bluetooth_remove_event_callback(dev, onKernelBtEvent);
device_put(dev);
}
viewEnabled = false;
}

View File

@ -436,12 +436,16 @@ void View::onEjectPressed() {
std::string mount_path = state->getSelectedChildPath();
LOG_I(TAG, "Ejecting %s", mount_path.c_str());
struct Device* msc_dev = device_find_first_active_by_type(&USB_HOST_MSC_TYPE);
if (!msc_dev || !usb_msc_eject(msc_dev, mount_path.c_str())) {
Device* msc_dev = nullptr;
if (device_get_first_active_by_type(&USB_HOST_MSC_TYPE, &msc_dev) != ERROR_NONE || !usb_msc_eject(msc_dev, mount_path.c_str())) {
LOG_W(TAG, "usb_msc_eject: %s not found", mount_path.c_str());
alertdialog::start("Eject failed", "Could not eject \"" + file::getLastPathSegment(mount_path) + "\".");
}
if (msc_dev) {
device_put(msc_dev);
}
onNavigate();
state->setEntriesForPath(state->getCurrentPath());
update();

View File

@ -25,15 +25,18 @@ namespace tt::app::kerneldisplay {
constexpr auto* TAG = "KernelDisplay";
static Device* getBacklightDevice() {
Device* display = device_find_first_by_type(&DISPLAY_TYPE);
check(display);
Device* display;
check(device_get_first_by_type(&DISPLAY_TYPE, &display) == ERROR_NONE);
// Boards not yet migrated to the kernel display driver register a placeholder device (so the
// devicetree node resolves) with a NULL api - nothing for display_get_backlight() to act on.
if (device_get_driver(display)->api == nullptr) {
device_put(display);
return nullptr;
}
Device* backlight = nullptr;
return display_get_backlight(display, &backlight) == ERROR_NONE ? backlight : nullptr;
display_get_backlight(display, &backlight);
device_put(display);
return backlight;
}
class KernelDisplayApp final : public App {

View File

@ -28,9 +28,10 @@ static uint32_t timeoutMsToIndex(uint32_t ms) {
static void applyKeyboardBacklight(bool enabled, uint8_t brightness) {
// TODO: Get keyboard backlight from (optional) keyboard child device
Device* backlight = device_find_by_name("keyboard_backlight");
if (backlight != nullptr) {
Device* backlight;
if (device_get_by_name("keyboard_backlight", &backlight) == ERROR_NONE) {
backlight_set_brightness(backlight, enabled ? brightness : 0);
device_put(backlight);
}
}

View File

@ -124,8 +124,10 @@ static void bt_event_bridge(Device*, void* /*context*/, BtEvent event) {
}
if (has_hid_host_auto) {
LOG_I(TAG, "HID host auto-connect peer found — starting scan");
if (Device* dev = device_find_first_active_by_type(&BLUETOOTH_TYPE)) {
Device* dev;
if (device_get_first_active_by_type(&BLUETOOTH_TYPE, &dev) == ERROR_NONE) {
bluetooth_scan_start(dev);
device_put(dev);
}
} else if (has_hid_device_auto) {
LOG_I(TAG, "HID device auto-start (bonded peer found)");
@ -231,10 +233,12 @@ static void bt_event_bridge(Device*, void* /*context*/, BtEvent event) {
}
}
if (has_auto) {
if (Device* dev = device_find_first_active_by_type(&BLUETOOTH_TYPE)) {
Device* dev;
if (device_get_first_active_by_type(&BLUETOOTH_TYPE, &dev) == ERROR_NONE) {
if (!bluetooth_is_scanning(dev)) {
bluetooth_scan_start(dev);
}
device_put(dev);
}
}
});
@ -337,10 +341,19 @@ const char* radioStateToString(RadioState state) {
}
RadioState getRadioState() {
Device* dev = device_find_first_active_by_type(&BLUETOOTH_TYPE);
if (dev == nullptr) return RadioState::Off;
BtRadioState state = BT_RADIO_STATE_OFF;
bluetooth_get_radio_state(dev, &state);
// Scoped to safeguard dev usage
{
Device* dev = nullptr;
device_get_first_active_by_type(&BLUETOOTH_TYPE, &dev);
if (dev == nullptr) {
return RadioState::Off;
}
bluetooth_get_radio_state(dev, &state);
device_put(dev);
}
switch (state) {
case BT_RADIO_STATE_OFF: return RadioState::Off;
case BT_RADIO_STATE_ON_PENDING: return RadioState::OnPending;
@ -405,9 +418,10 @@ void pair(const std::array<uint8_t, 6>& /*addr*/) {
}
void unpair(const std::array<uint8_t, 6>& addr) {
Device* dev = device_find_first_active_by_type(&BLUETOOTH_TYPE);
if (dev != nullptr) {
Device* dev;
if (device_get_first_active_by_type(&BLUETOOTH_TYPE, &dev) == ERROR_NONE) {
bluetooth_unpair(dev, addr.data());
device_put(dev);
}
settings::remove(settings::addrToHex(addr));
}
@ -442,9 +456,11 @@ void disconnect(const std::array<uint8_t, 6>& addr, int profileId) {
bluetooth_hid_device_stop(dev);
}
} else {
Device* dev = device_find_first_active_by_type(&BLUETOOTH_TYPE);
if (dev == nullptr) return;
bluetooth_disconnect(dev, addr.data(), (BtProfileId)profileId);
Device* dev;
if (device_get_first_active_by_type(&BLUETOOTH_TYPE, &dev) == ERROR_NONE) {
bluetooth_disconnect(dev, addr.data(), (BtProfileId)profileId);
device_put(dev);
}
}
}

View File

@ -500,12 +500,14 @@ static void hidHostSubscribeNext(HidHostCtx& ctx) {
}
device.name = name;
settings::save(device);
if (Device* dev = device_find_first_active_by_type(&BLUETOOTH_TYPE)) {
Device* dev;
if (device_get_first_active_by_type(&BLUETOOTH_TYPE, &dev) == ERROR_NONE) {
BtEvent e = {};
e.type = BT_EVENT_PROFILE_STATE_CHANGED;
e.profile_state.state = BT_PROFILE_STATE_CONNECTED;
e.profile_state.profile = BT_PROFILE_HID_HOST;
bluetooth_fire_event(dev, e);
device_put(dev);
}
});
return;
@ -660,13 +662,15 @@ static int hidHostGapCb(struct ble_gap_event* event, void* /*arg*/) {
} else {
LOG_W(TAG, "Connect failed status=%d", event->connect.status);
hid_host_ctx.reset();
if (Device* dev = device_find_first_active_by_type(&BLUETOOTH_TYPE)) {
Device* dev;
if (device_get_first_active_by_type(&BLUETOOTH_TYPE, &dev) == ERROR_NONE) {
bluetooth_set_hid_host_active(dev, false);
struct BtEvent e = {};
BtEvent e = {};
e.type = BT_EVENT_PROFILE_STATE_CHANGED;
e.profile_state.state = BT_PROFILE_STATE_IDLE;
e.profile_state.profile = BT_PROFILE_HID_HOST;
bluetooth_fire_event(dev, e);
device_put(dev);
}
}
break;
@ -685,13 +689,15 @@ static int hidHostGapCb(struct ble_gap_event* event, void* /*arg*/) {
hid_host_mouse_btn.store(false);
hid_host_mouse_active.store(false);
if (Device* dev = device_find_first_active_by_type(&BLUETOOTH_TYPE)) {
Device* dev;
if (device_get_first_active_by_type(&BLUETOOTH_TYPE, &dev) == ERROR_NONE) {
bluetooth_set_hid_host_active(dev, false);
struct BtEvent e = {};
e.type = BT_EVENT_PROFILE_STATE_CHANGED;
e.profile_state.state = BT_PROFILE_STATE_IDLE;
e.profile_state.profile = BT_PROFILE_HID_HOST;
bluetooth_fire_event(dev, e);
device_put(dev);
}
getMainDispatcher().dispatch([saved_kb, saved_mouse, saved_cursor, saved_queue] {
@ -793,7 +799,13 @@ void hidHostConnect(const std::array<uint8_t, 6>& addr) {
}
// Notify driver that a HID host central connection is starting.
if (Device* dev = device_find_first_active_by_type(&BLUETOOTH_TYPE)) bluetooth_set_hid_host_active(dev, true);
{
Device* dev;
if (device_get_first_active_by_type(&BLUETOOTH_TYPE, &dev) == ERROR_NONE) {
bluetooth_set_hid_host_active(dev, true);
device_put(dev);
}
}
// Look up the addr_type from the cached scan results.
ble_addr_t ble_addr = {};
@ -813,7 +825,8 @@ void hidHostConnect(const std::array<uint8_t, 6>& addr) {
if (rc != 0) {
LOG_W(TAG, "ble_gap_connect failed rc=%d", rc);
hid_host_ctx.reset();
if (Device* dev = device_find_first_active_by_type(&BLUETOOTH_TYPE)) {
Device* dev;
if (device_get_first_active_by_type(&BLUETOOTH_TYPE, &dev) == ERROR_NONE) {
bluetooth_set_hid_host_active(dev, false);
// Fire IDLE so bt_event_bridge can start a new scan and retry.
BtEvent e = {};
@ -821,6 +834,7 @@ void hidHostConnect(const std::array<uint8_t, 6>& addr) {
e.profile_state.state = BT_PROFILE_STATE_IDLE;
e.profile_state.profile = BT_PROFILE_HID_HOST;
bluetooth_fire_event(dev, e);
device_put(dev);
}
} else {
LOG_I(TAG, "Connecting...");
@ -867,11 +881,13 @@ void autoConnectHidHost() {
auto peers = settings::loadAll();
for (const auto& peer : peers) {
if (peer.autoConnect && peer.profileId == BT_PROFILE_HID_HOST) {
if (Device* dev = device_find_first_active_by_type(&BLUETOOTH_TYPE)) {
Device* dev;
if (device_get_first_active_by_type(&BLUETOOTH_TYPE, &dev) == ERROR_NONE) {
if (!bluetooth_is_scanning(dev)) {
LOG_I(TAG, "Auto-connect HID host: device not in scan, retrying scan");
bluetooth_scan_start(dev);
}
device_put(dev);
}
break;
}

View File

@ -179,8 +179,11 @@ static void usbHidInputTask(void* arg) {
UsbHidEvent hid_evt;
if (xQueueReceive(ctx->hid_queue, &hid_evt, pdMS_TO_TICKS(100)) != pdTRUE) {
if (!ctx->subscribed) {
struct Device* hid_dev = device_find_first_active_by_type(&USB_HOST_HID_TYPE);
if (hid_dev) ctx->subscribed = usb_host_hid_subscribe(hid_dev, ctx->hid_queue);
Device* hid_dev;
if (device_get_first_active_by_type(&USB_HOST_HID_TYPE, &hid_dev) == ERROR_NONE) {
ctx->subscribed = usb_host_hid_subscribe(hid_dev, ctx->hid_queue);
device_put(hid_dev);
}
}
continue;
}
@ -300,14 +303,23 @@ void startUsbHidInput() {
return;
}
struct Device* hid_dev = device_find_first_active_by_type(&USB_HOST_HID_TYPE);
if (hid_dev) ctx->subscribed = usb_host_hid_subscribe(hid_dev, ctx->hid_queue);
Device* hid_dev = nullptr;
if (device_get_first_active_by_type(&USB_HOST_HID_TYPE, &hid_dev) == ERROR_NONE) {
ctx->subscribed = usb_host_hid_subscribe(hid_dev, ctx->hid_queue);
device_put(hid_dev);
}
ctx->running = true;
if (xTaskCreate(usbHidInputTask, "usb_hid_inp", TASK_STACK, ctx, TASK_PRIORITY, &ctx->task) != pdPASS) {
LOG_E(TAG, "failed to create task");
ctx->running = false;
if (hid_dev) usb_host_hid_unsubscribe(hid_dev, ctx->hid_queue);
if (ctx->subscribed) {
Device* cleanup_dev = nullptr;
if (device_get_first_active_by_type(&USB_HOST_HID_TYPE, &cleanup_dev) == ERROR_NONE) {
usb_host_hid_unsubscribe(cleanup_dev, ctx->hid_queue);
device_put(cleanup_dev);
}
}
vQueueDelete(ctx->hid_queue);
vQueueDelete(ctx->key_queue);
vSemaphoreDelete(ctx->task_done);
@ -345,8 +357,11 @@ void stopUsbHidInput() {
ctx->task = nullptr;
if (ctx->subscribed) {
struct Device* hid_dev = device_find_first_active_by_type(&USB_HOST_HID_TYPE);
if (hid_dev) usb_host_hid_unsubscribe(hid_dev, ctx->hid_queue);
Device* hid_dev;
if (device_get_first_active_by_type(&USB_HOST_HID_TYPE, &hid_dev) == ERROR_NONE) {
usb_host_hid_unsubscribe(hid_dev, ctx->hid_queue);
device_put(hid_dev);
}
}
vQueueDelete(ctx->hid_queue);
vQueueDelete(ctx->key_queue);

View File

@ -19,7 +19,7 @@ constexpr auto* TAG = "RtcTime";
Device* RtcTimeService::findRtcDevice() {
if (!rtcDevice) {
rtcDevice = device_find_first_active_by_type(&RTC_TYPE);
device_get_first_active_by_type(&RTC_TYPE, &rtcDevice);
}
return rtcDevice;
}
@ -130,6 +130,11 @@ void RtcTimeService::onStop(ServiceContext& serviceContext) {
kernel::unsubscribeSystemEvent(timeEventSubscription);
timeEventSubscription = 0;
}
if (rtcDevice) {
device_put(rtcDevice);
rtcDevice = nullptr;
}
}
extern const ServiceManifest manifest = {

View File

@ -166,8 +166,18 @@ class StatusbarService final : public Service {
void updateBluetoothIcon() {
auto radio_state = bluetooth::getRadioState();
Device* btdev = device_find_first_active_by_type(&BLUETOOTH_TYPE);
bool scanning = btdev ? bluetooth_is_scanning(btdev) : false;
bool scanning;
{
Device* btdev;
if (device_get_first_active_by_type(&BLUETOOTH_TYPE, &btdev) == ERROR_NONE) {
scanning = bluetooth_is_scanning(btdev);
device_put(btdev);
} else {
scanning = false;
}
}
Device* serial_dev = bluetooth_serial_get_device();
Device* midi_dev = bluetooth_midi_get_device();
bool connected = (serial_dev && bluetooth_serial_is_connected(serial_dev)) ||
@ -211,11 +221,27 @@ class StatusbarService final : public Service {
}
}
void updateUsbIcon() {
Device* hid_dev = device_find_first_active_by_type(&USB_HOST_HID_TYPE);
Device* midi_dev = device_find_first_active_by_type(&USB_HOST_MIDI_TYPE);
static bool isHidOrMidiConnected() {
Device* hid_dev = nullptr;
device_get_first_active_by_type(&USB_HOST_HID_TYPE, &hid_dev);
Device* midi_dev = nullptr;
device_get_first_active_by_type(&USB_HOST_MIDI_TYPE, &midi_dev);
bool connected = (hid_dev && usb_host_hid_is_connected(hid_dev)) ||
(midi_dev && usb_midi_is_connected(midi_dev));
if (hid_dev) {
device_put(hid_dev);
}
if (midi_dev) {
device_put(midi_dev);
}
return connected;
}
void updateUsbIcon() {
bool connected = isHidOrMidiConnected();
if (!connected) {
// MSC: scan filesystems for any mounted /usb* path
file_system_for_each(&connected, [](struct FileSystem* fs, void* ctx) -> bool {

View File

@ -334,45 +334,11 @@ void device_for_each_of_type(const struct DeviceType* type, void* callback_conte
bool device_exists_of_type(const struct DeviceType* type) ;
/**
* Find a device by its name.
*
* @param[in] name non-null device name to look up
* @return the device pointer if found, or NULL if not found
*/
struct Device* device_find_by_name(const char* name) __attribute__((deprecated("Use device_get_by_name() and device_put()")));
/**
* Find the first started device of the given type.
*
* @param[in] type non-null device type pointer
* @return the first started device of the given type, or NULL if none found
*/
struct Device* device_find_first_active_by_type(const struct DeviceType* type) __attribute__((deprecated("Use device_get_first_active_by_type() and device_put()")));
/**
* Find the first device of the given type.
*
* @param[in] type non-null device type pointer
* @return the first device of the given type, or NULL if none found
*/
struct Device* device_find_first_by_type(const struct DeviceType* type) __attribute__((deprecated("Use device_get_first_by_type() and device_put()")));
/**
* Find the first device whose driver matches the given compatible string.
*
* @param[in] compatible non-null compatible string to match
* @return the first matching device, or NULL if none found
*/
struct Device* device_find_first_by_compatible(const char* compatible) __attribute__((deprecated("Use device_get_first_by_compatible() and device_put()")));
/**
* Find a device by name and atomically take a reference on it (equivalent to a device_find_by_name()
* Find a device by name and atomically take a reference on it.
* immediately followed by a successful device_get(), but race-free: the lookup and the reference
* are taken under the same lock, so a device that gets torn down concurrently either isn't found
* or is safely referenced - there is no gap where a caller could be handed a pointer that's about
* to become invalid). Prefer this over device_find_by_name() + device_get() for any device that
* might be dynamically constructed/destructed at runtime (e.g. a hot-pluggable child device),
* rather than a static devicetree-defined one.
* to become invalid).
*
* @param[in] name non-null device name to look up
* @param[out] out_device receives the found device on success; untouched on failure
@ -383,9 +349,7 @@ struct Device* device_find_first_by_compatible(const char* compatible) __attribu
error_t device_get_by_name(const char* name, struct Device** out_device);
/**
* Find the first device of the given type and atomically take a reference on it. See
* device_get_by_name() for why this is preferred over device_find_first_by_type() + device_get()
* for dynamically constructed/destructed devices.
* Find the first device of the given type and atomically take a reference on it.
*
* @param[in] type non-null device type pointer
* @param[out] out_device receives the found device on success; untouched on failure
@ -396,9 +360,7 @@ error_t device_get_by_name(const char* name, struct Device** out_device);
error_t device_get_first_by_type(const struct DeviceType* type, struct Device** out_device);
/**
* Find the first started device of the given type and atomically take a reference on it. See
* device_get_by_name() for why this is preferred over device_find_first_active_by_type() +
* device_get() for dynamically constructed/destructed devices.
* Find the first started device of the given type and atomically take a reference on it.
*
* @param[in] type non-null device type pointer
* @param[out] out_device receives the found device on success; untouched on failure
@ -417,9 +379,7 @@ error_t device_get_first_active_by_type(const struct DeviceType* type, struct De
bool device_has_active_by_type(const struct DeviceType* type);
/**
* Find the first device whose driver matches the given compatible string and atomically take a
* reference on it. See device_get_by_name() for why this is preferred over
* device_find_first_by_compatible() + device_get() for dynamically constructed/destructed devices.
* Find the first device whose driver matches the given compatible string and atomically take a reference on it.
*
* @param[in] compatible non-null compatible string to match
* @param[out] out_device receives the found device on success; untouched on failure

View File

@ -43,11 +43,9 @@ struct DeviceInternal {
} state;
/** Attached child devices */
std::vector<Device*> children {};
// Outstanding device_get() holders. Guarded by `mutex`. device_get() refuses new refs once
// state.stopping is set, and device_stop() refuses to set state.stopping while this is > 0 -
// together that guarantees ref_count > 0 implies state.started == true, so by the time
// device_remove()/device_destruct() run (both already require !started), this is always
// already 0.
// Outstanding device_get() holders. Guarded by `mutex`. Independent of state.started -
// device_get()/device_put() bracket construct/destruct, not start/stop, so a ref can be held
// across a device_stop(). device_destruct() refuses to run while this is > 0.
int32_t ref_count = 0;
};
@ -94,6 +92,10 @@ error_t device_destruct(Device* device) {
auto* internal = device->internal;
if (internal->ref_count > 0) {
unlock_internal(device->internal);
return ERROR_RESOURCE_BUSY;
}
if (internal->state.started || internal->state.added) {
unlock_internal(device->internal);
return ERROR_INVALID_STATE;
@ -102,13 +104,6 @@ error_t device_destruct(Device* device) {
unlock_internal(device->internal);
return ERROR_INVALID_STATE;
}
// Callers are expected to sequence teardown correctly (device_stop() already refuses to
// clear `started` while ref_count > 0, so by the time !started holds above, ref_count is
// already 0) - this is a cheap defense-in-depth check, not a substitute for that discipline.
if (internal->ref_count > 0) {
unlock_internal(device->internal);
return ERROR_RESOURCE_BUSY;
}
LOG_D(TAG, "destruct %s", device->name);
device->internal = nullptr;
@ -253,11 +248,6 @@ error_t device_stop(Device* device) {
return ERROR_NONE;
}
if (internal->ref_count > 0) {
unlock_internal(internal);
return ERROR_RESOURCE_BUSY;
}
// Already stopping on another thread
if (internal->state.stopping) {
unlock_internal(internal);
@ -266,10 +256,6 @@ error_t device_stop(Device* device) {
internal->state.stopping = true;
unlock_internal(internal);
// driver_unbind() runs the driver's stop_device callback, which may remove/destruct child
// devices (device_remove() takes ledger_lock) - `mutex` must stay released across this call,
// same reasoning as device_start(). state.stopping keeps device_get() from handing out a new
// ref while ref_count is meant to stay at 0 during the unbind.
error_t unbind_error = driver_unbind(internal->driver, device);
lock_internal(internal);
@ -393,11 +379,10 @@ bool device_is_constructed(const Device* device) {
error_t device_get(Device* device) {
auto* internal = device->internal;
lock_internal(internal);
if (!internal->state.started || internal->state.stopping) {
unlock_internal(internal);
if (!internal) {
return ERROR_INVALID_STATE;
}
lock_internal(internal);
internal->ref_count++;
unlock_internal(internal);
return ERROR_NONE;
@ -466,54 +451,6 @@ bool device_exists_of_type(const DeviceType* type) {
return found;
}
Device* device_find_by_name(const char* name) {
Device* found = nullptr;
ledger_lock();
for (auto* device : ledger.devices) {
if (device->name != nullptr && std::strcmp(device->name, name) == 0) {
found = device;
break;
}
}
ledger_unlock();
return found;
}
Device* device_find_first_active_by_type(const DeviceType* type) {
Device* found = nullptr;
device_for_each_of_type(type, &found, [](Device* dev, void* ctx) -> bool {
if (device_is_ready(dev)) {
*static_cast<Device**>(ctx) = dev;
return false;
}
return true;
});
return found;
}
Device* device_find_first_by_type(const DeviceType* type) {
Device* found = nullptr;
device_for_each_of_type(type, &found, [](Device* dev, void* ctx) -> bool {
*static_cast<Device**>(ctx) = dev;
return false;
});
return found;
}
Device* device_find_first_by_compatible(const char* compatible) {
struct Ctx { Device* found; const char* compatible; };
Ctx ctx = { nullptr, compatible };
device_for_each(&ctx, [](Device* dev, void* raw_ctx) -> bool {
auto* c = static_cast<Ctx*>(raw_ctx);
if (device_is_compatible(dev, c->compatible)) {
c->found = dev;
return false;
}
return true;
});
return ctx.found;
}
error_t device_get_by_name(const char* name, Device** out_device) {
ledger_lock();
Device* found = nullptr;

View File

@ -81,10 +81,6 @@ const struct ModuleSymbol KERNEL_SYMBOLS[] = {
DEFINE_MODULE_SYMBOL(device_for_each_child),
DEFINE_MODULE_SYMBOL(device_for_each_of_type),
DEFINE_MODULE_SYMBOL(device_exists_of_type),
DEFINE_MODULE_SYMBOL(device_find_by_name),
DEFINE_MODULE_SYMBOL(device_find_first_active_by_type),
DEFINE_MODULE_SYMBOL(device_find_first_by_type),
DEFINE_MODULE_SYMBOL(device_find_first_by_compatible),
DEFINE_MODULE_SYMBOL(device_get),
DEFINE_MODULE_SYMBOL(device_put),
DEFINE_MODULE_SYMBOL(device_get_by_name),

View File

@ -32,7 +32,7 @@ Driver test_driver = {
} // namespace
TEST_CASE("device_get should fail with ERROR_INVALID_STATE when the device is not started") {
TEST_CASE("device_get should succeed even when the device is not started") {
Device device = { .name = "get_not_started", .config = nullptr, .parent = nullptr };
CHECK_EQ(driver_construct_add(&test_driver), ERROR_NONE);
@ -40,13 +40,30 @@ TEST_CASE("device_get should fail with ERROR_INVALID_STATE when the device is no
device_set_driver(&device, &test_driver);
CHECK_EQ(device_add(&device), ERROR_NONE);
CHECK_EQ(device_get(&device), ERROR_INVALID_STATE);
// Ref-counting brackets construct/destruct, not start/stop.
CHECK_EQ(device_get(&device), ERROR_NONE);
device_put(&device);
CHECK_EQ(device_remove(&device), ERROR_NONE);
CHECK_EQ(device_destruct(&device), ERROR_NONE);
CHECK_EQ(driver_remove_destruct(&test_driver), ERROR_NONE);
}
TEST_CASE("device_get should fail with ERROR_INVALID_STATE once the device has been destructed") {
Device device = { .name = "get_after_destruct", .config = nullptr, .parent = nullptr };
CHECK_EQ(driver_construct_add(&test_driver), ERROR_NONE);
CHECK_EQ(device_construct(&device), ERROR_NONE);
device_set_driver(&device, &test_driver);
CHECK_EQ(device_add(&device), ERROR_NONE);
CHECK_EQ(device_remove(&device), ERROR_NONE);
CHECK_EQ(device_destruct(&device), ERROR_NONE);
CHECK_EQ(device_get(&device), ERROR_INVALID_STATE);
CHECK_EQ(driver_remove_destruct(&test_driver), ERROR_NONE);
}
TEST_CASE("device_get should succeed once started, and device_put should release it") {
Device device = { .name = "get_started", .config = nullptr, .parent = nullptr };
@ -65,7 +82,7 @@ TEST_CASE("device_get should succeed once started, and device_put should release
CHECK_EQ(driver_remove_destruct(&test_driver), ERROR_NONE);
}
TEST_CASE("device_stop should fail with ERROR_RESOURCE_BUSY while a reference is held, from a concurrent thread") {
TEST_CASE("device_stop should succeed while a reference is held, but device_destruct should fail with ERROR_RESOURCE_BUSY until it is released") {
static Device device = { .name = "get_put_concurrent", .config = nullptr, .parent = nullptr };
static std::atomic<bool> acquired { false };
static std::atomic<bool> release { false };
@ -103,21 +120,22 @@ TEST_CASE("device_stop should fail with ERROR_RESOURCE_BUSY while a reference is
delay_millis(1);
}
// Held by the worker thread right now - device_stop() must fail fast, not block.
CHECK_EQ(device_stop(&device), ERROR_RESOURCE_BUSY);
// Held by the worker thread right now - device_stop() is independent of ref-counting, so it
// still succeeds; only device_destruct() gates on outstanding refs.
CHECK_EQ(device_stop(&device), ERROR_NONE);
CHECK_EQ(device_remove(&device), ERROR_NONE);
CHECK_EQ(device_destruct(&device), ERROR_RESOURCE_BUSY);
release = true;
CHECK_EQ(thread_join(thread, 200, 1), ERROR_NONE);
thread_free(thread);
// Reference released - device_stop() now succeeds.
CHECK_EQ(device_stop(&device), ERROR_NONE);
CHECK_EQ(device_remove(&device), ERROR_NONE);
// Reference released - device_destruct() now succeeds.
CHECK_EQ(device_destruct(&device), ERROR_NONE);
CHECK_EQ(driver_remove_destruct(&test_driver), ERROR_NONE);
}
TEST_CASE("device_get_by_name should find and reference a started device, or fail if not found/not started") {
TEST_CASE("device_get_by_name should find and reference an added device regardless of started state, or fail if not found") {
Device device = { .name = "get_by_name_device", .config = nullptr, .parent = nullptr };
CHECK_EQ(driver_construct_add(&test_driver), ERROR_NONE);
@ -127,7 +145,11 @@ TEST_CASE("device_get_by_name should find and reference a started device, or fai
Device* out = nullptr;
CHECK_EQ(device_get_by_name("does_not_exist", &out), ERROR_NOT_FOUND);
CHECK_EQ(device_get_by_name("get_by_name_device", &out), ERROR_INVALID_STATE);
// Not started yet - lookup still succeeds, since it only requires the device to be added.
CHECK_EQ(device_get_by_name("get_by_name_device", &out), ERROR_NONE);
CHECK_EQ(out, &device);
device_put(out);
CHECK_EQ(device_start(&device), ERROR_NONE);
CHECK_EQ(device_get_by_name("get_by_name_device", &out), ERROR_NONE);