Mon code est-il juste ???

Bien le bonjour
Je suis entrain de faire faire mon premier programe pour arduino
Ce programme sert a diriger un robot via une ligne noir
J'ai voulu testé sous proteus ISIS(je n'ais pas encore la carte et le matériel) mais ... sa apas été =( donc je vien a vous pour voir si mon code est juste et ci j'ai fait des erreur, m'expliquer pourquoi et comment les corriger
Voila mon code actuelle (il peut se diriger tout droit uniquement)
Nouveau (dernière modif 1/02/12 22:00)

//int debug = 1; // -> utiliser un couple variable + if pour un test de debug n'est pas génial   
#define __DEBUG__ // Mieux vaut utiliser un define et un test AVANT la compilation (#ifdef)   

boolean sTop = 0; // Préférer le type byte (1 octet) au int (2 octets) pour les valeurs <255      
boolean start = 0; //
byte pin_read[11]; //
unsigned long temp_depart; // Les valeurs retourner par millis() sont des unsigned long pas des int 

void setup()
{
  //capteurs
  pinMode(2, INPUT);
  pinMode(3, INPUT);
  pinMode(4, INPUT);
  pinMode(5, INPUT);
  pinMode(6, INPUT);
 
  pinMode(7, INPUT); //Arret bouton
  pinMode(8, INPUT); //Démarage via cordon
  pinMode(9, INPUT);//debug on/off
  
  //moteur
  pinMode(10, OUTPUT);
  pinMode(11, OUTPUT);
  pinMode(12, OUTPUT);
  pinMode(13, OUTPUT);
  
  Serial.begin(9600); //Port série
}

void loop()
{
  for(int i = 2; i <= 10; i++) // l'utiliation d'une boucle for est ici bien plus intéréssante et pratique
    pin_read[i] = digitalRead(i);

  if (pin_read[7] == HIGH) sTop = 1; // Les accolades sont inutile si il n'y a qu'une seul intrustion dans le if 

     if (pin_read[9] == HIGH) debug =1; 
     else if (pin_read[9] == LOW) debug = 0;

  if (!pin_read[8] && !start) // défini le démarage et enregistre le temp de départ
  {
    temp_depart = millis();
    start = 1;
  }
  //int temp_ecoule = millis() - temp_depart;
  unsigned long temp_ecoule = millis() - temp_depart; // Les valeurs retourner par millis() sont des unsigned long pas des int
  
  if (!sTop && temp_ecoule < 90000 && start) 
  {
#ifdef __DEBUG__ // Remplacement du if debug == 1
     for(int i = 2; i <= 10; i++)
     {
        Serial.print("Capteur ");
        Serial.print(i);
		Serial.print(' : ');
        Serial.println(analogRead(i));
        delay(10);
     }
#endif

     if (!pin_read[2] && !pin_read[3] && pin_read[4] && !pin_read[5] && !pin_read[6]) //Tout droit
     {
       digitalWrite(9, 1);
       digitalWrite(10, 0);
       digitalWrite(11, 1);
       digitalWrite(12, 0);
     }
  }
}

ancien :

/*
Capteur
2 = extreme gauche
3 = gauche
4 = tout droit
5 = droit
6 =  extreme droit
7 = moteur droit
8 = moteur gauche
9 = bp STOP
*/
const int capteur[10] = {2, 3, 4, 5, 6, 7, 8, 9, 10};
int debug = 1; // debug actif
int mode = 0 ;// Modde du robot 0 = auto, 1 = manuelle
int sTop = 0;
int start = 0;
int temp_depart;
int pin_read[10];
int init_pin;

void setup()
{
  pinMode(2, INPUT);
  pinMode(3, INPUT);
  pinMode(4, INPUT);
  pinMode(5, INPUT);
  pinMode(6, INPUT);
  pinMode(7, OUTPUT);//moteur1
  pinMode(8, OUTPUT);//Moteur1
  pinMode(11, OUTPUT);//Moteur 2
  pinMode(12, OUTPUT);//Moteur 2
  pinMode(9, INPUT);// bp
  pinMode(10, INPUT);//Ficelle qui définis le démarage
  Serial.begin(9600); //port série
}

void loop()
{
  init_pin = 0;
   while(init_pin < 11) // lis la valeur des 10 premier pin
  {
    pin_read[init_pin] = 0;
    pin_read[init_pin] = digitalRead(init_pin);
    init_pin++;
  }
  if (pin_read[9] == HIGH) // SI le BP est appuier Le robot passe sTop^a 1 se qui l'arretera
  {
    int sTop = 1;
  }
  if (pin_read[10] == LOW && start == 0) // défini le démarage et remet millis à 0
  {
    temp_depart = millis();
    start = 1;
  }
  int temp_ecoule = millis() - temp_depart;
  if (sTop == 0 && temp_ecoule < 90000 && start == 1) // tant que sTop = 0 ET que millis() est < à 90000 Milliseconds (90 seconde) ET que start = 1 le robot ira 
  {
    int ncapteur = 1;
     while (ncapteur < 11 && debug == 1)
     {
        Serial.print("\n By Adrien V. \n Capteur n-");
        Serial.print(ncapteur);
        Serial.print(analogRead(capteur[ncapteur]));
        ncapteur++;
        delay(10);
     }
     /*
     Commande motteur
     relais mot 1: pin 8 et 9
     relais mot 2 : pin 11 et 12
     Capteur : pin 2 à 6
     */
     if (pin_read[2] == LOW && pin_read[3] == LOW && pin_read[4] == HIGH && pin_read[5] == LOW && pin_read[6] == LOW) //Tout droit
     {
       digitalWrite(8, 1);
       digitalWrite(9, 0);
       digitalWrite(11, 1);
       digitalWrite(12, 0);
     }
  }
}

Merci d’avance :slight_smile:

Pour ce que je peut lire (sur mon portable) il y a une premiere erreur :
Si tu a un tableau monTableau de 10 cases(int monTableau[10];), la premiere casessera monTableau[0] et la derniere monTableau[9].
En esperant t'avoir aide.

Salut,

Voila ton code avec mes corrections + commentaires :wink:
Dans l'ensemble c'est relativement pas mal mais "un peu" fouillis.

Ce que j'ai pu voir :

  • une erreur d'index sur ton tableau,
  • quelques optimisations simple à faire au niveau des types de variable,
  • des assignations de variable inutile,
  • une grosse erreur de redéfinition locale,
  • des boucles mal pensé (juste mais pas idéalement conçu).
//int debug = 1; // -> utiliser un couple variable + if pour un test de debug n'est pas génial
#define __DEBUG__ // Mieux vaut utiliser un define et un test AVANT la compilation (#ifdef)

const byte capteur[10] = {2, 3, 4, 5, 6, 7, 8, 9, 10};
//int mode = 0 ; // Variable inutilisé
byte sTop = 0; // Préférer le type byte (1 octet) au int (2 octets) pour les valeurs <255
byte start = 0; //
byte pin_read[10]; //
unsigned long temp_depart; // Les valeurs retourner par millis() sont des unsigned long pas des int

void setup()
{
  pinMode(2, INPUT);
  pinMode(3, INPUT);
  pinMode(4, INPUT);
  pinMode(5, INPUT);
  pinMode(6, INPUT);
  
  pinMode(7, OUTPUT);
  pinMode(8, OUTPUT);
  pinMode(11, OUTPUT);
  pinMode(12, OUTPUT);
  
  pinMode(9, INPUT);
  pinMode(10, INPUT);
  
  Serial.begin(9600);
}

void loop()
{
  //init_pin = 0;
  //while(init_pin < 11)
  //{
  //  pin_read[init_pin] = 0; // Initialisation inutile, la valeur est assignation à la ligne suivante
  //  pin_read[init_pin] = digitalRead(init_pin);
  //  init_pin++;
  //}
  for(int i = 0; i < 10; i++) // l'utiliation d'une boucle for est ici bien plus intéréssante et pratique
    pin_read[i] = digitalRead(capteur[i]);

  //if (pin_read[9] == HIGH)
  //{
  //  int sTop = 1; // Redéfinition de sTop dans le if -> erreur
  //}
  if (pin_read[9] == HIGH) sTop = 1; // Les accolades sont inutile si il n'y a qu'une seul intrustion dans le if 

  if (pin_read[10] == LOW && start == 0) // défini le démarage et remet millis à 0
  {
    temp_depart = millis();
    start = 1;
  }
  //int temp_ecoule = millis() - temp_depart;
  unsigned long temp_ecoule = millis() - temp_depart; // Les valeurs retourner par millis() sont des unsigned long pas des int
  
  if (sTop == 0 && temp_ecoule < 90000 && start == 1) 
  {
    //int ncapteur = 1;
    //while (ncapteur < 11 && debug == 1) // Erreur ncapteur va de 1 à 11 or capteur[] va de l'index 0 à l'index 9
    //{
    //   Serial.print("\n By Adrien V. \n Capteur n-");
    //   Serial.print(ncapteur); // Affichage collé à la valeur -> ilisible
    //   Serial.print(analogRead(capteur[ncapteur]));
    //   ncapteur++;
    //   delay(10);
    //}
#ifdef __DEBUG__ // Remplacement du if debug == 1
     for(int i = 0; i < 10; i++)
     {
        Serial.print("Capteur ");
        Serial.print(i + 1);
		Serial.print(' ');
        Serial.println(analogRead(capteur[i]));
        delay(10);
     }
#endif

     if (pin_read[2] == LOW && pin_read[3] == LOW && pin_read[4] == HIGH && pin_read[5] == LOW && pin_read[6] == LOW) //Tout droit
     {
       digitalWrite(8, 1);
       digitalWrite(9, 0);
       digitalWrite(11, 1);
       digitalWrite(12, 0);
     }
  }
}

Merci de ta réponsse pourais tu m'expliquer ce que c'est " #ifdef DEBUG "

adrien4607:
Merci de ta réponsse pourais tu m'expliquer ce que c'est " #ifdef DEBUG "

C'est une instruction pré-processeur qui permet d'ignorer ou non une partie du code source lors de la compilation.
-> Programmation C++/Le préprocesseur — Wikilivres

Edit de Jean-François : modification du lien XD

Merci :slight_smile:
Voila ce que sa donne, j'ai rajouté 1 truc (le debug) car j'avaies oublier de prendre en compte l’interrupteur qui fait aler ou non le debug

//int debug = 1; // -> utiliser un couple variable + if pour un test de debug n'est pas génial
#define __DEBUG__ // Mieux vaut utiliser un define et un test AVANT la compilation (#ifdef)

const byte capteur[10] = {2, 3, 4, 5, 6, 7, 8, 9, 10};
byte sTop = 0; // Préférer le type byte (1 octet) au int (2 octets) pour les valeurs <255
byte start = 0; //
byte pin_read[11]; //
unsigned long temp_depart; // Les valeurs retourner par millis() sont des unsigned long pas des int

void setup()
{
  //capteurs
  pinMode(2, INPUT);
  pinMode(3, INPUT);
  pinMode(4, INPUT);
  pinMode(5, INPUT);
  pinMode(6, INPUT);
  
  //moteur
  pinMode(7, OUTPUT);
  pinMode(8, OUTPUT);
  pinMode(11, OUTPUT);
  pinMode(12, OUTPUT);
  
  pinMode(9, INPUT); //Arret bouton
  pinMode(10, INPUT); //Démarage via cordon
  pinMode(13, INPUT);//debug on/off
  
  Serial.begin(9600); //Port série
}

void loop()
{
  for(int i = 0; i < 11; i++) // l'utiliation d'une boucle for est ici bien plus intéréssante et pratique
    pin_read[i] = digitalRead(capteur[i]);

  if (pin_read[8] == HIGH) sTop = 1; // Les accolades sont inutile si il n'y a qu'une seul intrustion dans le if 

     if (pin_read[12] == HIGH) debug =1; 
     else if (pin_read[12] == LOW) debug = 0;

  if (pin_read[10] == LOW && start == 0) // défini le démarage et enregistre le temp de départ
  {
    temp_depart = millis();
    start = 1;
  }
  //int temp_ecoule = millis() - temp_depart;
  unsigned long temp_ecoule = millis() - temp_depart; // Les valeurs retourner par millis() sont des unsigned long pas des int
  
  if (sTop == 0 && temp_ecoule < 90000 && start == 1) 
  {
#ifdef __DEBUG__ // Remplacement du if debug == 1
     for(int i = 0; i < 10; i++)
     {
        Serial.print("Capteur ");
        Serial.print(i + 1);
		Serial.print(' ');
        Serial.println(analogRead(capteur[i]));
        delay(10);
     }
#endif

     if (pin_read[2] == LOW && pin_read[3] == LOW && pin_read[4] == HIGH && pin_read[5] == LOW && pin_read[6] == LOW) //Tout droit
     {
       digitalWrite(7, 1);
       digitalWrite(8, 0);
       digitalWrite(11, 1);
       digitalWrite(12, 0);
     }
  }
}

Yop,
Tableau constant de taille 10 ne contenant que 9 valeurs.
Tableau inutiles par ailleurs vu que ce sont des valeurs connues et successive,dis toi que les suites de nombre (2,3,4,5,6,7,8,9,10) et (0,1,2,3,4,5,6,7,8,9) c'est la même chose augmenté de 2 pour la première, tu dois facilement trouvé l'algorithme logique qui te permettras d'évité ce tableau :grin: ...
Il y a vraiment besoin de stocker les valeurs des entrée au lieux de simplement les lire directement là ou tu en as besoin ?
Je vois que tu redéclares certaine variables dans le code comme "int sTop = 1;" ?
Comme je l'indique souvent aux débutant, utilisation à profusion du type int (il n'y a pas que int qui peux prendre des nombre entier, même un char peux en prendre) pour des valeurs ne dépassant jamais 1 octet (valeur -127 à 128 signé ou de 0 à 255 en non signé) et encore pire pour des booléens qui n'auront jamais que deux états, 1 ou 0, vrai ou faux.

#define __DEBUG__

boolean mode = 0 ;// Modde du robot 0 = auto, 1 = manuelle
boolean sTop = 0;
boolean start = 0;
unsigned long temp_depart;

void setup()
{
  pinMode(2, INPUT);
  pinMode(3, INPUT);
  pinMode(4, INPUT);
  pinMode(5, INPUT);
  pinMode(6, INPUT);
  pinMode(7, OUTPUT);//moteur1
  pinMode(8, OUTPUT);//Moteur1
  pinMode(11, OUTPUT);//Moteur 2
  pinMode(12, OUTPUT);//Moteur 2
  pinMode(9, INPUT);// bp
  pinMode(10, INPUT);//Ficelle qui définis le démarage
  Serial.begin(9600); //port série
}

void loop()
{
  if (digitalRead(9)) // SI le BP est appuier Le robot passe sTop^a 1 se qui l'arretera
  {
    sTop = 1;
  }
  if (!digitalRead(10) && start == 0) // défini le démarage et remet millis à 0
  {
    temp_depart = millis();
    start = 1;
  }

  unsigned long temp_ecoule = millis() - temp_depart;
  if (!sTop && temp_ecoule < 90000 && start) // tant que sTop = 0 ET que millis() est < à 90000 Milliseconds (90 seconde) ET que start = 1 le robot ira 
  {
#ifdef __DEBUG__ 
     for(byte pin = 2; pin <= 10; pin++)
     {
        Serial.print("\n By Adrien V. \n Capteur n-");
        Serial.print(pin);
        Serial.print(analogRead(pin);
        delay(10);
     }
#endif

     /*
     Commande motteur
     relais mot 1: pin 8 et 9
     relais mot 2 : pin 11 et 12
     Capteur : pin 2 à 6
     */
     if (!digitalRead(2) && !digitalRead(3) && digitalRead(4) && !digitalRead(pin) && digitalRead(pin)!) //Tout droit
     {
       digitalWrite(8, 1);
       digitalWrite(9, 0);
       digitalWrite(11, 1);
       digitalWrite(12, 0);
     }
  }
}

Enfin je ne vois là que de petites erreurs que tout les débutants font, juste un petit manque de formation ? :grin:
Le mieux c'est de posé des questions sur ce que tu ne comprend pas ou que tu n'as pas encore vus.

Tableau de taille 11 pour contenir 9 valeurs.
je vois un problème dans les "for" les valeurs irons de 0 à 10 ce qui fais une boucle de 11 tours pour la première et 10 pour la deuxième, alors qu'il n'y a que 9 pin en entrée (pin2 à pin 10) ...

adrien4607:

  for(int i = 0; i < 11; i++)

pin_read[i] = digitalRead(capteur[i]);

for(int i = 0; i < 10; i++)
    {
       Serial.print("Capteur ");
       Serial.print(i + 1);
Serial.print(' ');
       Serial.println(analogRead(capteur[i]));
       delay(10);
    }

}

