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

Updating old Mad Noodle Keypads, and added support for all Mad Noodle keypads, past and future (upcoming keypad launch) #22685

Closed
wants to merge 19 commits into from

Conversation

jessel92
Copy link
Contributor

Updating old Mad Noodle Keypads, and added support for all Mad Noodle keypads, past and future (upcoming keypad launch).

Description

Hey there!

I'm a small keypad manufacturer (one-man operation), The Mad Noodle. I actually only did a QMK pull request for my first gen of keypads, so this is adding everything I've produced so far. Also tried to clean things up to make things a little easier for my customers.

I've updated older keypads and added all of the keypads I've sold/manufactured up to this point. Plus one new keypad that has yet to launch but hopefully will be ready to go by the end of the month.

I believe I followed all the pull request etiquette to the best of my ability. But please let me know if I did anything incorrectly.

Thanks in advance!

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

  • Updated older keypads to new JSON format
  • Add multiple newer keypads

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 Dec 17, 2023
@jessel92
Copy link
Contributor Author

ALSO! Forgot to add this!
I know this is a lot of changes so I'm super sorry.
But please note that files for ncc1701, noodlepad/v1, and v2 are almost identical except for minor pin definitions and the readme.md.

If this is too much for a single pull request, I can split this up. But I've been using this FW for 4ish years and extensively tested it on my end. So ideally, it should be easy on your end hopefully lol. but I barely know what I'm talking about when it comes to git, so I could be totally wrong hahaha.

@sigprof
Copy link
Contributor

sigprof commented Dec 17, 2023

Sorry, but this PR does not comply with several important items of the PR Checklist:

multiple keyboards at the same time is not acceptable

You need to split this to separate PRs for each keyboard, not lumped in like this.

keyboard moves within the repository must go through the develop branch instead of master, so as to ensure compatibility for users

Apparently you are trying to move themadnoodle/noodlepad to themadnoodle/noodlepad/v1; this rename needs to go through the develop branch (and the addition of themadnoodle/noodlepad/micro and themadnoodle/noodlepad/v2 too, as they depend on the rename).

There are also some more problems:

  • Your PR includes changes to some files which should not be touched, like .vscode/settings.json.
  • Some config.h settings, like RGBLIGHT_DEFAULT_SPD, now have info.json equivalents which should be used instead.
  • ENCODER_MAP_ENABLE = yes at the keyboard level rules.mk is not allowed — this option can be set only in the keymap (so a bunch of your default keymaps would need rules.mk with that setting, and most of rules.mk at the keyboard level should become just # This file intentionally left blank, as the other settings in there should be moved to info.json).

@sigprof sigprof added the pr_checklist_pending Needs changes as per the PR checklist label Dec 17, 2023
@jessel92
Copy link
Contributor Author

jessel92 commented Dec 17, 2023

Hey ok i understand.
But im a little confused on some aspects:

  • How do I add encoder_map_enabled at the keymap level? I was just following the docs and there's is nothing about it. Are you saying add a separate rule.mk in the individual default keymaps folders? (similar to the via keymap? Do i still need the RGBLIGHT_ENABLE = yes as well?
  • Same with the RGB settings in JSON. How would I write that?

Any guidance would be greatly appreciated!
Im having troubled since the docs don't really explain the JSON stuff.
And thanks in advance! Sorry for the annoyance!

@jessel92
Copy link
Contributor Author

jessel92 commented Dec 18, 2023

Also should I wait for the v1 rename PR to the Dev branch to go through before I make PRs for the other Noodlepads? Or can I make all the PRs at once?

@jessel92 jessel92 closed this Dec 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keyboard keymap pr_checklist_pending Needs changes as per the PR checklist via Adds via keymap and/or updates keyboard for via support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants