esp8266 / Arduino

ESP8266 core for Arduino
GNU Lesser General Public License v2.1
15.98k stars 13.34k forks source link

noTone() does not release timer1 #7364

Open Rob58329 opened 4 years ago

Rob58329 commented 4 years ago

Issue:

After issuing a “tone(D1,1550); delay(500); noTone(D1);” sequence, you can no longer use timer1 (I suspect as something to do with the tone-interrupt still using it)?

I note that the below 2 issues/commits do not appear to have effected/fixed this:

Below is an example sketch:

#define Hz 1200 
#define count_80MHz  ((80000000/16)/Hz)

volatile boolean pip_now=false;
void  ICACHE_RAM_ATTR timer_callback(void) {
  static uint16_t isr_loop_debug=0; isr_loop_debug++;
  if (isr_loop_debug>(4*Hz)) {isr_loop_debug=0; pip_now=true;} // every 4sec
  timer1_write(count_80MHz);
}

void setup() {
  Serial.begin(74880);
  Serial.println("\nStart");

  // comment out the below 'tone(D1,1550)' line, and this sketch will work!
  Serial.println("Using 'tone' (this line blocks timer1 so it can't be used again)");
  tone(D1,1550);
  delay(500); noTone(D1);  // Note that "noTone()" on its own [without a proceeding "tone()"] does NOT block timer1

  // startWaveform(D1, 100000, 100000, 0); delay(500); stopWaveform(D1); // this also blocks timer1 [same as "tone()"]

  Serial.println("Enable timer1 (you should see 'pip' every 4 seconds)");

  // xt_rsil(15); // level 15 will disable ALL interrupts // BUT does not disable timer1-interrupts
  // setTimer1Callback(nullptr); // has no apparent effect (needs: #include "core_esp8266_waveform.h") 
  // os_timer_disarm(nullptr);   // just crashes esp8266

  timer1_disable(); 
  timer1_detachInterrupt(); 
  timer1_attachInterrupt(timer_callback);
  timer1_isr_init();
  timer1_enable(TIM_DIV16, TIM_EDGE, TIM_SINGLE);
  timer1_write(count_80MHz);
  Serial.println("End of Setup");
}

void loop() {
  if (pip_now) {pip_now=false; Serial.println("pip");}
  delay(100);
}
dok-net commented 4 years ago

@devyte @igrr This is a duplicate of #4598, from 2018. It seems the bug in NmiTimSetFunc never did get consideration, then? I've verified this issue, it still exists and is caused by a single call to ETS_FRC_TIMER1_NMI_INTR_ATTACH, in master and the PRs for a new waveform implementation.

Tech-TX commented 4 years ago

That's why I'm doing ESP. restart () between tests as I'm looking at the two PRs - don't want invalid test conditions left over from a previous test. ;-)

devyte commented 4 years ago

@dok-net thanks for the find about duplicate with #4598. I'm not sure I understand the problem. Is it that you can't detach, can't reattach, or can't re-enable after trying to detach? Is there a workaround, e.g.: attach with nullptr?

Is this the problem and workaround? From here, thanks to @AlfonsMittelmeyer:

  // ETS_FRC_TIMER1_NMI_INTR_ATTACH(NULL); // doesn't work, bug in sdk for core version 2.4.0 and 2.4.1
  ETS_FRC_TIMER1_INTR_ATTACH(shared_timer_isr,NULL);
dok-net commented 4 years ago

Once attached as NMI, can only do NMI thereafter. The chatty stuff by AlfonsMittelmyer is OT and TL;DR

dok-net commented 4 years ago

@devyte More precisely:

This: https://github.com/esp8266/Arduino/blob/1bfb29395f71da2caa5d14cbc6bdf8cf9c092d7a/cores/esp8266/core_esp8266_waveform.cpp#L89 is not reverted by https://github.com/esp8266/Arduino/blob/1bfb29395f71da2caa5d14cbc6bdf8cf9c092d7a/cores/esp8266/core_esp8266_waveform.cpp#L95 Because, as we well know, startWaveform works, re-attaching the ISR as NMI handler, but one cannot ETS_FRC_TIMER1_INTR_ATTACH(), any non-NMI handler never gets called.

In cores/esp8266/core_esp8266_timer.cpp there is

void ICACHE_RAM_ATTR timer1_isr_init(){
    ETS_FRC_TIMER1_INTR_ATTACH(timer1_isr_handler, NULL);
}

In tools/sdk/include/ets_sys.h there is

#define ETS_FRC_TIMER1_NMI_INTR_ATTACH(func) \
    NmiTimSetFunc(func)

and

#define ETS_FRC_TIMER1_INTR_ATTACH(func, arg) \
    ets_isr_attach(ETS_FRC_TIMER1_INUM, (int_handler_t)(func), (void *)(arg))
devyte commented 4 years ago

@dok-net thanks, that is the explanation I was looking for. Keeping this open, it requires either a fix from Espressif, or reverse engineering of the nmi attach function to see what gets set.

igor-d-n commented 2 years ago

Still not fixed?) Tested today - use tone(..) and you need to power off chip for releasing timer1