Codeverbesserung ?

[Matthias I. (matze5)]

Hi,

ich bin grade dabei von Bascom auf C umzusteigen.
Nun hab ich folgenden Code fuer meinen Robo gebastelt.
Intressanterweise funktioniert er nicht auf WinAVR mit gcc 4.3
Unter Linux mit gcc 4.8 geht er astrein.

Er enthaelt hoechstwahrscheinlich einige Fehler.
Anbei noch der Programmablauf, Schaltplan.

 
#define F_CPU 3688000UL 
#include <avr/io.h>
#include <util/delay.h>
#include <stdlib.h>
 
int main(void) {
   DDRB |= (1 << PB0); // Ausgang setzen
   DDRB |= (1 << PB1);
   DDRB |= (1 << PB2);
   DDRB |= (1 << PB3); 

   DDRD |= (1 << PD5); //  Ausgang LED
   DDRD |= (1 << PD4); //  Ausgang Piezzo

  void long_delay(uint16_t ms)
  {
  for(; ms>0; ms--) _delay_ms(1);
  }

   while(1) {
    if ( PIND & (1 <<3) ){
    unsigned char min = 1;
        unsigned char max = 2;
        unsigned char zufall = 0;
       zufall = (rand() % (max-min+1) +min);

    PORTD |= (1 << PD5); // LED an
    PORTB |= (1 << PB0); // Rueckwaerts (default)
                PORTB &= ~(1 << PB1);
                PORTB |= (1 << PB2);
                PORTB &= ~(1 << PB3);
    long_delay(1000);

    if ( zufall == 1 ) {
          PORTB |= (1 << PB0); // Drehen (default)
          PORTB &= ~(1 << PB1);
          PORTB &= ~(1 << PB2);
          PORTB |= (1 << PB3);
      long_delay(800);
    }
    else {
      PORTB &= ~(1 << PB0); // Drehen (default)
                  PORTB |= (1 << PB1);
                  PORTB |= (1 << PB2);
                  PORTB &= ~(1 << PB3);
                  long_delay(800);
    }

    }   
       else {
    PORTD &= ~(1 << PD5); // led AUS
  
    PORTB &= ~(1 << PB0); // Vorwaerts (default)
    PORTB |= (1 << PB1);
    PORTB &= ~(1 << PB2);
    PORTB |= (1 << PB3);
    }
     }
   return 0;
}

Vieleicht kann mir ja der eine oder andere ein paar Tipps geben.


Gruss Matze

[Random .. (thorstendb)]

Kommentare und Funktions-/Variablennamen auf Englisch. Liesst sich 
leichter.

[Random .. (thorstendb)]

ist das compilierbar? Funktion in einer Funktion?

Und: Klammern vernünftig setzen, 2 SPACEs pro Einrückung z.B.

[Dussel (Gast)]

Random ... schrieb:
> Kommentare und Funktions-/Variablennamen auf Englisch. Liesst sich
> leichter.
Wenn man davon absieht, dass wir in einem deutschsprachigen Forum sind.


Was funktioniert denn nicht? Lässt es sich nicht compilieren oder 
funktioniert es nicht wie es soll? Was für Fehler treten auf?

[Dussel (Gast)]

Übrigens ist das hier das falsche Forum. Vielleicht kann jemand das zum 
Verschieben melden.

[RaumenergiePotentialwirbel (Gast)]

Matthias I. schrieb:
> Vieleicht kann mir ja der eine oder andere ein paar Tipps geben.

An den Pins 3,4,5,7 und 9 von JP1 fehlen die Knotenpunkte. ;-)
Ebenso verwirren die Referenzkennzeichen IC1 und IC2, in Eagle muss 
jedes Bauteil U$1...U$x heißen. ;-)

[Julian W. (julian-w)]

evtl. wäre das hier aber im GCC Forum besser aufgehoben...

[Karl H. (kbuchegg)]

Matthias I. schrieb:

> Intressanterweise funktioniert er nicht auf WinAVR mit gcc 4.3
> Unter Linux mit gcc 4.8 geht er astrein.

Ich bin jetzt nicht im Bilde, ob es im 4.3 ein paar bekannte Bugs gab 
oder nicht.

>
>    DDRD |= (1 << PD5); //  Ausgang LED
>    DDRD |= (1 << PD4); //  Ausgang Piezzo


Wenn du dir ein paar #define machst

#define F_CPU 3688000UL 
#include <avr/io.h>
#include <util/delay.h>
#include <stdlib.h>

#define LED    (1<<PD5)
#define PIEZO  (1<<PD4)

dann kannst du durch die Textersetzung erreichen, dass sich dieser 
Original-Abschnitt hier so liest

   DDRD |= LED;
   DDRD |= PIEZO;

(es ginge auch

#define LED   PD5
#define PIEZO PD4
....

   DDRD |= (1 << LED);
   DDRD |= (1 << PIEZO);

)
was den Vorteil hat, dass du dir den Kommentar ganz offensichtlich 
sparen kannst, was an und für sich eine gute Sache ist. Hast du die Wahl 
zwischen einem Kommentar und einer Möglichkeit den Code so 
umzuformulieren, dass der Kommentar überflüssig wird, dann bevorzuge 
letzteres! Denn der Code ist immer das was compiliert wird - sprich: 
Kommentare können auch falsch sein ohne das es jemandem auffällt, der 
nicht genau überprüft ob der Kommentar mit dem Code zusammenstimmt.
Wenn ich hier im Forum Codefehler suche, dann ignoriere ich Kommentare 
zb vollständig. Eben weil sie immer wieder sich mal als falsch 
herausstellen.

>
>   void long_delay(uint16_t ms)
>   {
>   for(; ms>0; ms--) _delay_ms(1);
>   }

Funktionen in Funktionen zu definieren ist in C eigentlich nicht 
zulässig.
Aber eigentlich brauchst du gar keine Funktion long_delay. _delay_ms 
kann derartige Zeiten wunderbar ganz für sich alleine abhandlen. einfach 
_delay_ms( 1000 ); aufrufen und gut ists.

>
>    while(1) {
>     if ( PIND & (1 <<3) ){

Hier dasselbe. An deinem Portbit 3 hängt ein Taster, der vielleicht 
sogar eine Beschriftung hat (sagen wir mal "Bumper") bzw. wenn er nicht 
beschriftet ist, dann hat er zumindest eine logische Funktion (in deinem 
Fall ist das eben der Front-Bumper

....
#define FRONT_BUMPER    PD3

....


       if( PIND & ( 1 << FRONT_BUMPER ) )
...

liest sich doch gleich viel besser. Und wenn mal mehrere Bumper oder 
Tasten im Spiel sind, dann erleichtert dir ein entsprechender 
Makro-Ersatz auch das Auseinanderhalten im Code, mit welchem externen 
Bumper oder Taster du es hier an dieser Codestelle zu tun hast.

>     unsigned char min = 1;
>         unsigned char max = 2;
>         unsigned char zufall = 0;
>        zufall = (rand() % (max-min+1) +min);
>
>     PORTD |= (1 << PD5); // LED an

Das wäre dann (mit der 2.ten Makro Variante)

      PORTD |= ( 1 << LED );

... und wieder ist der Kommentar überflüssig geworden

>     PORTB |= (1 << PB0); // Rueckwaerts (default)
>                 PORTB &= ~(1 << PB1);
>                 PORTB |= (1 << PB2);
>                 PORTB &= ~(1 << PB3);

selbiges auch hier: "Benenne" deine Ausgänge je nach Funktionalität mit 
hilfe eines Makros. Du willst derartige Bezeichnungen wie PB1, PB2, aber 
auch PORTB etc. eigentlich nicht direkt im Code stehen haben!

[Johann L. (radiostar)]

> Wenn ich hier im Forum Codefehler suche, dann ignoriere ich Kommentare
> zb vollständig. Eben weil sie immer wieder sich mal als falsch
> herausstellen.

Na, immerhin steht im Kommentar, was der Code tun SOLL.

[Karl H. (kbuchegg)]

Ich würde mir auch Funktionen für die Beschaltung der MOtoren für 
"typische Bewegungsmuster" machen. Eine Funktion frü Vorwärts, eine 
Funktion für Rückwärts, eine für Rechts, eine für Links.

void Forward()
{
  PORTB &= ~(1 << PB0);
  PORTB |= (1 << PB1);
  PORTB &= ~(1 << PB2);
  PORTB |= (1 << PB3);
}

void Backward()
{
  PORTB |= (1 << PB0);
  PORTB &= ~(1 << PB1);
  PORTB |= (1 << PB2);
  PORTB &= ~(1 << PB3);
}

void TurnRight()
{
  PORTB |= (1 << PB0);
  PORTB &= ~(1 << PB1);
  PORTB &= ~(1 << PB2);
  PORTB |= (1 << PB3);
}

void TurnLeft()
{
  PORTB &= ~(1 << PB0);
  PORTB |= (1 << PB1);
  PORTB |= (1 << PB2);
  PORTB &= ~(1 << PB3);
}

int random( int min, max )
{
  return rand() % (max-min+1) + min;
}

int main()
{
  ....

  while( 1 ) {
    if ( PIND & (1 << FRONT_BUMPER) ) {

      PORTD |= (1 << LED);
      Backward();

      _delay_ms( 1000 );

      if( random( 1, 2 ) == 1 ) {
        TurnLeft();
      else
        TurnRight();

      _delay_ms( 800 );
    }

    else {
      PORTD &= ~(1 << LED);
      Forward();
   }

   return 0;

siehst du, um wieviel leichter die Hauptschleife in main() zu verfolgen 
und zu verstehen ist? Das liegt daran, dass ich mich als Leser nicht 
mehr mit den Details von zb. dem Rückwärtsfahren beschäftigen muss. Im 
Code in der Hauptschleife steht einfach Backward() und das genügt mir 
beim Lesen des Codes um zu verstehen, das an dieser Stelle im Code das 
Fahrzeug zurück setzen soll. Wenn mich die Details interessieren, wie 
dieses Rückwärts fahren funktioniert, dann kann ich mir die Funktion 
ansehen. WEnn es aber darum geht, die Logik zu verstehen, wie das 
Fahrzeug einem Hindernis ausweicht, dann muss ich nur wissen, dass das 
Fahreug mit dem Aufruf dieser Funktion rückwärts fährt.

Die in der Hauptschleife kodierte Logik des Ausweichens ist so viel 
kürzer formuliert und auch in problembezogeneren Ausdrücken, so dass es 
viel leichter fällt sich davon zu überzeugen, dass diese Logik 
eigentlich funktionieren müsste, ohne das ich ständig über irgendwelche 
technischen Details stolpere.

[Karl H. (kbuchegg)]

J. L. schrieb:
>> Wenn ich hier im Forum Codefehler suche, dann ignoriere ich Kommentare
>> zb vollständig. Eben weil sie immer wieder sich mal als falsch
>> herausstellen.
>
> Na, immerhin steht im Kommentar, was der Code tun SOLL.

Dein Wort in Gottes Ohr.
Alles schon gehabt:
Code richtig, Kommentar falsch
Code falsch, Kommentar von der Idee her richtig
Code falsch, Kommentar falsch
Code falsch, Kommentar passt überhaupt nicht dazu und redet von etwas 
ganz anderem
Code richtig, Kommentar richtig. Allerdings: Kommentar überflüssig
und natürlich auch
Code richtig, Kommentar richtig und tatsächlich hilfreich

[Sven (Gast)]

J. L. schrieb:
>> Wenn ich hier im Forum Codefehler suche, dann ignoriere ich
> Kommentare
>> zb vollständig. Eben weil sie immer wieder sich mal als falsch
>> herausstellen.
>
> Na, immerhin steht im Kommentar, was der Code tun SOLL.

Ein guter Kommentar sollte nicht nur das Was sondern das WARUM 
dokumentieren. Das was steht schon da in Form von Code.

Zum Thema verbessern: "Clean code" lesen.

[Matthias I. (matze5)]

So ich hab mal einiges nach den Vorschlaegen hier geaendert :)
Das ganze ist so etwas uebersichtlicher!
Jetzt muss ich noch den Buzzer zum Piepen bringen.
Mit Bascom ist das ja ganz einfach, so wie es wohl aussieht brauch ich 
unter C eine PWM und einen Timer.
Mal sehen ob ich das zum laufen bekomme.

#define F_CPU 3688000UL 
#include <avr/io.h>
#include <util/delay.h>
#include <stdlib.h>

#define LED (1<<PD5)
#define SPEAKER (1<<PD4)
#define FRONT_BUMPER PD3
#define MOTOR_A (1<<PB0)
#define MOTOR_B (1<<PB1)
#define MOTOR_C (1<<PB2)
#define MOTOR_D (1<<PB3)


void rueckwaerts(){
    PORTB |= (1 << PB0);
    PORTB &= ~(1 << PB1);
    PORTB |= (1 << PB2);
    PORTB &= ~(1 << PB3);
}

void vorwaerts(){
    PORTB &= ~(1 << PB0);
    PORTB |= (1 << PB1);
    PORTB &= ~(1 << PB2);
    PORTB |= (1 << PB3);
}

void links(){
    PORTB |= (1 << PB0);
    PORTB &= ~(1 << PB1);
    PORTB &= ~(1 << PB2);
    PORTB |= (1 << PB3);
}

void rechts(){
    PORTB &= ~(1 << PB0);
    PORTB |= (1 << PB1);
    PORTB |= (1 << PB2);
    PORTB &= ~(1 << PB3);
}

int main(void) {
   DDRB |= MOTOR_A; // Ausgang setzen
   DDRB |= MOTOR_B;
   DDRB |= MOTOR_C;
   DDRB |= MOTOR_D; 

   DDRD |= LED;
   DDRD |= SPEAKER;


while(1){

    if ( PIND & (1 << FRONT_BUMPER) )
    {
    unsigned char min = 1;
        unsigned char max = 2;
        unsigned char zufall = 0;
       zufall = (rand() % (max-min+1) +min);

    PORTD |= LED; // LED an
    rueckwaerts();
    _delay_ms(1000);

    if ( zufall == 1 ) {
          links();
     _delay_ms(800);
    }
    
    else {
     rechts();
           _delay_ms(800);
    }

    }   
       else {

    PORTD &= LED; // led AUS
    vorwaerts();  
    }
  }
   return 0;
}

[Falk B. (falk)]

@ Matthias I. (matze5)

>Jetzt muss ich noch den Buzzer zum Piepen bringen.
>Mit Bascom ist das ja ganz einfach, so wie es wohl aussieht brauch ich
>unter C eine PWM und einen Timer.

Kann man machen. Aber du bist ja anscheinend Mr. Jan Delay ;-)

>Mal sehen ob ich das zum laufen bekomme.

Versuchs mal mit einer gescheiten Einrückung, das kann (WILL!) doch 
keiner lesen!

http://www.mikrocontroller.net/articles/Strukturierte_Programmierung_auf_Mikrocontrollern#Formatierung_des_Quelltextes

[Matthias I. (matze5)]

Btw. hab ich ein Problem pack ich die Funktion in Main() geht alles.
Pack ich sie extra, gehts nicht.

Edit.. ausserdem hab ich ich den Falschen Code oben gepostet.. war die 
Falsche Datei :)

[Matthias I. (matze5)]

Der richtige Code:


#define F_CPU 3688000UL
#include <avr/io.h>
#include <util/delay.h>
#include <stdlib.h>
#include <motorun.h>

#define LED (1<<PD5)
#define SPEAKER (1<<PD4)
#define FRONT_BUMPER PD3
#define MOTOR_A (1<<PB0)
#define MOTOR_B (1<<PB1)
#define MOTOR_C (1<<PB2)
#define MOTOR_D (1<<PB3)

void rueckwaerts(){
    PORTB |= MOTOR_A;
    PORTB &= MOTOR_B;
    PORTB |= MOTOR_C;
    PORTB &= MOTOR_D;
}

void vorwaerts(){
    PORTB &= ~MOTOR_A;
    PORTB |= MOTOR_B;
    PORTB &= ~MOTOR_C;
    PORTB |= MOTOR_D;
}

void links(){
    PORTB |= MOTOR_A;
    PORTB &= ~MOTOR_B;
    PORTB &= ~MOTOR_C;
    PORTB |= MOTOR_D;
}

void rechts(){
    PORTB &= ~MOTOR_A;
    PORTB |= MOTOR_B;
    PORTB |= MOTOR_C;
    PORTB &= ~MOTOR_D;
}

int main(void) {
   DDRB |= MOTOR_A; // Ausgang setzen
   DDRB |= MOTOR_B;
   DDRB |= MOTOR_C;
   DDRB |= MOTOR_D;

   DDRD |= LED;
   DDRD |= SPEAKER;


while(1){

    if ( PIND & (1 << FRONT_BUMPER) )
    {
    unsigned char min = 1;
        unsigned char max = 2;
        unsigned char zufall = 0;
       zufall = (rand() % (max-min+1) +min);

    PORTD |= LED; // LED an
    rueckwaerts();
    _delay_ms(1000);

    if ( zufall == 1 ) {
          links();
     _delay_ms(800);
    }

    else {
     rechts();
           _delay_ms(800);
    }

    }
       else {

    PORTD &= ~LED; // led AUS
    vorwaerts();
    }
  }
   return 0;
}

Und die Datei motorun.h:
#ifndef MOTORUN_H_INCLUDED
#define MOTORUN_H_INCLUDED

void links();

void rechts();

void rueckwaerts();

void vorwaerts();
#endif


Tjo funktioniert nicht.
Lass ich den Header weg und pack die Funktionen mit in Main() gehts..
Verstehe ich nicht das er die Funktionen nicht aufrufen kann.

Der Weg ist lang und steinig :)

Btw. kann man im Forum irgendwie Syntax Highlight anmachen ?

[Uwe (de0508)]

Ja steht direkt in der Eingabe:

# http://www.mikrocontroller.net/articles/Formatierung_im_Forum

[Matthias I. (matze5)]

So funktionierts uebrigens.. obwohls offensichtlich falsch ist... :

#define F_CPU 3688000UL 
#include <avr/io.h>
#include <util/delay.h>
#include <stdlib.h>
// #include "motorun.h"

#define LED (1<<PD5)
#define SPEAKER (1<<PD4)
#define FRONT_BUMPER PD3
#define MOTOR_A (1<<PB0)
#define MOTOR_B (1<<PB1)
#define MOTOR_C (1<<PB2)
#define MOTOR_D (1<<PB3)


int main(void) {
   DDRB |= MOTOR_A; // Ausgang setzen
   DDRB |= MOTOR_B;
   DDRB |= MOTOR_C;
   DDRB |= MOTOR_D; 

   DDRD |= LED;
   DDRD |= SPEAKER;

void rueckwaerts(){
    PORTB |= MOTOR_A;
    PORTB ~&= MOTOR_B;
    PORTB |= MOTOR_C;
    PORTB ~&= MOTOR_D;
}

void vorwaerts(){
    PORTB &= ~MOTOR_A;
    PORTB |= MOTOR_B;
    PORTB &= ~MOTOR_C;
    PORTB |= MOTOR_D;
}

void links(){
    PORTB |= MOTOR_A;
    PORTB &= ~MOTOR_B;
    PORTB &= ~MOTOR_C;
    PORTB |= MOTOR_D;
}

void rechts(){
    PORTB &= ~MOTOR_A;
    PORTB |= MOTOR_B;
    PORTB |= MOTOR_C;
    PORTB &= ~MOTOR_D;
}


while(1){

    if ( PIND & (1 << FRONT_BUMPER) )
    {
    unsigned char min = 1;
        unsigned char max = 2;
        unsigned char zufall = 0;
       zufall = (rand() % (max-min+1) +min);

    PORTD |= LED; // LED an
    rueckwaerts();
    _delay_ms(1000);

    if ( zufall == 1 ) {
          links();
     _delay_ms(800);
    }
    
    else {
     rechts();
           _delay_ms(800);
    }

    }   
       else {

    PORTD &= ~LED; // led AUS
    vorwaerts();  
    }
  }
   return 0;
}

[Falk B. (falk)]

@ Matthias I. (matze5)

>So funktionierts uebrigens.. obwohls offensichtlich falsch ist... :

Kannst du oder willst du nicht lesen? Dein Tabulatoren sind SCHROTT!

[Falk B. (falk)]

#define F_CPU 3688000UL 
#include <avr/io.h>
#include <util/delay.h>
#include <stdlib.h>
// #include "motorun.h"

#define LED_ON      PORTD |=  (1<<PD5);
#define LED_OFF     PORTD &= ~(1<<PD5);
#define SPEAKER_ON  PORTD |=  (1<<PD4);
#define SPEAKER_OFF PORTD &= ~(1<<PD4);
#define FRONT_BUMPER (PIND & (1<<PD3))
#define MOTOR_A (1<<PB0)
#define MOTOR_B (1<<PB1)
#define MOTOR_C (1<<PB2)
#define MOTOR_D (1<<PB3)


void rueckwaerts(){
    PORTB |=  MOTOR_A;
    PORTB ~&= MOTOR_B;
    PORTB |=  MOTOR_C;
    PORTB ~&= MOTOR_D;
}

void vorwaerts(){
    PORTB &= ~MOTOR_A;
    PORTB |=  MOTOR_B;
    PORTB &= ~MOTOR_C;
    PORTB |=  MOTOR_D;
}

void links(){
    PORTB |=  MOTOR_A;
    PORTB &= ~MOTOR_B;
    PORTB &= ~MOTOR_C;
    PORTB |=  MOTOR_D;
}

void rechts(){
    PORTB &= ~MOTOR_A;
    PORTB |=  MOTOR_B;
    PORTB |=  MOTOR_C;
    PORTB &= ~MOTOR_D;
}


int main(void) {
    DDRB |= MOTOR_A;        // Ausgang setzen
    DDRB |= MOTOR_B;
    DDRB |= MOTOR_C;
    DDRB |= MOTOR_D; 

    LED_ON
    SPEAKER_OFF

    while(1){

        if ( FRONT_BUMPER )
        {
            unsigned char min = 1;
            unsigned char max = 2;
            unsigned char zufall = 0;
            zufall = (rand() % (max-min+1) +min);
    
            LED_ON
            rueckwaerts();
            _delay_ms(1000);
        
            if ( zufall == 1 )
            {
                links();
                _delay_ms(800);
            }
            else
            {
                rechts();
                _delay_ms(800);
            }
        
        }   
        else
        {
            LED_OFF
            vorwaerts();  
        }
    } 
    
    return 0;
}

[Johann L. (gjlayde)]

Random ... schrieb:
> ist das compilierbar? Funktion in einer Funktion?

Ja, lokale Fnuktionen sind eine GCC-Erweiterung, die in GNU-C zur 
Verfügung steht.  Harrig wird's, wenn die lokale Funktion auf Variablen 
der enthaltenden Funktion zugreift und diese die Adresse der lokalen 
Funktion nimmt.  Instruktives Beispiel:

extern void dispatch (void(*)(void));

int fun (int x)
{
    void callback (void)
    {
        x++;
    }
    
    dispatch (callback);
    return x;
}

callback bekommt von dispatch keine Argumente; es hat das Call-Interface 
jeder anderen void-Funktion.  Die Adresse von x ist zur Ladezeit nicht 
bekannt, und zwischen fun und callback können beliebig viele Callframes 
liegen.  Trotzdem muß callback irgendwie an x rankommen.

Mit avr-gcc ist das übrigens nicht übersetzbar, da dort kein Code auf 
dem Stack ausführbar ist.

[Matthias I. (matze5)]

Falk: mit deinen Code funktionierts auch nicht.

Das erklaert auch nicht warum das Programm nicht ablaueft wenn die 
Funktionen ausserhalb von Main sind, wo sie hingehoeren.

[Johann L. (gjlayde)]

Es war eine Erklärung und Hintergrundinfo dafür, warum der Compiler für 
long_delay keinen Fehler ausgegeben hat.

[apr (Gast)]

Matthias I. schrieb:
> Falk: mit deinen Code funktionierts auch nicht.
>
> Das erklaert auch nicht warum das Programm nicht ablaueft wenn die
> Funktionen ausserhalb von Main sind, wo sie hingehoeren.

main.c: In function ‘rueckwaerts’:
main.c:20:11: error: expected ‘;’ before ‘~’ token
     PORTB ~&= MOTOR_B;
           ^
main.c:22:11: error: expected ‘;’ before ‘~’ token
     PORTB ~&= MOTOR_D;
           ^

[Falk B. (falk)]

@ Matthias I. (matze5)

>Falk: mit deinen Code funktionierts auch nicht.

Ich hab nur schön geschrieben, nicht kompiliert oder getestet!

[Falk B. (falk)]

Ach ja, beim Aufräumen ist wohl ein neuer Bug reingekommen. Die Ausgänge 
für LED und Speaker müssen noch aktiviert werden.

[Dirk B. (dirkb2)]

Du kannst deinen Zufall noch aufräumen
Von

unsigned char min = 1;
            unsigned char max = 2;
            unsigned char zufall = 0;
            zufall = (rand() % (max-min+1) +min);

bleibt dann noch

            zufall = rand() % 2;

oder du schreibst das rand() % 2 gleich beim if rein.

[Matthias I. (matze5)]

Das hab ich schon alles gefunden und behoben, dennoch lauefts nicht :)
Speaker wird eh nicht benutzt im Moment.

Kompilieren tut es astrein, aber die funktionen laufen nur wenn sie in 
main() sind...
Das ist das einzige Problem, egal wie ich da was formatiere.. oder 
Tabulatoren setze..

[Matthias I. (matze5)]

So funktioniert alles:

#define F_CPU 3688000UL 
#include <avr/io.h>
#include <util/delay.h>
#include <stdlib.h>

#define LED (1 << PD5)
#define SPEAKER (1 << PD4)
#define FRONT_BUMPER PD3
#define MOTOR_A (1 << PB0)
#define MOTOR_B (1 << PB1)
#define MOTOR_C (1 << PB2)
#define MOTOR_D (1 << PB3)


int main(void) {
DDRB |= MOTOR_A;
DDRB |= MOTOR_B;
DDRB |= MOTOR_C;
DDRB |= MOTOR_D;

DDRD |= LED;
DDRD |= SPEAKER;

void rueckwaerts(){
  PORTB |= MOTOR_A;
  PORTB &= ~MOTOR_B;
  PORTB |= MOTOR_C;
  PORTB &= ~MOTOR_D;
}

void vorwaerts(){
  PORTB &= ~MOTOR_A;
  PORTB |= MOTOR_B;
  PORTB &= ~MOTOR_C;
  PORTB |= MOTOR_D;
}

void links(){
  PORTB |= MOTOR_A;
  PORTB &= ~MOTOR_B;
  PORTB &= ~MOTOR_C;
  PORTB |= MOTOR_D;
}

void rechts(){
  PORTB &= ~MOTOR_A;
  PORTB |= MOTOR_B;
  PORTB |= MOTOR_C;
  PORTB &= ~MOTOR_D;
}

while(1){
  if ( PIND & (1 << FRONT_BUMPER) )
  {
  unsigned char min = 1;
  unsigned char max = 2;  unsigned char zufall = 0;
  zufall = (rand() % (max-min+1) +min);
  PORTD |= LED; 
  
  rueckwaerts();
  _delay_ms(1000);
  
  if ( zufall == 1 ) {
  links();
  _delay_ms(800);
  }
  
  else {
  rechts();
  _delay_ms(800);
  }

  }   
  else {
  PORTD &= ~LED;
  vorwaerts();  
  }
  }

return 0;
}

So funktioniert nichts:

#define F_CPU 3688000UL 
#include <avr/io.h>
#include <util/delay.h>
#include <stdlib.h>

#define LED (1 << PD5)
#define SPEAKER (1 << PD4)
#define FRONT_BUMPER PD3
#define MOTOR_A (1 << PB0)
#define MOTOR_B (1 << PB1)
#define MOTOR_C (1 << PB2)
#define MOTOR_D (1 << PB3)

void rueckwaerts(){
  PORTB |= MOTOR_A;
  PORTB &= ~MOTOR_B;
  PORTB |= MOTOR_C;
  PORTB &= ~MOTOR_D;
}

void vorwaerts(){
  PORTB &= ~MOTOR_A;
  PORTB |= MOTOR_B;
  PORTB &= ~MOTOR_C;
  PORTB |= MOTOR_D;
}

void links(){
  PORTB |= MOTOR_A;
  PORTB &= ~MOTOR_B;
  PORTB &= ~MOTOR_C;
  PORTB |= MOTOR_D;
}

void rechts(){
  PORTB &= ~MOTOR_A;
  PORTB |= MOTOR_B;
  PORTB |= MOTOR_C;
  PORTB &= ~MOTOR_D;
}


int main(void) {
DDRB |= MOTOR_A;
DDRB |= MOTOR_B;
DDRB |= MOTOR_C;
DDRB |= MOTOR_D;

DDRD |= LED;
DDRD |= SPEAKER;


while(1){
  if ( PIND & (1 << FRONT_BUMPER) )
  {
  unsigned char min = 1;
  unsigned char max = 2;  unsigned char zufall = 0;
  zufall = (rand() % (max-min+1) +min);
  PORTD |= LED; 
  
  rueckwaerts();
  _delay_ms(1000);
  
  if ( zufall == 1 ) {
  links();
  _delay_ms(800);
  }
  
  else {
  rechts();
  _delay_ms(800);
  }

  }   
  else {
  PORTD &= ~LED;
  vorwaerts();  
  }
  }

return 0;
}

Ich steh irgendwie auf dem Schlauch. :o

[Unterschied (Gast)]

Matthias I. schrieb:
> So funktioniert alles:#define F_CPU 3688000UL
...

Was ist denn jetzt der Unterschied? Ich gehe ganz sicher nicht jede 
Zeile durch.

[Hans (Gast)]

Rück den Code halt wenigstens mal ordentlich ein. Die wenigsten hier 
haben Lust, erst mal die zusammengehörigen Klammern zu suchen ...

[Steffen R. (steffen_rose)]

Matthias I. schrieb:
> So funktioniert nichts:

Was heißt das genau? Welches Verhalten zeigt sich?

[Jogi Löw (Gast)]

Steffen Rose schrieb:
>> So funktioniert nichts:
>
> Was heißt das genau? Welches Verhalten zeigt sich?

Hat er doch schon geschrieben: Es funktioniert nicht. Erwartest Du jetzt 
noch Fehlermeldung (Screenshot) und dergleichen?

[Matthias I. (Gast)]

Es gibt keine Screenshots der Code compiliert nur laueft das Programm 
nicht ab.
Sobald ich das in den Tiny Flashe tut sich nichts mehr.
Der "Falsche Code" geht astrein.

Nur laueft keine Funktion die ich erstelle.
Ausser sie sind in Main().
Dann gehen sie.

[Steffen R. (steffen_rose)]

Ich erwarte eine genaue Fehlerbeschreibung.
z.B. Er fährt nur vorwärts. Oder er ändert ständig die Richtung ohne an 
den Buzzer zu kommen.

Du gehts ja auch nicht zum Arzt und sagst nur "Mir gehts nicht gut".

Daher nicht wundern, dass bei solch einer nichtssagenden 
Fehlerbeschreibung nur die Codequalität diskutiert wird. Was soll man 
auch anderes machen, als nach einer falsch gesetzte Klammer zu suchen.

[Steffen R. (steffen_rose)]

Matthias I. schrieb:
> Sobald ich das in den Tiny Flashe tut sich nichts mehr.

Was sagt die LED (an, aus, flackern)?

Du kannst Das Programm auch erstmal nur auf "vorwärts" reduzieren. Und 
dann Stück für Stück weiter.

Wenn wirklich garnicht geht, prüfen, ob main überhaupt durchlaufen wird. 
Dafür kann man die LED nutzen, wenn man sonst nichts hat.

Wie siehst mit Compilerwarnings aus? Warnungen eingeschaltet? Die 
könnten einen Hinweis geben. Oder warnt der Linker?

Ich bezweifle momentan, dass das Problem am geposteten Code liegt.

[Stefan E. (sternst)]

Steffen Rose schrieb:
> Ich bezweifle momentan, dass das Problem am geposteten Code liegt.

Vermutlich hat er ein grundlegendes Problem in seinem Build-Prozess. Bei 
der beschriebenen Symptomatik könnte ich mir z.B. vorstellen, dass er 
das ELF-File in den Controller brennt (oder sogar nur ein ungelinktes 
Object-File).

[Matthias I. (matze5)]

Also fuer Dumm verkaufen lasse ich mich sicher nicht.
Elf file flashen und so. Muss hier ja oft vorkommen.

Der Roboter macht einfach gar nichts, kein LED blinken nichts.
Wenn ich den Code mit den Funktionen in Main() als Hex brenne passt 
alles.
Wenn die Funktionen ausserhalb von Main() sind gehts nicht.
ist das so schwer zu verstehen ?
Geht nicht heisst, es geht nichts.
Da blinkt nichts, Kein Motor dreht, einfach nichts.
Der Controller zuckt kein Bisschen.

avr-gcc -mmcu=attiny2313 -Os -c motorun.c -o motorun.o
avr-gcc motorun.o -o motorun.elf
avr-objcopy -O ihex -j .text -j .data motorun.elf motorun.hex

avrdude  -p t2313 -c avrisp2 -P usb -U flash:w:motorun.hex

ich wuesste nicht was da falsch ist.

Mein Geraet funktioniert und tut auch was es soll.
Nur eben mit den unschoenen Funktionen in der Main()

Edit: wenn der Code minimal reduziert ist funktionierts auch nicht.
Also fehlt wohl irgendwas um Funktionen zu unterstuetzen.

Denn es geht auch keine simple Funktion wie:
(Ausgang natuerlich in main definiert..)

void led_an(){
PORTD |= LED;
}

[Matthias I. (Gast)]

Ahja der Compiler:

Es werden eingebaute Spezifikationen verwendet.
COLLECT_GCC=/usr/x86_64-pc-linux-gnu/avr/gcc-bin/4.8.2/avr-gcc
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/avr/4.8.2/lto-wrapper
Ziel: avr
Konfiguriert mit: /var/tmp/portage/cross-avr/gcc-4.8.2/work/gcc-4.8.2/configure --host=x86_64-pc-linux-gnu --target=avr --build=x86_64-pc-linux-gnu --prefix=/usr --bindir=/usr/x86_64-pc-linux-gnu/avr/gcc-bin/4.8.2 --includedir=/usr/lib/gcc/avr/4.8.2/include --datadir=/usr/share/gcc-data/avr/4.8.2 --mandir=/usr/share/gcc-data/avr/4.8.2/man --infodir=/usr/share/gcc-data/avr/4.8.2/info --with-gxx-include-dir=/usr/lib/gcc/avr/4.8.2/include/g++-v4 --with-python-dir=/share/gcc-data/avr/4.8.2/python --enable-languages=c,c++ --enable-obsolete --enable-secureplt --disable-werror --with-system-zlib --enable-nls --without-included-gettext --enable-checking=release --with-bugurl=https://bugs.gentoo.org/ --with-pkgversion='Gentoo 4.8.2 p1.3r1, pie-0.5.8r1' --enable-libstdcxx-time --enable-poison-system-directories --enable-shared --disable-threads --disable-bootstrap --enable-multilib --disable-altivec --disable-fixed-point --disable-libgcj --disable-libgomp --disable-libmudflap --disable-libssp --disable-libquadmath --enable-lto --without-cloog
Thread-Modell: single
gcc-Version 4.8.2 (Gentoo 4.8.2 p1.3r1, pie-0.5.8r1)

eventuell liegts ja am Compiler ?

[Stefan E. (sternst)]

Matthias I. schrieb:
> Also fuer Dumm verkaufen lasse ich mich sicher nicht.
> ...
> ist das so schwer zu verstehen ?

Kein Grund hier pampig zu werden.


Matthias I. schrieb:
> ich wuesste nicht was da falsch ist.

Code ist nicht für den richtigen Controller gelinkt.

[Matthias I. (Gast)]

Besten Dank,

tut mir leid das ich etwas ungehalten war, ich war schlichtweg entnervt.
Nun geht tatsaechlich alles wie es soll..

Gleichzeitig bin ich noch auf einen Bug im Gentoo avr-gcc gestossen
Die Linkerscripte waren nicht verlinkt.
ln -s /usr/lib/binutils/avr/2.24/ldscripts/ /usr/avr/lib/ldscripts
behebt das folgende Problem:
Cannot open linker script file ldscripts/avr25.xn: Datei oder 
Verzeichnis nicht gefunden

Und was ich nicht wusste war:
avr-gcc -mmcu=attiny2313 -O2 -c motorun.c -o motorun.o
avr-gcc -g -mmcu=attiny2313 motorun.o -o motorun.elf
avr-objcopy -O ihex -j .text -j .data motorun.elf motorun.hex

So gehts nun.

[Jogi Löw (Gast)]

Hast Deinen Code jetzt wenigstens manierlich formatiert mit korrekten 
Tabulatoren und Kommentaren? ;-)

[Matthias I. (Gast)]

Ich arbeite dran :)

dafuer das ich mir alles selbstbeigebracht hab und weder Ahnung von C 
noch Elektrik hatte ist es ganz gut geworden.

http://www.youtube.com/watch?v=TYfIhrqZPVU

[Johann L. (gjlayde)]

Matthias I. schrieb:
> Ahja der Compiler:
>
> Konfiguriert mit: --disable-fixed-point

Na prima.  Da wird mit viel Aufwand Assembler-optimerter Fixed-Point 
Support in avr-gcc eingebaut, und dann wird's einfach ausgeknipst...

[Falk B. (falk)]

@Johann L. (gjlayde) Benutzerseite

>> Konfiguriert mit: --disable-fixed-point

>Na prima.  Da wird mit viel Aufwand Assembler-optimerter Fixed-Point
>Support in avr-gcc eingebaut, und dann wird's einfach ausgeknipst...

Gefrickel 3.0

Wenn man schon mti Kommandozeile und selbstgestricktem Makefile arbeiten 
will, sollte man es wenigstens können. Da lob ich mir mein AVR-Studio!

F7!

[Matthias I. (Gast)]

Wenn man immer Fertigkost zu sich nimmt.
Wird es kaum besser.
Immerhin ist der Fehler nicht meiner, sondern der des Maintainers des 
EBuilds :)