Pointer in STRUCT wollen einfach nicht funktionieren! (AVR-GCC)

[Patrick Z. (zorpat)]

Hallo Leute,
ich werd noch blöde.. :)

Ich versuche gerade einen kleinen Modbus Slave aufzubauen und kämpfe 
gerade mit Pointern die auf structs mit weiteren Pointern ziele 
sollten..

Klingt kompliziert, ist es aber nur wenn man den Wald vor lauter Bäumen 
nicht mehr sieht.. :P

Die Deklaration meines Structs:

typedef struct {
  uint8_t slave_address;
  uint8_t function_code;
  uint16_t *data;
  uint16_t crc;
} modbus_frame_t;

Nun würde ich gerne dieses Struct in meiner Empfangsroutine mit Daten 
füllen..
Dafür teste ich gerade einen Code wie den folgenden.. Der Compiler 
meldet (natürlich) keinen Fehler da der Code an sich ja "richtig" ist, 
aber nicht das gewünschte Ergebnis bringt da ich die Pointer usw. 
sicherlich falsch benutze..

void Modbus_RX_handler(void)
{
   modbus_frame_t *modbus_frame;

   modbus_frame = malloc(sizeof(*modbus_frame));
   modbus_frame->data = malloc(sizeof(*modbus_frame->data) * 3);
   modbus_frame->slave_address = RS485_getc();
   modbus_frame->function_code = RS485_getc();

   *modbus_frame->data = RS485_getc();
   *modbus_frame->data++ = RS485_getc();

   free(modbus_frame->data);
   free(modbus_frame);
}

Zum Testen versuche ich mit folgendem Code ein "Echo" an den Absender zu 
versenden..

   RS485_putc(*modbus_frame->data);
   RS485_putc(*modbus_frame->data++);

Die 1. Zeile funktioniert, aber die 2. nicht!

Wie mache ich das Richtig?
Ich komm einfach nicht drauf..

Bitte helft mir! :)

Vielen Dank!

[Rufus Τ. F. (rufus)]

Das Problem liegt nicht an der Struktur, sondern an Deiner Verwendung 
der Pointer.

> *modbus_frame->data = RS485_getc();

Und worauf zeigt "modbus_frame->data"?

Wann hast Du diesen Pointer initialisiert?


Auch wenn Du ihn initialisierst (also irgendwo Speicher zur Verfügung 
stellst und den Pointer auf dessen Anfang zeigen lässt), geht Dein 
nächster Zugriff in die Hose.

> *modbus_frame->data++ = RS485_getc();

Und wie kommst Du danach an die Adresse, auf die modubus_frame->data 
vorher zeigte?

So jedenfalls nicht.

> RS485_putc(*modbus_frame->data);
> RS485_putc(*modbus_frame->data++);

++ ist der Inkrement-Operator. Du willst aber hier modbus_frame->data 
nicht erhöhen, jedenfalls nicht dauerhaft. Denn dann weißt Du nicht 
mehr, wo der Speicherbereich anfängt (oder aufhört), auf den dieser 
Pointer eigentlich zeigen sollte.

[Rene H. (Gast)]

De Zordo P. schrieb:
> modbus_frame = malloc(sizeof(*modbus_frame));

Das sollte wohl eher:

modbus_frame = malloc(sizeof(modbus_frame_t));

sein.

De Zordo P. schrieb:
> modbus_frame->data = malloc(sizeof(*modbus_frame->data) * 3);

???
Das kann auch nicht sein. Der Inhalt von modbus_frame->data ist erstens 
unbestimmt und zweitens macht die ganze Zeile null Sinn.

Wieviel willst Du denn für data allokieren? Das ist aus diesen Zeilen 
wirklich nicht erkennbar ausser das es drei mal soviel sein sollte ;-).

Grüsse,
René

[Daniel A. (daniel-a)]

modbus_frame ist global, und du reservierst dafür dynamisch speicher, am 
anfang einer funktion, befüllst es dann mit werten, nur um am ende der 
funktion die daten ohne sie zu nutzen freizugeben?!?

Du speicherst 2 werte in einem array aus drei uint16_t werten?

Was willst du erreichen???

[Daniel A. (daniel-a)]

Rene H. schrieb:
>> malloc(sizeof(*modbus_frame->data) * 3);
>
> ???
> Das kann auch nicht sein.

Das ist korrekt. Es findet keine dereferenzierung statt, die grösse 
eines uint16_t (type von *modbus_frame->data) muss zur kompiletime von 
compiler ermittelt werden.

[Patrick Z. (zorpat)]

Daniel A. schrieb:
> modbus_frame ist global, und du reservierst dafür dynamisch speicher, am
> anfang einer funktion, befüllst es dann mit werten, nur um am ende der
> funktion die daten ohne sie zu nutzen freizugeben?!?

Für den Moment ja, ich habe den gesamten Code "verworfen" welcher im 
"Modbus_RX_handler(void)" das Protokoll auswertet. Ich wollte mich 
Schritt für Schritt dem Problem nähern.

Die Ausgabe:

   RS485_putc(*modbus_frame->data);
   RS485_putc(*modbus_frame->data++);

wird aktuell auch noch im "Modbus_RX_handler(void)" ausgeführt.
Also so in etwa:

void Modbus_RX_handler(void)
{
   modbus_frame = malloc(sizeof(*modbus_frame));
   modbus_frame->data = malloc(sizeof(*modbus_frame->data) * 3);

   modbus_frame->slave_address = RS485_getc();
   modbus_frame->function_code = RS485_getc();
   *modbus_frame->data = RS485_getc();
   *modbus_frame->data++ = RS485_getc();

   RS485_putc(*modbus_frame->data);
   RS485_putc(*modbus_frame->data++);

   free(modbus_frame->data);
   free(modbus_frame);
}

Kannst du mir "quick and dirty" sagen wie ich das Problem lösen kann?
Ich versuche dann die Pointer besser zu verstehen..

[Patrick Z. (zorpat)]

Rene H. schrieb:
> De Zordo P. schrieb:
>> modbus_frame->data = malloc(sizeof(*modbus_frame->data) * 3);
>
> ???
> Das kann auch nicht sein. Der Inhalt von modbus_frame->data ist erstens
> unbestimmt und zweitens macht die ganze Zeile null Sinn.

Ich wollte damit die eine "data"-Variable innerhalb des Structs mit der 
Größe von 3 x uint16_t reservieren/allokieren.
Insgesamt lese ich also 2+2*3+2=10 Bytes in meinen Struct.

Danach würde ich (für den Moment) folgeden Werte in die Variablen 
einlesen wollen:

   slave_address = 0x01
   function_code = 0x02
   data = 0x0003
   data+1 = 0x0004
   data+2 = 0x0005
   crc16 = 0xFFFF

und natürlich auch wieder darauf zugreifen wollen.. :)

[Patrick Z. (zorpat)]

Rene H. schrieb:
> De Zordo P. schrieb:
>> modbus_frame = malloc(sizeof(*modbus_frame));
>
> Das sollte wohl eher:
> modbus_frame = malloc(sizeof(modbus_frame_t));
>
> sein.

Sollte lt. meiner Recherche aber funktionieren..? :/
Ja?

[Patrick Z. (zorpat)]

Rufus Τ. Firefly schrieb:
>> *modbus_frame->data++ = RS485_getc();
>
> Und wie kommst Du danach an die Adresse, auf die modubus_frame->data
> vorher zeigte?
>
> So jedenfalls nicht.
>
>> RS485_putc(*modbus_frame->data);
>> RS485_putc(*modbus_frame->data++);
>
> ++ ist der Inkrement-Operator. Du willst aber hier modbus_frame->data
> nicht erhöhen, jedenfalls nicht dauerhaft. Denn dann weißt Du nicht
> mehr, wo der Speicherbereich anfängt (oder aufhört), auf den dieser
> Pointer eigentlich zeigen sollte.

Ist das in meinem Beispiel nicht egal?
Bei jedem Austritt von "Modbus_RX_handler(void)" wird der Speicher eh 
wieder freigegeben..? Oder stehe ich wieder aufm Schlauch.. ;)

[Daniel A. (daniel-a)]

De Zordo P. schrieb:
> Kannst du mir "quick and dirty" sagen wie ich das Problem lösen kann?

Ich bevorzuge saubere lösungen ;)
Zuerst würde ich die dynamische Speicherreservierung entfernen.
Ist der Rückgabewert von RS485_getc() ein uint8_t oder char?

static const int MAX_DATAS = 3;

typedef struct {
  uint8_t slave_address;
  uint8_t function_code;
  uint16_t data[MAX_DATAS];
  uint16_t crc;
} modbus_frame_t;

void Modbus_RX_handler(void){
   modbus_frame_t modbus_frame;

   modbus_frame.crc=0xffff;

   modbus_frame.slave_address = RS485_getc();
   modbus_frame.function_code = RS485_getc();

   modbus_frame.data[0] = (
    (((unsigned char)RS485_getc())<<8) |
     ((unsigned char)RS485_getc())
   );
   modbus_frame.data[1] = (
    (((unsigned char)RS485_getc())<<8) |
     ((unsigned char)RS485_getc())
   );
   modbus_frame.data[2] = (
    (((unsigned char)RS485_getc())<<8) |
     ((unsigned char)RS485_getc())
   );

   RS485_putc(modbus_frame->data[0]);
   RS485_putc(modbus_frame->data[0]>>8);
   RS485_putc(modbus_frame->data[1]);
   RS485_putc(modbus_frame->data[1]>>8);
   RS485_putc(modbus_frame->data[3]);
   RS485_putc(modbus_frame->data[3]>>8);
}

Ich nehme jetzt an, dass RS485_getc ein char zurückgibt
RS485_putc und RS485_getc funktionieren,
RS485_getc die richtigen werte empfängt,
Die bytes in networkbyteorder (big endian) ankommen,
RS485_getc auf ankommende daten wartet.

Ziemlich viele annahmen...

[Panda (Gast)]

Musst du wirklich dynamisch allocieren?

Eine Variable auf dem Stack ist nach verlassen der Funktion auch wieder 
weg.

void Modbus_RX_handler(void)
{
  modbus_frame_t modbus_frame;
  modbus_frame_t* ptr_modbus_frame = &modbus_frame;
  uint16_t dataarray[3];

   modbus_frame.data = dataarray;
   modbus_frame.slave_address = RS485_getc();
   modbus_frame.function_code = RS485_getc();
   modbus_frame.data[0] = RS485_getc();
   modbus_frame.data[1] = RS485_getc();
   
   RS485_putc(((uint16_t*)&ptr_modbus_frame->data)[0]);
   RS485_putc(((uint16_t*)&ptr_modbus_frame->data)[1]);
}

[Patrick Z. (zorpat)]

Deine Annahmen sind gut.. ;)
Ich arbeite grundsätzlich mit "saueberen" Variablen, keine "chars" oder 
"int" welche jeder Compiler etwas anders interpretiert.. :P

   RS485_putc(uint8_t c);
   RS485_getc(void); -> gibt einen "uint8_t" zurück

Zur dynamischen Alloziierung..
1. Ich muss den Speicher leider sehr sehr effizient nutzen.. da ich nur 
einen "kleinen" ATTINY2313 benutzen kann (das Board ist vorgegeben) und 
ich soll damit einen Modbus Slave bauen welcher einige der 
Modbus-Befehle können muss. Für alle ist er sowiso zu klein..

2. kenne ich die gesamte übertragene Datenpaket erst nachdem ich das 1. 
"data", sprich das 3. und 4. Byte gelesen und ausgewerten habe. In 
diesen beiden Bytes steht die weitere Größe des Pakets, welches auch 30 
Bytes groß sein könnte.

Deshalb gehe ich davon aus, dass ich nur mit der dynamischen 
Alloziierung weiterkommen kann. Mir geht sonst das RAM schon beim 
Kompilieren aus! :)

[Daniel A. (daniel-a)]

De Zordo P. schrieb:
> Ist das in meinem Beispiel nicht egal?
> Bei jedem Austritt von "Modbus_RX_handler(void)" wird der Speicher eh
> wieder freigegeben..? Oder stehe ich wieder aufm Schlauch.. ;)

Da war noch ein Fehler. Er hatte dieses Schema:

  char data = malloc(1);
  *data++; // Hier wird der pointer inkrementiert, nicht die daten, auf welcher dieser zeigt
  free(data); // falsche adresse

[Patrick Z. (zorpat)]

Daniel A. schrieb:
> De Zordo P. schrieb:
>> Ist das in meinem Beispiel nicht egal?
>> Bei jedem Austritt von "Modbus_RX_handler(void)" wird der Speicher eh
>> wieder freigegeben..? Oder stehe ich wieder aufm Schlauch.. ;)
>
> Da war noch ein Fehler. Er hatte dieses Schema:
>

>   char data = malloc(1);
>   *data++; // Hier wird der pointer inkrementiert, nicht die daten, auf 
> welcher dieser zeigt
>   free(data); // falsche adresse
>

Ein Fehler weniger.. ;)

   *modbus_frame->data--;
   free(modbus_frame->data);

Danke!

[Daniel A. (daniel-a)]

De Zordo P. schrieb:
> Deshalb gehe ich davon aus, dass ich nur mit der dynamischen
> Alloziierung weiterkommen kann. Mir geht sonst das RAM schon beim
> Kompilieren aus! :)

Durch dynamischen Alloziierung wird der RAM auch nicht grösser.
Dafür bekommt man Memory Fragmentation. Können mehrere Datenpakete 
gleichzeitig empfangen werden? Müssen die Daten gespeichert werden?
Werden die Daten auf dem uc ausgewertet? Dann kann dafür eine State 
Maschine verwendet werden, und das Speichern der Daten wird 
möglicherweise überflüssig.
Oder werden die Daten weitergesendet?
Dann könnte man einen Ring Buffer verwenden und die Daten Häppchenweise 
weitersenden.

[Daniel A. (daniel-a)]

De Zordo P. schrieb:
> Ein Fehler weniger.. ;)
>    *modbus_frame->data--;
>    free(modbus_frame->data);

Das ist auch gefährlich, das sollte man besser so lösen:

  uint16_t current_value = modbus_frame->data;
  *current_value++ = 1;
  *current_value++ = 2;
  free(modbus_frame->data);

[Patrick Z. (zorpat)]

Daniel A. schrieb:
> De Zordo P. schrieb:
>> Deshalb gehe ich davon aus, dass ich nur mit der dynamischen
>> Alloziierung weiterkommen kann. Mir geht sonst das RAM schon beim
>> Kompilieren aus! :)
>
> Durch dynamischen Alloziierung wird der RAM auch nicht grösser.
> Dafür bekommt man Memory Fragmentation. Können mehrere Datenpakete
> gleichzeitig empfangen werden? Müssen die Daten gespeichert werden?
> Werden die Daten auf dem uc ausgewertet? Dann kann dafür eine State
> Maschine verwendet werden, und das Speichern der Daten wird
> möglicherweise überflüssig.
> Oder werden die Daten weitergesendet?
> Dann könnte man einen Ring Buffer verwenden und die Daten Häppchenweise
> weitersenden.

Die Fragmentierung sollte nicht eintreten, da ich eben bei allen void's 
(momentan nur 1 zum Empfang) den Speicher am Anfang reserviere und am 
Ende wieder freigebe.
Aktiv werden wohl höchstens 2 Reservierungen meines Structs aktiv sein.

Die Daten werden nicht gespeichert oder an andere Routinen 
weitergegeben, sie werden im "Modbus_RX_handler(void)" 
verarbeitet/ausgewertet und ein ACK über den RS485 Bus zurückgesandt => 
danach DESTROY.

Ein Ringbuffer exiertiert bereits im "ISR(USART_RX_vect)". Im Interrupt 
möchte ich aber keine externen Aufrufe machen.

:)

[Rene H. (Gast)]

uint16_t *current_value = ...

Hast das * vergesssen, sonst gibts nichts zu dereferenzieren.

[Patrick Z. (zorpat)]

Daniel A. schrieb:
> De Zordo P. schrieb:
>> Ein Fehler weniger.. ;)
>>    *modbus_frame->data--;
>>    free(modbus_frame->data);
>
> Das ist auch gefährlich, das sollte man besser so lösen:
>

>   uint16_t current_value = modbus_frame->data;
>   *current_value++ = 1;
>   *current_value++ = 2;
>   free(modbus_frame->data);
>

Ja dann habe ich mein Problem doch wieder oder?
Ich muss eine "doppelte" Speicherreservierung machen, 1. im Struct und 
danach 2. im "Modbus_RX_handler(void)".
Das würde bedeuten bei 10 Byte großen "data" effektiv 20 Bytes zu 
verbraten..
Die habe ich einfach nicht beim Tiny.. :(

[Patrick Z. (zorpat)]

Rene H. schrieb:
> uint16_t *current_value = ...
>
> Hast das * vergesssen, sonst gibts nichts zu dereferenzieren.

==> klingt besser, jap!

[Stefan R. (srand)]

Daniel A. schrieb:
> Durch dynamischen Alloziierung wird der RAM auch nicht grösser.
> Dafür bekommt man Memory Fragmentation.

Vor allem belegen die Verwaltungsinformationen selbst auch Platz.

Bei Stackvariablen wird nur der Stackpointer 
inkrementiert/dekrementiert.

[Patrick Z. (zorpat)]

Stefan Rand schrieb:
> Daniel A. schrieb:
>> Durch dynamischen Alloziierung wird der RAM auch nicht grösser.
>> Dafür bekommt man Memory Fragmentation.
>
> Vor allem belegen die Verwaltungsinformationen selbst auch Platz.
>
> Bei Stackvariablen wird nur der Stackpointer
> inkrementiert/dekrementiert.

OIOIOI... ÄÄÄÄm gibts'n Beispiel dafür? :)
Ich meine ein komplettes, damit ich Zeile für Zeile verstehen kann wie 
die Abfolge genau funktioniert... :/ Bitte?? :O

[Daniel A. (daniel-a)]

Wenn ein RingBuffer existiert, und die Daten nur ausgewertet werden 
sollen, würde ich die Daten ganz anders auswerten. Zuerst eine schleife 
der form:

while(datasAvaiable)
  (*parse)(RS485_getc());

Danach eine Statemachine zum parsen der daten:

void parse_slave_address(const char);

void(*parse)(const char) = &parse_slave_address;

typedef union {
  uint8_t u8[4];
  uint16_t u16[2];
  uint32_t u32;
  int8_t i8[4];
  int16_t i16[2];
  int32_t i32;
} temporary_t;

temporary_t tempdatas;

typedef struct {
  uint8_t slave_address;
  uint16_t crc;
} required_t;

required_t required;

enum function_codes_t {
  FUNCTION_A = 0,
  FUNCTION_B = 1,
};

void parse_function_code(const char c){
  switch((enum function_codes_t)c){
    case FUNCTION_A:
      parse = &parse_func_a_blabla;
    break;
    case FUNCTION_B:
      parse = &parse_func_b_blabla;
    break;
  }
}

void parse_slave_address(const char c){
  required.slave_address = c;
  parse = parse_function_code;
}

[Daniel A. (daniel-a)]

De Zordo P. schrieb:
> OIOIOI... ÄÄÄÄm gibts'n Beispiel dafür? :)
> Ich meine ein komplettes, damit ich Zeile für Zeile verstehen kann wie
> die Abfolge genau funktioniert... :/ Bitte?? :O

Der Stack Pointer ist ein Register. Der Compiler kümmert sich darum, 
deshalb sollte man in C nicht daran herumbasteln.

[Patrick Z. (zorpat)]

Daniel A. schrieb:
> Wenn ein RingBuffer existiert, und die Daten nur ausgewertet werden
> sollen, würde ich die Daten ganz anders auswerten. Zuerst eine schleife
> der form:
>

> while(datasAvaiable)
>   (*parse)(RS485_getc());
>

>
> Danach eine Statemachine zum parsen der daten:
>

> 
> void parse_slave_address(const char);
> 
> void(*parse)(const char) = &parse_slave_address;
> 
> typedef union {
>   uint8_t u8[4];
>   uint16_t u16[2];
>   uint32_t u32;
>   int8_t i8[4];
>   int16_t i16[2];
>   int32_t i32;
> } temporary_t;
> 
> temporary_t tempdatas;
> 
> typedef struct {
>   uint8_t slave_address;
>   uint16_t crc;
> } required_t;
> 
> required_t required;
> 
> enum function_codes_t {
>   FUNCTION_A = 0,
>   FUNCTION_B = 1,
> };
> 
> void parse_function_code(const char c){
>   switch((enum function_codes_t)c){
>     case FUNCTION_A:
>       parse = &parse_func_a_blabla;
>     break;
>     case FUNCTION_B:
>       parse = &parse_func_b_blabla;
>     break;
>   }
> }
> 
> void parse_slave_address(const char c){
>   required.slave_address = c;
>   parse = parse_function_code;
> }
> 
>

Wirklich "lesenswert".. ich vertehe leider fast nur Bahnhof.. ;)
Ich glaube du Programmierst doch schon etwas länger als ich.. ;D

Ne, Scherz..

WARUM will mir denn niemand meinen Ansatz verfolgen und mir ein Beispiel 
geben? :/

Ist doch nicht so schwer, oder?

Nochmals kurz als Erweiterung meiner Erklärung..

- Mein Empfangspuffer (Ringpuffer) wird per Interrupt beschrieben.
- Sofern folgende Bedingungen eintreten (Daten im Ringpuffer > 0 && 
Interframetimeout abgelaufen) dann wird "Modbus_RX_handler(void)" 
aufgerufen

- Im "Modbus_RX_handler(void)" soll nun das Byte 1 und 2 in die 
Variablen geschrieben werden.

   modbus_frame->slave_address = RS485_getc();
   modbus_frame->function_code = RS485_getc();

- weiters wird nun das 3. Byte gelesen (nur das LSB der "data" Variable 
vorerst.. in einem 2. späteren Schritt wird um 8 Bits 
geschoben..blahblah.. es wären ja 2 Bytes, da uint16_t, aber das 
vernachlässigen wir im Moment)

   modbus_frame->data[0] = RS485_getc(); // data[0] = data ohne offset (z.B. 0x01)

"modbus_frame->data[0] = 0x01" würde bedeuten es kommt nur noch 1 
weiteres Datenbyte (eigentlich 2, da uint16, aber das vernachlässigen 
wir aber wieder!)

- Im 4. Byte stehen dann die eigentlichen Nutzdaten

   modbus_frame->data[1] = RS485_getc(); // data[2] = data + 1 offset (z.B. 0xFF)

- Im 5. und 6. Byte kommt dann noch das CRC dazu

   modbus_frame->crc = (uint16_t)RS485_getc()<<8;
   modbus_frame->crc |= RS485_getc();

Und das alles mit "DYNAMISCHER SPEICHERALLOZIIERUNG"! :))
Bitte! :)

[Patrick Z. (zorpat)]

Folgendes "scheint" bedingt zu Funktionieren..
Ist das so auch richtig Programmiert?

void Modbus_RX_handler(void)
{
   modbus_frame_t *modbus_frame;
   modbus_frame = malloc(sizeof(*modbus_frame)); // Allocate memory dynamically
   modbus_frame->data = malloc(sizeof(*modbus_frame->data) * 2);

   modbus_frame->slave_address = RS485_getc();
   modbus_frame->function_code = RS485_getc();
   RS485_putc(modbus_frame->slave_address);
   RS485_putc(modbus_frame->function_code);

   uint16_t *data_value1 = modbus_frame->data;
   *data_value1 = RS485_getc();
   RS485_putc(*data_value1);
   *data_value1 = RS485_getc(); // Warum wird hier der Pointer automatisch inkrementiert???
   RS485_putc(*data_value1);

   free(modbus_frame->data);
   free(modbus_frame);
}

[Rene H. (Gast)]

Wie kommst Du darauf, dass der Pointer automatisch inkrementiert wird?

Wenn ja, stellt sich mir die Frage auch.

Grüsse,
René

[alex (Gast)]

De Zordo P. schrieb:
> Ist das so auch richtig Programmiert?

Jede Benutzung von malloc ohne ein Betriebssystem mit Speichermanagement 
ist Selbstmord und wird über kurz oder lang zu Problemen führen (hier 
wohl eher "über kurz").

De Zordo P. schrieb:
> WARUM will mir denn niemand meinen Ansatz verfolgen und mir ein Beispiel
> geben? :/

Weil die Mehrheit der hier Beteiligten dich schon darauf hingewiesen 
hat, dass das das falsche Werkzeug ist.

Sieh es so: Du bist im Wasser und wir werfen dir Rettungsringe aus 
schwimmfähigem Material hin, aber du verlangst einen Rettungsring aus 
Zement; ja, er ist rund, aber er wird dir nicht helfen.

------

So genug belehrt, jetzt auch noch etwas Hilfestellung :)

De Zordo P. schrieb:
> *data_value1 = RS485_getc(); // Warum wird hier der Pointer automatisch
> inkrementiert???

Er sollte da nicht inkrementiert werden (und wird es auch nicht). Wie 
kommst du zu dem Schluss?

[Karl H. (kbuchegg)]

De Zordo P. schrieb:

>    uint16_t *data_value1 = modbus_frame->data;
>    *data_value1 = RS485_getc();
>    RS485_putc(*data_value1);
>    *data_value1 = RS485_getc(); // Warum wird hier der Pointer
> automatisch inkrementiert???

Wird er doch gar nicht.

>    RS485_putc(*data_value1);

Aus welchem Grund benutzt du Pointer Syntax, wenn du gar nicht musst und 
damit auf Kriegsfuss stehst?

...
   uint16_t *data_value1 = modbus_frame->data;

   data_value1[0] = RS485_getc();
   RS485_putc( data_value1[0] );

   data_value1[1] = RS485_getc();
   RS485_putc( data_value1[1] );

   free(modbus_frame->data);
   free(modbus_frame);
...

Du scheinst den Sinn hinter der lokalen temporären Variablen nicht 
verstanden zu haben. Der Sinn dieser Variablen besteht darin, dass du 
Manipulationen an diesem Pointer machen kannst, ohne dir den originalen 
Wert (so wie du ihn von malloc bekommen hast) zu zerstören. Denn du 
brauchst diesen Wert wieder, um ihn an free zu übergeben!
Und nein. Den Pointer schamlos zu inkrementieren um dann hinterher eine 
entsprechende Anzahl an Dekrements zu machen, ist eine ganz blöde Idee.

1) benutzt du Array Index Syntax, um mit dem Pointer zu arbeiten, würde 
man die Hilfsvarable nicht benötigen. Die Schreibweisen mit der 
Hilfsvariablen würden ein wenig einfacher und man kann auch davon 
ausgehen, dass der Compiler sie sowieso wieer rauswerfen wird. Daher: 
benutze die Hilfsvariablen!
Aber wenns denn sein muss, dann kann man natürlich auch dem Compiler 
etwas Arbeit abnehmen, auch wenn der das durchaus selber kann.

void Modbus_RX_handler(void)
{
   modbus_frame_t *modbus_frame;
   modbus_frame = malloc(sizeof(*modbus_frame)); // Allocate memory dynamically
   uint8_t dataLen = sizeof(*modbus_frame->data) * 2;
   modbus_frame->data = malloc( dataLen );

   modbus_frame->slave_address = RS485_getc();
   modbus_frame->function_code = RS485_getc();

   RS485_putc(modbus_frame->slave_address);
   RS485_putc(modbus_frame->function_code);

   for( uint8_t i = 0; i < dataLen; i++ )
   {
     modbus_frame->data[i] = RS485_getc();
     RS485_putc( modbus_frame->data[i] );
   }

   free( modbus_frame->data );
   free( modbus_frame );
}

2) du solltest den Rückgabewert von malloc prüfen, ob du auch wirklich 
Speicher bekommen hast. je kleiner der Prozessor umso 'ungerner' benutzt 
man dynamische Allokierung.

[Karl H. (kbuchegg)]

Ich geh mal davon aus, dass die ganze Funktion erst mal ein Test ist. 
Denn im Moment ist es völlig sinnlos die Werte zwischenzuspeichern, wenn 
du damit sowieso nichts anderes tust, als sie per RS485 weiterzureichen. 
Da jedes einzelne Byte 1:1 weiter gereicht wird, braucht man da 
überhaupt nichts grossartig zwischenspeichern. Man muss nur wissen, 
wieviele Bytes es sein werden und genau so viele Bytes werden dann vom 
Bus geholt und per RS485 weitergegeben. Das funktioniert sogar dann, 
wenn sich die komplette Anzahl an Bytes erst aus dem Function Code 
ergibt, der empfangen wurde.

[Rene H. (Gast)]

Karl Heinz schrieb:
> 2) du solltest den Rückgabewert von malloc prüfen, ob du auch wirklich
> Speicher bekommen hast. je kleiner der Prozessor umso 'ungerner' benutzt
> man dynamische Allokierung.

Naja, man weiss das dynamisches allokieren bei einem Mikrocontroller in 
einem Desaster endet. Immer!
Aber diese Erfahrung müssen manche selber machen (ein Grund, weshalb man 
kein klassisches c++ verwenden kann).

Wenn man im Speicherdenken nicht Erfahren ist, dann eh.

Grüsse,
R.

[Patrick Z. (zorpat)]

Rene H. schrieb:
> Wie kommst Du darauf, dass der Pointer automatisch inkrementiert wird?
>
> Wenn ja, stellt sich mir die Frage auch.
>
> Grüsse,
> René

Ich sende folgende Hex-Bytes via RS485 an den Controller:
AA BB CCDD EEFF (sind also 6 Bytes)

modbus_frame->slave_address = 0xAA
modbus_frame->function_code = 0xBB
modbus_frame->data[0] = 0x00CC
modbus_frame->data[1] = 0x00DD
modbus_frame->crc = 0xEEFF

Ich schnalls nicht..
Ich inkrementiere den Pointer NICHT und trotzdem bekommen ich die 
richtigen Werte als Echo..

   uint16_t *data_value1 = modbus_frame->data;
   *data_value1 = RS485_getc();
   RS485_putc(*data_value1);
   *data_value1 = RS485_getc(); // ==> 0xCC
   RS485_putc(*data_value1); // ==> 0xDD

Warum denn das?

[Patrick Z. (zorpat)]

alex schrieb:
> Jede Benutzung von malloc ohne ein Betriebssystem mit Speichermanagement
> ist Selbstmord und wird über kurz oder lang zu Problemen führen (hier
> wohl eher "über kurz").
>
> De Zordo P. schrieb:
>> WARUM will mir denn niemand meinen Ansatz verfolgen und mir ein Beispiel
>> geben? :/
>
> Weil die Mehrheit der hier Beteiligten dich schon darauf hingewiesen
> hat, dass das das falsche Werkzeug ist.

Ach Mensch.. :/

Mehrere Wege führen nach Rom.. ;)
Warum kann ichs nicht mit Pointern lösen?
Das ist doch keine Hexerei oder? :))))))

[Karl H. (kbuchegg)]

Rene H. schrieb:
> Karl Heinz schrieb:
>> 2) du solltest den Rückgabewert von malloc prüfen, ob du auch wirklich
>> Speicher bekommen hast. je kleiner der Prozessor umso 'ungerner' benutzt
>> man dynamische Allokierung.
>
> Naja, man weiss das dynamisches allokieren bei einem Mikrocontroller in
> einem Desaster endet. Immer!

Absolut.

Gerade in diesem Beispiel, in dem die Daten in derselben Funktion wieder 
freigegeben werden, ist malloc so unnotwendig wie ein Kropf.

Version 1:

void foo()
{
  uint16_t* pI = malloc( 20 * sizeof(uint16_t) );

  // stellvertretend für eine Bearbeitung
  for( i = 0; i < 20; i++ )
    *pI[i] = i;

  free( pI );
}

Version 2:

void foo()
{
  uint16_t pI[20];

  // stellvertretend für eine Bearbeitung
  for( i = 0; i < 20; i++ )
    *pI[i] = i;
}

Der Unterschied zwischen Version 1 und Version 2 besteht darin, dass 
Version 1 mehr Speicher verbraucht.

> Aber diese Erfahrung müssen manche selber machen.

Absolut. Blöd ist nur, dass sowas schwer zu findende Fehler ergibt. 
Daher der gute Rat, wenigstens den Rückgabewert von malloc zu prüfen, 
auch wenn das nicht alle Fehler abfangt. Wenn der Stack in den Heap 
hineinwächst, dann kann man alle Hoffnung fahren lassen, dass da 
irgendwas sinnvolles passieren wird, wenn der Speicher zu Ende ist.

[Karl H. (kbuchegg)]

De Zordo P. schrieb:

> Mehrere Wege führen nach Rom.. ;)

Mag sein.
Aber deiner führt in die Hölle und nicht nach Rom.

Wenn der Speicher zu Ende ist, dann ist er zu Ende. Und mit malloc geht 
das schneller als ohne.

[Rene H. (Gast)]

Das verstehe nun nur begrenzt: du derefenzierst durch die direkte 
Indexierung. Ich verstehe deshalb nicht wo der Pointer inkrementiert 
werden soll.

Grüsse,
René

[Karl H. (kbuchegg)]

De Zordo P. schrieb:

> Ich inkrementiere den Pointer NICHT und trotzdem bekommen ich die
> richtigen Werte als Echo..

Logisch.
Weil du nur 1 Byte von deinem Data Feld benutzt.
Dort schreibst du das empfangene Byte hinein, echost es sofort und für 
das nächste übertragene Byte benutzt du dann wieder dasselbe 
Speicherbyte.

Logisch, das das alles funktioniert, wenn du nacheinander die Daten alle 
im selben Byte speicherst und sofort echost.

Nur dass du dann eben am Ende nur mit dem zuletzt übertragenen Byte 
dastehst und nicht mit allen.


Um das mal zu veranschaulichen. Was du gemacht hast, ist eine 
komplizierte Version von

   uint8_t c;

   c =  RS485_getc();
   RS485_putc( c );

   c =  RS485_getc();
   RS485_putc( c );

   c =  RS485_getc();
   RS485_putc( c );

   c =  RS485_getc();
   RS485_putc( c );

   c =  RS485_getc();
   RS485_putc( c );

selbstverständlich ist dein Echo korrekt. Das Problem beginnt erst dann, 
wenn du ganz zum Schluss den Wert des ersten empfangenen Bytes benötigen 
würdest.

[Alexander I. (a-irrgang)]

De Zordo P. schrieb:
> alex schrieb:
>> Jede Benutzung von malloc ohne ein Betriebssystem mit Speichermanagement
>> ist Selbstmord und wird über kurz oder lang zu Problemen führen (hier
>> wohl eher "über kurz").
>>
>> De Zordo P. schrieb:
>>> WARUM will mir denn niemand meinen Ansatz verfolgen und mir ein Beispiel
>>> geben? :/
>>
>> Weil die Mehrheit der hier Beteiligten dich schon darauf hingewiesen
>> hat, dass das das falsche Werkzeug ist.
>
> Ach Mensch.. :/
>
> Mehrere Wege führen nach Rom.. ;)
> Warum kann ichs nicht mit Pointern lösen?
> Das ist doch keine Hexerei oder? :))))))

Natürlich kannst du es mit Pointern lösen. Aber Pointer != malloc.

typedef struct {
  uint8_t slave_address;
  uint8_t function_code;
  uint16_t *data;
  uint16_t crc;
} modbus_frame_t;

struct modbus_frame_t xyz;
struct modbus_frame_t* pXyz = &xyz

Arbeite ab dort mit pXyz und du benutzt Pointer, aber ohne dynamische 
Allokierung.

[Karl H. (kbuchegg)]

Alexander I. schrieb:

> Arbeite ab dort mit pXyz und du benutzt Pointer, aber ohne dynamische
> Allokierung.

Kann man machen. Muss man aber nicht.
Bringt das irgendwas?
Ausser dass das Ego des Programmierers gestreichelt wird, weil er einen 
Pointer benutzt hat - nein, bringt nichts.

Und Zordo. Genau das ist der springende Punkt. Man benutzt Pointer, wenn 
man muss und wenn es was bringt. Man benutzt sie nicht, weil es so 
grossen Spass macht.

[Alexander I. (a-irrgang)]

Natürlich nicht. Aber wenn er Pointer so unbedingt benutzen möchte, habe 
ich da nichts dagegen. Mir ging es mehr darum auszudrücken, dass die 
Nutzung von Pointern nicht die dynamische Speicherallokierung erfordert. 
Und die Lösung wird vom OP doch leider massiv forciert und ich versuche 
ihm diesen Spuk auszutreiben ;) Muss ja nicht jeder auf die harte Tour 
lernen :)

Um es ganz klar zu machen:
In meinen Augen ist der erste Schritt zu einer stabilen und effizienten 
Lösung die Entfernung jedweder malloc und zugehöriger free Aufrufe 
(dadruch entfallen die Pointer von selbst). Danach kann man sich dem 
eigentlichen algorithmischen Problem widmen.

[Karl H. (kbuchegg)]

> Die Fragmentierung sollte nicht eintreten, da ich eben bei allen
> void's (momentan nur 1 zum Empfang) den Speicher am Anfang reserviere
> und am Ende wieder freigebe.
> Aktiv werden wohl höchstens 2 Reservierungen meines Structs aktiv sein.

Oh, oh.

Gerade dann läufst du mit fliegenden Fahnen in die Fragmentierung 
hinein.

Der ganze Ansatz ist Quatsch. Da ist es noch besser, definierst die 
selber ein globales Byte-Array, von dem sich die Funktionen ihren Teil 
holen können, wenn was da ist. So hast du wenigstens eine Abschätzung 
darüber, wie klamm der Speicher mit globalen Variablen und Stack 
wirklich ist, und du kannst dir eine Art Handle-basierte 
Speicherverwaltung bauen, so dass du zur Not auch im laufenden Betrieb 
die Daten reorganisieren kannst um aus der Fragmentierungsfalle 
rauszukommen. Einfach ist das allerdings nicht zu programmieren. Da 
sollte man schon recht gut sein, um das hinzukriegen.

[Patrick Z. (zorpat)]

Karl Heinz schrieb:
> Logisch, das das alles funktioniert, wenn du nacheinander die Daten alle
> im selben Byte speicherst und sofort echost.

ach gott bin ich blöde.. ich bastle solange schon da rum dass ich 
wirklich die EINFACHSTEN Dinge NICHT mehr sehe.. OOOOH MANN!

[Patrick Z. (zorpat)]

Karl Heinz schrieb:
> Und Zordo. Genau das ist der springende Punkt. Man benutzt Pointer, wenn
> man muss und wenn es was bringt. Man benutzt sie nicht, weil es so
> grossen Spass macht.

Ich verstehe Euch ja..
ABER(!!), wenn ich dann z.B. in einer weiteren Routine auf den selben 
"Speicher" (Struct) zugreifen möchte kann ich doch einfach einen Pointer 
übergeben und muss nichts neu anlegen und kann den "alten" Struct im 
Speicher direkt ansprechen..

So etwa wie:

void RS485_puts(const char *s)
{
  while(*s)
  {
    RS485_putc(*s++);
  }
}

Das Programm arbeitet nicht nur dieses void ab! Im Moment ja, aber 
später macht der µC viele weitere Sachen wenn keine neuen Daten vom Bus 
anstehen..
Deshalb dachte ich mir eben... Pointer sind die Lösung, da man alles nur 
als Speicheraddressen sieht.

Ist das nicht der BESTE WEG? :/

Gruß!

[Karl H. (kbuchegg)]

De Zordo P. schrieb:

> So etwa wie:
>

> void RS485_puts(const char *s)
> {
>   while(*s)
>   {
>     RS485_putc(*s++);
>   }
> }
>

Aber auch diese Funktion muss man nicht so benutzen

...
  char* data = malloc( 5 ),
  data[0] = 't';
  data[1] = 'e';
  data[2] = 's';
  data[3] = 't';
  data[4] = '\0';

  RS485_puts( data );

  free( data );

man kann, darf und soll die Funktion ruhig auch so benutzen:

  char data[] = "test";
  RS485_puts( data );

bzw. gleich überhaupt

  RS485_puts( "test" );

Nur weil eine Funktion einen Pointer haben will, bedeutet das nicht, 
dass der Aufrufer den entsprechenden Speicher dazu dynamisch allokiert 
haben muss.

> Das Programm arbeitet nicht nur dieses void ab! Im Moment ja, aber
> später macht der µC viele weitere Sachen wenn keine neuen Daten vom Bus
> anstehen..
> Deshalb dachte ich mir eben... Pointer sind die Lösung, da man alles nur
> als Speicheraddressen sieht.

Der Pointer ist nicht das eigentliche Problem. Der malloc ist das 
Problem! DU kannst, wenn es blöd läuft, in Summe genug Speicher frei 
haben aber nicht genug freien Speicher am Stück, um die Anforderung zu 
bedienen!

Beispiel.
Du hast 60 Bytes freien Speicher.

(jedes Kästchen steht für 10 Bytes)
     +--+--+--+--+--+--+
     |  |  |  |  |  |  |
     +--+--+--+--+--+--+

Die erste Anforderung benötigt 10 Bytes. Also werden 10 Bytes reserviert

     +--+--+--+--+--+--+
     |XX|  |  |  |  |  |
     +--+--+--+--+--+--+

Während diese Anforderung noch besteht, kommt die nächste Anforderung 
mit 40 Bytes. Auch diese 40 Bytes werden reserviert

     +--+--+--+--+--+--+
     |XX|XX|XX|XX|XX|  |
     +--+--+--+--+--+--+

Die Bearbeitung der ersten Daten ist erledigt und die 10 Bytes werden 
frei gegeben.

     +--+--+--+--+--+--+
     |  |XX|XX|XX|XX|  |
     +--+--+--+--+--+--+

Die nächste Anforderung braucht 15 Bytes.
rein rechnerisch hast du 20 Bytes frei, denn von den 60 Bytes total sind 
gerade 40 Bytes belegt.

Trotzdem, schau auf die Zeichnung, du hast keine 15 Bytes. Du hast zwar 
20 Bytes frei, aber keine 15 Bytes am Stück! Die 10 freien Bytes vor der 
Reservierung und die 10 Bytes nach der bestehenden Reservierung helfen 
dir nichts.

[Patrick Z. (zorpat)]

Karl Heinz schrieb:
> Beispiel.
> Du hast 60 Bytes freien Speicher.(jedes Kästchen steht für 10 Bytes)
>      +--+--+--+--+--+--+
>      |  |  |  |  |  |  |
>      +--+--+--+--+--+--+
>
> Die erste Anforderung benötigt 10 Bytes. Also werden 10 Bytes reserviert
> +--+--+--+--+--+--+
>      |XX|  |  |  |  |  |
>      +--+--+--+--+--+--+
>
> Während diese Anforderung noch besteht, kommt die nächste Anforderung
> mit 40 Bytes. Auch diese 40 Bytes werden reserviert
> +--+--+--+--+--+--+
>      |XX|XX|XX|XX|XX|  |
>      +--+--+--+--+--+--+
>
> Die Bearbeitung der ersten Daten ist erledigt und die 10 Bytes werden
> frei gegeben.     +--+--+--+--+--+--+
>      |  |XX|XX|XX|XX|  |
>      +--+--+--+--+--+--+
>
> Die nächste Anforderung braucht 15 Bytes.
> rein rechnerisch hast du 20 Bytes frei, denn von den 60 Bytes total sind
> gerade 40 Bytes belegt.
>
> Trotzdem, schau auf die Zeichnung, du hast keine 15 Bytes. Du hast zwar
> 20 Bytes frei, aber keine 15 Bytes am Stück! Die 10 freien Bytes vor der
> Reservierung und die 10 Bytes nach der bestehenden Reservierung helfen
> dir nichts.

DANKE DANKE DANKE! Diese Ausführung bringt mein mentales Chaos doch mal 
in eine gerade Linie! :)

Ich habe meinen Code mal wie folgt "umgeschrieben"..

Was noch fehlt ist die Abfrage ob das Alloziieren überhaupt geklappt 
hat!!

typedef struct {
  uint8_t slave_address;
  uint8_t function_code;
  uint16_t *data;
  uint16_t crc;
} modbus_frame_t;

-----------------------------------------------------------

void Modbus_RX_handler(void)
{
  if (RS485_new_packet)
  {
    modbus_frame_t *modbus_frame;
    modbus_frame = malloc(sizeof(*modbus_frame)); // Ist das so OK und brauchbar?
    
    modbus_frame->slave_address = RS485_getc();
    modbus_frame->function_code = RS485_getc();
    RS485_putc(modbus_frame->slave_address);
    RS485_putc(modbus_frame->function_code);

    modbus_frame->data = malloc(sizeof(*modbus_frame->data) * 3); // Ist das so OK?
    
    uint16_t *data_value = modbus_frame->data;
    data_value[0] = RS485_getc();
    RS485_putc(data_value[0]);

    data_value[1] = RS485_getc();
    RS485_putc(data_value[1]);
    
    modbus_frame->crc = (uint16_t)RS485_getc()<<8;
    modbus_frame->crc |= RS485_getc();
    RS485_putc(modbus_frame->crc>>8);
    RS485_putc(modbus_frame->crc);
    
    /* TESTING */
    RS485_putc(RS485_new_packet);
    RS485_putc(sizeof(*modbus_frame));
    RS485_putc(sizeof(*modbus_frame->data));
    /* ------- */
    
    free(modbus_frame->data);
    free(modbus_frame);
    RS485_new_packet = 0; // reset "new packet" flag
  }
}

Soweit so gut, also es funktioniert bis hierher.. (Ich HOFFE auch 
"immer", nicht dass ich irgendwann meine Variable ins "NICHTS" pointe.. 
:D

Zur Fragmentierung..
Wenn ich nun aber wirklich nur 2 Funktionen habe welche Speicher 
dynamisch alloziieren, dann kann ich das doch einfach Überschauen.. also 
ich meine z.B.
- 15 Byte in "Modbus_RX_handler(void)" alloziieren (diese 15 Bytes 
werden irgendwann mal mit "free()" freigegeben)
- 20 Bytes in "Modbus_TX_handler(void)" alloziieren (diese 20 Bytes 
werden irgendwann mal mit "free()" freigegeben)

Der Kompiler sagt mir ich hätte noch 50 Bytes freien RAM-Speicher, somit 
komme ich nicht "außerhalb" des maximalen Speichers..oder?

Auf was muss ich aber nun achten? Wenn ich irgendwo lokale Variablen 
benutze, kann es denn dann passieren, dass z.B. gerade diese in meine 35 
Bytes "reinpfuschen"?

Viele Fragen, doch ich muss es genau wissen bevor ich mich an die 
Codeausarbeitung wage.

[Patrick Z. (zorpat)]

Karl Heinz schrieb:
> Mag sein.
> Aber deiner führt in die Hölle und nicht nach Rom.

xD

[Patrick Z. (zorpat)]

Karl Heinz schrieb:
> man kann, darf und soll die Funktion ruhig auch so benutzen:
>   char data[] = "test";
>   RS485_puts( data );
>
> bzw. gleich überhaupt  RS485_puts( "test" );
>
> Nur weil eine Funktion einen Pointer haben will, bedeutet das nicht,
> dass der Aufrufer den entsprechenden Speicher dazu dynamisch allokiert
> haben muss.

Ja korrekt.. das mache ich auch immer so, dass ich die 
"Kurzschreibweise"

   RS485_puts( "test" );

beim Aufruf benutze. :)

[Alexander I. (a-irrgang)]

Die Lösung mit malloc wird dich pro malloc 2 Byte Speicher kosten 
[http://www.nongnu.org/avr-libc/user-manual/malloc.html - Abschnitt 
"Implementation details"]. Und zusätzlich Nerven beim debuggen 
unerklärlicher Fehler. Weiterhin wird es deinen Code unnötig 
verkomplizieren.

Das angehängte Beispiel tut im Prinzip das selbe und zeigt dir, wie du 
auf den selben "handler-lokalen Speicher" auch in weiteren 
Unterfunktionen zugreifen kannst.
Da deine Handler-Funktion nur einmal aufgerufen wird ist der Speicher 
nur so lange auf dem Stack belegt, wie die Funktion läuft und ist danach 
weg.

static const uint8_t MAX_DATA = 2;

typedef struct {
  uint8_t slave_address;
  uint8_t function_code;
  uint16_t data[MAX_DATA];
  uint16_t crc;
} modbus_frame_t;


void Modbus_RX_handler()
{
  // Hier ist modbus_frame lokal in der Funktion auf dem Stack
  // Jeder Aufruf der Funktion = Platz auf dem Stack angelegt.
  struct modbus_frame_t modbus_frame;

  modbus_frame.slave_address = RS485_getc();
  modbus_frame.function_code = RS485_getc();

  RS485_putc(modbus_frame.slave_address);
  RS485_putc(modbus_frame.function_code);

  for(uint8_t i = 0; i < MAX_DATA; i++)
  {
    modbus_frame.data[i] = RS485_getc();
    RS485_putc(modbus_frame.data[i]);
  }
   
  verify_crc(&modbus_frame);
  func2(&modbus_frame);
}

void verify_crc(struct modbus_frame_t* pModbus_frame)
{
  // Hier arbeitest du auf der selben Variable (dem selben Speicher) wie im Handler
  // du hast nicht erneut Speicher für das struct verbraucht
  ... tu was mit modbus_frame...
  ... z.B.: unit16_t crc = pModbus_frame->data[0] ^ pModbus_frame->data[1]; ...
}

void func2(struct modbus_frame_t *pModbus_frame)
{
  ... tu was anderes mit modbus_frame...
}

[Patrick Z. (zorpat)]

Alexander I. schrieb:
> static const uint8_t MAX_DATA = 2;

MAX_DATA weiß ich nicht vor Abarbeitung der ersten 2 Bytes.. MAX_DATA 
kann auch 20 oder mehr Bytes bzw. uint16 sein.. :/

> uint16_t data[2];

Hier das Selbe.. ich kann vor Abarbeitung der ersten 2 Bytes nicht sagen 
ob ich mit 2 x uint16_t oder 10 x uint16_t auskomme..

Deshalb dachte ich eben ich mache das STRUCT und definiere dann erst den 
"uint16_t data[xxxx];" sobald ich weiß wie große denn mein Datenstrom 
sein wird! :)

Oder kann bzw. soll ich die Definition von Struct auch während der 
Laufzeit machen?

[Alexander I. (a-irrgang)]

Dann belegst du MAX_DATA mit deinem definierten Maximum. Auch dynamisch 
wirst du diese Zahl nie überschreiten können. Im Gegenteil: durch den 
"Speicherverlust" pro malloc gewinnst du so gegenüber der 
malloc-Variante sogar ein paar Bytes.

[Karl H. (kbuchegg)]

De Zordo P. schrieb:
> Alexander I. schrieb:
>> static const uint8_t MAX_DATA = 2;
>
> MAX_DATA weiß ich nicht vor Abarbeitung der ersten 2 Bytes.. MAX_DATA
> kann auch 20 oder mehr Bytes bzw. uint16 sein.. :/
Was du nicht weisst, das ist wieviele Btes du zur Bearbeitung einer 
Busmessage tatsächlich benötigst.

Du definierst aber, wie lange eine Busmessage MAXIMAL sein kann.
D.h. jede MEssage hat mglw. ein wenig Speicherverschnitt, den es aus dem 
Array theoretisch nicht benötigt.

Aber: Es ist immer noch besser eine Maximal-Größe garantiert bedienen zu 
können, als (wenn es blöd läuft) bei kleineren Messages schon W.O. geben 
zu müssen.

Ein µC ist kein Desktop Rechner. Ein µC muss immer zuverlässig 
funktionieren. Eine Meldung "Momentan stehen keine Resourcen zur 
Verfügung, versuchen sie die Operation später noch einmal" ist auf einem 
irgendwo eingebauten µC nicht akzeptabel. Da ist es schon besser, wenn 
seine mögliche Leistung etwas geringer als das theoretische Maximum ist, 
wenn dafür diese Leistung aber 100% garantiert ist und nicht auf einem 
wackeligem "ja nach Wasserstand in der Donau kann das funktionieren oder 
auch nicht" beruht.

Hast du 60 Bytes Speicher zur Verfügung und kannst du maximal 2 Messages 
im Speicher haben, dann kann jede Message ein Maximum von 30 Bytes 
beanspruchen. Und zwar immer und zu jedem Zeitpunkt. Das ist zwar 
schlechter, als die Lösung, in der auch eine Message mit 40 Bytes 
verarbeitet werden könnte aber immer noch besser als die streikende 
Variante, in der du eine 15 Byte Message nicht mehr behandeln kannst, 
weil der Speicher fragmentiert ist.

[Patrick Z. (zorpat)]

Alexander I. schrieb:
> Dann belegst du MAX_DATA mit deinem definierten Maximum. Auch dynamisch
> wirst du diese Zahl nie überschreiten können. Im Gegenteil: durch den
> "Speicherverlust" pro malloc gewinnst du so gegenüber der
> malloc-Variante sogar ein paar Bytes.

Hm... also soll ich wirklich ein Maximum definieren welches "nie" 
komplett erreicht werden kann, ok. Es klingt vernünftig, da ich 
ansonsten eh den Speicher doppelt belegen würde wenn ich kein 
Speichermanagement programmiere.. und dann geht mir der Flash auch aus, 
denn für das Speichermanagement gehen wohl auch einige Bytes drauf.. ;)

[Bastler (Gast)]

MAX_DATA hat seinen Namen davon, daß es das Maximum der möglichen 
Datenlänge ist. Das ist fix!

[Patrick Z. (zorpat)]

Karl Heinz schrieb:
> Ein µC ist kein Desktop Rechner. Ein µC muss immer zuverlässig
> funktionieren. Eine Meldung "Momentan stehen keine Resourcen zur
> Verfügung, versuchen sie die Operation später noch einmal" ist auf einem
> irgendwo eingebauten µC nicht akzeptabel. Da ist es schon besser, wenn
> seine mögliche Leistung etwas geringer als das theoretische Maximum ist,
> wenn dafür diese Leistung aber 100% garantiert ist und nicht auf einem
> wackeligem "ja nach Wasserstand in der Donau kann das funktionieren oder
> auch nicht" beruht.

Richtig!
Ich glaub der Lösungsansatz von Alexander ist sehr gut geeignet.. und 
das MAX_DATA muss ich einfach definieren.. größer als mein 
RS485_RX_BUFFER kanns eh nicht sein.. ;)

Sollte ich eigentlich meinen Ringpuffer bereits mit dem Struct versehen 
damit ich keinen "doppelten" Speicher benötige, oder?

[Patrick Z. (zorpat)]

Meine Empfangsroutine in der "RS485.c"-Datei:

ISR(USART_RX_vect)
{
  uint8_t tmp_head = 0;
  tmp_head = (rx_head + 1) % RX_BUFFER_SIZE;
  if (tmp_head == rx_tail) {
    // buffer overflow error!
    RS485_clear_RX_buffer();
  } else 
  {
    rx_buffer[rx_head] = UDR;
    //RS485_putc(UDR); // DEBUG
    rx_head = tmp_head;
    RS485_packet_complete_timeout = SetDelay(RS485_PACKET_TIMEOUT); // set new hard timeout value
    RS485_hard_packet_timeout_value = SetDelay(RS485_TIMEOUT); // set new inter frame timeout value
  }
}

Könnte ich nun in der "Modbus.h"-Datei mein Struct als "extern" 
definieren und direkt in der "ISR(USART_RX_vect)" mein Struct füllen?
Im "Modbus_RX_handler" kann ich dann im Prinzip "nur" mehr parsen..?

:)

[Alexander I. (a-irrgang)]

Im Header sollte man nichts als extern definieren mit Ausnahme von "C", 
das auf Controllern aber irrelevant ist.

An der Stelle müsste dein Interrupthandler deutlich mehr Logik bekommen, 
um zu wissen, was das empfangene Byte denn ist und wo es in die Struktur 
hin gehört.

Im Prinzip gefällt mir dein Interrupthandler also von seinen Aufgaben so 
wie er ist ganz gut. Ich würde dir empfehlen die Umsetzung mit diesem 
Interrupthandler zu starten und bei der Implementierung eine Anpassung 
im Hinterkopf zu behalten, so dass du dich in keine Sackgasse 
entwickelst.

Solltest du dann wirklich an die Grenzen stoßen, kann man über 
Optimierungen am Interrupthandler nachdenken.

[Patrick Z. (zorpat)]

Alexander I. schrieb:
> Im Header sollte man nichts als extern definieren mit Ausnahme von "C",
> das auf Controllern aber irrelevant ist.
>
> An der Stelle müsste dein Interrupthandler deutlich mehr Logik bekommen,
> um zu wissen, was das empfangene Byte denn ist und wo es in die Struktur
> hin gehört.
>
> Im Prinzip gefällt mir dein Interrupthandler also von seinen Aufgaben so
> wie er ist ganz gut. Ich würde dir empfehlen die Umsetzung mit diesem
> Interrupthandler zu starten und bei der Implementierung eine Anpassung
> im Hinterkopf zu behalten, so dass du dich in keine Sackgasse
> entwickelst.
>
> Solltest du dann wirklich an die Grenzen stoßen, kann man über
> Optimierungen am Interrupthandler nachdenken.

Vielen Dank für das Lob! (Sonst gabs heute fast nur Tadel.. :P)

Mein ISR hat schon einiges an Logik bekommen..
1. Hard Timeout zwischen 2 Übertragungen - Pakete löschen (NICHT 
BLOCKIERENDER Aufruf!)

   RS485_packet_complete_timeout = SetDelay(RS485_PACKET_TIMEOUT);

Es passiert manchmal, dass ein Master im Netzwerk ein Paket nicht 
vollständig Überträgt da z.B. gerade ein neuer Busteilnehmer im Betrieb 
auf den Bus aufgeschaltet worden ist. (Sozusagen "Hot-Swap"). Dabei kann 
es sein, dass einige der Clients nur einen Teil eines gesamten 
Datenstrings empfangen haben. Der µC würde ewig in externen 
Programmschleifen stehenbleiben. Durch meinen Code wird das Paket aber 
nach 500ms einfach Verworfen. Diese Zeit ist ja eine Ewigkeit in der 
Kommunikationsschicht mehrerer Controller.

2. Soft Timeout, auch "Inter Frame Timeout" genannt (NICHT BLOCKIERENDER 
Aufruf!)

   RS485_hard_packet_timeout_value = SetDelay(RS485_TIMEOUT);

Dieser Timeout ruft den eigentlichen "Modbus_RX_handler()" auf der dann 
die gelesenen Bytes parst. Der Timeout läuft 5ms nach dem Empfang des 
letzten Bytes bzw. Frame ab. Bei 19200 Baud ist das mehr als genug..

Das Besondere, das ganze funktioniert OHNE das Programm zu BLOCKIEREN..
Hierzu generiere ich mit Hilfe des Timer1 einen 1ms Interrupt der im 
Hintergrund eine Laufvariable hochzählt..