Fixing incorrect keymap build when switching between multiple keymap jsons

[dkjer]

Switching between building one keymap.json in a keyboard to another keymap.json doesn't actually regenerate a new keymap.c, and the old keymap.json/keymap.c is used in the build.

Description

Repro:

Build a keymap based on a keymap.json:

$ make clean
$ make ferris/0_1:default
$ shasum $(find .build -name keymap.c)
9fd4e6fccca4fd2d224db7f583afb1dfd2bb54d8  .build/obj_ferris_0_1/src/keymap.c

Build another keymap.json for the same keyboard:

$ make ferris/0_1:pierrec83
$ shasum $(find .build -name keymap.c)
9fd4e6fccca4fd2d224db7f583afb1dfd2bb54d8  .build/obj_ferris_0_1/src/keymap.c

Notice that the keymap.c has not been changed, and the new build uses the wrong keymap.

Workaround: clean build

$ make clean
$ make ferris/0_1:pierrec83
$ shasum $(find .build -name keymap.c)
95ef52001932451a0c254516dedad235226fe63e  .build/obj_ferris_0_1/src/keymap.c

A clean build will use the correct keymap.c.

This PR aims to differentiate generated keymap.c files so that they are not named the same, and so shouldn't falsely satisfy the keymap.c make target of another keymap.

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

[skullydazed]

Thanks for reporting this bug!

I think we probably want to use $(KEYMAP_OUTPUT) instead of $(KEYBOARD_OUTPUT) here, and then you don't have to add the keymap name to the path.

[dkjer]

@skullydazed I was thinking about doing it that way initially, but it looked 'cleaner' to have the .c file next to the .h files in the src/ directory instead of in a obj_*/ directory.

Let me know if you'd like to me switch to using $(KEYMAP_OUTPUT), and i'll make the change

[skullydazed]

Yes, please make the change. It should have gone there in the first place but I think I had blinders on when I initially wrote this.

[dkjer]

@skullydazed Updated 👍

[skullydazed]

Thanks for updating that. There's one more thing I should have looked at more closely the first time around- why have you moved the logic to the end of build_keyboard.mk? That will prevent setting certain features at the keymap level- CTPC and USER_NAME at a glance, but there are probably others.

[dkjer]

@skullydazed The logic was moved to after the definition of $(KEYMAP_OUTPUT) since it is now used in build_json.mk.
I could move the definition and build json logic up to around line 115 when $(TARGET) becomes resolved, perhaps?

[dkjer]

Rebased off develop.

@skullydazed I moved the logic in build_keyboard.mk back to where it was originally, and instead moved the definitions of TARGET and KEYMAP_OUTPUT to before we include build_json.mk

[stale]

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

[fauxpark]

#pragma weak

[skullydazed]

Apologies for not getting back to this sooner, the tab got lost in the shuffle of travelling and working.