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(core): Optionally disable automatic endpoint fallback #2572

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ReFil
Copy link
Contributor

@ReFil ReFil commented Oct 19, 2024

By default if only one endpoint is connected but the other is selected, the connected endpoint will be chosen as a fallback, for boards with physical endpoint selection slide switches this can lead to confusing behaviour where one is selected but the other is used.

This adds a kConfig option CONFIG_ZMK_ENDPOINT_DISABLE_FALLBACK that, when enabled, switches the board to a different endpoint selection system that will only return the preferred transport or anew transport ZMK_TRANSPORT_NONE that just swallows any HID data.

Tested with various combinations of USB and BLE and it seems to work as intended

@ReFil ReFil requested review from a team as code owners October 19, 2024 16:00
@caksoylar
Copy link
Contributor

Can we add a link to https://zmk.dev/docs/keymaps/behaviors/outputs pointing to this? Perhaps an additional paragraph at the end of the summary section.

@@ -118,6 +132,24 @@ int zmk_endpoints_toggle_transport(void) {

struct zmk_endpoint_instance zmk_endpoints_selected(void) { return current_instance; }

struct zmk_endpoint_instance zmk_endpoints_preferred(void) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks like mostly duplicated code with get_selected_instance(). We should pull the common logic out into a separate function that both things can call so we don't keep the two functions in sync.

case ZMK_TRANSPORT_USB:
return is_usb_ready() ? ZMK_TRANSPORT_USB : ZMK_TRANSPORT_NONE;

default:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not a fan of default cases with no logic. If you omit the default case, then the compiler can verify that you have a case for every enum value. Then, if you add a new enum value later but forget to update this code, it will give a diagnostic.

I suggest having a case for ZMK_TRANSPORT_NONE which just returns ZMK_TRANSPORT_NONE, and then treating it as an error if none of the cases are hit. Maybe even split this into two completely separate implementations of the function like this?

#if IS_ENABLED(CONFIG_ZMK_ENDPOINT_DISABLE_FALLBACK)

static enum zmk_transport get_selected_transport(void) {
    switch (preferred_transport) {
        case ZMK_TRANSPORT_BLE: 
            return ...
        case ZMK_TRANSPORT_USB: 
            return ...
        case ZMK_TRANSPORT_NONE: 
            return ZMK_TRANSPORT_NONE;
    }

    LOG_ERR("Unknown transport %d", preferred_transport);
    return ZMK_TRANSPORT_NONE;
}

#else

// original implementation

#endif

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants