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.

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

feat: only send HID reports to one endpoint #269

Merged
merged 7 commits into from Oct 31, 2020

Conversation

joelspadin
Copy link
Collaborator

@joelspadin joelspadin commented Oct 11, 2020

Fixes #206.

By default reports are sent to USB if both USB and BLE are connected. This also adds a new endpoints behavior to switch between sending to USB or BLE in that case.

@innovaker innovaker added behaviors bluetooth Bluetooth related items core Core functionality/behavior of ZMK enhancement New feature or request hid usb labels Oct 11, 2020
@joelspadin joelspadin force-pushed the endpoint-selection branch 2 times, most recently from e8f77b4 to 3ec5837 Compare October 11, 2020 23:41
@joelspadin joelspadin force-pushed the endpoint-selection branch 2 times, most recently from d623f0e to 8839b56 Compare October 24, 2020 22:56
@joelspadin joelspadin marked this pull request as ready for review October 24, 2020 23:04
app/src/ble.c Outdated
@@ -163,9 +169,9 @@ int update_advertising() {
struct bt_conn *conn;
enum advertising_type desired_adv = ZMK_ADV_NONE;

if (active_profile_is_open() || !active_profile_is_connected()) {
if (active_profile_is_open() || !zmk_ble_active_profile_is_connected()) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Was the !active_profile_is_connected() intended here. It looks like that makes the else if case below dead code.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, this was here due to me playing w/ the directed advertising. I think this can be removed and allow the below else if block handle the situation properly.


static int zmk_endpoints_init(struct device *_arg) {
#if IS_ENABLED(CONFIG_SETTINGS)
settings_subsys_init();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This seems a bit redundant with the code in settings.c and ble.c. Is there a better way to do this?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is redundant, but the method properly handles being called multiple times and exits early. We can leave for now, until we have a more nuanced settings load order handling

Copy link
Contributor

@petejohanson petejohanson left a comment

Choose a reason for hiding this comment

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

A few comments. Are the changes to docs/package-lock.json really required?

app/include/dt-bindings/zmk/endpoints.h Outdated Show resolved Hide resolved
app/include/dt-bindings/zmk/outputs.h Outdated Show resolved Hide resolved
app/src/ble.c Outdated
@@ -163,9 +169,9 @@ int update_advertising() {
struct bt_conn *conn;
enum advertising_type desired_adv = ZMK_ADV_NONE;

if (active_profile_is_open() || !active_profile_is_connected()) {
if (active_profile_is_open() || !zmk_ble_active_profile_is_connected()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, this was here due to me playing w/ the directed advertising. I think this can be removed and allow the below else if block handle the situation properly.

@joelspadin
Copy link
Collaborator Author

A few comments. Are the changes to docs/package-lock.json really required?

Nope. I wasn't intending to change that and probably just accidentally included it.

Added some utility functions and an event for tracking the state of the
USB connection.

Updated endpoints.c to select a single endpoint to send HID reports to
based on the status of the USB and BLE connections. Partially fixes zmkfirmware#206.

Future commits will add a user setting to control which endpoint is used if
both USB and BLE are ready.
This prevents stuck keys when switching endpoints by clearing
everything in the HID report and sending one last report before
switching to the new endpoint.
Added zmk_ble_active_profile_is_connected() to allow code outside ble.c to check
the status of the active profile, and changed the ble_active_profile_changed
event to also notify when the active profile connects or disconnects.

Changed endpoint selection to to also update when the active profile changes,
connects, or disconnects.
Added a new setting to remember the user's preferred endpoint. When both USB and
BLE are connected, the preferred endpoint will be used.

Added a new behavior to control this setting. It supports commands:

    &end END_USB - Prefer USB output
    &end END_BLE - Prefer BLE output
    &end END_TOG - Toggle between USB and BLE
"Outputs" is probably easier for most people to understand than "endpoints".
@joelspadin
Copy link
Collaborator Author

Tested using a nice!nano, and Windows (USB and BLE), and an Android phone (BLE only).

(I don't really see any good way to unit test this unless you have a way to simulate different endpoints.)

@adamcstephens
Copy link

This works as I'd expect. I'm able to send to USB and BLE separately as toggled while remaining plugged in.

Copy link
Contributor

@petejohanson petejohanson left a comment

Choose a reason for hiding this comment

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

Love it! Thanks


static int zmk_endpoints_init(struct device *_arg) {
#if IS_ENABLED(CONFIG_SETTINGS)
settings_subsys_init();
Copy link
Contributor

Choose a reason for hiding this comment

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

This is redundant, but the method properly handles being called multiple times and exits early. We can leave for now, until we have a more nuanced settings load order handling

@petejohanson petejohanson merged commit 2d31e1d into zmkfirmware:main Oct 31, 2020
@joelspadin
Copy link
Collaborator Author

You're welcome!

innovaker added a commit to innovaker/zmk that referenced this pull request Oct 31, 2020
petejohanson pushed a commit that referenced this pull request Oct 31, 2020
innovaker added a commit to innovaker/zmk that referenced this pull request Nov 2, 2020
MangoIV pushed a commit to MangoIV/zmk that referenced this pull request Dec 18, 2020
feat: only send HID reports to one endpoint
MangoIV pushed a commit to MangoIV/zmk that referenced this pull request Dec 18, 2020
tyalie pushed a commit to tyalie/zmk that referenced this pull request Nov 15, 2022
feat: only send HID reports to one endpoint
tyalie pushed a commit to tyalie/zmk that referenced this pull request Nov 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
behaviors bluetooth Bluetooth related items core Core functionality/behavior of ZMK enhancement New feature or request hacktoberfest-accepted hid usb
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ability to switch between USB output and other BLE profiles
4 participants