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: Make escape work after changing the backlight settings on the HS60 V2 #5193

Merged
merged 1 commit into from
Feb 20, 2019

Conversation

Hornwall
Copy link
Contributor

@Hornwall Hornwall commented Feb 20, 2019

Description

This pull request fixes a bug where the keymap for the the top left key (usually escape) would be overwritten by the config for the rgb-backlight when it was written to the EEPROM. This would render the key unusable as the first byte of the keymap seems to be overwritten with the last byte of the backlight config. This is fixed by offsetting the keymap and everything there after by moving it by one byte

Steps to reproduce

  1. Flash the current version of HS60 V2 ANSI to the board make hs60/v2:ansi:dfu-util
  2. The escape key should be working as expected
  3. Change backlight effect/hue/brightness
  4. The escape key will no longer be responsive, layer 2+ and the escape key still works fine.
  5. Reset the EEPROM Unplug -> Hold escape -> Plug in -> Unplug -> Release escape -> Plug in
  6. The escape key should now be functional until you change the backlight settings again.

My friend tested the current version on the ISO version of this PCB and the issue does not seem to be present on that version. He also tested this branch and it did not break anything for him.

Types of Changes

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

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

@drashna
Copy link
Member

drashna commented Feb 20, 2019

@yiancar ?

@yiancar
Copy link
Contributor

yiancar commented Feb 20, 2019

Yes this is correct!

@drashna
Copy link
Member

drashna commented Feb 20, 2019

Awesome, just wanted to make sure!

@drashna drashna merged commit 657b44c into qmk:master Feb 20, 2019
@yiancar
Copy link
Contributor

yiancar commented Feb 20, 2019

Yep thank you! Thanks @Hornwall for figuring this out

@Hornwall
Copy link
Contributor Author

Happy to help! Thank you for the quick response!

@drashna
Copy link
Member

drashna commented Feb 20, 2019

there are a few of us that check PRs often. :)

And we're glad to see people help out! It's a huge part of what makes QMK awesome!

@Hornwall Hornwall deleted the hs60-v2-escape-fix branch February 21, 2019 18:10
@andys8
Copy link
Contributor

andys8 commented Feb 25, 2019

@yiancar I had issues today, where the keyboard was not working at all after flashing QMK. Can this change be related?

My solution was to rebase to the initial commit that added hs60v2, enable Bootmagic in the config, delete the eeprom with space + backspace, and it worked afterwards.

Escape was not working at all (this PR is/was not applied). I disabled backlighting and ESC is working.

But regarding this PR and the current state of master: Is it working? And is the flash with dfu-util working with initial firmware, or could the EEPROM change mess things up? If it's necessary to reset the EEPROM, we could add it to the README :)

Update

I tested, if it's only relying on previously stored EEPROM data, resulting in weird behavior. And it looks like that's the case. With the instructions above, one can delete the EEPROM and this branch is working fine. But if somebody doesn't and/or isn't aware of having to do it, this could be an issue.

@yiancar
Copy link
Contributor

yiancar commented Feb 25, 2019

Yes the update goes hand in hand with an EEPROM clear. If you follow the instructions that came with the pcb, it asks to put the pcb in bootloader mode by holding down the ESC key while plugging in the usb. This also resets the EEPROM.

@andys8
Copy link
Contributor

andys8 commented Feb 25, 2019

Thanks for the reply. Are the instructions available online? I don't have any (could have thrown them away by a accident).

@yiancar
Copy link
Contributor

yiancar commented Feb 25, 2019

@andys8
Copy link
Contributor

andys8 commented Feb 25, 2019

Whoa, awesome.

Mechboards could add a link to it on the product page. I'll ask them via email.

Can we add the reset instructions to the readme file? I'll create a small PR.

@yiancar
Copy link
Contributor

yiancar commented Feb 25, 2019

Sure feel free to add the instructions on the readme:)

@andys8
Copy link
Contributor

andys8 commented Feb 25, 2019

#5241

zer09 pushed a commit to zer09/qmk_firmware that referenced this pull request Mar 2, 2019
slugger7 pushed a commit to slugger7/qmk_firmware that referenced this pull request Mar 7, 2019
slugger7 pushed a commit to slugger7/qmk_firmware that referenced this pull request Apr 3, 2019
dlhextall pushed a commit to dlhextall/qmk_firmware that referenced this pull request May 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants