Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Applications: nrf_desktop: Add subscriber event #13451

Merged
merged 3 commits into from
Dec 28, 2023
Merged

Applications: nrf_desktop: Add subscriber event #13451

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0260b12
applications: nrf_desktop: Add hid subscriber event
KAGA164 Dec 15, 2023
17d2e3c
applications: nrf_desktop: Update documentation
KAGA164 Dec 15, 2023
7eb98df
changelog: Update changelog with the new event
KAGA164 Dec 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions applications/nrf_desktop/doc/event_propagation.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -799,11 +799,7 @@ nRF Desktop event propagation
+-----------------------------------------------+-----------------------------------+---------------+----------------------+-------------------------------------------+
| Source Module | Input Event | This Module | Output Event | Sink Module |
+===============================================+===================================+===============+======================+===========================================+
| :ref:`nrf_desktop_ble_adv` | ``ble_peer_event`` | ``hid_state`` | | |
+-----------------------------------------------+ | | | |
| :ref:`nrf_desktop_ble_state` | | | | |
+-----------------------------------------------+-----------------------------------+ | | |
| :ref:`nrf_desktop_hid_forward` | ``hid_report_event`` | | | |
| :ref:`nrf_desktop_hid_forward` | ``hid_report_event`` | ``hid_state`` | | |
+-----------------------------------------------+ | | | |
| :ref:`nrf_desktop_hid_state` | | | | |
+-----------------------------------------------+ | | | |
Expand All @@ -815,6 +811,10 @@ nRF Desktop event propagation
+-----------------------------------------------+ | | | |
| :ref:`nrf_desktop_usb_state` | | | | |
+-----------------------------------------------+-----------------------------------+ | | |
| :ref:`nrf_desktop_hids` | ``hid_report_subscriber_event`` | | | |
+-----------------------------------------------+ | | | |
| :ref:`nrf_desktop_usb_state` | | | | |
+-----------------------------------------------+-----------------------------------+ | | |
| :ref:`nrf_desktop_hids` | ``hid_report_subscription_event`` | | | |
+-----------------------------------------------+ | | | |
| :ref:`nrf_desktop_usb_state` | | | | |
Expand All @@ -823,8 +823,6 @@ nRF Desktop event propagation
+-----------------------------------------------+-----------------------------------+ | | |
| :ref:`nrf_desktop_motion` | ``motion_event`` | | | |
+-----------------------------------------------+-----------------------------------+ | | |
| :ref:`nrf_desktop_usb_state` | ``usb_hid_event`` | | | |
+-----------------------------------------------+-----------------------------------+ | | |
| :ref:`nrf_desktop_wheel` | ``wheel_event`` | | | |
+-----------------------------------------------+-----------------------------------+ | | |
| :ref:`nrf_desktop_buttons` | ``button_event`` | | | |
Expand Down Expand Up @@ -897,6 +895,8 @@ nRF Desktop event propagation
| | | | +---------------------------------------------+
| | | | | :ref:`nrf_desktop_motion` |
| | | +-----------------------------------+---------------------------------------------+
| | | | ``hid_report_subscriber_event`` | :ref:`nrf_desktop_hid_state` |
| | | +-----------------------------------+---------------------------------------------+
| | | | ``hid_report_subscription_event`` | :ref:`nrf_desktop_hid_forward` |
| | | | +---------------------------------------------+
| | | | | :ref:`nrf_desktop_hid_state` |
Expand Down Expand Up @@ -1277,6 +1277,8 @@ nRF Desktop event propagation
| | | | +---------------------------------------------+
| | | | | :ref:`nrf_desktop_motion` |
| | | +-----------------------------------+---------------------------------------------+
| | | | ``hid_report_subscriber_event`` | :ref:`nrf_desktop_hid_state` |
| | | +-----------------------------------+---------------------------------------------+
| | | | ``hid_report_subscription_event`` | :ref:`nrf_desktop_hid_forward` |
| | | | +---------------------------------------------+
| | | | | :ref:`nrf_desktop_hid_state` |
Expand All @@ -1285,8 +1287,6 @@ nRF Desktop event propagation
| | | +-----------------------------------+---------------------------------------------+
| | | | ``module_state_event`` | :ref:`nrf_desktop_module_state_event_sinks` |
| | | +-----------------------------------+---------------------------------------------+
| | | | ``usb_hid_event`` | :ref:`nrf_desktop_hid_state` |
| | | +-----------------------------------+---------------------------------------------+
| | | | ``usb_state_event`` | :ref:`nrf_desktop_battery_charger` |
| | | | +---------------------------------------------+
| | | | | :ref:`nrf_desktop_ble_conn_params` |
Expand Down
1 change: 0 additions & 1 deletion applications/nrf_desktop/doc/event_rel_modules.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ Sink modules for ble_peer_event
* :ref:`nrf_desktop_ble_state_pm`
* :ref:`nrf_desktop_dfu`
* :ref:`nrf_desktop_hid_forward`
* :ref:`nrf_desktop_hid_state`
* :ref:`nrf_desktop_led_state`
* :ref:`nrf_desktop_ble_state`

