MarlinFirmware / Marlin

Marlin is an optimized firmware for RepRap 3D printers based on the Arduino platform. Many commercial 3D printers come with Marlin installed. Check with your vendor if you need source code for your specific machine.
https://marlinfw.org
GNU General Public License v3.0
16.16k stars 19.21k forks source link

Long "buzzes" block Marlin #3913

Closed maukcc closed 8 years ago

maukcc commented 8 years ago

#define LCD_FEEDBACK_FREQUENCY_DURATION_MS 100 affects the duration it takes from the time you click the jog-dial to a reaction of the menu.

So if I set the time to 2000 it takes 2 sec to go into the menu (and all other clicks) scrolling is not affected.

I thought this was only to set the duration of sounds.

jbrazio commented 8 years ago

Are you using the latest RCBugfix ?

maukcc commented 8 years ago

1.1.0-RC6 when I set

#define SPEAKER 

I can actually hear something When I set the duration to 2000 it plays the beep for 2 sec. and then does the click switch so the menu waits for the buzzer to finish

jbrazio commented 8 years ago

The buzzer is a blocking function which means it will block anything other than the ISR while is "buzzing", so you should keep it short.

maukcc commented 8 years ago

I would not close this, but call it a bug. Or do you want to call it a feature?

jbrazio commented 8 years ago

It's not a big nor a feature.. who want a buzzer buzzing for 2S ?..

maukcc commented 8 years ago

Not me, not you, but maybe someone else wants to piss someone off :) Fact is that it should not work in this way, and therefore is a bug.

Blue-Marlin commented 8 years ago

@maukcc I agree. buzz() is far from perfect. But it's a reasonable compromise, if you don't try crazy things. It might be a bit more complex than you think.

If you have a new, good idea this is welcome. Otherwise, for now, we have to live with what we have. Feel free to change buzzer.cpp to your needs.

jbrazio commented 8 years ago

After looking into it, I have a way to improve it.. We could refactor the code into an object™ and get rid of all the delay() stuff by adding a tick() and some glue-logic behind it.

But (is) it worth the effort ?

thinkyhead commented 8 years ago

@jbrazio Any solution that works in the background is going to add overhead in most loop cycles. The LCD update only happens 100 times a second, so this might be the best place to have something like:

if (buzzer_off_ms && ELAPSED(millis(), buzzer_off_ms)) turn_off_buzzer();

…of course that doesn't work for multiple beeps in a row.

thinkyhead commented 8 years ago

A problem I can see is that when you want to run GCode that plays tones, you need to delay after each GCode to allow the tone to complete. So our other alternative is to make a simple tone queue and have all beeps, buzzes, and tones go into the queue – frequency and duration. I don't see any other alternative, if we want to eliminate the use of delay().

jbrazio commented 8 years ago

solution that works in the background is going to add overhead in most loop cycles

@thinkyhead I have something cooking already, we will build upon it.

So our other alternative is to make a simple tone queue and have all beeps, buzzes, and tones go into the queue – frequency and duration

jbrazio commented 8 years ago

@thinkyhead Do you know any boards who use the Marlin's BUZZER definition i.e. piezo with self-resonating element ? All boards I've seen have a piezo but you "play" them thus they should be defined as SPEAKER within Marlin ?

thinkyhead commented 8 years ago

@jbrazio I don't know which specific models use a single-tone buzzer rather than a multi-tonal piezo SPEAKER. But there must be a few out there. We could set SPEAKER automatically for some LCD controllers, those that are known to have a piezo speaker.

jbrazio commented 8 years ago

OK I will rename the thing.. because "speaker" is far away from a "piezo buzzer", @thinkyhead do you agree with:

thinkyhead commented 8 years ago

I think this should be left as-is. We settled on SPEAKER a while ago, after a lot of finagling. It's a simple single option. And neither "beeper" nor "buzzer" is any improvement, because neither conveys multi-tonality.

jbrazio commented 8 years ago

I always set on my config file the BUZZER and not the SPEAKER because I "knew" I didn't had a speaker on the LCD board but instead had a [piezo] buzzer.

buzzer

speaker

I will not insist more, but those are the common names and associations people do.

thinkyhead commented 8 years ago

I understand your rationale, and thank you for the visual aids. Of course we all know it's a Piezo. The reasoning for the word SPEAKER to my thinking is that a "speaker" is capable of doing more than a "beeper" or a "buzzer." So I suspect we settled on that term due to its functional implications rather than its literal meaning. Not only can these little piezo "speakers" make simple tones, they can synthesize more complex sounds with the right driver, even voice.

jbrazio commented 8 years ago

You're right, there are no questions about it but from a user standpoint it might be difficult to differentiate because [I believe], at least for a non native speaker, this is one of those cases where the "common word" does not coincide with the "technical definition", the visual queues was just to show what people associate with which word in a generic way.

thinkyhead commented 8 years ago

So far we have had no complaints about this particular setting being confusing.

AnHardt commented 8 years ago

As far as i know almost all boards use buzzers (with own resonator) and almost no board uses a speaker (without resonator). A test if your board has a buzzer or not is simple. Do not define SPEAKER, flash, play a tone with M300. If you can hear a tone you have a buzzer. We changed the default some time ago to buzzer. We did not get any complain about not hearing anything. I'd assume more issues if there are boards with speakers out there. At least that means our description is ok.

Most buzzers can be (mis)used as a speaker, but what you can hear is somewhat unpredictable. You can not play tones with a lower frequency than the resonator has. The volume of the tone completely depends on the electronic realization of the resonator. High frequencys mostly will have a much lower volume. (Thats the reason for thinky and me never could agree upon a common default value for frequency and duration) In ether case there is no way to adjust the volume directly.

So the safe way, for us, to get a high audible tone is to not define speaker by default. If someone is annoyed by to much noise he can change the config.

@jbrazio Please do not change the names. Please google for "piezo speaker" and try to decide if you see a piezo buzzer or a piezo speaker. We have no space for a "sound box" on the board - that's obvious.

Let's see if we can eliminate the busy waiting.

define LCD_FEEDBACK_FREQUENCY_HZ 1000.

Means we have to switch on the pin 1000 times per second and to switch off 1000 times a second. That means we need a loop where we are in at least 2000 times a second.

Setting a pin to low for stopping a tone from a buzzer is possible in idle(). Duration is much less critical than frequency.

I'm writing to slow. :-( Or do i think and investigate to much? :-)

Just tried an acoustic idle time fedback - funny!

jbrazio commented 8 years ago

Please do not change the names.

I said I was not insisting on it, will just update to be non-blocking.

thinkyhead commented 8 years ago

Non-blocking tones will be a cool addition. Looking forward to the "tone buffer" … soon?

jbrazio commented 8 years ago

It already plays the Imperial March but requires polishing, expect the PR this weekend.

Roxy-3D commented 8 years ago

It already plays the Imperial March but requires polishing, expect the PR this weekend.

Aren't we the good guys? I don't think we should be playing tunes from the Dark Side!

https://www.youtube.com/watch?v=-bzWSJG93P8

thinkyhead commented 8 years ago
M300 S0 P1
M300 S698 P300
M300 S0 P50
M300 S523 P50
M300 S0 P25
M300 S494 P50
M300 S0 P25
M300 S523 P100
M300 S0 P50
M300 S554 P300
M300 S0 P100
M300 S523 P300
M300 S0 P400
M300 S659 P300
M300 S0 P100
M300 S698 P300
jbrazio commented 8 years ago

Aren't we the good guys?

Welcome to the dark side. :zap: #3962

github-actions[bot] commented 2 years ago

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.