Replaced device_find_* with device_get_*

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.

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:
 

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);
+        Device* display = nullptr;
-        // Boards not yet migrated to the kernel display driver register a placeholder device (so
+        if (device_get_first_by_type(&DISPLAY_TYPE, &display) == ERROR_NONE) {
-        // 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* 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");
         }

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);
+    Device* dev;
-    if (!dev) return;
+    if (device_get_first_by_type(&BLUETOOTH_TYPE, &dev) == ERROR_NONE) {
-    bool radio_on = bluetooth::isRadioOnOrPending(dev);
+        bool radio_on = bluetooth::isRadioOnOrPending(dev);
-    if (requestOn && !radio_on) {
+        if (requestOn && !radio_on) {
-        bluetooth::start(dev);
+            LOG_I(TAG, "Turning on");
-    } else if (!requestOn && radio_on) {
+            bluetooth::start(dev);
-        bluetooth::stop(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);
     }
 

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<BtManage>(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);

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);
+    Device* display;
-    check(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 {

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;

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;
         }

Tactility/Source/service/statusbar/Statusbar.cpp

@@ -221,21 +221,27 @@ class StatusbarService final : public Service {
         }
     }
 
-    void updateUsbIcon() {
+    static bool isHidOrMidiConnected() {
-        bool connected;
+        Device* hid_dev = nullptr;
-        {
+        device_get_first_active_by_type(&USB_HOST_HID_TYPE, &hid_dev);
-            Device* hid_dev;
+        Device* midi_dev = nullptr;
-            device_get_first_active_by_type(&USB_HOST_HID_TYPE, &hid_dev);
+        device_get_first_active_by_type(&USB_HOST_MIDI_TYPE, &midi_dev);
-            Device* midi_dev;
-            device_get_first_active_by_type(&USB_HOST_MIDI_TYPE, &midi_dev);
 
-            connected = (hid_dev && usb_host_hid_is_connected(hid_dev)) ||
+        bool connected = (hid_dev && usb_host_hid_is_connected(hid_dev)) ||
-                             (midi_dev && usb_midi_is_connected(midi_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 {

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
+ * Find the first device of the given type and atomically take a reference on it.
- * device_get_by_name() for why this is preferred over device_find_first_by_type() + device_get()
- * for dynamically constructed/destructed devices.
  *
  * @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
+ * Find the first started device of the given type and atomically take a reference on it.
- * device_get_by_name() for why this is preferred over device_find_first_active_by_type() +
- * device_get() for dynamically constructed/destructed devices.
  *
  * @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
+ * Find the first device whose driver matches the given compatible string and atomically take a reference on it.
- * 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.
  *
  * @param[in] compatible non-null compatible string to match
  * @param[out] out_device receives the found device on success; untouched on failure

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<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;

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),