diff --git a/Documentation/ideas.md b/Documentation/ideas.md index 9fd8bb37..f2cd305a 100644 --- a/Documentation/ideas.md +++ b/Documentation/ideas.md @@ -7,11 +7,7 @@ ## Higher Priority -- Get rid of TactilityC in favour of TactilityKernel and kernel modules -- Add tests for app stdin/stdout - CrashDiagnostics shouldn't show a QR when there's no callstack -- Apps currently have a `Context` object with an `appInstanceId` in it, purely for being able to close the app. - Change it so that the app has its own termination signal that it waits for in the loop, it should subscribe to the event group. - stopAppFromToolbar() in Tactility.cpp stops the top-most app. Change it so the toolbar knows for which app id it is created, so it can rely on that. - Warn if file operations are done from prohibited tasks (e.g. lvgl task) - Move USB host task stacks to SPIRAM when available: esp32_usbhost*.cpp @@ -20,8 +16,6 @@ - Add bold fonts for e-ink readability improvement - Improve Setup: Show "Step done" screen - Improve Setup: Add keyboard/keypad navigation explanation -- 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 (e.g. bluetooth_hid_device_get_device()) - Improve kernel_init.cpp (and other modules): create driver_ensure_added() and driver_ensure_destructed() - Drivers/audio-codec-module is not a module. Move it somewhere else. Or make it an actual module. - LilyGO T-Dongle S3: 1 button control, stop auto-launching web server @@ -51,7 +45,7 @@ - Make USB host driver disabled by default, so it doesn't consume memory - TactilityTool: Make API compatibility table (and check for compatibility in the tool itself) - Bug: Crash handling app cannot be exited with an EncoderDevice. (current work-around is to manually reset the device) -- Move HttpServer implementation to http-module +- Refactor HttpServer into C code and move implementation to http-module - Use GPS time to set/update the current time ## Lower Priority diff --git a/Tactility/Source/app/boot/Boot.cpp b/Tactility/Source/app/boot/Boot.cpp index de443902..aa1b24f3 100644 --- a/Tactility/Source/app/boot/Boot.cpp +++ b/Tactility/Source/app/boot/Boot.cpp @@ -99,6 +99,7 @@ void setupDisplay() { } else { LOG_E(TAG, "Failed to set brightness of %s", backlight->name); } + device_put(backlight); } else { LOG_I(TAG, "No backlight for %s", display->name); } diff --git a/Tactility/Source/app/display/Display.cpp b/Tactility/Source/app/display/Display.cpp index 8bf2d332..736c84da 100644 --- a/Tactility/Source/app/display/Display.cpp +++ b/Tactility/Source/app/display/Display.cpp @@ -74,6 +74,7 @@ void onBacklightSliderEvent(lv_event_t* event) { ctx->displaySettings.backlightDuty = static_cast(slider_value); ctx->displaySettingsUpdated = true; backlight_set_brightness(backlight, ctx->displaySettings.backlightDuty); + device_put(backlight); } void onOrientationSet(lv_event_t* event) { @@ -184,6 +185,8 @@ void createWidgets(lv_obj_t* parent, void* userData) { lv_slider_set_value(brightness_slider, ctx->displaySettings.backlightDuty, LV_ANIM_OFF); } + // Only compared against nullptr below, never dereferenced again, so releasing it here is safe. + device_put(backlight); } // Orientation diff --git a/Tactility/Source/bluetooth/Bluetooth.cpp b/Tactility/Source/bluetooth/Bluetooth.cpp index d710e072..efc1dc09 100644 --- a/Tactility/Source/bluetooth/Bluetooth.cpp +++ b/Tactility/Source/bluetooth/Bluetooth.cpp @@ -138,20 +138,23 @@ static void bt_event_bridge(BtEvent event) { } } else if (has_hid_device_auto) { LOG_I(TAG, "HID device auto-start (bonded peer found)"); - if (Device* dev = bluetooth_hid_device_get_device()) { + if (Device* dev = bluetooth_hid_device_get()) { bluetooth_hid_device_start(dev, BT_HID_DEVICE_MODE_KEYBOARD); + device_put(dev); } } else { if (settings::shouldSppAutoStart()) { LOG_I(TAG, "Auto-starting SPP server"); - if (Device* dev = bluetooth_serial_get_device()) { + if (Device* dev = bluetooth_serial_get()) { bluetooth_serial_start(dev); + device_put(dev); } } if (settings::shouldMidiAutoStart()) { LOG_I(TAG, "Auto-starting MIDI server"); - if (Device* dev = bluetooth_midi_get_device()) { + if (Device* dev = bluetooth_midi_get()) { bluetooth_midi_start(dev); + device_put(dev); } } } @@ -476,18 +479,21 @@ void connect(const std::array& addr, int profileId) { if (profileId == BT_PROFILE_HID_HOST) { hidHostConnect(addr); } else if (profileId == BT_PROFILE_HID_DEVICE) { - if (Device* dev = bluetooth_hid_device_get_device()) { + if (Device* dev = bluetooth_hid_device_get()) { bluetooth_hid_device_start(dev, BT_HID_DEVICE_MODE_KEYBOARD); + device_put(dev); } } else if (profileId == BT_PROFILE_SPP) { - if (Device* dev = bluetooth_serial_get_device()) { + if (Device* dev = bluetooth_serial_get()) { bluetooth_serial_start(dev); settings::setSppAutoStart(true); + device_put(dev); } } else if (profileId == BT_PROFILE_MIDI) { - if (Device* dev = bluetooth_midi_get_device()) { + if (Device* dev = bluetooth_midi_get()) { bluetooth_midi_start(dev); settings::setMidiAutoStart(true); + device_put(dev); } } } @@ -497,8 +503,9 @@ void disconnect(const std::array& addr, int profileId) { if (profileId == BT_PROFILE_HID_HOST) { hidHostDisconnect(); } else if (profileId == BT_PROFILE_HID_DEVICE) { - if (Device* dev = bluetooth_hid_device_get_device()) { + if (Device* dev = bluetooth_hid_device_get()) { bluetooth_hid_device_stop(dev); + device_put(dev); } } else { Device* dev; diff --git a/Tactility/Source/service/displayidle/DisplayIdle.cpp b/Tactility/Source/service/displayidle/DisplayIdle.cpp index faeffab6..6dff4489 100644 --- a/Tactility/Source/service/displayidle/DisplayIdle.cpp +++ b/Tactility/Source/service/displayidle/DisplayIdle.cpp @@ -38,7 +38,6 @@ static void setBacklightBrightness(uint8_t brightness) { if (device_get_first_active_by_type(&DISPLAY_TYPE, &display) == ERROR_NONE) { ::Device* backlight; if (display_get_backlight(display, &backlight) == ERROR_NONE) { - device_get(backlight); backlight_set_brightness(backlight, brightness); device_put(backlight); } @@ -53,6 +52,7 @@ static bool hasDisplayWithBacklight() { ::Device* backlight; if (display_get_backlight(display, &backlight) == ERROR_NONE) { result = true; + device_put(backlight); } device_put(display); } diff --git a/Tactility/Source/service/statusbar/Statusbar.cpp b/Tactility/Source/service/statusbar/Statusbar.cpp index a42629cc..40d9f069 100644 --- a/Tactility/Source/service/statusbar/Statusbar.cpp +++ b/Tactility/Source/service/statusbar/Statusbar.cpp @@ -178,10 +178,16 @@ class StatusbarService final : public Service { } } - Device* serial_dev = bluetooth_serial_get_device(); - Device* midi_dev = bluetooth_midi_get_device(); + Device* serial_dev = bluetooth_serial_get(); + Device* midi_dev = bluetooth_midi_get(); bool connected = (serial_dev && bluetooth_serial_is_connected(serial_dev)) || (midi_dev && bluetooth_midi_is_connected(midi_dev)); + if (serial_dev) { + device_put(serial_dev); + } + if (midi_dev) { + device_put(midi_dev); + } const char* desired_icon = getBluetoothStatusIcon(radio_state, scanning, connected); if (bt_last_icon != desired_icon) { if (desired_icon != nullptr) { diff --git a/TactilityKernel/include/tactility/drivers/bluetooth_hid_device.h b/TactilityKernel/include/tactility/drivers/bluetooth_hid_device.h index ef041879..d939ec41 100644 --- a/TactilityKernel/include/tactility/drivers/bluetooth_hid_device.h +++ b/TactilityKernel/include/tactility/drivers/bluetooth_hid_device.h @@ -108,8 +108,13 @@ struct BtHidDeviceApi { extern const struct DeviceType BLUETOOTH_HID_DEVICE_TYPE; -/** Find the first ready BLE HID device child device. Returns NULL if unavailable. */ -struct Device* bluetooth_hid_device_get_device(void); +/** + * @brief Find the first ready BLE HID device child device. + * @warning On success, the returned device has an added reference (see device_get()). + * The caller must call device_put() on it once done. + * @return the device, or NULL if unavailable + */ +struct Device* bluetooth_hid_device_get(void); error_t bluetooth_hid_device_start(struct Device* device, enum BtHidDeviceMode mode); error_t bluetooth_hid_device_stop(struct Device* device); diff --git a/TactilityKernel/include/tactility/drivers/bluetooth_midi.h b/TactilityKernel/include/tactility/drivers/bluetooth_midi.h index b5cf44bd..fa6da625 100644 --- a/TactilityKernel/include/tactility/drivers/bluetooth_midi.h +++ b/TactilityKernel/include/tactility/drivers/bluetooth_midi.h @@ -52,8 +52,13 @@ struct BtMidiApi { extern const struct DeviceType BLUETOOTH_MIDI_TYPE; -/** Find the first ready BLE MIDI child device. Returns NULL if unavailable. */ -struct Device* bluetooth_midi_get_device(void); +/** + * @brief Find the first ready BLE MIDI child device. + * @warning On success, the returned device has an added reference (see device_get()). + * The caller must call device_put() on it once done. + * @return the device, or NULL if unavailable + */ +struct Device* bluetooth_midi_get(void); error_t bluetooth_midi_start(struct Device* device); error_t bluetooth_midi_stop(struct Device* device); diff --git a/TactilityKernel/include/tactility/drivers/bluetooth_serial.h b/TactilityKernel/include/tactility/drivers/bluetooth_serial.h index d956013a..1ad8d819 100644 --- a/TactilityKernel/include/tactility/drivers/bluetooth_serial.h +++ b/TactilityKernel/include/tactility/drivers/bluetooth_serial.h @@ -63,8 +63,13 @@ struct BtSerialApi { extern const struct DeviceType BLUETOOTH_SERIAL_TYPE; -/** Find the first ready BLE serial child device. Returns NULL if unavailable. */ -struct Device* bluetooth_serial_get_device(void); +/** + * @brief Find the first ready BLE serial child device. + * @warning On success, the returned device has an added reference (see device_get()). + * The caller must call device_put() on it once done. + * @return the device, or NULL if unavailable + */ +struct Device* bluetooth_serial_get(void); error_t bluetooth_serial_start(struct Device* device); error_t bluetooth_serial_stop(struct Device* device); diff --git a/TactilityKernel/include/tactility/drivers/display.h b/TactilityKernel/include/tactility/drivers/display.h index 97de217e..3518e53c 100644 --- a/TactilityKernel/include/tactility/drivers/display.h +++ b/TactilityKernel/include/tactility/drivers/display.h @@ -210,6 +210,8 @@ struct DisplayApi { /** * @brief Gets the backlight device associated with this display, if any. * @warning Function pointer should be null if capability not available. + * @warning Implementations must not take a reference on *backlight: display_get_backlight() + * adds it on their behalf, once, so drivers don't each have to remember to. * @param[in] device the display device * @param[out] backlight the associated backlight device * @retval ERROR_NONE when a backlight is available and *backlight was set @@ -336,6 +338,8 @@ uint8_t display_get_frame_buffer_count(struct Device* device); /** * @brief Gets the backlight device associated with the specified display, if any. + * @warning On success, *backlight is returned with an added reference (see device_get()). + * The caller must call device_put() on it once done. * @retval ERROR_NONE when a backlight is available and *backlight was set * @retval ERROR_NOT_SUPPORTED when the display has no associated backlight */ diff --git a/TactilityKernel/include/tactility/module.h b/TactilityKernel/include/tactility/module.h index 682f088c..05455e38 100644 --- a/TactilityKernel/include/tactility/module.h +++ b/TactilityKernel/include/tactility/module.h @@ -16,6 +16,8 @@ /** For an overloaded symbol (e.g. libc++/libstdc++'s float/double/long double math overloads), * where a bare `&symbol` is ambiguous - `type` picks the overload to take the address of. */ #define DEFINE_MODULE_SYMBOL_SIGNATURE(symbol, type) { #symbol, (void*)(type)&symbol } +/** Export `symbol` under a different exported `name`, for renamed symbols the loader must still resolve by their old name. */ +#define DEFINE_MODULE_SYMBOL_ALIAS(name, symbol) { name, (void*)&symbol } #ifdef __cplusplus extern "C" { diff --git a/TactilityKernel/source/drivers/bluetooth_hid_device.cpp b/TactilityKernel/source/drivers/bluetooth_hid_device.cpp index 6a78e2ed..7ae93d54 100644 --- a/TactilityKernel/source/drivers/bluetooth_hid_device.cpp +++ b/TactilityKernel/source/drivers/bluetooth_hid_device.cpp @@ -10,10 +10,11 @@ const struct DeviceType BLUETOOTH_HID_DEVICE_TYPE = { .name = "bluetooth-hid-device", }; -struct Device* bluetooth_hid_device_get_device() { +struct Device* bluetooth_hid_device_get() { struct Device* found = nullptr; device_for_each_of_type(&BLUETOOTH_HID_DEVICE_TYPE, &found, [](struct Device* dev, void* ctx) -> bool { - if (device_is_ready(dev)) { + // device_get() while still inside the ledger-locked callback, same as device_get_by_name(). + if (device_is_ready(dev) && device_get(dev) == ERROR_NONE) { *static_cast(ctx) = dev; return false; } diff --git a/TactilityKernel/source/drivers/bluetooth_midi.cpp b/TactilityKernel/source/drivers/bluetooth_midi.cpp index ef753af9..59773f9b 100644 --- a/TactilityKernel/source/drivers/bluetooth_midi.cpp +++ b/TactilityKernel/source/drivers/bluetooth_midi.cpp @@ -10,10 +10,11 @@ const struct DeviceType BLUETOOTH_MIDI_TYPE = { .name = "bluetooth-midi", }; -struct Device* bluetooth_midi_get_device() { +struct Device* bluetooth_midi_get() { struct Device* found = nullptr; device_for_each_of_type(&BLUETOOTH_MIDI_TYPE, &found, [](struct Device* dev, void* ctx) -> bool { - if (device_is_ready(dev)) { + // device_get() while still inside the ledger-locked callback, same as device_get_by_name(). + if (device_is_ready(dev) && device_get(dev) == ERROR_NONE) { *static_cast(ctx) = dev; return false; } diff --git a/TactilityKernel/source/drivers/bluetooth_serial.cpp b/TactilityKernel/source/drivers/bluetooth_serial.cpp index 924fa13f..74c67fbf 100644 --- a/TactilityKernel/source/drivers/bluetooth_serial.cpp +++ b/TactilityKernel/source/drivers/bluetooth_serial.cpp @@ -10,10 +10,11 @@ const struct DeviceType BLUETOOTH_SERIAL_TYPE = { .name = "bluetooth-serial", }; -struct Device* bluetooth_serial_get_device() { +struct Device* bluetooth_serial_get() { struct Device* found = nullptr; device_for_each_of_type(&BLUETOOTH_SERIAL_TYPE, &found, [](struct Device* dev, void* ctx) -> bool { - if (device_is_ready(dev)) { + // device_get() while still inside the ledger-locked callback, same as device_get_by_name(). + if (device_is_ready(dev) && device_get(dev) == ERROR_NONE) { *static_cast(ctx) = dev; return false; } diff --git a/TactilityKernel/source/drivers/display.cpp b/TactilityKernel/source/drivers/display.cpp index ab083098..aaa65ff5 100644 --- a/TactilityKernel/source/drivers/display.cpp +++ b/TactilityKernel/source/drivers/display.cpp @@ -118,7 +118,16 @@ error_t display_get_backlight(Device* device, Device** backlight) { if (api->get_backlight == nullptr) { return ERROR_NOT_SUPPORTED; } - return api->get_backlight(device, backlight); + Device* found = nullptr; + error_t error = api->get_backlight(device, &found); + if (error != ERROR_NONE) { + return error; + } + error = device_get(found); + if (error == ERROR_NONE) { + *backlight = found; + } + return error; } const struct DeviceType DISPLAY_TYPE { diff --git a/TactilityKernel/source/symbols.c b/TactilityKernel/source/symbols.c index c59d3060..a4efa0ab 100644 --- a/TactilityKernel/source/symbols.c +++ b/TactilityKernel/source/symbols.c @@ -340,7 +340,8 @@ const struct ModuleSymbol KERNEL_SYMBOLS[] = { DEFINE_MODULE_SYMBOL(bluetooth_fire_event), DEFINE_MODULE_SYMBOL(BLUETOOTH_TYPE), // drivers/bluetooth_serial - DEFINE_MODULE_SYMBOL(bluetooth_serial_get_device), + DEFINE_MODULE_SYMBOL(bluetooth_serial_get), + DEFINE_MODULE_SYMBOL_ALIAS("bluetooth_serial_get_device", bluetooth_serial_get), // TODO remove in 2027 DEFINE_MODULE_SYMBOL(bluetooth_serial_start), DEFINE_MODULE_SYMBOL(bluetooth_serial_stop), DEFINE_MODULE_SYMBOL(bluetooth_serial_write), @@ -348,14 +349,16 @@ const struct ModuleSymbol KERNEL_SYMBOLS[] = { DEFINE_MODULE_SYMBOL(bluetooth_serial_is_connected), DEFINE_MODULE_SYMBOL(BLUETOOTH_SERIAL_TYPE), // drivers/bluetooth_midi - DEFINE_MODULE_SYMBOL(bluetooth_midi_get_device), + DEFINE_MODULE_SYMBOL(bluetooth_midi_get), + DEFINE_MODULE_SYMBOL_ALIAS("bluetooth_midi_get_device", bluetooth_midi_get), // TODO remove in 2027 DEFINE_MODULE_SYMBOL(bluetooth_midi_start), DEFINE_MODULE_SYMBOL(bluetooth_midi_stop), DEFINE_MODULE_SYMBOL(bluetooth_midi_send), DEFINE_MODULE_SYMBOL(bluetooth_midi_is_connected), DEFINE_MODULE_SYMBOL(BLUETOOTH_MIDI_TYPE), // drivers/bluetooth_hid_device - DEFINE_MODULE_SYMBOL(bluetooth_hid_device_get_device), + DEFINE_MODULE_SYMBOL(bluetooth_hid_device_get), + DEFINE_MODULE_SYMBOL_ALIAS("bluetooth_hid_device_get_device", bluetooth_hid_device_get), // TODO remove in 2027 DEFINE_MODULE_SYMBOL(bluetooth_hid_device_start), DEFINE_MODULE_SYMBOL(bluetooth_hid_device_stop), DEFINE_MODULE_SYMBOL(bluetooth_hid_device_send_key),