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 the AT101 PCB firmware #3785

Merged
merged 13 commits into from
Aug 29, 2018
Merged

Adding the AT101 PCB firmware #3785

merged 13 commits into from
Aug 29, 2018

Conversation

blindassassin111
Copy link
Contributor

No description provided.

Copy link
Member

@drashna drashna left a comment

Choose a reason for hiding this comment

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

I've flagged a number of changes for the files.

Also, if you could, rename the folder so it's using all lower case. Since we have users with a mix of OS's, we want to avoid case sensitivity issues.


#include "quantum.h"

#define KEYMAP( \
Copy link
Member

Choose a reason for hiding this comment

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

Two things here:

  • Please change KEYMAP to LAYOUT here
  • this macro is "wrong". You should reformat it so that it looks like the actual keyboard, rather than what the grid looks like.

Specially, the macro here is so that you can convert the physical (electric) matrix into something "human friendly" (eg, the visual layout).

A good example of this is the clueboard 66's matrix:
https://github.com/qmk/qmk_firmware/blob/master/keyboards/clueboard/66/rev3/rev3.h#L33-L50

And to clarify, the positions don't have to match in location, just in where they end up in the matrix.

So, for instance, the top like would:

#define LAYOUT( \
  K000, K0100, K001, K0101, K0002, K0102, K0003, K0103, K0004, K0104, K0005, K0105, K0006, K0106, K0007, K0107, \

That would be KC_ESC, to KC_PAUS on the first row.

Reformating this may be tedious, but would be a good idea.

If you need help figuring this out, it may be worth dropping this into your process_record_user function, to tell which position is which:

  xprintf("KL: row: %u, column: %u, pressed: %u\n", record->event.key.row, record->event.key.col, record->event.pressed);

And make sure that CONSOLE_ENABLE is set to "yes", and that you hook up HID Listen (you can use the QMK Toolbox on Windows or macOS to do this).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for this! I had no idea that was possible and really helps clean up the layout. Will fix this right away.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, would the section in brackets for the layout need to also be formatted in the same manner or is the way it is now correct?

Copy link
Member

Choose a reason for hiding this comment

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

The second half, the part in the {curly brackets}, is fine. Only the top part needs changing.

Copy link
Member

Choose a reason for hiding this comment

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

Welcome!

The top part is what "users" will use, so that's what needs to reformatted.

The bottom half is what the keyboard uses, and shouldn't be reformatted.

@@ -0,0 +1,46 @@
#ifndef CONFIG_H
Copy link
Member

Choose a reason for hiding this comment

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

Could you change the top two lines to just #pragma once, and remove the endif at the end.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@@ -0,0 +1,65 @@
#include "AT101_Blackheart.h"
Copy link
Member

Choose a reason for hiding this comment

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

Please change this to #include QMK_KEYBOARD_H

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.


const uint16_t PROGMEM keymaps[][MATRIX_ROWS][MATRIX_COLS] = {

KEYMAP(
Copy link
Member

Choose a reason for hiding this comment

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

Please change the KEYMAPs to LAYOUT.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

return true;
}

void led_set_kb(uint8_t usb_led) {
Copy link
Member

Choose a reason for hiding this comment

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

This should be led_set_user, since it's in "user" code, and not the keyboard code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I copied this bit of code from my omnikey_blackheart firmware, and got the code from Terry Mathews originally(the guy who wrote the firmware for the TKC1800).

Copy link
Member

Choose a reason for hiding this comment

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

Ah, well, that probably should be changed too then. :)

But for now, if you could make sure that this is led_set_user, that would be appreciated.



# Boot Section Size in *bytes*
OPT_DEFS += -DBOOTLOADER_SIZE=4096
Copy link
Member

Choose a reason for hiding this comment

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

Do you know what controller this is? On PCB?

If so, do you know what bootloader it is? DFU/teensy/caterina?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Keyboard uses a Teensy2.0. Did I set it up wrong?

Copy link
Member

Choose a reason for hiding this comment

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

It's preferable to name the bootloader that the keyboard's MCU uses.

@drashna can and should correct me if needed, but you should change Lines 40-41 to the following:

# Bootloader selection
BOOTLOADER = halfkay

# Boot Section Size in *bytes*
# OPT_DEFS += -DBOOTLOADER_SIZE=4096

And yes, the OPT_DEFS line should be commented out or omitted. It's only needed if the specific bootloader used isn't known.

# Build Options
# comment out to disable the options.
#
BOOTMAGIC_ENABLE ?= yes # Virtual DIP switch configuration(+1000)
Copy link
Member

Choose a reason for hiding this comment

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

The question marks are not needed here, at all. If you wouldn't mind removing them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry...Started off with files from kbfirmware to save me some time and being new I have no idea what is actually needed for some things. I apologize for this.

@noroadsleft
Copy link
Member

@drashna didn't mention it that I saw, but all the files and folders should be fully lowercase — at101_blackheart rather than AT101_Blackheart.

@blindassassin111
Copy link
Contributor Author

I tried to make everything lowercase, but haven't figured out how to rename the folder. I have changed it locally, and committing it did not change the case, I was able to fix the files via git, but not seeing a way to rename the folder.

@noroadsleft
Copy link
Member

Try this:

git checkout master
cd keyboards
git mv AT101_Blackheart at101_blackheart2
git mv at101_blackheart2 at101_blackheart

Assuming you're on Windows, you have to do two moves because Windows is stupid about character case and treats AT101_Blackheart and at101_blackheart as the same string.

@drashna
Copy link
Member

drashna commented Aug 29, 2018

@noroadsleft I did. Check the top comment. :)

@blindassassin111 yeah, it's a pain. Try naming it to something else completely, commit, and then rename it to the correct name in lowercase, and commit again.

@drashna
Copy link
Member

drashna commented Aug 29, 2018

Awesome! thanks!

@drashna drashna merged commit 190fcdd into qmk:master Aug 29, 2018
@blindassassin111
Copy link
Contributor Author

blindassassin111 commented Aug 29, 2018

Okay, I believe everything is fixed now. Sorry for so much trouble, the last time I did firmware it went way easier, so I wasn't expecting this many issues.

@drashna
Copy link
Member

drashna commented Aug 29, 2018

Yup, it passes the Travis checks, and looks good to human inspection. So it's good to go (and has been merged already)

And that's possible. My guess is that you had a more "reasonable"matrix last time.

@noroadsleft
Copy link
Member

noroadsleft commented Aug 29, 2018

@blindassassin111 not really wrong or anything, but I find debugging switch matrices in the firmware easier if the rows and columns use a different "character set", like using letters to denote the rows and numbers for the columns:

#define LAYOUT( \
    KA0, KA1, KA2, KA3, \
    KB0, KB1, KB2, KB3, \
    KC0, KC1, KC2,      \
    KD0, KD1, KD2, KD3, \
    KE0,      KE2       \
) { \
    { KA0, KA1,   KA2, KA3   }, \
    { KB0, KB1,   KB2, KB3   }, \
    { KC0, KC1,   KC2, KC_NO }, \
    { KD0, KD1,   KD2, KD3   }, \
    { KE0, KC_NO, KE2, KC_NO }  \
}

As long as you can keep them straight, it doesn't matter. I just thought this might make things easier on you in future.

@blindassassin111
Copy link
Contributor Author

Not a bad suggestion, but the naming scheme now lets me reference my schematic and easily check to make sure the switches are in the correct row and column.

@noroadsleft
Copy link
Member

If the firmware matches what your schematic says, that's a very good reason to continue using your current method.

Shinichi-Ohki added a commit to Shinichi-Ohki/qmk_firmware that referenced this pull request Aug 31, 2018
* 'master' of https://github.com/qmk/qmk_firmware: (73 commits)
  Keymap: Updated keymap with the "pretty" layout (qmk#3812)
  Keyboard: CTRL and ALT updates (qmk#3810)
  Docs: Tabulate Modifier & Mod-Tap keycode listings in advanced keycodes docs (qmk#3799)
  Keymap: Wanleg updates (qmk#3794)
  Keymap: Added new tada68 keymap (qmk#3788)
  Tweak the wording in "Becoming a QMK Collaborator"
  Docs: add process_terminal() and update links to other functions (qmk#3778)
  Keymap: Update keymap including LAYOUT_planck_grid (qmk#3779)
  Keyboard: remove old comment from keyboards/helix/rules.mk (qmk#3795)
  Keyboard: Update CTRL and ALT keyboard readme (qmk#3796)
  Mask off keycode/layer/mod where possible in LT(), MT(), etc. (qmk#3430)
  Autodetect lack of screen presence
  Massdrop keyboards readme update (qmk#3791)
  move massdrop boards into its own directory for configurator visibility
  STM32 EEPROM Emulation (qmk#3741)
  Massdrop keyboard support (qmk#3780)
  Keymap: Add german layout for redox keyboard (qmk#3695)
  Keyboard: Adding the AT101 PCB (qmk#3785)
  Updated templates for use by new_project.sh (qmk#3783)
  Fixed bootloader target Fixed matching grep matches (for PRODUCT in particular) Fixed " Bootloader" concatenation for WSL (windows line-endings)
  ...
alexey-danilov pushed a commit to alexey-danilov/qmk_firmware that referenced this pull request Sep 6, 2018
* Adding the AT101 PCB firmware

* Fixed AT101 keymap error

* Fixing AT101 firmware

* More Fixes for AT101 firmware

* Rename AT101_Blackheart.c to at101_blackheart.c

* Rename AT101_Blackheart.h to at101_blackheart.h

* Update readme.md

* Renaming AT101 folder pt1

* Renaming AT101 folder pt2

* Fixing AT101 LED function name

* Redoing AT101 folder naming pt2

Last round had issues, should be the last rename.

* Fixing missing comma in at101_blackheart.h
ChrissiQ pushed a commit to ChrissiQ/qmk_firmware that referenced this pull request Sep 25, 2018
* Adding the AT101 PCB firmware

* Fixed AT101 keymap error

* Fixing AT101 firmware

* More Fixes for AT101 firmware

* Rename AT101_Blackheart.c to at101_blackheart.c

* Rename AT101_Blackheart.h to at101_blackheart.h

* Update readme.md

* Renaming AT101 folder pt1

* Renaming AT101 folder pt2

* Fixing AT101 LED function name

* Redoing AT101 folder naming pt2

Last round had issues, should be the last rename.

* Fixing missing comma in at101_blackheart.h
yamad pushed a commit to yamad/qmk_firmware that referenced this pull request Apr 10, 2019
* Adding the AT101 PCB firmware

* Fixed AT101 keymap error

* Fixing AT101 firmware

* More Fixes for AT101 firmware

* Rename AT101_Blackheart.c to at101_blackheart.c

* Rename AT101_Blackheart.h to at101_blackheart.h

* Update readme.md

* Renaming AT101 folder pt1

* Renaming AT101 folder pt2

* Fixing AT101 LED function name

* Redoing AT101 folder naming pt2

Last round had issues, should be the last rename.

* Fixing missing comma in at101_blackheart.h
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants