Should OTAs disable timer1 interrupts?

[sven337]

I'm working on a esp8266-based babymonitor. I use timer1 interrupts to sample an external ADC at 12.5kHz.
When attempting to use OTAs, the ESP8266 resets with exception code 0 (theoretically "illegal instruction", but I don't think it's true in this case). Note that the program works perfectly without OTAs.

This is the decoded exception:

0x40201fa8: sample_isr() at ./babymonitor/xmit/xmit.ino line 52 (this line number is bogus)
0x4010188a: pp_post at ?? line ?
0x40101535: ppEnqueueRxq at ?? line ?
0x4010505b: Cache_Read_Enable_New at ?? line ?
0x40106b1c: timer1_isr_handler at /usr/share/arduino/hardware/esp8266com/esp8266/cores/esp8266/core_esp8266_timer.c line 57
0x40106b64: timer1_isr_handler at /usr/share/arduino/hardware/esp8266com/esp8266/cores/esp8266/core_esp8266_timer.c line 57
0x40205482: ArduinoOTAClass::_runUpdate() at /usr/share/arduino/hardware/esp8266com/esp8266/libraries/ArduinoOTA/ArduinoOTA.cpp line 233
0x402056da: ArduinoOTAClass::handle() at /usr/share/arduino/hardware/esp8266com/esp8266/libraries/ArduinoOTA/ArduinoOTA.cpp line 319
0x40102904: wDev_ProcessFiq at ?? line ?
0x40206ae1: esp_schedule at /usr/share/arduino/hardware/esp8266com/esp8266/cores/esp8266/core_esp8266_main.cpp line 43
0x40206ae1: esp_schedule at /usr/share/arduino/hardware/esp8266com/esp8266/cores/esp8266/core_esp8266_main.cpp line 43
0x401059fa: spi_flash_get_id at ?? line ?
0x401059c6: spi_flash_get_id at ?? line ?
0x40205720: EspClass::getFlashChipRealSize() at /usr/share/arduino/hardware/esp8266com/esp8266/cores/esp8266/Esp.cpp line 467
0x402057b7: EspClass::checkFlashConfig(bool) at /usr/share/arduino/hardware/esp8266com/esp8266/cores/esp8266/Esp.cpp line 467
0x40206260: UpdaterClass::begin(unsigned int, int) at /usr/share/arduino/hardware/esp8266com/esp8266/cores/esp8266/Updater.cpp line 343
0x40205485: ArduinoOTAClass::_runUpdate() at /usr/share/arduino/hardware/esp8266com/esp8266/libraries/ArduinoOTA/ArduinoOTA.cpp line 233
0x40106e5c: vPortFree at /usr/share/arduino/hardware/esp8266com/esp8266/cores/esp8266/heap.c line 18
0x402035b5: UdpContext::send(ip_addr*, unsigned short) at /usr/share/arduino/hardware/esp8266com/esp8266/libraries/ESP8266WiFi/src/WiFiUdp.cpp line 283
:  (inlined by) WiFiUDP::endPacket() at /usr/share/arduino/hardware/esp8266com/esp8266/libraries/ESP8266WiFi/src/WiFiUdp.cpp line 191
0x402035d8: WiFiUDP::endPacket() at /usr/share/arduino/hardware/esp8266com/esp8266/libraries/ESP8266WiFi/src/WiFiUdp.cpp line 283
0x402056da: ArduinoOTAClass::handle() at /usr/share/arduino/hardware/esp8266com/esp8266/libraries/ArduinoOTA/ArduinoOTA.cpp line 319
0x40202259: loop at ./babymonitor/xmit/xmit.ino line 120
0x40206ae1: esp_schedule at /usr/share/arduino/hardware/esp8266com/esp8266/cores/esp8266/core_esp8266_main.cpp line 43
0x40206b14: loop_wrapper at /usr/share/arduino/hardware/esp8266com/esp8266/cores/esp8266/core_esp8266_main.cpp line 43
0x40100958: cont_norm at /usr/share/arduino/hardware/esp8266com/esp8266/cores/esp8266/cont.S line 109

This is the isr:

void sample_isr(void)
{
    // Read a sample from ADC
    digitalWrite(scePin, LOW);
    adc_buf[current_adc_buf][adc_buf_pos] = SPI.transfer16(0x00);
    digitalWrite(scePin, HIGH);

    adc_buf_pos++;
    if (adc_buf_pos > sizeof(adc_buf[0])/sizeof(adc_buf[0][0])) {
        adc_buf_pos = 0;
        current_adc_buf = !current_adc_buf;
        send_samples_now = 1;
    }
}

And this is how it is set up:

void spiBegin(void)
{
  pinMode(scePin, OUTPUT);

  SPI.begin();
  SPI.setDataMode(SPI_MODE0);
  SPI.setBitOrder(MSBFIRST);
  SPI.setClockDivider(SPI_CLOCK_DIV8);
  digitalWrite(scePin, HIGH);
}

void setup(void)
{ 
       // [...] trimmed irrelevant calls
    ArduinoOTA.begin();

    spiBegin(); 

    timer1_isr_init();
    timer1_attachInterrupt(sample_isr);
    timer1_enable(TIM_DIV16, TIM_EDGE, TIM_LOOP);
    timer1_write(clockCyclesPerMicrosecond() / 16 * 80); //80us = 12.5kHz sampling freq
}

Am I right in thinking that the OTA updater should disable the timer1 user callback? I have tried to do it and it makes this problem go away, but upon updating the ESP8266 resets after a "Soft WDT reset" message. The stack trace decodes to that:

0x40105b2c: spi_flash_write at ?? line ?
0x40206af8: esp_yield at /usr/share/arduino/hardware/esp8266com/esp8266/cores/esp8266/core_esp8266_main.cpp line 43
0x402058ea: EspClass::flashWrite(unsigned int, unsigned int*, unsigned int) at /usr/share/arduino/hardware/esp8266com/esp8266/cores/esp8266/Esp.cpp line 480
0x402063c2: UpdaterClass::_writeBuffer() at /usr/share/arduino/hardware/esp8266com/esp8266/cores/esp8266/Updater.cpp line 343
0x402055d8: write  at /usr/share/arduino/hardware/esp8266com/esp8266/cores/esp8266/Updater.h line 118
:  (inlined by) ArduinoOTAClass::_runUpdate() at /usr/share/arduino/hardware/esp8266com/esp8266/libraries/ArduinoOTA/ArduinoOTA.cpp line 283
0x40106e5c: vPortFree at /usr/share/arduino/hardware/esp8266com/esp8266/cores/esp8266/heap.c line 18
0x40205712: ArduinoOTAClass::handle() at /usr/share/arduino/hardware/esp8266com/esp8266/libraries/ArduinoOTA/ArduinoOTA.cpp line 320
0x40202295: loop at ./xmit/xmit.ino line 120
0x40206b19: esp_schedule at /usr/share/arduino/hardware/esp8266com/esp8266/cores/esp8266/core_esp8266_main.cpp line 43
0x40206b4c: loop_wrapper at /usr/share/arduino/hardware/esp8266com/esp8266/cores/esp8266/core_esp8266_main.cpp line 43
0x40100958: cont_norm at /usr/share/arduino/hardware/esp8266com/esp8266/cores/esp8266/cont.S line 109

[sven337]

And this comment probably explains what is happening:
#819 (comment)

[igrr]

You need to mark the ISR with ICACHE_RAM_ATTR. Also if running the ADC during an OTA is not required in your application, you can probably use OTA callback to disable the timer.

[sven337]

I have put the ISR in RAM, and am poking the SPI and GPIO registers from RAM too (I have pulled the SPI and digitalWrite implementations directly in the sketch). This way everything in the ISR is supposed to run from RAM.
In addition, I've used the callback to disable timer1 as you suggested (the reason I opened this issue originally was that I thought it might be good to do it by default in the ArduinoOTA class, but I realize all timer1 ISRs aren't incompatible with updating the flash).

Although it's probably not your bug, I'm still getting a WDT reset, with the following stack trace:

0x40105b2c: spi_flash_write at ?? line ? 
0x4020566e: EspClass::flashWrite(unsigned int, unsigned int*, unsigned int) at /usr/share/arduino/hardware/esp8266com/esp8266/cores/esp8266/Esp.cpp line 480
0x402061c2: UpdaterClass::_writeBuffer() at /usr/share/arduino/hardware/esp8266com/esp8266/cores/esp8266/Updater.cpp line 351
0x4020121a: delay at /usr/share/arduino/hardware/esp8266com/esp8266/cores/esp8266/core_esp8266_wiring.c line 53
0x40205340: write  at /usr/share/arduino/hardware/esp8266com/esp8266/libraries/ArduinoOTA/ArduinoOTA.cpp line 55
:  (inlined by) ArduinoOTAClass::_runUpdate() at /usr/share/arduino/hardware/esp8266com/esp8266/libraries/ArduinoOTA/ArduinoOTA.cpp line 287
0x40205a79: Print::write(char const*) at /usr/share/arduino/hardware/esp8266com/esp8266/cores/esp8266/Print.cpp line 164
0x40205496: ArduinoOTAClass::handle() at /usr/share/arduino/hardware/esp8266com/esp8266/libraries/ArduinoOTA/ArduinoOTA.cpp line 55
0x4020232c: loop at ./babymonitor/xmit/xmit.ino line 174
0x40206964: loop_wrapper at /usr/share/arduino/hardware/esp8266com/esp8266/cores/esp8266/core_esp8266_main.cpp line 43
0x40100958: cont_norm at /usr/share/arduino/hardware/esp8266com/esp8266/cores/esp8266/cont.S line 109

After some printf debugging(TM), I traced it to this call:
result = ESP.flashWrite(_currentAddress, (uint32_t*) _buffer, _bufferLen);
... which seems to take forever and trip the WDT.

This is the top of the stack trace, that wasn't decoded automatically:

3fff0c80:  00000000 4000444e 3fff0c90 40205b30 ROM: SPI_write_enable calling Wait_SPI_Idle
3fff0c90:  00000000 4000420c 60000200 3fff278c  ROM: ???
3fff0ca0:  00000100 40004ae8 00000500 002d0000   ROM: SPIWrite

So Wait_SPI_Idle it taking ages and trips the watchdog. Mmh, my ADC is also using SPI, but it's using SPI1 (I'm not sure how you call it), not SPI0 where the flash is, so I'm surprised it would have any impact.
Do you have any idea why it could happen?

I appreciate your help (and your code). Thanks.

[igrr]

I don't know if there is any interaction between SPI0 and SPI1 that Wait_SPI_Idle doesn't account for. Could you please check if disabling your SPI ADC code makes this issue disappear?

[sven337]

Strangely enough, a sketch with nothing but OTA code does not make the issue disappear. This makes me believe there may be a HW issue - either power supply (but flash from UART works fine) or flash issues.

[sven337]

I moved the circuit from a breadboard to the veroboard, with different decoupling caps, and OTAs seem to work now, as I suspected earlier.

[stash-s]

I encountered same problem. It boils down to Arduino SPI calls that are compiled without ICACHE_RAM_ATTR. I copy pasted critical code from Arduino library into my ISR.

Ended up with following:
`

// this is inline, no need for ICACHE_RAM_ATTR
inline void setDataBits(uint16_t bits) {
    const uint32_t mask = ~((SPIMMOSI << SPILMOSI) | (SPIMMISO << SPILMISO));
    bits--;
    SPI1U1 = ((SPI1U1 & mask) | ((bits << SPILMOSI) | (bits << SPILMISO)));
}

uint32_t payload=0;

ICACHE_RAM_ATTR
void sample_isr () {

    // latch low
    digitalWrite (15, LOW);

    while(SPI1CMD & SPIBUSY) {}

    setDataBits (8 * sizeof(payload));
    SPI1W0 = payload;
    SPI1W1 = payload >> 32;

    SPI1CMD |= SPIBUSY;
    while(SPI1CMD & SPIBUSY) {}

    // latchhigh
    digitalWrite (15, HIGH);
}

I am updating 'payload' value in 'loop()' code. Works like a champ with no resets and reliable OTA.