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

[Core] Rework PS/2 driver selection #17892

Merged
merged 8 commits into from
Aug 31, 2022
Merged

Conversation

gamelaster
Copy link
Contributor

Description

PS/2 driver selection in common_features.mk was using old approach (PS2_USE_INT, PS2_USE_BUSYWAIT etc.).
This PR reworks this to use same approach as Serial or WS2812 drivers, so it also supports vendor drivers.

This PR is first stage of merging my PS/2 driver for RP2040, which uses PIO. The second PR will contain the driver itself, but requires this PR to work.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

Enabling and selecting PS/2 driver was using old approach,
so it was reworked to current approach, inspired by Serial
and WS2812 driver selections.
@gamelaster gamelaster changed the title PS/2 Driver selection rework [Core] Rework PS/2 driver selection Aug 3, 2022
@gamelaster gamelaster mentioned this pull request Aug 3, 2022
14 tasks
@KarlK90 KarlK90 requested a review from a team August 3, 2022 14:15
Copy link
Member

@KarlK90 KarlK90 left a comment

Choose a reason for hiding this comment

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

@gamelaster Thanks for your PR! I finally had the time to take a better look and these changes look good to me.

@KarlK90
Copy link
Member

KarlK90 commented Aug 29, 2022

One thing I would like you to add to this PR is the addition of the PS/2 driver selection to the data driven config mappings in https://github.com/qmk/qmk_firmware/blob/develop/data/mappings/info_rules.json#L33-L34 just like it has been done for the steno protocols. Thank you.

@KarlK90 KarlK90 requested a review from a team August 29, 2022 18:03
@gamelaster
Copy link
Contributor Author

Hi @KarlK90 , I fixed one indentation mistake, and implemented PS2 driver selection to data driver config mappings as you suggested. Please, let me know if it looks good. Thanks 😊

Co-authored-by: Drashna Jaelre <drashna@live.com>
@KarlK90
Copy link
Member

KarlK90 commented Aug 30, 2022

@gamelaster Great job, works like a charm. While playing around with the changes I recognized that we should probably add the clock and data pins to the data driven mappings as well, so for a "simple" setup we don't need to touch config.h at all. I've attached the necessary changes as a patch -> clock_data_pin.diff.txt

@KarlK90 KarlK90 merged commit 0237ff0 into qmk:develop Aug 31, 2022
@KarlK90
Copy link
Member

KarlK90 commented Aug 31, 2022

@gamelaster merged! Thanks for adding the pins.

ramonimbao pushed a commit to ramonimbao/qmk_firmware that referenced this pull request Nov 28, 2022
* [Core] Rework PS/2 driver selection

Enabling and selecting PS/2 driver was using old approach,
so it was reworked to current approach, inspired by Serial
and WS2812 driver selections.

* [Keyboard] Update keyboards using PS/2 to use new PS/2 driver selection

* [Docs] Update PS/2 documentation to use new PS/2 driver selection

* Fix indentation

* [Core] Add PS2 to data driver

* Fix oversight in property name

Co-authored-by: Drashna Jaelre <drashna@live.com>

* Add PS/2 pins to data driven mappings

Co-authored-by: Drashna Jaelre <drashna@live.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants