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

Optical keyboard #17852

Closed
wants to merge 20 commits into from
Closed

Optical keyboard #17852

wants to merge 20 commits into from

Conversation

girishji
Copy link

Optical keyboard with Ergo layout

Description

This is an optical matrix keyboard corresponding to pcb here: https://github.com/girishji/optical-keyboard. It uses Gateron optical switches with a blackpill stm32f401 mcu. There is also a custom matrix to drive the Led's and PT's.

Types of Changes

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

Issues Fixed or Closed by This PR

Checklist

  • [x ] My code follows the code style of this project: C, Python
  • [ x] 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.
  • [ x] I have tested the changes and verified that they work and don't break anything (as well as I can manage).

M  quantum/matrix.c
M  quantum/matrix.c
Keyboard suitable for Gateron optical switches. Columns are Output
  pins that drive Led and PT, while row pins are Input pins that
  read status (0 if key not pressed, 1 if pressed)

A  keyboards/opticalkb/blackpill_f401.c
A  keyboards/opticalkb/chconf.h
A  keyboards/opticalkb/config.h
A  keyboards/opticalkb/halconf.h
A  keyboards/opticalkb/info.json
A  keyboards/opticalkb/keymaps/default/keymap.c
A  keyboards/opticalkb/keymaps/default/readme.md
A  keyboards/opticalkb/matrix.c
A  keyboards/opticalkb/mcuconf.h
A  keyboards/opticalkb/opticalkb.c
A  keyboards/opticalkb/opticalkb.h
A  keyboards/opticalkb/readme.md
A  keyboards/opticalkb/rules.mk
M  keyboards/opticalkb/readme.md
@drashna
Copy link
Member

drashna commented Jul 30, 2022

Lint has flagged a few issues. Please resolve.

Also, add an option to use pullup at PT

M  keyboards/opticalkb/config.h
M  keyboards/opticalkb/info.json
M  keyboards/opticalkb/matrix.c
M  keyboards/opticalkb/rules.mk
@girishji
Copy link
Author

Fixed the lint issue

@girishji
Copy link
Author

girishji commented Jul 31, 2022 via email

M  keyboards/opticalkb/config.h
M  keyboards/opticalkb/matrix.c
Copy link
Member

@zvecr zvecr left a comment

Choose a reason for hiding this comment

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

  • should use custom matrix "lite"
    • currently not implementing debounce correctly
    • unnecessary duplication of code
  • info.json render does not match keyboard layout

keyboards/opticalkb/blackpill_f401.c Outdated Show resolved Hide resolved
keyboards/opticalkb/rules.mk Outdated Show resolved Hide resolved
keyboards/opticalkb/mcuconf.h Outdated Show resolved Hide resolved
keyboards/opticalkb/halconf.h Outdated Show resolved Hide resolved
keyboards/opticalkb/chconf.h Outdated Show resolved Hide resolved
@girishji
Copy link
Author

girishji commented Aug 1, 2022

  • should use custom matrix "lite"

    • currently not implementing debounce correctly

I am using an optical matrix. Optical switches do not need debounce. It takes about 15us for phototransistor to rise and IO pin will read accurate values after that. I get stable switching even at very high scan rates (delay of 20us between reading columns). Debounce code is not applicable here. No need to keep 2 matrix copies and deal with timers. This will slow down scan rate.

  • unnecessary duplication of code

The duplication is unavoidable unless code is refactored in matrix_common

  • info.json render does not match keyboard layout

I need some help here. How is this checked? I see 5x15 keys in info.json and it matches matrix definition

D  keyboards/opticalkb/blackpill_f401.c
D  keyboards/opticalkb/chconf.h
D  keyboards/opticalkb/halconf.h
 M keyboards/opticalkb/info.json
 M keyboards/opticalkb/keymaps/default/keymap.c
D  keyboards/opticalkb/mcuconf.h
 M keyboards/opticalkb/rules.mk
@girishji
Copy link
Author

girishji commented Aug 1, 2022 via email

