Das ganze läuft auf einem AtMega328PB. Würdet ihr das auch so machen
oder gibt es da bessere Wege?
Sicher. Man kann eine binäre Suche machen, wenn die Kurve monoton ist, damit ist man schneller. Man kann auch Speicherplatz gegen GEschwindigkeit eintauschen, wenn man mehr Werte in die Tabelle schreibt, dann reicht ein einzelner Zugriff.
Aber die allererste Frage lautet. MUSS es WIRKLICH schneller sein? Wie oft wird die Funktion aufgerufen?
Anzahl der Datenpunkte des Kennfeldes (eine Zweierpotenz + 1) machen.
Input-Range auf 0...(Zweierpotenz - 1) oder -(Zweierpotenz - 1)...+(Zweierpotenz - 1) normieren.
Input in äquidistanten Punkten vorgeben. Kann auch mit Offset sein, der dann zuvor abgezogen wird.
Folgen davon:
Index der Tabelle kann durch schieben des Input nach Rechts gefunden werden
Lineare Interpolation kann durch Multiplikation des Nachkommateils (das was zuvor durch Rechtsschieben für Index herunter gefallen ist) und schieben gewonnen werden.
Contra Optimierung:
Für eine Tankanzeige läuft deine Rechnung wohl schnell genug, dass man die nicht optimieren muss.
Thema bitte schließen, sonst artet das aus in einen sinnlosen Wettbewerb.
Solange das verzögerungsfrei ausgeführt wird, der Speicher nicht voll ist, und fehlerfrei läuft, Finger weg.
bis jetzt läuft das alles ziemlich rund. Es ging nur um die grundsätzliche Richtung da es ein paar mehr Anzeigen sind. Wäre blöd am Ende festzustellen das ich an die hälfte des Codes noch mal ran muß. Optimiert wird natürlich nur wenn es eng wird. Bis jetzt ist das kein Problem.
Guck dir erstmal die Werte an, das sieht nicht sonderlich toll aus. Der
Wert bei 72 % passt nicht.
Wäre blöd am Ende festzustellen das ich an die hälfte des Codes noch mal
ran muß.
Dann wird da noch mehr per Kennfeld umgerechnet? Wie wäre es, aus der Linearen Interpolation eine Funktion zu machen, die nicht für jede Anzeige, die es braucht, Copy-Paste-Legasthenie enthält?
Danach brauchst Du nur noch per binärer Suche nach Deinem adc Wert suchen. Die neuen Elemente am Anfang und Ende sorgen dafür, das Du immer im Array bleibst.
Und die Prozent braucht es wirklich ? Fuer eine Anzeige ?
Anzeigen sind immer langsam, mehr wie 3 updates pro Sekunde werden als nervoes wahrgenommen.
Als Optimierung koennte zb ein Fehler zugelassen werden. Sind zB 3% in der Anzeige zulaessig, dann koennte man die Funktion approximieren. zB als Polynom, oder stueckweise stetig.
Als weitere Optimierung koennte zB mit 1/128 gearbeitet werden. Ein Stellglied arbeitet auch besser mit 1/128 wie mit 1/100
Wenn höchste Performance wichtig ist, würde ich das konstante Array (kennfeld) nicht in den Flash Speicher legen, sondern ins RAM. Weil die CPU darauf schneller zugreifen kann.
Das letzte Element befindet sich bei Index kennfeld_len - 1. Außerdem kann man "100" aus dem Array lesen; __flash ist ja transparent für avr-gcc, daher geht das ohne Overhead:
Die ersten beiden Vergleiche + returns brauchen weniger als 15 Instruktionen, da kann eh nix mehr gespart werden.
Die eigentliche Berechnung würd ich so notieren, dass besser zu sehen ist, was abgeht (und auch da braucht man kein "" als Zeilentrenner / -fortsetzung.
Dividend und Divisor sind int, daher stellt sich auch die Frage nach der Genauigkeit der Berechnung. Falls die Genauigkeit ungenügend ist, kann man die ADC-Werte einfach hochskalieren und genauere Werte verwenden, zum Beispiel indem man am unteren Ende der .adc_value 4 Bit dranhängt, was immer noch locker in 16-Bit Integer passt. Zu Beginn der Routine muss dann natürlich auch adc_value angepasst werden per adc_value <<= 4.
Und warum wird die Berechnung signed gemacht? Man kann die Berechnung so hinschreiben, dass nur positive Werte auftreten, weil: .adc_value ist positiv und monoton (abgesehen von dem Fehler bei 72), .prozent ist positiv und monoton.
Die eigentliche Berechnung würd ich so notieren, dass besser zu sehen
ist, was abgeht (und auch da braucht man kein "" als Zeilentrenner /
-fortsetzung.
Zumal sie ja auch einfach falsch ist. Kurze Bezeichner (wie in der Mathematik) erleichtern das Lesen und verstehen ungemein. Beispiel: adc_value = a, kennfeld = k, prozent = v (für value) und max = i. Dann steht da:
r = (a-k[i].a) * (k[i-1].v-k[i].v)/(k[i-1].a-k[i].a)+k[i].v;
leichter zu lesen und zu verstehen sein soll, erschließt sich mir nicht. Das Projekt, drei Monate in der Schublade, und selbst der Entwickler wird grübeln, was denn nun a ist, was k usw. Variablen dürfen heute echt länger sein als ein-zwei Buchstaben, daran sollte man nicht sparen. Die Zeiten, wo man eine Variable mit lediglich einem Buchstaben abkürzen musste, sind schon lange vorbei, auch im Embedded-Bereich.
Damit hieht man direkt die Gleichung für eine Gerade, d.h. Linearität in adc_value, und dass
adc_value = x2 auf y2 abbildet, und adc_value = x1 auf y1 abbildet.
Tatsächlich kann man ein Kennfeld einfach als Abbildung x → ƒ(x) = y betrachten. Ob da nun Bananen auf Tomaten abgebildet werden oder ADC-Werte auf Prozente spielt keine Rolle für die Berechnungen. Das einzige was zählt sind Wertebereich und Bildbereich (hier int16_t und int8_t).
Der Vorteil der aktuellen Implementierung ist dann nur, dass die Länge des Arrays zur Compilezeit bekannt ist, was ein paar Bytes Code einspart verglichen mit einem verallgemeinerten Ansatz, der die Array-Länge als Parameter bekommen würde.
Schwieriger ist hingegen die Korrektheit, d.h. dass es innerhalb der Berechnung nicht zu einem Überlauf kommt. Es wird ja int16_t * int8_t gerechnet und davon ausgegangen, dass das Ergebnis noch in int16_t passt.
Thema bitte schließen, sonst artet das aus in einen sinnlosen
Wettbewerb.
Solange das verzögerungsfrei ausgeführt wird, der Speicher nicht voll
ist, und fehlerfrei läuft, Finger weg.
Joa, Thema ist wie erwartet gelaufen. Dafür bitte nochmal 5 Downvotes.
Danke für die rege Teilnahme an der Diskusion. War diese Woche bischen ausgelastet. Deswegen erst jetzte eine Rückmeldung. Ich habe jetzt mal diese Änderungen eingefügt (der Fehler im ersten Kennfeld ist immer noch drin):
value liest man zwar oft in Software und Quellcode, ist aber in den allermeisten Fällen Unfug, denn es ist ja immer ein Wert! Das muss man nicht extra schreiben.
int16_t adc_value_2_human(int16_t adc_value, const __flash _kf* kf){
uint8_t max = 0;
while (kf[max].adc_value > adc_value){
max++;
}
...
}
Ist adc_value < 0, terminiert die Iteration nicht, Du läufst aus dem Array und hast UB.
Warum ist adc_value ein vorzeichenbehafteter Typ?
Falls das so sein soll, gehört da mindestens eine Assertion für den Wertebereich hin. Oder ändere Dein Sentinel.
Ist adc_value < 0, terminiert die Iteration nicht, Du läufst aus dem
Array und hast UB.
adc_value kommt direct aus ADCL / ADCH und kann somit nicht kleiner 0 werden.
Warum ist adc_value ein vorzeichenbehafteter Typ?
Damit in der Funktion adc_value_2_human kein Typwechsel erforderlich wird.
Ein uint16_t führt zu einem falschen Ergebnis.
Hat das was mit Humus zu tun? Oder Homus? ;-)
Ups :-)
value liest man zwar oft in Software und Quellcode, ist aber in den
allermeisten Fällen Unfug, denn es ist ja immer ein Wert! Das muss man
nicht extra schreiben.
Kann ich nachvollziehen. Ist mir aber so lieber, also bleibt es wie es ist.
Falls das so sein soll, gehört da mindestens eine Assertion für den
Wertebereich hin. Oder ändere Dein Sentinel.
Typedefs nennt man typisch _t, also kf_t.
Die binäre Suche hast du auch nicht. Wenn dein Wert am Ende der Kennlinie liegt, musst du 32 mal die Schleife abklappern. Mit der binären Suche reichen 5 Durchläufe. Und der restliche Krümelkram. Eher so, siehe Anhang.
Der TO hat oben eine Code mit implizit bekannte Feldgröße präsentiert. Ein wirklicher CodeSmell. Dann hat er das deutlich verbessert, auch wenn Grenzen-Checks noch fehlen und es ähnlich fragil (aber üblich) ist wie ein String mit terminierender Null.
Und jetzt soll er eine binäre Suche implementieren, die die Qualität auf Anfang rückt? Als Fingerübung OK, aber in Produktionscode läuft das regelmäßig in noch schwerer zu findende Fehler.
Typedefs nennt man typisch _t, also kf_t.
Die binäre Suche hast du auch nicht. Wenn dein Wert am Ende der
Kennlinie liegt, musst du 32 mal die Schleife abklappern. Mit der
binären Suche reichen 5 Durchläufe. Und der restliche Krümelkram. Eher
so, siehe Anhang.
Das mit den signed DT ist immer noch Mist. Nimm unsigned und Du kannst wieder eine Abfrage sparen. Zudem drückt der Code besser das aus, was wirklich gemeint ist.
Es wird nirgends sichergestellt, dass die Tabellen sortiert sind. Das ist ganz großer Mist.
Das mit den signed DT ist immer noch Mist. Nimm unsigned und Du kannst
wieder eine Abfrage sparen. Zudem drückt der Code besser das aus, was
wirklich gemeint ist.
OK.
Es wird nirgends sichergestellt, dass die Tabellen sortiert sind. Das
ist ganz großer Mist.
Wer sagt das? Ich sagte, daß die binäre Suche auf montone Kennlinien angewiesen ist. Und wenn man nicht total behämmert ist, speichert man die Kennlinie ganz normal, wie man es in einer Exeltabelle tun würde. In aufsteigender Reihenfolge, sodaß man einen Graphen darstellen kann.
Der Zeiger auf des Kennfeld ist ein Zeiger auf const. Bei adc_value ist es egal, denn das ist immer eine Kopie eines Werts. Selbst wenn man da drauf schreiben würde, würde das keinerlei Nebenwirkungen im Programm haben. Ein const ist dort reine Kosmetik.
Variablen sollten wenn möglich read-only sein, so auch Parametervariablen. Die Bedeutung von adc_value ist eben der Wert des ADC, wenn ich dummerweise irgendwo in(!) der Funktion adc_value /= 2 machen würde, wäre das Paar (Name, Wert) nicht mehr korrekt.
Bei kf ist es sogar noch schlimmer: kf ist der Zeiger auf eine Tabelle. Ein kf -= 42 referenziert wahrlich nichts sinnvolles und man hat UB.
Variablen sollten wenn möglich read-only sein, so auch
Parametervariablen.
Jaja, man kann alles übertreiben. Eine einfache Parametervariable kann keinen Schaden außerhalb der Funktion anrichten, das können nur Pointer.
Die Bedeutung von adc_value ist eben der Wert des
ADC, wenn ich dummerweise irgendwo in(!) der Funktion adc_value /= 2
machen würde, wäre das Paar (Name, Wert) nicht mehr korrekt.
Ohje, die Oberkorrekten.
Bei kf ist es sogar noch schlimmer: kf ist der Zeiger auf eine Tabelle.
Ein kf -= 42 referenziert wahrlich nichts sinnvolles und man hat UB.
Variablen sollten wenn möglich read-only sein, so auch
Parametervariablen.
Jaja, man kann alles übertreiben. Eine einfache Parametervariable kann
keinen Schaden außerhalb der Funktion anrichten, das können nur Pointer.
In diesem Fall ja nicht, weil es ein Input-Parameter ist. Hatte ich auch geschrieben.
Die Bedeutung von adc_value ist eben der Wert des
ADC, wenn ich dummerweise irgendwo in(!) der Funktion adc_value /= 2
machen würde, wäre das Paar (Name, Wert) nicht mehr korrekt.
Ohje, die Oberkorrekten.
Nun, der TO hatte nach Optimierung gefragt. Und optimieren kann man in verschiedene Richtungen. Auch das sollte Dir klar sein.
Ich denke, dass auch Du Dich manchmal fragst, was der Wert in einer Variablen für eine Bedeutung hat. Und es ist gut, wenn der Name darüber Auskunft gibt.
Ansonsten braüchte man die Variable auch nicht adc_value zu nennen, sondern könnte einfach x als Name nehmen. Ich denke, das hast auch Du verstanden.
Bei kf ist es sogar noch schlimmer: kf ist der Zeiger auf eine Tabelle.
Ein kf -= 42 referenziert wahrlich nichts sinnvolles und man hat UB.
Und schlimmen AbKüFi.
Dir scheint entgangen zu sein, dass kf der Name der Parametervariablen ist. Also nix AbKüFi, jedenfalls nicht von mir. Denn ich habe nur die Namen des TO übernommen.