Arduino Münzprüfer Fehler bei mehreren Eingängen

[Marian G. (houwdy)]

Hallo,
ich habe einen simplen Münzprüfer gebaut welcher nach Münzeinwurf und 
betätigen eines Tasters die darunterliegende Steckdose für 3 Stunden mit 
Strom versorgt. Bei Einwurf einer Münze welche über eine Rampe mit 
Öffnungen rollt wird wenn sie die mechanische Prüfung besteht und nicht 
durch die Öffnungen fällt ein Taster (Pin0) betätigt. Anschliesend soll 
einer der 16 Taster (Pin1-16) für die jeweilige Schuko gedrückt werden, 
dadurch wird der jeweilige Ausgang( Pin 18-33) für eine einstellbare 
Zeit auf High geschaltet.

Soweit die Theorie. Das ganze funktioniert mit zwei IOs auch ganz 
normal. leider mit 16 nicht mehr und ich schau schon seit 2 Stunden 
darauf und find den Fehler nicht...

kann von euch bitte mal einer darüberschauen und mir meinen Fehler 
sagen?
Die Hardware ist ein Arduino Mega2560

Ich bedanke mich schon mal herzlich.

hier der Code:

const int coinPin = 0;

const int buttonPin01 = 1;
const int buttonPin02 = 2;
const int buttonPin03 = 3;
const int buttonPin04 = 4;
const int buttonPin05 = 5;
const int buttonPin06 = 6;
const int buttonPin07 = 7;
const int buttonPin08 = 8;
const int buttonPin09 = 9;
const int buttonPin10 = 10;
const int buttonPin11 = 11;
const int buttonPin12 = 12;
const int buttonPin13 = 13;
const int buttonPin14 = 14;
const int buttonPin15 = 15;
const int buttonPin16 = 16;

const int ledPin1 =  18;
const int ledPin2 =  19;
const int ledPin3 =  20;
const int ledPin4 =  21;
const int ledPin5 =  22;
const int ledPin6 =  23;
const int ledPin7 =  24;
const int ledPin8 =  25;
const int ledPin9 =  26;
const int ledPin10 =  27;
const int ledPin11 =  28;
const int ledPin12 =  29;
const int ledPin13 =  30;
const int ledPin14 =  31;
const int ledPin15 =  32;
const int ledPin16 =  33;

int buttonState01 = 0;
int buttonState02 = 0;
int buttonState03 = 0;
int buttonState04 = 0;
int buttonState05 = 0;
int buttonState06 = 0;
int buttonState07 = 0;
int buttonState08 = 0;
int buttonState09 = 0;
int buttonState10 = 0;
int buttonState11 = 0;
int buttonState12 = 0;
int buttonState13 = 0;
int buttonState14 = 0;
int buttonState15 = 0;
int buttonState16 = 0;

int buttonVal01 =0;
int buttonVal02 =0;
int buttonVal03 =0;
int buttonVal04 =0;
int buttonVal05 =0;
int buttonVal06 =0;
int buttonVal07 =0;
int buttonVal08 =0;
int buttonVal09 =0;
int buttonVal10 =0;
int buttonVal11 =0;
int buttonVal12 =0;
int buttonVal13 =0;
int buttonVal14 =0;
int buttonVal15 =0;
int buttonVal16 =0;

int coinState =0;
int Coin =0;
int timeVal = 50;

void setup() {

pinMode(ledPin1, OUTPUT);
pinMode(ledPin2, OUTPUT);
pinMode(ledPin3, OUTPUT);
pinMode(ledPin4, OUTPUT);
pinMode(ledPin5, OUTPUT);
pinMode(ledPin6, OUTPUT);
pinMode(ledPin7, OUTPUT);
pinMode(ledPin8, OUTPUT);
pinMode(ledPin9, OUTPUT);
pinMode(ledPin10, OUTPUT);
pinMode(ledPin11, OUTPUT);
pinMode(ledPin12, OUTPUT);
pinMode(ledPin13, OUTPUT);
pinMode(ledPin14, OUTPUT);
pinMode(ledPin15, OUTPUT);
pinMode(ledPin16, OUTPUT);

pinMode(buttonPin01, INPUT);
pinMode(buttonPin02, INPUT);
pinMode(buttonPin03, INPUT);
pinMode(buttonPin04, INPUT);
pinMode(buttonPin05, INPUT);
pinMode(buttonPin06, INPUT);
pinMode(buttonPin07, INPUT);
pinMode(buttonPin08, INPUT);
pinMode(buttonPin09, INPUT);
pinMode(buttonPin10, INPUT);
pinMode(buttonPin11, INPUT);
pinMode(buttonPin12, INPUT);
pinMode(buttonPin13, INPUT);
pinMode(buttonPin14, INPUT);
pinMode(buttonPin15, INPUT);
pinMode(buttonPin16, INPUT);

pinMode(coinPin, INPUT);

}
void loop(){
coinState = digitalRead(coinPin);

if (coinState == LOW) {
  Coin = 1;
}
if (digitalRead(buttonPin01) == LOW&& Coin ==1) {
  buttonVal01 = timeVal;
  Coin =0;
}
else if (digitalRead(buttonPin02) == LOW&& Coin ==1) {
  buttonVal02 = timeVal;
  Coin =0;
}

else if (digitalRead(buttonPin03) == LOW&& Coin ==1) {
  buttonVal03 = timeVal;
  Coin =0;
}
else if (digitalRead(buttonPin04) == LOW&& Coin ==1) {
  buttonVal04 = timeVal;
  Coin =0;
}
else if (digitalRead(buttonPin05) == LOW&& Coin ==1) {
  buttonVal05 = timeVal;
  Coin =0;
}
else if (digitalRead(buttonPin06) == LOW&& Coin ==1) {
  buttonVal06 = timeVal;
  Coin =0;
}
else if (digitalRead(buttonPin07) == LOW&& Coin ==1) {
  buttonVal07 = timeVal;
  Coin =0;
}
else if (digitalRead(buttonPin08) == LOW&& Coin ==1) {
  buttonVal08 = timeVal;
  Coin =0;
}
else if (digitalRead(buttonPin09) == LOW&& Coin ==1) {
  buttonVal09 = timeVal;
  Coin =0;
}
else if (digitalRead(buttonPin10) == LOW&& Coin ==1) {
  buttonVal10 = timeVal;
  Coin =0;
}
else if (digitalRead(buttonPin11) == LOW&& Coin ==1) {
  buttonVal11 = timeVal;
  Coin =0;
}
else if (digitalRead(buttonPin12) == LOW&& Coin ==1) {
  buttonVal12 = timeVal;
  Coin =0;
}
else if (digitalRead(buttonPin13) == LOW&& Coin ==1) {
  buttonVal13 = timeVal;
  Coin =0;
}
else if (digitalRead(buttonPin14) == LOW&& Coin ==1) {
  buttonVal14 = timeVal;
  Coin =0;
}
else if (digitalRead(buttonPin15) == LOW&& Coin ==1) {
  buttonVal15 = timeVal;
  Coin =0;
}
else if (digitalRead(buttonPin16) == LOW&& Coin ==1) {
  buttonVal16 = timeVal;
  Coin =0;
}

if (buttonVal01 <<0 ) {
  digitalWrite(ledPin1, HIGH);
  buttonVal01=buttonVal01-1;
}
if (buttonVal01 ==0 ) {
  digitalWrite(ledPin1, LOW);
}
if (buttonVal02 <<0 ) {
  digitalWrite(ledPin2, HIGH);
  buttonVal02=buttonVal02-1;
}
if (buttonVal02 ==0 ) {
  digitalWrite(ledPin2, LOW);
}

if (buttonVal03 <<0 ) {
  digitalWrite(ledPin3, HIGH);
  buttonVal03=buttonVal03-1;
}
if (buttonVal03 ==0 ) {
  digitalWrite(ledPin3, LOW);
}

if (buttonVal04 <<0 ) {
  digitalWrite(ledPin4, HIGH);
  buttonVal04=buttonVal04-1;
}
if (buttonVal04 ==0 ) {
  digitalWrite(ledPin4, LOW);
}

if (buttonVal05 <<0 ) {
  digitalWrite(ledPin5, HIGH);
  buttonVal05=buttonVal05-1;
}
if (buttonVal05 ==0 ) {
  digitalWrite(ledPin5, LOW);
}
if (buttonVal06 <<0 ) {
  digitalWrite(ledPin6, HIGH);
  buttonVal06=buttonVal06-1;
}
if (buttonVal06 ==0 ) {
  digitalWrite(ledPin6, LOW);
}
if (buttonVal07 <<0 ) {
  digitalWrite(ledPin7, HIGH);
  buttonVal07=buttonVal07-1;
}
if (buttonVal07 ==0 ) {
  digitalWrite(ledPin7, LOW);
}
if (buttonVal08 <<0 ) {
  digitalWrite(ledPin8, HIGH);
  buttonVal08=buttonVal08-1;
}
if (buttonVal08 ==0 ) {
  digitalWrite(ledPin8, LOW);
}
if (buttonVal09 <<0 ) {
  digitalWrite(ledPin9, HIGH);
  buttonVal09=buttonVal09-1;
}
if (buttonVal09 ==0 ) {
  digitalWrite(ledPin9, LOW);
}
if (buttonVal10 <<0 ) {
  digitalWrite(ledPin10, HIGH);
  buttonVal10=buttonVal10-1;
}
if (buttonVal10 ==0 ) {
  digitalWrite(ledPin10, LOW);
}
if (buttonVal11 <<0 ) {
  digitalWrite(ledPin11, HIGH);
  buttonVal11=buttonVal11-1;
}
if (buttonVal11 ==0 ) {
  digitalWrite(ledPin11, LOW);
}
if (buttonVal12 <<0 ) {
  digitalWrite(ledPin12, HIGH);
  buttonVal12=buttonVal12-1;
}
if (buttonVal12 ==0 ) {
  digitalWrite(ledPin12, LOW);
}
if (buttonVal13 <<0 ) {
  digitalWrite(ledPin13, HIGH);
  buttonVal13=buttonVal13-1;
}
if (buttonVal13 ==0 ) {
  digitalWrite(ledPin13, LOW);
}

if (buttonVal14 <<0 ) {
  digitalWrite(ledPin14, HIGH);
  buttonVal14=buttonVal14-1;
}
if (buttonVal14 ==0 ) {
  digitalWrite(ledPin14, LOW);
}

if (buttonVal15 <<0 ) {
  digitalWrite(ledPin15, HIGH);
  buttonVal15=buttonVal15-1;
}
if (buttonVal15 ==0 ) {
  digitalWrite(ledPin15, LOW);
}

if (buttonVal16 <<0 ) {
  digitalWrite(ledPin16, HIGH);
  buttonVal16=buttonVal16-1;
}
if (buttonVal16 ==0 ) {
  digitalWrite(ledPin16, LOW);
}
delay(100);
}

[Brain 2.0 (Gast)]

Was genau funktioniert nicht ?

Hast du für die Eingänge Pullup-Widerstände verbaut ?

[houwdy (Gast)]

Interessanterweise macht er gar nichts mehr mit drei oder mehr 
Eingängen, die Pegel sind definiert mit pullups.

[Dussel (Gast)]

Was soll
if (buttonVal01 <<0 ) {
bringen? Damit testest du, ob buttonValxx ungleich null ist. Willst du 
das? Warum dann so komisch geschrieben?

[Brain 2.0 (Gast)]

houwdy schrieb:
> Interessanterweise macht er gar nichts mehr mit drei oder mehr
> Eingängen, die Pegel sind definiert mit pullups.

Seltsam !!
Pullups Hardwaremäßig ?
Wie groß ?

[Kaj (Gast)]

Ich hab mal den Code aufgeraeumt. Sollte aber äqui­va­lent sein

#include <stdint.h>


const int16_t coinPin = 0;

const int16_t buttonPins[16] = {1, 2, 3, 4, 5, 6, 7, 8, 9,
                            10, 11, 12, 13, 14, 15, 16};

const int16_t ledPins[16] = {18, 19, 20, 21, 22, 23, 24, 25,
                         26, 27, 28, 29, 30, 31, 32, 33};

int16_t buttonState[16] = {0};

int16_t buttonVal[16] = {0};

int16_t coinState = 0;
int16_t Coin = 0;
int16_t timeVal = 50;


void setup()
{
    for (uint8_t i = 0; i < 16; i++) {
        pinMode(ledPins[i], OUTPUT);
        pinMode(buttonPins[i], INPUT);
    }

    pinMode(coinPin, INPUT);
}


void loop()
{
    coinState = digitalRead(coinPin);

    if (coinState == LOW) {
        Coin = 1;
    }


    for (uint8_t i = 0; i < 16; i++) {
        if ((digitalRead(buttonPins[i]) == LOW) && (Coin == 1)) {
            buttonVal[i] = timeVal;
            Coin = 0;

            break;
        }
    }


    for (uint8_t i = 0; i < 16; i++) {
        if (buttonVal[i] << 0) {
            digitalWrite(ledPins[i], HIGH);
            buttonVal[i] -= 1;
        }

        if (buttonVal[i] == 0) {
            digitalWrite(ledPins[i], LOW);
        }
    }

    delay(100);
}

Den fehler vom TO 'if (buttonValxx << 0)' hab ich jetzt einfach mal 
uebernommen. keine Ahnung was er da machen moechte.

[Einer (Gast)]

Das << sollte von der Logik wohl eher ein > sein.

[Eric B. (beric)]

Kaj schrieb:
>

>     for (uint8_t i = 0; i < 16; i++) {
>         if ((digitalRead(buttonPins[i]) == LOW) && (Coin == 1)) {
>             buttonVal[i] = timeVal;
>             Coin = 0;
> 
>             break;
>         }
>     }
>

Das kann noch vereinfacht werden:

    if (Coin == 1) {
        for (uint8_t i = 0; i < 16; i++) {
            if (digitalRead(buttonPins[i]) == LOW) {
                buttonVal[i] = timeVal;
                Coin = 0;

                break;
            }
        }
    }

[Marian G. (houwdy)]

Hallo,
sorry ich hab mich gestern glaub ich etwas undeutlich ausgedrückt.
Das lag warscheinlich an der späten Stunde.

Hier nocheinmal die Aufgabenstellung:
Wir bewirtschafen eine Schutzhütte und dort ist der Strom recht rar da 
er nur von einer Photovoltaikanlage zur Verfügung. Um dies kontrollieren 
zu können ( da einige Herrschaften von Fön bis Tauchsieder alles 
mitttragen und uns damit die Hütte finster machen) und auch die 
Betriebskosten zu senken kann man bei uns neben der Küchentüre mit einem 
Euro für 3h Strom kaufen. Diese Schukos sind auf 0,5A mittels 
elektronischer Sicherung abgesichert. Wir hatten bis jetzt 6 Schukos, 
allerdings ist der Bedarf gestiegen und somit habe ich das ganze auf 15 
erweitert. Meine erste Version baute ich in der HTL und sie arbeitet mit 
einem PIC. nun bin ich auf Arduino umgestiegen.

Die Handhabung durch den Kunden läuft wiefolgt:

Euro einwerfen,
Taste über Steckdose drücken,
dann ist für 3h an der jeweiligen Schuko Energie verfügbar.

Vom Code hab ich schon jetzt viel von euch gelernt.
bei uns hies es immer es ist ein Nogo bei der Initialisierung Schleifen 
zu
verwenden. Super wenn er das kann.

Das ganze funktioniert intressanterweise mit zwei Ein und Ausgängen ganz 
normal, so ich 3 nehme macht der Arduino gar nichts mehr.

ich mach noch schnell ein Foto von dem Vorgänger damit ihr euch da etwas 
vorstllen könnt.

Danke Marian

[Marian G. (houwdy)]

So sieht die Pic Version aus

[Dussel (Gast)]

Es ist ja schön, dass du so viel schreibst, aber die Frage bleibt, was 
du mit
if (buttonVal[i] << 0) {
bewirken willst.

[Leroy M. (mayl)]

Wie ist die Hardware denn ausgeführt/verschaltet?
Werden die LEDs über einen Treiber versorgt? Ich habe die Daten nicht im 
Kopf, aber ich kann mir vorstellen, 16 gleichzeitig leuchtende LEDs sind 
schon ziemlich viel Strom für den kleinen Arduino.


Zumn Code habe ich noch einen Vorschlag. Derzeit ist die Stromausgabe 
nicht verriegelt. Ich kann also mehrfach hintereinander die gleiche 
Steckdose "kaufen".

Für mich wäre es ziemlich ärgerlich, wenn ich mich vertippe, also eine 
bereits eingeschaltete Steckdose auswähle, und dadurch meinen Euro 
verschenke.
Davor solltest du mit der Programmierung vorgehen, heutzutage gibt es 
genug Idioten, für die das bereits aus, um zum Vandalen zu werden...

[Marian G. (houwdy)]

int timeVal = 50; // gibt mir die Zeit an die der Ausgang gesetzt werden 
soll

if (coinState == LOW) {
  Coin = 1;            // Wenn eine Münze eingeworfen wird wird Coin zu 1

if (digitalRead(buttonPin01) == LOW&& Coin ==1) { // wenn Coin 1 und der Schalter der jeweiligen schuko gedrückt wird wird in Button Val die Zeitkonstante geschrieben und coin auf 0 gesetzt
  buttonVal01 = timeVal;
  Coin =0;
}

//solange Buttonval größer 0 ist wird die Steckdose eingeschaltet 
//das habe ich so realisiert damit jeder der 16 Outputs seine "eigene Zeit" //hat 
//und immer die ganze schleife durchlaufen wird ohne das der controller in //einer hängen bleibt. buttonval-- wollte der Compiler komischerweise nicht.

if (buttonVal01 <<0 ) {
  digitalWrite(ledPin1, HIGH);
  buttonVal01=buttonVal01-1;
}

//hat die ButtonVal variable dann 0 erreicht wird der Output wider auf 0 gesetzt.
if (buttonVal01 ==0 ) {
  digitalWrite(ledPin1, LOW);
}
delay(100); // definiert einen Durchlauf auf ~100ms

Die Outputs gehen natürlich via Optokoppler auf eine Relaiskarte.

[Martin K. (maart)]

Ich habe jetzt keine Lust mich durch den Code zu wühlen.
Ich sitze jetzt vor meinem Mega und habe das Programm aus dem 
Eröffnungpost drauf.
Was muss ich jetzt wo anschließen um es mal zu testen.

[Marian G. (houwdy)]

an IO 0 kommt ein Taster für die Münzerkennung der kurz betätigt wird

an Io 1-16 die Tster für die Schukofreigabe.

an IO 18-33 die LED bzw Relais

Wenn du das ganze mit 3 Tastern und 3 Leds mit IO 1, 2,3 und IO 18, 19 
20 probierst und du kommentierst den Code von

else if (digitalRead(buttonPin03) == LOW&& Coin ==1) {
  buttonVal03 = timeVal;
  Coin =0;
}

bis

else if (digitalRead(buttonPin16) == LOW&& Coin ==1) {
  buttonVal03 = timeVal;
  Coin =0;
}

aus funktioniert das ganze einwandfrei io 0 betätigen, anschliesend io1 
oder/ und io2 und die Leds leuchten für die Zeit.

so wie du

else if (digitalRead(buttonPin03) == LOW&& Coin ==1) {
  buttonVal03 = timeVal;
  Coin =0;
}

dazu nimmst geht gar nichts mehr??

Danke schonmal

Marian :)

[Marian G. (houwdy)]

was ich oben noch überlesen habe die Pullups haben 100k ich  könnte aber 
auch  auf die Internen vom Arduino wechseln.

[Patrick B. (p51d)]

Dussel schrieb:
> Was soll
> if (buttonVal01 <<0 ) {
> bringen?

Einer schrieb:
> Das << sollte von der Logik wohl eher ein > sein.

Kaj schrieb:
> Den fehler vom TO 'if (buttonValxx << 0)' hab ich jetzt einfach mal
> uebernommen. keine Ahnung was er da machen moechte.

Dussel schrieb:
> Es ist ja schön, dass du so viel schreibst, aber die Frage bleibt, was
> du mit
> if (buttonVal[i] << 0) {
> bewirken willst.

Marian G. schrieb:
> //solange Buttonval größer 0 ist wird die Steckdose eingeschaltet
> //das habe ich so realisiert damit jeder der 16 Outputs seine "eigene
> Zeit" //hat
> //und immer die ganze schleife durchlaufen wird ohne das der controller
> in //einer hängen bleibt. buttonval-- wollte der Compiler komischerweise
> nicht.
>
> if (buttonVal01 <<0 ) {
>   digitalWrite(ledPin1, HIGH);
>   buttonVal01=buttonVal01-1;
> }

Ehm, ich will dir nicht zu nahe treten Marian, aber bist du Resistent 
gegen konkrete Verbesserungsvorschläge bei offensichtlichen Fehlern?
Kennst du den Unterschied zwischen

buttonVal01 << 0

und

buttonVal01 < 0

Das eine ist eine Schiebeoperation, und das andere ein Vergleich!

if (buttonVal01 <<0 ) {
  //
}

=> Entspricht
if ((buttonVal01 <<0 ) != 0) {
  //
}

Konkret schiebst du die Variable buttonVal01 also um 0 Bit nach links 
und prüfst anschliessend ob diese grösser als 0 ist.


Wenn du auf 0 oder grösser 0 prüfen möchtest, dann mach doch das so wie 
99.99% alle anderen:

if(buttonVal01 != 0){
}
if(buttonVal01 > 0){
}

Das kann man wenigstens lesen und versteht es auf anhieb.

Noch was. Du setzt deine Coin Variable nur in der Schlaufe zurück. Ist 
das so gewollt? Sollte das nicht etwa so sein?

if (coinState == LOW) {
  Coin = 1;
}
else{
  Coin = 0;
}

Versuch doch mit diesem Code mit 1, 2, oder 3 "akzeptieren" Kanälen zu 
Arbeiten (for-Schlaufen limitieren). In der Simulation oder beim 
Debuggen Schritt für Schritt. Dann siehste ja sofort, wo etwas wann 
nicht so soll wie gedacht.

[Martin K. (maart)]

Versuche erst mal das hier:

const int coinPin = 17; //war 0

const int buttonPin01 = 1;
const int buttonPin02 = 2;
const int buttonPin03 = 3;
const int buttonPin04 = 4;
const int buttonPin05 = 5;
const int buttonPin06 = 6;
const int buttonPin07 = 7;
const int buttonPin08 = 8;
const int buttonPin09 = 9;
const int buttonPin10 = 10;
const int buttonPin11 = 11;
const int buttonPin12 = 12;
const int buttonPin13 = 13;
const int buttonPin14 = 14;
const int buttonPin15 = 15;
const int buttonPin16 = 16;

const int ledPin1 =  18;
const int ledPin2 =  19;
const int ledPin3 =  20;
const int ledPin4 =  21;
const int ledPin5 =  22;
const int ledPin6 =  23;
const int ledPin7 =  24;
const int ledPin8 =  25;
const int ledPin9 =  26;
const int ledPin10 =  27;
const int ledPin11 =  28;
const int ledPin12 =  29;
const int ledPin13 =  30;
const int ledPin14 =  31;
const int ledPin15 =  32;
const int ledPin16 =  33;

int buttonState01 = 0;
int buttonState02 = 0;
int buttonState03 = 0;
int buttonState04 = 0;
int buttonState05 = 0;
int buttonState06 = 0;
int buttonState07 = 0;
int buttonState08 = 0;
int buttonState09 = 0;
int buttonState10 = 0;
int buttonState11 = 0;
int buttonState12 = 0;
int buttonState13 = 0;
int buttonState14 = 0;
int buttonState15 = 0;
int buttonState16 = 0;

int buttonVal01 =0;
int buttonVal02 =0;
int buttonVal03 =0;
int buttonVal04 =0;
int buttonVal05 =0;
int buttonVal06 =0;
int buttonVal07 =0;
int buttonVal08 =0;
int buttonVal09 =0;
int buttonVal10 =0;
int buttonVal11 =0;
int buttonVal12 =0;
int buttonVal13 =0;
int buttonVal14 =0;
int buttonVal15 =0;
int buttonVal16 =0;

int coinState =0;
int Coin =0;
int timeVal = 5;

void setup() {

pinMode(ledPin1, OUTPUT);
pinMode(ledPin2, OUTPUT);
pinMode(ledPin3, OUTPUT);
pinMode(ledPin4, OUTPUT);
pinMode(ledPin5, OUTPUT);
pinMode(ledPin6, OUTPUT);
pinMode(ledPin7, OUTPUT);
pinMode(ledPin8, OUTPUT);
pinMode(ledPin9, OUTPUT);
pinMode(ledPin10, OUTPUT);
pinMode(ledPin11, OUTPUT);
pinMode(ledPin12, OUTPUT);
pinMode(ledPin13, OUTPUT);
pinMode(ledPin14, OUTPUT);
pinMode(ledPin15, OUTPUT);
pinMode(ledPin16, OUTPUT);

pinMode(buttonPin01, INPUT_PULLUP);
pinMode(buttonPin02, INPUT_PULLUP);
pinMode(buttonPin03, INPUT_PULLUP);
pinMode(buttonPin04, INPUT_PULLUP);
pinMode(buttonPin05, INPUT_PULLUP);
pinMode(buttonPin06, INPUT_PULLUP);
pinMode(buttonPin07, INPUT_PULLUP);
pinMode(buttonPin08, INPUT_PULLUP);
pinMode(buttonPin09, INPUT_PULLUP);
pinMode(buttonPin10, INPUT_PULLUP);
pinMode(buttonPin11, INPUT_PULLUP);
pinMode(buttonPin12, INPUT_PULLUP);
pinMode(buttonPin13, INPUT_PULLUP);
pinMode(buttonPin14, INPUT_PULLUP);
pinMode(buttonPin15, INPUT_PULLUP);
pinMode(buttonPin16, INPUT_PULLUP);

pinMode(coinPin, INPUT_PULLUP);

Serial.begin(9600);
}
void loop(){
coinState = digitalRead(coinPin);

if (coinState == LOW) {
  Coin = 1;
  Serial.println("Coin aktiv");
}
if (digitalRead(buttonPin01) == LOW&& Coin ==1) {
  Serial.println("Taste 1");
  buttonVal01 = timeVal;
  Coin =0;
}
else if (digitalRead(buttonPin02) == LOW&& Coin ==1) {
  Serial.println("Taste 2");
  buttonVal02 = timeVal;
  Coin =0;
}

else if (digitalRead(buttonPin03) == LOW&& Coin ==1) {
  Serial.println("Taste 3");
  buttonVal03 = timeVal;
  Coin =0;
}
else if (digitalRead(buttonPin04) == LOW&& Coin ==1) {
  Serial.println("Taste 4");
  buttonVal04 = timeVal;
  Coin =0;
}
else if (digitalRead(buttonPin05) == LOW&& Coin ==1) {
  Serial.println("Taste 5");
  buttonVal05 = timeVal;
  Coin =0;
}
else if (digitalRead(buttonPin06) == LOW&& Coin ==1) {
  Serial.println("Taste 6");
  buttonVal06 = timeVal;
  Coin =0;
}
else if (digitalRead(buttonPin07) == LOW&& Coin ==1) {
  Serial.println("Taste 7");
  buttonVal07 = timeVal;
  Coin =0;
}
else if (digitalRead(buttonPin08) == LOW&& Coin ==1) {
  Serial.println("Taste 8");
  buttonVal08 = timeVal;
  Coin =0;
}
else if (digitalRead(buttonPin09) == LOW&& Coin ==1) {
  Serial.println("Taste 9");
  buttonVal09 = timeVal;
  Coin =0;
}
else if (digitalRead(buttonPin10) == LOW&& Coin ==1) {
  Serial.println("Taste 10");
  buttonVal10 = timeVal;
  Coin =0;
}
else if (digitalRead(buttonPin11) == LOW&& Coin ==1) {
  Serial.println("Taste 11");
  buttonVal11 = timeVal;
  Coin =0;
}
else if (digitalRead(buttonPin12) == LOW&& Coin ==1) {
  Serial.println("Taste 12");
  buttonVal12 = timeVal;
  Coin =0;
}
else if (digitalRead(buttonPin13) == LOW&& Coin ==1) {
  Serial.println("Taste 13");
  buttonVal13 = timeVal;
  Coin =0;
}
else if (digitalRead(buttonPin14) == LOW&& Coin ==1) {
  Serial.println("Taste 14");
  buttonVal14 = timeVal;
  Coin =0;
}
else if (digitalRead(buttonPin15) == LOW&& Coin ==1) {
  Serial.println("Taste 15");
  buttonVal15 = timeVal;
  Coin =0;
}
else if (digitalRead(buttonPin16) == LOW&& Coin ==1) {
  Serial.println("Taste 16");
  buttonVal16 = timeVal;
  Coin =0;
}

/*
if (buttonVal01 <<0 ) {
  Serial.println("Dose 1 HIGH");
  digitalWrite(ledPin1, HIGH);
  buttonVal01=buttonVal01-1;
}
if (buttonVal01 ==0 ) {
  Serial.println("Dose 1 LOW");
  digitalWrite(ledPin1, LOW);
}
if (buttonVal02 << 0 ) {
  Serial.println("Dose 2 HIGH");
  digitalWrite(ledPin2, HIGH);
  buttonVal02=buttonVal02-1;
}
if (buttonVal02 ==0 ) {
  Serial.println("Dose 2 LOW");
  digitalWrite(ledPin2, LOW);
}

if (buttonVal03 <<0 ) {
  Serial.println("Dose 3 HIGH");
  digitalWrite(ledPin3, HIGH);
  buttonVal03=buttonVal03-1;
}
if (buttonVal03 ==0 ) {
  Serial.println("Dose 3 LOW");
  digitalWrite(ledPin3, LOW);
}

if (buttonVal04 <<0 ) {
  Serial.println("Dose 4 HIGH");
  digitalWrite(ledPin4, HIGH);
  buttonVal04=buttonVal04-1;
}
if (buttonVal04 ==0 ) {
  Serial.println("Dose 4 LOW");
  digitalWrite(ledPin4, LOW);
}

if (buttonVal05 <<0 ) {
  Serial.println("Dose 5 HIGH");
  digitalWrite(ledPin5, HIGH);
  buttonVal05=buttonVal05-1;
}
if (buttonVal05 ==0 ) {
  Serial.println("Dose 5 LOW");
  digitalWrite(ledPin5, LOW);
}
if (buttonVal06 <<0 ) {
  Serial.println("Dose 6 LOW");
  digitalWrite(ledPin6, HIGH);
  buttonVal06=buttonVal06-1;
}
if (buttonVal06 ==0 ) {
  Serial.println("Dose 6 LOW");
  digitalWrite(ledPin6, LOW);
}
if (buttonVal07 <<0 ) {
  Serial.println("Dose 7 HIGH");
  digitalWrite(ledPin7, HIGH);
  buttonVal07=buttonVal07-1;
}
if (buttonVal07 ==0 ) {
  Serial.println("Dose 7 LOW");
  digitalWrite(ledPin7, LOW);
}
if (buttonVal08 <<0 ) {
  Serial.println("Dose 8 HIGH");
  digitalWrite(ledPin8, HIGH);
  buttonVal08=buttonVal08-1;
}
if (buttonVal08 ==0 ) {
  Serial.println("Dose 8 LOW");
  digitalWrite(ledPin8, LOW);
}
if (buttonVal09 <<0 ) {
  Serial.println("Dose 8 HIGH");
  digitalWrite(ledPin9, HIGH);
  buttonVal09=buttonVal09-1;
}
if (buttonVal09 ==0 ) {
  Serial.println("Dose 8 LOW");
  digitalWrite(ledPin9, LOW);
}
if (buttonVal10 <<0 ) {
  Serial.println("Dose 10 HIGH");
  digitalWrite(ledPin10, HIGH);
  buttonVal10=buttonVal10-1;
}
if (buttonVal10 ==0 ) {
  Serial.println("Dose 10 LOW");
  digitalWrite(ledPin10, LOW);
}
if (buttonVal11 <<0 ) {
  Serial.println("Dose 11 HIGH");
  digitalWrite(ledPin11, HIGH);
  buttonVal11=buttonVal11-1;
}
if (buttonVal11 ==0 ) {
  Serial.println("Dose 11 LOW");
  digitalWrite(ledPin11, LOW);
}
if (buttonVal12 <<0 ) {
  Serial.println("Dose 12 HIGH");
  digitalWrite(ledPin12, HIGH);
  buttonVal12=buttonVal12-1;
}
if (buttonVal12 ==0 ) {
  Serial.println("Dose 12 LOW");
  digitalWrite(ledPin12, LOW);
}
if (buttonVal13 <<0 ) {
  Serial.println("Dose 13 HIGH");
  digitalWrite(ledPin13, HIGH);
  buttonVal13=buttonVal13-1;
}
if (buttonVal13 ==0 ) {
  Serial.println("Dose 13 LOW");
  digitalWrite(ledPin13, LOW);
}

if (buttonVal14 <<0 ) {
  Serial.println("Dose 14 HIGH");
  digitalWrite(ledPin14, HIGH);
  buttonVal14=buttonVal14-1;
}
if (buttonVal14 ==0 ) {
  Serial.println("Dose 14 LOW");
  digitalWrite(ledPin14, LOW);
}

if (buttonVal15 <<0 ) {
  Serial.println("Dose 15 HIGH");
  digitalWrite(ledPin15, HIGH);
  buttonVal15=buttonVal15-1;
}
if (buttonVal15 ==0 ) {
  Serial.println("Dose 15 LOW");
  digitalWrite(ledPin15, LOW);
}

if (buttonVal16 <<0 ) {
  Serial.println("Dose 16 HIGH");
  digitalWrite(ledPin16, HIGH);
  buttonVal16=buttonVal16-1;
}
if (buttonVal16 ==0 ) {
  Serial.println("Dose 16 LOW");
  digitalWrite(ledPin16, LOW);
}

*/
delay(1000);
}

Prüfe mal mit dem seriellen Monitor, ob die einzelnen Taster überhaupt 
erkannt werden.
Ich habe "Coin" auf Pin 17 geklemmt, sonst ist die serielle 
Schnittstelle nicht nutzbar.

[Martin K. (maart)]

Taste 13 wird so immer erkannt werden, da der interne Pullup für die 
serienmäßige LED zu schwach ist, also nicht wundern, oder Pin 13 direkt 
auf 5V legen.

[Marian G. (houwdy)]

Patrick B. schrieb:
> Ehm, ich will dir nicht zu nahe treten Marian, aber bist du Resistent
> gegen konkrete Verbesserungsvorschläge bei offensichtlichen Fehlern?
> Kennst du den Unterschied zwischenbuttonVal01 << 0
> undbuttonVal01 < 0
>
> Das eine ist eine Schiebeoperation, und das andere ein Vergleich!

und genau das war der Fehler!
Es tut mir leid aber das war ein Blackout im Sinne " ich seh den Wald 
vor lauter Bäumen nicht mehr"
Ich hab Dussels Statement durch meinen Tunnelblick nicht kapiert.
Klar kanns mit Bitshiftoperatoren nicht gehn.

Danke jedenfalls für eure Hilfe! ich hab jetz den Ungleichoperator (!=) 
verwendet und jetzt funktioniert es.

Im Moment läuft das ganze noch mit 3 Kanälen am Steckbrettchen aber ich 
denke ich werde es heute Abend auf die Hardware spielen und wenn der 
Code dort funktioniert poste ich denselben nochmal der Vollständigkeit 
halber.

Tausend Dank euch allen erstmal.

Marian

[Hubert G. (hubertg)]

Ich habe deinen ersten Code verwendet. Interne PullUp aktiviert. 
Funktioniert mit 4 Tasten.
Die Pin 0 und 1 werden für den Upload benötigt und sollten nicht 
verwendet werden.

[Martin K. (maart)]

Marian G. schrieb:
>> Das eine ist eine Schiebeoperation, und das andere ein Vergleich!
>
> und genau das war der Fehler!

Was aber in deinem Fall durch Zufall nicht schlimm ist. Die if-Bedingung 
trifft zu, solange das Ergebnis ungleich null ist. Die Bitschieberei um 
null Stellen verändert da nichts.
Wie gesagt, bei mir funktioniert auch deine Codevariante.

[Einer K. (Gast)]

Martin K. schrieb:
> Taste 13 wird so immer erkannt werden, da der interne Pullup für
> die
> serienmäßige LED zu schwach ist, also nicht wundern, oder Pin 13 direkt
> auf 5V legen.

Das ist so nicht richtig!
Siehe: 
https://www.arduino.cc/en/uploads/Main/arduino-mega2560_R3-sch.pdf

[Axel R. (Gast)]

StromTuner

Marian G. schrieb:
> Im Moment läuft das ganze noch mit 3 Kanälen am Steckbrettchen

...lief es doch vorher auch schon...

>leider mit 16 nicht mehr und ich schau schon seit 2 Stunden
>darauf und find den Fehler nicht...

Ob Du nun Bishiftleft zero machst und auf ungleich null testest, oder 
sorum. Ist zwar mit "<<" völlig am Ziel vorbei, sollte an der 
Fehlerbeschreibung nichts ändern, die am Anfang angegeben hast. Es 
funktioniert ja (hier eher zufällig)
Bin ich ja gespannt!

[Peter D. (peda)]

Kaj schrieb:
> Ich hab mal den Code aufgeraeumt.

Hier bieten sich Schleifen geradezu an.
Nicht nur der Code wird nen ordentlichen Zacken kürzer und damit 
leserlicher.
Auch sinkt die Fehlerwarscheinlichkeit drastisch. Man muß ja nur einen 
Schleifendurchgang prüfen und nicht 16 Copy&Paste Stellen.

Ich nehme Schleifen schon ab 2 weitgehend identischen Codeteilen. 
Copy&Paste Code vermeide ich, wenn es nur irgend geht.

[Lothar M. (Firma: Titel) (lkmiller) (Moderator)]

Nur mal kurz in die Runde gefragt, was ist denn daran so schwer zu 
verstehen:

Antwort schreiben
Wichtige Regeln - erst lesen, dann posten!

    Längeren Sourcecode nicht im Text einfügen, sondern als Dateianhang

Ich habe einen Fehler im Programm gefunden:
> delay(100);
Denn in der Hauptschleife aktiv Rechenzeit zu vergeuden ist mit 
Sicherheit ein Fehler.

[Leroy M. (mayl)]

Lothar M. schrieb:
> Ich habe einen Fehler im Programm gefunden:
>> delay(100);
> Denn in der Hauptschleife aktiv Rechenzeit zu vergeuden ist mit
> Sicherheit ein Fehler.

In dem Fall nicht, das Programm ist einfach laienhaft geschrieben.


Das Delay wird im Endeffekt mit int timeVal multipliziert und ergibt 
somit die Zeit, die der Output high ist.

An den TO:

Wenn du statt

>>if (digitalRead(buttonPin01) == LOW&& Coin ==1) {
>>  buttonVal01 = timeVal;
>>  Coin =0;
>>}

das hier

>>if (digitalRead(buttonPin01) == LOW&& Coin ==1) {
>>  buttonVal01 = buttonVal01 + timeVal;
>>  Coin =0;
>>}

schreibst, kannst du die Zeit verlängern, in der die Steckdose an ist, 
wenn eine neue Münze eingeworfen wurde. ;)

[Lenny D. (le-do)]

Ob das mit dem Fehler zusammenhängt kann ich nicht beurteilen, aber 
mache dir am besten nochmal Gedanken über entprellung und 
schaltgeschwindigkeit bei den Tastern aber v.a. bei der Münzerkennung.
Wenn ich das richtig sehe kann eine schnell vorbeirollende münze während 
dem delay unerkannt bleiben, wenn man das delay verkürzt eine münze 
aufgrund von Prellung aber mehrfach registriert werden (ok das ignoriert 
der Code bisher, aber vielleicht mal später)

[Martin K. (maart)]

Arduino F. schrieb:
> Martin K. schrieb:
>> Taste 13 wird so immer erkannt werden, da der interne Pullup für
>> die
>> serienmäßige LED zu schwach ist, also nicht wundern, oder Pin 13 direkt
>> auf 5V legen.
>
> Das ist so nicht richtig!
> Siehe:
> https://www.arduino.cc/en/uploads/Main/arduino-mega2560_R3-sch.pdf

Was genau meinst Du? Ich habe ja den Test vorhin direkt mit der Hardware 
gemacht.

[Peter (Gast)]

Oh man wie kann man so beratungsresistent sein und den auf einem 
Silbertablett serviereten super zusammen gefasten Code nicht zu nehmen.

[Kaj (Gast)]

Marian G. schrieb:
> ich hab jetz den Ungleichoperator (!=)
> verwendet und jetzt funktioniert es.
Ja, aber ist das auch das was du willst?
Willst du testen ob es groesser 0 ist (nur positiver Wertebereich, ohne 
die 0), oder willst du testen ob es ungleich 0 ist (negativer und 
positiver Wertebereich, ohne die 0)

[Einer K. (Gast)]

Martin K. schrieb:
> Was genau meinst Du? Ich habe ja den Test vorhin direkt mit der Hardware
> gemacht.

Auf dem Arduino Mega ist ein Impedanzwandler (OP?) verbaut, um genau das 
von dir genannte Problem zu unterbinden.

Pin13 kann also problemlos mit INPUT_PULLUP verwendet werden.

Warum dein Mega das nicht hat, kann ich dir nicht sagen.

[Martin K. (maart)]

Arduino F. schrieb:
> Warum dein Mega das nicht hat, kann ich dir nicht sagen.

Ok.
Vermutlich, weil es ein Klon ist.

[Patrick J. (ho-bit-hun-ter)]

Hi

Martin K. schrieb:
> Vermutlich, weil es ein Klon ist.
Hat bei Dolly aber anders ausgesehen ... äh nein, eigentlich genau 
gleich :)

Was Du meinst: Ein günstiger Nachbau, landläufig aber auch gerne, da 
gebe ich Dir Recht, Klon genannt, wenn's auch nicht wirklich trifft.

Davon ab - laufen jetzt alle 16 Taster/Steckdosen?
Zum Probieren muß man ja nicht jedes Mal 2 Stunden warten - eine Zahl im 
Programm geändert und die Zeit wird auf der Basis 10 Sekunden 
verwurschtelt.

MfG

[Marian G. (houwdy)]

Lothar M. schrieb:
> Nur mal kurz in die Runde gefragt, was ist denn daran so schwer zu
> verstehen:Antwort schreiben
> Wichtige Regeln - erst lesen, dann posten!
>
>     Längeren Sourcecode nicht im Text einfügen, sondern als Dateianhang
>
> Ich habe einen Fehler im Programm gefunden:
>> delay(100);
> Denn in der Hauptschleife aktiv Rechenzeit zu vergeuden ist mit
> Sicherheit ein Fehler.

Zum Ersten: dafür entschuldige ich mich und gelobe fürs nächste mal 
Besserung.

Wie würdest du das delay 100 umgehen in diesem Fall?

Peter schrieb:
> Oh man wie kann man so beratungsresistent sein und den auf einem
> Silbertablett serviereten super zusammen gefasten Code nicht zu nehmen.

Ich hab den Code ja genommen, auserdem hab ich mir recht viele Gedanken 
gemacht wie ich das hier gesehene beim nächsten Projekt anwenden kann, 
worauf bezieht sich dann meine Beratungsresistenz? Würde ich so 
programmieren wie ich Wig schweisse dann müsste ich nicht nachfragen.

Patrick J. schrieb:
> Davon ab - laufen jetzt alle 16 Taster/Steckdosen?
> Zum Probieren muß man ja nicht jedes Mal 2 Stunden warten - eine Zahl im
> Programm geändert und die Zeit wird auf der Basis 10 Sekunden
> verwurschtelt.
>
> MfG

10 laufen :) die anderen 6 kommen morgen nach der Arbeit drann :) dann 
teile ich euch das Ergebnis mit.

Mfg Marian

[Axel R. (Gast)]

Woran lag es "denn dann denn" nun, das erst nur zwei Ausgänge 
funktionierten und nun alles ok ist?
Doch sicher nicht an der (<<0) ...
Kann das mal jemand zusammenfassen?

StromTuner

[Einer K. (Gast)]

Axel R. schrieb:
> Kann das mal jemand zusammenfassen?

Ich versuchs mal...

Aus einer unsäglichen, fehlerträchtigen, Codewurst, ist ein 
übersichtliches Konstrukt mit Schleifen und Arrays geworden.

[Martin K. (maart)]

Arduino F. schrieb:
> Axel R. schrieb:
>> Kann das mal jemand zusammenfassen?
>
> Ich versuchs mal...
>
> Aus einer unsäglichen, fehlerträchtigen, Codewurst, ist ein
> übersichtliches Konstrukt mit Schleifen und Arrays geworden.

Ich tippe eher auf Hardwareprobleme. Die "Codewurst" funktioniert ja auf 
meinem Mega.
Das die Wurst nur aus Zufall funktioniert, will ich nicht abstreiten.

[Marian G. (houwdy)]

Hier der funktionierende Code. ich poste ihn im Beitrag da es ja nur ein 
par Zeilen sind. Ich hoffe das ist in Ordnung.

Danke noch einmal für eure Hilfe. Ich glaube der ist jetzt um einiges
schöner als der Erste.

#include <stdint.h>
const int16_t coinPin = 4;
const int16_t buttonPins[16] =  {23, 25, 27, 29, 31, 53, 51, 49,  
                                 47, 45, 43, 41, 39, 37, 35, 33};    
const int16_t ledPins[16] =     {14, 15, 16, 17, 18, 19, 20, 36,
                                34, 32, 30, 28, 26, 24, 22, 20};
int16_t buttonState[16] = {0};
unsigned long buttonVal[16] = {0};
int16_t coinState = 0;
int16_t Coin = 0;
unsigned long timeVal = 1000000;

void setup()
{
    for (uint8_t i = 0; i < 16; i++) {
        pinMode(ledPins[i], OUTPUT);
        pinMode(buttonPins[i], INPUT_PULLUP);    }

    pinMode(coinPin, INPUT_PULLUP); 
}
void loop()
{
    if ((digitalRead(coinPin)) == LOW) {
        Coin = 1;
        
    for (uint8_t i = 0; i < 16; i++) {
        if ((digitalRead(buttonPins[i]) == LOW) && (Coin == 1)) {
            buttonVal[i] = timeVal;
            Coin = 0;
            break;
        } }  }

    for (uint8_t i = 0; i < 16; i++) {
        if (buttonVal[i] != 0) {
            digitalWrite(ledPins[i], LOW);
            buttonVal[i] -= 1;}

        if (buttonVal[i] == 0) {
            digitalWrite(ledPins[i], HIGH);  }  }

    delay(10);
}

[Patrick J. (ho-bit-hun-ter)]

Hi

Wenn Du die Klammern mit etwas mehr Sinn setzt, ergeben Diese direkt die 
zusammenhängenden Blöcke:

 #include <stdint.h>
 const int16_t coinPin = 4;
 const int16_t buttonPins[16] =  {23, 25, 27, 29, 31, 53, 51, 49,
                                  47, 45, 43, 41, 39, 37, 35, 33};
 const int16_t ledPins[16] =     {14, 15, 16, 17, 18, 19, 20, 36,
                                 34, 32, 30, 28, 26, 24, 22, 20};
 int16_t buttonState[16] = {0};
 unsigned long buttonVal[16] = {0};
 int16_t coinState = 0;
 int16_t Coin = 0;
 unsigned long timeVal = 1000000;
 
 void setup()
 {
     for (uint8_t i = 0; i < 16; i++) {
         pinMode(ledPins[i], OUTPUT);
         pinMode(buttonPins[i], INPUT_PULLUP);
-->  }
 
     pinMode(coinPin, INPUT_PULLUP);
 }
 void loop()
 {
     if ((digitalRead(coinPin)) == LOW) {
         Coin = 1;
 
         for (uint8_t i = 0; i < 16; i++) {
            if ((digitalRead(buttonPins[i]) == LOW) && (Coin == 1)) {
                buttonVal[i] = timeVal;
                Coin = 0;
                break;
-->         }
-->      }
-->  }
 
     for (uint8_t i = 0; i < 16; i++) {
         if (buttonVal[i] != 0) {
             digitalWrite(ledPins[i], LOW);
             buttonVal[i] -= 1;
-->      }
         if (buttonVal[i] == 0) {
             digitalWrite(ledPins[i], HIGH);
-->      }
-->  }
 
     delay(10);
 }

