diff --git a/Documentation/ideas.md b/Documentation/ideas.md index afe5b753f..c12417b63 100644 --- a/Documentation/ideas.md +++ b/Documentation/ideas.md @@ -13,6 +13,8 @@ ## Higher Priority +- 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. diff --git a/Tactility/Private/Tactility/app/btmanage/BtManagePrivate.h b/Tactility/Private/Tactility/app/btmanage/BtManagePrivate.h index 2e03793ce..c22fe7d0e 100644 --- a/Tactility/Private/Tactility/app/btmanage/BtManagePrivate.h +++ b/Tactility/Private/Tactility/app/btmanage/BtManagePrivate.h @@ -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: diff --git a/Tactility/Source/app/boot/Boot.cpp b/Tactility/Source/app/boot/Boot.cpp index c7612b147..a713d6861 100644 --- a/Tactility/Source/app/boot/Boot.cpp +++ b/Tactility/Source/app/boot/Boot.cpp @@ -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"); } diff --git a/Tactility/Source/app/btmanage/BtManage.cpp b/Tactility/Source/app/btmanage/BtManage.cpp index 72c89c65c..33fc08452 100644 --- a/Tactility/Source/app/btmanage/BtManage.cpp +++ b/Tactility/Source/app/btmanage/BtManage.cpp @@ -18,20 +18,28 @@ 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; if (device_get_first_active_by_type(&BLUETOOTH_TYPE, &dev) != ERROR_NONE) { + LOG_W(TAG, "Scan: No bluetooth device found"); return; } @@ -91,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); @@ -117,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* 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: @@ -146,7 +157,7 @@ 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* dev = nullptr; device_get_first_active_by_type(&BLUETOOTH_TYPE, &dev); state.setScanning(dev ? bluetooth_is_scanning(dev) : false); @@ -160,6 +171,7 @@ void BtManage::onShow(AppContext& app, lv_obj_t* parent) { unlock(); if (btDevice) { + // Decrease refcount before re-ssignment device_put(btDevice); } diff --git a/Tactility/Source/app/btmanage/View.cpp b/Tactility/Source/app/btmanage/View.cpp index f23233a51..d54469575 100644 --- a/Tactility/Source/app/btmanage/View.cpp +++ b/Tactility/Source/app/btmanage/View.cpp @@ -46,7 +46,7 @@ static void onEnableOnBootParentClicked(lv_event_t* event) { static void onScanButtonClicked(lv_event_t* event) { auto bt = std::static_pointer_cast(getCurrentApp()); - Device* dev; + Device* dev = nullptr; device_get_first_active_by_type(&BLUETOOTH_TYPE, &dev); bool scanning = dev ? bluetooth_is_scanning(dev) : false; device_put(dev); diff --git a/Tactility/Source/app/kerneldisplay/KernelDisplay.cpp b/Tactility/Source/app/kerneldisplay/KernelDisplay.cpp index d37ffef37..ff6eca909 100644 --- a/Tactility/Source/app/kerneldisplay/KernelDisplay.cpp +++ b/Tactility/Source/app/kerneldisplay/KernelDisplay.cpp @@ -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 { diff --git a/Tactility/Source/bluetooth/Bluetooth.cpp b/Tactility/Source/bluetooth/Bluetooth.cpp index dca79bd6c..b5811c042 100644 --- a/Tactility/Source/bluetooth/Bluetooth.cpp +++ b/Tactility/Source/bluetooth/Bluetooth.cpp @@ -345,7 +345,7 @@ RadioState getRadioState() { // Scoped to safeguard dev usage { - Device* dev; + Device* dev = nullptr; device_get_first_active_by_type(&BLUETOOTH_TYPE, &dev); if (dev == nullptr) { return RadioState::Off; diff --git a/Tactility/Source/lvgl/UsbHidInput.cpp b/Tactility/Source/lvgl/UsbHidInput.cpp index b70297209..a91f70696 100644 --- a/Tactility/Source/lvgl/UsbHidInput.cpp +++ b/Tactility/Source/lvgl/UsbHidInput.cpp @@ -182,8 +182,8 @@ static void usbHidInputTask(void* arg) { 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); } - device_put(hid_dev); } continue; } diff --git a/Tactility/Source/service/statusbar/Statusbar.cpp b/Tactility/Source/service/statusbar/Statusbar.cpp index 5a064827e..3bbce017a 100644 --- a/Tactility/Source/service/statusbar/Statusbar.cpp +++ b/Tactility/Source/service/statusbar/Statusbar.cpp @@ -221,21 +221,27 @@ class StatusbarService final : public Service { } } - void updateUsbIcon() { - bool connected; - { - Device* hid_dev; - device_get_first_active_by_type(&USB_HOST_HID_TYPE, &hid_dev); - Device* midi_dev; - device_get_first_active_by_type(&USB_HOST_MIDI_TYPE, &midi_dev); + 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); - connected = (hid_dev && usb_host_hid_is_connected(hid_dev)) || - (midi_dev && usb_midi_is_connected(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 { diff --git a/TactilityKernel/include/tactility/device.h b/TactilityKernel/include/tactility/device.h index ade4f224f..d919d9f23 100644 --- a/TactilityKernel/include/tactility/device.h +++ b/TactilityKernel/include/tactility/device.h @@ -333,22 +333,6 @@ void device_for_each_of_type(const struct DeviceType* type, void* callback_conte */ bool device_exists_of_type(const struct DeviceType* type) ; -/** - * 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. * immediately followed by a successful device_get(), but race-free: the lookup and the reference @@ -365,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 @@ -378,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 @@ -399,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 diff --git a/TactilityKernel/source/device.cpp b/TactilityKernel/source/device.cpp index f6cf974e3..dac392245 100644 --- a/TactilityKernel/source/device.cpp +++ b/TactilityKernel/source/device.cpp @@ -466,29 +466,6 @@ bool device_exists_of_type(const DeviceType* type) { 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(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(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; diff --git a/TactilityKernel/source/symbols.c b/TactilityKernel/source/symbols.c index 7f5790917..47875d6ca 100644 --- a/TactilityKernel/source/symbols.c +++ b/TactilityKernel/source/symbols.c @@ -81,9 +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_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),