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

[keyboard] bastardkb: restructure folder hierarchy #16778

Merged
merged 34 commits into from
Jul 25, 2022

Conversation

0xcharly
Copy link
Contributor

@0xcharly 0xcharly commented Apr 3, 2022

Upstream changes to Bastard Keyboard boards.

This PR:

  • Restructure folder hierarchy ahead of supporting other adapters/mcus
  • Add support for 2 new keyboards:
    • Charybdis Mini (aka. Cmini, aka. Charybdis 3x6)
    • Dilemma
  • Add support for adapter v2
  • Add support for RP2040-based Splinky MCU

This PR is on behalf of @bstiq.

Description

Bastard Keyboards support for the following (adapter, mcu) pairs:

This PR contains the following changes:

  • Move previous implementation to a sub v1/elitec folder
  • Add support for adapter v2
  • Add support for Charybdis 3x6 format
  • Add support for Dilemma
  • Add support for RP2040-based MCU across all Bastard Keyboards
  • Move keyboard USB IDs and strings to data driven
  • Update headers to keep maintainers list up to date
  • Run qmk format-c
  • Minor no-op cleanups

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

@0xcharly 0xcharly force-pushed the upstream-bkb-master-patch-1 branch from 66d8fb1 to a35881d Compare April 3, 2022 11:21
@0xcharly 0xcharly marked this pull request as ready for review April 3, 2022 11:25
@0xcharly 0xcharly mentioned this pull request Apr 5, 2022
14 tasks
@0xcharly 0xcharly force-pushed the upstream-bkb-master-patch-1 branch from a35881d to 3349589 Compare April 17, 2022 03:45
@0xcharly
Copy link
Contributor Author

A couple updates:

  • Split the PR into 2 commits to make it easier to review: moved the changes from qmk format-c into a separate commit
  • Rebased onto latest qmk:master

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.

Also, rgb matrix modes removed, without being re-added elsewhere?

keyboards/bastardkb/charybdis/3x5/config.h Outdated Show resolved Hide resolved
keyboards/bastardkb/charybdis/4x6/config.h Outdated Show resolved Hide resolved
keyboards/bastardkb/skeletyl/config.h Outdated Show resolved Hide resolved
keyboards/bastardkb/tbkmini/config.h Outdated Show resolved Hide resolved
@tzarc tzarc changed the base branch from master to develop April 18, 2022 11:52
@0xcharly
Copy link
Contributor Author

Also, rgb matrix modes removed, without being re-added elsewhere?

Correct, they were added back in a follow-up planned PR. Fixed and added back in this one.

(apologies for the delayed answer.)

@drashna
Copy link
Member

drashna commented Jun 21, 2022

bump?

@drashna
Copy link
Member

drashna commented Jun 21, 2022

@0xcharly, some concerns here:

Ideally, we don't want a bunch of specific boards listed. Converters should handle that, eventually. I'd argue v1_elite_c v2_elite_c and blackpill would be better names for the revisions here.

A revision for each carrier board or wiring harness rather than the controller used. Part of this is because we do have a specific direction we want to head with the converter stuff. Eg, CONVERT_TO, and PIN_COMPATIBLE, rather than twenty "CONVERT_TO_CONTROLLER_NAME" type settings.

@0xcharly 0xcharly force-pushed the upstream-bkb-master-patch-1 branch from a2d1ed8 to eb62efb Compare July 23, 2022 04:46
@github-actions github-actions bot added keymap via Adds via keymap and/or updates keyboard for via support labels Jul 23, 2022
0xcharly and others added 13 commits July 23, 2022 13:47
…pters/mcus

Upcoming support for the following (adapter, mcu) pairs will be
submitted in follow-up PRs:

- `v2/elitec`
- `v2/stemcell`
- `blackpill`

This PR contains the following changes:

- Move previous implementation to an inner `v1/elitec` folder
- Move keyboard USB IDs and strings to data driven
- Update headers to update maintainers list
- Run `qmk format-c`
For the Charybdis 3x5 (respectively 4x6), the LED config now simulates
36 (respectively 58) LEDs instead of the actual 35 (respectively 56) to
prevent confusion when testing LEDs during assembly when handedness is
not set correctly.  Those fake LEDs are bound to the physical
bottom-left corner.
Merge pull request #5 from Nathancooke7/update_charybdis_readme_v2_shield.
@0xcharly 0xcharly force-pushed the upstream-bkb-master-patch-1 branch from eb62efb to 55bd457 Compare July 23, 2022 04:48
keyboards/bastardkb/charybdis/3x5/blackpill/rules.mk Outdated Show resolved Hide resolved
keyboards/bastardkb/charybdis/3x5/keymaps/bstiq/config.h Outdated Show resolved Hide resolved
keyboards/bastardkb/charybdis/3x5/keymaps/bstiq/vial.json Outdated Show resolved Hide resolved
keyboards/bastardkb/charybdis/3x5/v2/elitec/rules.mk Outdated Show resolved Hide resolved
keyboards/bastardkb/charybdis/3x6/blackpill/rules.mk Outdated Show resolved Hide resolved
keyboards/bastardkb/skeletyl/blackpill/rules.mk Outdated Show resolved Hide resolved
keyboards/bastardkb/tbkmini/blackpill/rules.mk Outdated Show resolved Hide resolved
keyboards/bastardkb/charybdis/3x5/v1/elitec/rules.mk Outdated Show resolved Hide resolved
keyboards/bastardkb/charybdis/4x6/v1/elitec/rules.mk Outdated Show resolved Hide resolved
keyboards/bastardkb/charybdis/3x5/keymaps/bstiq/rules.mk Outdated Show resolved Hide resolved
@0xcharly
Copy link
Contributor Author

A few notes that summaries the various conversations I had with @drashna, @filterpaper, and @KarlK90.

Stability

At this stage, those firmwares are rather well tested and considered stable. They are used daily by multiple members of our community, and are shipped with @bstiq's prebuilds (bastardkb.com).

They have been available through the github.com/bastardkb/bastardkb-qmk fork for close to a year, and have evolved to adopt the latest and greatest from QMK.

Freshness

To avoid the firmwares becoming stale, our fork is constantly updated against the latest changes that are upstreamed on qmk:master and qmk:develop, and against the latest designs and improvements that @drashna and @KarlK90 always kindly notifies us of.

We have been amongst the first few to add support for the STeMCell, and more recently the RP2040. We have integrated the recent improvements to the pointing device driver, and the latest feature for the Circque trackpad.

Converters

We've had several discussions with @drashna and @filterpaper w.r.t folder structure and use of converters instead of maintaining several sub folders for each boards.

  • While the converters route seems promising, it does not seem to be fully adequate yet for BastardKB boards: we support multiple boards (pro micro, blackpill, elite-c, Splinky, STeMCell) for each keyboards, and while some of these share the same physical footprint (Elite-c/Splinky/STeMCell), these boards do not share the same feature set (mostly because we have to cut down a lot of features on AVR), nor do they always share the same configuration (eg. WS2812 driver configuration for the STeMCell). Using a multi folder structure simplifies the maintenance, and the build documentation
  • Current converters seem to target the pro micro footprint, which does not include the bottom 5 pins that are available on boards that share the elite-c footprint, which the Bastard Keyboard line use, so converters as they stand today would only partially factorize the configuration, while still requiring per-board setup
  • While the current structure and configurations are now well tested and I have a high level of confidence that they work as expected, I would not be able to say the same if we were to make changes now to these keyboard configurations (there are 26 different configurations, and multiple keymaps that would need retesting after such changes)

That being said, I am looking forward to the progress the core team is making with converters, and to being able to take advantage of them for the Bastard Keyboard firmwares. However, I believe this will be best addressed in future PR, to avoid further delaying the upstream of the Bastard Keyboard firmware implementations.

@0xcharly
Copy link
Contributor Author

