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

refactor: address transport switch enumeration warning #1994

Merged
merged 1 commit into from
Feb 20, 2024
Merged

refactor: address transport switch enumeration warning #1994

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
48 changes: 33 additions & 15 deletions app/src/endpoints.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,57 +122,69 @@ struct zmk_endpoint_instance zmk_endpoints_selected(void) {

static int send_keyboard_report(void) {
switch (current_instance.transport) {
#if IS_ENABLED(CONFIG_ZMK_USB)
case ZMK_TRANSPORT_USB: {
#if IS_ENABLED(CONFIG_ZMK_USB)
int err = zmk_usb_hid_send_keyboard_report();
if (err) {
LOG_ERR("FAILED TO SEND OVER USB: %d", err);
}
return err;
}
#else
LOG_ERR("USB endpoint is not supported");
return -ENOTSUP;
#endif /* IS_ENABLED(CONFIG_ZMK_USB) */
}

#if IS_ENABLED(CONFIG_ZMK_BLE)
case ZMK_TRANSPORT_BLE: {
#if IS_ENABLED(CONFIG_ZMK_BLE)
struct zmk_hid_keyboard_report *keyboard_report = zmk_hid_get_keyboard_report();
int err = zmk_hog_send_keyboard_report(&keyboard_report->body);
if (err) {
LOG_ERR("FAILED TO SEND OVER HOG: %d", err);
}
return err;
}
#else
LOG_ERR("BLE HOG endpoint is not supported");
return -ENOTSUP;
#endif /* IS_ENABLED(CONFIG_ZMK_BLE) */
}
}

LOG_ERR("Unsupported endpoint transport %d", current_instance.transport);
LOG_ERR("Unhandled endpoint transport %d", current_instance.transport);
return -ENOTSUP;
}

static int send_consumer_report(void) {
switch (current_instance.transport) {
#if IS_ENABLED(CONFIG_ZMK_USB)
case ZMK_TRANSPORT_USB: {
#if IS_ENABLED(CONFIG_ZMK_USB)
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't this cause this to fall through to the next label with this change? Maybe better to only define the given transport in the enum if that feature is enabled, instead?

Copy link
Collaborator

Choose a reason for hiding this comment

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

enum zmk_transport is written to settings, so conditionally defining values could cause the meaning of a saved value to change from one build to another. Sort of a theoretical problem when we only have two transports, and disabling one means you can't switch at all, but that could become a problem if we ever added another RF protocol as a transport for example.

I'd recommend changing these to something more like

case ZMK_TRANSPORT_USB:
#if IS_ENABLED(CONFIG_ZMK_USB)
    return ...
#else
    return -ENOTSUP;
#endif

to avoid the fallthrough.

Alternatively, you could add a default case, but then you wouldn't get a warning if a new transport is added and someone forgot to add handling for it somewhere.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good call on the settings storage issue. I agree with the -ENOTSUP version you proposed.

int err = zmk_usb_hid_send_consumer_report();
if (err) {
LOG_ERR("FAILED TO SEND OVER USB: %d", err);
}
return err;
}
#else
LOG_ERR("USB endpoint is not supported");
return -ENOTSUP;
#endif /* IS_ENABLED(CONFIG_ZMK_USB) */
}

#if IS_ENABLED(CONFIG_ZMK_BLE)
case ZMK_TRANSPORT_BLE: {
#if IS_ENABLED(CONFIG_ZMK_BLE)
struct zmk_hid_consumer_report *consumer_report = zmk_hid_get_consumer_report();
int err = zmk_hog_send_consumer_report(&consumer_report->body);
if (err) {
LOG_ERR("FAILED TO SEND OVER HOG: %d", err);
}
return err;
}
#else
LOG_ERR("BLE HOG endpoint is not supported");
return -ENOTSUP;
#endif /* IS_ENABLED(CONFIG_ZMK_BLE) */
}
}

LOG_ERR("Unsupported endpoint transport %d", current_instance.transport);
LOG_ERR("Unhandled endpoint transport %d", current_instance.transport);
return -ENOTSUP;
}

Expand All @@ -194,29 +206,35 @@ int zmk_endpoints_send_report(uint16_t usage_page) {
#if IS_ENABLED(CONFIG_ZMK_MOUSE)
int zmk_endpoints_send_mouse_report() {
switch (current_instance.transport) {
#if IS_ENABLED(CONFIG_ZMK_USB)
case ZMK_TRANSPORT_USB: {
#if IS_ENABLED(CONFIG_ZMK_USB)
int err = zmk_usb_hid_send_mouse_report();
if (err) {
LOG_ERR("FAILED TO SEND OVER USB: %d", err);
}
return err;
}
#else
LOG_ERR("USB endpoint is not supported");
return -ENOTSUP;
#endif /* IS_ENABLED(CONFIG_ZMK_USB) */
}

#if IS_ENABLED(CONFIG_ZMK_BLE)
case ZMK_TRANSPORT_BLE: {
#if IS_ENABLED(CONFIG_ZMK_BLE)
struct zmk_hid_mouse_report *mouse_report = zmk_hid_get_mouse_report();
int err = zmk_hog_send_mouse_report(&mouse_report->body);
if (err) {
LOG_ERR("FAILED TO SEND OVER HOG: %d", err);
}
return err;
}
#else
LOG_ERR("BLE HOG endpoint is not supported");
return -ENOTSUP;
#endif /* IS_ENABLED(CONFIG_ZMK_BLE) */
}
}

LOG_ERR("Unsupported endpoint transport %d", current_instance.transport);
LOG_ERR("Unhandled endpoint transport %d", current_instance.transport);
return -ENOTSUP;
}
#endif // IS_ENABLED(CONFIG_ZMK_MOUSE)
Expand Down
Loading