Alles auf gleicher Ebene ist gleich weit eingerückt, finde ich einfacher 
und besser lesbar.

Danke für's Zeigen!

MfG

[Joachim B. (jar)]

Patrick J. schrieb:
> Wenn Du die Klammern mit etwas mehr Sinn setzt, ergeben Diese direkt die
> zusammenhängenden Blöcke:
>

.....
>      for (uint8_t i = 0; i < 16; i++) {
>          pinMode(ledPins[i], OUTPUT);
>          pinMode(buttonPins[i], INPUT_PULLUP);
> -->  }
>

> Alles auf gleicher Ebene ist gleich weit eingerückt, finde ich einfacher
> und besser lesbar.

besser lesbar?
halte ich für diskutierbar, ich finde Öffnungsklammern irgendwo hinten 
schlecht lesbar

.....
void loop()
{  if ((digitalRead(coinPin)) == LOW) 
   {  Coin = 1;
      for (uint8_t i = 0; i < 16; i++) 
      {  if ((digitalRead(buttonPins[i]) == LOW) && (Coin == 1)) 
         {  buttonVal[i] = timeVal;
            Coin = 0;
            break;
         } 
      }  
   }

   for (uint8_t i = 0; i < 16; i++) 
   {  if (buttonVal[i] != 0) 
      {  digitalWrite(ledPins[i], LOW);
         buttonVal[i] -= 1;
      }
      if (buttonVal[i] == 0) 
         digitalWrite(ledPins[i], HIGH);  
   }
   delay(10);
}

das halte ich für lesbarer, Einzeiler brauchen selten Klammern

ist vielleicht nur meine Meinung, aber ich sehe wo Blöcke anfangen und 
wo enden