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

Adding Dygma Raise #13543

Merged
merged 14 commits into from
Jan 9, 2022
Merged

Adding Dygma Raise #13543

merged 14 commits into from
Jan 9, 2022

Conversation

ibash
Copy link

@ibash ibash commented Jul 14, 2021

This adds support for the Dygma Raise. Since QMK doesn't yet support the SAMD21 MCU (which the neuron uses), instead this uses a STM32 Blackpill that connects to each half.

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

This adds support for the Dygma Raise. Since QMK doesn't yet support the
SAMD21 MCU, instead this uses a STM32 Blackpill that connects to each
half.
Copy link

@jjerrell jjerrell left a comment

Choose a reason for hiding this comment

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

js is sort of foreign to me but all else looks good!

LAYOUT_all is just a copy of the iso layout, but as per docs, this is
recommended.
@ibash
Copy link
Author

ibash commented Jul 17, 2021

@drashna could I get a review?

keyboards/dygma/raise/config.h Outdated Show resolved Hide resolved
keyboards/dygma/raise/config.h Outdated Show resolved Hide resolved
keyboards/dygma/raise/create-led-config.js Outdated Show resolved Hide resolved
keyboards/dygma/raise/keymaps/ansi/keymap.c Outdated Show resolved Hide resolved
keyboards/dygma/raise/matrix.c Outdated Show resolved Hide resolved
keyboards/dygma/raise/readme.md Outdated Show resolved Hide resolved
keyboards/dygma/raise/raise.c Outdated Show resolved Hide resolved
keyboards/dygma/raise/raise.c Outdated Show resolved Hide resolved
keyboards/dygma/raise/matrix.c Outdated Show resolved Hide resolved
keyboards/dygma/raise/matrix.c Outdated Show resolved Hide resolved
@zvecr zvecr requested a review from a team July 17, 2021 23:46
ibash and others added 3 commits July 19, 2021 02:07
Co-authored-by: Joel Challis <git@zvecr.com>
1. Move keyboard to handwired
2. Consolidate led config away from keymaps
3. Add copyright headers
@ibash ibash requested review from zvecr and removed request for a team July 19, 2021 09:43
@ibash
Copy link
Author

ibash commented Jul 19, 2021

Thanks @zvecr -- back to you!

This fixes a bug with leds not turning off completely
@elfmimi
Copy link
Contributor

elfmimi commented Aug 7, 2021

What will happen when proper SAMD21 arrives? I mean, will this STM32 approach go away and give its way to SAMD21 one?

oh, I noticed. this one is placed under keyboards/handwired .
so more proper one should come in as keyboards/dygma/raise . then it's all alright.

@drashna drashna requested a review from a team August 8, 2021 04:56
@ibash ibash requested review from drashna and removed request for a team August 15, 2021 00:38
@ibash
Copy link
Author

ibash commented Aug 15, 2021

@drashna || @zvecr can this be merged?

@drashna drashna requested a review from a team August 16, 2021 13:34
keyboards/handwired/dygma/raise/keymaps/ansi/readme.md Outdated Show resolved Hide resolved
keyboards/handwired/dygma/raise/keymaps/ansi/keymap.c Outdated Show resolved Hide resolved
keyboards/handwired/dygma/raise/keymaps/default/keymap.c Outdated Show resolved Hide resolved
keyboards/handwired/dygma/raise/keymaps/default/readme.md Outdated Show resolved Hide resolved
keyboards/handwired/dygma/raise/keymaps/iso/keymap.c Outdated Show resolved Hide resolved
keyboards/handwired/dygma/raise/keymaps/iso/readme.md Outdated Show resolved Hide resolved
keyboards/handwired/dygma/raise/raise.c Outdated Show resolved Hide resolved
keyboards/handwired/dygma/raise/readme.md Outdated Show resolved Hide resolved
keyboards/handwired/dygma/raise/rules.mk Outdated Show resolved Hide resolved
keyboards/handwired/dygma/raise/create-led-config.js Outdated Show resolved Hide resolved
ibash and others added 2 commits August 16, 2021 10:36
@ibash ibash requested a review from fauxpark August 16, 2021 23:47
@ibash
Copy link
Author

ibash commented Aug 22, 2021

@fauxpark we previously discussed converting to matrix lite, most of the matrix lite functionality would have to be bypassed, which doesn't make much sense: https://github.com/qmk/qmk_firmware/pull/13543/files#r680152929

Is that a blocker?


The qmk build files still confuse me, can you be more specific on what should go in what file? Is this right?

  1. handwired/dygma/raise/ansi/info.json

    • Copy of info.json but only with LAYOUT_ansi and not with LAYOUT_iso
    • Should this have LAYOUT_all or not?
  2. handwired/dygma/raise/ansi/ansi.h

    • #define LAYOUT_ansi(
    • Should this have LAYOUT_all or not?
  3. handwired/dygma/raise/iso/info.json

    • Copy of info.json but only with LAYOUT_iso and not with LAYOUT_ansi
    • Should this have LAYOUT_all or not?
  4. handwired/dygma/raise/iso/iso.h

    • #define LAYOUT_iso(
    • Should this have LAYOUT_all or not?
  5. Should keymaps also be moved to subfolders?

  6. Should I delete LAYOUT_all then?


Any other changes required to get this merged?

@fauxpark
Copy link
Member

1 to 4 are correct, and each should have LAYOUT_all.
5. Given that there seems to be a hardware difference between the ANSI and ISO "revisions", I'd say yes, they each need their own keymaps folder.
6. Nope.

@ibash
Copy link
Author

ibash commented Sep 13, 2021

I have the changes on a branch, just need to test it. Coming soon!

keyboards/handwired/dygma/raise/rules.mk Outdated Show resolved Hide resolved
keyboards/handwired/dygma/raise/raise.h Outdated Show resolved Hide resolved
keyboards/handwired/dygma/raise/raise.h Show resolved Hide resolved
keyboards/handwired/dygma/raise/iso/info.json Outdated Show resolved Hide resolved
keyboards/handwired/dygma/raise/ansi/info.json Outdated Show resolved Hide resolved
keyboards/handwired/dygma/raise/readme.md Outdated Show resolved Hide resolved
Co-authored-by: Ryan <fauxpark@gmail.com>
@ibash
Copy link
Author

ibash commented Sep 21, 2021

Thanks for the quick review @fauxpark! Applied and I tested it on hardware.

@ibash
Copy link
Author

ibash commented Sep 29, 2021

@fauxpark any chance this can be merged?

@stale
Copy link

stale bot commented Jan 3, 2022

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 awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@ibash
Copy link
Author

ibash commented Jan 4, 2022

Bump

@ibash ibash requested a review from drashna January 4, 2022 18:01
@stale stale bot removed the awaiting changes label Jan 4, 2022
keyboards/handwired/dygma/raise/rules.mk Outdated Show resolved Hide resolved
# Build Options
# change yes to no to disable
#
BOOTMAGIC_ENABLE = no # Enable Bootmagic Lite
Copy link
Member

Choose a reason for hiding this comment

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

We do highly recommend enabling this, since it adds a way to get into the bootloader to reflash, if a reset button isn't readily accessible.

keyboards/handwired/dygma/raise/readme.md Outdated Show resolved Hide resolved
ibash and others added 2 commits January 4, 2022 13:55
Co-authored-by: Drashna Jaelre <drashna@live.com>
Co-authored-by: Drashna Jaelre <drashna@live.com>
@ibash ibash requested a review from drashna January 4, 2022 21:55
@drashna drashna requested a review from a team January 5, 2022 00:06
@ibash ibash removed the request for review from a team January 5, 2022 00:22
Copy link
Contributor

@ridingqwerty ridingqwerty left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@drashna
Copy link
Member

drashna commented Jan 9, 2022

Thanks!

@drashna drashna merged commit ec78aca into qmk:master Jan 9, 2022
ryphon pushed a commit to ryphon/qmk_firmware that referenced this pull request Jan 12, 2022
Co-authored-by: Joel Challis <git@zvecr.com>
Co-authored-by: Ryan <fauxpark@gmail.com>
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

7 participants