M  keyboards/opticalkb/config.h
M  keyboards/opticalkb/info.json
M  keyboards/opticalkb/keymaps/default/keymap.c
M  keyboards/opticalkb/rules.mk
@zvecr
Copy link
Member

zvecr commented Aug 1, 2022

I am using an optical matrix. Optical switches do not need debounce. It takes about 15us for phototransistor to rise and IO pin will read accurate values after that

You should still implement it correctly, but set the DEBOUNCE config option to zero.

The duplication is unavoidable unless code is refactored in matrix_common

Using lite removes the duplication of various things, which is why its been suggested.

@zvecr
Copy link
Member

zvecr commented Aug 1, 2022

Layout Layout:
┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐
│K0││K0││K0││K0││K0││K0││K0││K0││K0││K0││K0││K0││K0││K0││K0│
└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘
┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐
│K1││K1││K1││K1││K1││K1││K1││K1││K1││K1││K1││K1││K1││K1││K1│
└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘
┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐
│K2││K2││K2││K2││K2││K2││K2││K2││K2││K2││K2││K2││K2││K2││K2│
└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘
┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐
│K3││K3││K3││K3││K3││K3││K3││K3││K3││K3││K3││K3││K3││K3││K3│
└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘
┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐
│K4││K4││K4││K4││K4││K4││K4││K4││K4││K4││K4││K4││K4││K4││K4│
└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘

does not represent what is shown in

image

keyboards/opticalkb/keymaps/default/keymap.c Outdated Show resolved Hide resolved
keyboards/opticalkb/keymaps/default/keymap.c Outdated Show resolved Hide resolved
keyboards/opticalkb/rules.mk Outdated Show resolved Hide resolved
keyboards/opticalkb/rules.mk Outdated Show resolved Hide resolved
keyboards/opticalkb/readme.md Outdated Show resolved Hide resolved
keyboards/opticalkb/readme.md Outdated Show resolved Hide resolved
Created a rev1 directory and moved keyboard files there. This creates
flexibility to add another revision to keyboard.
Changed matrix implementation to 'lite' as requested
Fixed info.json to reflext pcb geometry

RM keyboards/opticalkb/config.h -> keyboards/opticalkb/rev1/config.h
RM keyboards/opticalkb/info.json -> keyboards/opticalkb/rev1/info.json
R  keyboards/opticalkb/keymaps/default/keymap.c -> keyboards/opticalkb/rev1/keymaps/default/keymap.c
R  keyboards/opticalkb/keymaps/default/readme.md -> keyboards/opticalkb/rev1/keymaps/default/readme.md
RM keyboards/opticalkb/matrix.c -> keyboards/opticalkb/rev1/matrix.c
RM keyboards/opticalkb/readme.md -> keyboards/opticalkb/rev1/readme.md
RM keyboards/opticalkb/opticalkb.c -> keyboards/opticalkb/rev1/rev1.c
R  keyboards/opticalkb/opticalkb.h -> keyboards/opticalkb/rev1/rev1.h
RM keyboards/opticalkb/rules.mk -> keyboards/opticalkb/rev1/rules.mk
All suggested changes requested in pull request review

M  keyboards/opticalkb/rev1/config.h
M  keyboards/opticalkb/rev1/info.json
M  keyboards/opticalkb/rev1/keymaps/default/keymap.c
M  keyboards/opticalkb/rev1/matrix.c
M  keyboards/opticalkb/rev1/readme.md
M  keyboards/opticalkb/rev1/rev1.c
M  keyboards/opticalkb/rev1/rules.mk
@girishji
Copy link
Author

girishji commented Aug 2, 2022

Layout Layout:
┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐
│K0││K0││K0││K0││K0││K0││K0││K0││K0││K0││K0││K0││K0││K0││K0│
└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘
┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐
│K1││K1││K1││K1││K1││K1││K1││K1││K1││K1││K1││K1││K1││K1││K1│
└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘
┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐
│K2││K2││K2││K2││K2││K2││K2││K2││K2││K2││K2││K2││K2││K2││K2│
└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘
┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐
│K3││K3││K3││K3││K3││K3││K3││K3││K3││K3││K3││K3││K3││K3││K3│
└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘
┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐┌──┐
│K4││K4││K4││K4││K4││K4││K4││K4││K4││K4││K4││K4││K4││K4││K4│
└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘└──┘

does not represent what is shown in

Thanks, changed it. I am not sure how I can verify this graphically though, but changed the geometry values

@girishji
Copy link
Author

girishji commented Aug 2, 2022

I am using an optical matrix. Optical switches do not need debounce. It takes about 15us for phototransistor to rise and IO pin will read accurate values after that

You should still implement it correctly, but set the DEBOUNCE config option to zero.

The duplication is unavoidable unless code is refactored in matrix_common

Using lite removes the duplication of various things, which is why its been suggested.

Thanks, changed as requested. I am glad setting DEBOUNCE to 0 eliminates all debouce related processing

M  keyboards/opticalkb/rev1/readme.md
M  keyboards/opticalkb/rev1/rules.mk
@girishji
Copy link
Author

girishji commented Aug 2, 2022

Thank you, I think I have incorporated all suggestions

To reduce power consumption one can increase time during which
IR is not powered

M  keyboards/opticalkb/rev1/matrix.c
@girishji girishji requested a review from zvecr August 3, 2022 08:56
@girishji
Copy link
Author

girishji commented Aug 4, 2022

Please review and merge, thanks :)

This keyboard code references a different pcb,
https://github.com/girishji/optical-keyboard-mx.
The components on this keyboard pcb reflect the delay
times embedded in the scan matrix.
config.h, info.json and keymap files have been changed to reflect
new pin connections on blackpill as well as slightly different
thumb cluster.

M  keyboards/opticalkb/rev1/config.h
M  keyboards/opticalkb/rev1/info.json
M  keyboards/opticalkb/rev1/keymaps/default/keymap.c
M  keyboards/opticalkb/rev1/matrix.c
M  keyboards/opticalkb/rev1/readme.md
M  keyboards/opticalkb/rev1/rev1.h
M  keyboards/opticalkb/rev1/rules.mk
M  keyboards/opticalkb/rev1/readme.md
M  keyboards/opticalkb/rev1/readme.md
keyboards/opticalkb/rev1/config.h Outdated Show resolved Hide resolved
keyboards/opticalkb/rev1/config.h Outdated Show resolved Hide resolved
keyboards/opticalkb/rev1/keymaps/default/keymap.c Outdated Show resolved Hide resolved
- Remove config related to FS USB since it is included by default
- Remove RGB animation config
- Simplify keymap; Users can add bells and whistles as they wish

M  keyboards/opticalkb/rev1/config.h
M  keyboards/opticalkb/rev1/keymaps/default/keymap.c
keyboards/opticalkb/rev1/rules.mk Outdated Show resolved Hide resolved
- Remove unwanted lines
- Add comments to compile flags

M  keyboards/opticalkb/rev1/rules.mk
@github-actions
Copy link

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with bug, awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@github-actions github-actions bot added the stale Issues or pull requests that have become inactive without resolution. label Nov 12, 2022
@girishji girishji requested review from drashna and zvecr and removed request for zvecr and drashna November 12, 2022 10:15
@github-actions github-actions bot removed the stale Issues or pull requests that have become inactive without resolution. label Nov 13, 2022
@drashna drashna requested a review from a team November 19, 2022 21:37
Co-authored-by: Ryan <fauxpark@gmail.com>
@github-actions
Copy link

github-actions bot commented Jan 6, 2023

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with bug, awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@github-actions github-actions bot added the stale Issues or pull requests that have become inactive without resolution. label Jan 6, 2023
@github-actions
Copy link

github-actions bot commented Feb 5, 2023

Thank you for your contribution!
This pull request has been automatically closed because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, or re-open when it's ready.
// [stale-action-closed]

@github-actions github-actions bot closed this Feb 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keyboard keymap stale Issues or pull requests that have become inactive without resolution.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants