Hilfe bei Programm-Bug mit AtMega8 benötigt

[Thomas W. (ram99)]

Hallo,

ich habe mir ein kleines Programm für eine selbstgebastelte 
Rollladensteuerung geschrieben und kämpfe gerade mit einem Problem, 
welches ich überhaupt nicht verstehe.
Das Programm steuert einen Motor (rechts/linkslauf) welcher über 
Nockenschalter (CamUp, CamDown) abgeschaltet wird.
Der Rollladen kann über den Schalter SwitchAutoOn zwischen Automatik und 
Manuell gesteuert werden. Im Automatikmodus wird der Rollladen über 
einen Fotowiderstand gesteuert. Im manuellen Modus wird über Taster 
(ButtonDown, ButtonUp und ButtonStop + Nockenschalter gesteuert. 
Zusätzlich gibt es noch ein Funkmodul welches an die anderen Rollläden 
den Trigger für Auf oder Ab sendet.
Die Hardware funktioniert da ich den Rollladen schon seit Jahren 
betreibe. Jetzt habe ich aus Langeweile einige Programmteile geändert 
und hänge an einer Stelle die ich nicht verstehe.

In der Unterfunktion „Schalter“ wird eine Case Schrittkette ausgeführt. 
Dort hänge ich im Case Schritt 2 wenn auf die Nockenschalter gewartet 
wird.
Warum auch immer werden in diesem Schritt die CamUp und CamDown Merker 
nicht ausgewertet. Betätige ich den ButtonStop oder ist der Errorcounter
Erreicht, wird die Kette fortgesetzt. Nur eben auf CamUp und CamDown 
wird nicht reagiert. Die zwei Variablen werden in der Unterfunktion 
„MAP_INPUTS“ gesetzt.
Noch zur Info – im Case Schritt 0 wird auch auf die zwei Variablen 
abgefragt. Dort funktioniert die If Bedingung abhängig vom Zustand der 
Nockenschalter. Nur im Case Schritt 2 nicht als ob die zwei Variablen 
irgendwie überschrieben werden. In dem Schritt habe ich mir testweiße 
die Zustände der zwei Variablen auf dem Display angezeigt. Dort sehe ich 
wie der Zustand zwischen 0 und 1 wechselt, wenn ich die Schalter 
betätige.

Es wäre super, wenn jemand die Case Kette + Mapping der Eingänge 
anschauen könnte. Alles andere in dem Programm funktioniert wie ich es 
möchte. Nur eben das erkennen der Nockenschalter im Case 2 nicht.
Sollte etwas grober Unfug in dem Programm sein freue ich mich über eine 
Info 😊

Ich arbeite mit Microchip Studio V7.

Viele Grüße
Thomas

[Thomas W. (ram99)]

Hier noch die Stelle wo es klemmt

[Nemopuk (nemopuk)]

Hast du den AVCC Pin und beide GND Pins an die Stromversorgung 
angeschlossen?

[Paul B. (paule201)]

Ich habs nur mal fix durch die KI gejagt, nicht selbst geschaut. Die KI 
sagt folgendes, schau mal nach ob es das ist und gib gern ein Zeichen ob 
es passt oder nicht:

⚙️ Fehlerursache

Der Kernfehler liegt in der Logik der Abfrage und in der Polarität 
deiner Eingänge.

Du liest die Nockenschalter nicht invertiert ein, d. h.:

CamUp = (PINC & (1<<iCamUp));     // ergibt 0 oder !=0
CamDown = (PINC & (1<<iCamDown)); // ergibt 0 oder !=0

→ Wenn der Pin geschlossen (LOW) ist, liefert CamUp = 0.
→ Wenn der Pin offen (HIGH) ist (wegen Pull-Up), liefert CamUp != 0 
(meist 0x20 oder 0x10).

In der Logik deiner Schrittkette verwendest du aber beides 
durcheinander:

Fall  Abfrage  Erwartete Logik
Schritt 0 (Start)  (CamDown == 0) → Rollladen kann runterfahren  OK, 
Schalter geschlossen = „unten nicht erreicht“
Schritt 2 (Stop)  (CamDown == 1) → Rollladen hat unten erreicht  ❌ Hier 
liegt der Fehler: du prüfst auf 1, aber dein Eingang liefert 0, wenn 
geschlossen!

Damit wartest du in Schritt 2 auf den Zustand „CamDown == 1“, also nicht 
betätigt, während du eigentlich „CamDown == 0“ (betätigt) brauchst.

✅ Lösung

Ändere in Case 2 die Abfrage folgendermaßen:

case 2:
  LED = Toggle1s;
  Errorcounter++;

  if ((Errorcounter >= (Abschaltung * 100)) || ((SwitchAutoOn == 0) && (ButtonStop == 1)))
  {
      RelaisOn = 0;
      SwitchStep++;
  }

  // Richtige Polarität!
  if (((DownFlag == 1) && (CamDown == 0)) || ((UpFlag == 1) && (CamUp == 0)))
  {
      RelaisOn = 0;
      SwitchStep++;
  }
  break;

👉 Begründung:

Du verwendest Pullups, also:
Taster/Nockenschalter geschlossen → Pegel LOW → 0

Daher ist „ausgelöst“ = 0 und nicht 1.

🧠 Warum es im Schritt 0 trotzdem funktioniert

In Schritt 0 fragst du genau so ab:

if ((CamDown == 0) && (ButtonDown == 1) && (DownFlag == 0))

→ Das ist korrekt, deswegen funktioniert’s dort.
In Schritt 2 war nur die Polarität verdreht.

💡 Empfehlung zur Robustheit

Damit du das nie wieder verwechselst, definiere dir am Anfang saubere 
Makros:

#define IS_ACTIVE(x)   ((x) == 0)   // low-aktiv
#define IS_INACTIVE(x) ((x) != 0)

und nutze dann:

if ((DownFlag == 1) && IS_ACTIVE(CamDown))

Gib den Eingangsbits sprechende Namen mit ihrer Logikrichtung:

uint8_t CamDown_ActiveLow;

oder

CamDown = !(PINC & (1<<iCamDown));

Dann arbeitest du im Code immer mit 0 = aus, 1 = an und sparst dir 
späteres Umdenken.

🔍 Optionaler Check

Wenn du im Display siehst:
„CamDown = 0“ während Schalter gedrückt → korrekt.
Dann sollte nach der Änderung oben der Fall direkt erkannt und 
SwitchStep weitergeschaltet werden.

📋 Fazit

Der Fehler ist kein Timing- oder Interrupt-Problem, sondern:
Falsche Polarität der Nockenschalter in der Bedingung von Case 2.

Änderung:

- if (((DownFlag == 1) && (CamDown == 1)) || ((UpFlag == 1) && (CamUp == 1)))
+ if (((DownFlag == 1) && (CamDown == 0)) || ((UpFlag == 1) && (CamUp == 0)))

Danach sollte dein Rollladen sofort wieder sauber stoppen, wenn der 
Endschalter erreicht ist.

[Sebastian R. (sebastian_r569)]

Paul B. schrieb:
> Die KI
> sagt folgendes, schau mal nach ob es das ist und gib gern ein Zeichen ob
> es passt oder nicht:

Ein bisschen selber nachdenken wirst du wohl müssen. Ich glaube nicht, 
dass in diesem Forum jemand gewillt ist, irgendwelchen KI-Text zu 
überprüfen.

[Nemopuk (nemopuk)]

Da hat die KI wie erwartet viel irreführenden Text fabuliert.

[Norbert (der_norbert)]

Sebastian R. schrieb:
> Ich glaube nicht,
> dass in diesem Forum jemand gewillt ist, irgendwelchen KI-Text zu
> überprüfen.

Da muss man sich nur eine einzige Zeile ansehen.
Wenn eine KI noch nicht einmal in der Lage ist, die operator precedence 
gescheit und nicht doppelt und dreifach gemoppelt und umklammert 
anzuwenden, kann der Rest auch nur Schwachsinn sein.

== bindet stärker als && und das stärker als ||.

Just sayin'

[Paul B. (paule201)]

Sebastian R. schrieb:
> Ich glaube nicht,
> dass in diesem Forum jemand gewillt ist, irgendwelchen KI-Text zu
> überprüfen.

Das verlangt auch keiner von den Mitgliedern und dem TE ist es selbst 
überlassen die Info zu nutzen.

Nemopuk schrieb:
> Da hat die KI wie erwartet viel irreführenden Text fabuliert.

Ich sehe Kritik an der KI, aber ich sehe keine, nicht irreführende, 
Lösung von dir. Actus non verba.

[Thomas W. (ram99)]

Nemopuk schrieb:
> Hast du den AVCC Pin und beide GND Pins an die Stromversorgung
> angeschlossen?


Ja, habe ich gemacht. Die Schaltung funktioniert ja auch schon seit 
kanpp 10 Jahren. Ich habe jetzt nur die Case Kette anstelle von If/Else 
eingesetzt und die EIn/Ausgänge über die MAP Funktionen umgeschrieben.

[Thomas W. (ram99)]

Nemopuk schrieb:
> Da hat die KI wie erwartet viel irreführenden Text fabuliert.

Ja, das ist quatsch was da generiert wurde.
Ich sehe ja auch das CamUp und CamDown beim betätigen vom Schalter auf 1 
wechselt wenn ich die VAriablen aufs Display lege.

[Paul B. (paule201)]

Norbert schrieb:
> Wenn eine KI noch nicht einmal in der Lage ist, die operator precedence
> gescheit und nicht doppelt und dreifach gemoppelt und umklammert
> anzuwenden, kann der Rest auch nur Schwachsinn sein.

Die Basis ist der Quelltext aus dem Eingangspost...man kann der KI das 
aber durchaus mitteilen das anders zu machen. In dem Fall verändert sie 
aber wenig am Originalquelltext um es besser nachvollziehbar zu halten. 
Aber wie gesagt, man muss die KI nicht nutzen, ähnlich wie Smartphones, 
das Internet oder das Auto. Wurde damals auch alles verteufelt, heute 
nicht mehr wegzudenken. Wobei im Nachbarthread grad ein Handy ohne alles 
gesucht wird, Ausnahmen bestätigen die Regel ;).

Aber auch in deinem Post sehe ich nur Kritik an der KI, aber keine 
Lösung des Problems.

[Thomas W. (ram99)]

Norbert schrieb:
> Sebastian R. schrieb:
>> Ich glaube nicht,
>> dass in diesem Forum jemand gewillt ist, irgendwelchen KI-Text zu
>> überprüfen.
>
> Da muss man sich nur eine einzige Zeile ansehen.
> Wenn eine KI noch nicht einmal in der Lage ist, die operator precedence
> gescheit und nicht doppelt und dreifach gemoppelt und umklammert
> anzuwenden, kann der Rest auch nur Schwachsinn sein.
>
> == bindet stärker als && und das stärker als ||.
>
> Just sayin'

Es klappt aber auch nicht wenn ich nur einen Ausdruck verwende.

if ( CamDown == 1 )
{
  RelaisOn = 0;
  SwitchStep++;
}

[Johann L. (gjlayde)]

Norbert schrieb:
> == bindet stärker als && und das stärker als ||.

Stimmt zwar, aber GCC zum Beispiel warnt per default wenn man alle 
unnötigen Klammern weglässt:

bool fun (int a, int b, int c)
{
  return a && b || c;
}

warning: suggest parentheses around '&&' within '||' [-Wparentheses]

[Andreas M. (amesser)]

CamUp = (PINC &(1<<iCamUp));
CamDown = (PINC &(1<<iCamDown));

- CamUp ist nicht 0 oder 1 sondern 0 oder 0x20
- CamDown ist nicht 0 oder 1 sondern 0 oder 0x10

Du vergleichst aber auf "1". Variablen die als boolsche Werte verwendet 
werden sollte man niemals explizit auf einen Werte prüfen. Statt "(CamUp 
== 1)" einfach "(CamUp)", wahlweise ohne Klammern.

[Johann L. (gjlayde)]

Andreas M. schrieb:

> CamUp = (PINC &(1<<iCamUp));
> CamDown = (PINC &(1<<iCamDown));

> - CamUp ist nicht 0 oder 1 sondern 0 oder 0x20
> - CamDown ist nicht 0 oder 1 sondern 0 oder 0x10
>
> Du vergleichst aber auf "1". Variablen die als boolsche Werte verwendet
> werden sollte man niemals explizit auf einen Werte prüfen. Statt "(CamUp
> == 1)" einfach "(CamUp)", wahlweise ohne Klammern.

Oder einfach

// In/Out flags
bool CamUp, CamDown, ButtonUp, ButtonDown, ButtonStop, SwitchAutoOn, ...

statt uint8_t.

[Thomas W. (ram99)]

Andreas M. schrieb:
>

> CamUp = (PINC &(1<<iCamUp));
> CamDown = (PINC &(1<<iCamDown));
>

>
> - CamUp ist nicht 0 oder 1 sondern 0 oder 0x20
> - CamDown ist nicht 0 oder 1 sondern 0 oder 0x10
>
> Du vergleichst aber auf "1". Variablen die als boolsche Werte verwendet
> werden sollte man niemals explizit auf einen Werte prüfen. Statt "(CamUp
> == 1)" einfach "(CamUp)", wahlweise ohne Klammern.

TaTa, das ist die Lösung :)
Ich habe die == 1 entfernt und es geht.
Warume es aber beim ButtonStop == 1 klappt verstehe ich nicht. Wird das 
nach dem Zufallsprinzip beschrieben?
Hast du bei CamUp mit 0x20 einen Tippfehler? Wenn nicht verstehe ich es 
überhaupt nicht.

[Thomas W. (ram99)]

> Oder einfach
>

> // In/Out flags
> bool CamUp, CamDown, ButtonUp, ButtonDown, ButtonStop, SwitchAutoOn, 
> ...

> statt uint8_t.

bool gibt es leider nicht beim GCC

[Johann L. (gjlayde)]

Thomas W. schrieb:
> bool gibt es leider nicht beim GCC

Natürlich gibt's das, zumindest seit C99

#include <stdbool.h>

Und der Code steht eh in C99, verwendet zB // Kommentare.

[Andreas M. (amesser)]

Thomas W. schrieb:
> Warume es aber beim ButtonStop == 1 klappt verstehe ich nicht. Wird das
> nach dem Zufallsprinzip beschrieben?

Nein ist kein Zufall

ButtonStop =! (PIND &(1<<iButtonStop));

Das "!" ist die boolsche Negation. D.h. der Ausdruck dahinter wird 
zunächst in einen bool umgewandelt und das Ergebnis davon ist wieder ein 
bool. Meistens ist es so, das der boolsche Wert "true" dem Zahlenwert 
"1" entspricht.

Thomas W. schrieb:
> Hast du bei CamUp mit 0x20 einen Tippfehler? Wenn nicht verstehe ich es
> überhaupt nicht.

Wenn ich mich nicht verrechnet habe, dann ist "1<<iCamUp" => "0x01 << 5" 
=> "0x20". Da steht dann also "PIND & 0x20" (Bitweises UND), damit kann 
da nur 0 oder 0x20 rauskommen.

[Johann L. (gjlayde)]

Andreas M. schrieb:

> ButtonStop =! (PIND &(1<<iButtonStop));

> Das "!" ist die boolsche Negation. D.h. der Ausdruck dahinter wird
> zunächst in einen bool umgewandelt [...]

Nitpick: Der Ausdruck hinter dem ! ist vom Typ int, und der ganze ! 
Ausdruck ebenso.

[Paul B. (paule201)]

Paul B. schrieb:
> → Wenn der Pin geschlossen (LOW) ist, liefert CamUp = 0.
> → Wenn der Pin offen (HIGH) ist (wegen Pull-Up), liefert CamUp != 0
> (meist 0x20 oder 0x10).

Thomas W. schrieb:
> Ja, das ist quatsch was da generiert wurde.

Andreas M. schrieb:
> - CamUp ist nicht 0 oder 1 sondern 0 oder 0x20
> - CamDown ist nicht 0 oder 1 sondern 0 oder 0x10

Thomas W. schrieb:
> TaTa, das ist die Lösung :)
> Ich habe die == 1 entfernt und es geht.

Ich lass das mal so stehen, wa ;).

Andreas hat es aber deutlich besser und klarer formuliert, keine Frage 
:)

[Thomas W. (ram99)]

@Andreas M.
Also, ich verstehe es jetzt erst richtig. Mein Fehler war die Annahme 
das ich einfach den Zustand des Eingangs in eine Variable schreiben 
kann.
  CamUp = (PINC &(1<<iCamUp));
  CamDown = (PINC &(1<<iCamDown));
Das in CamUp und CamDown der Binäre Wert von der Pin Position des Ports 
gesetzt wird habe ich beim erstellen nicht gerafft.

@Paul B.
Damit ergibt die Antowrt der KI auch etwas mehr Sinn auch wenn die 
Lösung irreführend ist.

@ Johann L.
Cool, wieder etwas neues gelernt. 1-2 kleine Projekte alle 3 Jahre 
reicht einfach nicht aus. Eben die <stdbool.h> hinzugefügt die zukünftig 
verwendet wird.


Vielen Dank an alle für eure tollen Erklärungen. Davon kann ich einiges 
mitnehmen. Das Programm funktioniert jetzt jedenfalls wie erwartet.
Sollte jemand Langeweile haben würde ich mich über Codeverbesserungen 
freuen :)

[Nemopuk (nemopuk)]

Paul B. schrieb:
> aber ich sehe keine, nicht irreführende, Lösung von dir.

korrekt. Ich habe die Fehlerursache nich nicht erkannt. Deswegen halte 
ich mich (anders als die plappernde KI) diesbezüglich zurück.

[Nemopuk (nemopuk)]

Andreas M. schrieb:
> CamUp ist nicht 0 oder 1 sondern 0 oder 0x20

Sehr gut, respekt!

[Norbert (der_norbert)]

Johann L. schrieb:
> Norbert schrieb:
>> == bindet stärker als && und das stärker als ||.
>
> Stimmt zwar, aber GCC zum Beispiel warnt per default wenn man alle
> unnötigen Klammern weglässt:bool fun (int a, int b, int c)
> {
>   return a && b || c;
> }
> warning: suggest parentheses around '&&' within '||' [-Wparentheses]

Johann, nur aus Interesse, welcher tiefere Sinn verbirgt sich dahinter?
Also für eine sowohl syntaktisch als auch logisch zu 100% korrekt 
formulierte Programmzeile eine Warnung zu werfen?

[Nemopuk (nemopuk)]

Johann L. schrieb:
> GCC zum Beispiel warnt per default wenn man alle
> unnötigen Klammern weglässt

Norbert schrieb:
> welcher tiefere Sinn verbirgt sich dahinter?
> Also für eine sowohl syntaktisch als auch logisch zu 100% korrekt
> formulierte Programmzeile eine Warnung zu werfen?

Lass dich nicht verarschen. Der gcc warnt in diesem Fall nicht per 
default.

[Norbert (der_norbert)]

Nemopuk schrieb:
> Lass dich nicht verarschen. Der gcc warnt in diesem Fall nicht.

Selbstverständlich warnt er. Man muss nur mal etwas ›sensitiver‹ 
schalten. So wie es (hoffentlich nicht nur) meine Standard-Einstellung 
ist.

#include <stdbool.h>

bool test(int a, int b,int c) {
    return a && b || c;
}

int main(void) {
    bool z = test(1,2,3);
    (void)z;
    return 0;
}

// gcc -Wall -Wextra -pedantic -std=c11 -no-pie -Os -o "unbenannt" "unbenannt.c" 
// unbenannt.c: In function ‘test’:
// unbenannt.c:6:14: warning: suggest parentheses around ‘&&’ within ‘||’ [-Wparentheses]
//     6 |     return a && b || c;
//       |            ~~^~~~
// Kompilierung erfolgreich beendet.

[Nemopuk (nemopuk)]

Norbert schrieb:
> Man muss nur mal etwas ›sensitiver‹ schalten.
> -Wall -Wextra -pedantic

Das ist das Gegenteil von "per default".

[Norbert (der_norbert)]

Nemopuk schrieb:
> Das ist das Gegenteil von "per default".

Gut.

Dann nenne ich es den sinnvollen default. ;-)

Der andere ist eher der – Achtung: bashing – Arduino default. (Was ich 
nicht weiß, macht mich nicht heiß)

[Nemopuk (nemopuk)]

Norbert schrieb:
> Dann nenne ich es den sinnvollen default.

Was der Compiler ohne besondere Parameter nicht tut, ist kein Default. 
Defaults sind die Standardvorgaben, die der Compiler ohne Anweisung von 
sich aus anwendet (so wie ich das vorgeführt habe).

> Der andere ist eher der – Achtung: bashing – Arduino default

Arduino deaktiviert mit seinen Vorgaben keine Warnungen des Compilers, 
sondern unterlässt es lediglich, die optionalen (nicht default) 
Warnungen zu aktivieren.

Dass -Wall sinnvoll ist, will ich gar nicht in Abrede stellen. Aber du 
redest dich hier mit falschen Begründungen heraus.

[Norbert (der_norbert)]

Das ist schon durchgedrungen.

Aber wer bitteschön schaltet denn noch nicht einmal Warnungen ein?
Bei der Software-Entwicklung komplett blind zu fahren, das ist doch nun 
wirklich Quadratunsinn.

Und selbst ein einfaches

gcc -Wall -o "unbenannt" "unbenannt.c"

reicht für die Warnung aus.
Aber ich werde mich mal aus dieser kleinteiligen Diskussion subtrahieren 
und warten ob Johann etwas dazu sagen kann.

[Nemopuk (nemopuk)]

Norbert schrieb:
> Aber wer bitteschön schaltet denn noch nicht einmal Warnungen ein?

Zum Beispiel Leute, die eine IDE mit Linter benutzen und kein Interesse 
an doppelt gemoppelten Verbesserungsvorschlägen haben.

[Matthias S. (dachs)]

Thomas W. schrieb:
> die Zustände der zwei Variablen auf dem Display angezeigt. Dort sehe ich
> wie der Zustand zwischen 0 und 1 wechselt

Es wäre sehr interessant, wie genau das gemacht wurde, dort sollten 
eigentlich die Werte 0x10 0x20 oder deren Werte in einem anderen 
Zahlenformat erscheinen.

[Nemopuk (nemopuk)]

Matthias S. schrieb:
> Es wäre sehr interessant, wie genau das gemacht wurde,

Siehe Anhang vom Eröffnungsbeitrag.

[Matthias S. (dachs)]

Jetzt habe ich es gefunden:

 // Überwachung Nockenschalter
  lcd_setcursor( 14, 2 );
  if ( CamDown == 0 )
  { lcd_string("0"); }
  else
  { lcd_string("1"); }

Da ist natürlich klar, dass er den Fehler nicht sehen kann, die 
Variablen mit

itoa( CamDown, Buffer, 10 );
lcd_string(Buffer);

auszugeben wäre erheblich aussagekräftiger gewesen.