SPI zwischen zwei ATMega8

[Marcel D. (diablokiller999)]

Hi Leute!
Ich wollte für meine Studenten ein kleines Labor aufziehen, in dem sie 
zwei ATMega8 per SPI verbinden und vom Master zum Slave Signale zum 
schalten von LEDs senden. Nur leider funktioniert das bei mir nicht so 
ganz, finde den Fehler einfach nicht.

Vorgehensweise:
SPI-Master und SPI-Slave initialisieren, vom Master per Timer-Interrupt 
jede Sekunde 0x00 bzw. 0xFF senden. Diese vom Slave empfangen, 
interpretieren und entsprechend LEDs schalten.

Laut Oszi kommt aus dem Master Takt, SS und Daten raus. Mein Slave 
scheint zumindest ein erstes Mal zu funktionieren, die zwei LEDs zur 
Anzeige leuchten. Nur leider schalten sie sich nicht wieder ab, obwohl 
der Master laut Kontroll-LED und Oszi 0x00 verschickt. Vielleicht kann 
mir einer von euch helfen, kann nicht so kompliziert sein SPI auf einem 
ATMega ans laufen zu bekommen ^^

Source Code liegt bei...

[S. Landolt (Gast)]

> PORTB ^= (0<<PB2);
Ist das ein XOR mit 0?

[Karl H. (kbuchegg)]

  PORTB ^= (0<<PB2);            //SS am Slave Low --> Beginn der Übertragung

erstens wird das so nichts.
Zweitens: wenn du einen Pin auf Low setzen willst, dann schreib das auch 
genau so: Pin auf Low setzen. Und das geht auf jeden Fall nicht mit 
einem XOR

[S. Landolt (Gast)]

> PORTB ^= (1<<PB2); //SS High --> Ende der Übertragung
Klar, durch das XOR kommt das SS low dann nach der Übertragung.

[Stefan S. (sschultewolter)]

PORTB &= ~(0<<PB2);  
//...
PORTB |= (1<<PB2);

[Karl H. (kbuchegg)]

Stefan S. schrieb:

> PORTB &= ~(0<<PB2);

fast

[S. Landolt (Gast)]

> PORTB &= ~(0<<PB2);
Sind Sie sicher? Ich verstehe den Sinn der Null nicht.

[Karl H. (kbuchegg)]

S. Landolt schrieb:
>> PORTB &= ~(0<<PB2);
> Sind Sie sicher? Ich verstehe den Sinn der Null nicht.

Es gibt auch keinen.

[Marcel D. (diablokiller999)]

> PORTB &= ~(1<<PB2);
Wow, da war ich wohl Betriebsblind! Muss natürlich negierte Verundung 
sein.
Dennoch kam laut Oszi ein vernünftiges Slave Select heraus, da muss noch 
was Anderes im argen liegen...

[Stefan S. (sschultewolter)]

Ja, ist richtig, ich hab die Zeile nur von oben kopiert (die zweite 
Zeile schnell selber getippt), und da ist mir der Fehler unterlaufen!


So sollte es eigentlich sein

PORTB &= ~(1<<PB2);  
//...
PORTB |= (1<<PB2);

[Karl H. (kbuchegg)]

Marcel D. schrieb:
>> PORTB &= ~(1<<PB2);
> Wow, da war ich wohl Betriebsblind! Muss natürlich negierte Verundung
> sein.
> Dennoch kam laut Oszi ein vernünftiges Slave Select heraus, da muss noch
> was Anderes im argen liegen...

Da liegt nichts im Argen.
WIe weiter oben schon geschrieben. Der zweite Toggler funktioniert ja. 
Nur kommt der dann eben NACH der SPI Übertragung.

Kurz gesagt: du gibst dem anderen µC den Slave Select erst mal, nachdem 
alles vorbei ist. Dann überträgst du das nächste Datenbyte, welches auch 
wirklich übertragen wird. An dessen Ende toggelst du den Pin wieder, 
womit der Slave wieder abgeschaltet wird. DIe darauffolgende Übertragung 
geht wieder ins leere.

usw. usw. Dein Slave ist nur bei jedem 2.ten Byte selektiert. Und da du 
nur 2 verschiedene Bytes überträgst, kriegt der Slave immer dasselbe.

Das was mir jetzt noch auf der Zunge liegt, behalte ich erst mal für 
mich.

[Karl H. (kbuchegg)]

Das scheint bei dir Methode zu haben

  else if (spi_data == 0x00) 
  {  
    PORTC= (0<<PC0);

[S. Landolt (Gast)]

> Dennoch kam laut Oszi ein vernünftiges Slave Select heraus
Miau, Slave Select schon, aber nicht vernünftig.
Und da gibt's noch so eine dubiose Null:
> PORTC= (0<<PC0);

[Cyblord -. (cyblord)]

Mit was haben die Studenten so einen Tutor verdient? Hoffentlich zahlt 
dir niemand Geld dafür... Womöglich noch aus öffentlichen Kassen? Na 
Danke!

[Karl H. (kbuchegg)]

Das ist auch Quatsch

void init_master()
{
...
  SPCR = (1<<7) | (1<<6) | (1<<4);      //Aktiviere SPI,SPI Interrupt und SPI Master

...

void master_transmit (uint8_t spi_data) {
....
  SPDR = spi_data;            //Schreiben der Daten

   while(!(SPSR & (1<<SPIF)));    // Warte auf verschicken des 
Bytes
....

ISR(SPI_STC_vect)
{  spi_flags = SPSR;      //Clear Register
}

wenn du die Interrupt Behandlung frei gibst, dann musst du alles über 
den Interrupt abwickeln. Mischen geht dann nicht mehr. Das folgt ganz 
klar daraus, wie Interrupts auf einem AVR funktionieren und das das 
jeweilige Interrupt aufgetreten Flag bei fast allen Interrupts mit dem 
Eintritt in die ISR automatisch gelöscht wird.


Was sagtest du? Für deine Studenten?
Wie wärs wenn du selbst erst mal die Grundlagen lernst
(Sorry, aber spätestens jetzt kann ich mich nicht mehr zurückhalten)

[Karl H. (kbuchegg)]

Cyblord -. schrieb:
> Mit was haben die Studenten so einen Tutor verdient?

Lass uns beten, dass er nur Tutor ist und nicht Assistent.

[Marcel D. (diablokiller999)]

Eher Tutor ^^
Keine Ahnung wie sich die Fehler eingeschlichen haben, danke jedenfalls 
dafür!
Bin grad dabei mich in die AVR-Welt einzuarbeiten, komme eigentlich aus 
der ARM-Welt mit dem Cortex-M3 usw.
Hab da wohl vor Eifer und Data Sheet geblätter etwas Schlampigkeit 
einfließen lassen -_-

[spess53 (Gast)]

Hi

>Bin grad dabei mich in die AVR-Welt einzuarbeiten, komme eigentlich aus
>der ARM-Welt mit dem Cortex-M3 usw.

Als notorischer Assemblerprogrammierer sieht das für mich eher nach 
massiven Defiziten in C aus.

MfG Spess

[Cyblord -. (cyblord)]

spess53 schrieb:
> Hi
>
>>Bin grad dabei mich in die AVR-Welt einzuarbeiten, komme eigentlich aus
>>der ARM-Welt mit dem Cortex-M3 usw.
>
> Als notorischer Assemblerprogrammierer sieht das für mich eher nach
> massiven Defiziten in C aus.

Zustimmung. Die Ausrede "unseres" Tutors ist sehr schlecht ;-)

[Mathias O. (m-obi)]

Jetzt weiß ich, warum ich lieber Autodidakt bin. Mochte schon früher 
Schule nicht und Uni werde ich erst recht nicht.

Da hilft auch keine Entschuldigung, dass man aus der ARM-Welt kommt.
Das ist Grundlagen Digitaltechnik.
Innerhalb von Nullen, Nullen zu schieben ist schon seltsam.

[(prx) A. K. (prx)]

Mathias O. schrieb:
> Innerhalb von Nullen, Nullen zu schieben ist schon seltsam.

Kann zwecks Dokumentation sinnvoll sein:

UCSRA = 1<<TXC | 0<<U2X | 0<<MPCM;
UCSRB = 0<<RXCIE | 0<<TXCIE | 0<<UDRIE | 0<<RXEN | 1<<TXEN | 0<<UCSZ2;
UCSRC = 1<<URSEL | 0<<UMSEL | 1<<UPM1 | 0<<UPM0 | 0<<USBS | 1<<UCSZ1 | 1<<UCSZ0 | 0<<UCPOL;

[Mathias O. (m-obi)]

A. K. schrieb:
> Mathias O. schrieb:
>> Innerhalb von Nullen, Nullen zu schieben ist schon seltsam.
>
> Kann zwecks Dokumentation sinnvoll sein:
>

> UCSRA = 1<<TXC | 0<<U2X | 0<<MPCM;
> UCSRB = 0<<RXCIE | 0<<TXCIE | 0<<UDRIE | 0<<RXEN | 1<<TXEN | 0<<UCSZ2;
> UCSRC = 1<<URSEL | 0<<UMSEL | 1<<UPM1 | 0<<UPM0 | 0<<USBS | 1<<UCSZ1 | 
> 1<<UCSZ0 | 0<<UCPOL;
>

Dafür gibts Kommentare.
Und ich kann es irgendwie nicht glauben, dass du es so z.B. machst.

[(prx) A. K. (prx)]

Mathias O. schrieb:
> Dafür gibts Kommentare.

Das sind gewissermassen Kommentare. Aber warum sollte ich hier Nullen 
und Einsen händisch sortieren, die Guten ins Töpfchen, die schlechten 
ins Kröpfchen?

> Und ich kann es irgendwie nicht glauben, dass du es so z.B. machst.

Was spricht konkret dagegen?

[S. Landolt (Gast)]

Es ist übersichtlich, und es erleichert den Wechsel z.B. bei 
Vorteiler-Bits. Ist aber ein ganz anderer Fall als bei diablokiller.

[Cyblord -. (cyblord)]

A. K. schrieb:
> Mathias O. schrieb:
>> Dafür gibts Kommentare.
>
> Das sind gewissermassen Kommentare. Aber warum sollte ich hier Nullen
> und Einsen händisch sortieren, die Guten ins Töpfchen, die schlechten
> ins Kröpfchen?
>
>> Und ich kann es irgendwie nicht glauben, dass du es so z.B. machst.
>
> Was spricht konkret dagegen?

Es spricht dagegen dass hier suggeriert wird, ein Bit werde auf 0 
gesetzt. Aber so ist es ja nicht. Es passiert gar nichts. Man verlässt 
sich also darauf dass das Bit bereits 0 ist. Genau DAS wird aber eben 
nicht deutlich. Fasse ich das Bit gar nicht an, so ist klar, das Bit hat 
nach wie vor den Defaultwert. Hier sieht es auf den ersten Blick aber so 
aus, als werde mit dem Bit etwas getan. Das ist ein ganz klarer Verstoß 
gegen jede wie auch immer geartete Clean-Code Regel.

[S. Landolt (Gast)]

> Es spricht dagegen dass hier suggeriert wird...
Aber doch nur dem unbedarften Anfänger.

[Cyblord -. (cyblord)]

S. Landolt schrieb:
>> Es spricht dagegen dass hier suggeriert wird...
> Aber doch nur dem unbedarften Anfänger.

Nein, die 0 könnte jeder übersehen und stattdessen eine echte 
Schiebeoperation vermuten. Es geht ja darum Code schnell in seiner 
Funktion erfassen zu können. Diese Schreibweise macht das Gegenteil.
Manche finden es ja cool unleserlichen oder unverständlichen Code zu 
schreiben. Das Ziel sollte aber eben übersichtlicher und sauberer Code 
sein.
Aber ich glaube auch, wer keinerlei Verständnis für Clean-Code Konzepte 
hat, dem kann man es auch nur schwer erklären.

[S. Landolt (Gast)]

Als Programmierer eine einzelne Null mit einer Eins verwechseln? Na, ich 
weiß nicht. Aber ich will nicht streiten, jeder nach seiner Fasson.

[Justus S. (jussa)]

Cyblord -. schrieb:
> Man verlässt
> sich also darauf dass das Bit bereits 0 ist.

nein tut man nicht, und es ist auch völlig egal, was das Bit vorher 
ist/war...

[Cyblord -. (cyblord)]

Justus S. schrieb:
> Cyblord -. schrieb:
>> Man verlässt
>> sich also darauf dass das Bit bereits 0 ist.
>
> nein tut man nicht, und es ist auch völlig egal, was das Bit vorher
> ist/war...

Warum das? Wenn das Bit seinen Zustand nicht verändert musst du den 
vorherigen Zustand wissen um den aktuellen Zustand zu erfahren.
Wird ein Bit dagegen wirklich auf 1 oder 0 gesetzt sieht man das im Code 
und der vorherige Zustand ist irrelevant.

Was also wird mit dieser 0 Schieberei dem Leser jetzt genau 
dokumentiert? Was darf man daraus schließen wenn man das liest?

[Justus S. (jussa)]

Cyblord -. schrieb:
> Warum das?

weil bei einer Zuweisung

UCSRA = ...

der vorherige Zustand der einzelnen Bits nunmal keine Rolle mehr 
spielt...

[(prx) A. K. (prx)]

Cyblord -. schrieb:
> Es spricht dagegen dass hier suggeriert wird, ein Bit werde auf 0
> gesetzt. Aber so ist es ja nicht. Es passiert gar nichts.

Bei
  UCSRA = 1<<TXC | 0<<U2X | 0<<MPCM;
werden keine Nullen geschrieben? Dann solltest du dein Exemplar 
auswechseln, es ist kaputt.

[Cyblord -. (cyblord)]

A. K. schrieb:
> Cyblord -. schrieb:
>> Es spricht dagegen dass hier suggeriert wird, ein Bit werde auf 0
>> gesetzt. Aber so ist es ja nicht. Es passiert gar nichts.
>
> Bei
>   UCSRA = 1<<TXC | 0<<U2X | 0<<MPCM;
> werden keine Nullen geschrieben? Dann solltest du dein Exemplar
> auswechseln, es ist kaputt.

Ja ist ja gut. Bei einer einfachen Zuweisung mag das stimmen. Aber 
sobald jemand

UCSRA |= 1<<TXC | 0<<U2X | 0<<MPCM;

schreibt trifft das nicht mehr zu. Das ganze Konzept ist einfach 
gefährlich uneindeutig.