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.

Sign up for GitHub

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

Jump to bottom

New Keyboard "KiwiKeebs MacroBoard v1.0" #10080

Merged
merged 25 commits into from
Aug 25, 2020
Merged

New Keyboard "KiwiKeebs MacroBoard v1.0" #10080

merged 25 commits into from
Aug 25, 2020

Conversation

AKiwi92
Copy link
Contributor

@AKiwi92 AKiwi92 commented Aug 18, 2020 •
edited
Loading

Description

My first Git PR! So users can easily use QMK configuration when receiving the board.

Added in a few files for a new keyboard design which I am launching very soon, I want to make sure others can easily use the QMK configurator to customise the board however they like! 👍 Want to get this up so I can test the QMK Configurator as an end user!

Types of Changes

No changes to the core of QMK, only addition of a new keyboard folder for KiwiKeebs MacroBoard v1.0.

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • [x ] Keyboard (addition or update)
  • [x ] 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.
  • [x ] 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).

@AKiwi92 AKiwi92 changed the title Adding new Keyboard "KiwiKeebs MacroBoard v1.0" - So users can easily use QMK configuration when receiving the board New Keyboard "KiwiKeebs MacroBoard v1.0" Aug 18, 2020
zvecr
zvecr requested changes Aug 18, 2020
View reviewed changes
keyboards/kiwikeebsmacro/kiwikeebsmacro.c Outdated Show resolved Hide resolved
keyboards/kiwikeebsmacro/kiwikeebsmacro.h Outdated Show resolved Hide resolved
keyboards/kiwikeebsmacro/readme.md Outdated Show resolved Hide resolved
keyboards/kiwikeebsmacro/keymaps/default/readme.md Outdated Show resolved Hide resolved
keyboards/kiwikeebsmacro/keymaps/default/keymap.c Outdated Show resolved Hide resolved
keyboards/kiwikeebsmacro/keymaps/default/keymap.c Outdated Show resolved Hide resolved
keyboards/kiwikeebsmacro/info.json Outdated Show resolved Hide resolved
keyboards/kiwikeebsmacro/config.h Outdated Show resolved Hide resolved
@zvecr zvecr added the keyboard label Aug 18, 2020
@zvecr zvecr requested a review from a team August 18, 2020 23:31
noroadsleft
noroadsleft reviewed Aug 19, 2020
View reviewed changes
Copy link
Member

@noroadsleft noroadsleft left a comment

Choose a reason for hiding this comment

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

Just a general comment for now:

If you think you may do other devices under the KiwiKeebs branding, you may want to use a vendor folder (keyboards/kiwikeebs/macro/ instead of keyboards/kiwikeebsmacro/).

AKiwi92 and others added 7 commits August 19, 2020 10:03
@AKiwi92
Copy link
Contributor Author

AKiwi92 commented Aug 19, 2020

Just a general comment for now:

If you think you may do other devices under the KiwiKeebs branding, you may want to use a vendor folder (keyboards/kiwikeebs/macro/ instead of keyboards/kiwikeebsmacro/).

Great idea, not sure if I will but never say never!

noroadsleft
noroadsleft requested changes Aug 19, 2020
View reviewed changes
keyboards/kiwikeebs/macro/config.h Outdated Show resolved Hide resolved
keyboards/kiwikeebs/macro/info.json Outdated Show resolved Hide resolved
keyboards/kiwikeebs/macro/keymaps/default/keymap.c Outdated Show resolved Hide resolved
keyboards/kiwikeebs/macro/keymaps/default/keymap.c Outdated Show resolved Hide resolved
@AKiwi92
Copy link
Contributor Author

AKiwi92 commented Aug 19, 2020

Okay so apart from the outstanding placement of the encoder script are we looking okay?

I am unsure of how to progress with that part, is there anyway to setup the encoder to use variables that the user can pass into buttons on QMK configurator so they can still configure it using the GUI?

noroadsleft
noroadsleft approved these changes Aug 20, 2020
View reviewed changes
Copy link
Member

@noroadsleft noroadsleft 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.

@noroadsleft
Copy link
Member

Okay so apart from the outstanding placement of the encoder script are we looking okay?

