[AVR|C] Codeschloss Projekt - wie den Code verbessern

[J. W. (jw-lighting)]

Hallo,

in diesem Thread geht es mir vorallem darum, anhand dieses Projektes die 
Mikrocontrollerprogrammierung zu erlernen und ein wenig Fachwissen 
heraus zu bilden. Im Vordergrund steht also das 'Learning by doing', mir 
geht es gar nicht so sehr darum eine funktionierende Hard- und Software 
zu erhalten.
Wie ich gesehen habe, gibt es ein solches Projekt auch schon hier als 
'Fertiglösung'. Das möchte ich bewusst nur als Anreiz nehmen, und 
erhoffe mir aus der eigenen Entwicklung vorallem einen Lernprozess.
Meine Vorkenntnisse habe ich aus einem Praktikum vor 2 Jahren, und im 
Bereich der C Programmierung durch PHP, welches ich gut beherrsche.
Soviel vorweg.

Zur Entwicklung habe ich das Pollin Evaluationboard 2.01 und das 
Addonboard 1.0. Derzeit verwende ich einen ATtiny2313.



Das Codeschloss verwendet 7 Taster um den Code einzugeben.
War der Code richtig, wird von einer roten LED auf eine grüne 
Umgeschaltet, und an einem weiteren Pin zusätzlich ein Relais für die zu 
sperrende Schaltung eingeschaltet.
Taster 3 dient dazu den eingegebenen Code zu löschen und die Schaltung 
wieder zu sperren.
Nebenbei können die LEDs 1+2 auf dem Evalboard durch die Taster 1+2 
ausgetastet werden.

In der Datei key.c findet ihr meinen bisherigen, funktionierenden Code.
Ich habe bereits versucht den ein wenig zu optimieren, scheitere 
allerdings an der Übergabe der Register an die Funktionen (zumindest 
glaube ich, das der Hund dort begraben ist).
Der 'optimierte Code' findet sich in der Datei key_new.c
Dieses Problem gelöst zu bekommen, wäre jetzt der erste Schritt.


Die Pinbelegung:
PB0-6: Taster 1-7 (Addonboard) für den Code
PB7: Relais schalten, wenn entsperrt.
PD0: Grüne LED (entsperrt)
PD1: Rote LED (gesperrt)
PD2+3: Taster 1+2 um LED 1+2 austasten
PD4: Eingegebenen Code löschen (Taster3 auf dem Evalboard)
PD5+6: LED1+2 auf dem Evalboard


Aus einem anderen Thread schon ein paar Überlegungen, was man noch 
verbessern kann (und meine Fragen dazu):

Stefan B. schrieb (/topic/83283#1587736):
>Im Quelltext selbst kann man einiges µC-typischer schreiben. Man muss
>nicht so "aasen" mit Datentypen (int) und mit Funktionen. Makros oder
>inline Funktionen sind meist günstiger. Die Bitschreibweise mit
>symbolischen Namen (1<<irgendwas) ist mir am liebsten und am besten wird
>das durchgängig angewendet.

Wie genau ist das gemeint 'mit Datentypen und Funktionen 'aassen''?
Meinst du ich soll ruhig weniger Funktionen verwenden?
Die Funktionen bis() und bic() kann man sich sicherlich sparen, wie 
steht es aber mit z.B. get_toogle() oder get_ls_high_bit() aus? Ich 
finde es ziemlich unpraktisch den Code immer neu zu schreiben. Wenn es 
anders üblich ist, muss ich mich da wohl anpassen ;).


>Zur Hardware - die meiste Zeit ist das Codeschloss sicher "inaktiv",
>d.h. man kann über einen Sleep-Modus in der Software nachdenken
>insbesondere, wenn das Codeschloss per Akku/Batterie versorgt wird. Das
>Relais sollte selbsthaltend sein. Und die Sperr-LED sollte im Sleepfall
>leuchten und von AVR aktiv ausgeschaltet werden.
>
>Wenn das Codeschloss an eine Tür soll, dann den Notfall einplanen. Wie
>bekommt man die Tür im Fluchtfall beim Stromausfall auf.
>
>Wenn das Codeschloss an einen Tresor oder eine Schatztruhe soll, dann
>überlegen, wie man die Kiste aufbekommt, wenn die Batterie/Akku leer.

Batteriebetrieb ist erstmal nicht geplant. Diese Überlegungen gehen 
schon ein wenig weiter, sollten aber nicht vernachlässigt werden. 
Momentan ist es mir erstmal wichtiger, das Problem mit der 
Registerübergabe zu klären, und mich mal an eine Ausgabe über das LCD zu 
machen. Dazu würde ich dann einen ATmega16 verwenden wollen. Diese 
Überlegungen sollten wir dann später nochmal aufgreifen.



Ich danke schonmal allen, die mir hier auf meinem Weg helfen wollen.

LG:
Jan Wiese

[Peter D. (peda)]

Jan W. schrieb:
> Wie genau ist das gemeint 'mit Datentypen und Funktionen 'aassen''?
> Meinst du ich soll ruhig weniger Funktionen verwenden?

Auf keinen Fall, man sollte ruhig die einzelnen Schritte in Funktionen 
unterteilen.

Aber du verwendest für alle Variablen 16Bit, was auf nem 8Bitter 
ziemliche Umstände macht.
Daher alle Variablen im Bereich [0..255] bzw. -128..127 als [u]int8_t.

> und mich mal an eine Ausgabe über das LCD zu
> machen. Dazu würde ich dann einen ATmega16 verwenden wollen.

Ein Text-LCD schafft der ATtiny2313 auch.


Peter

[J. W. (jw-lighting)]

Peter Dannegger schrieb:
> Auf keinen Fall, man sollte ruhig die einzelnen Schritte in Funktionen
> unterteilen.

Dann haben wir jetzt schon 2 Meinungen. Wie krieg ich das den hin mit 
den Registern in den Funktionen? Ich habe vermutlich einfach zu wenig 
Fachkenntniss in C. Der AVR-GCC Artikel hilft mir nicht weiter, dort 
habe ich mir schon so manches abgeguckt.

Peter Dannegger schrieb:
> Aber du verwendest für alle Variablen 16Bit, was auf nem 8Bitter
> ziemliche Umstände macht.
> Daher alle Variablen im Bereich [0..255] bzw. -128..127 als [u]int8_t.

autsch... das wusste ich nicht. Ich bin davon ausgegangen 'int' ist ein 
8Bit Integer...
Kennt jmd. ein gutes C-Buch, was nicht auf die Windoof-Programmierung 
aus ist?
Dann werde ich das mal als erstes abändern.
Danke ;)

Peter Dannegger schrieb:
> Ein Text-LCD schafft der ATtiny2313 auch.

Klar. Aber in Zusammenhang mit der Tastatur und den Anzeige LEDs habe 
ich nicht mehr genügend I/Os ;)

[Peter D. (peda)]

Ich glaub nicht, daß Dein Problem irgendwelche Register sind.

Es wird das Hauptproblem Nr.1 sein, also das Entprellen:

Beitrag "Entprellen für Anfänger"

Du hast 9 Tasten, also schreib ne Funktion, die 1..9 zurück gibt bzw. 0 
für keine Taste.
Und damit kannst Du dann bequem in einem Switch-Ausduck die nötigen 
Aktionen machen.


Peter

[J. W. (jw-lighting)]

Naja, ich habe hier eigentlich nur 7 Tasten, von denen ich bisher 6 
nutze.

Meine Funktion dafür ist:

int get_ls_high_bit(const volatile int *PINX){ // get less significant high bit
  if(get_toggle(*PINX, 0)){
    return 1;
  }
  else if(get_toggle(*PINX, 1)){
    return 2;
  }
  else if(get_toggle(*PINX, 2)){
    return 3;
  }
  else if(get_toggle(*PINX, 3)){
    return 4;
  }
  else if(get_toggle(*PINX, 4)){
    return 5;
  }
  else if(get_toggle(*PINX, 5)){
    return 6;
  }
  else if(get_toggle(*PINX, 6)){
    return 7;
  }
  /*
  else if(get_toggle(*PINX, 7)){
    return 8;
  }*/
  else{
    return 0;
  }
}

Das Entprellen habe ich in der Funktion get_toggle schon bedacht, und es 
hat bisher auch immer so funktioniert. Erst seit dem Umschreiben (s. 
key.c -> key_new.c) funktioniert es nichtmehr. Daher denke ich eher das 
es daran liegt das beim Übergeben des Registers an die Funktion nur eine 
Kopie des aktuellen Zustandes angefertigt wird, die sich folglich nicht 
mehr - wie das Register selbst - ändert.

Hier nochmal get_toogle:

int get_toggle(const volatile int *PINX, int BIT){
  int i;
  if(*PINX & (1<<BIT)){
    while(*PINX & (1<<BIT)){ // wait
      mainwork();
    }
    for(i=0;i<=255;i++){ // zeit zum entprellen
      mainwork();
    }
    return 1;
  }
  else{
    return 0;
  }
}

mainwork() ist der Teil der sich um LED 1+2 und die beiden Taster 
kümmert. kann aber auch eine andere Aufgabe sein, die während des 
entprellens weiter ausgeführt werden muss.

Wenn man davon ausgeht, dass man den Taster auch beim einschalten 
entprellen will, müsste man die For-Schleife auch vor die While-Schleife 
setzen - ist ja nicht das Ding. kann aber auch gut sein, das ich dabei 
noch etwas nicht bedacht habe.

[Stefan B. (stefan)]

Frag N Leute und du bekommst N+1 Antworten :)

Bei den Funktionen fange ich besonders dann an zu überlegen, wenn ich 
viele Sachen Copy&Paste&Modify programmieren muss. Dann ist was komisch 
und ich überlege, ob es eine Rechenvorschrift geben könnte, die in 
weniger Schritten  ein Ergebnis liefert. Eine solche CPM-Stelle in 
deinem Quelltext ist IMHO die überflüssige Zwischenfunktion zwischen 
main() und get_toggle().

Anderseits: Vorzeitige Optimierung beim Quelltextschreiben hat 
Roesenpotenzial Probleme in den Quellcode zu schauffeln. Ein gut 
lesbares Programm ist einem ultrakniffeligen vorzuziehen.

Vor dem LCD würde ich versuchen die UART hinzubekommen. Die ist 
einfacher in den Griff zu bekommen und die Hardware ist ja auf dem 
Pollinboard drauf. Die UART kann man hervorragend für Ein- und Ausgaben 
beim Debuggen benutzen. IMHO ist eine serielle Verbindung im 
Entwicklungsstadium mehr Wert als das LCD.

In den Kopfkommentar deiner Quelltexte solltest du die wichtigsten 
Hardwareparameter aufnehmen. Also mindetsens AVR-Typ, Taktrate und 
eventuell noch Targetboard und besondere Anschlussbelegungen. Das 
erleichtert Dritten den Einstieg, wenn sie nicht zwischen Diskusson und 
Quellcode wechseln müssen,

[J. W. (jw-lighting)]

Problem beim UART: Mein seriell Anschluss ist an meinen Desktop PC 
hinüber. Ich flashe ja jetzt immer mit meinem alten Server.
Wenn ich mir den Artikel zum LCD hier anschaue, sieht das gar nicht so 
kompliziert aus... kann man ja schon fast als Framework einsetzen.

get_toogle() und get_ls_high_bit() habe ich getrennt um auf get_toogle() 
auch später nochmal zurückgreifen zu können.
Mehr Kommentare mit den Angaben zum verwendetem Controller macht Sinn.

Vielen Dank soweit leider haben wir das Problem immer noch nicht gelöst.

LG

[Stefan B. (stefan)]

Jan W. schrieb:
> Problem beim UART: Mein seriell Anschluss ist an meinen Desktop PC
> hinüber. Ich flashe ja jetzt immer mit meinem alten Server.
> Wenn ich mir den Artikel zum LCD hier anschaue, sieht das gar nicht so
> kompliziert aus... kann man ja schon fast als Framework einsetzen.
>
> get_toogle() und get_ls_high_bit() habe ich getrennt um auf get_toogle()
> auch später nochmal zurückgreifen zu können.
> Mehr Kommentare mit den Angaben zum verwendetem Controller macht Sinn.
>
> Vielen Dank soweit leider haben wir das Problem immer noch nicht gelöst.

Dann räume mal als erstes die beiden unterschiedlichen Varianten auf, 
mit denen du auf IO-Variablen zugreifst.

Tipp #1:

bis() und bic() sind "anders" als im AVR-GCC-Tutorial unter 
http://www.mikrocontroller.net/articles/AVR-GCC-Tutorial#IO-Register_als_Parameter_und_Variablen 
beschrieben. Die beiden anderen Funktionen, die mit IO-Registern 
arbeiten, hast du besser geschrieben.

Tipp #2:

Kommen in ähnlichen Zeilen wie folgender keine Warnungen vom GCC? Hast 
du Warnungen eingeschaltet?

  if(get_toggle(*PINX, 0)){

Vergleich mal wie du get_toggle() aus main aufrufen würdest und wie du 
es hier machst. Dran denken in der Zwischenfunktion hast du bereits 
einen Pointer.

Tipp #3:

Funktionsprototypen konnen Probleme wie #1 und #2 erkennbar machen. In 
deinem key_new.c sind keine drin. Nachbessern :)

"Alte Hasen" schlunzen bei den Prototypen auch gerne. Solange die 
Funktionen im Quellcode definiert werden bevor der Aufruf kommt (aus 
sicht des Compilers) geht das.

[Peter D. (peda)]

Hier mal ein Schnellschuß (unvollständig), wie ich mir das vorstellen 
könnte:

// Target: ATtiny2313

#include <avr/io.h>

#define F_CPU    1e6
#include <util/delay.h>

#define  DEBOUNCE_TIME  10e-3
#include "debounce.h"

/*
PB0-6: Taster 1-7 (Addonboard) für den Code
PB7: Relais schalten, wenn entsperrt.
PD0: Grüne LED (entsperrt)
PD1: Rote LED (gesperrt)
PD2+3: Taster 1+2 um LED 1+2 austasten
PD4: Eingegebenen Code löschen (Taster3 auf dem Evalboard)
PD5+6: LED1+2 auf dem Evalboard
*/

uint8_t scan_all_keys( void )
{
  if( debounce( PINB, PB0 ))
    return 1;

  // ... insert other keys

  if( debounce( PIND, PD4 ))
    return 10;
  return 0;
}

uint8_t code[] = { 1, 2, 3, 1, 1, 7, 0 };     // 0 = end mark

int main(void)
{
  uint8_t digit = 0, false = 0;

  // ... insert init stuff

  for(;;){
    uint8_t i;

    i = scan_all_keys();
    switch( i ){
      case 1 ... 7:             // all number keys
        if( code[digit] != i )
          false = 1;            // sorry, wrong number
        digit++;                // point to next digit
        if( code[digit] == 0 ){ // last digit compared
          if( false == 0 )
            PORTD |= 1<<PD0;    // green
          else
            PORTD |= 1<<PD1;    // red
        }
        break;

      case 8:
        PORTD ^= 1<<PD2;        // toggle LED1
        break;

      case 9:
        PORTD ^= 1<<PD3;        // toggle LED2
        break;

      case 10:
        PORTD &= ~(1<<PD0);     // green off
        PORTD &= ~(1<<PD1);     // red off
        digit = 0;
        false = 0;
    }
  }
}

Das Entprellen (low aktiv) muß noch ans Pollin (high aktiv) angepaßt 
werden (die Cs an den Tasten hast Du hoffentlich schon gekillt).


Peter

[J. W. (jw-lighting)]

Hallo,

bin gard erst nach Hause gekommen, und muss morgen um 7 schon wieder im 
Betrieb sein... Betriebspraktikum.

Daher habe ich jetzt nur schnell drüber geguckt.
Ich werde eure Vorschläge dann moregen mal durcharbeiten. Ein paar von 
den Begriffen werde ich aber googeln müssen, da ist schon viel 
Fachlatein bei - was solls, muss ich ja auch lernen :p


Wie genau meinst du: Kondensatoren an den Tasten killen?
Dein Code  sieht auf den ersten Blick schon viel versprechend aus, ich 
brauche dann dringend ein gutes C-Buch. hat jmd. einen Tipp?

LG:
Jan

[Stefan B. (stefan)]

Jan W. schrieb:

> Wie genau meinst du: Kondensatoren an den Tasten killen?

http://www.mikrocontroller.net/articles/Diskussion:Pollin_ATMEL_Evaluations-Board

[J. W. (jw-lighting)]

Aja. Hatte mich eh schon gefragt, warum die dort sitzen :p
Dann muss das Display ja auch nochmal raus... was solls.

Den Code habe ich denke ich zu min. 90% geblickt, und werde wohl bis 
morgen den Rest dazu geschrieben haben. An den unterschiedlichen 
Ansätzen sieht man denke ich schon, wo ich noch lernen muss.
An diese Art Arrays zu definieren werde ich mich mächtig gewöhnen 
müssen, zumal ja anscheinend alle Werte vom selben Typ sein müssen.

Ich suche immer noch nach einem guten Buch, finde aber entweder 
überwiegend schlechte Rezessionen, einen Hammer Preis (und sehr, sehr 
viele Seiten), oder nur speziell auf die Windo(of|ws)-programmierung 
ausgelegte Bücher, die mir dann nicht alles benötigte an Wissen zu 
diesem Thema liefern.
Hat wirklich keiner einen Tipp?

[J. W. (jw-lighting)]

Hallo again,

ich habe den Code 'soweit fertig', also die noch nötigen Ergänzungen 
vorgenommen und ein wenig angepasst.

Kompiliert habe ich mit:

avr-gcc -mmcu=attiny2313 -Os -o key_peda.out key_peda.c
avr-objcopy key_peda.out key_peda.hex

unter rcos (live). Ohne Kompilerfehler.
Ich habe das Programm geflasht, und es funktioniert nicht wirklich, 
naja, gar nicht.
Das einige Funktionen abweichen (zB) die LEDs 1+2 ist klar.
Die rote LED (PD5) leuchtet.
Alle andern LEDs sind aus, auf die Tastereingabe zeigt er keine 
Reaktion.

Die Cs an den Tastern habe ich noch nicht ausgelötet, dazu bin ich 
schlicht noch nicht gekommen.

Freue mich schon auf die Fehlersuche :p

LG:
Jan

[J. W. (jw-lighting)]

EDIT: Habe anfangs die DDR Register vergessen richtig steht es in der 
Datei mit dem entsprechenden Namen ;) Hat sich aber nix am Ergebnis 
geändert.

[Peter D. (peda)]

Deine Pinverwendung für Ein- und Ausgänge stimmt nicht mit dem Kommentar 
überein.
Und beim Richtung setzen auch nicht.


Peter

[J. W. (jw-lighting)]

Hmm..

Also der Teil der PB7 abfragt muss auf jeden Fall schonmal Weg:

if( debounce( PINB, PB7 )){
  return 8;
}

Den Fehler beim Richtung Setzen erblicke ich noch nicht ..?

[Peter D. (peda)]

Jan W. schrieb:
> Den Fehler beim Richtung Setzen erblicke ich noch nicht ..?

Ja, scheint doch richtig zu sein.
Aber bei den Tasten und Ausgängen sind viele Fehler.


Peter

[J. W. (jw-lighting)]

Oje...

Muss ich das:

PORTD |= 1<<PD0;

doch so schreiben:

PORTD |= (1<<PD0);

 ??


Auf jeden Fall noch hier:

if( debounce( PIND, PD1 )){
  return 11;
}
if( debounce( PIND, PD2 )){
  return 12;
}
if( debounce( PIND, PD3 )){
  return 13;
}

ZU

if( debounce( PIND, PD2 )){
  return 11;
}
if( debounce( PIND, PD3 )){
  return 12;
}
if( debounce( PIND, PD4 )){
  return 13;
}

[Peter D. (peda)]

Jan W. schrieb:
> doch so schreiben:PORTD |= (1<<PD0);

Nö, das |= ist nachrangig:

In der folgenden Tabelle, die die Rangfolge der Operatoren darstellt, sind
die Borland C++ Operatoren in 16 Kategorien unterteilt.

Die Kategorie 1 hat die höchste Priorität; die Kategorie 2 (Unäre
Operatoren) hat die zweithöchste Priorität usw. bis zum Komma-Operator, der
die niedrigste Priorität besitzt.

Die Operatoren innerhalb einer Kategorie haben den gleichen Rang.

Die unären (Kategorie 2), konditionalen (Kategorie 14) und
Zuweisungsoperatoren (Kategorie 15) ordnen von rechts nach links zu, alle
anderen von links nach rechts.

 ════════════════Ð══════════Ð══════════════════════════════════════════
   #  Kategorie  │ Operator │ Beschreibung
 ════════════════Ï══════════Ï══════════════════════════════════════════
   1.            │    ()    │ Funktionsaufruf
                 │    []    │ Array-Subskript
                 │    ->    │ Indirekte Komponentenauswahl in C++
                 │    ::    │ Gültigkeitsbereichszugriff
                 │          │ und Zugriffsauflösung in C++
                 │     .    │ Direkte Komponentenauswahl in C++
 ────────────────┼──────────┼──────────────────────────────────────────
   2. Unär       │     !    │ Logische Negation (NOT)
                 │     ~    │ Bitweises Komplement
                 │     +    │ Unäres Plus
                 │     -    │ Unäres Minus
                 │    ++    │ Prä- oder Post-Inkrementierung
                 │    --    │ Prä- oder Post-Dekrementierung
                 │     &    │ Adresse
                 │     *    │ Umleitung
                 │  sizeof  │ Gibt die Größe des Operanden in Bytes zurück
                 │    new   │ Dynamische Speicherzuweisung in C++
                 │  delete  │ Dynamische Speicherfreigabe in C++
 ────────────────┼──────────┼──────────────────────────────────────────
   3. Zugriff auf│    .*    │ C++ Dereferenzierung
      Klassen-   │    ->*   │ C++ Dereferenzierung
      elemente   │          │
 ────────────────┼──────────┼──────────────────────────────────────────
   4. Multipli-  │     *    │ Multiplikation
      kativ      │     /    │ Division
                 │     %    │ Modulo CRest
 ────────────────┼──────────┼──────────────────────────────────────────
   5. Additiv    │     +    │ Binäres Plus
                 │     -    │ Binäres Minus
 ────────────────┼──────────┼──────────────────────────────────────────
   6. Shift      │    <<    │ Shift links
                 │    >>    │ Shift rechts
 ────────────────┼──────────┼──────────────────────────────────────────
   7. Relational │     <    │ Kleiner
                 │    <=    │ Kleiner gleich
                 │     >    │ Größer
                 │    >=    │ Größer gleich
 ────────────────┼──────────┼──────────────────────────────────────────
   8. Gleichheit │    ==    │ Gleich
                 │    !=    │ Ungleich
 ────────────────┼──────────┼──────────────────────────────────────────
   9.            │     &    │ Bitweises UND
 ────────────────┼──────────┼──────────────────────────────────────────
  10.            │     ^    │ Bitweises XOR
 ────────────────┼──────────┼──────────────────────────────────────────
  11.            │     |    │ Bitweises ODER
 ────────────────┼──────────┼──────────────────────────────────────────
  12.            │    &&    │ Logisches UND
 ────────────────┼──────────┼──────────────────────────────────────────
  13.            │    ||    │ Logisches ODER
 ────────────────┼──────────┼──────────────────────────────────────────
  14. Bedingung  │    ?:    │ (a ? x : y  bedeutet
                 │          │ ("wenn a dann x, sonst y")
 ────────────────┼──────────┼──────────────────────────────────────────
  15. Zuweisung  │     =    │ Einfache Zuweisung
                 │    *=    │ Produkt zuweisen
                 │    /=    │ Quotient zuweisen
                 │    %=    │ Rest zuweisen (Modulo)
                 │    +=    │ Summe zuweisen
                 │    -=    │ Differenz zuweisen
                 │    &=    │ Bitweises UND zuweisen
                 │    ^=    │ Bitweises XOR zuweisen
                 │    |=    │ Bitweises ODER zuweisen
                 │   <<=    │ Linksschieben zuweisen
                 │   >>=    │ Rechtsschieben zuweisen
 ────────────────┼──────────┼──────────────────────────────────────────
  16. Komma      │     ,    │ Auswerten
 ════════════════¤══════════¤══════════════════════════════════════════

Peter

[J. W. (jw-lighting)]

Gut. Vielen Dank für die Tabelle, ist abgespeichert und ausgedruckt.
Dann scheinen ja jetzt keine Fehler mehr da zu sein.
Also werde ich mich nachher mit dem auslöten der Kpondensatoren 
beschäftigen und das Programm mal kompilieren und flashen.

LG und vielen Dank

[J. W. (jw-lighting)]

So, die Kondensatoren sind gekillt (und wie...) und die Änderungen im 
Programm habe ich auch gemacht.

Ergebnis: leider immer noch negativ.
Die beiden LEDs auf dem Evalboard tun gar nichts mehr.
Die rote LED für gesperrt leuchtet.
Code im Anhang.

Sorry, das ich so lange nichts mehr geschrieben hatte, das Interesse ist 
auf jeden Fall noch da. Saß die letzten Tage immer bis min. 18Uhr in der 
Schule, wegen einer Theaterproduktion.

[Jens (Gast)]

Hallo!

Als gutes Buch kann ich Dir 'Kernighan/Ritchie - Programmieren in C' 
empfehlen.

Gruss
Jens

[Stefan B. (stefan)]

Jan W. schrieb:

> Die beiden LEDs auf dem Evalboard tun gar nichts mehr.

Richtig. Das hast du auch programmiert.

Denn

  DDRD = 0x63; // 01100011

passt nicht zu

        PORTD ^= 1<<PD2;        // toggle LED1
        PORTD ^= 1<<PD3;        // toggle LED2

Hättest du bei dem Setzen von DDRD die Makroschreibweise benutzt, wäre 
das wahrscheinlich nicht passiert.

[Karl H. (kbuchegg)]

Hmm.
Ich werde das Gefühl nicht los, dass deine beste Option darin besteht, 
erst mal den ganzen Code, so wie er jetzt ist, beiseite zu legen und 
komplett frisch von vorne zu beginnen. Aber diesmal mit etwas 
Einfacherem!

1 Taste - 1 Led

Wird auf die Taste gedrückt, geht die Led an und umgekehrt. Die Led 
leuchtet solange die Taste gedrückt ist.

Danach das debounce hinzufügen
Ein Tastendruck schaltet die Led ein. Der nächste schaltet sie wieder 
aus.

usw.

Beim einfachen beginnen und sich langsam hochtasten. Ein Codeschloss ist 
schon gar nicht mehr sooo simpel, wenn man auf nichts Funktionierendes 
zurückgreifen kann.

Das Schlimmste jedoch was du tun kannst, ist: Einen (für deine 
Verhältnisse) riesigen Code in einem Rutsch schreiben. Das geht mit 
Sicherheit schief. Und dann stehst du mit dem Haufen Code da und weißt 
nicht, wo du mit der Fehlersuche anfangen sollst.

[J. W. (jw-lighting)]

Stefan B. schrieb:
> Richtig. Das hast du auch programmiert. [...]

Ja, ja  Das sind so die Fehler, die die meiste Lernwirkung haben.
Also eher so:

        PORTD ^= 1<<PD0;        // toggle LED1
        PORTD ^= 1<<PD1;        // toggle LED2

Und vorsorglich besser:

        DDRD |= (1<<PD7)|(1<<PD6)|(1<<PD1)|(1<<PD0);

so richtig?

Jens schrieb:
> Als gutes Buch kann ich Dir 'Kernighan/Ritchie - Programmieren in C'
> empfehlen.

Danke, gucke ich mir auf alle Fälle mal an. Momentan gucke ich die 
ganzen Operatoren wie |, ~, ^ und << eigentlich mehr ab, und ich hasse 
es Sachen auswendig zu lernen und dabei nicht wirklich zu verstehen und 
logisch nachvollziehen zu können.



Karl heinz Buchegger schrieb:
> Ich werde das Gefühl nicht los, dass deine beste Option darin besteht,
> erst mal den ganzen Code, so wie er jetzt ist, beiseite zu legen und
> komplett frisch von vorne zu beginnen.

Habe ich auch schon gedacht. Ich möchte aber hier auch mit den fehlern 
lernen, und daher ruhig diesen Code so weit debuggen, das er 
funktionsfähig ist.

Karl heinz Buchegger schrieb:
> Aber diesmal mit etwas Einfacherem!

Ich hatte ja jetzt zwischendurch eine etwas längere Pause, dabei ist 
auch wieder eine ganze Menge Gelerntes verloren gegangen.

Wenn dieser Code soweit läuft (bis dahin wollte ich ja aus den Fehlern 
hier lernen), ist das daher sicher sinnvoll.
Und dann möchte ich auch mal Assembler und C parallel verwenden, um die 
Vorgänge im µC etwas besser zu blicken.
Da ich von Assembler irgendwo zwischen 0 und 1/2 Plan habe, wird das 
auch zwangsweise etwas Simples.

Vielen Dank für die Antworten.
Ich werde jetzt gleich die Änderungen einarbeiten und neu kompilieren.

LG:
Jan

[J. W. (jw-lighting)]

Und ich habe natürlich wieder gleich Fehler gemacht:

        PORTD ^= 1<<PD0;        // toggle LED1
        PORTD ^= 1<<PD1;        // toggle LED2

muss statt PD0 und PD1, PD5 und PD6 sein.

        DDRD |= (1<<PD7)|(1<<PD6)|(1<<PD1)|(1<<PD0);

muss statt PD7 und PD6, PD6 und PD5 sein.
Der ganze Code kommt dann mit der Rückmeldung, ich kann beim Ändern des 
Beitrags hier irgendwie keine Anhänge mehr mit abändern und hinzufügen.

[Karl H. (kbuchegg)]

Jan W. schrieb:
> Und ich habe natürlich wieder gleich Fehler gemacht:
>
>

>         PORTD ^= 1<<PD0;        // toggle LED1
>         PORTD ^= 1<<PD1;        // toggle LED2
>

>
> muss statt PD0 und PD1, PD5 und PD6 sein.
>
>
>

>         DDRD |= (1<<PD7)|(1<<PD6)|(1<<PD1)|(1<<PD0);
>

>
> muss statt PD7 und PD6, PD6 und PD5 sein.


Damit du diesen Fehler nicht noch einmal machst

#define LED_1       PD5
#define LED_2       PD6


....

        DDRD |= (1<<LED_1) | (1<<LED_2) | (1<<PD1)|(1<<PD0);

....


        PORTD ^= 1<<LED_1;        // toggle LED1
        PORTD ^= 1<<LED_2;        // toggle LED2

Jetzt ist die Zuordnung der LED zum Portpin an EINER einzigen Stelle 
zusammengefasst.
* Ändert sich die Zuordnung, muss nur an einer Stelle geändert werden
* im restlichen Code arbeitet man mit LED Pins, und nicht mehr mit Pin
  Nummern, was die Fehlerwahrscheinlichkeit verringert

Wenn es eine logische Zuordnug oder Funktion der Leds gibt, dann ist es 
noch besser, diese Funktion auch im #define Ausdruck zu verleihen

#define LED_LOCKED       PD5
#define LED_OPEN         PD6


....

        DDRD |= (1<<LED_LOCKED) | (1<<LED_OPEN) | (1<<PD1)|(1<<PD0);

....


        PORTD ^= 1<<LED_LOCKED;       // toggle LED1
        PORTD ^= 1<<LED_OPEN;        // toggle LED2

(nur um jetzt einfach mal ein paar Namen zu erfinden. Ich weiss schon 
das LED1 und LED2 bei dir nicht 'SChloss zu' bzw. 'Schloss offen' 
anzeigen.

Beachte auch wie der Code

        PORTD ^= 1<<LED_LOCKED;       // toggle LED1
        PORTD ^= 1<<LED_OPEN;        // toggle LED2

den Kommentar völlig überflüssig macht. Ganz im Gegenteil, der Code 
erzählt mir jetzt sogar mehr als das was im Code steht. Im Code steht 
jetzt dass da eine LED getoggelt wird und auch welche LED das ist 
(welche Funktion sie erfüllt). Und das hat dann schon eine ganz andere 
Code-Qualität.

[J. W. (jw-lighting)]

Danke,

das werde ich dann gleich umsetzen.
Das Ergebnis ist jetzt schon besser: Das ein-/ausschalten der beiden 
LEDs funktioniert jetzt auch.
Das Schloss selber erkennt den Code aber noch nicht :(
Die Hardware habe ich gerade schon gründlich durchgecheckt.

Im Anhang den aktuellen Code. Beim kompilieren hatte ich die Konstanten 
jedoch noch nicht drin.


dann würde ich gerne noch wissen, ob ich beim Aufruf des Kompiliers noch 
etwas verbessern kann, oder Sachen untüpisch sind. Aktuell:

avr-gcc -mmcu=attiny2313 -Os -o name.out name.c

LG:
jan

[Karl H. (kbuchegg)]

Jan W. schrieb:

> Das Schloss selber erkennt den Code aber noch nicht :(

Dan würde ich vorschlagen die benutzt wenisgtens diesmal die Strategie 
von oben: Vereinfachen!

Anstelle von 25 Codezahlen die stimmen müssen, benutze nur 1!
Dann kannst du in Gedanken deinen Source mal durchgehen und dir 
überlegen, ob die Logik stimmen kann.
Wenns mit 1 funktioniert, dann den Code auf 2 Stellen erweitern. Wieder: 
durchspielen und überlegen. Ruhig auch mal auf dem Papier Computer 
spielen und händisch den Source Code durchackern, wenn die richtigen 
bzw. die falschen 2 Tasten hintereinander gedrückt werden.

[J. W. (jw-lighting)]

Danke, finde ich super, das ich auch gleich gute Methoden für's 
Debugging mitlernen kann ;)

Werde ich mal machen, wahrscheinlich finde ich den Fehler dann ja sogar 
selbst. Ich melde mich wieder ;)

LG
Jan

[Karl H. (kbuchegg)]

Jan W. schrieb:

> Werde ich mal machen, wahrscheinlich finde ich den Fehler dann ja sogar
> selbst.

Genau das ist das Ziel der Übung :-)

Gib einem Mann einen Fisch und er kann einen Tag lang essen.
Zeige ihm wie man fischt, und er hat sein Leben lang zu essen.

[J. W. (jw-lighting)]

So.

Ich habe die Codefolge erstmal auf eine Zahl verkürzt.
Egal welchen Pin für die Nummern ich nach einem Reset auch auf high 
setze, er ist und bleibt gesperrt.
Auch ein strukturiertes Durchsehen des Codes hat mich nicht auf den 
Fehler gebracht.

Dann habe ich mir meine key_new.c nochmal geschnappt und nach euren 
Kriterien verändert:

1. Funktionen so anordnen, dass sie in der Reihenfolge im Quelltext 
stehen, wie Sie vom Compiler aufgerufen werden.
2. bis() und bic() entfernt
3. Die Schreibweise mit |=, statt Hexcode verwendet
4. Den Pins durch Konstanten sinnvolle Namen gegeben
5. Durch die Fehlermeldungen des avr-gcc habe ich es dann auch 
geschafft, die Pointer zum funktionieren zu bringen.

Kompiliert (war mein Aufruf mit Parametern richtig (s. vorheriger 
Post)??), geflasht, ausprobiert. Siehe da: positiv! Es funktioniert.

Dann wollte ich die mainwork()-Funktion von damals, die nicht schaltet, 
sondern tastet umschreiben sodass sie tastet.
Dadurch hätte sich die Funktion dann wieder selber aufgerufen... (ich 
habe die Zwischenschritte leider überschrieben)
... ich habe es dann nochmal mit eigenen Endprellfunktionen versucht ...
... aber letztendlich get_toggle() durch das debounce() Makro ersetzt, 
sodass ich mir damit auch keine Gedanken über Pointer mehr machen 
musste.
Auch das Endergebnis funktioniert, und findet sich wie immer im Anhang.

Die Version von peda wird durch die mir teilweise unter C noch nicht 
genug erschlossene Logik Fehler haben. Denkbar wäre für mich 
beispielsweise eine andere Syntax für switch als ich es von PHP her 
kenne.

Nun stellt sich die Frage, ob ich dieses Projekt weiter optimiere, 
vielleicht mal Timer/PWM, Watchdog, ADC, EEPROM mit einbeziehe, oder 
erstmal etwas einfacheres in C und Assembler parallel entwickle.
Ich habe bereits eine Erweiterungsschaltung für mein 
Meanwell-Schaltnetzteil im Kopf, für die ich gerne einen ATtiny12 
verwenden möchte.
Sicher nicht der Einstiegs AVR, wäre dann aber auch etwas was wirklich 
einen Sinn ergibt.

LG und großes Danke:
Jan

[J. W. (jw-lighting)]

Ach Mist, der ATtiny12 ist ja gar nicht mit dem GCC kompatibel.
Nächstes mal google ich bevor ich bestelle :p

Gibt es keine Kommentare zum Code?
Wie soll ich jetzt am Besten weitermachen, um mal zu einen µC-Freak zu 
werden? :D

LG

[Karl H. (kbuchegg)]

Das

uint8_t get_ls_high_bit(uint8_t PINX){
  if(debounce(PINX, 0)){
    return 1;
  }
  else if(debounce(PINX, 1)){
    return 2;

geht mit Sicherheit nicht.

Die debounce-'Funktion' ist darauf angewiesen, den richtigen Port zu 
bekommen, nicht eine Variable mit dem momentanen Port Inhalt. Ich hab 
mir jetzt nicht die Beschreibung dazu durchgelesen, aber die 'Funktion' 
ist ein Wolf im Schafspelz. Sie sieht zwar aus wie eine Funktion, ist es 
aber nicht.

[J. W. (jw-lighting)]

Argh... und warum funktioniert das dann?? Ist das wieder einer von 
diesen mysterischen Effekten, wie mit der roten LED und dem 
Schreibtisch??

Wärs besser, wenn ich wieder einen Pointer verwende? Wie genau muss ich 
eigentlich mit so einem Pointer umgehen, und wie mache ich eine Variable 
zu einem Pointer? (Ich weiß, ich weiß. C-Buch lesen)

LG

[Karl H. (kbuchegg)]

Jan W. schrieb:
> Argh... und warum funktioniert das dann??

Was?
Das Codeschloss?

Mit der debounce-'Funktion'

#define debounce( port, pin )            \
({                  \
  static uint8_t flag = 0;           /* new variable on every macro usage */  \
  uint8_t i = 0;                     \
                                     \
  if( flag ){                        /* check for key release: */    \
    for(;;){                         /* loop ... */        \
      if( !(port & 1<<pin) ){        /* ... until key pressed or ... */  \
        i = 0;                       /* 0 = bounce */      \
        break;                       \
      }                              \
      _delay_us( 98 );               /* * 256 = 25ms */      \
      if( --i == 0 ){                /* ... until key >25ms released */  \
        flag = 0;                    /* clear press flag */      \
        i = 0;                       /* 0 = key release debounced */    \
        break;                       \
      }                              \
    }                                \
  }else{                             /* else check for key press: */    \
    for(;;){                         /* loop ... */        \
      if( (port & 1<<pin) ){         /* ... until key released or ... */  \
        i = 0;                       /* 0 = bounce */      \
        break;                       \
      }                              \
      _delay_us( 98 );               /* * 256 = 25ms */      \
      if( --i == 0 ){                /* ... until key >25ms pressed */  \
        flag = 1;                    /* set press flag */      \
        i = 1;                       /* 1 = key press debounced */    \
        break;                       \
      }                              \
    }                                \
  }                                  \
  i;                                /* return value of Macro */    \
})

Die kann nicht funktionieren.
Du hast nur Glück, dass PeDa ein guter Programmierer ist, sodass sein 
Makro zumindest nicht in einer Endlosschleife hängt, wenn es falsch 
benutzt wird und er die Schleifendurchlaufzeiten so dimensioniert hat, 
dass es für die meisten Taster reichen wird.

Mit Entprellen hat das allerdings nichts mehr zu tun.

Und warum das überhaupt funktioniert, so wie du das benutzt, muss ich 
mir sowieso noch mal genauer ansehen. Denn so wie ich das sehe, müsste 
sich der Code eigentlich ein einer Endlosschleife fangen, aus der er nie 
wieder rauskommt, weil sich die Pin Zustände innerhalb der 
Endlosschleifen bei dir eben nicht ändern können.

[J. W. (jw-lighting)]

Also mehr ein Glücksfall durch gutes programmieren..

Und wie sieht es aus, wenn ich nun Pointer verwende? Hilft das? Oder 
kann ich das Makro gar nicht in einer anderen Funktion einsetzen?

[Karl H. (kbuchegg)]

Jan W. schrieb:
> Also mehr ein Glücksfall durch gutes programmieren..
>
> Und wie sieht es aus, wenn ich nun Pointer verwende? Hilft das? Oder
> kann ich das Makro gar nicht in einer anderen Funktion einsetzen?



Du kannstz es schon einsetzen. Aber dann muss die Funktion das richtig 
machen. Verwende in deiner Abfragefunktion den PINB direkt und lass dir 
den nicht übergeben.
Das ist die einfachste Variante.

[Peter D. (peda)]

Karl heinz Buchegger schrieb:
> Dasuint8_t get_ls_high_bit(uint8_t PINX){
>   if(debounce(PINX, 0)){
>     return 1;
>   }
>   else if(debounce(PINX, 1)){
>     return 2;
> geht mit Sicherheit nicht.

Naja sie funktioniert schon noch, sie entprellt nur nicht mehr so gut.
Der Pin wird nur einmal eingelesen und dann das Delay ausgeführt.

Ich sollte sie wohl besser groß schreiben, damit man sieht, daß es ein 
Macro ist.


Peter

[J. W. (jw-lighting)]

Gut, wenn ich die selbe Funktion an mehreren Registern (müssen ja nicht 
immer Eingänge sein) anwenden will, wie mache ich das dann am 
schlausten. Was spricht den da gegen, das ich jetzt lerne wie es mit den 
Pointern genau funktioniert?

LG

[Peter D. (peda)]

Jan W. schrieb:
> Was spricht den da gegen, das ich jetzt lerne wie es mit den
> Pointern genau funktioniert?

Daß man Pointer nicht da nimmt, wo sie unsinnig sind.

Eine Entprellfunktion soll an der gleichen Stelle auch immer die gleiche 
Taste auswerten und sie nicht bunt durcheinander würfeln.
Genau deshalb wurde sie ja als Macro geschrieben.
Jede Taste erhält ihre eigene private Entprellfunktion.


Peter

[Peter D. (peda)]

Es programmiert sich deutlich angenehmer und fehlerfreier, wenn man die 
ganzen Portbytes/-bits komplett aus dem Code verbannt und in ein 
Headerfie auslagert.

Anbei mal der funktionierende Code damit.
Die LEDs und Tasten auf meinem STK500 sind allerdings low-aktiv. Aber 
das sollte zu schaffen sein.


Peter

[J. W. (jw-lighting)]

Hallo peda,

vielen Dank für deine Mühe.
Ich habe mir deinen Code mal angeschaut.
Da ich das nicht einfach abgucken, sondern auch verstehen und abändern 
können möchte, werde ich hier ohne deutlich bessere Kenntnisse in C 
nicht weiter kommen.

Wenn ich soweit bin werde ich mich hier wieder melden.

Ich arbeite mich jetzt erstmal anhand des Tutorials hier in Assembler 
ein, vor allem um ein besseres Gefühl für die Hardware zu bekommen.

LG:
jan

[Peter D. (peda)]

Frag ruhig, wenn Du was nicht verstehst.

Übrigends hätte man hier auch einen Pointer für den Zugriff auf das 
Array mit dem Code nehmen können. Aber ich wollte das Programm nicht zu 
kompliziert machen.


Peter

[J. W. (jw-lighting)]

Vor allem der Code in sbit.h ist super geil, aber für mich nicht 
erschliessbar ;)