From e5c78405300e117562394e130bf27dc3864bb17a Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Fri, 6 Sep 2024 23:08:08 +0200 Subject: [PATCH] Simplify UHID outputs routing There was a registration mechanism to listen to HID outputs with a specific HID id. However, the UHID gamepad processor handles several ids, so it will not work. We could complexify the registration mechanism, but instead, directly dispatch to the expected processor based on the UHID id. Concretely, instead of passing a sc_uhid_devices instance to construct a sc_keyboard_uhid, so that it can register itself, construct the sc_uhid_devices with all the UHID instances (currently only sc_keyboard_uhid) so that it can dispatch HID outputs directly. --- app/src/receiver.c | 10 ++-------- app/src/scrcpy.c | 15 ++++++++++----- app/src/uhid/keyboard_uhid.c | 23 ++++++----------------- app/src/uhid/keyboard_uhid.h | 9 +++++---- app/src/uhid/uhid_output.c | 30 ++++++++++++++++-------------- app/src/uhid/uhid_output.h | 30 ++++++------------------------ 6 files changed, 45 insertions(+), 72 deletions(-) diff --git a/app/src/receiver.c b/app/src/receiver.c index 16bcf0e7e0..0ca82551f2 100644 --- a/app/src/receiver.c +++ b/app/src/receiver.c @@ -63,14 +63,8 @@ static void task_uhid_output(void *userdata) { struct sc_uhid_output_task_data *data = userdata; - struct sc_uhid_receiver *uhid_receiver = - sc_uhid_devices_get_receiver(data->uhid_devices, data->id); - if (uhid_receiver) { - uhid_receiver->ops->process_output(uhid_receiver, data->data, - data->size); - } else { - LOGW("No UHID receiver for id %" PRIu16, data->id); - } + sc_uhid_devices_process_hid_output(data->uhid_devices, data->id, data->data, + data->size); free(data->data); free(data); diff --git a/app/src/scrcpy.c b/app/src/scrcpy.c index 810bf18347..d0e2a5acf6 100644 --- a/app/src/scrcpy.c +++ b/app/src/scrcpy.c @@ -401,7 +401,6 @@ scrcpy(struct scrcpy_options *options) { bool timeout_started = false; struct sc_acksync *acksync = NULL; - struct sc_uhid_devices *uhid_devices = NULL; uint32_t scid = scrcpy_generate_scid(); @@ -724,6 +723,8 @@ scrcpy(struct scrcpy_options *options) { assert(options->mouse_input_mode != SC_MOUSE_INPUT_MODE_AOA); #endif + struct sc_keyboard_uhid *uhid_keyboard = NULL; + if (options->keyboard_input_mode == SC_KEYBOARD_INPUT_MODE_SDK) { sc_keyboard_sdk_init(&s->keyboard_sdk, &s->controller, options->key_inject_mode, @@ -731,14 +732,12 @@ scrcpy(struct scrcpy_options *options) { kp = &s->keyboard_sdk.key_processor; } else if (options->keyboard_input_mode == SC_KEYBOARD_INPUT_MODE_UHID) { - sc_uhid_devices_init(&s->uhid_devices); - bool ok = sc_keyboard_uhid_init(&s->keyboard_uhid, &s->controller, - &s->uhid_devices); + bool ok = sc_keyboard_uhid_init(&s->keyboard_uhid, &s->controller); if (!ok) { goto end; } - uhid_devices = &s->uhid_devices; kp = &s->keyboard_uhid.key_processor; + uhid_keyboard = &s->keyboard_uhid; } if (options->mouse_input_mode == SC_MOUSE_INPUT_MODE_SDK) { @@ -758,6 +757,12 @@ scrcpy(struct scrcpy_options *options) { gp = &s->gamepad_uhid.gamepad_processor; } + struct sc_uhid_devices *uhid_devices = NULL; + if (uhid_keyboard) { + sc_uhid_devices_init(&s->uhid_devices, uhid_keyboard); + uhid_devices = &s->uhid_devices; + } + sc_controller_configure(&s->controller, acksync, uhid_devices); if (!sc_controller_start(&s->controller)) { diff --git a/app/src/uhid/keyboard_uhid.c b/app/src/uhid/keyboard_uhid.c index 11d41e400a..83f66f27b6 100644 --- a/app/src/uhid/keyboard_uhid.c +++ b/app/src/uhid/keyboard_uhid.c @@ -95,21 +95,19 @@ sc_keyboard_uhid_to_sc_mod(uint8_t hid_led) { return mod; } -static void -sc_uhid_receiver_process_output(struct sc_uhid_receiver *receiver, - const uint8_t *data, size_t len) { +void +sc_keyboard_uhid_process_hid_output(struct sc_keyboard_uhid *kb, + const uint8_t *data, size_t size) { assert(sc_thread_get_id() == SC_MAIN_THREAD_ID); - assert(len); + assert(size); // Also check at runtime (do not trust the server) - if (!len) { + if (!size) { LOGE("Unexpected empty HID output message"); return; } - struct sc_keyboard_uhid *kb = DOWNCAST_RECEIVER(receiver); - uint8_t hid_led = data[0]; uint16_t device_mod = sc_keyboard_uhid_to_sc_mod(hid_led); kb->device_mod = device_mod; @@ -117,8 +115,7 @@ sc_uhid_receiver_process_output(struct sc_uhid_receiver *receiver, bool sc_keyboard_uhid_init(struct sc_keyboard_uhid *kb, - struct sc_controller *controller, - struct sc_uhid_devices *uhid_devices) { + struct sc_controller *controller) { sc_hid_keyboard_init(&kb->hid); kb->controller = controller; @@ -137,14 +134,6 @@ sc_keyboard_uhid_init(struct sc_keyboard_uhid *kb, kb->key_processor.hid = true; kb->key_processor.ops = &ops; - static const struct sc_uhid_receiver_ops uhid_receiver_ops = { - .process_output = sc_uhid_receiver_process_output, - }; - - kb->uhid_receiver.id = SC_HID_ID_KEYBOARD; - kb->uhid_receiver.ops = &uhid_receiver_ops; - sc_uhid_devices_add_receiver(uhid_devices, &kb->uhid_receiver); - struct sc_hid_open hid_open; sc_hid_keyboard_generate_open(&hid_open); assert(hid_open.hid_id == SC_HID_ID_KEYBOARD); diff --git a/app/src/uhid/keyboard_uhid.h b/app/src/uhid/keyboard_uhid.h index 639a338462..1628a678fe 100644 --- a/app/src/uhid/keyboard_uhid.h +++ b/app/src/uhid/keyboard_uhid.h @@ -7,12 +7,10 @@ #include "controller.h" #include "hid/hid_keyboard.h" -#include "uhid/uhid_output.h" #include "trait/key_processor.h" struct sc_keyboard_uhid { struct sc_key_processor key_processor; // key processor trait - struct sc_uhid_receiver uhid_receiver; struct sc_hid_keyboard hid; struct sc_controller *controller; @@ -21,7 +19,10 @@ struct sc_keyboard_uhid { bool sc_keyboard_uhid_init(struct sc_keyboard_uhid *kb, - struct sc_controller *controller, - struct sc_uhid_devices *uhid_devices); + struct sc_controller *controller); + +void +sc_keyboard_uhid_process_hid_output(struct sc_keyboard_uhid *kb, + const uint8_t *data, size_t size); #endif diff --git a/app/src/uhid/uhid_output.c b/app/src/uhid/uhid_output.c index 3b095faf16..05e691dab5 100644 --- a/app/src/uhid/uhid_output.c +++ b/app/src/uhid/uhid_output.c @@ -1,25 +1,27 @@ #include "uhid_output.h" #include +#include -void -sc_uhid_devices_init(struct sc_uhid_devices *devices) { - devices->count = 0; -} +#include "uhid/keyboard_uhid.h" +#include "util/log.h" void -sc_uhid_devices_add_receiver(struct sc_uhid_devices *devices, - struct sc_uhid_receiver *receiver) { - assert(devices->count < SC_UHID_MAX_RECEIVERS); - devices->receivers[devices->count++] = receiver; +sc_uhid_devices_init(struct sc_uhid_devices *devices, + struct sc_keyboard_uhid *keyboard) { + devices->keyboard = keyboard; } -struct sc_uhid_receiver * -sc_uhid_devices_get_receiver(struct sc_uhid_devices *devices, uint16_t id) { - for (size_t i = 0; i < devices->count; ++i) { - if (devices->receivers[i]->id == id) { - return devices->receivers[i]; +void +sc_uhid_devices_process_hid_output(struct sc_uhid_devices *devices, uint16_t id, + const uint8_t *data, size_t size) { + if (id == SC_HID_ID_KEYBOARD) { + if (devices->keyboard) { + sc_keyboard_uhid_process_hid_output(devices->keyboard, data, size); + } else { + LOGW("Unexpected keyboard HID output without UHID keyboard"); } + } else { + LOGW("HID output ignored for id %" PRIu16, id); } - return NULL; } diff --git a/app/src/uhid/uhid_output.h b/app/src/uhid/uhid_output.h index e13eed8799..cd6a800f87 100644 --- a/app/src/uhid/uhid_output.h +++ b/app/src/uhid/uhid_output.h @@ -9,37 +9,19 @@ /** * The communication with UHID devices is bidirectional. * - * This component manages the registration of receivers to handle UHID output - * messages (sent from the device to the computer). + * This component dispatches HID outputs to the expected processor. */ -struct sc_uhid_receiver { - uint16_t id; - - const struct sc_uhid_receiver_ops *ops; -}; - -struct sc_uhid_receiver_ops { - void - (*process_output)(struct sc_uhid_receiver *receiver, - const uint8_t *data, size_t len); -}; - -#define SC_UHID_MAX_RECEIVERS 1 - struct sc_uhid_devices { - struct sc_uhid_receiver *receivers[SC_UHID_MAX_RECEIVERS]; - unsigned count; + struct sc_keyboard_uhid *keyboard; }; void -sc_uhid_devices_init(struct sc_uhid_devices *devices); +sc_uhid_devices_init(struct sc_uhid_devices *devices, + struct sc_keyboard_uhid *keyboard); void -sc_uhid_devices_add_receiver(struct sc_uhid_devices *devices, - struct sc_uhid_receiver *receiver); - -struct sc_uhid_receiver * -sc_uhid_devices_get_receiver(struct sc_uhid_devices *devices, uint16_t id); +sc_uhid_devices_process_hid_output(struct sc_uhid_devices *devices, uint16_t id, + const uint8_t *data, size_t size); #endif