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 ARM Audio issues and its EEPROM persistence #4936

Merged
merged 4 commits into from
Feb 15, 2019

Conversation

drashna
Copy link
Member

@drashna drashna commented Jan 25, 2019

Description

While investigating a bug with Audio Click on ARM... I ran into the fact that ARM Audio was never actually updated to persist in EEPROM storage! The code was there, but commented out.

I've updated the audio code to support EEPROM if STM32 EEPROM support is enabled, as well as fixed the clicky issue on ARM.

I've also updated the files that I've touched to conform to the QMK Coding conventions, but I can revert those changes if needed.

I've also employed Drashna CI to ensure that these changes work. And from what I can tell, yes they do.

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. (https://docs.qmk.fm/#/contributing)
  • 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).

@pelrun
Copy link
Contributor

pelrun commented Jan 25, 2019

I've no problems with the formatting changes (maybe because I've been doing the same thing in my PR's...)

You can turn on "ignore whitespace changes" in the diff settings at the top of the page and most of the noise disappears, if it's obscuring the actual code changes. In this instance, it works pretty well.

@drashna
Copy link
Member Author

drashna commented Jan 25, 2019

You can turn on "ignore whitespace changes" in the diff settings at the top of the page and most of the noise disappears, if it's obscuring the actual code changes. In this instance, it works pretty well.

Ooo, neat! I didn't know that, actually! :)

I've no problems with the formatting changes (maybe because I've been doing the same thing in my PR's...)

Thanks. :)

And yeah, I would like to see it all updated, but I don't think it's reasonable to go and just do that for the entire project. Not to mention all the time it would take. Slow, incremental updates is awesome.

@pelrun
Copy link
Contributor

pelrun commented Jan 25, 2019

I almost always have "ignore whitespace changes" turned on in git when I'm making commits. Sometimes it's a little confusing when git tells me a file is modified but the diff is empty, but it's so much easier to see the actual bones of the commit when you don't have incidental indentation changes making a giant mess.

Copy link
Contributor

@mechmerlin mechmerlin left a comment

Choose a reason for hiding this comment

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

Thanks!

@mechmerlin mechmerlin merged commit 85022f8 into qmk:master Feb 15, 2019
@drashna drashna deleted the fix/audio_clicky_issue branch February 18, 2019 17:20
dlhextall pushed a commit to dlhextall/qmk_firmware that referenced this pull request May 24, 2019
* Don't click if turning audio off

On ARM, playing the click when turning off audio causes the audio get stuck and continually play the tone

* Fix Audio EEPROM support for ARM

* Update touched files to conform to QMK Coding Conventions

* Add better check for ARM EEPROM support
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants