Vermtliches Pointer Problem

[Antny (Gast)]

Hallo Leute, kann mal bitte jemand in den Code gucken und mir sagen, 
warum die werte in dem Struct nicht aktualisiert werden, bzw. nur 
unvollständig ich bekomme es irgendwie einfach nicht zum laufen...

Dankeschön

void ADC_Messung ()
{
  if (ADCprescaler >6)
  {
    PORT_CS_ADC_STEU &= ~( 1 << PIN_CS_ADC_STEU );
    SPCR =  (1<<SPE)  |  //SPI- Enable
        (1<<MSTR)  |
        (1<<SPR1);  //Set Master

    _delay_ms(10);

    switch (nextChannel)
    {
      case TempCh1:  read_adc(&(Ch1.Temperatur[2]));
              Ch1.Temperatur[2]&=0x0F;
              read_adc(&(Ch1.Temperatur[1]));
              read_adc(&(Ch1.Temperatur[0]));
              nextChannel=ULast1;
              break;
      case CurrCh1:  read_adc(&Ch1.Strom[2]);
              Ch1.Strom[2]&=0x0F;
              read_adc(&Ch1.Strom[1]);
              read_adc(&Ch1.Strom[0]);
              nextChannel=ULast1;
              break;
      case ULast1:  read_adc(&Ch1.Spannung[2]);
              Ch1.Spannung[2]&=0x0F;
              read_adc(&Ch1.Spannung[2]);
              read_adc(&Ch1.Spannung[2]);
              nextChannel=TempCh1;
              break;
      /*case TempCh2:  Ch2.Temperatur[2]=0x0F & read_adc();
              Ch2.Temperatur[1]=read_adc();
              Ch2.Temperatur[0]=read_adc();
              nextChannel=CurrCh2;
              break;
      case CurrCh2:  Ch2.Strom[2]=0x0F & read_adc();
              Ch2.Strom[1]=read_adc();
              Ch2.Strom[0]=read_adc();
              nextChannel=ULast2;
              break;
      case ULast2:  Ch2.Spannung[2]=0x0F & read_adc();
              Ch2.Spannung[1]=read_adc();
              Ch2.Spannung[0]=read_adc();
              nextChannel=TempCh1;
              //luefterregelung(&msb,&Ch1.PWM_Fan,1,&Ch1.temp_old,&Ch1.t_delta_sum);
              break;*/
    }
    PORT_CS_ADC_STEU |=  ( 1 << PIN_CS_ADC_STEU );
    _delay_ms(1);
    SPDR=nextChannel;
    while ( ! ( SPSR&(1<<SPIF) ) );
    SPDR=nextChannel;
    while ( ! ( SPSR&(1<<SPIF) ) );

    uint16_t msb=Ch1.Temperatur[2]<<11;
    msb|=Ch1.Temperatur[1]<<3;
    msb|=Ch1.Temperatur[0]>>3;
    ADCprescaler=0;
    uart_sendc('A');
//    uart_sendc(&Ch1.Temperatur[2]);
//    uart_sendc(&Ch1.Temperatur[1]);
//    uart_sendc(&Ch1.Temperatur[0]);
//    uart_sendc('B');
//    uart_sendc(&Ch1.Spannung[2]);
//    uart_sendc(&Ch1.Spannung[1]);
//    uart_sendc(&Ch1.Spannung[0]);

    //Einstellen des mächsten Kanals



  }
}

void read_adc (uint8_t *value)
{
  PORT_CS_ADC_STEU &= ~( 1 << PIN_CS_ADC_STEU );
  SPCR =  (1<<SPE)  |  //SPI- Enable
      (1<<MSTR)  |
      (1<<SPR1);  //Set Master
  SPDR=0b10101010;
  while ( ! ( SPSR&(1<<SPIF) ) );
  *value=SPDR;
}

[Karl H. (kbuchegg)]

Antny schrieb:
> Hallo Leute, kann mal bitte jemand in den Code gucken und mir sagen,
> warum die werte in dem Struct nicht aktualisiert werden,

welche Struct?

> bzw. nur
> unvollständig

Welche werden aktualisiert, welche nicht?

> ich bekomme es irgendwie einfach nicht zum laufen...

Das könnte an deiner Codeformatierung liegen.
Viel zu dicht.


Das sieht seltsam aus:

>     switch (nextChannel)
>     {
>       case TempCh1:  read_adc(&(Ch1.Temperatur[2]));
>               Ch1.Temperatur[2]&=0x0F;
>               read_adc(&(Ch1.Temperatur[1]));
>               read_adc(&(Ch1.Temperatur[0]));
>               nextChannel=ULast1;
>               break;

beim nächsten Aufruf gehts also weiter mit ULast1


>       case CurrCh1:  read_adc(&Ch1.Strom[2]);
>               Ch1.Strom[2]&=0x0F;
>               read_adc(&Ch1.Strom[1]);
>               read_adc(&Ch1.Strom[0]);
>               nextChannel=ULast1;
>               break;

auch hier gehts beim nächsten mal weiter mit ULast1


>       case ULast1:  read_adc(&Ch1.Spannung[2]);
>               Ch1.Spannung[2]&=0x0F;
>               read_adc(&Ch1.Spannung[2]);
>               read_adc(&Ch1.Spannung[2]);
>               nextChannel=TempCh1;
>               break;

und wieder von vorne, beim nächsten mal mit TempCh1.

Wenn du ausserhalb nie nextChannel auf CurrCh1 setzt, wirst du den Fall 
das nextChannel irgendwann CurrCh1 ist, nie haben. Damit sind schon mal 
Ch1.Strom[2], [1], [0] ohne Werte. Ich vermute mal im Fall TempCh1 
sollte nextChannel eigentlich auf CurrCh1 gesetzt werden und nicht auf 
ULast1

Aus Analogiegründen sieht auch das seltsam aus:

>       case ULast1:  read_adc(&Ch1.Spannung[2]);
>               Ch1.Spannung[2]&=0x0F;
>               read_adc(&Ch1.Spannung[2]);
>               read_adc(&Ch1.Spannung[2]);
>               nextChannel=TempCh1;
>               break;

In allen anderen Fällen verteilst du die ADC Ergebnisse jeweils auf [2], 
[1], [0]. Nur hier nicht. Hier landen alle 3 Ergebnisse in [2], die 
letzte Zuweisung gewinnt.

[Antny (Gast)]

Hallo danke für die schnelle Antwort, joa das ist "richtig" so weil ich 
den Code gerade komplett durchgeplügt habe um das System zu verstehen 
was da schief geht, allerdings bisher erfolglos.

Hier nocheinmal der komplette Code, TempCh1 usw. sind defines, später 
wenn es irgendwann mal laufen sollte... ist die switch/case- Struktur 
dazu gedacht periodisch die Kanäle durchzuscannen und die Structs zu 
aktualisieren.

#include <avr/io.h>
#include <stdio.h>
#include "uart.h"
#include "config.h"
#include "twi.h"
#include <util/delay.h>
#include <avr/interrupt.h>
#define Startbit 2
#define Single_Ended 1

struct KanalMesswerte {
   uint8_t Temperatur[3];
   uint8_t Strom[3];
   uint8_t Spannung[3];
   uint8_t PWM_Fan;
   uint16_t temp_old;
   int16_t t_delta_sum;
}Ch1={"","","",PWM_Start,0},Ch2={"","","",PWM_Start,0};

uint8_t nextChannel=TempCh1; //Erster Kanal, welcher gelesen wird
volatile uint8_t ADCprescaler=0;


//Schreiben und konfigurieren des ADC's
void ADC_Messung ()
{
  if (ADCprescaler >6)
  {
    PORT_CS_ADC_STEU &= ~( 1 << PIN_CS_ADC_STEU );
    SPCR =  (1<<SPE)  |  //SPI- Enable
        (1<<MSTR)  |
        (1<<SPR1);  //Set Master

    _delay_ms(10);

    switch (nextChannel)
    {
      case TempCh1:  read_adc(&(Ch1.Temperatur[2]));
              Ch1.Temperatur[2]&=0x0F;
              read_adc(&(Ch1.Temperatur[1]));
              read_adc(&(Ch1.Temperatur[0]));
              nextChannel=TempCh1;
              break;
//      case CurrCh1:  read_adc(&Ch1.Strom[2]);
//              Ch1.Strom[2]&=0x0F;
//              read_adc(&Ch1.Strom[1]);
//              read_adc(&Ch1.Strom[0]);
//              nextChannel=ULast1;
//              break;
//      case ULast1:  read_adc(&Ch1.Spannung[2]);
//              Ch1.Spannung[2]&=0x0F;
//              read_adc(&Ch1.Spannung[2]);
//              read_adc(&Ch1.Spannung[2]);
//              nextChannel=TempCh1;
//              break;
      /*case TempCh2:  Ch2.Temperatur[2]=0x0F & read_adc();
              Ch2.Temperatur[1]=read_adc();
              Ch2.Temperatur[0]=read_adc();
              nextChannel=CurrCh2;
              break;
      case CurrCh2:  Ch2.Strom[2]=0x0F & read_adc();
              Ch2.Strom[1]=read_adc();
              Ch2.Strom[0]=read_adc();
              nextChannel=ULast2;
              break;
      case ULast2:  Ch2.Spannung[2]=0x0F & read_adc();
              Ch2.Spannung[1]=read_adc();
              Ch2.Spannung[0]=read_adc();
              nextChannel=TempCh1;
              //luefterregelung(&msb,&Ch1.PWM_Fan,1,&Ch1.temp_old,&Ch1.t_delta_sum);
              break;*/
    }
    PORT_CS_ADC_STEU |=  ( 1 << PIN_CS_ADC_STEU );
    _delay_ms(1);
    SPDR=nextChannel;
    while ( ! ( SPSR&(1<<SPIF) ) );
    SPDR=nextChannel;
    while ( ! ( SPSR&(1<<SPIF) ) );

    uint16_t msb=Ch1.Temperatur[2]<<11;
    msb|=Ch1.Temperatur[1]<<3;
    msb|=Ch1.Temperatur[0]>>3;
    ADCprescaler=0;
    uart_sendc('A');
    uart_sendc(&Ch1.Temperatur[2]);
    uart_sendc(&Ch1.Temperatur[1]);
    uart_sendc(&Ch1.Temperatur[0]);
//    uart_sendc('B');
//    uart_sendc(&Ch1.Spannung[2]);
//    uart_sendc(&Ch1.Spannung[1]);
//    uart_sendc(&Ch1.Spannung[0]);

    //Einstellen des mächsten Kanals



  }
}

void read_adc (uint8_t *value)
{
  PORT_CS_ADC_STEU &= ~( 1 << PIN_CS_ADC_STEU );
  SPCR =  (1<<SPE)  |  //SPI- Enable
      (1<<MSTR)  |
      (1<<SPR1);  //Set Master
  SPDR=0b10101010;
  while ( ! ( SPSR&(1<<SPIF) ) );
  *value=SPDR;
}





//Interrupt alle 37 ms
ISR(TIMER0_OVF_vect)
{
  TCNT0 = 0;
  ADCprescaler++;

}

[Antny (Gast)]

Und hier noch ein Foto von dem was ausgegeben wird, wie es weiter geht 
kann man unten sehen ich habe mal versucht das mit Linien zu 
verdeutlichen.

[Karl H. (kbuchegg)]

Ich kann mir nicht vorstellen, dass dich hier

    uart_sendc(&Ch1.Temperatur[2]);


das Low-Byte der Adresse interessiert, an der Ch1.Temperatur[2] 
gespeichert ist.

Hat dein Compiler da gar nichts dazu gesagt, dass die Funktion einen 
char haben möchte und du ihm aber einen unsigned char* (mit der Betonung 
auf *) anbietest?

[Johann L. (gjlayde)]

Antny schrieb:

struct KanalMesswerte {
    uint8_t Temperatur[3];
    uint8_t Strom[3];
    uint8_t Spannung[3];
    uint8_t PWM_Fan;
    uint16_t temp_old;
    int16_t t_delta_sum;
}Ch1={"","","",PWM_Start,0},Ch2={"","","",PWM_Start,0};

Was sollen denn dies Initialiser??? Du initialisierst ein uint8-Array 
mit char*?

[Karl H. (kbuchegg)]

Johann L. schrieb:
> Antny schrieb:
>
>

> struct KanalMesswerte {
>     uint8_t Temperatur[3];
>     uint8_t Strom[3];
>     uint8_t Spannung[3];
>     uint8_t PWM_Fan;
>     uint16_t temp_old;
>     int16_t t_delta_sum;
> }Ch1={"","","",PWM_Start,0},Ch2={"","","",PWM_Start,0};
>

>
> Was sollen denn dies Initialiser??? Du initialisierst ein uint8-Array
> mit char*?

Ist zwar ein wenig seltsam, letzten Endes werden dadurch nur 
Temperatur[0], Strom[0] und Spannung[0] mit '\0' initialisiert, die 
anderen haben keine definierten Werte.

ist auch nichts anders als

   char Dummy[20] = "Hallo World";

[Mark B. (markbrandis)]

Vielleicht hab ich mich verzählt und brauche eine neue Brille, aber ich 
sehe da sechs Member in der struct und nur fünf davon werden 
initialisiert. Wenn schon denn schon.

[Antny (Gast)]

@Mark uups richtig, der Ordnung halber...
@Karl- Heinz mensch auf dich ist immer noch Verlass, bei meinen ersten 
Gehversuchen mit dem µP hast du mir schon super das mit Bittoggeln mit 
Hilfe von und/ oder erklärt, und jetzt wieder den Fehler gefunden 
dankeschön :)

[Andreas F. (aferber)]

Karl heinz Buchegger schrieb:
> Ist zwar ein wenig seltsam, letzten Endes werden dadurch nur
> Temperatur[0], Strom[0] und Spannung[0] mit '\0' initialisiert, die
> anderen haben keine definierten Werte.

Doch, die Variablen haben static storage duration, deshalb werden alle 
nicht explizit aufgeführten Elemente implizit mit 0 initialisiert.

Andreas

[Andreas F. (aferber)]

Andreas Ferber schrieb:
> Doch, die Variablen haben static storage duration, deshalb werden alle
> nicht explizit aufgeführten Elemente implizit mit 0 initialisiert.

Bzw., um mich selbst zu ergänzen: selbst bei automatic storage duration 
werden bei teilweiser Initialisierung (sei es wegen weniger Elementen in 
einer Initializer-Liste oder eben auch einem Stringliteral) die 
restlichen Elemente implizit mit 0 initialisiert (bzw. NULL-Pointer bei 
Pointertypen). Siehe Absatz 21 in Abschnitt 6.7.8 im C99-Standard.

Andreas

[Andreas F. (aferber)]

Ingrid^2: die Initialisierung des uint8_t-Arrays mit einem Stringliteral 
ist allerdings eigentlich nicht zulässig, nur "array of character 
type" darf so initialisiert werden, und "character type" sind 
ausschliesslich "char", "signed char" und "unsigned char".

Vermutlich wird man aber den Bastard C-Compiler From Hell suchen müssen, 
um einen zu finden, bei dem der Unterschied in diesem Fall wirklich 
relevant wird.

Andreas

[Florian L. (muut)]

Andreas Ferber schrieb:
> ist allerdings eigentlich nicht zulässig, nur "array of character
> type" darf so initialisiert werden, und "character type" sind
> ausschliesslich "char", "signed char" und "unsigned char".

wird daran liegen das uint8_t eigentlich ein unsigned char ist. 
(typedef)

Über den Sinn und Unsinn, einen Zahlenwert mit dem NUL-Symbol zu 
initialisieren, kann man sich streiten :-D

[Karl H. (kbuchegg)]

Andreas Ferber schrieb:

> Doch, die Variablen haben static storage duration, deshalb werden alle
> nicht explizit aufgeführten Elemente implizit mit 0 initialisiert.

Ah ja. Du hast natürlich recht.
Darauf hab ich jetzt nicht geachtet.

Ich geh normalerweise nach dem Grundsatz vor:
Wenn ich initialisiere, dann alles.

Das ist mir nämlich zu mühsam, bei einer Strukturänderung im Nachhinein 
wieder abzählen zu müssen, was ich initialisiert habe, was der Compiler 
für mich macht und was keine Initialisierung bekommt.

[Karl H. (kbuchegg)]

Florian Löffler schrieb:

> Über den Sinn und Unsinn, einen Zahlenwert mit dem NUL-Symbol zu
> initialisieren, kann man sich streiten :-D

Ich denke das Problem wird gewesen sein, dass die korrekte Syntax nicht 
bekannt war

struct KanalMesswerte {
   uint8_t Temperatur[3];
   uint8_t Strom[3];
   uint8_t Spannung[3];
   uint8_t PWM_Fan;
   uint16_t temp_old;
   int16_t t_delta_sum;
}

 Ch1 = { { 0, 0, 0 },
         { 0, 0, 0 },
         { 0, 0, 0 },
         PWM_Start,
         0,
         0
       },

 Ch2 = { { 0, 0, 0 },
         { 0, 0, 0 },
         { 0, 0, 0 },
         PWM_Start,
         0,
         0
       };

Das ich von solchen 'all in one line' Rundumschlägen nicht viel halte, 
genausowenig von diesem 'möglichst alles in eine Deklaration quetschen 
und am besten noch ein typedef davor' Kompaktschreibweisen, sollte 
diejenigen die meinen Stil mittlerweile kennen, nicht weiter wundern.

[Andreas F. (aferber)]

Karl heinz Buchegger schrieb:
> Ich geh normalerweise nach dem Grundsatz vor:
> Wenn ich initialisiere, dann alles.
>
> Das ist mir nämlich zu mühsam, bei einer Strukturänderung im Nachhinein
> wieder abzählen zu müssen, was ich initialisiert habe,

In aller Regel benutze ich bei struct-Initializern inzwischen 
grundsätzlich Designatoren, damit die Initialisierung komplett 
unabhängig von der Abfolge der struct-Felder wird. Erspart schonmal ein 
paar Fehlerquellen.

> was der Compiler
> für mich macht und was keine Initialisierung bekommt.

Wie gesagt, selbst bei automatischen Variablen, sobald auch nur ein 
einziges Feld initialisiert wird, wird der gesamte Rest (rekursiv) in 
jedem Fall auf 0 initialisiert, exakt wie es bei statischen Objekten der 
Fall wäre. Oder anders: ein "teilweise" initialisiertes Aggregate gibt 
es in C nur im Zusammenhang mit dynamischer Speicherverwaltung, in allen 
anderen Fällen wird entweder ganz oder garnicht initialisiert.

Andreas

[Andreas F. (aferber)]

Beispiel wär vielleicht auch nicht schlecht. Die vorliegende struct 
würde ich persönlich ungefähr so initialisieren:

struct KanaMesswerte {
    [...]
};

struct KanalMesswerte Ch1 = { .PWM_Fan = PWM_Start },
                      Ch2 = { .PWM_Fan = PWM_Start };

Andreas

[Antny (Gast)]

Hey, das finde ich gut, dass ihr mir gleich noch gezeigt habt wie es 
besser geht, in der Tat ich wußte einfach nicht wie man sie anders 
initialisieren kann.