I am unsure of how to progress with that part, is there anyway to setup the encoder to use variables that the user can pass into buttons on QMK configurator so they can still configure it using the GUI?

Not at this time. I think this is something QMK will have to solve.

For now, my only suggestion is to change encoder_update_user to encoder_update_kb in macro.c.

@AKiwi92
Copy link
Contributor Author

AKiwi92 commented Aug 20, 2020

Okay so apart from the outstanding placement of the encoder script are we looking okay?
I am unsure of how to progress with that part, is there anyway to setup the encoder to use variables that the user can pass into buttons on QMK configurator so they can still configure it using the GUI?

Not at this time. I think this is something QMK will have to solve.

For now, my only suggestion is to change encoder_update_user to encoder_update_kb in macro.c.

In that case we will go for that! Thanks for your help, what is the process now to get this merged so I can test it on the actual configurator? Sorry newbie to PR here!

@AKiwi92 AKiwi92 requested a review from zvecr August 20, 2020 10:28
noroadsleft
noroadsleft requested changes Aug 21, 2020
View reviewed changes
Copy link
Member

@noroadsleft noroadsleft left a comment

Choose a reason for hiding this comment

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

A couple of previous changes seem to have been reverted.

keyboards/kiwikeebs/macro/config.h Outdated Show resolved Hide resolved
keyboards/kiwikeebs/macro/info.json Outdated Show resolved Hide resolved
@AKiwi92
Copy link
Contributor Author

AKiwi92 commented Aug 24, 2020

A couple of previous changes seem to have been reverted.

Apologies I think something got a little muddled in the middle of all of this, should be good now I hope 👍

Cheers,
Ash

@noroadsleft noroadsleft requested a review from a team August 25, 2020 02:00
@noroadsleft noroadsleft merged commit ecb2121 into qmk:master Aug 25, 2020
@noroadsleft
Copy link
Member

Thanks!

For future reference, we recommend against committing to your master branch as you've done here, because pull requests from modified master branches can make it more difficult to keep your QMK fork updated. It is highly recommended for QMK development – regardless of what is being done or where – to keep your master updated, but NEVER commit to it. Instead, do all your changes in a branch (branches are basically free in Git) and issue PRs from your branches when you're developing.

There are instructions on how to keep your fork updated here:

Best Practices: Your Fork's Master: Update Often, Commit Never

Fixing Your Branch will walk you through fixing up your master branch moving forward. If you need any help with this just ask.

nicocesar pushed a commit to nicocesar/qmk_firmware that referenced this pull request Sep 6, 2020
@AKiwi92 @nicocesar
[Keyboard] KiwiKeebs MacroBoard v1.0 (qmk#10080)
f919555 
* KiwiKeebs v.10 QMK

* Rename files and folder structure for QMK pull request

* Deleted unused files

* Added starter keymap

* Changes to make pull request compliant

* Removed lines for PR error

* Error fix in json for PR

* Update keyboards/kiwikeebsmacro/kiwikeebsmacro.h

* Update keyboards/kiwikeebsmacro/readme.md

* Update keyboards/kiwikeebsmacro/keymaps/default/readme.md

* Update keyboards/kiwikeebsmacro/keymaps/default/keymap.c

* Update keyboards/kiwikeebsmacro/info.json

* Update keyboards/kiwikeebsmacro/keymaps/default/keymap.c

* Update keyboards/kiwikeebsmacro/config.h

* Changed structure to allow for future boards

* Update keyboards/kiwikeebs/macro/config.h

* Update keyboards/kiwikeebs/macro/info.json

* Update keyboards/kiwikeebs/macro/keymaps/default/keymap.c

* Update keyboards/kiwikeebs/macro/keymaps/default/keymap.c

* Amended rotarty to use kb instead of user

* Updated structure after pull request commits

* Update keyboards/kiwikeebs/macro/config.h

* Update keyboards/kiwikeebs/macro/info.json
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@drashna drashna drashna left review comments

@noroadsleft noroadsleft noroadsleft approved these changes

@fauxpark fauxpark fauxpark approved these changes

@zvecr zvecr Awaiting requested review from zvecr

Assignees
No one assigned
Labels
keyboard
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@AKiwi92 @noroadsleft @drashna @fauxpark @zvecr @ashkwil