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

LED Indicators (bortoz work with minor fixes) #2041

Merged
merged 3 commits into from
Nov 28, 2023

Conversation

petejohanson
Copy link
Contributor

@petejohanson petejohanson commented Nov 27, 2023

Creating this PR to follow up on some fixed identified as needed in #999. Thanks so much to @bortoz for sticking with this for 2 whole years. Hoping to help get this across the finish line without wasting any more of their time.

In particular:

  • Put the core feature, and split state propagation, behind two new Kconfig flags, with updated conditional bits as needed. Added the options to the system configuration docs page.
  • Rebase to address current conflicts with main and integrate the HID defines added in refactor(hid): Use proper defines for HID values. #1993
  • Tweaked the GATT service structure a bit to put the input GATT attributes after the ones we notify on, to avoid attr numbering funkiness.
  • Added the small split fix from chrisandreae@bc824de
  • Minor refactor for the type naming of zmk_hid_indicators.

@petejohanson petejohanson added enhancement New feature or request core Core functionality/behavior of ZMK usb labels Nov 27, 2023
@petejohanson petejohanson self-assigned this Nov 27, 2023
@petejohanson petejohanson requested a review from a team as a code owner November 27, 2023 22:48
| `CONFIG_ZMK_HID_CONSUMER_REPORT_SIZE` | int | Number of consumer keys simultaneously reportable | 6 |
| Config | Type | Description | Default |
| ------------------------------------- | ---- | -------------------------------------------------------------- | ------- |
| `CONFIG_ZMK_HID_INDICATORS` | bool | Enable reciept of HID/LED indicator state from connected hosts | n |
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
| `CONFIG_ZMK_HID_INDICATORS` | bool | Enable reciept of HID/LED indicator state from connected hosts | n |
| `CONFIG_ZMK_HID_INDICATORS` | bool | Enable receipt of HID/LED indicator state from connected hosts | n |

Is there a reason not to default this to on?

Copy link
Contributor Author

@petejohanson petejohanson Nov 27, 2023

Choose a reason for hiding this comment

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

After the USB BOOT work caused some unexpected issues (for you even), I'm wary to enable this by default until the actual version merged into ZMK main gets a bit more extensive testing by folks.

@petejohanson petejohanson merged commit 817ce87 into zmkfirmware:main Nov 28, 2023
84 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core functionality/behavior of ZMK enhancement New feature or request usb
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants