Ottimizzazione codice.

Salve a tutti!!!
A scopo puramente ludico (ma non più di tanto) ho messo su un circuito molto semplice su uno dei miei Mega2560. Roba semplice:

  • un display LCD 20x4 I2C
  • una SD
  • una sonda DS18B20
  • una LDR GL5539

Il codice in IDE 1.8.10 per Linux è:

#include <RtcDateTime.h>
#include <ThreeWire.h>
#include <RtcDS1302.h>
#include <SD.h>

#include <DallasTemperature.h>
#include<LiquidCrystal_I2C_Hangul.h>

/*
   Definizione valori pin per ThreeWire (RTC);
*/
#define TWI_RST_PIN 2
#define TWI_DAT_PIN 3
#define TWI_CLK_PIN 4

ThreeWire mywire(TWI_DAT_PIN, TWI_CLK_PIN, TWI_RST_PIN);
RtcDS1302<ThreeWire> realtimeclock(mywire);

/*
   Definizione valori per LCD I2C
*/
#define LCD_ADDR 0x27
#define LCD_COLS 20
#define LCD_ROWS 4
LiquidCrystal_I2C_Hangul lcd(LCD_ADDR, LCD_COLS, LCD_ROWS);

/*
   Definizione valori pin per OneWire sensore DS18B20
*/
#define OWI_PIN 5
OneWire oneWire_in(OWI_PIN);
DallasTemperature sensor_0(&oneWire_in);

/*
 * Funzione/Macro per ottenere la dimensione effettiva di un array.
 */
#define countof(a) (sizeof(a) / sizeof(a[0]))

/*
   Definizione valori da modificare per il resistore LDR GL5539
*/
#define LDR_part 9825
#define LDR_mul 208510000
#define LDR_pow 1.485

/*
   Variabile per la correzione del drifting di RTC DS1302
*/
#define RTCREF 10800012L
unsigned long rtc_millis;

void setup() {
  lcd.init();
  lcd.backlight();
  lcd.clear();

  if (!digitalRead(7)) {
    printSystemHeader();
    printSystemHalted();
    waitForSeconds(20);
    asm volatile("jmp 0");
  } else {
    printSystemHeader();
    lcd.setCursor(1, 2);
    lcd.print(F("Wait to start..."));
    delay(2500);
  }

  realtimeclock.Begin();

  lcd.clear();
  lcd.setCursor(0, 0);
  lcd.print(F("RTC Write... "));
  lcd.setCursor(0, 1);
  lcd.print(F("RTC Run... "));
  lcd.setCursor(0, 2);
  lcd.print(F("RTC Valid... "));
  lcd.setCursor(0, 3);
  lcd.print(F("SD Card... "));

  if (realtimeclock.GetIsWriteProtected()) {
    write_NOK(0);
    realtimeclock.SetIsWriteProtected(false);
    write_OK(0);
  } else {
    write_OK(0);
  }

  if (!realtimeclock.GetIsRunning()) {
    write_NOK(1);
    realtimeclock.SetIsRunning(true);
    write_OK(1);
  } else {
    write_OK(1);
  }

  if (!realtimeclock.IsDateTimeValid()) {
    write_NOK(2);
    while (1) ;
  } else {
    write_OK(2);
  }

  if (!SD.begin(53)) {
    write_NOK(3);
    waitForSeconds(30);
    asm volatile("jmp 0");
  } else {
    write_OK(3);
  }

  pinMode(7, INPUT);
  pinMode(8, INPUT);
  pinMode(9, INPUT);
  pinMode(13, OUTPUT);

  sensor_0.begin();
  rtc_millis = millis();
  delay(1000);
  digitalWrite(13, LOW);
  waitForSeconds(10);
}

void loop() {
  if (digitalRead(7)) {
    RtcDateTime adesso = realtimeclock.GetDateTime();

    lcd.clear();
    printSystemTime(0);

    int lightvalue = analogRead(A0);
    sensor_0.requestTemperatures();

    lcd.setCursor(0, 1);
    lcd.print(F("Light: "));
    lcd.print(lightvalue);
    lcd.print(' ');
    lcd.print(convertToLux(lightvalue));

    lcd.setCursor(0, 2);
    lcd.print(F("Temp: "));

    double mytemp = sensor_0.getTempCByIndex(0);
    lcd.print(mytemp);

    lcd.setCursor(0, 3);

    cli();
    digitalWrite(13, HIGH);
    File dataFile = SD.open(getFilename(), FILE_WRITE);
    if (dataFile) {
      dataFile.println(stringTimestamp(adesso) + F(":") + String(lightvalue, DEC) + F(":") + String(LDR_part, DEC) + F(":") + String(mytemp, DEC));
      dataFile.close();
      delay(500);
    } else {
      printSystemHeader();
      lcd.setCursor(1, 2);
      lcd.print(F("SD Card error..."));
      waitForSeconds(15);
      asm volatile("jmp 0");
    }
    digitalWrite(13, LOW);
    sei();

    lcd.setCursor(0, 3);
    lcd.print(F("DeltaT: "));
    lcd.print((unsigned long)(millis() - rtc_millis), DEC);

    delay(1000);
    waitForSeconds(5);
    if ((millis() - rtc_millis) >= RTCREF) {
      rtc_millis = millis();
      gobacktime(1);
    } else {
      if (millis() < rtc_millis) {
        printSystemHeader();
        lcd.setCursor(3, 2);
        lcd.print(F("Reset drift..."));
        rtc_millis = millis();
        waitForSeconds(5);
      }
    }
  } else {
    printSystemHeader();
    printSystemHalted();

    if (!digitalRead(8)) {
      goforwardtime(1);
      printSystemTime(3);
      delay(1000);
    } else {
      if (!digitalRead(9)) {
        gobacktime(1);
        printSystemTime(3);
        delay(1000);
      } else {
        waitForSeconds(10);
      }
    }
    delay(1000);
  }
}

float convertToLux(int adcValue) {
  if (adcValue == 0) {
    return 0;
  } else {
    float ratio = ((float) 1024 / (float) adcValue) - 1;
    float rld = LDR_part * ratio;

    return (float) LDR_mul / (pow(rld, LDR_pow));
  }
}

String getFilename() {
  RtcDateTime adesso = realtimeclock.GetDateTime();
  char tmp_filename[9];

  snprintf_P(tmp_filename, countof(tmp_filename), PSTR("%04u%02u%02u"), adesso.Year(), adesso.Month(), adesso.Day());

  return String(tmp_filename) + F(".TXT");
}

void gobacktime(int seconds) {
  RtcDateTime toSet = realtimeclock.GetDateTime();
  toSet -= seconds;
  realtimeclock.SetDateTime(toSet);
  delay(seconds * 1000);
}

void goforwardtime(int seconds) {
  RtcDateTime toSet = realtimeclock.GetDateTime();
  toSet += seconds;
  realtimeclock.SetDateTime(toSet);
  delay(seconds * 1000);
}

void printSystemHalted() {
  lcd.setCursor(3, 2);
  lcd.print(F("SYSTEM HALTED"));
}

void printSystemHeader() {
  lcd.clear();
  lcd.setCursor(3, 1);
  lcd.print(F("N3T LOG V1.9"));
}

void printSystemTime(int row) {
  RtcDateTime toGet = realtimeclock.GetDateTime();
  lcd.setCursor(0, row);
  lcd.print(stringDatime(toGet));
}

String stringDatime(const RtcDateTime& dt) {
  char datestring[20];

  snprintf_P(datestring, countof(datestring), PSTR("%02u/%02u/%04u %02u:%02u:%02u"), dt.Day(), dt.Month(), dt.Year(), dt.Hour(), dt.Minute(), dt.Second() );

  return String(datestring);
}

String stringTimestamp(const RtcDateTime& dt) {
  char timestring[15];

  snprintf_P(timestring, countof(timestring), PSTR("%04u%02u%02u%02u%02u%02u"), dt.Year(), dt.Month(), dt.Day(), dt.Hour(), dt.Minute(), dt.Second() );

  return String(timestring);
}

void waitForSeconds(int seconds) {
  RtcDateTime dt = realtimeclock.GetDateTime();
  while (dt.Second() % seconds) {
    dt = realtimeclock.GetDateTime();
    delay(250);
  }
}

void write_NOK(int row) {
  lcd.setCursor(13, row);
  lcd.print(F("NOK"));
}

void write_OK(int row) {
  lcd.setCursor(13, row);
  lcd.print(F("OK "));
}

Ottengo un “firmware” di 28190 bytes ed una occupazione RAM di 1998 byte. Avrei necessità di qualche suggerimento per una ottimizzazione, visto che comunque è un programma piuttosto semplice, che include anche una rozza procedura per correggere il drifting dei moduli DS1302 (il mio prendeva 8 secondi su 24 ore, ma nel modo che ho realizzato sembra tenere abbastanza bene).

Attendo i Vs. suggerimenti, e vi ringrazio anticipatamente.

Riccardo.

Così su due piedi, elimina in toto l'uso della classe String in favore delle stringhe classiche del C (array di char) che tanto alla lunga sono fonte di problemi su MCU tipo Arduino. Seconda cosa è la struttura di alcune sezioni che andrebbe migliorata, esempio non so cosa sia collegato al pin 7 ma resettare con il jump a zero è una pratica non tanto buona, puoi sostituire il tutto con un while finché sul pin non c'è il valore che ti interessa (HIGH) e poi proseguire, setssa cosa per l'SD.begin. La funzione waitforseconds, se non ho visto male i tempi di attesa massimi del tuo programma sono al massimo 5 secondi, per queste tempistiche l'uso di millis è sufficientemente preciso, elimini un po' di codice. Non saranno la svolta del programma ma è già qualcosa :)

