mirror of
https://github.com/MarekZegare4/MeshCore-Solo.git
synced 2026-07-26 14:58:12 +00:00
fix(eink): drive ringtone note advance from TIMER1 IRQ, not loop() poll
On nRF52 the buzzer advanced notes by polling millis() >= _note_end_ms in loop(). A blocking display refresh (e-ink endFrame) starves loop(), so a note boundary that falls inside a refresh is serviced late — the note plays long, or the next is skipped. The keypress-time 300 ms render delay only masked the first note. Advance notes from a hardware TIMER1 compare interrupt instead, scheduled for each note's exact duration, so timing is independent of render cadence. TIMER0 is the SoftDevice's; TIMER1 is free (tone() uses PWM2, nrfx TIMER1 driver is disabled). loop() becomes a no-op on nRF52; the UITask keypress render-delay workaround is removed. _disarmNoteTimer() clears the latched NVIC pending IRQ (not just the event) on stop()/_nrfBegin(), so a note-advance latched just before a stop can't fire spuriously — which would skip a new melody's first note or blip after an explicit stop. Event read-backs flush the write buffer per the nRF52 event anomaly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -1995,18 +1995,10 @@ void UITask::loop() {
|
|||||||
if (!_locked && curr) {
|
if (!_locked && curr) {
|
||||||
curr->handleInput(c);
|
curr->handleInput(c);
|
||||||
{ uint32_t aoff = autoOffMillis(); if (aoff > 0) _auto_off = millis() + aoff; } // extend auto-off timer
|
{ uint32_t aoff = autoOffMillis(); if (aoff > 0) _auto_off = millis() + aoff; } // extend auto-off timer
|
||||||
#ifdef PIN_BUZZER
|
// Note timing no longer depends on render cadence (TIMER1 IRQ advances
|
||||||
if (buzzer.isPlaying()) {
|
// notes directly — see buzzer.cpp), so a redraw right after a keypress
|
||||||
// Keep the next render at least 300 ms away so the blocking e-ink endFrame()
|
// can't clip a note; no need to hold it back while buzzer.isPlaying().
|
||||||
// doesn't extend the current note. 300 ms covers the slowest note at 120 BPM (1/4).
|
_next_refresh = 100; // trigger refresh immediately
|
||||||
unsigned long deadline = millis() + 300;
|
|
||||||
if (_next_refresh < deadline) _next_refresh = deadline;
|
|
||||||
} else {
|
|
||||||
_next_refresh = 100; // trigger refresh immediately
|
|
||||||
}
|
|
||||||
#else
|
|
||||||
_next_refresh = 100; // trigger refresh
|
|
||||||
#endif
|
|
||||||
} else if (_locked) {
|
} else if (_locked) {
|
||||||
// Locked: eat all keys — wake window is set only when display first turns on
|
// Locked: eat all keys — wake window is set only when display first turns on
|
||||||
_next_refresh = 0;
|
_next_refresh = 0;
|
||||||
|
|||||||
@@ -9,6 +9,20 @@ void genericBuzzer::begin() {
|
|||||||
#endif
|
#endif
|
||||||
pinMode(PIN_BUZZER, OUTPUT);
|
pinMode(PIN_BUZZER, OUTPUT);
|
||||||
digitalWrite(PIN_BUZZER, LOW); // need to pull low by default to avoid extreme power draw
|
digitalWrite(PIN_BUZZER, LOW); // need to pull low by default to avoid extreme power draw
|
||||||
|
#if defined(NRF52_PLATFORM)
|
||||||
|
_isr_instance = this;
|
||||||
|
NRF_TIMER1->TASKS_STOP = 1;
|
||||||
|
NRF_TIMER1->MODE = TIMER_MODE_MODE_Timer << TIMER_MODE_MODE_Pos;
|
||||||
|
NRF_TIMER1->BITMODE = TIMER_BITMODE_BITMODE_32Bit << TIMER_BITMODE_BITMODE_Pos;
|
||||||
|
NRF_TIMER1->PRESCALER = 4; // 16 MHz / 2^4 = 1 MHz -> 1 us/tick
|
||||||
|
NRF_TIMER1->SHORTS = TIMER_SHORTS_COMPARE0_CLEAR_Msk | TIMER_SHORTS_COMPARE0_STOP_Msk;
|
||||||
|
NRF_TIMER1->INTENSET = TIMER_INTENSET_COMPARE0_Msk;
|
||||||
|
// Lowest application priority: this only ever reschedules a tone, it must
|
||||||
|
// never contend with anything radio/BLE-timing-critical.
|
||||||
|
NVIC_SetPriority(TIMER1_IRQn, 7);
|
||||||
|
NVIC_ClearPendingIRQ(TIMER1_IRQn);
|
||||||
|
NVIC_EnableIRQ(TIMER1_IRQn);
|
||||||
|
#endif
|
||||||
startup();
|
startup();
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -150,7 +164,41 @@ void genericBuzzer::_nrfStopPwm() {
|
|||||||
_pwm_on = false;
|
_pwm_on = false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
genericBuzzer* genericBuzzer::_isr_instance = nullptr;
|
||||||
|
|
||||||
|
void genericBuzzer::_armNoteTimer(uint32_t dur_ms) {
|
||||||
|
NRF_TIMER1->TASKS_STOP = 1;
|
||||||
|
NRF_TIMER1->TASKS_CLEAR = 1;
|
||||||
|
NRF_TIMER1->EVENTS_COMPARE[0] = 0;
|
||||||
|
NRF_TIMER1->CC[0] = dur_ms * 1000UL; // 1 us/tick (see PRESCALER in begin())
|
||||||
|
NRF_TIMER1->TASKS_START = 1;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Halt the timer AND drop any interrupt it already latched. Stopping the timer
|
||||||
|
// and clearing EVENTS_COMPARE[0] de-asserts the IRQ source, but an interrupt
|
||||||
|
// the NVIC latched just before we stopped stays pending and would fire one
|
||||||
|
// spurious _nrfAdvance() after we return — skipping the first note of a new
|
||||||
|
// melody, or sounding a blip just after an explicit stop. Order matters: clear
|
||||||
|
// the event (with a read-back to flush the write buffer, per the nRF52 event
|
||||||
|
// anomaly) before clearing the NVIC, else the still-set event re-latches it.
|
||||||
|
void genericBuzzer::_disarmNoteTimer() {
|
||||||
|
NRF_TIMER1->TASKS_STOP = 1;
|
||||||
|
NRF_TIMER1->EVENTS_COMPARE[0] = 0;
|
||||||
|
(void)NRF_TIMER1->EVENTS_COMPARE[0];
|
||||||
|
NVIC_ClearPendingIRQ(TIMER1_IRQn);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Static member (not a free function) so it can reach private state without a
|
||||||
|
// friend declaration — same trick MomentaryButton's isrTrampolineN() uses.
|
||||||
|
// The real ISR (TIMER1_IRQHandler, below) is just a one-line dispatch to this.
|
||||||
|
void genericBuzzer::_timer1ISR() {
|
||||||
|
NRF_TIMER1->EVENTS_COMPARE[0] = 0;
|
||||||
|
(void)NRF_TIMER1->EVENTS_COMPARE[0]; // flush write buffer so the IRQ doesn't immediately re-fire (nRF52 anomaly)
|
||||||
|
if (_isr_instance) _isr_instance->_nrfAdvance();
|
||||||
|
}
|
||||||
|
|
||||||
void genericBuzzer::_nrfBegin(const char* melody) {
|
void genericBuzzer::_nrfBegin(const char* melody) {
|
||||||
|
_disarmNoteTimer(); // drop any in-flight/pending note advance before reconfiguring
|
||||||
_nrfStopPwm();
|
_nrfStopPwm();
|
||||||
if (!melody || !*melody) { _rtttl_done = true; return; }
|
if (!melody || !*melody) { _rtttl_done = true; return; }
|
||||||
const char* notes;
|
const char* notes;
|
||||||
@@ -163,7 +211,7 @@ void genericBuzzer::_nrfBegin(const char* melody) {
|
|||||||
void genericBuzzer::_nrfAdvance() {
|
void genericBuzzer::_nrfAdvance() {
|
||||||
uint16_t freq; uint32_t dur_ms;
|
uint16_t freq; uint32_t dur_ms;
|
||||||
if (_parseNext(_rtttl_pos, _def_dur, _def_oct, _def_bpm, freq, dur_ms)) {
|
if (_parseNext(_rtttl_pos, _def_dur, _def_oct, _def_bpm, freq, dur_ms)) {
|
||||||
_note_end_ms = millis() + dur_ms;
|
_armNoteTimer(dur_ms);
|
||||||
if (freq > 0) _nrfStartPwm(freq); else _nrfStopPwm();
|
if (freq > 0) _nrfStartPwm(freq); else _nrfStopPwm();
|
||||||
} else {
|
} else {
|
||||||
_nrfStopPwm();
|
_nrfStopPwm();
|
||||||
@@ -191,12 +239,20 @@ void genericBuzzer::playForced(const char* melody) {
|
|||||||
bool genericBuzzer::isPlaying() { return !_rtttl_done; }
|
bool genericBuzzer::isPlaying() { return !_rtttl_done; }
|
||||||
|
|
||||||
void genericBuzzer::stop() {
|
void genericBuzzer::stop() {
|
||||||
|
_disarmNoteTimer(); // ensure no latched note-advance fires after we stop
|
||||||
_nrfStopPwm();
|
_nrfStopPwm();
|
||||||
_rtttl_done = true;
|
_rtttl_done = true;
|
||||||
}
|
}
|
||||||
|
|
||||||
void genericBuzzer::loop() {
|
// No-op: TIMER1's compare interrupt (_timer1ISR -> _nrfAdvance) now drives
|
||||||
if (!_rtttl_done && (millis() >= _note_end_ms)) _nrfAdvance();
|
// note advancement directly, so timing no longer depends on how often (or
|
||||||
|
// whether) the caller's loop() gets to run. Kept as a real method, not
|
||||||
|
// removed, since UITask polls buzzer.loop() unconditionally for both
|
||||||
|
// platforms.
|
||||||
|
void genericBuzzer::loop() {}
|
||||||
|
|
||||||
|
extern "C" void TIMER1_IRQHandler(void) {
|
||||||
|
genericBuzzer::_timer1ISR();
|
||||||
}
|
}
|
||||||
|
|
||||||
void genericBuzzer::setVolume(uint8_t level) {
|
void genericBuzzer::setVolume(uint8_t level) {
|
||||||
|
|||||||
@@ -54,7 +54,6 @@ class genericBuzzer
|
|||||||
const char* _rtttl_pos = nullptr;
|
const char* _rtttl_pos = nullptr;
|
||||||
bool _rtttl_done = true;
|
bool _rtttl_done = true;
|
||||||
bool _pwm_on = false;
|
bool _pwm_on = false;
|
||||||
unsigned long _note_end_ms = 0;
|
|
||||||
uint8_t _def_dur = 4;
|
uint8_t _def_dur = 4;
|
||||||
uint8_t _def_oct = 5;
|
uint8_t _def_oct = 5;
|
||||||
uint16_t _def_bpm = 120;
|
uint16_t _def_bpm = 120;
|
||||||
@@ -69,5 +68,21 @@ class genericBuzzer
|
|||||||
uint16_t bpm, uint16_t& freq_hz, uint32_t& dur_ms);
|
uint16_t bpm, uint16_t& freq_hz, uint32_t& dur_ms);
|
||||||
static void _parseHeader(const char* melody, uint8_t& def_dur, uint8_t& def_oct,
|
static void _parseHeader(const char* melody, uint8_t& def_dur, uint8_t& def_oct,
|
||||||
uint16_t& bpm, const char*& notes_start);
|
uint16_t& bpm, const char*& notes_start);
|
||||||
|
|
||||||
|
// TIMER1-driven note advance: a hardware compare interrupt calls
|
||||||
|
// _nrfAdvance() at the exact moment the current note's duration ends,
|
||||||
|
// so playback timing survives a blocking display refresh (e-ink)
|
||||||
|
// instead of waiting for the next loop() poll. Same reasoning as
|
||||||
|
// MomentaryButton's GPIO-IRQ edge capture, just timer- rather than
|
||||||
|
// pin-driven. TIMER0 is reserved by the SoftDevice; TIMER1 is free.
|
||||||
|
void _armNoteTimer(uint32_t dur_ms);
|
||||||
|
void _disarmNoteTimer();
|
||||||
|
static genericBuzzer* _isr_instance;
|
||||||
|
|
||||||
|
public:
|
||||||
|
// Must be public: the extern "C" TIMER1_IRQHandler (see buzzer.cpp) is
|
||||||
|
// a free function — the vector table requires that exact symbol — so
|
||||||
|
// it needs access from outside the class to dispatch into it.
|
||||||
|
static void _timer1ISR();
|
||||||
#endif
|
#endif
|
||||||
};
|
};
|
||||||
|
|||||||
Reference in New Issue
Block a user