Hallo Zusammen,
ich grüble schon seit geraumer Zeit an einem Problem mit einem Array aus
Strukturen.
Und zwar benötige ich eine Art Lookup Table die zur Programmlaufzeit mit
Daten gefüllt wird.
Mein Problem ist nun, dass das Array zwar im SRAM richtig platziert wird
(durch Deklaration 'static', ansonsten werden Teile des Arrays
überschrieben), aber die Routine beschreibt nur maximal 6 Arrayelemente,
obwohl mit 'MAXVESSELS = 25' 25 Elemente erzeugt worden sind.
Lasse ich die Deklaration 'static' weg, klappt das Beschreiben aller
Elemente, aber die ersten Elemente werden (wahrscheinlich vom Stack)
teilweise wieder überschrieben.
Was kann ich da machen?
Vielen Dank!
MfG, Marten83
Dein Code ist (unnötigerweise) ziemlich kompliziert aufgebaut. Ich bin
noch nicht wirklich davon überzeugt, dass da alles mit rechten Dingen
zugeht.
Was sagt deine Tool-Chain? Wieviel SRAM hast du noch frei?
(Wenn ich mal einen AVR als Prozessor und WinAvr als Toolchain
voraussetze)
Welchen Controller verwendest du?
Wie groß/voll ist denn dein RAM?
7*25 = 175 Bytes RAM für AIS_data_section[MAXVESSELS]
und
83*5 = 415 Bytes für char AISBUFFER [AIS_BUFCOUNT][83];
Das sind nur 2 augenfällige Arrays, wer weiß, was sonst noch so
vergraben ist... :-/
1
/* global */
2
staticuint8_tlist_depth=0;
3
:
4
:
5
/*local variables*/
6
volatileuint8_tout=0;
7
volatileuint8_tmatch=0;
8
volatileuint16_tsection=0;
Du solltest dir das Thema Modifier (insbesondere static und volatile)
nochmal anschauen...
...entschuldigung, natürlich habe ich vergessen die Umgebung klar zu
machen.
Also der µC ist ein Mega644p mit 4kB SRAM wovon (laut AVR STudio) 1,4kB
(~35%) belegt sind.
Jetzt frage ich mich was eine TOOLCHAIN ist?
So wie der Code momentan aussieht, könnte man den Speicherverbrauch
eines AIS_MMSI_lookup_t ganz leicht von 7 Bytes auf 4 Bytes drücken.
Dein Array würde dann nur noch ~die Hälfte Speicher verbrauchen.
* Das occupied Flag braucht keiner. Offenbar gibt es eine ungültige
section-Nummer, nämlich 0xFFFF. Marikert man unbenutze Einträge mit
einer derartigen section-Nummer, erfüllt das den gleichen Zweck wie
das occupied Flag. (Was ist dir eigentlich dabei eingefallen, das als
1-Bit Variable zu machen? Das spart keinen Speicher, weil du keine
weiteren Bit-Variablen hast, zwingt aber den Compiler dazu, die
Zugriffe auf das Flag komplizierter als notwendig zu machen)
* Offenbar gibt es einen fixen Zusammenhand zwischen dem Index des
Eintrags und der Section Nummer.
section = search_sec * 0x40;
Wozu also die section-Nummer selbst speichern? Hat man bei einer
Abfrage durch Suchen eine MMSI gefunden, kann man die zugehörige
section-Nummer auch dort ganz leicht aus dem Index errechnen.
-> die section-Nummer braucht in Wirklichkeit kein Mensch.
Beides zusammen führt dazu, dass du nur noch ein Array von MMSI hast,
pro Eintrag also nur noch einen uint32_t
Hmm, also, das mit den Modifiern ist mir schon klar. Da ich noch nicht
genau weiss, worin mein Problem liegt habe ich einige Dinge probiert und
die noch nicht wieder entfernt, aber danke für den Hinweis.
Das 'occupied' bit benötige ich schon um zu wissen, ob der
Speicherbereich wieder frei ist.
Das mit der 'Section' ist allerdings ein guter Hinweis, daran habe ich
nicht gedacht.
Naja, die Optimierung kommt ja noch, erstmal soll die Einsortierung
klappen.
Das alles erklärt für mich, aufgrund der Tatsache, dass ich noch genug
SRAM zur Verfügung habe, nicht das Verhalten.
Ich benutze Übrigens die neuste Winavr Version und AVR Studio 4.16.
Und die Sache mit der list_depth würde ich mir auch noch einmal
gründlich überlegen.
So wie deine Eintragsfunktion (und wahrscheinlich auch die
Löschfunktion) aufgebaut ist, kann es vorkommen, dass im Array
unbenutzte Einträge und benutzte Einträge sich abwechseln. Wenn du also
7 benutzte Einträge im Array hast, ist dadurch nicht gesagt, dass diese
im Array an den Indexpositionen 0 bis 6 liegen. list_depth hat damit
überhaupt keine Aussagekraft, welche Indizes gültig sind und welche
nicht sondern nur darüber wieviele Einträge im Array benutzt sind. Wenn
du also nicht den Fall hast, dass du diese Anzahl tausendemale im
Programm benötigst, ist es nur eine Variable die du mitziehen musst, die
dir aber keine wirkliche Information gibt: Die Anzahl unbenutzer
Arrayeinträge kann man auch ganz leicht in einer Schleife ermitteln. Der
Nachteil ist, dass das dann zwar ein wenig länger dauert. Der Vorteil
ist aber, dass die so ermittelte Anzahl niemals falsch sein kann, was
bei einer zusätzlichen Variable nicht gegeben ist. Dort muss man dann
nämlich darauf aufpassen, diese Variable immer und überall korrekt
nachzuziehen.
Martin Stolper schrieb:
> Das 'occupied' bit benötige ich schon um zu wissen, ob der> Speicherbereich wieder frei ist.
Da du einen eigenen Wert für 'ungültige Nummer' hast, brauchst du ihn
eben nicht!
> Naja, die Optimierung kommt ja noch, erstmal soll die Einsortierung> klappen.
Der Fehler muss nicht unbedingt in dieser Funktion liegen.
toolchain ~entwicklungsumgebung == AVRStudio bei dir
aber sein etwas wirres herrumgewerfe von valatile und static is echt
irre ^^
und merke :
wenn der compiler da was wegoptimiert ohne volatile ...
hast du grundlegend was falsch programmiert :)
Der Sinn der Variable 'list_depth' liegt darin, dass ich in einer
anderen Routine die darin enthaltenen Informationen auswerte und nicht
das ganze Array durchsuchen muss.
Später sollen bis zu 255 Elemente möglich sein und die sollen nicht
durchsucht werden wenn sie leer sind und am ende kein belegter Platz
mehr ist (siehe uint16_t
AIS_search_data_section_get (void).
Wegen der Modifier.
Wie gesagt, ich weiss wann man sie benutzt, aber ich wollte Fehler
ausschliessen auch wenn ich in diesem Falle weiss, dass ich sie da nicht
brauche.
Martin Stolper schrieb:
> Der Sinn der Variable 'list_depth' liegt darin, dass ich in einer> anderen Routine die darin enthaltenen Informationen auswerte und nicht> das ganze Array durchsuchen muss.
Frage:
Kann es sein, dass sich in deinem Array benutzte und unbenutzte Einträge
abwechseln?
Wenn ja: list_depth liefert dir keine (oder nahezu keine) Information.
Was soll die Aussagekraft von list_depth sein? Wieviele Einträge belegt
sind. Schön. Nur welche sind es? Die von 0 bis list_depth? Mööp -
falsch. Die benutzten Einträge können über das ganze Array verstreut
sein.
Wenn nein: Warum machst du dann die Einfügeprozedur so kompliziert, wenn
dir list_depth sowieso den Index des nächsten freien Array-Elements
gibt?
Ich erkläre nochmal genauer was passieren soll:
Vorweg:der Verfall der Daten ist noch nicht existent, deswegen soll hier
erstmal nur einsortiert werden.
Also, ich habe eine Struktur mit Daten welche aus einem Stream des USART
gefüllt wird (classA_position_report). Diese Daten sollen in ein
externes SRAM gespeichert werden.
Um nicht das gesamte SRAM auf der Suche einer bestimmten MMSI zu
durchforsten (Performance) lege ich also eine Liste an in der die
wichtigsten Daten stehen (MMSI, belegt und Speicherort (habe ich jetzt
schon geändert)).
Diese Liste wird belegt und die entsprechenden Daten aus dem USART in
das externe SRAM gelegt.
Kommen nun neue Daten mit anderer MMSI sollen auch neue Speicherbereiche
belegt werden.
Kommen neue Daten mit bekannter MMSI soll der externe Speicherbereich
der entsprechenden MMSI überschrieben werden.
Später kommt noch hinzu, dass die Daten nach einer bestimmten Zeitspanne
veralten und der Speicherbereich wieder freigegeben wird.
>> Nicht 0xFFFFFFFFUL?
Habs nochmal geändert. Jetzt mit einem typedef für uint32_t und
(MMSI_t)-1
statt der 0xFFFF
Vorteil: wird der uint32_t irgendwann mal auf einen underen uint Type
geändert, passt der Compiler die Kennung für 'ungültig' (= alle Bits 1)
an.
>Mein Problem ist nun, dass das Array zwar im SRAM richtig platziert wird>(durch Deklaration 'static', ansonsten werden Teile des Arrays>überschrieben), aber die Routine beschreibt nur maximal 6 Arrayelemente,>obwohl mit 'MAXVESSELS = 25' 25 Elemente erzeugt worden sind.>Lasse ich die Deklaration 'static' weg, klappt das Beschreiben aller>Elemente, aber die ersten Elemente werden (wahrscheinlich vom Stack)>teilweise wieder überschrieben.>Was kann ich da machen?
Debuggen. Schritt für Schritt. Wenn das SRAM tatsächlich nicht voll ist,
sind da der Fehlerbeschreibung nach massive Programmfehler drin, und die
sollten sich im Debugger einfach finden lassen. Wenn da nur 6 Einträge
geschrieben werden, was passiert denn genau beim siebten?
Oliver
OK, der Code überzeugt natürlich schon. Aber löst das mein Problem mit
den maximal 6 Einträgen?
Mein code funktioniert nämlich (bis auf das genannte Problem) auch.
Ich werde den code jetzt mal testen, interessiert bin ich (falls
erfolgreich), trotzdem an der Ursache damit mir sowas nicht nochmal
passiert.
Karl heinz Buchegger schrieb:
> Simon K. schrieb:>>
1
>>AIS_data_section[i].MMSI=0xFFFFFFFF;
2
>>
>>>> Nicht 0xFFFFFFFFUL?>> Habs nochmal geändert. Jetzt mit einem typedef für uint32_t und> (MMSI_t)-1> statt der 0xFFFF>> Vorteil: wird der uint32_t irgendwann mal auf einen underen uint Type> geändert, passt der Compiler die Kennung für 'ungültig' (= alle Bits 1)> an.
Das ist natürlich noch viel besser :D Fine Tuning würde ich das nennen
:-)
Martin Stolper schrieb:
> OK, der Code überzeugt natürlich schon. Aber löst das mein Problem mit> den maximal 6 Einträgen?
Das Problem ist, dass in deinem Code nicht ersichtlich ist, warum dieses
Problem überhaupt existiert! Auf der anderen Seite ist dein Code
ziemlich kompliziert mit der while Schleife und den vielen Flags die die
while Schleife steuern.
Vergleich einmal mit meinem Code. Der ist gestreamlined. Keine Schleife
ist länger als ein paar Zeilen und die Funktionalität kann leicht
überblickt werden. Es gibt keine dubiosen Flags, die irgendwelche
Zustände anzeigen und die man im Kopf behalten muss, wenn man den Code
analysiert.
Das Problem kann auch ganz woanders in deinem Code stecken. Du siehst
beim Testen immer nur die Symptome aber nie die Ursache.
OT:
> gestreamlined
Sag doch bitte einfach:
begradigt, übersichtlicher oder durchschaubarer
Wir haben im Deutschen so schöne Übersetzungen für "gestreamlined" ;-)
OK, ich danke für diesen Hinweis.
Vielleicht habe ich meine Engstirnigkeit diesbezüglich dadurch
überwunden, denn ich habe schonmal rausgefunden, dass das was mit meinen
USART Buffern zu tun hat.
Ich werde mich da jetzt nochmal dran begeben.
Jetzt muss ich nochmal was fragen:
Ich benötige eine brauchbare Lösung um einen Ringpuffer aufzubauen (Da
scheint wirklich mein Problem zu liegen).
Ich will eigentlich nicht den FIFO aus RN-Wissen.de verwenden, weil ich
die ganzen Strings nicht kopieren und mich dann auch noch um den Pointer
kümmern will.
Ich habe mir einen Ringpuffer aus x Arrays gebaut und beschreibe diese.
Anscheinend schafft die Routine es nur bis zum letzten Array und hängt
dann fest. Wie kann ich sinnvoll das besetzen und freigeben der arrays
bewerkstelligen?
Hab ich das richtig herausgelesen:
Du benötigst einen Ringbuffer, wobei die Bufferelemente Strings sind?
> Ich habe mir einen Ringpuffer aus x Arrays gebaut und beschreibe> diese. Anscheinend schafft die Routine es nur bis zum letzten Array> und hängt dann fest. Wie kann ich sinnvoll das besetzen und> freigeben der arrays bewerkstelligen?
Am besten gar nicht. In einem µC möchtest du nach Möglichkeit keine
dynmischen Allokierungen machen. Daher wird wohl auch der RN-Ringbuffer
so aussehen, wie er aussieht (ich kenne die Implementierung nicht. Wenn
ich deinen Text lesen denke ich das ist ein ganz normaler
char-Ringbuffer)
Zeig mal was du bisher hast.
reads UART data register and stores character in buffer
20
-buffer area is changed if 'LF' is detected
21
-'AIS_UART.overflow' bit is set if no free buffer available
22
or actual buffer area runs out of space*/
23
ISR(USART0_RX_vect)
24
{
25
/*local variables*/
26
charAIS_chartemp;
27
staticuint8_tAIS_charcnt=0;
28
staticuint8_tAIS_bufcnt=0;
29
30
/*cache data register*/
31
AIS_chartemp=UDR0;
32
33
/*check if actual buffer is unoccupied*/
34
if((AIS_UART.occupied&&(1<<AIS_bufcnt))==0)
35
{
36
/*check if end of string is reached*/
37
if(AIS_chartemp==LF)
38
{
39
AIS_UART.occupied|=(1<<AIS_bufcnt);
40
if(AIS_bufcnt<AIS_BUFCOUNT)AIS_bufcnt++;
41
elseAIS_bufcnt=0;
42
AIS_charcnt=0;
43
AIS_UART.RX_complete++;
44
charbuffer67[10];
45
utoa(AIS_bufcnt,buffer67,16);
46
glcd_print(0,11,&buffer67[0]);
47
}
48
else
49
{
50
/*store cache into buffer if end not reached*/
51
if(AIS_charcnt<=81)
52
{
53
AISBUFFER[AIS_bufcnt][AIS_charcnt]=AIS_chartemp;
54
AIS_charcnt++;
55
}
56
else
57
{
58
AIS_UART.overflow=1;
59
}
60
}
61
}
62
else
63
{
64
AIS_UART.overflow=1;
65
}
66
}/*ISR (USART0_RX_vect)*/
Da funktioniert aber, wie gesagt, was mit der Array indizierung nicht.
ais_occbuf wird am ende auf 0b00100000 bzw 0b01000000 gesetzt und
AIS_bufcnt verharrt auf '4'.
Martin Stolper schrieb:
Hmm.
Du speicherst da keine sauberen C-Strings mit einem '\0' am Ende. Das
kann dir in der Weiterverarbeitung der Strings mächtig Probleme machen.
> Da funktioniert aber, wie gesagt, was mit der Array indizierung nicht.> ais_occbuf wird am ende auf 0b00100000 bzw 0b01000000 gesetzt und> AIS_bufcnt verharrt auf '4'.
Wo ist die Auslesefunktion, die die occupied Bits wieder freigeben muss?
(Edit: Die Frage nach der Auslesfunktion hat sich erledigt)
PS: Wusstest du, dass man solche unabhängige Teile wunderbar auf dem PC
debuggen kann. Da hat man wesentliche bessere Möglichkeiten zum
Debuggen. Nur so nebenbei :-)
Das weiss ich schon. Aber es ist ziemlich mühselig die USART Inputs
nachzubilden. Oder gibts da eine einfache möglichkeit? Ich kenne nur
zwei:
In einer Schleife das Register beschreiben und die Werte beim Debuggen
direkt ins register tippen.
Ich wäre Dankbar für eine bessere Lösung.
Martin Stolper schrieb:
> Das weiss ich schon. Aber es ist ziemlich mühselig die USART Inputs> nachzubilden.
Nicht wirklich.
Du hast eine Funktion, die einen character in den Buffer schreibt und du
hast eine Funktion, die eine Message abholt (sofern eine da ist).
Du machst dir am PC eine main(), die (hausnummer) 5 Zeichen über die
Funktion in den Buffer schreibt, dann einen LF und dann die get Funktion
aufruft (die Auswertung da drinnen schliesst du kurz, hat sowieso da
drinnen nichts verloren)
Das ganze machst du im main in einer Schleife und kannst so wunderbar
debuggen.
Die occupied Bits würde ich als erstes rausschmeissen, genauso wie den
complete Counter. Alles was du brauchst sind die beiden Zähler, die
jetzt statische Variablen der Funktionen sind. Die wandern als erstes in
deine UART Struktur und die komplette Auswertung ob Platz für neues
vorhanden ist bzw. ob eine Msg vorliegt, wird nur über diese beiden
Counter gemacht.
Mit solchen diversen (vielen) Flags und Countern schiesst man sich oft
ein Eigentor. Sie machen den Code des öfteren einfach nur
unübersichtlich.
Ach, das ist fies
if ((AIS_UART.occupied && (1<<AIS_getbuf)) == 1)
Der Vergleich ist Müll.
Der kann nur dann wahr werden, wenn AIS_getbuf genau 0 ist. In allen
anderen Fällen kommt da durch die Verundung nicht 1 raus (wohl aber
etwas, was ungleich 0 ist).
Manchmal ist es in C äusserst kontraproduktiv, wenn man zu explizit ist.
Boolsche Ergebnisse sind da ein häufiges Beispiel. Am besten fährt man
hier mit der Devise: weniger ist mehr.
Karl heinz Buchegger schrieb:
> Ach, das ist fies>> if ((AIS_UART.occupied && (1<<AIS_getbuf)) == 1)>>> Der Vergleich ist Müll.> Der kann nur dann wahr werden, wenn AIS_getbuf genau 0 ist. In allen> anderen Fällen kommt da durch die Verundung nicht 1 raus (wohl aber> etwas, was ungleich 0 ist).
Na, na, na. ;-)
Da steht ein "&&", kein "&".
So wie der Vergleich dort jetzt steht, könnte man auch schreiben:
Stefan Ernst schrieb:
> Karl heinz Buchegger schrieb:>> Ach, das ist fies>>>> if ((AIS_UART.occupied && (1<<AIS_getbuf)) == 1)>>>>>> Der Vergleich ist Müll.>> Der kann nur dann wahr werden, wenn AIS_getbuf genau 0 ist. In allen>> anderen Fällen kommt da durch die Verundung nicht 1 raus (wohl aber>> etwas, was ungleich 0 ist).>> Na, na, na. ;-)> Da steht ein "&&", kein "&".
Ja richtig!
Jetzt sehe ich es auch.
Da sollte aber ein & stehen :-)
Also gleich 2 Fehler in einer Zeile :-)
Sagte ich schon, dass solche Flags oftmals nur für Verwirrung sorgen :-)
Edit: In der ISR derselbe Fehler noch einmal
/*check if actual buffer is unoccupied*/
if ((AIS_UART.occupied && (1<<AIS_bufcnt)) == 0)
Auch hier muss es & heissen.
Hmm, da habe ich mal wieder Mist gebaut...
Das stimmt natürlich und ich habe das gleich geändert und getestet.
Und siehe da, es klappt!!!
Vielen Dank dafür!
Ich werde heute noch die meisten eurer Verbesserungsvorschläge in die
Tat umsetzen (hoffentlich verfalle ich nicht wieder in einen Flag Wahn).
Ich muss hiermit auch gleich ein Lob an diese Forum aussprechen:
Die Antworten prasseln hier nur so auf einen ein und die
Diskussionsfreudigkeit scheint sehr hoch zu sein.
Dann mach ich mich mal ans Werk!
MfG, Marten83