Candybar update

[TerryMathews]

Splits Candybar into two subprojects to allow config.qmk.fm to show both possible layouts.

Description

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

• Splits Candybar project into lefty and righty subprojects so that config.qmk.fm will show both layouts and still allow for Defaultish layouts.

Checklist

• My code follows the code style of this project.
• 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).

[tzarc]

Is this just to support showing default layouts on configurator?
I'd be quite reluctant to duplicate entire keyboard definitions.

[TerryMathews]

That is correct, however there are actually two board versions so this is probably the most correct way to handle the situation.

[drashna]

That is correct, however there are actually two board versions so this is probably the most correct way to handle the situation.

I would disagree with that. It's identical boards, but two different entries in the repo, so it will work with the configurator. That's ... a hack if I ever heard one. (and a lot of my code is VERY hacky 😂 )

I do, however, understand the desire to have this working correctly. However, I feel that this this is the wrong way to go about this, and merging it sets a bad precedent.

Ideally, this is something that should be fixed/improved on the configurator side, rather than here, as this basically artificially bloats the number of keyboards in the repo.

Unfortunately, I don't have a good alternative to this, in the meanwhile.

[TerryMathews]

So where do we go from here? Fixing config to recognize default keymaps for every layout is well over my skill level.

I promise if config gains the ability to do that, that I will roll Candybar back to the way it exists today.

[drashna]

Terry, I'm not exactly sure what is the best corse of action here. Sory for not making it clear, byt my comment was more so it was "on the record", and as an explanation is why I I haven't put my approval on the PR.

From a code standpoint, it looks fine. However, I think this is an issue that needs to be fixed on the configurator backen. And not by you specifically.

@yanfali @Erovia Thoughts?

[noroadsleft]

I actually have a stale branch of Configurator that allows setting a default keymap for each layout macro. @yanfali initially wasn't enthused about the implementation, and the fact of the matter is that I have a bit of a hard time maintaining all the keymaps that are already there (about 700 of them).

We do have an open issue of being able to make Configurator keymaps shareable via URL, in which case we could keep the system more or less as it is, and vendors could host their own Configurator keymap JSON files, and link them via the share URL.

qmk/qmk_configurator#606

[TerryMathews]

My $0.02 is that the long-term solution for this is to re-write that part of configurator so that it properly greps the keymaps from the keymap folder from the QMK Master branch. Forget the defaultish solution, have it properly parse the actual keymap people are uploading.

Again, way over my skill level to implement. In the meantime, some solution would be appreciated even if it's a short-term fix. For some reason, it really bothers people to map their left-hand Candybar using the right-hand view even though it works perfectly.

[skullydazed]

My $0.02 is that the long-term solution for this is to re-write that part of configurator so that it properly greps the keymaps from the keymap folder from the QMK Master branch.

I actually just removed the keymap parsing from the API because it's so problematic and was not useful. Long term the idea is to move default keymaps to JSON and then configurator will be able to fetch them directly. I'll be providing a list of available JSON keymaps in the API to facilitate this. A major step in accomplishing that goal was recently merged and will be deployed shortly.

[TerryMathews]

So it seems the consensus is this PR won't be merged? How do I need to structure the code to meet whatever format you will allow to happen? One subproject, two layouts?

[noroadsleft]

This is probably something we need to solve on the QMK Configurator and API side of the equation. The QMK Firmware codebase is working as intended, so the problem isn't there – the limitation is Configurator being able to support only one keymap per keyboard revision at this time.

edit:

For an example:

• keebio_nyquist_rev1_default.json
• keebio_nyquist_rev2_default.json
• keebio_nyquist_rev3_default.json

Only thing different is the value of keyboard.

[TerryMathews]

Right, but this isn't that. These are separate PCBs, the matrix is different as is the keymap.

Is there no possibility of this getting merged until the future improvement to configurator is ready?

[noroadsleft]

Oh okay, I missed that there are actually two different PCBs, rather than one PCB that supports both layouts.

This is another HS60 situation (#7847) where a board that should have been submitted as more than one revision was not. Moving the build targets like this would potentially break keymap exports for anyone who uses Configurator, and may be a breaking change.

I'll review this later tonight. I need to see what the impact is.

[TerryMathews]

Yeah, there's two variants that physically move the 10key.

[noroadsleft]

K, this would definitely break Configurator exports for the CandyBar, and in a way that can't be coded around at this time.

I'm also not keen on all the code duplication – the boards directories and the chconf.h, halconf.h and mcuconf.h files are identical between both lefty and righty variants (which is to be expected). bootloader_defs.h as well, and I'm not sure that that file is even required in this codebase at all, but I know next to nothing about ARM and ChibiOS, so I'm not sure here.

At the very least, the boards files should be at candybar/boards/ and shared between versions, as well as the (ch|hal|mcu)conf.h files, and bootloader_defs.h if those files are needed too. Alas, I've forgotten how to implement that, so I can't give you any concrete suggestions there. 🙁

[TerryMathews]

Requested changes made including moving STM32 stubs into project base directory - which may have to be revisited if we ever run a future revision of Candybar on a different MCU but it's fine for today.

[tzarc]

__early_init and boardInit? Yep, got #8330 in the pipeline for that, too.
https://github.com/qmk/qmk_firmware/blob/e3b67817779f3dda607834c881d703b26056700f/docs/platformdev_chibios_earlyinit.md

[tzarc]

Ideally, if #8330 gets merged you can do away with the board definitions in its entirety, and use the overridable hardware initialisation functions instead.

#8330 attempts to address the __early_init and boardInit problem
#8327 attempts to address the chibios conf files problem

[TerryMathews]

void __early_init(void) {
extern void enter_bootloader_mode_if_requested(void);
enter_bootloader_mode_if_requested();
stm32_gpio_init();
stm32_clock_init();
}

I believe this was the only change we made to the "base" file.

[tzarc]

There are mods in the boardInit which I'm guessing were duped from CannonKeys' F072 board defs.
Either way, should be covered :)

[zvecr]

As a FYI In this scenario, the boards folder is flat out not required since #8134. You can remove the files and BOARD = ST_STM32F072B_DISCOVERY from rules (I only kept it the same in that PR to minimise change and not have to tag every user for validation).

[tzarc]

Unless the current remap stuff is required... but considering the previous response I'd gather you're probably correct.

[TerryMathews]

Compiles correctly with Boards directory removed.

[noroadsleft]

Could you also update the Lefty readme.md with the same updates the Righty readme.md is getting?

Just this last set of changes and I think we'll be good.

[TerryMathews]

Done.

[zvecr]

Thanks!