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

cantor: fix LT(layer, kc) do not work after waking up PC #16406 #20205

Closed
wants to merge 1 commit into from

Conversation

nolith
Copy link

@nolith nolith commented Mar 21, 2023

Reset the cantor after a wakeup event to unblock LT(layer, kc) keys.

Description

Refactor of the original @philong workaround #16406 (comment).

Instead of a user function in the keymap this commit applies the workarund at the keyboard level to make it available to every cantor user.

As the author said in the issue,

It basically just resets the keyboard after waking up from suspend. Although it works for me, it does not fix the real cause of the issue.

I don't have the knowledge to make a proper fix for the BlackPill board, but this small workaround is working for me on cantor remix (36 keys version) and I think is worth make it more available to the users.

This could be reverted as soon as someone have a proper fix for the underlaying 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).

Refactor of the orignal Philong's workaround. Instead of a user function
in the keymap this commit applies the workarund at the keyboard level
to make it available to every cantor user.

Co-authored-by: Philong <philong@users.noreply.github.com>
// Workaround for https://github.com/qmk/qmk_firmware/issues/16406
__attribute__ ((weak))
void suspend_wakeup_init_kb(void) {
NVIC_SystemReset();
Copy link
Member

Choose a reason for hiding this comment

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

This isn't a fix. That's a hard reset...

You'd be better off just adding QK_BOOT to your keymap.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you @drashna for the review. Could you expand a bit more on how adding a QK_BOOT is better than this?

As I wrote in the description this is not a fix but a workaround.

With a 36 keys layout there is no space for a QK_BOOT on the first layer, and the bug itself, that blocks the dual function keys will make it impossible to have activate it from another layer.

I already shared in the PR description that my knowledge of the QMK internals is very poor, so I think I may be missing something here. Could you please explain me why an hard reset is so problematic with a simple design like the cantor that has no fancy features (no RGB, no diodes, no speakers, no rotary encoders).

Thank you

Copy link
Member

Choose a reason for hiding this comment

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

Force rebooting when it wakes is no t the right way to handle this, at all. This may actually cause issues for other users that are not experiencing the issue.

#19780 is aimed at fixing the issue correctly, in core.

In the meanwhile, NO_SUSPEND_POWER_DOWN = yes in your rules.mk would likely solve this issue, as would #define USB_SUSPEND_WAKEUP_DELAY 200 in your config.h.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for explaining, I'll close this PR

@nolith nolith closed this Mar 22, 2023
@nolith nolith deleted the fix-cantor-freeze branch March 22, 2023 10:43
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.

2 participants