Thanks for the review @drashna, let me cleanup the Vial-related stuff.

@0xcharly 0xcharly requested a review from drashna July 25, 2022 01:47
Copy link
Contributor

@keyboard-magpie keyboard-magpie left a comment

Choose a reason for hiding this comment

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

Very impressive work overall, edit: everything is in place already I requested!

@bstiq
Copy link
Contributor

bstiq commented Jul 25, 2022

Excellent work @0xcharly , thanks !
Looks good to me.

@keyboard-magpie keyboard-magpie merged commit 50a12c0 into qmk:develop Jul 25, 2022
nolanseaton pushed a commit to nolanseaton/qmk_firmware that referenced this pull request Jan 23, 2023
* bastardkb: restructure folder hierarchy ahead of supporting other adapters/mcus

Upcoming support for the following (adapter, mcu) pairs will be
submitted in follow-up PRs:

- `v2/elitec`
- `v2/stemcell`
- `blackpill`

This PR contains the following changes:

- Move previous implementation to an inner `v1/elitec` folder
- Move keyboard USB IDs and strings to data driven
- Update headers to update maintainers list
- Run `qmk format-c`

* bastardkb/charybdis: remove broken acceleration implementation

* bastardkb/charybdis: fix debug output

* bastardkb: add support for BastardKb the `v2/elitec` (adapter, mcu) pair

* bastardkb: add Blackpill support

* bastardkb/charybdis/3x5: add `bstiq` keymap

* bastardkb/charybdis: add fake LEDs to the configuration

For the Charybdis 3x5 (respectively 4x6), the LED config now simulates
36 (respectively 58) LEDs instead of the actual 35 (respectively 56) to
prevent confusion when testing LEDs during assembly when handedness is
not set correctly.  Those fake LEDs are bound to the physical
bottom-left corner.

* bastardkbk/charybdis/readme.md: update build commands

Merge pull request qmk#5 from Nathancooke7/update_charybdis_readme_v2_shield.

* bastardkb/charybdis: fix Via keymap with blackpill

* bastardkb/charybdis: add 3x6 configuration

* bastardkb/charybdis: remove unnecessary files

* bastardkb/charybdis: remove obsolete code

* bastardkb/charybdis/3x6: add Via keymap

* bastardkb: add support for Splinky (RP2040) board

* bastardkb: initial configuration for the Splinky (SPI not working yet)

* bastardkb/charybdis/3x5/v2/splinky: tentative change to enable trackball

* bastardkb/charybdis/3x5/v2/splinky: fix SCK, MISO, MOSI pins

* bastardkb/charybdis/3x5/v2/splinky: fix SCK, MISO, MOSI pins

* bastardkb/charybdis/4x6/v2/splinky: add SPI configuration and enable trackball

* bastardkb/charybdis/3x6: add splinky config

* bastardkb/*/v2/splinky: update drivers to `vendor`

* bastardkb/dilemma: add new board

* bastardkb/charybdis: fix infinite loop in `layer_state_set_user(…)` in the `via` keymaps

* bastardkb/dilemma: add `bstiq` keymap

* bastardkb: specify blackpill boards

* bastardkb/charybdis: fix blackpill-specific define syntax

* bastardkb: remove `NO_ACTION_MACRO` and `NO_ACTION_FUNCTION` which are no longer valid options

* bastardkb: fix `QK_BOOT` keycodes

* bastardkb/dilemma: fix mouse direction on X axis

* bastardkb/charybdis/3x6: adjust CS

* bastardkb/dilemma: adjust trackpad configuration

* charybdis: fix `PWM33XX_CS_PIN` defines

This is a follow-up of qmk#17613.

* bastardkb: remove Vial mentions from `bstiq` keymaps

* Cleanup unnecessary comments

Co-authored-by: Nathan <[email protected]>
Co-authored-by: Charly Delay <[email protected]>
@0xcharly 0xcharly deleted the upstream-bkb-master-patch-1 branch April 7, 2024 03:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

4 participants