Ottimizziamo il codice del core di Arduino

BALORDI!!! -w è di default, ora nascondono tutti i warning!! MALOOO!!!! apro subito un bug

:grin:

@PaoloP
Si risolve facilmente editando nel file platform.txt la riga
Ottimo, la 1.55 allora permette di modificare i flags, prova ad aggiungere -save-temps, ti tornerà utile in molte occasioni.

Ciao.

C'è comunque un problema ... anche se metti verbose, non mette la -Wall ... =( =( =(

Tocca metterla a mano ...

Guglielmo

P.S. : Ovviamente, mettendola, saltano fuori TUTTI i warning che Mauro ha risolto :wink:

Si Mauro.
E' altamente customizzabile e ci sono flag per tutti i gusti e tutte le situazioni

compiler.c.cmd=avr-gcc
compiler.c.flags=-c -g -Os -w -ffunction-sections -fdata-sections -MMD
compiler.c.elf.flags=-Os -Wl,--gc-sections
compiler.c.elf.cmd=avr-gcc
compiler.S.flags=-c -g -assembler-with-cpp
compiler.cpp.cmd=avr-g++
compiler.cpp.flags=-c -g -Os -w -fno-exceptions -ffunction-sections -fdata-sections -MMD
compiler.ar.cmd=avr-ar
compiler.ar.flags=rcs
compiler.objcopy.cmd=avr-objcopy
compiler.objcopy.eep.flags=-O ihex -j .eeprom --set-section-flags=.eeprom=alloc,load --no-change-warnings --change-section-lma .eeprom=0
compiler.elf2hex.flags=-O ihex -R .eeprom
compiler.elf2hex.cmd=avr-objcopy
compiler.ldflags=
compiler.size.cmd=avr-size
# this can be overriden in boards.txt
build.extra_flags=

Guglielmo hai fatto la modifica anche per il compilatore cpp?

grazie paolo, provvedo subito.
In tatnochi ha account dia mano forte a far apparire il bug: Hidden warning during compilation · Issue #1728 · arduino/Arduino · GitHub

quanto tempo che perdete con la roba GNU!

con roba ARDUINO vorrai dire...

PaoloP:
...
Guglielmo hai fatto la modifica anche per il compilatore cpp?

Si, ho tolto la -w ad entrambi e ... ora ho aggiunto la -Wall al entrambi :wink:

Guglielmo

-Wall abilita tutte le warning.
Le -Dxx sono macro utente visibili in ogni unit compile, come appunto la versione dell'ide -DARDUINO=155

Ciao.

clonato il repo: https://github.com/lestofante/Arduino/blob/ide-1.5.x

già rimossa l'infame -w, se mi fate un riassunto delle modifiche le carico, graaaazie :slight_smile:

partiamo dalla toolchain originale, arduino 1.5.4 (la 1.0.x lasciamola perdere per favore)

con -Wall ecco i warning di un codice base:

void setup() {
  // put your setup code here, to run once:

}

void loop() {
  // put your main code here, to run repeatedly: 
  
}
/home/mauro/arduino-1.5.4/hardware/arduino/avr/cores/arduino/HardwareSerial.cpp: In function 'void __vector_18()':
/home/mauro/arduino-1.5.4/hardware/arduino/avr/cores/arduino/HardwareSerial.cpp:89: warning: unused variable 'c'
/home/mauro/arduino-1.5.4/hardware/arduino/avr/cores/arduino/HardwareSerial.cpp: In member function 'void HardwareSerial::begin(long unsigned int, byte)':
/home/mauro/arduino-1.5.4/hardware/arduino/avr/cores/arduino/HardwareSerial.cpp:329: warning: unused variable 'current_config'
/home/mauro/arduino-1.5.4/hardware/arduino/avr/cores/arduino/Tone.cpp:119: warning: only initialized variables can be placed into program memory area
/home/mauro/arduino-1.5.4/hardware/arduino/avr/cores/arduino/Print.cpp: In member function 'size_t Print::print(const __FlashStringHelper*)':
/home/mauro/arduino-1.5.4/hardware/arduino/avr/cores/arduino/Print.cpp:44: warning: '__progmem__' attribute ignored

Non c'è l'ho operativa la 1.5.4, anzi se mi sai dire quella che ho scaricato dal repo chiamata 1.5.x che versione è?
In allegato trovi tutti i file, devi metterli tu nel posto giusto.

Comunque le modifiche sono poche, quindi ti basta fare un diff e in 10 minuti risolvi anche con la 1.5.4.

Aggiornamento zip:
Incluso HardwareSerial.cpp

Note: Il codice proviene da arduino-1.0.5 quindi, non copiare direttamente nel ramo 1.5x ma usa un programma diff e applica le modifiche.

Perché 1.0.5? Io operativa ho solo la 1.0.5, non sono in grado di fare il build della 1.5x, se avete info precise postate pure.

ciao.

arduinochanged.zip (10.4 KB)

Le versioni attualmente in download (http://arduino.cc/en/Main/Software) sono la 1.0.5 e la 1.5.5.

Questa --> GitHub - arduino/Arduino at ide-1.5.x
è la 1.5.5 con 1 o 2 modifiche effettuate dopo la pubblicazione.
In pratica è la versione 1.5.6 in sviluppo.

per la prima parte di warning: rimosse le 3 variabili.

UDR0 e UDR mi hannolasciato un po perplesso, essndo registrinon vorrei che la loro lettura scatenasse qualcosa. Credo di no, anzi c'è il riscio che l'istruzione sia rimossa dal compilatore, vedendo che è inutile, come un ciclo for finito ma vuoto, no?

per la parte 2 di tone, non so, c'è la progmem di mezzo e un array di un valore.. e tra l'altro mi sembra errata lal ibreria pèerchè fa

tone_pin_to_timer_PGM + i

invece io mi aspetterei fosse un
tone_pin_to_timer_PGM[i] ovvero che legge il prossimo

ma poi in realtà l'uso di più timer cozza con le definizioni multiple ehh. bho è una schifezza di codice, scritto metà per supportare più timer e metà per funzionare con un timer solo. Meriterebbe una riscrittura.

MauroTec opinioni sul da farsi?

per la 3° parte se ho capito bene avete già risolto, giusto? mi posti il codice corretto? graaazie :slight_smile:

per chi volesse clonare, giusto per testare e senza portarsi appresso tutto il mondo, ecco il comando git:

git clone --depth 1 -b ide-1.5.x https://github.com/lestofante/Arduino.git

git: comando e ci siamo
clone: crea un clone del repo
--depth 1: non impora la storia, ma soloilmomento attuale
-b ide-1.5.x: clona solo il branch ide-1.5.x
https://github.com/lestofante/Arduino.git: da dove pescare il codice

ps. nei preferencies.txt ho tolto il -w, ma NON ho messo il -Wall. Anche quì, sono aperto a suggerimenti
pps. e ai SAM nessuno ci pensa? mi aspeto avvenga di conseguenza,ma essendo parti del codice diverse...bhe vedremo poi

edit: la mia versione si basa sull'attuale 1.5.x, questo per evitareun pò di casini eventuali quando faremo la pull-request :slight_smile:

Per Tone.cpp non ho risolto, io credevo si trattasse di un bug del compilatore risolto con la 4.7.2, almeno così ho letto in rete.

Per UDR ecc, non ti seguo non vedo warning, comunque i warning unused lasciamoli stare al momento.

#if defined(UDR0)
    if (bit_is_clear(UCSR0A, UPE0)) {
      unsigned char c = UDR0;
      store_char(c, &rx_buffer);
    } [color=brown]else {
      unsigned char c = UDR0;[/color]
    };

Che senso ha questo codice in HardwareSerial?
Il ; dopo chiusura blocco. Il codice in brown color si deve eliminare, in quanto non ha senso creare c se poi nella funzione ISR non viene usata. Ho modificato anche altre cose in HardwareSerial che non ho incluso nello zipfile, l'ho dimenticato.

Rivalutazione
Il codice in brown color deve essere mantenuto, in quanto ha la funzione di scartare il carattere, nel caso in cui PE0 in UCSRnA is set, però io non vedo abilitato (UPMn1 = 1) in UCSRnC. Comunque mi sembra di aver risolto l'unused di c così:

 #if defined(UDR0)
    unsigned char c;    
    if (bit_is_clear(UCSR0A, UPE0)) {
      c = UDR0;
      store_char(c, &rx_buffer);
    } else {
      c = UDR0;
    }
  #elif defined(UDR)
    unsigned char c;
    if (bit_is_clear(UCSRA, PE)) {
      c = UDR;
      store_char(c, &rx_buffer);
    } else {
      c = UDR;
    }
  #else
    #error UDR not defined

Ciao.

nid69ita:
wiring.c (nella delay() richiama la yield() !?!)

La Due ha un proto-scheduler di tipo cooperativo basato sul SysTick, un timer interno, e sull'uso della funzione yeld per passare il controllo da un task all'altro. Non so però il motivo per cui la trovi nel core Avr, dovrebbe essere solo nel core Arm.

Ah, ho capito. Nel wiring.c del core Avr la funzione yield è una macro che serve ad attendere esattamente 1 microsecondo eseguendo una particolare istruzione assembly, ed è usata nella funzione delay per contare il tempo non usando più millis. In teoria quindi delay dovrebbe funzionare anche dentro una ISR.

Invece in wiring.c del core Sam la funzione di yield è proprio quella di eseguire altri task mentre il core è in un delay.

MauroTec:
Le -Dxx sono macro utente visibili in ogni unit compile, come appunto la versione dell'ide -DARDUINO=155

Si @Mauro è chiaro. Trovo strano però che nelle 2 versioni IDE vengano poi dichiarate altre -D diverse tra i due. A che serviranno? Influenzano qualche lib? Magari causano compilazioni diverse? Non credo sia importante però un ma, mi sovviene.

@Lesto
Per i SAM si usa la toolchian ARM che mi pare sia l'ultima.

Si @Mauro è chiaro. Trovo strano però che nelle 2 versioni IDE vengano poi dichiarate altre -D diverse tra i due. A che serviranno? Influenzano qualche lib? Magari causano compilazioni diverse? Non credo sia importante però un ma, mi sovviene.

Non ho avuto il tempo per indagare, sarebbe todo, se puoi cerca quelle macro in tutto il codice con find e vedi cosa fanno.

Per Tone.cpp non ho risolto, io credevo si trattasse di un bug del compilatore risolto con la 4.7.2, almeno così ho letto in rete.

Mi autoquoto. Il problema è stato risolto come pensavo a partire dalla 4.7.2 infatti nel file degli errori allegato da gpb01 non c'è quel warning.

Ciao.

MauroTec:
Per Tone.cpp non ho risolto, io credevo si trattasse di un bug del compilatore risolto con la 4.7.2, almeno così ho letto in rete.

io ho letto il codice della tone e mi pare una porcheria per i motivi cui sopra. Che poi un array dia warning starni sul progmem che cmq da warning anche sulle variabili ci sta.

MauroTec:
Rivalutazione
Il codice in brown color deve essere mantenuto, in quanto ha la funzione di scartare il carattere, nel caso in cui PE0 in UCSRnA is set, però io non vedo abilitato (UPMn1 = 1) in UCSRnC.

immaginavo, in oltre è obbligatorio leggere PRIMA UCSRA e poi UDR, però vedo che la cosa un pò varia, in oltre non è tenuto conte l'errore FE (Frame Error, bit 4) e DOR (Data OverRun, bit 3)

ora mi studio un pò il datasheet, vediamo che salta fuori

da datasheet:

unsigned int USART_Receive( void )
{
unsigned char status, resh, resl;
/* Wait for data to be received */
while ( !(UCSRnA & (1<<RXCn)) )
;
/* Get status and 9th bit, then data */
/* from buffer */
status = UCSRnA;
resh = UCSRnB;
resl = UDRn;
/* If error, return -1 */
if ( status & (1<<FEn)|(1<<DORn)|(1<<UPEn) )
return -1;
/* Filter the 9th bit, then return */
resh = (resh >> 1) & 0x01;
return ((resh << 8) | resl);
}

nid69ita:

/avr/bin/avr-g++ -c -g -Os -w -fno-exceptions -ffunction-sections -fdata-sections -MMD -mmcu=atmega328p -DF_CPU=16000000L -DARDUINO=155 -DARDUINO_AVR_UNO -DARDUINO_ARCH_AVR -I/...

\avr\bin\avr-g++ -c -g -Os -Wall -fno-exceptions -ffunction-sections -fdata-sections -MMD -mmcu=atmega328p -DF_CPU=16000000L  -DARDUINO=105 -DUSB_VID=null -DUSB_PID=null  -I/...



Le altre define che fanno? I vari -D qualcosa ?

Dalla versione 1.5.3 sono cambiate le define passate al compilatore per definire l'hardware per cui si compila.

Dalla 1.5.3 si usa -DARDUINO_{board} e poi -DARDUINO_ARCH_AVR oppure ARDUINO_ARCH_SAM
Mentre nella 1.0.x venivano usate USB_VID=xxx -DUSB_PID=xxx
Secondo me non è poi così banale, ma forse poco importante. Nel core e nelle librerie distribuite dal Team si saranno adeguati.
Chissà nelle librerie di terze parti se il tutto funziona ancora (se si basano su quelle define USB qualcosa).

Esempio nella libreria fornita dal Team TFT, c'era:

#if (USB_VID == 0x2341) && (USB_PID == 0x803C) // are we building for Esplora?

ora c'e':

#if ARDUINO_AVR_ESPLORA // are we building for Esplora?

Stranamente in 1.5.5 è poco presente questo ARDUINO_xxx e di fisso han messo USB_VID = 0x2341 in Arduino.h se SAM (usata questa macro in USBcore.cpp)

#define USB_VID            0x2341 // arduino LLC vid