From eddc8fba458730a7f7ad647406ebdf1e44c4c6db Mon Sep 17 00:00:00 2001 From: Marek Pieta Date: Mon, 29 Apr 2024 15:25:19 +0200 Subject: [PATCH] applications: nrf_desktop: Remove HID report queue per HID peripheral Change removes HID report queue per HID peripheral. The module relies only on HID report queue defined for HID subscriber. This is done to simplify implementation, reduce memory consumption and speed up retrieving subsequent enqueued HID input report. Jira: NCSDK-25890 Signed-off-by: Marek Pieta Signed-off-by: Pekka Niskanen --- applications/nrf_desktop/doc/hid_forward.rst | 4 +- .../nrf_desktop/src/modules/hid_forward.c | 100 +----------------- .../releases/release-notes-changelog.rst | 4 + 3 files changed, 9 insertions(+), 99 deletions(-) diff --git a/applications/nrf_desktop/doc/hid_forward.rst b/applications/nrf_desktop/doc/hid_forward.rst index 471694c9acf6..65a24bfd7fbc 100644 --- a/applications/nrf_desktop/doc/hid_forward.rst +++ b/applications/nrf_desktop/doc/hid_forward.rst @@ -107,8 +107,8 @@ Enqueuing incoming HID input reports The |hid_forward| forwards only one HID input report to the HID-class USB device at a time. Another HID input report may be received from a peripheral connected over Bluetooth before the previous one was sent. In that case, ``hid_report_event`` is enqueued and submitted later. -Up to the number of reports specified in :ref:`CONFIG_DESKTOP_HID_FORWARD_MAX_ENQUEUED_REPORTS ` reports can be enqueued at a time for each report type and for each connected peripheral. -If there is not enough space to enqueue a new event, the module drops the oldest enqueued event that was received from this peripheral (of the same type). +Up to the number of reports specified in :ref:`CONFIG_DESKTOP_HID_FORWARD_MAX_ENQUEUED_REPORTS ` Kconfig option can be enqueued at a time for each report type and for each HID subscriber (HID-class USB device). +If there is not enough space to enqueue a new event, the module drops the oldest enqueued event (of the same type) that was enqueued for a given HID subscriber. Upon receiving the ``hid_report_sent_event``, the |hid_forward| submits the ``hid_report_event`` enqueued for the peripheral that is associated with the HID-class USB device. The enqueued report to be sent is chosen by the |hid_forward| in the round-robin fashion. diff --git a/applications/nrf_desktop/src/modules/hid_forward.c b/applications/nrf_desktop/src/modules/hid_forward.c index b30625362d4c..f711bfa9311c 100644 --- a/applications/nrf_desktop/src/modules/hid_forward.c +++ b/applications/nrf_desktop/src/modules/hid_forward.c @@ -64,14 +64,11 @@ struct subscriber { struct enqueued_reports enqueued_reports; struct report_data out_reports[ARRAY_SIZE(output_reports)]; uint32_t saved_out_reports_bm; - bool busy; - uint8_t last_peripheral_id; }; struct hids_peripheral { struct bt_hogp hogp; - struct enqueued_reports enqueued_reports; uint32_t enqueued_out_reports_bm; struct k_work_delayable read_rsp; @@ -229,17 +226,6 @@ static bool is_report_enqueued(struct enqueued_reports *enqueued_reports, return true; } -static bool is_any_report_enqueued(struct enqueued_reports *enqueued_reports) -{ - for (size_t irep_idx = 0; irep_idx < ARRAY_SIZE(enqueued_reports->reports); irep_idx++) { - if (is_report_enqueued(enqueued_reports, irep_idx)) { - return true; - } - } - - return false; -} - static struct enqueued_report *get_enqueued_report(struct enqueued_reports *enqueued_reports, size_t irep_idx) { @@ -301,48 +287,6 @@ static struct enqueued_report *get_next_enqueued_report(struct enqueued_reports return item; } -static void migrate_enqueued_reports(struct enqueued_reports *dst_reports, - struct enqueued_reports *src_reports) -{ - /* Migrate only up to MAX_ENQUEUED_ITEMS newest items. - * As per can hold up only up to MAX_ENQUEUED_ITEMS at a time, - * migration from per to sub will affect entire list. - * When migrating from sub to par we will never get more items - * then the defined limit. - * Leaving the oldest items at sub will allow them to be sent - * out first. - */ - for (size_t irep_idx = 0; irep_idx < ARRAY_SIZE(dst_reports->reports); irep_idx++) { - if (is_report_enqueued(src_reports, irep_idx)) { - struct counted_list *dst = &dst_reports->reports[irep_idx]; - struct counted_list *src = &src_reports->reports[irep_idx]; - - sys_slist_t tmp_list; - size_t count = 0; - - sys_slist_init(&tmp_list); - - /* Move excessive items to a temporary list. */ - while (src->count > MAX_ENQUEUED_ITEMS) { - sys_snode_t *node = sys_slist_get(&src->list); - src->count--; - - sys_slist_append(&tmp_list, node); - count++; - } - - /* Source hold MAX_ENQUEUED_ITEMS items max. - * Migrate the entire list. */ - sys_slist_merge_slist(&dst->list, &src->list); - dst->count += src->count; - - /* Bring back excessive items to the source list. */ - src->list = tmp_list; - src->count = count; - } - } -} - static void enqueue_hid_report(struct enqueued_reports *enqueued_reports, size_t irep_idx, struct hid_report_event *report) @@ -410,13 +354,13 @@ static void forward_hid_report(struct hids_peripheral *per, uint8_t report_id, memcpy(&report->dyndata.data[1], data, size); if (!sub->busy) { - __ASSERT_NO_MSG(!is_report_enqueued(&per->enqueued_reports, irep_idx)); + __ASSERT_NO_MSG(!is_report_enqueued(&sub->enqueued_reports, irep_idx)); APP_EVENT_SUBMIT(report); - per->enqueued_reports.last_idx = irep_idx; + sub->enqueued_reports.last_idx = irep_idx; sub->busy = true; } else { - enqueue_hid_report(&per->enqueued_reports, irep_idx, report); + enqueue_hid_report(&sub->enqueued_reports, irep_idx, report); } } @@ -557,15 +501,6 @@ static int register_peripheral(struct bt_gatt_dm *dm, const uint8_t *hwid, per->sub_id = sub_id; - /* Migrate part of the unsent reports to this peripheral. - * This is needed to make sure that at any time number of - * allocated reports is within configured bounds. - */ - __ASSERT_NO_MSG(!is_any_report_enqueued(&per->enqueued_reports)); - ARG_UNUSED(is_any_report_enqueued); - migrate_enqueued_reports(&per->enqueued_reports, - &get_subscriber(per)->enqueued_reports); - __ASSERT_NO_MSG(hwid_len == HWID_LEN); memcpy(per->hwid, hwid, hwid_len); @@ -1005,10 +940,6 @@ static void disconnect_peripheral(struct hids_peripheral *per) } } - migrate_enqueued_reports(&get_subscriber(per)->enqueued_reports, - &per->enqueued_reports); - __ASSERT_NO_MSG(!is_any_report_enqueued(&per->enqueued_reports)); - per->enqueued_out_reports_bm = 0; bt_hogp_release(&per->hogp); @@ -1067,11 +998,8 @@ static void disable_subscription(struct subscriber *sub, uint8_t report_id) if (sub_disconnected) { send_empty_hid_out_reports(per, sub); } - - drop_enqueued_reports(&per->enqueued_reports, irep_idx); } - /* And also this subscriber. */ drop_enqueued_reports(&sub->enqueued_reports, irep_idx); if (sub_disconnected) { @@ -1175,7 +1103,6 @@ static void init(void) per->cfg_chan_id = CFG_CHAN_UNUSED_PEER_ID; per->enqueued_out_reports_bm = 0; - init_enqueued_reports(&per->enqueued_reports); } reset_peripheral_address(); @@ -1201,27 +1128,6 @@ static void send_enqueued_report(struct subscriber *sub) /* First try to send report left at subscriber. */ item = get_next_enqueued_report(&sub->enqueued_reports); - if (!item) { - /* Look for any report to sent at linked peripherals. */ - for (size_t i = 0; i < ARRAY_SIZE(peripherals); i++) { - size_t per_id = next_id(sub->last_peripheral_id + i, - ARRAY_SIZE(peripherals)); - struct hids_peripheral *per = &peripherals[per_id]; - - /* Check if peripheral is linked with the subscriber. */ - if (sub != get_subscriber(per)) { - continue; - } - - item = get_next_enqueued_report(&per->enqueued_reports); - - if (item) { - sub->last_peripheral_id = per_id; - break; - } - } - } - if (item) { APP_EVENT_SUBMIT(item->report); diff --git a/doc/nrf/releases_and_maturity/releases/release-notes-changelog.rst b/doc/nrf/releases_and_maturity/releases/release-notes-changelog.rst index ac44b0b75806..c3e48718a6a4 100644 --- a/doc/nrf/releases_and_maturity/releases/release-notes-changelog.rst +++ b/doc/nrf/releases_and_maturity/releases/release-notes-changelog.rst @@ -225,6 +225,10 @@ nRF Desktop The output report is also enabled for the ``nrf52kbd_nrf52832`` build target in the debug configuration to maintain consistency with the release configuration. * Replaced the :kconfig:option:`CONFIG_BT_L2CAP_TX_BUF_COUNT` Kconfig option with :kconfig:option:`CONFIG_BT_ATT_TX_COUNT` in nRF Desktop dongle configurations. This update is needed to align with the new approach introduced in ``sdk-zephyr`` by commit ``a05a47573a11ba8a78dadc5d3229659f24ddd32f``. + * The :ref:`nrf_desktop_hid_forward` no longer uses a separate HID report queue for a HID peripheral connected over Bluetooth LE. + The module relies only on the HID report queue of a HID subscriber. + This is done to simplify implementation, reduce memory consumption and speed up retrieving enqueued HID reports. + You can modify the enqueued HID report limit through the :ref:`CONFIG_DESKTOP_HID_FORWARD_MAX_ENQUEUED_REPORTS ` Kconfig option. Thingy:53: Matter weather station ---------------------------------