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

[FR] Solve sound printer pause issue, use event triggering instead of polling, host sounds #2845

Closed
rondlh opened this issue Sep 26, 2023 · 3 comments · Fixed by #2897
Closed
Labels
enhancement New feature or request

Comments

@rondlh
Copy link

rondlh commented Sep 26, 2023

Sounds on the TFT are played with the help of a sound queue, which means that a sound is first stored in a queue and then the sounds get played one after the other. This way the sounds are not blocking and other tasks like printing can continue without interruption while the sounds are playing, as so I thought...

Issue 1. Sound can block the printing process
When reviewing the buzzer code (buzzer.c) I came across a quite interesting piece of code. which handles what to do when the system runs out of sound queue space, here it is:

while (buzzer.count == BUZZER_CACHE_SIZE)
{
    loopBuzzer();
}

BUZZER_CACHE_SIZE is 5 by default.

This is a BLOCKING process, once a 6th sound arrives ALL processes are blocked, including printing until a sound buffer becomes available via the interrupt based background process. I'm not sure why this is done like this, perhaps stopping the printer will allow the user to listen to the sounds better. Or perhaps this way startup tunes can be played without needing much memory for the sound queue. Storing a sound only takes 4 bytes, so that should not be a big reason.
It might seem that it's not very likely that the sounds block printing, but it can happen, especially because many system predefined sounds consists of 3 to 5 sounds, and error often occur in pairs.
I see several ways to handle this case:

  1. Increase the sound queue to 8 or 16 sounds.
  2. Call loopProcess (or part of it) instead of loopBuzzer to keep the printing and TFT "alive". (loopBuzzer is part of loopProcess).
  3. Just drop/ignore the sound, it will not be played.

Issue 2. Event triggering instead of polling
There are several sources for sounds, the gcode that is being printed, or the TFT system giving warnings or errors. Like described these sounds are stored in a sound queue. Then the loopProcess, which is running contentiously, will come across loopBuzzer to find that a sound was stored and the sound playback is started. This currently means that the TFT is spending it's time to check the sound buffer about 35 thousand times a second to see if a sound is available. This process is called polling.
A smarter and more sophisticated process is called "event triggering". With event triggering the loopProcess does not have to check for sounds, but the event "a new sound arrived" is the trigger. So if a new sound arrives, the sound will first be stored in the queue, and then the playback of the queue will be started. If any more sounds arrive then they will also be stored in the queue, but they will not be played back immediately. They will have to wait their turn. So when a sounds playback is completed, the process will check if more sounds need to be played, and if so the process will continue. Otherwise the task is done. This all happens in the background without interfering with other TFT tasks, like printing.

Feature: Host as sound source
Most motherboard do not have a speaker or buzzer, but you can add one of course. Either way, I do not want to bother the host with sound generation. The TFT has a speaker and has far less tasks on hand than the host. So how about we let the host forward the sounds to the TFT to handle them. This can be implemented in Marlin with the help of the EXTUI interface to forward the sound, and then the TFT can handle the sound playback.

Please note that I have above mentioned topics already implemented and under test. I would like to discuss ideas and the direction to go in. I'm also interested to know what startup sounds (if any) are used.

@rondlh rondlh added the enhancement New feature or request label Sep 26, 2023
@rondlh
Copy link
Author

rondlh commented Sep 28, 2023

@kisslorand I'm not sure what you are doing again, but some discussion would be welcome. As I mentioned I already have this implemented under test. So for you to steal this and start to force-push this is quite despicable.
This is a place for open discussion and peer-review. This is a simple project, for which I want to try my first PR. You could support me in doing this, or steal my idea... Please make the right choice.

UPDATE: By now it's clear that @kisslorand is spreading misinformation, he is harming the interests of the Bigtreetech users. In PR #2850 and #2851 he is spreading misinformation, claiming that I found issues in #2824. Actually what happened is that we found an issue in #2840 when doing a stress test. @digant73 managed to locate the issue the same day, and it's solved already in #2840, it does NOT impact #2824.
@kisslorand is pushing to reverse #2824 which would be very painful because it involves a language and font update. I have no idea why @kisslorand wants to harm the community like this.

@rondlh
Copy link
Author

rondlh commented Sep 30, 2023

In Configuration.h there are the following lines:

/**
 * Buzzer Settings
 * The duration and frequency for the UI feedback sound.
 * Set these to 0 to disable audio feedback in the LCD menus.
 * Only valid for Touch Mode and if BUZZER_PIN is set or available.
 *
 * NOTE: Test audio output with the G-code:
 *       M300 S<frequency Hz> P<duration MilliSeconds>
 */
#define BUZZER_FREQUENCY_DURATION_MS    20  // in ms. Default: 20
#define BUZZER_FREQUENCY_HZ          10548  // in Hz (20Hz to 60000Hz). Default: 10548 (musical note "E")

The last 2 lines describe the touch screen feedback sound, this is the sound you hear when you touch the screen, if you have "touch sounds" enabled at least.

This last line really surprised me, a frequency of 10548Hz?

  1. This frequency is very high, 2 octaves higher than the highest E on a piano.
  2. Older people will not, or only hardly be able to hear this tone.
  3. To create this high tone, interrupts need to be generated at a frequency of over 21000Hz. Quite an additional load for the MCU

Practically I don't hear a high pitch tones, just a low click sound.
This is true for the magnetic buzzers and the fixed frequency piezo buzzers.
What you hear are probably just lower harmonics of the frequency generated.
The same, but louder, effect can be achieve by using a frequency of 100Hz with a duration of 10ms.
You can simulate this sound with a: M300 S100 P10

Please give this tone a try, and let me know if this is a suitable replacement for the current touch sound.. It saves MCU resources and is better suitable for older users.

Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked and limited conversation to collaborators May 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
1 participant