Strukt initialisieren probleme

[Chandler B. (chandler)]

Hallo,
ich möchte den Status von verschiedenen Relais in einet Struktur 
speichern

typedef enum
{
    RELAIS_1,
    RELAIS_2,
    RELAIS_3,
    RELAIS_4,
    RELAIS_5,
    NOF_RELAIS
} Relais_te;

typedef struct
{
    const uint8_t pin;
    bool state_b;
} Relais_ts;


static Relais_ts relais_as[NOF_RELAIS] = {
    [RELAIS_1] = { .pin = (1 << PINB1), .state_b = false },
    [RELAIS_2] = { .pin = (1 << PINB2), .state_b = false },
    [RELAIS_3] = { .pin = (1 << PINB3), .state_b = false },
    [RELAIS_4] = { .pin = (1 << PINB4), .state_b = false },
    [RELAIS_5] = { .pin = (1 << PINB5), .state_b = false }
};

jedoch bekomme ich bei der Initialisierung für jeden Pin den Wert 0 
heraus.
Ich verstehe es aber nicht, woran es noch liegen könnte.

Was mache ich bei der initialisierung falsch?

static void relaisInit_v(void);
static void relaisInit_v(void)
{
    char c;
    for (uint8_t relais_e=0; relais_e<NOF_RELAIS; relais_e++)
    {
        Relais_ts* relais_ps;
        relais_ps = &relais_as[RELAIS_5];
        c = relais_e + '0';
        uart_characterPut(c);
        c = relais_ps->pin + '0';
        uart_characterPut(c);

        UNSETPIN(PORTB, relais_ps->pin);
        relais_ps->state_b = false;
    }
}

hier bekomme dann als ausgabe
0010203040

[Klaus H. (klummel69)]

Liegt vermutlich daran, dass du in Zeile 8 immer das gleiche Element 
ansprichst.

relais_ps = &relais_as[RELAIS_5];

Da solte vermutlich der index relais_e rein?

[Chandler B. (chandler)]

Klaus H. schrieb:
> Liegt vermutlich daran, dass du in Zeile 8 immer das gleiche
> Element
> ansprichst.
>
> relais_ps = &relais_as[RELAIS_5];
>
> Da solte vermutlich der index relais_e rein?

Ja, da sollte relais_e rein.
Trotzdem sollte keine 0 rauskommen.
Auch wenn ich relais_e eintrage, bekomme ich immer 0 zurück.

Da 1 << PINB5 auch nicht mehr in einem character reinpasst das auch noch 
ein wenig angepasst.

... ...
        if (relais_ps->pin == 0)
        {
            c = 0 + '0';
        }
        else
        {
            c = 1 + '0';
        }
        //c = relais_ps->pin + '0';
        uart_characterPut(c);

[A. S. (rava)]

passt denn 1 << PINX in ein uint8?

[Klaus H. (klummel69)]

Das riecht ziemlich nach einem Overflow.

Sind die PIN Nummern >= 8 ?
Speichere mal direkt PINBx ohne Shift und schau dir die Ausgabe aus.

Hast Du die Warnings des Compilers ausgeschaltet?

Was nutzt Du denn für einen Compiler/Plattform?

[Peter D. (peda)]

Die Initialisierung funktioniert jedenfalls:

