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.

Sign up for GitHub

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

Jump to bottom

Stop sounds when suspended #11553

Merged
merged 3 commits into from
Jan 31, 2021
Merged

Stop sounds when suspended #11553

merged 3 commits into from
Jan 31, 2021

Conversation

spidey3
Copy link
Contributor

@spidey3 spidey3 commented Jan 15, 2021 •
edited
Loading

Description

The code to stop sounds when suspending was commented out long ago, because it interfered with the startup sound working correctly. The problem was that at the time of audio_init() the USB device status might still be in flux, passing through the Suspended state, which would call stop_all_notes().

The solution is to move the playing of the startup song into a routine that is run 300ms after startup, giving enough time for the USB state to settle.

Note: This has been tested thoroughly on AVR. I have provided code for ARM, but it is not as well tested. I understand that the state of audio on ARM is limited to Proton-C; I'd appreciate if someone with that architecture could test this before it is merged.

Types of Changes

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

Fixes issues

Fixes #8167

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

@github-actions github-actions bot added the core label Jan 15, 2021
@tzarc tzarc requested a review from a team January 19, 2021 23:23
@drashna
Copy link
Member

drashna commented Jan 20, 2021

This doesn't work on STM32F303. Specifically with the moonlander, which uses the same chip as the Proton C.

@spidey3
Copy link
Contributor Author

spidey3 commented Jan 20, 2021

This doesn't work on STM32F303. Specifically with the moonlander, which uses the same chip as the Proton C.

Does it just not fix the problem, or does it break other things?

Good news is I have a Proton C on order, expect to have it shortly...

@drashna
Copy link
Member

drashna commented Jan 23, 2021

It stops the startup sound from playing.

@spidey3
Copy link
Contributor Author

spidey3 commented Jan 24, 2021

It stops the startup sound from playing.

Hmmm...
I wonder - if you change the 300 to 500 at quantum.c#L682 does that make it work?

@tzarc
Copy link
Member

tzarc commented Jan 25, 2021 •
edited
Loading

Did some verification:

  • PR as-is: Proton-C produces a constant buzz after startup.
  • PR, with the change from 300ms to 500ms: no difference, same buzz
  • PR, with change to 5000ms: delay for 5 seconds, then constant buzz
  • PR, with change to 15000ms: delay for 15 seconds, then constant buzz
  • PR, with merge of develop branch: no difference
  • Untouched develop branch: normal startup "bleep" sound.

@spidey3
Copy link
Contributor Author

spidey3 commented Jan 25, 2021 •
edited
Loading

@drashna , @tzarc , can you please test again on F303?

I just pushed some code updates that I think solve the problems.

drashna
drashna approved these changes Jan 29, 2021
View reviewed changes
Copy link
Member

@drashna drashna left a comment

Choose a reason for hiding this comment

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

Yup, this works! And fixes the audio init issues on arm, for me.

@drashna drashna requested a review from a team January 29, 2021 01:39
@tzarc tzarc merged commit ae4ee75 into qmk:develop Jan 31, 2021
drashna pushed a commit to zsa/qmk_firmware that referenced this pull request Feb 1, 2021
@spidey3 @drashna
Stop sounds when suspended (qmk#11553)
9ee0271 
* fix stopping audio on suspend vs. startup sound

* trim firmware size

* fix stuck audio on startup (ARM)
@spidey3 spidey3 deleted the audio_suspend branch February 1, 2021 03:08
@spidey3 spidey3 mentioned this pull request Feb 14, 2021
Merged
14 tasks
@drashna drashna mentioned this pull request Aug 6, 2021
Merged
11 tasks
BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
@spidey3
Stop sounds when suspended (qmk#11553)
2825878 
* fix stopping audio on suspend vs. startup sound

* trim firmware size

* fix stuck audio on startup (ARM)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fauxpark fauxpark fauxpark approved these changes

@tzarc tzarc tzarc approved these changes

@drashna drashna drashna approved these changes

Assignees
No one assigned
Labels
core
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@spidey3 @drashna @tzarc @fauxpark