Need Tips For Memory Efficient Code to Stop Hanging

Hi. Thanks for taking a glance.

I've wired a Nano 33 IOT to several LED lights and programmed the board to turn these lights on and off if the moon is up or down (for my location), along with what lunar phase the moon is currently in. The data for lunar phase, moon rise, and moon set come from a call to the OpenWeather API.

Here is a high-level flow of the program. (Full code below.)

Boot -> Set global variables and necessary libraries
Setup -> Define LEDs, connect to Wifi, connect to MQTT (for logging errors)
Loop -> If defined time period has passed, call OpenWeather, read API response, update global variables, take action* depending on global variables, set new time period

*Action is the LEDs turning on or off and which ones for the correct lunar phase.

The Issue
After several days of running successfully, the program will hang and I haven't been able to determine why. My hunch is that working with web text files fills the memory and causes the hang to occur.

What I've tried:
Adding MQTT log reporting to flush out other potential errors, now handled in the code
Adjusting how variables are set up and used to improve memory usage
Implemented a system reboot via the Nano 33 IOT firmware to reset the board after a given time

Help that would be appreciated:
Any advice on how to better use memory
Tips on ways to raise error codes to better understand the issue
Other words of wisdom

#include "secrets.h"
#include <SPI.h>
#include <ArduinoJson.h>
// Library for the Arduino Nano 33 IOT's NINA-W102 multirado module
#include <WiFiNINA.h>
#include <ArduinoMqttClient.h>

//Build URL for HTTP request using the secrets above 
String apiUrl2 = "&exclude=minutely,hourly,alerts&appid=";
String apiUrl1 = "&lon=";
String apiUrl0 = "GET /data/2.5/onecall?lat=";
String apiUrl = apiUrl0 + SECRET_LATITUDE + apiUrl1 + SECRET_LONGITUDE + apiUrl2 + SECRET_APIKEY;

//Constant variables for networking
const char broker[] = "mqtt.tago.io";
const int port     = 1883;
const char topic[]  = "data";
const char server[] = "api.openweathermap.org";

//Global variables for monitoring state
int status = WL_IDLE_STATUS;  // the Wi-Fi radio's status
unsigned long current_time;
float moon_phase_0;
unsigned long moonrise_0;
unsigned long moonset_0;
unsigned long moonrise_1;
unsigned long moonset_1;
unsigned long moonrise_2;
unsigned long moontiming;
unsigned long moondelay;
unsigned long nextmoonmove;
unsigned long currentmoondelay;
unsigned long startMillis;  
unsigned long previousMillis;  
unsigned long serialPrintInterval = 10000; //in mS
String payload;

// Constants don't change. Used here to set a pin number for the LEDs:
const int led0 = 9;    // the number of the LED pin
const int led1 = 8;    // the number of the LED pin
const int led2 = 7;    // the number of the LED pin
const int led3 = 6;    // the number of the LED pin
const int led4 = 5;    // the number of the LED pin
const int led5 = 4;    // the number of the LED pin
const int led6 = 14;   // the number of the LED pin
const int led7 = 15;   // the number of the LED pin
const int led8 = 16;   // the number of the LED pin
const int led9 = 17;   // the number of the LED pin
const int led10 = 18;  // the number of the LED pin
const int led11 = 19;  // the number of the LED pin

// Build clients for connections to OpenWeather API and MQTT broker
WiFiClient client;
MqttClient mqttClient(client);

void setup() {

  // set the digital pins as output
  // odd numbers are on the top of moon
  pinMode(led0, OUTPUT);
  pinMode(led1, OUTPUT);
  pinMode(led2, OUTPUT);
  pinMode(led3, OUTPUT);
  pinMode(led4, OUTPUT);
  pinMode(led5, OUTPUT);
  pinMode(led6, OUTPUT);
  pinMode(led7, OUTPUT);
  pinMode(led8, OUTPUT);
  pinMode(led9, OUTPUT);
  pinMode(led10, OUTPUT);
  pinMode(led11, OUTPUT);

  // light up moon to show boot-up status
  lightLED(HIGH, LOW, LOW, LOW, LOW, LOW, LOW, LOW, LOW, LOW, LOW, LOW);

  //Initialize serial and wait for port to open.  Needed for native USB port only
  Serial.begin(9600);
  //while (!Serial)
    ;

  // confirm Serial start with moon lights
  lightLED(HIGH, LOW, HIGH, LOW, LOW, LOW, LOW, LOW, LOW, LOW, LOW, LOW);

  // attempt to connect to Wi-Fi network:
  while (status != WL_CONNECTED) {
    Serial.print(F("Attempting to connect to network: "));
    Serial.println(SECRET_SSID);
    // Connect to WPA/WPA2 network:
    status = WiFi.begin(SECRET_SSID, SECRET_PASS);
    lightLED(HIGH, LOW, HIGH, LOW, HIGH, LOW, LOW, LOW, LOW, LOW, LOW, LOW);

    // wait 10 seconds for before retrying connection:
    delay(5000);
    lightLED(HIGH, LOW, HIGH, LOW, HIGH, LOW, HIGH, LOW, LOW, LOW, LOW, LOW);
    delay(5000);
    lightLED(HIGH, LOW, HIGH, LOW, HIGH, LOW, HIGH, LOW, HIGH, LOW, LOW, LOW);
  }
  // you're connected now, so print out a confirmation
  lightLED(HIGH, LOW, HIGH, LOW, HIGH, LOW, HIGH, LOW, HIGH, LOW, HIGH, LOW);
  Serial.println(F("You're connected to the network"));

  // You can provide a unique client ID, if not set the library uses Arduino-millis()
  // Each client must have a unique client ID
  mqttClient.setId("nano33iot");

  // You can provide a username and password for authentication
  mqttClient.setUsernamePassword("******", "*******************");

  // initial start time for state monitor
  startMillis = millis();  //initial start time
}
void loop() {
  
  // call poll() regularly to allow the library to send MQTT keep alives which
  // avoids being disconnected by the broker
  mqttClient.poll();

	// get the current "time" (actually the number of milliseconds since the program started)

	// test whether the period has elapsed
  if (millis() - startMillis >= moondelay)
  {
  	//IMPORTANT to save the start time of the current LED state.
		startMillis = millis();

		// Call OpenWeather via One Call API to capture daily mooooon stats, including rise, set, and phase
		moonAPI();

  }

  if (millis() - previousMillis >= serialPrintInterval)
  {
    previousMillis = millis();

    //Print to serial on next lunar activity
    int currentmoondelay = (moondelay - (millis() - startMillis)) / 1000;
    Serial.print(F("Next moon activity is in: "));
    Serial.println(currentmoondelay);

  }
  
  // if the server's disconnected, stop the client:
  if (!client.connected()) {
    Serial.println();
    Serial.println(F("Disconnecting from server."));
    client.stop();
  }

  // Reset the board once the millis() function reaches a certain height
  if (millis() > (5 * 24 * 60 * 60 * 1000)) { // (5days * 24hours * 60minutes * 60seconds * 1000milliseconds)
    Serial.println(F("Resetting the NANO 33 IOT board."));
    mqttMessage(F("Resetting NANO 33 IOT board due to high millis() count."));
    delay(1000);
    NVIC_SystemReset(); //Built-in reset functionality for NANO 33 IOT
  }
  
}

// Send message to MQTT broker
void mqttMessage(const String& message) {

  //Serial.print("Attempting to connect to the MQTT broker: ");
  //Serial.println(broker);

  if (!mqttClient.connect(broker, port)) {
    Serial.print(F("MQTT connection failed! Error code = "));
    Serial.println(mqttClient.connectError());

    // freeze all motor functions
    while (1);
  }

  //Serial.println("You're connected to the MQTT broker!");

  // Send status message via the Print interface of ArduinoMQTT
  mqttClient.beginMessage(topic);
  mqttClient.print(message);
  mqttClient.endMessage();

	Serial.println(F("Message sent to the MQTT broker!"));

}


// Call OpenWeather to update global variables for moonrise and moonset, next call
void moonAPI() {
  Serial.println(F("Calling the OpenWeather server, beep boop beep.."));
  mqttMessage(F("Calling OpenWeather server"));

  // Close any connection before send a new request
  // This will free the socket on the WifiNINA module
  client.stop();

  //Serial.println("Starting connection to OpenWeather server...");
  if (client.connect(server, 80)) {
    // if you get a connection, report back via serial:
    Serial.println(F("Connected to server"));

    //Serial.println("Sending the HTTP GET Request");
    client.println(apiUrl);
    client.println(" HTTP/1.1");
    client.println("Host: api.openweathermap.org");
    client.println("Connection: close");
    client.println();
  } else {
    //if you couldn't make a connection:
    Serial.println(F("connection failed"));
    mqttMessage(F("Connection failed"));
  }

  if (client.read()) {
    // Using String data type to allow for trimming out
    // Byte Order Marks (BOM) from the client response
    payload = client.readString();
    Serial.println(payload);
    mqttMessage(payload);
  }

  //if you couldn't read the client
  else {
    Serial.println(F("unable to read client"));

    //do nothing forevermore
    while(true);
  }

  if (payload.length() > 3000) {
    // Taking only the JSON data from the payload to avoid hidden character issue
    int start = payload.indexOf('{');
    int end = payload.lastIndexOf('}');
    String body = payload.substring(start, end + 1);
    Serial.println(F("payload trimmed"));

    // Create a JSON object for the OpenWeather response data
    // Used https://arduinojson.org/v6/assistant to calculate doc size, here it was 6144, increased to 8192 to handle hazard messages, thanks wildfire smoke...
    DynamicJsonDocument doc(8192);
    DeserializationError error = deserializeJson(doc, body);
    
    //check for json error
    if (error) {
      Serial.print(F("deserializeJson() failed: "));
      Serial.println(error.f_str());
      mqttMessage(F("deserializeJson failed"));
      client.stop();
      return;
    }

    // Use global variables set outside of function
    JsonObject current = doc["current"];
    current_time = current["dt"];
    JsonArray daily = doc["daily"];
    JsonObject day_0 = daily[0];
    moon_phase_0 = day_0["moon_phase"];
    moonrise_0 = day_0["moonrise"];
    moonset_0 = day_0["moonset"];
    JsonObject day_1 = daily[1];
    moonrise_1 = day_1["moonrise"];
    moonset_1 = day_1["moonset"];
    JsonObject day_2 = daily[2];
    moonrise_2 = day_2["moonrise"];

		// Determine current moon state and upcoming activity using API data
		moonState();
      
  }
  
  // if the string length was too short
  else {
    Serial.print(F("payload string is too short: "));
    Serial.print(payload.length());
    mqttMessage(F("payload string is too short"));
    lightLED(HIGH, HIGH, LOW, LOW, LOW, LOW, LOW, LOW, LOW, LOW, HIGH, HIGH);

    // Set a temp moondelay in milliseconds to retry API call
    moondelay = 30 * 1000;
  } 
  
  Serial.println(F("Moon API call complete"));
  mqttMessage(F("Moon API call complete"));

}

  //Over thought logic to determine the moon's current state and the delay until the next state
void moonState() {

  //Check if two lunar activities occur in a day
  if (moonrise_0 > 0 && moonset_0 > 0) {
    if (moonrise_0 < moonset_0) {
      if (moonrise_0 <= current_time && current_time < moonset_0) {
        //RISE
        moonPhase(moon_phase_0);
        //DELAY -> moonset_0
        nextmoonmove = moonset_0;
      } else {
        //SET
        moonPhase(0);
        if (current_time < moonrise_0) {
          //DELAY -> moonrise_0
          nextmoonmove = moonrise_0;
        } else if (moonset_0 <= current_time) {
          //DELAY -> moonrise_1
          nextmoonmove = moonrise_1;
        }
      }
    } else if (moonset_0 < moonrise_0) {
      if (moonset_0 <= current_time && current_time < moonrise_0) {
        //SET
        moonPhase(0);
        //DELAY -> moonrise_0
        nextmoonmove = moonrise_0;
      } else {
        //RISE
        moonPhase(moon_phase_0);
        if (current_time < moonset_0) {
          //DELAY -> moonset_0
          nextmoonmove = moonset_0;
        } else if (moonrise_0 <= current_time) {
          //DELAY -> moonset_1
          nextmoonmove = moonset_1;
        }
      }
    }
  }
  //Otherwise confirm only one lunar activity occuring
  else if (moonrise_0 < 1 || moonset_0 < 1) {
    if (moonrise_0 > 1) {
      if (current_time >= moonrise_0) {
        //RISE
        moonPhase(moon_phase_0);
        //DELAY -> moonset_1
        nextmoonmove = moonset_1;
      } else {
        //SET
        moonPhase(0);
        //DELAY -> moonrise_0
        nextmoonmove = moonrise_0;
      }
    } else if (moonset_0 > 1) {
      if (current_time >= moonset_0) {
        //SET
        moonPhase(0);
        //DELAY -> moonrise_1
        nextmoonmove = moonrise_1;
      } else {
        //RISE
        moonPhase(moon_phase_0);
        //DELAY -> moonset_0
        nextmoonmove = moonset_0;
      }
    }
  } 

  // Use newly set nextmoonmove to calculate milliseconds until next activity from unix times
  moondelay = (nextmoonmove - current_time) * 1000;

}

// Determine moon phase from API data and map to LED state
void moonPhase(float moon_phase_0) {

  // logic to determine what LEDs to light
  Serial.print(F("Moon phase is: "));
  Serial.println(moon_phase_0);

  // new moon
  if (moon_phase_0 > 0.95 || moon_phase_0 <= 0.05) {
    Serial.println(F("Setting for a new moon phase."));
    mqttMessage(F("Setting for a new moon phase."));
    lightLED(LOW, LOW, LOW, LOW, LOW, LOW, LOW, LOW, LOW, LOW, LOW, LOW);
  }

  // waxing cresecent
  if (moon_phase_0 > 0.05 && moon_phase_0 <= 0.15) {
    Serial.println(F("Setting for a waxing crescent phase."));
    mqttMessage(F("Setting for a waxing crescent phase."));
    lightLED(LOW, LOW, LOW, LOW, LOW, LOW, LOW, LOW, LOW, LOW, HIGH, HIGH);
  }

  // waxing quarter (this phase is made up, to help smooth out the cycle)
  if (moon_phase_0 > 0.15 && moon_phase_0 <= 0.25) {
    Serial.println(F("Setting for a waxing quarter phase."));
    mqttMessage(F("Setting for a waxing quarter phase."));
    lightLED(LOW, LOW, LOW, LOW, LOW, LOW, LOW, LOW, HIGH, HIGH, HIGH, HIGH);
  }

  // first quarter
  if (moon_phase_0 > 0.25 && moon_phase_0 <= 0.35) {
    Serial.println(F("Setting for a first quarter phase."));
    mqttMessage(F("Setting for a first quarter phase."));
    lightLED(LOW, LOW, LOW, LOW, LOW, LOW, HIGH, HIGH, HIGH, HIGH, HIGH, HIGH);
  }

  // waxing gibbous
  if (moon_phase_0 > 0.35 && moon_phase_0 <= 0.45) {
    Serial.println(F("Setting for a waxing gibbous phase."));
    mqttMessage(F("Setting for a waxing gibbous phase."));
    lightLED(LOW, LOW, HIGH, HIGH, HIGH, HIGH, HIGH, HIGH, HIGH, HIGH, HIGH, HIGH);
  }

  // full moon
  if (moon_phase_0 > 0.45 && moon_phase_0 <= 0.55) {
    Serial.println(F("Setting for a full moon phase."));
    mqttMessage(F("Setting for a full moon phase."));
    lightLED(HIGH, HIGH, HIGH, HIGH, HIGH, HIGH, HIGH, HIGH, HIGH, HIGH, HIGH, HIGH);
  }

  // wanning gibbous
  if (moon_phase_0 > 0.55 && moon_phase_0 <= 0.65) {
    Serial.println(F("Setting for a wanning gibbous phase."));
    mqttMessage(F("Setting for a wanning gibbous phase."));
    lightLED(HIGH, HIGH, HIGH, HIGH, HIGH, HIGH, HIGH, HIGH, HIGH, HIGH, LOW, LOW);
  }

  // last quarter
  if (moon_phase_0 > 0.65 && moon_phase_0 <= 0.75) {
    Serial.println(F("Setting for a last quarter phase."));
    mqttMessage(F("Setting for a last quarter phase."));
    lightLED(HIGH, HIGH, HIGH, HIGH, HIGH, HIGH, LOW, LOW, LOW, LOW, LOW, LOW);
  }

  // waning quarter (this phase is made up, to help smooth out the cycle)
  if (moon_phase_0 > 0.75 && moon_phase_0 <= 0.85) {
    Serial.println(F("Setting for a waning quarter phase."));
    mqttMessage(F("Setting for a waning quarter phase."));
    lightLED(HIGH, HIGH, HIGH, HIGH, LOW, LOW, LOW, LOW, LOW, LOW, LOW, LOW);
  }
  // waning cresent
  if (moon_phase_0 > 0.85 && moon_phase_0 <= 0.95) {
    Serial.println(F("Setting for a waning crescent phase."));
    mqttMessage(F("Setting for a waning crescent phase."));
    lightLED(HIGH, HIGH, LOW, LOW, LOW, LOW, LOW, LOW, LOW, LOW, LOW, LOW);
  }
}

// Control the status of LEDs
void lightLED(int L0, int L1, int L2, int L3, int L4, int L5, int L6, int L7, int L8, int L9, int L10, int L11) {

  // input: control for each LED variable
  digitalWrite(led0, L0);
  digitalWrite(led1, L1);
  digitalWrite(led2, L2);
  digitalWrite(led3, L3);
  digitalWrite(led4, L4);
  digitalWrite(led5, L5);
  digitalWrite(led6, L6);
  digitalWrite(led7, L7);
  digitalWrite(led8, L8);
  digitalWrite(led9, L9);
  digitalWrite(led10, L10);
  digitalWrite(led11, L11);
} 

It is recommend to not use String for the reason you are observing - poor memory usage.

When you quit using C++ String objects for C char array zero-terminated strings, you can store the const strings in flash and print them from there, not RAM.

I would also suggest that your Opus would be shorter if you learned about using arrays before writing the code. Learn some more tricks and code will get easier and need less typing.

Strings are for large RAM environments. Most Arduino is small RAM.

C++ insulates programmers from the hardware as much as possible.

C lets engineers work with hardware. Only ASM gets closer to the metal.

Before you begin the work of removing use of String from your code, I have a concern. You are also using ArduinoJson library. I don't know, but I would guess this library may also use String extensively. But removing ArduinoJson library may require a lot more difficult code to be written, so that's not easy.

If ArduinoJson library does use Strings, and it is not the cause of the problems, then that means String can be used safely, and removing it from your own code may not be necessary.

I think more diagnosis is required before you start any major changes, to avoid wasted effort.

Thanks @xfpd, @GoForSmoke, @PaulRB for your recommendations around Strings. I have tried to remove/convert as many Strings as possible from the code. To @PaulRB point, I ran into issues with not using String when interacting with the WiFiNINA.h and ArduinoJSON.h libraries, mostly where I was in over my head. I've seen the examples for these libraries using CHAR but couldn't get them to work for my situation.

If you have recommendations on where I can utilize CHAR instead of String, I'd appreciate them!

Why not:

Serial.println(F(SECRET_SSID))

got 2 of these that could potentially be executed..
expand them a bit add some serial prints every second or so if it gets hit you'll see it..

this whole block is suspicious to me..
why was it needed and does it ever get a chance to fire off??

good luck.. ~q

@qubits-us Post # 1

ok, that answers one part of the two part question..
still would like to know if it ever triggers..
~q

Try storing a String in flash to save RAM.

Get an AMDuino like the Teensy 4.1 that doesn't just have loads of RAM and 32 bit words but an FPU as well!

With AVR's... the 1284P has 16K RAM. It's a bigger box than Uno, 2x2x2.

I have a Quadram 512K card for Mega. It gives 8 56K banks of heap with the internal 8K as dedicated stack space. Some ARM's have more RAM all in one space.

You -can- use String objects in small RAM and not crash. But you need to take care how/what you do and maybe use Strings with fixed buffers that don't copy themself to add a char, use cycles and shorgun the heap.
Arduino Reference on String reserve().

I dunno how the json library likes reserve or what it spawns and deallocates though. At least there are bigger hardware choices!

Thanks @qubits-us, I'll look into cleaning up the unneeded while(true);. I get the MQTT log message about the board resetting but I haven't been able to 100% confirm if the NVIC_SystemReset does fire off. This was being used to clear memory and start the program over.

Appreciate the advice @GoForSmoke. I've already gotten a larger board and plan to use a language with automated garbage collection. Distance from the silicon and Assembly is good for me. :slight_smile: But, as the NANO 33 IOT is already soldered in, I thought I'd give fixing the problem one last shot.