Edit: Entre () je mettrais toute les accolades {} au condition if et boucle même si pas indispensable, c'est plus propre et mieux comme repère pour un débutant.
Edit: Je vois que les pin 7 et 8 sont utilisé pour le moteurs1 donc en sortie ... c'était peut être pour celà l'histoire des tableaux ?

Bon revoilà mon code modifier et corrigé

//int debug = 1; // -> utiliser un couple variable + if pour un test de debug n'est pas génial   
#define __DEBUG__ // Mieux vaut utiliser un define et un test AVANT la compilation (#ifdef)        

boolean sTop = 0; // Préférer le type byte (1 octet) au int (2 octets) pour les valeurs <255         
boolean start = 0; //
byte pin_read[11]; //
unsigned long temp_depart; // Les valeurs retourner par millis() sont des unsigned long pas des int 

void setup()
{
  //capteurs
  pinMode(2, INPUT);
  pinMode(3, INPUT);
  pinMode(4, INPUT);
  pinMode(5, INPUT);
  pinMode(6, INPUT);
 
  pinMode(7, INPUT); //Arret bouton
  pinMode(8, INPUT); //Démarage via cordon
  pinMode(9, INPUT);//debug on/off
  
  //moteur
  pinMode(10, OUTPUT);
  pinMode(11, OUTPUT);
  pinMode(12, OUTPUT);
  pinMode(13, OUTPUT);
  
  Serial.begin(9600); //Port série
}

void loop()
{
  for(int i = 2; i <= 10; i++) // l'utiliation d'une boucle for est ici bien plus intéréssante et pratique
    pin_read[i] = digitalRead(i);

  if (pin_read[7] == HIGH) sTop = 1; // Les accolades sont inutile si il n'y a qu'une seul intrustion dans le if 

     if (pin_read[9] == HIGH) debug =1; 
     else if (pin_read[9] == LOW) debug = 0;

  if (!pin_read[8] && !start) // défini le démarage et enregistre le temp de départ
  {
    temp_depart = millis();
    start = 1;
  }
  //int temp_ecoule = millis() - temp_depart;
  unsigned long temp_ecoule = millis() - temp_depart; // Les valeurs retourner par millis() sont des unsigned long pas des int
  
  if (!sTop && temp_ecoule < 90000 && start) 
  {
#ifdef __DEBUG__ // Remplacement du if debug == 1
     for(int i = 2; i <= 10; i++)
     {
        Serial.print("Capteur ");
        Serial.print(i);
		Serial.print(' : ');
        Serial.println(analogRead(i));
        delay(10);
     }
#endif

     if (!pin_read[2] && !pin_read[3] && pin_read[4] && !pin_read[5] && !pin_read[6]) //Tout droit
     {
       digitalWrite(9, 1);
       digitalWrite(10, 0);
       digitalWrite(11, 1);
       digitalWrite(12, 0);
     }
  }
}

Je vois toujours un tableau de taille 11 -> "byte pin_read[11];" , pour 8 valeurs (de 2 à 9) seulement.
Si l'un commence à 2(les pins), le tableau lui commence bien à 0 donc i-2 (on aurais pus faire l'inverse digitalRead(i+2)).

for(byte i = 2; i <= 9; i++)
    pin_read[i-2] = digitalRead(i);

ou

for(byte i = 0; i <= 7; i++)
   pin_read[i] = digitalRead(i+2);

Ne pas hésité à suivre le cheminement d'une boucle en comptant sur les doigts ou en écrivant l'incrémentation sur papier, ça peut paraitre c.. et enfantin mais certaine partie visuel du code peux nous induire en erreurs. :wink:

Je fais pour ne pas me perde

Si Je veux avoir la valleur du pin 5, je tappe pin_read[5], alors que si je fait comme tu dit je doit mettre pin_read[3], et j'ai déja bloqué a cause de sa
J'ai peut-être deux valeur du tableau qui son vide donc qui ne me serve a rien mais est-ce vraiment grave ?

adrien4607:
Je fais pour ne pas me perde

Si Je veux avoir la valleur du pin 5, je tappe pin_read[5], alors que si je fait comme tu dit je doit mettre pin_read[3], et j'ai déja bloqué a cause de sa
J'ai peut-être deux valeur du tableau qui son vide donc qui ne me serve a rien mais est-ce vraiment grave ?

Je dirais que c'est pas dramatique mais ici sur l'atmega la mémoire est précieuse et il vaut mieux ne pas la gaspillé, sur pc on aurais pas insisté (idem pour les int-byte), mais ici :~.
Au temps prendre les bonne habitudes de programmeur, ça viendra avec le temps.