[ARDUINO]Kod się zacina

Question

Napisałem kod do mojego projektu arduino ale kod w pewnym miejscu się zacina

Kod:

#include <Wire.h>
#include <LiquidCrystal_I2C.h>
#include <PCF8574.h>
 
PCF8574 Expander1;
PCF8574 Expander2;
 
int PktP[2];
 
int P1B[8] = {0, 1, 2, 3, 4, 5, 6, 7}, P2B[8] = {0, 1, 2, 3, 4, 5, 6, 7};
 
#define BACKLIGHT_PIN 3
LiquidCrystal_I2C   lcd(0x27,2,1,0,4,5,6,7);
 
int StanP1[8], StanP2[8];
 
int intCzasDoStartu = 5;
 
int PlayerWin = 0;
 
int Stan;
 
int intRandom;
 
int Sprawdz(){
  switch(intRandom){
    case 0: if(StanP1[0] == LOW)PlayerWin = 1; if(StanP2[0] == LOW)PlayerWin = 2; break;
    case 1: if(StanP1[1] == LOW)PlayerWin = 1; if(StanP2[1] == LOW)PlayerWin = 2; break;
    case 2: if(StanP1[2] == LOW)PlayerWin = 1; if(StanP2[2] == LOW)PlayerWin = 2; break;
    case 3: if(StanP1[3] == LOW)PlayerWin = 1; if(StanP2[3] == LOW)PlayerWin = 2; break;
    case 4: if(StanP1[4] == LOW)PlayerWin = 1; if(StanP2[4] == LOW)PlayerWin = 2; break;
    case 5: if(StanP1[5] == LOW)PlayerWin = 1; if(StanP2[5] == LOW)PlayerWin = 2; break;
    case 6: if(StanP1[6] == LOW)PlayerWin = 1; if(StanP2[6] == LOW)PlayerWin = 2; break;
    case 7: if(StanP1[7] == LOW)PlayerWin = 1; if(StanP2[7] == LOW)PlayerWin = 2; break;
  }
  return PlayerWin;
}
 
void setup() {
  Expander1.begin(0x20);
  Expander2.begin(0x24);
  for(int i = 0; i < 8; i ++){Expander1.pinMode(P1B[i], INPUT_PULLUP);}
  for(int i = 0; i < 8; i ++){Expander2.pinMode(P2B[i], INPUT_PULLUP);}
  lcd.begin (16,2);
  lcd.setBacklightPin(BACKLIGHT_PIN,POSITIVE);
  lcd.setBacklight(HIGH);
  lcd.print("Witaj w gierce!");
  lcd.setCursor(0,1);
  for(int i = intCzasDoStartu; i > 0; i--){
    lcd.setCursor(11,1);
    lcd.print(" ");
    lcd.setCursor(0,1);
    lcd.print("Start za ");
    lcd.print(i);
    lcd.print("s");
    delay(1000);
  }
  randomSeed(analogRead(0));
}
 
void loop() {
  intRandom = random(8);
  lcd.clear();
  lcd.print("Nacisnij guzik:");
  lcd.setCursor(0,1);
  lcd.print("P1 ");
  lcd.print(PktP[0]);
  lcd.setCursor(7,1);
  lcd.print(intRandom + 1);
  lcd.setCursor(11,1);
  lcd.print("P2 ");
  lcd.print(PktP[1]);
  Stan = 0;
  while(Stan == 0){
    for(int i = 0; i < 8; i++){StanP1[i] = Expander1.digitalRead(P1B[i]);}
    for(int i = 0; i < 8; i++){StanP2[i] = Expander2.digitalRead(P2B[i]);}
#################################################Tu sie zacina###################
    if(Sprawdz() == 1){
      PktP[0]++;
      Stan = 1;
      lcd.clear();
      lcd.print("Punkt dla gracza");
      lcd.setCursor(8,1);
      lcd.print("1");
      delay(1500);
    }else if(Sprawdz() == 2){
      PktP[1]++;
      Stan = 1;
      lcd.clear();
      lcd.print("Punkt dla gracza");
      lcd.setCursor(8,1);
      lcd.print("2");
      lcd.print(PktP[1]);
      delay(1500);
    }
###########################################################################################
  }
}

cały czas dolicza punkty graczowi który jako pierwszy nacisnął guzik

W razie potrzeby mogę zrobić schemat połączeń.

Czemu się tak zacina?

Comments

Strasznie nieczytelny kod, musisz płacić za każdą nową linię którą użyjesz że ten kod jest tak bardzo ściśnięty?

 case 0: if(StanP1[0] == LOW)PlayerWin = 1; if(StanP2[0] == LOW)PlayerWin = 2; break;

Bardzo nieprzyjemnie się to analizuje, zgaduję że błąd leży w pętli while w loop()

---

A po co tu panu pętla w pętli?

 for(int i = 0; i < 8; i++){StanP1[i] = Expander1.digitalRead(P1B[i]);}
 for(int i = 0; i < 8; i++){StanP2[i] = Expander2.digitalRead(P2B[i]);}

jeśli pierwsza pętla wykona się 8 razy, i jednocześnie jeśli druga wykona się 8 razy uzyskamy 64 iteracje. O 48 za dużo. Jak widzę tu jest delay 1500, co oznacza, że tracisz 48*1,5s, czyli 72 sekundy. Wyjmij jedną pętlę z drugiej. A poza tym ten kod wygląda jakby miał właśnie dawać punkty dla tego który pierwszy wciśnie guzik.

Po 1: co to za guziki? Ile ich jest, co trzeba zrobić żeby dostać punkt?

Po 2: co mam rozumieć poprzez zacinanie się? Program nie pracuje dalej (być może to wina tych nadmiarowych 72 sekund)? A może coś innego się dzieje? Napisz jak najwięcej!

---

@CzikaCarry Gdzie ty tu widzisz zagnieżdżone pętle?

---

Aaa nie zauważyłem tych klamr zamykających. Mój błąd xD

---

Taa, wychodzi "dobra" jakość kodu

Answer 1

Zwracanie z funkcji wartości zmiennej globalnej jest... Dziwne.

Powinieneś resetować zmienną Stan, czyli przypisywać jej wartość 0, nie 1. Tak mi się zdaje przynajmniej. Nope, źle mi się zdaje. Nie zmienną Stan, tylko zmienną PlayerWin.

Nieładny jest ten kod, ciężko się go analizuje.

Answer 2

Już bez patrzenia na kod może masz coś nie tak z przyciskami czy są poprawnie podciągnięte pod mase?

Comments

Podłączenie jest OK! Bo testowałem

Answer 3

Problem rozwiązany! 

Co sądzicie o moim nowym kodzie?

KOD:

#include <Wire.h>
#include <LiquidCrystal_I2C.h>
#include <PCF8574.h>

PCF8574 Expander1;
PCF8574 Expander2;

#define BACKLIGHT_PIN 3
LiquidCrystal_I2C	lcd(0x27,2,1,0,4,5,6,7);

int StanP1[8], StanP2[8];
int intCzasDoStartu = 5;
int PlayerWin = 0;
int intRandom;
int intSprawdz;
int PktP[2];
int P1B[8] = {0, 1, 2, 3, 4, 5, 6, 7}, 
	P2B[8] = {0, 1, 2, 3, 4, 5, 6, 7};
int const LpScens = 2;
String strScena[LpScens] = {
	"Nacisnij guzik:",
	"Punkt dla gracza"
};

int Losowanie(){
	int randome = random(8);
	lcd.clear();
	lcd.print("Losowanie.");
	delay(100);
	lcd.print(".");
	delay(200);
	lcd.print(".");
	delay(300);
	lcd.print(".");
	delay(440);
	lcd.print(".");
	delay(550);
	lcd.print(".");
	delay(2000);
	return randome;
}

void LcdPrint(int NoScene, int Argum){
	lcd.clear();
	switch(NoScene){
		case 0:
			lcd.print(strScena[0]);
			lcd.setCursor(0, 1);
			lcd.print("P1:");
			lcd.print(PktP[0]);
			lcd.setCursor(7, 1);
			lcd.print(Argum);
			lcd.setCursor(11, 1);
			lcd.print("P2:");
			lcd.print(PktP[1]);
		  break;
		case 1:
			lcd.print(strScena[1]);
			lcd.setCursor(0,1);
			lcd.print("P1:");
			lcd.print(PktP[0]);
			lcd.setCursor(7, 1);
			lcd.print(Argum + 1);
			lcd.setCursor(11, 1);
			lcd.print("P2:");
			lcd.print(PktP[1]);
			delay(1500);
		  break;
	}
}

int Sprawdz(){
  switch(intRandom){
    case 0:
		if(StanP1[0] == LOW)PlayerWin = 1; 
		if(StanP2[0] == LOW)PlayerWin = 2; 
	  break;
    case 1:
		if(StanP1[1] == LOW)PlayerWin = 1;
		if(StanP2[1] == LOW)PlayerWin = 2;
	  break;
    case 2:
		if(StanP1[2] == LOW)PlayerWin = 1;
		if(StanP2[2] == LOW)PlayerWin = 2;
	  break;
    case 3:
		if(StanP1[3] == LOW)PlayerWin = 1;
		if(StanP2[3] == LOW)PlayerWin = 2;
	  break;
    case 4:
		if(StanP1[4] == LOW)PlayerWin = 1;
		if(StanP2[4] == LOW)PlayerWin = 2;
	  break;
    case 5:
		if(StanP1[5] == LOW)PlayerWin = 1;
		if(StanP2[5] == LOW)PlayerWin = 2;
	  break;
    case 6:
		if(StanP1[6] == LOW)PlayerWin = 1;
		if(StanP2[6] == LOW)PlayerWin = 2;
	  break;
    case 7:
		if(StanP1[7] == LOW)PlayerWin = 1;
		if(StanP2[7] == LOW)PlayerWin = 2;
	  break;
  }
  return PlayerWin;
}

void setup() {
  Expander1.begin(0x20);
  Expander2.begin(0x24);
  for(int i = 0; i < 8; i ++){
	  Expander1.pinMode(P1B[i], INPUT_PULLUP);
  }
  for(int i = 0; i < 8; i ++){
	  Expander2.pinMode(P2B[i], INPUT_PULLUP);
  }
  lcd.begin (16,2);
  lcd.setBacklightPin(BACKLIGHT_PIN,POSITIVE);
  lcd.setBacklight(HIGH);
  lcd.print("Witaj w gierce!");
  lcd.setCursor(0,1);
  for(int i = intCzasDoStartu; i > 0; i--){
    lcd.setCursor(11,1);
    lcd.print(" ");
    lcd.setCursor(0,1);
    lcd.print("Start za ");
    lcd.print(i);
    lcd.print("s");
    delay(1000);
  }
  randomSeed(analogRead(0));
}

void loop() {
	PlayerWin = 0;
	intRandom = Losowanie();
	LcdPrint(0, intRandom + 1);
	while(PlayerWin == 0){
		for(int i = 0; i < 8; i++){StanP1[i] = Expander1.digitalRead(P1B[i]);}
		for(int i = 0; i < 8; i++){StanP2[i] = Expander2.digitalRead(P2B[i]);}
		intSprawdz = Sprawdz();
		if(intSprawdz == 1){
			PlayerWin = 1;
			PktP[0]++;
			LcdPrint(1, 0);
		}else if(intSprawdz == 2){
			PlayerWin = 2;
			PktP[1]++;
			LcdPrint(1, 1);
		}
	}
}

 

Comments

Nie mam teraz czasu, żeby analizować wszystko (zastanawiam się, co ja w ogóle tu robię...? ), ale jedno aż rzuca się w oczy (już pomijając fakt zwracania przez funkcję globali i używania zmiennej globalnej intRandom w roli argumentu oraz w ogóle dużej ilości zmiennych globalnych).

Czy to:

int Sprawdz(){
  switch(intRandom){
    case 0:
        if(StanP1[0] == LOW)PlayerWin = 1; 
        if(StanP2[0] == LOW)PlayerWin = 2; 
      break;
    case 1:
        if(StanP1[1] == LOW)PlayerWin = 1;
        if(StanP2[1] == LOW)PlayerWin = 2;
      break;
    case 2:
        if(StanP1[2] == LOW)PlayerWin = 1;
        if(StanP2[2] == LOW)PlayerWin = 2;
      break;
    case 3:
        if(StanP1[3] == LOW)PlayerWin = 1;
        if(StanP2[3] == LOW)PlayerWin = 2;
      break;
    case 4:
        if(StanP1[4] == LOW)PlayerWin = 1;
        if(StanP2[4] == LOW)PlayerWin = 2;
      break;
    case 5:
        if(StanP1[5] == LOW)PlayerWin = 1;
        if(StanP2[5] == LOW)PlayerWin = 2;
      break;
    case 6:
        if(StanP1[6] == LOW)PlayerWin = 1;
        if(StanP2[6] == LOW)PlayerWin = 2;
      break;
    case 7:
        if(StanP1[7] == LOW)PlayerWin = 1;
        if(StanP2[7] == LOW)PlayerWin = 2;
      break;
  }
  return PlayerWin;
}

nie może być zastąpione tym:

int Sprawdz(){
    if(StanP1[intRandom] == LOW) PlayerWin = 1; 
    if(StanP2[intRandom] == LOW) PlayerWin = 2; 
    return PlayerWin;
}

?

---

Poprawione wg. twoich sugestii! :D

Wielkie dzięki za pomoc!

---

Widać poprawę jakości kodu, funkcja loop już jest dużo czytelniejsza. Idziesz w dobrym kierunku.

Jednak dużo jeszcze można zrobić, jeśli chodzi o jakość.

Przede wszystkim nazewnictwo. Przyjęła się konwencja, która mówi że nazwy typów rozpoczynamy wielką literą, nazwy zmiennych i funkcji - małą. Nie mieszamy języka angielskiego i polskiego - kod powinien być jednolity, najlepiej po angielsku. Ograniczamy użycie zmiennych globalnych. W Arduino trochę to utrudnili, bo nie mamy pełnej swobody w konstrukcji programu, ale i globale da się co najmniej ograniczyć.

Trochę posiedziałem nad Twoim kodem i postarałem się go zrobić bardziej czytelnym.
Nie wiem, czy w tej chwili on działa poprawnie, bo nie mam warunków żeby go skompilować, już nie mówiąc o przetestowaniu. W każdym razie powinien raczej działać identycznie, jeśli nie - wymagane byłyby kosmetyczne poprawki.

#include <Wire.h>
#include <LiquidCrystal_I2C.h>
#include <PCF8574.h>

enum PlayerNumber
{
    /*  Jawnie określiłem wartości, bo będą wskazywać indeksy w tablicy
        points[].
        points[PlayerFirst] == points[0], points[PlayerSecond] == points[1].
    */
    PlayerFirst = 0,
    PlayerSecond = 1,
    PlayerNone,
};

// 4 zmienne globalne
int points[2];
PCF8574 expander1;
PCF8574 expander2;
LiquidCrystal_I2C lcd(0x27,2,1,0,4,5,6,7);

// 2 stałe w zasięgu globalnym
/*  Bez sensu jest używanie stałej, określającej rozmiar tablicy w momencie,
    kiedy nigdzie poza definicją tablicy jej nie używasz. */
const String textScene[] =
{
    "Nacisnij guzik:",
    "Punkt dla gracza"
};
const int BACKLIGHT_PIN = 3;

// deklaracje funkcji pomocniczych
int doLottery();
PlayerNumber checkWinner(int randomNumber);

/*  Nie mogłem znaleźć lepszej nazwy dla drugiego argumentu (int argum),
    bo nie wiem co on oznacza. Postaraj się tak go zmienić, żeby było
    wiadomo na pierwszy rzut oka, do czego on służy.
*/
void printScene(int which, int argum);
void sayHello(int delayStart = 5);

void setupExpanders();
void setupLcd();
void setupRandom();

// program właściwy
void setup()
{
    setupExpanders();
    setupLcd();
    sayHello();
    setupRandom();
}

void loop()
{
    int randomNumber = doLottery();
    printScene(0, randomNumber + 1);
    PlayerNumber winner;
    do
    {
        winner = checkWinner(randomNumber);
        if(winner == PlayerFirst)
        {
            points[PlayerFirst]++;
            printScene(1, 0);
        }
        else if(winner == PlayerSecond)
        {
            points[PlayerSecond]++;
            printScene(1, 1);
        }
    }
    while(winner == PlayerNone);
}

// definicje funkcji pomocniczych
int doLottery()
{
    int randomNumber = random(8);
    lcd.clear();
    lcd.print("Losowanie.");
    delay(100);
    lcd.print(".");
    delay(200);
    lcd.print(".");
    delay(300);
    lcd.print(".");
    delay(440);
    lcd.print(".");
    delay(550);
    lcd.print(".");
    delay(2000);
    return randomNumber;
}

PlayerNumber checkWinner(int randomNumber)
{
    /*  Niepotrzebnie wprowadzałeś dodatkową tablicę do przechowywania stanów,
        można stan przycisków odczytywać bezpośrednio.
        Chyba że taki zabieg wprowadzał pożądane opóźnienie, wtedy powinna
        zostać wersja z dodatkową tablicą.  */

    /*  Pozbyłem się globalnej zmiennej PlayerWin, była ona niebezpieczna.
        Każda z funkcji mogła ją bezkarnie modyfikować, a była ona
        kluczowa do poprawnego wykonania programu.
    */

    /*  Tablice P1B i P2B były niepotrzebne.
        Zauważ, że P1B[0] == 0, P1B[1] == 1, P1B[2] == 2, itd...
    */
    PlayerNumber playerWin = PlayerNone; /* W razie gryby poniższe warunki
                                            nie były spełnione, playerWin
                                            będzie nadal mieć wartość PlayerNone
                                         */
    if(expander1.digitalRead(randomNumber) == LOW) playerWin = PlayerFirst;
    if(expander2.digitalRead(randomNumber) == LOW) playerWin = PlayerSecond;
    return playerWin;
}

void printScene(int which, int argum)
{
    lcd.clear();
    lcd.print(textScene[which]);
    lcd.setCursor(0, 1);
    lcd.print("P1:");
    lcd.print(points[0]);
    lcd.setCursor(7, 1);

    /*  Poniższego if-else można zastąpić linią:
        lcd.print(argum + which);
        ale jest to mniej czytelne.
    */
    if(which == 0)
    {
        lcd.print(argum);
    }
    else if(which == 1)
    {
        lcd.print(argum + 1);
    }
    lcd.setCursor(11, 1);
    lcd.print("P2:");
    lcd.print(points[1]);
    if(which == 1) delay(1500);
}

void sayHello(int delayStart)
{
    lcd.print("Witaj w gierce!");
    lcd.setCursor(0,1);

    for(int i = delayStart; i > 0; i--)
    {
        lcd.setCursor(11,1);
        lcd.print(" ");
        lcd.setCursor(0,1);
        lcd.print("Start za ");
        lcd.print(i);
        lcd.print("s");
        delay(1000);
    }
}

void setupExpanders()
{
    expander1.begin(0x20);
    expander2.begin(0x24);

    /*  Dwie pętle wykonujące to samo zadanie nie były potrzebne.
    */

    /*  Tablice P1B i P2B były niepotrzebne.
        Zauważ, że P1B[0] == 0, P1B[1] == 1, P1B[2] == 2, itd...
    */
    for(int i = 0; i < 8; i ++)
    {
        expander1.pinMode(i, INPUT_PULLUP);
        expander2.pinMode(i, INPUT_PULLUP);
    }
}

void setupLcd()
{
    lcd.begin(16,2);
    lcd.setBacklightPin(BACKLIGHT_PIN, POSITIVE);
    lcd.setBacklight(HIGH);
}

void setupRandom()
{
    randomSeed(analogRead(0));
}

 

---

Kod działa poprawnie i na pewno wiele się nauczyłem się z niego! :D

Wielkie dzięki!!!

Później jeszcze podeśle wam schemat.

---

Na zdrowie. :)

---

Oddaje w wasze ręce schemat!

Plik projektu FRITZING:

https://www.dropbox.com/s/gkajhpdgzn51j8n/My%20Game.fzz?dl=0

Zdjęcie:

http://postimg.org/image/8wv4u288x/

---

Pokaż jeszcze jakiś filmik z działania gotowego układu. :)

---

Ok Później dodam! :D

---

Link do filmiku:

https://youtu.be/dIaMvB-Scd4