Anche goforward e goback sono unificabili, visto che int ha segno. goforward (-1) é uguale a goback (1). Al massimo nella delay() usa la "abs()" per ottenere il valore assoluto del numero con segno

Inoltre puoi unificare la writeNOK e writeOK con una sola funzione write che accetti in ingresso anche una stringa di 4 caratteri, che conterrà "OK " o "NOK" a seconda del necessario

Concordo sull'unione di writeOK e writeNOK ma mantenendo come parametro un byte e non una stringa, in quanto ad ottimizzazione è meglio fare una cosa del tipo:

void funzione(byte riga, bool isOK)
{
   lcd.setCursor(13, riga);
   if(isOK)
   {
     lcd.print(F(" OK"));
    }
    else
    {
        lcd.print(F("NOK"));
    }
}

nn che passare un puntatore ad una striga sia meno efficiente ma è più semplice da fare se si vuuo mantenere le stringhe in flash a mezzo della funzione F

Se vogliamo ottimizzare, scriviamo una sola volta “OK” e poi: :slight_smile:

   if(isOK)
   {
     lcd.print(F(' '));
   }
   else
   {
     lcd.print(F('N'));
   }

Anche eliminare gli oggetti String aiuterebbe

Infatti è la prima cosa che ho suggerito, tra le varie cose l'abolizione delle String sarebbe la vera ottimizzazione

Innanzitutto grazie a tutti per le risposte... Modificando ed unificando le funzioni writeOK e writeNOK in una writeStatus, e con una modifica alla waitForSeconds il codice oggetto è calato di parecchio... Siamo a 27996, 200 bytes in meno. Con la waitForSeconds ho comunque il log ogni 5 secondi, anche se non multipli di 5... poco male... sembra abbastanza precisa.

Il codice ora è stato così modificato:

void waitForSeconds(int seconds) {
  while(millis() % (seconds * 1000)) ;
}

void writeStatus(int row, bool ok) {
  lcd.setCursor(13, row);
  if(!ok) {
    lcd.print(F("N"));
  }
  lcd.print(F("OK "));
}
void correctTime(int seconds) {
  RtcDateTime toSet = realtimeclock.GetDateTime();
  toSet += seconds;
  realtimeclock.SetDateTime(toSet);
  delay(abs(seconds) * 1000);
}

Allora... Il pin 7 è connesso ad un pulsante (normalmente in stato HIGH), la cui pressione blocca il sistema per 10 secondi ogni volta che viene rilevato. I pin 8 e 9 (stesso criterio sempre in stato HIGH), posto il 7 in stato HIGH, servono per la correzione dell'orologio +/- 1 secondo.

Ora... il portare le stringhe in char *... Uso malloc in puro stile C???

P.S.: Noto un maggiore drift del DS1302 che già non brilla per precisione...

Grazie a tutti, Riccardo.

Ora... il portare le stringhe in char *... Uso malloc in puro stile C???

Non avrebbe senso, dietro le quinte la classe String usa malloc &. Quindi non usare String per evitare di allocare dinamicamente la ram facendo quindi tutto statico.

Per di più già usi buffer locali per snprintf.

Comunque se la dimensione allocata dinamicamente avviene una sola volta e non la liberi mai non si verificano alcun problema di frammentazione della ram, al massimo malloc ti dice che non è riuscito ad allocare la porzione di ram richiesta e fai lampeggiare un led bloccando l'applicazione o riavviandola con il cane da guardia.

Per essere più esplicito, il problema si verifica prima o poi allocando (o riallocando) e liberando l'allocazione, questo perché non c'è nessun sistema operativo e non c'è la memoria virtuale che hai sul PC e il paging gestito dal kernel.

Ciao.

Rieccomi... Allora... Utilizzare tutto statico senza allocazioni è possibile, alla fine dr vai a vedere i dati che tratto sono dozzinali. Ma quello che ruba RAM sul serio sono le librerie/funzioni per il display I2C, per la RTC e la SD.

Prossimi passi diciamo di evoluzione applicativa, saranno quelli di gestire la correzione del tempo e l'interruzione deli pin 7, 8 e 9 tramite Interrupt, ma ho visto che mi rimangio un bel margine... e lì non potendo utilizzare né millis né delay... iniziano i grattacapi.

Per una eventuale linea di produzione, se metto mano al codice delle librerie o le riscrivo addirittura, stralciando tutto la poesia che non uso e lasciando le due funzioni in croce che mi servono? Oppure... caso limite... metto mano all'assemblato e buonanotte, ma la seconda non mi sorride come soluzione.

Grazie ancora, R.

Togliere le funzioni che non usi è inutile, perché ciò che non usi non viene compilato.