Wieso funktioniert mein Programm nicht? (Led ein-/aus -schalten)

[Leie Jonas (Gast)]

Ich möchte eine Led mittels Taster ein bzw. ausschalten.
Diese Fehler sind zu beobachten: Die Led leuchtet nicht, auch nicht nach 
den Knopfdruck. An der Delay5ms Funktion liegt es nicht!

.equ XTAL = 16000000 ;16 mHz

.equ BTN  = 2 ;Pinnumber of PORT_D
.equ LED  = 3 ;Pinnumber of PORT_D

.def temp1 = r16
.def temp2 = r17

  ;Init stackpointer
  ldi temp1, HIGH(RAMEND)
  out SPH,   temp1
  ldi temp1, LOW(RAMEND)
  out SPL,   temp1

  ;Set led pin to output
  ldi temp1, (1 << LED)
  out DDRD, temp1

loop:
  in    temp1, PIND
  sbrc  temp1, (1 << BTN) ;Check if button is pressed
  rcall led_toggle
  rjmp  loop

led_toggle:
  ldi   temp2, (1 << LED)  
  eor   temp1, temp2 ;xor the led pin
  out   PORTD, temp1
  ldi   temp2, 0x64  ;Delay 5ms * 100
led_toggle_0:
  dec   temp2
  rcall delay5ms
  brcc  led_toggle_0
  ret
...

[Tipp (Gast)]

Warum asm? Schreibs in C.

[Felix (Gast)]

Kein Pullup? Im Programm wird der interne Pullup nicht genutzt.

[M.K. B. (mkbit)]

Es kann an der falschen Beschaltung oder Programmierung des Tasters oder 
der LED liegen. Versuch doch erstmal, ob die LED ohne Taster direkt 
leuchten kann.

[S. Landolt (Gast)]

> An der Delay5ms Funktion liegt es nicht!

Ich würde trotzdem gerne sehen, dass in ihr tatsächlich SREG 
gesichert&wiederhergestellt wird.

[S. Landolt (Gast)]

PS:
Die Befehlsreihenfolge nach §led_toggle_0 ist nämlich absolut unüblich; 
und ich vergaß: dec setzt ohnehin das Carry nicht.

[S. Landolt (Gast)]

PPS:
>   sbrc  temp1, (1 << BTN) ;Check if button is pressed
ist auch falsch:
  sbrc  temp1, BTN ;Check if button is pressed

[Sascha W. (sascha-w)]

S. Landolt schrieb:
> PS:
> Die Befehlsreihenfolge nach §led_toggle_0 ist nämlich absolut unüblich;
> und ich vergaß: dec setzt ohnehin das Carry nicht.
Und mit Sicherheit wird Delay5ms die Flags auch ändern

@ Leie Jonas
also wenn dann so ...

led_toggle_0:
  rcall delay5ms
  dec   temp2
  brne  led_toggle_0

Sascha

[Ben B. (Firma: Funkenflug Industries) (stromkraft)]

> Warum asm? Schreibs in C.
Warum nutzt Du C? Schreib doch alles in Assembler.
Oder noch besser: Bau es in Hardware!

Edit:

Schalte den Taster vom AVR-Pin nach GND und aktive den Pullup:
- Bit im DDR-Register auf Eingang
- Bit PORT-Register auf 1.

[Leie Jonas (Gast)]

S. Landolt schrieb:
> PPS:
>>   sbrc  temp1, (1 << BTN) ;Check if button is pressed
> ist auch falsch:
>   sbrc  temp1, BTN ;Check if button is pressed

Habe dein Beitrag leider zu schnell überflogen.
Mir ist es erst hinterher beim mehrfachem Debuggen aufgefallen, dass man 
keine Bitmaske übergibt.
Trotzdem danke, dass war halt das einzige Problem.

Ben B. schrieb:
> Oder noch besser: Bau es in Hardware!

hahaha ich kann nicht mehr, immer diese Trolls

[Leie Jonas (Gast)]

Nachtrag:
Auch wenn das Programm jetzt fehlerfrei ist, so richtig scheint das dann 
doch nicht zu funktionieren.
Etwa bei jedem 3 Drücken der Taste schaltet die Led viel zu schnell.
Ich habe es auch mit längeren Delays probiert aber, hilft nur teilweise.
Egal wie kurz ich auf den Taster drücke, es ändert nichts an dem 
Problem.

[Ben B. (Firma: Funkenflug Industries) (stromkraft)]

Kannst Du bitte das Programm nochmal posten wie Du es jetzt hast?

Ist der Fehler immer periodisch, also immer beim 3. mal aktivieren?

[Leie Jonas (Gast)]

Ben B. schrieb:
> Kannst Du bitte das Programm nochmal posten wie Du es jetzt hast?
>
> Ist der Fehler immer periodisch, also immer beim 3. mal aktivieren?

Ja gerne.
https://pastebin.com/yJTW9LSx

[Ben B. (Firma: Funkenflug Industries) (stromkraft)]

Zeile 32 und 33 vertauschen

[Heinz R. (Gast)]

S. Landolt schrieb:
> An der Delay5ms Funktion liegt es nicht!

>>Ich würde trotzdem gerne sehen, dass in ihr tatsächlich SREG
gesichert&wiederhergestellt wird.

Denk auch darüber nochmal nach.

[Ben B. (Firma: Funkenflug Industries) (stromkraft)]

In den meisten Funktionen macht es keinen Sinn, SREG zu sichern. Man 
führt eigentlich keine Funktionen zwischen Vergleich und Sprungbefehl 
oder wo immer man die Flags braucht aus.

Wo das aber zwingend erforderlich ist, sind Interruptfunktionen. Die 
können zu jeder Zeit aufgerufen werden ohne daß man weiß wo, also auch 
mitten in Vergleichen oder Rechen-/BitShift-Operationen mit Carry-Flag. 
Das führt dann zu sehr komischen Fehlern.

[Leie Jonas (Gast)]

Ben B. schrieb:
> Zeile 32 und 33 vertauschen

Thats it
Die SREG abzusichern, darauf wäre ich nie gekommen.
Zum Glück bin ich diesem Fehler heute begegnet.

Heinz R. schrieb:
> S. Landolt schrieb:
>> An der Delay5ms Funktion liegt es nicht!
>
>>>Ich würde trotzdem gerne sehen, dass in ihr tatsächlich SREG
> gesichert&wiederhergestellt wird.
>
> Denk auch darüber nochmal nach.

Als ich SREG las, dachte ich mir erstmal nichts dabei, weil ich es für 
einen normalen GP-Register gehalten habe. Danke dass du mich darauf 
aufmerksam gemacht hast

Ben B. schrieb:
> Wo das aber zwingend erforderlich ist, sind Interruptfunktionen.

Werde ich mir merken

[Ben B. (Firma: Funkenflug Industries) (stromkraft)]

So schwer ist's aber nicht

push r16
in r16, SREG
push r16
[...]
pop r16
out SREG,r16
pop r16
ret oder reti

[Tipp (Gast)]

Ben B. schrieb:
> Warum nutzt Du C? Schreib doch alles in Assembler.
> Oder noch besser: Bau es in Hardware!

Fährst du wie fred Feuerstein Auto? Die Erde dreht sich weiter, auch 
wenn du es nicht merkst!

[J. T. (chaoskind)]

Tipp schrieb:
> Ben B. schrieb:
>> Warum nutzt Du C? Schreib doch alles in Assembler.
>> Oder noch besser: Bau es in Hardware!
>
> Fährst du wie fred Feuerstein Auto? Die Erde dreht sich weiter, auch
> wenn du es nicht merkst

Wer asm kann, kennt seinen Controller vernünftig ;-).