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

Implement KEY_INDEX and add getKeyswitchStateAtPosition to the HID facade #336

Merged
merged 3 commits into from Jun 9, 2018

Conversation

algernon
Copy link
Contributor

@algernon algernon commented Jun 9, 2018

To be used by the hardware implementations, KEY_INDEX tells us the index of a key, from which we can figure out the row and column as needed. The index starts at one, so that plugins that work with a list of key indexes can use zero as a sentinel. This is important, because when we initialize arrays with fewer elements than the declared array size, the remaining elements will be zero. We can use this to avoid having to explicitly add a sentinel in user-facing code.

Additionally, we add getKeyswitchStateAtPosition to the HID facade. See its documentation in Kaleidoscope-Hardware.

…cade

To be used by the hardware implementations, `KEY_INDEX` tells us the index of a
key, from which we can figure out the row and column as needed. The index starts
at one, so that plugins that work with a list of key indexes can use zero as a
sentinel. This is important, because when we initialize arrays with fewer
elements than the declared array size, the remaining elements will be zero. We
can use this to avoid having to explicitly add a sentinel in user-facing code.

Additionally, we add `getKeyswitchStateAtPosition` to the HID facade. See its
documentation in `Kaleidoscope-Hardware`.

Signed-off-by: Gergely Nagy <algernon@keyboard.io>
@noseglasses
Copy link
Collaborator

Why not use a constexpr function keyIndex instead of a function macro? That way it would be type safe and could be used at compile time.

@algernon
Copy link
Contributor Author

algernon commented Jun 9, 2018

One issue with using a constexpr keyIndex is that then I need to move the function out of Kaleidoscope, and into the hardware plugin, because it needs access to COLS and ROWS. With a macro, that's evaluated late, when defining the RxCy macros, by which time they are available.

Mind you, moving keyIndex to the hardware plugin is probably not wrong.

To allow the hardware plugin to use a more efficient way of representing the
index (if need be), and to be able to turn it into a constexpr, move it to the
hardware plugin.

Signed-off-by: Gergely Nagy <algernon@keyboard.io>
Signed-off-by: Gergely Nagy <algernon@keyboard.io>
@obra obra merged commit a54b139 into master Jun 9, 2018
@obra obra deleted the f/getKeyswitchStateAtPosition branch June 9, 2018 22:29
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.

None yet

3 participants