Expand Down
10 changes: 6 additions & 4 deletions applications/nrf_desktop/doc/hid_state.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,10 +228,12 @@ Tracking state of transports

The |hid_state| refers collectively to all transports as _subscribers_.

The module tracks the state of the connected Bluetooth® LE peers and the state of USB by listening to ``ble_peer_event`` and ``usb_state_event``, respectively.
When the connection to the host is indicated by any of these events, the |hid_state| will create a subscriber associated with the transport.
The module tracks the state of the connected Bluetooth® LE peers and the state of USB by listening to :c:struct:`hid_report_subscriber_event`.
When the connection to the host is indicated by this event, the |hid_state| will create a subscriber associated with the transport.

The subscriber that is associated with USB has priority over any Bluetooth LE peer subscriber.
Each subscriber reports its priority as part of the :c:struct:`hid_report_subscriber_event`.
The subscriber priority must be unique, that mean two or more subscriber cannot share the same priority value.
By default, the subscriber that is associated with USB has priority over any Bluetooth LE peer subscriber.
As a result, when the device connects to the host through USB, all HID reports will be routed to USB.

Tracking state of HID report notifications
Expand All @@ -242,7 +244,7 @@ These are tracked in the subscriber's structure :c:struct:`subscriber`.
This structure's member ``state`` is an array of :c:struct:`report_state` structures.
Each element corresponds to one available HID report.

The subscriber connects to the HID reports by submitting ``hid_report_subscription_event``.
The subscriber connects to the HID reports by submitting :c:struct:`hid_report_subscription_event`.
Depending on the connection method, this event can be submitted:

* For Bluetooth, when the notification is enabled for a given HID report.
Expand Down
7 changes: 0 additions & 7 deletions applications/nrf_desktop/src/events/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,13 +103,6 @@ config DESKTOP_INIT_LOG_USB_STATE_EVENT
help
Log USB state events in nRF Desktop application.

config DESKTOP_INIT_LOG_USB_HID_EVENT
bool "Log USB HID events"
depends on DESKTOP_USB_ENABLE
default y
help
Log USB HID events in nRF Desktop application.

config DESKTOP_INIT_LOG_WHEEL_EVENT
bool "Log wheel events"
default y
Expand Down
20 changes: 16 additions & 4 deletions applications/nrf_desktop/src/events/hid_event.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,15 @@ static void log_hid_report_subscriber_event(const struct app_event_header *aeh)
const struct hid_report_subscriber_event *event =
cast_hid_report_subscriber_event(aeh);

APP_EVENT_MANAGER_LOG(aeh, "report subscriber %p was %sconnected",
event->subscriber, (event->connected)?(""):("dis"));
if (event->connected) {
APP_EVENT_MANAGER_LOG(aeh, "report subscriber %p was connected, priority: 0x%x "
"pipeline size: %u, max report count: %u",
event->subscriber, event->params.priority,
event->params.pipeline_size, event->params.report_max);
} else {
APP_EVENT_MANAGER_LOG(aeh, "report subscriber %p was disconnected",
event->subscriber);
}
}

static void profile_hid_report_subscriber_event(struct log_event_buf *buf,
Expand All @@ -87,11 +94,16 @@ static void profile_hid_report_subscriber_event(struct log_event_buf *buf,

nrf_profiler_log_encode_uint32(buf, (uint32_t)event->subscriber);
nrf_profiler_log_encode_uint8(buf, event->connected);

nrf_profiler_log_encode_uint8(buf, event->params.priority);
nrf_profiler_log_encode_uint8(buf, event->params.pipeline_size);
nrf_profiler_log_encode_uint8(buf, event->params.report_max);
}

APP_EVENT_INFO_DEFINE(hid_report_subscriber_event,
ENCODE(NRF_PROFILER_ARG_U32, NRF_PROFILER_ARG_U8),
ENCODE("subscriber", "connected"),
ENCODE(NRF_PROFILER_ARG_U32, NRF_PROFILER_ARG_U8, NRF_PROFILER_ARG_U8,
NRF_PROFILER_ARG_U8, NRF_PROFILER_ARG_U8),
ENCODE("subscriber", "connected", "priority", "pipeline size", " report max"),
profile_hid_report_subscriber_event);

APP_EVENT_TYPE_DEFINE(hid_report_subscriber_event,
Expand Down
12 changes: 12 additions & 0 deletions applications/nrf_desktop/src/events/hid_event.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,18 @@ struct hid_report_subscriber_event {
struct app_event_header header; /**< Event header. */

const void *subscriber; /**< Id of the report subscriber. */
struct {
uint8_t priority; /**< Subscriber priority. The bigger value means the
* higher priority. The subscriber priority must be unique.
* Two or more subscriber must not use the same priority value.
*/
uint8_t pipeline_size; /**< Pipeline size. */
uint8_t report_max; /**< Maximum number of reports with different ID, which can be
* processed.
*/
} params; /**< Subscriber parameters. Only needed when a subscriber is connecting.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we actually should make the parameters a structure and fill it conditionally. Looking at the current event creation code, maybe we could fill of the parameters unconditionally no matter if subscriber connects or disconnects?

HID state may not use the parameters when handling subscriber disconnection at all (or it may make a simple comparison to ensure the parameters match the ones reported during subscriber connection)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see making a check of parameters when disconnecting. Now modules are free to fill parameters either on connecting or disconnecting. The only limitation is that the modules shouldn't use parameters data when receiving disconnection event

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it wouldn't be better to fill the parameter in both cases or zero them (to avoid submitting event with garbage data)

* Ignored when disconnecting.
*/
bool connected; /**< True if subscriber is connected to the system. */
};

Expand Down
15 changes: 0 additions & 15 deletions applications/nrf_desktop/src/events/usb_event.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,3 @@ APP_EVENT_TYPE_DEFINE(usb_state_event,
APP_EVENT_FLAGS_CREATE(
IF_ENABLED(CONFIG_DESKTOP_INIT_LOG_USB_STATE_EVENT,
(APP_EVENT_TYPE_FLAGS_INIT_LOG_ENABLE))));

static void log_usb_hid_event(const struct app_event_header *aeh)
{
const struct usb_hid_event *event = cast_usb_hid_event(aeh);

APP_EVENT_MANAGER_LOG(aeh, "id:%p %sabled", event->id,
(event->enabled)?("en"):("dis"));
}

APP_EVENT_TYPE_DEFINE(usb_hid_event,
log_usb_hid_event,
NULL,
APP_EVENT_FLAGS_CREATE(
IF_ENABLED(CONFIG_DESKTOP_INIT_LOG_USB_HID_EVENT,
(APP_EVENT_TYPE_FLAGS_INIT_LOG_ENABLE))));
12 changes: 0 additions & 12 deletions applications/nrf_desktop/src/events/usb_event.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,18 +55,6 @@ struct usb_state_event {
};
APP_EVENT_TYPE_DECLARE(usb_state_event);

/** @brief USB HID event. */
struct usb_hid_event {
/** Event header. */
struct app_event_header header;

/** USB HID device id. */
const void *id;
/** USB HID device enabled state. */
bool enabled;
};
APP_EVENT_TYPE_DECLARE(usb_hid_event);

/**
* @}
*/
Expand Down
9 changes: 9 additions & 0 deletions applications/nrf_desktop/src/modules/Kconfig.hids
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,15 @@ config DESKTOP_HIDS_FIRST_REPORT_DELAY
By default, nRF Desktop keyboard uses a delay of 500 ms to prevent
dropping HID reports right after reconnection.

config DESKTOP_HIDS_SUBSCRIBER_PRIORITY
MarekPieta marked this conversation as resolved.
Show resolved Hide resolved
int "HID service reports subscriber priority"
default 1
KAGA164 marked this conversation as resolved.
Show resolved Hide resolved
range 1 255
help
HID Service over GATT reports subscriber priority. The lower value means the lower
priority in subscribtion to HID reports. By default, the HID service uses the lowest
possible priority.

choice BT_HIDS_DEFAULT_PERM
default BT_HIDS_DEFAULT_PERM_RW_ENCRYPT
help
Expand Down
9 changes: 9 additions & 0 deletions applications/nrf_desktop/src/modules/Kconfig.usb_state
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,15 @@ config DESKTOP_USB_REMOTE_WAKEUP
Enable USB remote wakeup functionality. The USB wakeup request is
triggered on wake_up_event.

config DESKTOP_USB_SUBSCRIBER_REPORT_PRIORITY
int "USB HID reports subscriber priority"
default 255
range 1 255
help
USB reports subscriber priority. The lower value means the lower
priority in subscribtion to HID reports. By default, the USB uses the highest
possible priority.

config DESKTOP_USB_SELECTIVE_REPORT_SUBSCRIPTION
bool "Subscribe only for predefined subset of HID reports"
depends on DESKTOP_HID_STATE_ENABLE
Expand Down
Loading
Loading