Code funktioniert nur manchmal

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 :confused: :confused:

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:

#include 

#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
#include 

#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

Hallo,

du liest und vergleichst einzelne Ziffern und nicht die Zahlenfolge.

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.

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

    default : code[counter]=key; // just add the keystroke to the code
      if (counter

Sonst kannst du die letzte Ziffer immer wieder überschreiben

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

Sonst kannst du die letzte Ziffer immer wieder überschreiben

das versteh ich nicht ganz.so:?

default : 
if(counter

wo genau ist der unterschied?

someuser:
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.

Hallo,

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

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

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.

habs jetzt soweit am laufen, tnx.

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

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?

#include 

#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;
  }
}

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:

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

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.

Hi

Wieder was gelernt - goto gibt's hier auch noch :o

MfG

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

:D 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?

someuser: 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?

Du hast doch schon eine immer wiederkehrende Schleife: loop

Zum millis-Überlauf wurden hier im Forum schon mehrere Diskussionen bis zum endgültigen Beweis geführt. Nutze einfach die Suchfunktion.

Gruß Tommy

someuser: 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.

Du willst einen Zustandsautomaten

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

Das ist du falsch verstanden. 'A' || 'B' ist true

würde do while() gehen?

Denke anders! Komme davon ab irgendwo stehen bleiben zu wollen. Deine Schleife ist loop(). Dann machst du pro Durchlauf nur das was zu tun ist. Oder nichts.

Hi

Mit millis() prüfst Du, ob genug Zeit vergangen ist, um den nächsten Schritt zu machen. Natürlich kannst Du weiterhin while-Schleifen benutzen, nur nicht für's Warten! Wenn Du millis wie folgt benutzt: millis()-letzte_Startzeit>=Wartezeit Passiert auch in 47,batsch Tagen Nichts Außergewöhnliches! Die Werte sollten alle unsigned (vorzeichenlos) long (32bit) sein, damit der Compiler nicht meckert. Wenn Dir eine Wartezeit von 255ms reicht, könnte Wartezeit (bzw. klein geschrieben, da man Variablennamen klein beginnt, KONSTANTEN komplett groß und noch ein/zwei Besonderheiten, auf Die man sich Mal geeinigt hat, damit man den Code besser lesen kann) auch den Typ byte bekommen (ist 'von Haus aus' vorzeichenlos) ist 8bit breit und kann Werte von 0...255 annehmen. Hierbei könnte aber der Compiler meckern, da die Differenz zweier 32-bit-Zahlen mit einer 8-bit-Zahl verglichen werden soll.

Mit Deinem 'A' || 'B' bekommst Du IMMER true, da beide Werte != Null sind und true oder true ist true.

'A' müsste 65 sein (@ ist 64 ;) ), Du kannst also 'A'-'E' mit 65...69 gleich setzen. Bei den Zahlen, '0' ist 0x30 (hexadezimal, die letzte Ziffer entspricht der Wertigkeit unseres Zahlensystem, nebenbei) oder 48 in Dezimal. '0'...'9' wäre dann 48...57.

Oder Du benutzt switch case:

switch (variable){
  case 'A':
  case 'B':
  ...
  case '7':
    //deinen Code, Der ausgeführt werden soll, wenn die Taste A, B, ..., oder 7 war.
}

WICHTIG hierbei: nach dem Einsprung (dem passendem case) werden ALLE folgenden Befehle abgearbeitet, bis break; gefunden wird, oder die switch-Abfrage zu ende ist. Wenn also bei 'A' was Anderes als bei 'B' gemacht werden soll, muß am Ende vom 'A'-Bereich ein break; stehen.

MfG

Am kürzesten geht es so:

if ((taste >= 'A' && taste <= 'D') || taste == '*' || taste == '#' || (taste >= '0' && taste <= '9') )

Oder so:

switch (taste)
{
   case 'A' ... 'D':
   case '0' ... '9':
   case '*':
   case '#':
}

Aber wichtiger ist erst mal den grundlegenden Ablauf des Programms zu ändern hin zu einem Zustandsautomaten. Komplett anders muss es nicht sein. Ein switch/case ist ja da. Das muss nur richtig verwendet werden. Also erst mal auf den aktuellen Zustand abfragen. Und dann - wenn nötig - auf einen Tastendruck.

puh das muss ich mir morgen mal alles live anschauen.

würde denn:

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

und was macht

if (taste)

? auf den ersten blick funktioniert es so wie ich denke und schaltet im richtigen case weiter soblad irgendeine taste gedrückt wird.