Add forgotten Caps Lock Indicator for Keychron C3 Pro

[adophoxia]

Description

As title. Quickly checked default_keyboard.c after compiling for both variants to check their LED indexes for Caps Lock and they are the same for each other.

Types of Changes

• Core
• Bugfix
• New feature
• Enhancement/optimization
• Keyboard (addition or update)
• Keymap/layout (addition or update)
• Documentation

Issues Fixed or Closed by This PR

• N/A

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

[zvecr]

The following defines within both keyboards/keychron/c3_pro/ansi/red/config.h and keyboards/keychron/c3_pro/ansi/rgb/config.h are unused?

LED_MAC_OS_PIN
LED_WIN_OS_PIN
LED_OS_PIN_ON_STATE

[adophoxia]

Now that I think about it, there was code related to the defines that I pulled from playground here:

void keyboard_post_init_kb(void) {
    setPinOutputPushPull(LED_MAC_OS_PIN);
    setPinOutputPushPull(LED_WIN_OS_PIN);
    writePin(LED_MAC_OS_PIN, !LED_OS_PIN_ON_STATE);
    writePin(LED_WIN_OS_PIN, !LED_OS_PIN_ON_STATE);

...

    keyboard_post_init_user();
}
...
void housekeeping_task_kb(void) {
    if (default_layer_state == (1U << 0)) {
        writePin(LED_MAC_OS_PIN, LED_OS_PIN_ON_STATE);
        writePin(LED_WIN_OS_PIN, !LED_OS_PIN_ON_STATE);
    }
    if (default_layer_state == (1U << 2)) {
        writePin(LED_MAC_OS_PIN, !LED_OS_PIN_ON_STATE);
        writePin(LED_WIN_OS_PIN, LED_OS_PIN_ON_STATE);
    }

...

    housekeeping_task_user();
}
...
void suspend_power_down_kb(void) {
    writePin(LED_WIN_OS_PIN, !LED_OS_PIN_ON_STATE);
    writePin(LED_MAC_OS_PIN, !LED_OS_PIN_ON_STATE);
    suspend_power_down_user();
}

Admittedly, I didn't add this in because at the time, I had thought this was a part of Keychron's "factory" boilerplate code that they stick onto to do tasks that could already be done natively in QMK. I can add this back in if this is deemed necessary with how the code is done.

[iamdanielv]

Maybe use the enum names to make it clear what the leds are reacting to:

enum layers{
    MAC_BASE,
    MAC_FN,
    WIN_BASE,
    WIN_FN,
};

[adophoxia]

Maybe use the enum names to make it clear what the leds are reacting to:

enum layers{
    MAC_BASE,
    MAC_FN,
    WIN_BASE,
    WIN_FN,
};

Not feasible since those enums for the layer names are defined in the keymap level, where c3_pro.c can't infer from due to it sitting outside of it in keyboard level, hence needing to use numerical values for the layers. IMO, this isn't an issue because, since this board uses 4 layers by default, with 2 being designated as "FN" layers, it's more easier to assume that 0 and 2 are the "default" layer for both OS since layer 0, unless defined otherwise, will always be the default layer.

[iamdanielv]

Maybe use the enum names to make it clear what the leds are reacting to:

enum layers{
    MAC_BASE,
    MAC_FN,
    WIN_BASE,
    WIN_FN,
};

Not feasible since those enums for the layer names are defined in the keymap level, where c3_pro.c can't infer from due to it sitting outside of it in keyboard level, hence needing to use numerical values for the layers. IMO, this isn't an issue because, since this board uses 4 layers by default, with 2 being designated as "FN" layers, it's more easier to assume that 0 and 2 are the "default" layer for both OS since layer 0, unless defined otherwise, will always be the default layer.

Yeah, was just thinking about this. Another approach would be to put the enum in the c3_pro.h since these values are already assuming that you will have 4 layers where 0 and 2 are assigned to Mac and Win layers. Kind of a hard decision to make. 😊

In my case I don't use a Mac Layer so I repurposed that LED and I only have it light up on a different layer, not on layer 0.

Maybe something to put into void housekeeping_task_user(void)?
That gives the most flexibility, but also adds complexity.

So probably easier to just assume users will keep the default layers and keep the hard coded values.

[adophoxia]

Another approach would be to put the enum in the c3_pro.h since these values are already assuming that you will have 4 layers where 0 and 2 are assigned to Mac and Win layers.

That would already be problematic if someone decides to not have 4 layers exactly (they change it to have more or less than 4 in their keymap.c), and forget to account for it, only for the CLI to complain about the number of layers you have not being equal to the enum from c3_pro.h.

Maybe something to put into void housekeeping_task_user(void)?

This is a _user function, which is what you can use in your keymap folder.

[adophoxia]

Closing this PR as I had not realized the Caps Lock Indicator was already defined in keyboard.json for the red and rgb variants, making the indicators functions in c3_pro.credundant. I'll make a new PR sometime later to remove the #define CAPS_LOCK_LED_INDEX and any references to it in c3_pro.cwhile still adding in the code for the following defines:

LED_MAC_OS_PIN 
LED_WIN_OS_PIN 
LED_OS_PIN_ON_STATE