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

Add Durgod Taurus K310 keyboard #12314

Merged
merged 7 commits into from
Jul 22, 2021
Merged

Add Durgod Taurus K310 keyboard #12314

merged 7 commits into from
Jul 22, 2021

Conversation

dkjer
Copy link

@dkjer dkjer commented Mar 21, 2021

Add support for new keyboard: Durgod Taurus K310 (non-backlit) full-sized 104/105-key ANSI/ISO.

This PR is based off @tylert's PR #12274, and includes additional changes to share code with the Durgod K320 as well as improvements and cleanup from @nomis.

Description

This PR contains the following changes:

  • Add support for Durgod Taurus K310 non-backlit full-sized 104/105-key keyboards.
  • Refactoring shared durgod/k310 and durgod/k320 code into durgod/k3x0.
  • Renaming default_toggle_mac_windows to chimera keymap
  • Enable LTO for all durgod/k3x0 keymaps
  • Jump to bootloader when the hardware reset button is pressed
  • Add Durgod K3x0 LAYOUT_tkl to support qmk_configurator for K320

Types of Changes

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

Issues Fixed or Closed by This PR

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).

@github-actions github-actions bot added keyboard keymap via Adds via keymap and/or updates keyboard for via support labels Mar 21, 2021
@dkjer
Copy link
Author

dkjer commented Mar 21, 2021

Once this code makes its way to master, I will submit a PR to qmk/qmk_configurator with matching changes: https://github.com/dkjer/qmk_configurator/tree/durgod_k3x0

Copy link

@nomis nomis left a comment

Choose a reason for hiding this comment

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

The info.json label for the ` key needs fixing for the ISO layout.

I don't know how the 2 and 3 keys are supposed to be labelled. Other keyboards have @, # which are the scan codes but it means there are two keys labelled #. I don't know why these aren't just labelled with the numbers?

I've not reviewed the mac layouts.

Copy link

@tylert tylert left a comment

Choose a reason for hiding this comment

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

LGTM! 👍 I very much like the look of the cleaner diagrams in the comments too.

keyboards/durgod/k3x0/config.h Show resolved Hide resolved
@tylert
Copy link

tylert commented Mar 21, 2021

For reference, the VIA work for K320 was merged in the-via/keyboards#588.

@dkjer
Copy link
Author

dkjer commented Mar 22, 2021

The info.json label for the ` key needs fixing for the ISO layout.

I don't know how the 2 and 3 keys are supposed to be labelled. Other keyboards have @, # which are the scan codes but it means there are two keys labelled #. I don't know why these aren't just labelled with the numbers?

I've not reviewed the mac layouts.

@nomis I don't believe the labels really matter in the info.json files; I think they are just there for to make it more human readable (please correct me if I'm wrong). We could replace the 2nd # with NUHS, and the \\ with NUBS if that make mores sense?

@dkjer
Copy link
Author

dkjer commented Mar 22, 2021

For reference, the VIA work for K320 was merged in the-via/keyboards#588.

I have a VIA branch ready to add K310 support, as well: https://github.com/dkjer/keyboards/tree/durgod_k310

@tylert Would you mind testing https://github.com/dkjer/keyboards/blob/durgod_k310/src/durgod/k310/k310.json out on a K310 to verify the numpad section of that via keymap has the correct matrix values?

@nomis
Copy link

nomis commented Mar 22, 2021

The info.json file content doesn't really matter for my use; it appears to be ok to do it this way for other keyboards.

@tylert
Copy link

tylert commented Mar 22, 2021

@tylert Would you mind testing https://github.com/dkjer/keyboards/blob/durgod_k310/src/durgod/k310/k310.json out on a K310 to verify the numpad section of that via keymap has the correct matrix values?

Well, the via QMK keymap works fine on an ANSI K310 keyboard when used as a regular keyboard but the 1.3.1 (.deb) version of VIA doesn't automatically detect the K310 and I'm not sure how to build VIA from source yet or sneak a JSON file into the installed version of VIA so I'm unable to do much on the VIA front at the moment. (There might be a minor bug at LGUI on layer1 in the VIA keyboard repo for the ISO K320. The VIA 1.3.1 AppImage also comletely fails to run on Debian 10.x amd64 and the .deb has quite a few rough edges too.)

I'll be doing some more verification of the rest of the QMK keymaps built from commit e631cf618 shortly on the ANSI K310 and K320 keyboards and look at fighting with VIA again later.

@dkjer
Copy link
Author

dkjer commented Mar 22, 2021

Well, the via QMK keymap works fine on an ANSI K310 keyboard when used as a regular keyboard but the 1.3.1 (.deb) version of VIA doesn't automatically detect the K310 and I'm not sure how to build VIA from source yet or sneak a JSON file into the installed version of VIA so I'm unable to do much on the VIA front at the moment.

On the Mac version of VIA, you can 'Import Keymap' and just give it the k310.json file from that branch. It worked for me on a K320, but of course I couldn't check the numpad section.

@tylert
Copy link

tylert commented Mar 22, 2021

Well, the via QMK keymap works fine on an ANSI K310 keyboard when used as a regular keyboard but the 1.3.1 (.deb) version of VIA doesn't automatically detect the K310 and I'm not sure how to build VIA from source yet or sneak a JSON file into the installed version of VIA so I'm unable to do much on the VIA front at the moment.

On the Mac version of VIA, you can 'Import Keymap' and just give it the k310.json file from that branch. It worked for me on a K320, but of course I couldn't check the numpad section.

Ah, that worked great, thanks. Yes, VIA recognized the via-enabled K310 with that imported JSON and the key tester and matrix tester checks out on all keys, including the numpad.

The LGUI key on layer1 shows up as 0x5d2b just as it does on the K320.

@dkjer
Copy link
Author

dkjer commented Mar 22, 2021

The LGUI key on layer1 shows up as 0x5d2b just as it does on the K320.

I don't think there is a way around that since VIA doesn't know about our custom keycodes, besides removing the Windows-Key lock functionality from VIA. I don't think it hurts anything, and it should be able to be overridden if someone doesn't want that functionality.

@drashna
Copy link
Member

drashna commented Apr 19, 2021

I don't think there is a way around that since VIA doesn't know about our custom keycodes, besides removing the Windows-Key lock functionality from VIA. I don't think it hurts anything, and it should be able to be overridden if someone doesn't want that functionality.

Actually, there is. VIA supports up to 16 custom keycodes. If you use USER00 instead of SAFE_RANGE when via is enabled, there is a "customKeycodes" section that is supported for the VIA json file (if you check out the ploopy mouse/trackball you can see this).

@dkjer
Copy link
Author

dkjer commented Apr 19, 2021

I don't think there is a way around that since VIA doesn't know about our custom keycodes, besides removing the Windows-Key lock functionality from VIA. I don't think it hurts anything, and it should be able to be overridden if someone doesn't want that functionality.

Actually, there is. VIA supports up to 16 custom keycodes. If you use USER00 instead of SAFE_RANGE when via is enabled, there is a "customKeycodes" section that is supported for the VIA json file (if you check out the ploopy mouse/trackball you can see this).

Nice, I’ll take a look at that. Thanks!

@drashna
Copy link
Member

drashna commented Apr 19, 2021

It's a bit messy, but:

enum ploopy_keycodes {
#ifdef VIA_ENABLE
DPI_CONFIG = USER00,
#else
DPI_CONFIG = SAFE_RANGE,
#endif
DRAG_SCROLL,
#ifdef VIA_ENABLE
PLOOPY_SAFE_RANGE = SAFE_RANGE,
#else
PLOOPY_SAFE_RANGE,
#endif
};

And the via repo:
https://github.com/the-via/keyboards/blob/3ccde52851f22785d9eaf098cdd353feb1a6559d/src/ploopyco/trackball/trackball.json#L6-L9

@dkjer
Copy link
Author

dkjer commented Apr 20, 2021

Rebase off develop

Converted WinLock key to a custom VIA key. Fixed default_mac layout.

New VIA keyboard changes to match: https://github.com/dkjer/keyboards/tree/durgod_k310

@dkjer
Copy link
Author

dkjer commented May 11, 2021

Rebasing off develop

@github-actions github-actions bot removed the core label May 12, 2021
@drashna drashna requested a review from a team May 14, 2021 02:17
@tylert
Copy link

tylert commented Jun 2, 2021

Is there anything else preventing this PR from being merged?

tylert and others added 5 commits June 20, 2021 02:42
… durgod/k3x0.

Combining chimera keymap with default_toggle_mac_windows keymap
* Remove NUM_LOCK from the K320
* Remove row 8 from the K320
* Enable LTO for all keymaps
** This increases the matrix scan rate by making it more efficient
* Jump to bootloader when the hardware reset button is pressed
* Add extra hardware reset button check
** The interrupt is edge-triggered so check that it's not already pressed
* Moving current impl to 'base' variant to make space for future variants
* Fixing compiler error due to SH_T define
* Adding custom VIA WinLock key
@dkjer
Copy link
Author

dkjer commented Jun 20, 2021

Rebasing off develop

keyboards/durgod/k3x0/readme.md Outdated Show resolved Hide resolved
keyboards/durgod/k3x0/readme.md Outdated Show resolved Hide resolved
keyboards/durgod/k3x0/k3x0.h Outdated Show resolved Hide resolved
keyboards/durgod/k3x0/keymaps/chimera/keymap.c Outdated Show resolved Hide resolved
@dkjer dkjer requested a review from fauxpark July 7, 2021 21:33
@fauxpark fauxpark requested a review from a team July 8, 2021 12:18
@dkjer
Copy link
Author

dkjer commented Jul 10, 2021 via email

@b10102016
Copy link

b10102016 commented Jul 10, 2021 via email

@drashna
Copy link
Member

drashna commented Jul 22, 2021

Thanks!

@drashna drashna merged commit 13a5149 into qmk:develop Jul 22, 2021
cadusk pushed a commit to cadusk/qmk_firmware that referenced this pull request Jul 22, 2021
* qmk/develop: (141 commits)
  Implement GPIO abstraction for atsam (qmk#13567)
  [Keyboard][Bug] Add timer_avr to includes for broken builds (qmk#13641)
  [Keyboard] adds new revision: dztech/dz60rgb/v2_1 (qmk#13636)
  [Keyboard] Fixing info.json for h0oni/hotduck (qmk#13640)
  [Keyboard] Ymd40v2new layouts and fixed per-switch backlight (qmk#13622)
  [Bug] Develop - DC01 left (qmk#13597)
  [Keyboard] Fix clawsome/hatchback and reviung5 compile issues (qmk#13607)
  [Keyboard] add ogurec (qmk#13242)
  [Keyboard] Add Durgod Taurus K310 keyboard (qmk#12314)
  [Keyboard] Updated keyboard & keymaps (qmk#12667)
  [Keyboard] add SPRH keyboard (qmk#12999)
  Retain brightness with lighting layers (qmk#13025)
  [Keyboard] Add keypad Satxri6key (qmk#13423)
  [Keyboard] Initial Tron Guy Labs keyboard implementation. (qmk#13438)
  [Keyboard] Add Ristretto Keyboard (qmk#13479)
  [Keyboard] New Keyboard - OBE (qmk#13545)
  [Keyboard] Add personal planck keymap (qmk#13635)
  [Keymap] narze/xd004 (qmk#13634)
  [Keyboard] Add h0oni hotduck keyboard (qmk#13609)
  Fix API generation failure caused by GRS-70EC (qmk#13631)
  ...
@tylert
Copy link

tylert commented Jul 22, 2021

Thanks again to all the awesome QMK contributors.

@dkjer I'll retest all the ANSI K320 and ANSI K310 "chimera" and "typhon" keymaps again in a few days off the latest develop branch and I'll give it another try after it makes it to master too (hopefully during the next "breaking change" release in a mere few weeks :); Aug 28, 2021 perhaps?). I'm not expecting any surprises but I'll keep you posted.

I might give that VIA stuff another try sometime after https://github.com/dkjer/keyboards/tree/durgod_k310 gets merged too.

nhongooi pushed a commit to nhongooi/qmk_firmware that referenced this pull request Dec 5, 2021
Co-authored-by: Tyler Tidman <[email protected]>
Co-authored-by: Simon Arlott <sa.me.uk>
BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
Co-authored-by: Tyler Tidman <[email protected]>
Co-authored-by: Simon Arlott <sa.me.uk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review keyboard keymap via Adds via keymap and/or updates keyboard for via support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants