Code funktioniert nur manchmal

[someuser]

hi,


Ich habe meinen code mal versucht wie vorgeschlagen zu ändern.

zwei Beispiele unten,
das erste funktioniert reibungslos, das zweite geht ein paar mal hintereinander, ein paar mal nicht,1
dann geht´s wieder   

im seriellen Monitor kann ich die Eingabe der Zahlen verfolgen, und auch das
Ausgang a oder b geschaltet wird. das gehrt mal 3 mal fünfmal hintereinander, dann
sehe ich nur noch den code, aber es springt nicht weiter. nach ein paar weiteren versuchen
geht es wieder 1-2 mal nach einem hardwarereset auch meistens.

Habe das Gefühl das Programm verhaspelt sich beim codereset.


Kurze Zwischenfrage zu:
static byte counter = 0;
Warum muss es ein static byte sein, kein int, bzw warum  funktioniert int nicht?

und return false ist doch das gleiche wie return 0?


hier die codes:

Code: [Select]


#include <Keypad.h>

#define LED 12

char masterCode[] = "123456";
char limitedCode[]= "112233";

const byte ROWS = 4; // vier Reihen
const byte COLS = 4; // vier Spalten
// Define the Keymap
char keys[ROWS][COLS] = {
  {'1','2','3','A'},
  {'4','5','6','B'},
  {'7','8','9','C'},
  {'*','0','#','D'}
};
// Connect keypad ROW0, ROW1, ROW2 and ROW3 to these Arduino pins.
byte rowPins[ROWS] = { 2, 3, 4, 5 };
// Connect keypad COL0, COL1 and COL2 to these Arduino pins.
byte colPins[COLS] = { 6, 7, 8, 9 };


Keypad keypad = Keypad( makeKeymap(keys), rowPins, colPins, ROWS, COLS );


boolean validCode()
{
 static byte counter = 0;
 static char code[sizeof(masterCode)];
 char key = keypad.getKey();
 if (!key) return false; // keine Taste gedrückt, kann nicht gültig sein
 if (counter==0) memset(code,0,sizeof(code)); // clear code string
Serial.println(key); // comment out if not needed for debugging
 switch (key)
 {
   case '*': counter=0; // start new code
             break;
   case '#': if (strcmp(code,masterCode)==0) // maybe correct code?
             {
               counter=0;
               return true; // correct masterCode was entered
             }
             if (true==true) // hier die limitierende Bedingung einsetzen
             {
               if (strcmp(code,limitedCode)==0) // mayby the limitedCode?
               {
                 counter=0;
                 return true; // yes, the limitedCode is correct
               }
             }
             break;
   default : code[counter]=key; // just add the keystroke to the code
             if (counter<sizeof(code)-1) counter++;
 
 }
 return false; // es wurde kein gültiger Code eingegeben 
}


void setup(){
 Serial.begin(9600);
 pinMode(LED,OUTPUT);
 digitalWrite(LED,HIGH);
}
 
void loop(){
 if (validCode()) 
 {
   Serial.println("VALID CODE");
 
   
     digitalWrite(LED,LOW);
     delay(2500);
     digitalWrite(LED,HIGH);
     
   
 }
}


Code: [Select]


#include <Keypad.h>

#define LED1 12
#define LED2 11

char masterCode[] = "123456";
char limitedCode[] = "112233";

const byte ROWS = 4; // vier Reihen
const byte COLS = 4; // vier Spalten
// Define the Keymap
char keys[ROWS][COLS] = {
  {'1','2','3','A'},
  {'4','5','6','B'},
  {'7','8','9','C'},
  {'*','0','#','D'}
};
// Connect keypad ROW0, ROW1, ROW2 and ROW3 to these Arduino pins.
byte rowPins[ROWS] = { 2, 3, 4, 5 };
// Connect keypad COL0, COL1 and COL2 to these Arduino pins.
byte colPins[COLS] = { 6, 7, 8, 9 };


Keypad keypad = Keypad( makeKeymap(keys), rowPins, colPins, ROWS, COLS );


int validCode()
{
static byte counter = 0;
 static char code[sizeof(masterCode)];
 char key = keypad.getKey();
 if (!key) return false; // keine Taste gedrückt, kann nicht gültig sein
 if (counter==0) memset(code,0,sizeof(code)); // clear code string
Serial.println(code); // comment out if not needed for debugging
   Serial.println(key); // comment out if not needed for debugging
 switch (key)
 {
   case '*': counter=0; // start new code
             break;
   case '#': if (strcmp(code,masterCode)==0) // maybe correct code?
      {
        counter=0;
        return 1; // correct masterCode was entered
      }
        if (true==true) // hier die limitierende Bedingung einsetzen
      {
        if (strcmp(code,limitedCode)==0) // mayby the limitedCode?
        {
          counter=0;
          return 2; // yes, the limitedCode is correct
        }
      }
      break;
    default : code[counter]=key; // just add the keystroke to the code
      if (counter<sizeof(code)-1) counter++;
 }
  if (counter == 0) memset(code,0, sizeof(code)); // clear code string
 return false; // es wurde kein gültiger Code eingegeben
}


void setup(){
 Serial.begin(9600);
 pinMode(LED1,OUTPUT);
  digitalWrite(LED1, LOW);
  pinMode(LED2, OUTPUT);
 digitalWrite(LED2,HIGH);
 
}

void loop(){

 if (validCode()==1)
{


        Serial.println("aVALID CODE");

        digitalWrite(LED1, HIGH);
        delay(1500);
        digitalWrite(LED1, LOW);

}

  if (validCode()==2)

{
       Serial.println("bVALID CODE");
 
        digitalWrite(LED2, LOW);
        delay(1500);
        digitalWrite(LED2, HIGH);

 }       
}


[Doc_Arduino]

Hallo,

du liest und vergleichst einzelne Ziffern und nicht die Zahlenfolge.

[someuser]

verstehe ich nicht ganz.
an welcher stelle?

ich sehe im monitor ja eigentlich den ganzen code und er wird
auch nur akzeptiert wenn er vollständig ist.

[Serenifly]

validCode() solltest du in loop() nur einmal aufrufen. Speichere den Rückgabewert in einer Variablen. Dann kannst du diese mehrmals vergleichen

Dann hast du für den Rückgabewert den Datentyp boolean, aber Werte von 0-2. Das geht nicht. boolean kann nur 0 oder 1 sein. Verwende Byte als Rückgabewert wenn du mehr als true/false brauchst!
Und um den Werten sprechende und aussagekräftige Namen zu geben gibt es enums.

Kurze Zwischenfrage zu:
static byte counter = 0;
Warum muss es ein static byte sein, kein int, bzw warum  funktioniert int nicht?

Lokale statische Variablen behalten ihren Inhalt nach dem Verlassen der Funktion


Hier sollte beides in die if-Abfrage

Code: [Select]


    default : code[counter]=key; // just add the keystroke to the code
      if (counter<sizeof(code)-1) counter++;

Sonst kannst du die letzte Ziffer immer wieder überschreiben

[someuser]

validCode() solltest du in loop() nur einmal aufrufen. Speichere den Rückgabewert in einer Variablen. Dann kannst du diese mehrmals vergleichen

hab irgendwo gelesen das das zwar nicht schön ist aber geht.
glaube aber inzwischen auch, das so wie ich das aufrufe keine gute idee ist.

Dann hast du für den Rückgabewert den Datentyp boolean, aber Werte von 0-2.

hab in dere 2. version eigentlich auf int gestellt. warum byte statt int? wegen weniger speicher?

Hier sollte beides in die if-Abfrage
Code: [Select]

    default : code[counter]=key; // just add the keystroke to the code
      if (counter<sizeof(code)-1) counter++;

Sonst kannst du die letzte Ziffer immer wieder überschreiben

das versteh ich nicht ganz.so:?

Code: [Select]


default :
if(counter<sizeof(code)-1)
{
code[counter]=key; // just add the keystroke to the code
counter++
};


wo genau ist der unterschied?

[Serenifly]

hab in dere 2. version eigentlich auf int gestellt. warum byte statt int? wegen weniger speicher?

Int geht auch. Aber du hast hier keine negativen Zahlen.

Int nimmt man bei sowas um mit 0-255 eine Anzahl oder einen Wert zurückzugeben und -1 für "nichts" oder "ungültig". Vergleiche z.B. Serial.read()

das versteh ich nicht ganz.so:?

Gehe es im Kopf mal durch, dann sollte das doch sofort auffallen. Bei deiner Version wird eine Ziffer am Ende des Array eingetragen, auch wenn es voll ist. Du verhinderst zwar dass man darüber hinaus schreibt, aber die letzte Stelle wird immer überschrieben wenn man weiter drückt. Das verursacht keinen Fehler, aber es ist einfach nicht intuitiv.

[Doc_Arduino]

Hallo,

sorry das ich den Code nicht richtig gelesen habe, dass das in switch-default abläuft habe ich übersehen.

[someuser]

Bei deiner Version wird eine Ziffer am Ende des Array eingetragen

eigentlich hab ich den hier irgendwo geklaut

dass das in switch-default abläuft habe ich übersehen

das hat ne ziemliche weile gedauert bis ich das raus hatte...


werds morgen nochmal testen

[Doc_Arduino]

Hallo,

die letzte Stelle immer überschreiben würde ich sogar hier für nicht falsch halten. Damit bekommt eine fremde Person nicht die Anzahl der Ziffern mit die überhaupt abgefragt werden. Falls jemand blind probiert.

Das case-switch würde ich jedoch zum Zustandsautomaten umbauen mit richtigen Namen, wie WARTEN, READ, START, ENDE oder sonstwas was leserliches und die Start/Ende Abfrage # & # mit in key beim einlesen abfragen. Je nachdem würde hier dann der Zustand geändert. Stichwort enum.
Der Code springt also nicht wie mit if ins case sondern er verweilt im letzten case Zustand.
default würde ich "frei" lassen zum Fehler abfangen. Wenn man das fürs einlesen verwendet hat man keine Möglichkeit mehr Fehler abzufangen. Jedensfalls nicht so einfach. Für jeden benötigten Zustand sollte es ein case geben. Wenn dann etwas schief geht, was nie geplant war kann das mit default abgefangen werden.

Das wären so meine Gedanken.

[someuser]

habs jetzt soweit am laufen, tnx.

hab noch ne frage zu einer erweiterung des codes.
mache da morgen am besten nen neuen thread zu.

[someuser]

tja, zu früh gefreut


im Prinzip funktioniert es.
Es soll nach erstmaligem akzeptieren des codes, für weitere 10 sekunden
möglich sein mit einer beliebigen taste den Ausgang nochmal zu schalten.

Das geht soweit auch, wenn man eine taste innerhalb der 10 sek drückt wird geschaltet
und nach Ablauf der Zeit geht es nicht mehr.
drückt man allerdings nicht während dieser Zeit, kann man beliebig später durch einen
beliebigen Tastendruck den Ausgang schalten.
Die Schleife wird dann scheinbar nicht verlassen.

jemand eine idee?

Code: [Select]


#include <Keypad.h>

#define Tor1 11
#define Tor2 12

char masterCode[] = "123456";
char limitedCode[] = "112233";

const byte ROWS = 4; // vier Reihen
const byte COLS = 4; // vier Spalten
// Define the Keymap
char keys[ROWS][COLS] = {
  {'1', '2', '3', 'A'},
  {'4', '5', '6', 'B'},
  {'7', '8', '9', 'C'},
  {'*', '0', '#', 'D'}
};
// Connect keypad ROW0, ROW1, ROW2 and ROW3 to these Arduino pins.
byte rowPins[ROWS] = { 2, 3, 4, 5 };
// Connect keypad COL0, COL1 and COL2 to these Arduino pins.
byte colPins[COLS] = { 6, 7, 8, 9 };


Keypad keypad = Keypad( makeKeymap(keys), rowPins, colPins, ROWS, COLS );


int validCode()
{
  static byte counter = 0;
  static char code[sizeof(masterCode)];
  char key = keypad.getKey();
  if (!key) return false; // keine Taste gedrückt, kann nicht gültig sein
  if (counter == 0) memset(code, 0, sizeof(code)); // clear code string
  Serial.println(code); // comment out if not needed for debugging
  Serial.println(key); // comment out if not needed for debugging
  switch (key)
  {
    case '*': counter = 0; // start new code
      break;
    case '#': if (strcmp(code, masterCode) == 0) // maybe correct code?
      {
        counter = 0;
        return 1; // correct masterCode was entered
      }
      if (true == true) // hier die limitierende Bedingung einsetzen
      {
        if (strcmp(code, limitedCode) == 0) // mayby the limitedCode?
        {
          counter = 0;
          return 2; // yes, the limitedCode is correct
        }
      }
      break;
    default : code[counter] = key; // just add the keystroke to the code
      if (counter < sizeof(code) - 1) counter++;
  }
  if (counter == 0) memset(code, 0, sizeof(code)); // clear code string
  return false; // es wurde kein gültiger Code eingegeben
}


void setup() {
  Serial.begin(9600);
  pinMode(Tor1, OUTPUT);
  digitalWrite(Tor1, HIGH);
  pinMode(Tor2, OUTPUT);
  digitalWrite(Tor2, HIGH);

}


void loop() {

  byte schluessel = validCode();
  unsigned long startzeit = millis();

  switch (schluessel) {
    case 1:
      Serial.println("Master Code");
      Serial.println("aVALID CODE");

      digitalWrite(Tor1, LOW);
      delay(800);
      digitalWrite(Tor1, HIGH);


      while (millis() - startzeit <= 10000)
      {
label2:
        char taste = keypad.getKey();

        if (!taste) goto label2; // keine Taste gedrückt, kann nicht gültig sein
        Serial.println("Fehler Anfang while");
        if (taste == 'A' || 'B' || 'C' || 'D' || '*' || '#' || '0' || '1' || '2' || '3' || '4' || '5' || '6' || '7' || '8' || '9')
          Serial.println(taste);


        digitalWrite(Tor1, LOW);
        delay(800);
        digitalWrite(Tor1, HIGH);


      };

      // mach was bei Master Code
      break;

     
    case 2:
      Serial.println("Limited Code");
      Serial.println("bVALID CODE");

      digitalWrite(Tor2, LOW);
      delay(800);
      digitalWrite(Tor2, HIGH);

      while (millis() - startzeit <= 10000)
      {
label3:
        char taste = keypad.getKey();

        if (!taste) goto label3; // keine Taste gedrückt, kann nicht gültig sein
        Serial.println("Fall 2");
        if (taste)
          Serial.println(taste);

        digitalWrite(Tor2, LOW);
        delay(800);
        digitalWrite(Tor2, HIGH);

      };
      // mach was bei Limited Code
      break;
  }
}



[Serenifly]

Goto hat bis auf sehr wenige, sehr spezielle Ausnahmen in einem Programm nichts verloren

Mache wie oben gesagt einen Zustandsautomaten daraus. Dann hast du diese Probleme nicht. Jeder Fall/Case ist dann der aktuelle Zustand. Aber statt in einem Zustand irgendwie durch Schleifen zu bleiben läuft loop() einfach immer durch


Das macht nicht was du denkst:

Code: [Select]


if (taste == 'A' || 'B' || 'C' || 'D' || '*' || '#' || '0' || '1' || '2' || '3' || '4' || '5' || '6' || '7' || '8' || '9')

Naiv musst du jedes Zeichen einzeln vergleichen

Schau dir die ASCII Tabelle an. Und verinnerliche dir dass ASCII Zeichen auch nur Integer sind. Dann sollte dir klar werden dass man die auch mit <= und >= auf Bereiche abfragen kann

[michael_x]

Goto hat bis auf sehr wenige, sehr spezielle Ausnahmen in einem Programm nichts verloren

... und wenn du schon mit millis() arbeitest (sehr schön), solltest du dir auch while - Schleifen abgewöhnen, damit es einfacher wird.

Kannst ja mal zum Spass eine LED diese 8 Sekunden lang leuchten lassen. Das wäre dann der Zustand, in dem die Zusatz-Taste gültig wäre.

[postmaster-ino]

Hi

Wieder was gelernt - goto gibt's hier auch noch

MfG

[someuser]

Goto hat bis auf sehr wenige, sehr spezielle Ausnahmen in einem Programm nichts verloren

 dachte mir das das kommt.

im original Programm wurde die if bedingung durch return 0 beendet, das hat mich aber unerwarteter weise nicht  an den anfang von while geworfen, sondern wohl an den anfang der ganzen funktion. wusste nicht wie ich das ohne große änderungen hinbekomme und auf den ersten blick scheint es zu funktionieren.

Das macht nicht was du denkst:
Code: [Select]

if (taste == 'A' || 'B' || 'C' || 'D' || '*' || '#' || '0' || '1' || '2' || '3' || '4' || '5' || '6' || '7' || '8' || '9')

Hab im inet grade nur das gefunden:

this is c++, the conditions will all be checked until the first one fails the condition, and then it will fall out.

heisst das sobald eine taste "nicht true" ist beendet die if funktion? geht aber irgendwie...

im zweiten case habe ich die ganzen tastenabfragen weggelassen, war mir aber nicht sicher ob man
sich damit nicht irgendwelche undefinierten zustände einfängt. wäre diese Methode besser?

Kannst ja mal zum Spass eine LED diese 8 Sekunden lang leuchten lassen. Das wäre dann der Zustand, in dem die Zusatz-Taste gültig wäre.

wenn die zeit in while unter 10 sek. ist, spingt die funktion dann nach:

char taste = keypad.getKey();

        if (!taste) goto label3; // keine Taste gedrückt, kann nicht gültig sein
        Serial.println("Fall 2");
        if (taste)

und bleibt da hängen?

würde do while() gehen?

... und wenn du schon mit millis() arbeitest (sehr schön), solltest du dir auch while - Schleifen abgewöhnen, damit es einfacher wird.

generell keine while schleifen? warum das?
apropos millis(): laufe ich in Probleme nach 50 tagen oder gehts einfach von vorne los?
ist unsigned long lang genug?