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

Two strategies to address a stuck buzzer #4265

Merged
merged 2 commits into from
Jul 11, 2016
Merged

Two strategies to address a stuck buzzer #4265

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7167405
Two strategies to address a stuck buzzer
thinkyhead Jul 11, 2016
d1dffc7
timestamp => endtime
thinkyhead Jul 11, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions Marlin/buzzer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class Buzzer {
private:
struct state_t {
tone_t tone;
uint32_t timestamp;
uint32_t endtime;
} state;

protected:
Expand Down Expand Up @@ -82,7 +82,7 @@ class Buzzer {
*/
void reset() {
this->off();
this->state.timestamp = 0;
this->state.endtime = 0;
}

public:
Expand All @@ -97,7 +97,7 @@ class Buzzer {
/**
* @brief Add a tone to the queue
* @details Adds a tone_t structure to the ring buffer, will block IO if the
* queue is full waiting for one slot to get available.
* queue is full waiting for one slot to get available.
*
* @param duration Duration of the tone in milliseconds
* @param frequency Frequency of the tone in hertz
Expand All @@ -114,17 +114,17 @@ class Buzzer {
/**
* @brief Loop function
* @details This function should be called at loop, it will take care of
* playing the tones in the queue.
* playing the tones in the queue.
*/
virtual void tick() {
if (!this->state.timestamp) {
if (!this->state.endtime) {
if (this->buffer.isEmpty()) return;

this->state.tone = this->buffer.dequeue();
this->state.timestamp = millis() + this->state.tone.duration;
this->state.endtime = millis() + this->state.tone.duration;
if (this->state.tone.frequency > 0) this->on();
}
else if (millis() >= this->state.timestamp) this->reset();
else if (ELAPSED(millis(), this->state.endtime)) this->reset();
Copy link
Contributor

Choose a reason for hiding this comment

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

Use the ELAPSED macro for robustitude

This is subjective Scott, we don't have to use macros everywhere. In fact we should prefer to use plain standard C code which everyone can read instead of custom macros.

Copy link
Member Author

@thinkyhead thinkyhead Jul 12, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Actually, this matters. I didn't think so either, but I came up against it.
So this macro actually enforces a necessary methodology.

Copy link
Contributor

Choose a reason for hiding this comment

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

Does it have to do with the cast to long ?
millis() and state.timestamp should be both of them long.

Copy link
Member Author

@thinkyhead thinkyhead Jul 13, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure I remember all too well. At that time I made test code to run random time values in a loop. I believe that the cast to long in the PENDING macro is just there to allow a comparison using var < 0. The compiled code probably just tests the high bit.

}
};

Expand Down
4 changes: 3 additions & 1 deletion Marlin/ultralcd.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2334,12 +2334,14 @@ void kill_screen(const char* lcd_msg) {
lcdDrawUpdate = LCDVIEW_CLEAR_CALL_REDRAW;
next_button_update_ms = millis() + 500;

// Buzz and wait. The delay is needed for buttons to settle!
#if ENABLED(LCD_USE_I2C_BUZZER)
lcd.buzz(LCD_FEEDBACK_FREQUENCY_DURATION_MS, LCD_FEEDBACK_FREQUENCY_HZ);
delay(10);
#elif PIN_EXISTS(BEEPER)
buzzer.tone(LCD_FEEDBACK_FREQUENCY_DURATION_MS, LCD_FEEDBACK_FREQUENCY_HZ);
for (int8_t i = 5; i--;) { buzzer.tick(); delay(2); }
#endif
delay(10); // needed for buttons to settle
}

/**
Expand Down