int main(void)
{
  PORTB = relais_as[RELAIS_4].pin;
  a4:  80 e1         ldi  r24, 0x10  ; 16
  a6:  85 b9         out  0x05, r24  ; 5
  PORTB = relais_as[RELAIS_5].pin;
  a8:  80 e2         ldi  r24, 0x20  ; 32
  aa:  85 b9         out  0x05, r24  ; 5

[Andreas M. (amesser)]

Möglicherweise wird relais_as beim Anlaufen von der runtime nicht sauber 
initialisiert? Man könnte mal in den de-assemblierten Code schauen um 
das zu prüfen.

[Oliver S. (oliverso)]

Chandler B. schrieb:
> Hallo,
> ich möchte den Status von verschiedenen Relais in einet Struktur
> speichern

Du postest in der Rubrik "PC-Programmierung", der Code lässt aber auf 
einen AVR schliessen.

Welcher Compiler in welcher Version für welche Zielhardware ist es denn?

Oliver

[Chandler B. (chandler)]

Oliver S. schrieb:
> Du postest in der Rubrik "PC-Programmierung", der Code lässt aber auf
> einen AVR schliessen.
>
> Welcher Compiler in welcher Version für welche Zielhardware ist es denn?

oh ?, tatsache :/
Ja, es geht auch um einem AVR auf einem Arduino Board (atmega328p)

als compiler benutze ich den avr-gcc compiler und flashe das ganze mit 
avrdude über isp auf dem atmega.

Klaus H. schrieb:
> Sind die PIN Nummern >= 8 ?
> Speichere mal direkt PINBx ohne Shift und schau dir die Ausgabe aus.
Die Pins sind PINB0=0, PINB1=1, PINB2=2, ..., PINB7=7 also sowieso 
kleiner als 8 und ich benutze auch nur bis PINB5

Ich muss das morgen noch einmal testen, wenn ich wieder zu hause bin.

[Sebastian W. (wangnick)]

Passiert bei dieser vereinfachten Initialisierung dasselbe?

static Relais_ts relais_as[NOF_RELAIS] = {
    {1 << PINB1},
    {1 << PINB2},
    {1 << PINB3},
    {1 << PINB4},
    {1 << PINB5}
};

LG, Sebastian

[Peter D. (peda)]

Nur mal Interesse halber, welcher tiefere Sinn steckt dahinter, die 
Pinzugriffe derartig zu verschwurbeln.
D.h. wie soll die praktische Anwendung aussehen?

Vermutlich wird der Compiler dadurch gezwungen, die Zugriffe memory 
mapped zu machen, also nicht einfach über direkte Bitbefehle und auch 
nicht atomar (mögliche Konflikte beim Setzen in Interrupts).

[Oliver S. (oliverso)]

Chandler B. schrieb:
> als compiler benutze ich den avr-gcc compiler

Schön, und welche Version genau? Windows oder Linux?

Oliver

[Jörg W. (dl8dtl) (Moderator)]

Bitte poste mal ein compilierbares Minimalbeispiel, sowohl den 
Sourcecode als auch das bei dir compilierte ELF-File.

[Klaus H. (klummel69)]

Peter D. schrieb:
> Vermutlich wird der Compiler dadurch gezwungen, die Zugriffe memory
> mapped zu machen, also nicht einfach über direkte Bitbefehle und auch
> nicht atomar (mögliche Konflikte beim Setzen in Interrupts).

Vermutlich. Wobei gcc durchaus einige Varianten erkennt und entsprechend 
sbi oder cbi einsetzt.

Würde hier aber auch empfehlen statt der Maske eher die Pin-Nummer zu 
speichern und im Code "PORTB |= (1<< x.pin)"... zu nutzen.

[Jörg W. (dl8dtl) (Moderator)]

Klaus H. schrieb:
> Würde hier aber auch empfehlen statt der Maske eher die Pin-Nummer zu
> speichern und im Code "PORTB |= (1<< x.pin)"... zu nutzen.

Ineffizient. Worst case hast du dann zur Laufzeit eine Schleife nur 
dafür, dass du die Bitmaske ausrechnest.

[Klaus H. (klummel69)]

Jörg W. schrieb:
> Ineffizient. Worst case hast du dann zur Laufzeit eine Schleife nur
> dafür, dass du die Bitmaske ausrechnest.

Warum?
Ineeffizient bedeutet unwirtschaftlich oder nicht leistungsfähig.
Aber das gilt im Sinne der Ressourcen nur wenn es von der 
Geschwindigkeit darauf ankommt. Wenn ich beim Programmieren schnell zum 
Ziel komme ist das durchaus "effizient"....

Wenn die Pinnummer statt der Maske gespeichert wird, kann man auch 
direkt die asm Befehle (sbi, cbi ) nutzen. Schneller geht's nicht. Aber 
das würde ich nur machen wenn es aus Performance Gründen Sinn macht.

Goldene Regel der Softwareprogrammierung:
Nie im Voraus manuell optimieren. Nur wenn eine messbare Verbesserung 
daraus entsteht. Aktuelle Compiler können es normalerweise besser.

[Chandler B. (chandler)]

Jörg W. schrieb:
> Bitte poste mal ein compilierbares Minimalbeispiel, sowohl den
> Sourcecode als auch das bei dir compilierte ELF-File.

siehe anhang

Oliver S. schrieb:
> Schön, und welche Version genau? Windows oder Linux?

Windows
avr-gcc (GCC) 5.3.0

[Jörg W. (dl8dtl) (Moderator)]

Klaus H. schrieb:

> Wenn die Pinnummer statt der Maske gespeichert wird, kann man auch
> direkt die asm Befehle (sbi, cbi ) nutzen. Schneller geht's nicht.

Du denkst viel zu sehr in Assembler.

In C gibt es keine Notation für eine Bit-Nummer. Es gibt nur Bitmasken, 
mit denen man logische Verknüpfungen machen kann. Wenn du die Bitmaske 
gleich in der Initialisierung erzeugst, dann ist sie zumindest 
garantiert für den Compiler sichtbar eine Konstante. Erzeugst du sie 
erst später, weil du nur eine Bit-Nummer hast, dann hängt es vom Kontext 
ab, ob der Compiler erkennen kann, dass sie konstant ist. Kann er es 
nicht, läufst du Gefahr, dass dort eine Schleife aus Schiebebefehlen 
produziert wird.

Die Umwandlung einer konstanten Bitmaske in CBI/SBI (oder SBIS/SBIC) ist 
ohnehin Aufgabe des Compilers, die kannst du ihm nicht abnehmen, indem 
du irgendwo nur Bit-Nummern abspeicherst.

Chandler B. schrieb:
> siehe anhang

Der Code sieht korrekt aus. Deine struct steht als Einziges in .data:

Contents of section .data:
 800100 02000400 08001000 2000               ........ .

.data wird auch ordnungsgemäß in den RAM geladen beim Start:

00000074 <__do_copy_data>:
  74:   11 e0           ldi     r17, 0x01       ; 1
  76:   a0 e0           ldi     r26, 0x00       ; 0
  78:   b1 e0           ldi     r27, 0x01       ; 1
  7a:   e6 ed           ldi     r30, 0xD6       ; 214
  7c:   f2 e0           ldi     r31, 0x02       ; 2
  7e:   02 c0           rjmp    .+4             ; 0x84 <__do_copy_data+0x10>
  80:   05 90           lpm     r0, Z+
  82:   0d 92           st      X+, r0
  84:   aa 30           cpi     r26, 0x0A       ; 10
  86:   b1 07           cpc     r27, r17
  88:   d9 f7           brne    .-10            ; 0x80 <__do_copy_data+0xc>

Falls du nicht direkt das ELF programmierst sondern das Hex-File, auch 
das sieht korrekt aus:

:0A02D60002000400080010002000E0

Die Abfrage der einzelnen Pin-Masken ist etwas umständlich, aber 
korrekt:

 1b0:   00 e0           ldi     r16, 0x00       ; 0
 1b2:   11 e0           ldi     r17, 0x01       ; 1
 1b4:   80 e3           ldi     r24, 0x30       ; 48
 1b6:   f8 2e           mov     r15, r24
 1b8:   8f 2d           mov     r24, r15
 1ba:   0e 94 53 00     call    0xa6    ; 0xa6 <uart_characterPut>
 1be:   f8 01           movw    r30, r16
 1c0:   80 81           ld      r24, Z
 1c2:   88 23           and     r24, r24
 1c4:   11 f0           breq    .+4             ; 0x1ca <main+0x4e>
 1c6:   81 e3           ldi     r24, 0x31       ; 49
 1c8:   01 c0           rjmp    .+2             ; 0x1cc <main+0x50>
 1ca:   80 e3           ldi     r24, 0x30       ; 48
 1cc:   0e 94 53 00     call    0xa6    ; 0xa6 <uart_characterPut>
 1d0:   95 b1           in      r25, 0x05       ; 5
 1d2:   f8 01           movw    r30, r16
 1d4:   80 81           ld      r24, Z
 1d6:   89 2b           or      r24, r25
 1d8:   85 b9           out     0x05, r24       ; 5
…

In r30:r31 steht (anfangs) die Adresse 0x100 (aus r16:r17 übernommen). 
Der entsprechende Wert wird zuerst gelesen und auf 0 / !0 verglichen, in 
Abhhängig davon wird '0' oder '1' in r24 geladen und uart_characterPut() 
gerufen. Danach wird PORTB eingelesen, der Wert erneut von 0x100 
gelesen, mit dem Wert aus PORTB verODERt und auf PORTB geschrieben.

[Oliver S. (oliverso)]

Jörg W. schrieb:
> Der Code sieht korrekt aus.

Ja, aber das hier dürfte dann der Quell allen Übels sein:

static volatile unsigned char strTxBuffer[1024];
static volatile unsigned char strRxBuffer[1024];

Oliver

[Klaus H. (klummel69)]

Jörg W. schrieb:
> Du denkst viel zu sehr in Assembler.
Ups das wollte ich eigentlich nicht. Ich wollte ausdrücken dass man 
nicht zu schnell optimieren sollte.

Ist schon eine Weile her, dass ich avr programmiert habe. Das meiste 
wurde mit Bitmasken gemacht. Nur in speziellen Optimierungsfällen 
(Geschwindigkeit einer Pin Ausgabe) hab ich darauf geachtet, dass 
sbi/cbi genutzt wurde.

Hatte nicht mehr daran gedacht dass die Pin-Nummer im Opcode des Befehls 
gespeichert ist. Der Compiler kann es dann nicht in allen Fällen 
auflösen.
Asche auf mein Haupt.

[Jörg W. (dl8dtl) (Moderator)]

Oliver S. schrieb:
> Jörg W. schrieb:
>> Der Code sieht korrekt aus.
>
> Ja, aber das hier dürfte dann der Quell allen Übels sein:

Uff, ja. :-}

2 KiB allein an Puffern für einen Controller, der insgesamt 2 KiB SRAM 
hat.

Klaus H. schrieb:
> Nur in speziellen Optimierungsfällen (Geschwindigkeit einer Pin
> Ausgabe) hab ich darauf geachtet, dass sbi/cbi genutzt wurde.

Ist ja durchaus sinnvoll.

> Hatte nicht mehr daran gedacht dass die Pin-Nummer im Opcode des Befehls
> gespeichert ist.

Ja, leider. Das ist wohl der Preis dafür, dass man diese Befehle in 
einem Taktzyklus abarbeiten kann.