Go Down

Topic: Code funktioniert nur manchmal (Read 4873 times) previous topic - next topic

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  :smiley-confuse:  :smiley-confuse:

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.
Tschau
Doc Arduino '\0'

Messschieber auslesen: http://forum.arduino.cc/index.php?topic=273445
EA-DOGM Display - Demos: http://forum.arduino.cc/index.php?topic=378279

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

#3
Mar 06, 2018, 08:59 pm Last Edit: Mar 06, 2018, 08:59 pm by 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.


Quote
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

Quote
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.

Quote
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?

Quote
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()

Quote
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.
Tschau
Doc Arduino '\0'

Messschieber auslesen: http://forum.arduino.cc/index.php?topic=273445
EA-DOGM Display - Demos: http://forum.arduino.cc/index.php?topic=378279

someuser

Quote
Bei deiner Version wird eine Ziffer am Ende des Array eingetragen
eigentlich hab ich den hier irgendwo geklaut ::)


Quote
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.
Tschau
Doc Arduino '\0'

Messschieber auslesen: http://forum.arduino.cc/index.php?topic=273445
EA-DOGM Display - Demos: http://forum.arduino.cc/index.php?topic=378279

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

Quote
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 :o

MfG
Dein Problem, Dein Sketch, Deine Bilder.
Ob ich ohne Diese an Deinem Problem arbeiten will, entscheide aber immer noch ich.
Große Buchstaben? Immer wieder, neben Punkt und Komma, gerne gesehen.

someuser

Quote
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.

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


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

Quote
... 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?

Go Up