earlephilhower / arduino-pico

Raspberry Pi Pico Arduino core, for all RP2040 and RP2350 boards
GNU Lesser General Public License v2.1
2.03k stars 423 forks source link

Apparent problem when using WiFi, repeating timer and LED_BUILTIN inside the repeating timer callback #2281

Closed dehne closed 3 months ago

dehne commented 3 months ago

The code below demonstrates what appears to be a problem in Arduino-Pico. In particular, with unexpected behavior when simultaneously using WiFi, the Raspberry Pi Pico W's "repeating timer" functionality and the built-in LED used inside the repeating timer callback function.

/****
 * @file main.cpp
 * @version 0.1.0
 * @date July, 2024
 * 
 * This is a demonstration of what appears to be a problem in Arduino-Pico. In particular, with 
 * unexpected behavior when simultaneously using WiFi, the Raspberry Pi Pico W's "repeating timer" 
 * functionality and the built-in LED inside the repeating timer callback function. 
 * 
 * I got here because, in a more complicated chunk of code, I ran into what looked like a crash 
 * with the symptom that the repeating timer callback stopped operating as it should and the rest 
 * of the code became super slow. After a spell of looking for loose pointers, out-of-control 
 * indices and the like, I started fresh and built things up to where I saw the same symptoms. 
 * Here's what it does.
 * 
 * The setup() function starts the Serial port so it can report what's going on. It then connects 
 * to WiFi and starts a repeating timer callback. The callback uses digitalWrite() to turn on the 
 * pin LED. It spins for a few cycles and then turns LED off. Meanwhile, using millis(), the 
 * loop() function repeatedly checks how long it's been since it reported on the state of things. 
 * If it's been more than DEAD_MILLIS, it reports that an unexpectedly long lapse has occurred. 
 * Otheriwse, if it's been longer than HEARTBEAT_MILLIS, it reports that things are going as 
 * expected.
 *  
 * I can run with WiFi and a repeating timer callback so long as I don't use the built-in LED 
 * from inside the repeating timer callback. It will run for many hours without reporting a 
 * problem. Similarly, I can run just fine using the built-in LED in the repeating timer callback 
 * so long as WiFi is not connected. But if I connect to WiFi and use the built-in LED from inside 
 * the repeating timer callback, at some point the Pico seems to start spending a lot of time in 
 * code that's not mine. This doesn't happen immediately, sometimes in a few minutes, sometimes 
 * longer, but -- so far -- always less than 30 mins.
 * 
 * I haven't tried using the WiFi and the built-in LED without the repeating timer, though. (I'm 
 * guessing that's been done by lots of people and works okay.) 
 * 
 *****
 * 
 * Copyright (C) 2024 D.L. Ehnebuske
 * 
 * Permission is hereby granted, free of charge, to any person obtaining a copy of this software 
 * and associated documentation files (the "Software"), to deal in the Software without 
 * restriction, including without limitation the rights to use, copy, modify, merge, publish, 
 * distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the 
 * Software is furnished to do so, subject to the following conditions:
 * 
 * The above copyright notice and this permission notice shall be included in all copies or 
 * substantial portions of the Software.
 * 
 * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING 
 * BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND 
 * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, 
 * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, 
 * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. 
 * 
 ****/
#include <Arduino.h>                                    // Basic Arduino framework stuff
#include <WiFi.h>                                       // Pico WiFi support
#include <pico/time.h>                                  // Pico time & timer support

#define BANNER              "Repeating Timer Test v0.1.0"
#define LED                 (LED_BUILTIN)               // Blink the LED attached to this pin (Fails if it's LED_BUILTIN; okay if it's 13)
#define PAUSE_MS            (500)                       // millis() to pause between various initialization retries
#define SERIAL_WAIT_MS      (20000)                     // millis() to wait for Serial to begin before charging ahead
#define WIFI_CONN_MAX_RETRY (3)                         // How many times to retry WiFi.begin() before giving up
#define SSID                "Put SSID here"             // SSID of the WiFi to connect to
#define PW                  "Put pw here"               // The WiFi password to use
#define TIMER_INTERVAL      (256)                       // Timer interval in us
#define HEARTBEAT_MILLIS    (5000)                      // How often (millis()) to print the heartbeat message
#define DEAD_MILLIS         (6000)                      // How long (millis()) before we decide we're dead
#define SPIN_COUNT          (500)                       // Spin-delay count

/****
 * Global variables
 ****/
repeating_timer_t rt;                                   // Where Pico SDK will put the repeating timer info

/**
 * @brief Connect to the WiFi network using SSID and PW #defines
 * 
 * @return true     Success
 * @return false    Failure
 */
bool connectToWifi() {
    int8_t retryCount = 0;
    Serial.printf(String("Attempting to connect to WiFi with ssid '%s'.\n").c_str(), SSID);
    while (WiFi.status() != WL_CONNECTED && retryCount < WIFI_CONN_MAX_RETRY) {
        WiFi.begin(SSID, PW);
        if (WiFi.status() != WL_CONNECTED) {
            if (++retryCount >= WIFI_CONN_MAX_RETRY) {
                return false;   // Give up on connecting to WiFi
            }
        }
    }
    return true;
}

/**
 * @brief   The callback function for the Pico "repeating timer." Called each time the 
 *          timer interval passes. Just turn the LED on, pause briefly and then turn it off.
 * 
 * @param rt        The repeating timer structure associated with this invocation.
 * @return true     Always return this: Keep the repeating timer active
 * @return false    Never return this: Stop the repeating timer.
 */
bool repeatingTimerCallback(repeating_timer_t *rt) {
    digitalWrite(LED, HIGH);
    for (int16_t i = 0; i < SPIN_COUNT; i++) {
        // Make the LED duty cycle high enough to be able to see it
    }
    digitalWrite(LED, LOW);
    return true;
}

void setup() {
    // Init the LED
    pinMode(LED, OUTPUT);
    digitalWrite(LED, HIGH);

    // Get Serial up and running
    Serial.begin(9600);
    long msStart = millis();
    while (!Serial && millis() - msStart < SERIAL_WAIT_MS) {
        delay(PAUSE_MS);
        digitalWrite(LED, digitalRead(LED) == HIGH ? LOW : HIGH);
    }
    Serial.println(BANNER);
    digitalWrite(LED, LOW);

    // Get WiFi up and going
    if (connectToWifi()) {
        Serial.println("Successfully connected to WiFi.");
    } else {
        Serial.println("Unable to connect to WiFi.");
    }

    // Get the repeating timer up and going
    Serial.printf("Adding repeating timer with a period of %d us.\n", TIMER_INTERVAL);
    if (!add_repeating_timer_us(TIMER_INTERVAL, repeatingTimerCallback, NULL, &rt)) {
        Serial.println("Unable to add repeating timer.");
    }
}

void loop() {
    static unsigned long lastMillis = millis();
    if (millis() - lastMillis > DEAD_MILLIS) {
        Serial.println("I'm dead, Jim.");
        lastMillis = millis();
    } else if (millis() - lastMillis > HEARTBEAT_MILLIS) {
        Serial.println("Stayin' alive.");
        lastMillis = millis();
    }
}

I can run with WiFi and a repeating timer callback so long as I don't use the built-in LED from inside the repeating timer callback. It will run for many hours without reporting a problem. Similarly, I can run just fine using the built-in LED in the repeating timer callback so long as WiFi is not connected. But if I connect to WiFi and use the built-in LED from inside the repeating timer callback, at some point the Pico seems to start spending a lot of time in code that's not mine. This doesn't happen immediately, sometimes in a few minutes, sometimes longer, but -- so far -- always less than 30 mins.

I haven't tried using the WiFi and the built-in LED without the repeating timer. (I'm guessing that's been done by lots of people and works okay.)

Am I missing something?

earlephilhower commented 3 months ago

The LED on the PicoW is driven by the WiFi chip and controlled via SPI messages from the Pico to the CYW. My guess is that you are causing re-entrancy in the SPI. Basically there's one SPI going on to the CYW for some operation and in the middle you start another one. So everyone gets confused and I'd be surprised if the CYW or Pico can recover comms after that. You could also be breaking internal state for the WiFi chip driver in doing so.

If you disable the LED flash, or flash an external LED on some other real GPIO pin, I bet this won't happen. Can you give it a go (increment some global counter or something instead to verify your periodic calls are happening)?

(Also, I have a strong suspicion that your busywait for loop will be optimized completely away. You can try adding a memory barrier or incrementing that global value mentioned above to disallow that optimization.)

In net, it's illegal to do a write to LED_BUILTIN on the PicoW from an interrupt. Regular Picos, LED_BUILTIN is a simple GPIO and it is fine to do.

dehne commented 3 months ago

Many thanks for your quick response; I'm sure your diagnosis is correct. This issue can be closed.

In the code I'm actually building (as opposed to the demonstration code), using an external LED is exactly what I do, and, of course, there's no problem.

(And, yes, you're probably right that the compiler has probably optimized the busy-wait loop away.)