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

fix: always call layer_state_set_user #403

Closed
wants to merge 1 commit into from

Conversation

marienz
Copy link

@marienz marienz commented Dec 7, 2024

Description

Before commit b46f5e2, layer_state_set_kb was only defined if MOONLANDER_USER_LEDS was not set. With MOONLANDER_USER_LEDS set, the default layer_state_set_kb provided by QMK takes care of calling layer_state_set_user.

After that commit, layer_state_set_kb is defined unconditionally, but most of its body is still ifdef'd out if MOONLANDER_USER_LEDS is set. That includes layer_state_set_user, which looks like an oversight.

Run layer_state_set_user unconditionally.

This is the same fix as #396 for voyager. It looks like the other ZSA keyboards do not have the same problem.

Alternatively, we could go back to the old approach of defining the function conditionally, but this makes it a little harder to reintroduce the same problem.

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

Before commit b46f5e2,
layer_state_set_kb was only defined if MOONLANDER_USER_LEDS was not set.
With MOONLANDER_USER_LEDS set, the default layer_state_set_kb provided
by QMK takes care of calling layer_state_set_user.

After that commit, layer_state_set_kb is defined unconditionally, but
most of its body is still ifdef'd out if MOONLANDER_USER_LEDS is set.
That includes layer_state_set_user, which looks like an oversight.

Run layer_state_set_user unconditionally.

This is the same fix as zsa#396 for voyager. It looks like the other ZSA
keyboards do not have the same problem.

Alternatively, we could go back to the old approach of defining the
function conditionally, but this makes it a little harder to reintroduce
the same problem.
@drashna drashna closed this Feb 5, 2025
@marienz
Copy link
Author

marienz commented Feb 7, 2025

If you closed this PR because there is a problem with it, can you please clarify what it is?

(#396 did get merged, and this is the same fix for a different keyboard...)

@Amuxix
Copy link

Amuxix commented Feb 18, 2025

I'm also wondering why this was closed, without this defining MOONLANDER_USER_LEDS doesn't allow customizing the leds, could you take a look at this again @drashna?

PS: Maybe updating the readme to reflect the renaming of ML_LED_*(bool) into STATUS_LED_*(bool) could also be done here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants