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

refactor: Hook into CMake loading in a better spot. #1989

Merged
merged 3 commits into from
Dec 6, 2023
Merged

refactor: Hook into CMake loading in a better spot. #1989

merged 3 commits into from
Dec 6, 2023

Conversation

petejohanson
Copy link
Contributor

@petejohanson petejohanson commented Nov 6, 2023 •
edited
Loading

  • Shift to using an extra Zephyr module to do keymap location
    work after all board roots are resolved. This avoids duplicate work
    and allows us to load custom boards from Zephyr modules as well as
    user config setups.

@petejohanson petejohanson self-assigned this Nov 6, 2023
@petejohanson petejohanson added core Core functionality/behavior of ZMK refactor labels Nov 6, 2023
@petejohanson petejohanson force-pushed the build/zephyr-module-keymap-loading-refactor branch from 0db22df to 70b72ef Compare November 6, 2023 18:10
@petejohanson
Copy link
Contributor Author

Ok, so testing this branch now with the Blank Slate board definition moved to a Zephyr module (https://github.com/petejohanson/blank-slate-module) and building in a config repo that now just has the keymap for overrides, and it's happy to do so: https://github.com/petejohanson/blank-slate-zmk-config/actions/runs/6775125865/job/18413694799

Will continue testing with other iterations, e.g. shields also in a module, but so far I'm optimistic about this.

@petejohanson
Copy link
Contributor Author

Working for shields as well: https://github.com/petejohanson/revxlp-config/actions/runs/6775393542/job/18414589428

@petejohanson petejohanson marked this pull request as ready for review November 6, 2023 19:23
@petejohanson petejohanson requested a review from a team as a code owner November 6, 2023 19:23
@petejohanson
Copy link
Contributor Author

Works in a nice "aggergate repo" that pulls in two modules, and builds boards/shields found in them: https://github.com/petejohanson/aggregating-config-repo/actions/runs/6775597409

@petejohanson petejohanson force-pushed the build/zephyr-module-keymap-loading-refactor branch 2 times, most recently from 54a6b78 to 1ee535d Compare November 15, 2023 20:14
@petejohanson petejohanson force-pushed the build/zephyr-module-keymap-loading-refactor branch 11 times, most recently from fd9ba46 to 203f6a0 Compare November 26, 2023 01:17
@petejohanson
Copy link
Contributor Author

@caksoylar Ok, pushed the tweaks to the new shield guide. I will open a PR against the unified-zmk-config-template tonight that includes the changes necessary to match the doc.

caksoylar
caksoylar reviewed Nov 27, 2023
View reviewed changes
Copy link
Contributor

@caksoylar caksoylar left a comment

Choose a reason for hiding this comment

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

LGTM in general, but I think the build+flash page might require a mention of -DZMK_EXTRA_MODULES=/full/path/to/your/module for building with modules.

docs/docs/development/new-shield.md Outdated Show resolved Hide resolved
docs/docs/development/new-shield.md Outdated Show resolved Hide resolved
docs/docs/development/new-shield.md Outdated Show resolved Hide resolved
docs/docs/development/new-shield.md Outdated Show resolved Hide resolved
docs/docs/development/new-shield.md Outdated Show resolved Hide resolved
docs/docs/development/new-shield.md Show resolved Hide resolved
docs/docs/development/new-shield.md Outdated Show resolved Hide resolved
docs/docs/development/new-shield.md Outdated Show resolved Hide resolved
docs/docs/development/new-shield.md Outdated Show resolved Hide resolved
@petejohanson petejohanson force-pushed the build/zephyr-module-keymap-loading-refactor branch from c762316 to dd468a0 Compare November 27, 2023 17:51
@petejohanson petejohanson mentioned this pull request Nov 27, 2023
Merged
caksoylar
caksoylar approved these changes Nov 27, 2023
View reviewed changes
Copy link
Contributor

@caksoylar caksoylar left a comment

Choose a reason for hiding this comment

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

A few nits I didn't catch yesterday, but otherwise LGTM. Do we want to note module builds in the build+flash page later?

docs/docs/development/new-shield.md Outdated Show resolved Hide resolved
docs/docs/development/new-shield.md Outdated Show resolved Hide resolved
docs/docs/development/new-shield.md Outdated Show resolved Hide resolved
@caksoylar caksoylar mentioned this pull request Dec 4, 2023
Merged
@petejohanson
refactor: Hook into CMake loading in a better spot.
d46b350 
* Shift to using an extra Zephyr module to do keymap location
  work after all board roots are resolved. This avoids duplicate work
  and allows us to load custom boards from Zephyr modules as well as
  user config setups.
@petejohanson
feat(build): Build unified config/mod repos.
58adefc 
* Detect in our build script if our config repo is *also* a Zephyr
  module and if so pass to ZMK build in ZMK_EXTRA_MODULES
  define.
* Copy config directory contents to new independent temp workspace
  to avoid Kconfig conflicts between the build repo's zephyr module
  directory and the checkout zephyr pulled in by `west update`.
@petejohanson petejohanson force-pushed the build/zephyr-module-keymap-loading-refactor branch from c130e63 to 6319906 Compare December 4, 2023 23:45
@petejohanson @caksoylar
refactor(docs): New shield guide to suggest modules
3e789f3 
* Update the new shield guide to position Zephyr module location
  as the default when creating new shields, with a note about
  using ZMK repository itself as a last resort.
* Document building with additional Zephyr modules.

Co-authored-by: Cem Aksoylar <[email protected]>
@petejohanson petejohanson force-pushed the build/zephyr-module-keymap-loading-refactor branch from 6319906 to 3e789f3 Compare December 4, 2023 23:53
@petejohanson
Copy link
Contributor Author

A few nits I didn't catch yesterday, but otherwise LGTM. Do we want to note module builds in the build+flash page later?

Added a section to this doc, just so we don't forget to get back to it later.

@petejohanson petejohanson merged commit 291bbc2 into zmkfirmware:main Dec 6, 2023
21 checks passed
@TristanCacqueray TristanCacqueray mentioned this pull request Dec 6, 2023
Closed
@caksoylar caksoylar mentioned this pull request Dec 11, 2023
Open
@lesshonor lesshonor mentioned this pull request Dec 31, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@caksoylar caksoylar caksoylar approved these changes

Assignees

@petejohanson petejohanson

Labels
core Core functionality/behavior of ZMK refactor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@petejohanson @caksoylar