Code fehler bei 74hc165 kette (einzelner geht)

[Martin K. (thereallife)]

Moin moin,
ich habe ein kleines Problem mit meiner Kette von 74hc165er.
den ersten kann ich auslesen, allerdings nicht die 3 restlichen.
(Sie funktionieren allerdings, das weiß ich weil ich im Code mal 
spaßeshalber i<24 gemacht habe. Dann funktioniert der 2&3te 74hc165 
jedoch nicht mehr der erste.

Ich hab scheinbar also irgendwie nen denkfehler in meiner Routine, komme 
aber nicht drauf. Denke aber das jmd der etwas geübter ist den Fehler 
schnell entdeckt.

Mein Setup:
Prozessor ATmega32A
74hc165 mit tastern und drehencodern

Hier mal mein Code:

void Pulse165Clock()
{
  CLOCK165_low();
  CLOCK165_high();
}
////////////////////////////////////////////////////////////////////
void Pulse165Parallel()
{
  PARALLEL165_low(); // Erst null dann high! WICHTIG!!!
  PARALLEL165_high();
}
////////////////////////////////////////////////////////////////////
#if 0
void ShiftIn(int *var)
{
  for (unsigned char a = 0; a < 4; ++a)
  {
    CLOCK165_high();
    Pulse165Parallel();
    for (unsigned char i = 0; i < 8; ++i)
    {
      *var <<= 1;
      if (DATA165_read())
      *var |= 0x01;
      Pulse165Clock();
    }
    CLOCK165_low();
  }
}
#endif
////////////////////////////////////////////////////////////////////
static uint8_t DATA165_read(void)
{
  if (PINB & _BV(DATA165_PIN))
  {
    return (PINB & _BV(DATA165_PIN)) ? 1 : 0;
  }
  else
  return (PINB & _BV(DATA165_PIN)) ? 1 : 0;
}
////////////////////////////////////////////////////////////////////
uint32_t ShiftIn(void)
{
  uint32_t shiftResult;
  for (unsigned char a = 0; a < 4; ++a)
  {
    CLOCK165_high();
    Pulse165Parallel();    
    for (unsigned char i = 0; i < 8; ++i) 
    {
      shiftResult |= DATA165_read();
      shiftResult <<= 1;
      Pulse165Clock();
    }
    CLOCK165_low();
  }
  return shiftResult;
}

Main Routine:

#include <avr/io.h>
#include <avr/interrupt.h>
#ifndef F_CPU
#define F_CPU 8000000UL
#endif
#include <stdio.h>
#include <util/delay.h>
#include "ShiftOut.h"
#include "ShiftIn.h"
#include "UART.h"
#include "Encoder.h"
#define LED1  PA1
#define LED_DDR  DDRA
#define LED_PORT  PORTA
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
void prtbin(unsigned byte)
{
  for(char i=0; i<32; i++) printf("%u",(byte>>(32-i))& 0x0001);
}
 
int main(void)
{
  stdout = &mystdout;
  long baud = 9600UL;
  UART_Init(baud);
  InitShiftIn();
  int v165 = 0;
  InitShiftOut();
  setup_Encoder();
  while(1)
  {
    v165 = ShiftIn();
    ShiftOut(v165);
    get_Encoder_data(v165);
    read_Encoder();
    prtbin(v165);
    printf("\n");
  }
}

Ich weiß (jedenfalls wurde es mir erklärt) das man eigentlich keine 
Funktionen in includes schreiben soll. Jedoch ist es für mich als Anfang 
erstmal einfacher so um etwas mehr überblick zu behalten.

Vielen Dank im Vorraus!

Beste Grüße
Martin

[Karl H. (kbuchegg)]

Martin Kathke schrieb:


> ////////////////////////////////////////////////////////////////////
> uint32_t ShiftIn(void)
> {
>   uint32_t shiftResult;
>   for (unsigned char a = 0; a < 4; ++a)
>   {
>     CLOCK165_high();
>     Pulse165Parallel();


Du willst nur ein einziges mal ALLEN 165-ern einen Parallel Load Pulse 
geben.
Danach wird nur noch rein-geshiftet.
Auch die Sache mit dem Clock ist dubios.
Du fummelst mir da zwischenzeitlich zu viel an den beiden Pins rum. 
Bedenke: Für die Bausteine zählt jede Flanke, egal wo und wie die im 
Programm entsteht. D.h. du fummelst da quer im Programm nicht an den 
CLock Leitungen rum um sie definiert auf 0 oder 1 zu setzen, es sei denn 
du willst absichltich einen Puls erzeugen. Du setzt die Clock Leitung 
(und die Parallel Load) einmalig am Programmanfang auf 1 und dann bleibt 
sie auch auf 1, es sei denn du willst gewollt einen Puls erzeugen.
Alles andere hat irgendwo das Äquivalent eines epilleptischen Anfalls, 
bei dem man seine Pins (und das was die an der externen Beschaltung 
auslösen) nicht mehr unter Kontrolle hat.

 ////////////////////////////////////////////////////////////////////
 uint32_t ShiftIn(void)
 {
   uint32_t shiftResult;

   // CLOCK165_high bzw. Pulse165High kann man hier aufrufen, muss man
   // aber nicht. Wenn man sich als Programmierer mit sich selbst
   // einig ist, dass die beiden Leitungen grundsätzlich immer auf 1
   // zu stehen haben und das am Programmanfang sicher stellt, dann
   // ist das auch gut
   CLOCK165_high();  // Sicherheitshalber. Die Clock Leitung wird
                     // im Programm grundsätzlich auf High gehalten
   Pulse165High();   // ditto

   // an alle 165-er: Jetzt die 8 parallelen Eingänge einlesen
   Pulse165Parallel();

   // alles 4 Bausteine haben ihre Werte von extern übernommen
   // mal sehen, was sie rein bekommen haben
   for (unsigned char a = 0; a < 4; ++a)
   {
     for (unsigned char i = 0; i < 8; ++i)
     {
       shiftResult |= DATA165_read();
       shiftResult <<= 1;
       Pulse165Clock();
     }
   }

   // an dieser Stelle stehen Clock bzw ParallelLoad auf jeden Fall
   // wieder auf high. Dies ist durch die Art und Weise wie die Pulse
   // erzeugt werden, automatisch gewährleistet.

   return shiftResult;
}

Das hier ist auch doppelt gemoppelt

static uint8_t DATA165_read(void)
{
  if (PINB & _BV(DATA165_PIN))
  {
    return (PINB & _BV(DATA165_PIN)) ? 1 : 0;
  }
  else
  return (PINB & _BV(DATA165_PIN)) ? 1 : 0;
}

wenn PINB & _BV(DATA165_PIN) in der Abfrage TRUE ergibt, dann kennst du 
bereits das Ergebnis, das die Funktion zurückgeben muss. Das kann dann 
nur noch 1 sein, weil ja der Pin auf 1 gewesen sein muss, ansonsten wäre 
das Ergebnis deer Abfage nicht TRUE gewesen

static uint8_t DATA165_read(void)
{
  if (PINB & _BV(DATA165_PIN))
    return 1;

  return 0;
}

oder kürzer geschrieben (wobei ich mir nicht sicher bin, ob ich das hier 
tun würde. Die Version mit dem expliziten if ist hier fein)

static uint8_t DATA165_read(void)
{
  return (PINB & _BV(DATA165_PIN)) ? 1 : 0;
}

[Karl H. (kbuchegg)]

Generell:
Ich hab nichts gegen Funktionen. Ganz im Gegenteil.
Aber an dieser Stelle finde ich es keine so gute Idee, für die Erzeugung 
der Pulse eigene Funktionen zu schreiben. Ich denke das versteckt hier 
zu viel Information, die aber eigentlich vital ist. Ich würde es so 
schreiben

void InitShiftIn(void)
{
  ... Data Register einstellen ...

   // sicherstellen, dass alle wichtigen Leitungen den 'Ruhe'Pegel haben
   CLOCK165_high();
   Pulse165High();
}


////////////////////////////////////////////////////////////////////
 uint32_t ShiftIn(void)
 {
   uint32_t shiftResult;

   // an alle 165-er: Jetzt die 8 parallelen Eingänge einlesen
   Pulse165Low();
   Pulse165High();

   // alles 4 Bausteine haben ihre Werte von extern übernommen
   // mal sehen, was sie rein bekommen haben
   for (unsigned char a = 0; a < 4; ++a)
   {
     for (unsigned char i = 0; i < 8; ++i)
     {
       if (PINB & _BV(DATA165_PIN))
         shiftResult |= 0x0001;

       shiftResult <<= 1;

       CLOCK165_Low();
       CLOCK165_high();
     }
   }

   return shiftResult;
}

Dadurch sieht man in dieser dann doch schon recht low-Level-Funktion 
besser, wie die Pulspolaritäten eigentlich sind.
Man kann sich nämlich auch durch zuviele Funktion ein Ei legen. 
Irgendwann nimmt die Übersicht dann nicht mehr zu, sondern ab, weil man 
die wesentlichen Informationen über zu viele Funktionen verteilt hat.

[Vlad T. (vlad_tepesch)]

Anmerkung zum Naming:
es hat sich bewährt für Funktionsgruppen (wie es dieser PiSo-SR-Treiber 
ja ist) Präfixe anstelle von Suffixes zu verwenden.

So sieht man auf den ersten Blick, dass die Funktionen zusammengehören 
und auch die Code-Vervollständigung funktioniert besser.

Pulse165Clock    -> SR165_pulseClock
Pulse165Parallel -> SR165_pulseParallel
ShiftIn          -> SR165_shiftIn
DATA165_read     -> SR165_readData

[Karl H. (kbuchegg)]

Und noch eine Anmerkung zu dem was Vlad gesagt hat:
Selbst wenn du das nicht machst, achte darauf wenigstens ein konistentes 
Benamungsschema zu haben

Entweder die Funtkionen heissen
  Pulse165Low();
und
  Clock165Low();

oder sie heissen
  Pulse165_Low();
und
  Clock165_Low();

(also entweder überall ein _ rein oder nirgends)

Und auch auf Konsistenz in den Schreibweisen bzgl. Gross- und 
Kleinschreibung achten!

Das alles hat Auswirkungen darauf, wie leicht man Code schreiben bzw. 
lesen kann bzw. wieviele blödsinnige Tippfehler man in der ersten 
Version macht. Wenn du quer durch die Bank bei 5 Funktionen 5 
unterschiedliche Schemata benutzt, dann wird man wahnsinnig dabei sich 
daran zuerinnern, welches Schema jetzt das richtige ist. Überhaupt dann, 
wenn man dann irgenwan nicht 5 Funktionen sondern 50 hat, die alle 
irgendwie ihr eigenes Modul Namensschema haben.

[Martin K. (thereallife)]

Danke euch aufjedenfall für eure Tipps und Hilfen!

das ich den allen nach 8 Bits nen Pulse aufn Parallel Pin gegeben habe 
war ein reiner denkfehler... Irgendwie war ich der Meinung die brauchen 
das um die neuen daten zu übernehmen...

Das mit den zuvielen Funktionen habe ich beherzigt und korregiert auch 
die Namen habe ich nun einheitlich gestaltet. Allerdings waren für mich 
die mit _ makros, ohne waren funktionen...

Jetzt habe ich allerdings das Problem das die letzten bits "fliegen" sie 
springen ständig von null auf 1 etc, Daten vom eigentlichen drücken bzw 
drehen übernimmt er garnicht mehr bis auf eine seite von einem 
Drehencoder...

Das mit den Namen wie Vlad sie geschrieben hat werde ich ändern wenn es 
läuft, wenn ich jetzt noch neue Namen gebe komm ich bei der Fehlersuche 
glaube ich etwas sehr durcheinander^^

Hier der Code:

////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
//Include File fuer Shift In (74hc165)                            //////////////////////////////////////////////////////////////////
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
/*
int main(void)
{
  InitShiftIn();
  int v165 = 0;
  while(1)
  {
    v165 = ShiftIn();
  }
}
*/
#define DATA165_PIN      PB6
#define DATA165_PORT    PORTB
#define DATA165_DDR      DDRB
////////////////////////////
#define CLOCK165_PIN    PA5
#define CLOCK165_PORT    PORTA
#define CLOCK165_DDR    DDRA
#define CLOCK165_high()    (CLOCK165_PORT |= _BV(CLOCK165_PIN))
#define CLOCK165_low()      (CLOCK165_PORT &= ~_BV(CLOCK165_PIN))
////////////////////////////
#define PARALLEL165_PIN    PA4
#define PARALLEL165_PORT    PORTA
#define PARALLEL165_DDR     DDRA
#define PARALLEL165_high()  (PARALLEL165_PORT |= _BV(PARALLEL165_PIN))
#define PARALLEL165_low()   (PARALLEL165_PORT &= ~_BV(PARALLEL165_PIN))
////////////////////////////////////////////////////////////////////
void InitShiftIn()
{
  PARALLEL165_DDR |= _BV(PARALLEL165_PIN);
  CLOCK165_DDR |= _BV(CLOCK165_PIN);
  DATA165_DDR &= ~_BV(DATA165_PIN);
  DATA165_PORT &= ~_BV(DATA165_PIN);
  CLOCK165_high();
  PARALLEL165_high();
}
////////////////////////////////////////////////////////////////////
void Pulse165_Clock()
{
  CLOCK165_low();
  CLOCK165_high();
}
////////////////////////////////////////////////////////////////////
void Pulse165_Parallel()
{
  PARALLEL165_low(); // Erst null dann high! WICHTIG!!!
  PARALLEL165_high();
}
////////////////////////////////////////////////////////////////////
#if 0
void ShiftIn(int *var)
{
  PARALLEL165_low();
  PARALLEL165_high();
  for (unsigned char a = 0; a < 4; ++a)
  {
    for (unsigned char i = 0; i < 8; ++i)
    {
      *var <<= 1;
      if (PINB & _BV(DATA165_PIN))
        *var |= 0x01;
        CLOCK165_low();
        CLOCK165_high();
    //  if (DATA165_read())
    //  *var |= 0x01;
    //  Pulse165Clock();
    }
  }
}
#endif
////////////////////////////////////////////////////////////////////
static uint8_t DATA165_read(void)
{
  if (PINB & _BV(DATA165_PIN))
  {
    return (PINB & _BV(DATA165_PIN)) ? 1 : 0;
  }
  else
  return (PINB & _BV(DATA165_PIN)) ? 1 : 0;
}
////////////////////////////////////////////////////////////////////
uint32_t ShiftIn(void)
{
  uint32_t shiftResult;
  PARALLEL165_low();
  PARALLEL165_high();
//  Pulse165Parallel();  
  for (unsigned char a = 0; a < 4; ++a)
  {
    for (unsigned char i = 0; i < 8; ++i) 
    {
      if (PINB & _BV(DATA165_PIN))
        shiftResult |= 0x0001;
        shiftResult <<= 1;
        CLOCK165_low();
        CLOCK165_high();
      //shiftResult |= DATA165_read();
      //shiftResult <<= 1;
      //Pulse165Clock();
    }
  }
  return shiftResult;
}

[Karl H. (kbuchegg)]

Martin Kathke schrieb:

> Das mit den zuvielen Funktionen habe ich beherzigt und korregiert auch
> die Namen habe ich nun einheitlich gestaltet. Allerdings waren für mich
> die mit _ makros, ohne waren funktionen...

Für Makros hat sich weltweit eingebürgert, dass Makros grundsätzlich 
komplett in Grossbuchstaben geschrieben werden. Und auch umgekehrt: 
Alles was komplett in Grossbuchstaben geschrieben ist, ist ein Makro.

Das ist so ziemlich die einzige Konvention, an die sich weltweit 
Millionen von C Programmierer problemlos halten.

> Jetzt habe ich allerdings das Problem das die letzten bits "fliegen" sie
> springen ständig von null auf 1 etc

Du kannst mal probieren, die Pulse etwas zu verlängern. Wenn genügend 
viele Bausteine kaskadiert sind, die natürlich alle an ein und derselben 
CLock Leitung hängen, dann ist irgendwann mal Schluss mit lustig. Die 
Flanken verschleifen dann zu stark.

Also: kurze delays zwischen demjeweiligen Low und dem darauffolgenden 
High mal rein.
Zu Testzwecken kannst du da ruhig auch schon mal in den 
Millisekundenbereich oder noch höher vorstossen (auch Sekunden oder 
Minuten sind kein Problem - Tip: das lässt sich beim Debuggen zb gut 
benutzen indem man ein paar LED an die Leitungen hängt und so langsam 
taktet, dass man als Mensch verfolgen kann was passiert)

Wenn das nichts bringt, dann sieh dir deine Verkabelung an. 
Wackelkontakte, schlechte Kontakte. irgendsowas in dieser Richtung.

[Martin K. (thereallife)]

Karl Heinz schrieb:
> Makros grundsätzlich
> komplett in Grossbuchstaben geschrieben werden.

alles klar, werde ich mir einprägen.

5ms delay zwischen hab ich zwischen die einzelnen pulse gelegt. Selbes 
resultat. Wenn ich allerdings nur den ersten 165er auslese habe ich kein 
problem. Dieser ist auch am weitesten entfernt.
Nach dazu kommt auch nur noch eine 1 von einer seite der drehencoder an 
(alle werden gegen gnd) gezogen daher müssten dort 10x1 im normalfall 
stehen.

Ich arbeite auf einer Platine. Der Schaltplan wurde
mehrmals überprüft und da ich heute schon die drehencoder auslesen 
konnte, gehe ich mal nicht davon aus das dort ein wackler oder ähnliches 
enstehen konnte da die platine auch nicht bewegt wurde.
Bei bedarf kann ich allerdings auch mal eagle schalt und boardplan 
hochladen.

Ist das ganze eventuell ein Variablen fehler?
Ist irgendeine zu klein? meines erachtens nicht, aber ich bin auch noch 
ein anfänger und überseh vill was :D

/edit

da fällt mir grade ein, es kann gut sein das ein paar daten eingänge 
fliegend sind. Das letzte 165bauteil geht nämlich mit 7 data ins an ein 
lötpunkt welcher so vorgegsehen ist damit man ihn mit masse oder gnd 
verbinden kann. Dies soll später dazu da sein damit ich meiner platine 
einmalig eine adresse zuweisen kann (für I²C)

Diese 7 pins sind momentan also fliegend... was aber nicht erklärt warum 
keine daten von den andern sachen reinkommen...

/edit 2
nehme ich die for schleife mit dem a<4 raus,
und bleibe beim i bei einer 8, kann ich meine taster auslesen ( diese 
sind am ersten 165)
ersetze ich die 8 mit einer 24 so bekomme ich meine drehencoder 9 von 10
1en von den drehencodern (PIN A oder PIN B vom obersten drehencoder 
fehlt scheinbar)
165 Nummer 1 hat die 8 Taster
165 Nummer 2 hat die ersten 4 drehencoder
165 Nummer 3 hat den 5ten Drehencoder
165 Nummer 4 hat die "Lötschalter"(ich weiß leider nicht das fachwort 
für soetwas)

mit einem i<32 fangen die werte wieder an zu springen und lediglich ein 
PIN a oder b vom 5ten drehencoder kommt an (pin a oder b heißt einer von 
beiden, ich weiß nur nicht welcher).

[Martin K. (thereallife)]

Hier mal nen Auszug aus meiner Printf ausgabe:
-128
000000000111111111000000

um es nen bssl übersichtlicher zu machen:
-128
00 00 00 00 01 11 11 11 11 00 00 00
Er haut zwar 24 bits raus, allerdings sind die meiner meinung nach 
verschoben. Gans links müssten 8 nullen vor den 1en kommen und nicht 
6...

es scheint also irgendwas schief zu gehen... Vielleicht bringt das ja 
jmd auf die richtige fährte ;)

[Vlad T. (vlad_tepesch)]

dann leg doch mal ein (oder mehrere) eindeutigeres Muster an deine 
Eingänge.

[Martin K. (thereallife)]

Es ist defenetiv ein Code Fehler.

ist nur die Frage wo...

ich habs mal so ausprobiert:

////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
//Include File fuer Shift In (74hc165)                            //////////////////////////////////////////////////////////////////
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
/*
int main(void)
{
  InitShiftIn();
  int v165 = 0;
  while(1)
  {
    v165 = ShiftIn();
  }
}
*/
#define DATA165_PIN      PB6
#define DATA165_PORT    PORTB
#define DATA165_DDR      DDRB
////////////////////////////
#define CLOCK165_PIN    PA5
#define CLOCK165_PORT    PORTA
#define CLOCK165_DDR    DDRA
#define CLOCK165_high()    (CLOCK165_PORT |= _BV(CLOCK165_PIN))
#define CLOCK165_low()      (CLOCK165_PORT &= ~_BV(CLOCK165_PIN))
////////////////////////////
#define PARALLEL165_PIN    PA4
#define PARALLEL165_PORT    PORTA
#define PARALLEL165_DDR     DDRA
#define PARALLEL165_high()  (PARALLEL165_PORT |= _BV(PARALLEL165_PIN))
#define PARALLEL165_low()   (PARALLEL165_PORT &= ~_BV(PARALLEL165_PIN))
////////////////////////////////////////////////////////////////////
void InitShiftIn()
{
  PARALLEL165_DDR |= _BV(PARALLEL165_PIN);
  CLOCK165_DDR |= _BV(CLOCK165_PIN);
  DATA165_DDR &= ~_BV(DATA165_PIN);
  DATA165_PORT &= ~_BV(DATA165_PIN);
  CLOCK165_high();
  PARALLEL165_high();
}
////////////////////////////////////////////////////////////////////
void Pulse165_Clock()
{
  CLOCK165_low();
  CLOCK165_high();
}
////////////////////////////////////////////////////////////////////
void Pulse165_Parallel()
{
  PARALLEL165_low(); // Erst null dann high! WICHTIG!!!
  PARALLEL165_high();
}
////////////////////////////////////////////////////////////////////
#if 0
void ShiftIn(int *var)
{
  PARALLEL165_low();
  PARALLEL165_high();
  for (unsigned char a = 0; a < 3; ++a)
  {
    for (unsigned char i = 0; i < 8; ++i)
    {
      *var <<= 1;
      if (PINB & _BV(DATA165_PIN))
        *var |= 0x01;
        CLOCK165_low();
        CLOCK165_high();
    //  if (DATA165_read())
    //  *var |= 0x01;
    //  Pulse165Clock();
    }
  }
}
#endif
////////////////////////////////////////////////////////////////////
static uint8_t DATA165_read(void)
{
  if (PINB & _BV(DATA165_PIN))
  {
    return (PINB & _BV(DATA165_PIN)) ? 1 : 0;
  }
  else
  return (PINB & _BV(DATA165_PIN)) ? 1 : 0;
}
////////////////////////////////////////////////////////////////////
uint32_t ShiftIn1(void)
{
  uint32_t shiftResult;
  PARALLEL165_low();
  PARALLEL165_high();
//  Pulse165Parallel();  
    for (unsigned char i = 0; i < 8; ++i) 
    {
      if (PINB & _BV(DATA165_PIN))
        shiftResult |= 0x0001;
        shiftResult <<= 1;
        CLOCK165_low();
        CLOCK165_high();
      //shiftResult |= DATA165_read();
      //shiftResult <<= 1;
      //Pulse165Clock();
    }
  return shiftResult;
}
uint32_t ShiftIn2(void)
{
  uint32_t shiftResult;
  PARALLEL165_low();
  PARALLEL165_high();
  //  Pulse165Parallel();
    for (unsigned char a = 0; a < 8; ++a)
    {
      CLOCK165_low();
      CLOCK165_high();
    }
  for (unsigned char i = 0; i < 8; ++i)
  {
    if (PINB & _BV(DATA165_PIN))
    shiftResult |= 0x0001;
    shiftResult <<= 1;
    CLOCK165_low();
    CLOCK165_high();
    //shiftResult |= DATA165_read();
    //shiftResult <<= 1;
    //Pulse165Clock();
  }
  return shiftResult;
}
uint32_t ShiftIn3(void)
{
  uint32_t shiftResult;
  PARALLEL165_low();
  PARALLEL165_high();
  //  Pulse165Parallel();
  for (unsigned char a = 0; a < 16; ++a)
  {
    CLOCK165_low();
    CLOCK165_high();
  }
  for (unsigned char i = 0; i < 8; ++i)
  {
    if (PINB & _BV(DATA165_PIN))
    shiftResult |= 0x0001;
    shiftResult <<= 1;
    CLOCK165_low();
    CLOCK165_high();
    //shiftResult |= DATA165_read();
    //shiftResult <<= 1;
    //Pulse165Clock();
  }
  return shiftResult;
}

Main Routine

#include <avr/io.h>
#include <avr/interrupt.h>
#ifndef F_CPU
#define F_CPU 8000000UL
#endif
#include <stdio.h>
#include <util/delay.h>
#include "ShiftOut.h"
#include "ShiftIn.h"
#include "UART.h"
#include "Encoder.h"
#define LED1  PA1
#define LED_DDR  DDRA
#define LED_PORT  PORTA
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
void prtbin(unsigned byte)
{
  for(char i=0; i<8; i++) printf("%u",(byte>>(8-i))& 0x0001);
}
 
int main(void)
{
  stdout = &mystdout;
  long baud = 9600UL;
  UART_Init(baud);
  InitShiftIn();
  uint8_t v165 = 0;
  uint8_t v165_1 = 0;
  uint8_t v165_2 = 0;
  InitShiftOut();
  setup_Encoder();
  while(1)
  {
    v165 = ShiftIn1();
    v165_1 = ShiftIn2();
    v165_2 = ShiftIn3();
    ShiftOut(v165);
    get_Encoder_data(v165);
    read_Encoder();
    printf("%d \n", v165);
    prtbin(v165);
    prtbin(v165_1);
    prtbin(v165_2);
    printf("\n");
  }
}

so bekomme ich alle werte (fast) fast richtig raus. das einzigste was 
falsch ist, ist das nun beim 5ten drehencoder das erste bit fehlt. änder 
ich allerdings beim shift in 1 die 8 auf ne 7 und beim shift 2 die 16 
auf ne 15, ist dieses bit wieder da, dafür fehlt dann alerdings das 
letzte vom drehencoder 4 :D

es liegt also irgendwas im code krum... die spannende frage ist nur was 
:D

[Karl H. (kbuchegg)]

void prtbin(unsigned byte)
{
  for(char i=0; i<8; i++) printf("%u",(byte>>(8-i))& 0x0001);
}

7 - i

[Karl H. (kbuchegg)]

Ich würds auch mit Vlad halten

Den ganzen KLimbim in der Aussenbeschaltung runter und erst mal für 
gesicherte Verhältnisse sorgen.

Dazu ALLE (!) Eingänge ALLER (!) 165 mit Masse verbinden.
Die Ausgabe (es reicht dazu völlig aus, EINE EINZIGE EInlsefunktion zu 
haben, die alle 4 165 einliest) muss überall ein 0 Bit haben.

Dann fange ich bei einem Eingang (systematisch: erster Eingang vom 
ersten 165) an und lege den von Masse auf Vcc. In der Ausgabe muss an 
der entsprechenden Stelle eine 1 auftauchen.

Dann der nächste Eingang: von Masse auf Vcc gehen. Eine weitere 1 muss 
auftauchen.


Das wäre mein Vorgehen: sytematisch und erst mal nichts dem Zufall 
überlassen. Floatende Eingänge darf es da schon mal überhaupt nicht 
geben. Genausowenig wie mich da erst mal eine Encoder-Auswertung 
interessiert.
Im Endeffekt zeigt sich praktisch immer, das man mit einem derartigen 
systematischen Vorgehen schneller zum Ziel kommt als mit wildem 
unstrukturiertem RUmprobieren mit Drehencodern von denen man nicht weiss 
(ok, ich hier nicht weiss), ob die überhaupt sauber angeschlossen sind.

und ja. Zum Probieren teste ich die Schaltung erst mal auf einem 
Steckbrett aus, ehe ich mir eine Platine ätze.

ZUm Code. Der sieht meiner Meinung nach in Ordnung aus.
Entweder hast du bei der Ausgabe noch irgendwo einen Hund drinnen, oder 
es ist ein Hardware Problem.

Aber dieses wilde, unstrukturierte Probieren mit 'bei einem IC gehst, 
bei dreien nicht', bringt dich nicht weiter. IMHO ist das nur vertane 
Zeit in der man das Problem auch sinnvoll systematisch angehen könnte, 
in dem man erst mal an den Eingängen für kontrollierte gesicherte 
Verhätlnisse sorgt, die nicht noch von irgendwelcher zusätzlicher 
Hardware abhängen. Eine simple Drahtbrücke zu GND oder Vcc sorgt genau 
dafür. Blöd natürlich, wenn man den Steckbrett-Schritt übersprungen hat 
und jetzt nichts mehr einfach ändern kann.

[Vlad T. (vlad_tepesch)]

Karl Heinz schrieb:
> Um Code. Der sieht meiner Meinung nach in Ordnung aus.
> Entweder hast du bei der Ausgabe noch irgendwo einen Hund drinnen, oder
> es ist ein Hardware Problem.

weiß nicht, ob ich gerade auf dem Schlauch stehe, aber werden nicht die 
ganzen Bits nach dem Einlesen des letzten fälschlicherweise 
hochgeschoben? Hier sollte doch nur noch das Bit kopiert werden und 
nicht mehr alles weggeshiftet. das heißt, alles ist eins zu weit oben 
und das erste bit rutscht schon in das 4. Byte rein.

[Karl H. (kbuchegg)]

Vlad Tepesch schrieb:
> Karl Heinz schrieb:
>> Um Code. Der sieht meiner Meinung nach in Ordnung aus.
>> Entweder hast du bei der Ausgabe noch irgendwo einen Hund drinnen, oder
>> es ist ein Hardware Problem.
>
> weiß nicht, ob ich gerade auf dem Schlauch stehe, aber werden nicht die
> ganzen Bits nach dem Einlesen des letzten fälschlicherweise
> hochgeschoben? Hier sollte doch nur noch das Bit kopiert werden und
> nicht mehr alles weggeshiftet.

(nachdenk)
ja, da hast du recht.
Mit systeamtischem Vorgehen hätte man das schon gesehen, dass das 
falsche Bit auf 1 geht.

Muss also heissen

  for (unsigned char i = 0; i < 8; ++i)
  {
    shiftResult <<= 1;

    if (PINB & _BV(DATA165_PIN))
      shiftResult |= 0x0001;

    CLOCK165_low();
    CLOCK165_high();
  }

Und ich bin mir auch nicht sicher, ob man nach dem Parallel Load erst 
mal einen Clock Puls braucht oder nicht. Aber auch das würde man sehen, 
wenn man systematisch vorgeht und nicht gleich in die vollen in der 
Annahme, dass schon alles stimmen wird.

Was aber auf keinen Fall sein kann, dass irgendwelche Bits klappern, so 
wie es im Ausgangsposting noch der Fall war.

[Martin K. (thereallife)]

Ich wollte grade schreiben das immer das erste bit fehlt...
Deine Lösung war jetzt aber richtig :) nun läuft alles

danke euch