Code optimieren ?

OP #7432227
Lesenswert?

Hallo,

folgender Code wandelt Werte über ein Kennfeld um.

1
struct _kennfeld{
2
  int16_t adc_value;
3
  int8_t prozent;
4
};
5

6
#define kennfeld_len 31
7
const __flash struct _kennfeld kennfeld[] = {{972, 0},{951, 3},{950, 4},{929, 6},{907, 7},{886, 10},{864, 12},{843, 13},\
8
                      {842, 15},{821, 16},{820, 18},{798, 19},{775, 22},{752, 25},{729, 30},{706, 34},\
9
                      {684, 39},{669, 43},{662, 45},{639, 48},{616, 52},{594, 57},{571, 63},{552, 70},\
10
                      {562, 72},{548, 73},{526, 76},{502, 84},{481, 91},{457, 97},{432, 100}};
11
                  
12
int8_t adc_value_2_prozent(int16_t adc_value){
13
  if (adc_value >= kennfeld[0].adc_value){
14
    return 0;
15
  }else if(adc_value <= kennfeld[kennfeld_len].adc_value){
16
    return 100;
17
  }else{
18
    uint8_t max = 0;
19
    while (kennfeld[max].adc_value > adc_value){
20
      max++;
21
    }
22
    return (adc_value - kennfeld[max].adc_value) * (kennfeld[max-1].prozent - kennfeld[max].prozent)\
23
        / (kennfeld[max-1].adc_value - kennfeld[max].adc_value) + kennfeld[max].prozent;
24
  }
25
}
26

27
int main(void){
28
  uint8_t fuel_level;
29
  fuel_level = (uint8_t)adc_value_2_prozent(450);
30
    fuel_level = (uint8_t)adc_value_2_prozent(960);
31
  while (1){
32
    
33
    }
34
}

Das ganze läuft auf einem AtMega328PB. Würdet ihr das auch so machen oder gibt es da bessere Wege?

Gruß Rene

#7432245
Lesenswert?

Top S. schrieb:

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?

https://www.mikrocontroller.net/articles/AVR-GCC-Codeoptimierung#Prinzipien_der_Optimierung

#7432247
Lesenswert?

Mögliche Maßnahmen:

  • 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.

mfg mf

OP #7432262
Lesenswert?

Hallo,

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.

Danke, da ist was schief gelaufen.

Gruß Rene

Angehängte Dateien:
#7432305
Lesenswert?

Unwartbar: kennfeld_len und kennfeld müssen gemeinsam geändert werden, Abhilfe ofsetof

Der Code enthält Daten der Kennlinie, das ist ebenfalls redundant (unwartbar). Die ersten zwei if können entfallen. Und nur max am Ende testen.

Eine Funktion für jede Kennlinie, Abhilfe: Array als Pointer

Fehler: Off by one.

Wenn return im if, dann kein else if.

#7432308
Lesenswert?

Wenn Du den ersten und letzten Datenpunkt anpasst, dann kannst Du Dir ein paar Checks für die Grenzwerte Sparen. E.g:

1
const __flash struct _kennfeld kennfeld[] = {
2
  {INT16_t_MAX, 0},{972, 0},{951, 3},{950, 4},
3
  {929, 6},{907, 7},{886, 10},{864, 12},{843, 13},
4
  {842, 15},{821, 16},{820, 18},{798, 19},{775, 22},
5
  {752, 25},{729, 30},{706, 34},{684, 39},{669, 43},
6
  {662, 45},{639, 48},{616, 52},{594, 57},{571, 63},
7
  {552, 70},{562, 72},{548, 73},{526, 76},{502, 84},
8
  {481, 91},{457, 97},{432, 100},{0, 100}};

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.

#7433001
Lesenswert?

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

Persönliche Seite #7433108
Lesenswert?

Etwas Kosmethik:

1
typedef struct
2
{
3
  int16_t adc_value;
4
  int8_t prozent;
5
} kennfeld_t;
6

7
#define ARRAY_SIZE(X) (sizeof (X) / sizeof (*X))
8
#define kennfeld_len ARRAY_SIZE (kennfeld)
9

10
const __flash kennfeld_t kennfeld[] =
11
{
12
    // Zeilenumbruch braucht kein "\".
13
    { 972, 0 }, { 951, 3 }, { 950, 4 }, { 929, 6 },{ 907, 7 }, { 886, 10 },
14
    { 864, 12 }, //...
15
};

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:

1
int8_t adc_value_2_prozent (int16_t adc_value)
2
{
3
  if (adc_value >= kennfeld[0].adc_value)
4
  {
5
      return kennfeld[0].prozent;
6
  }
7
  else if (adc_value <= kennfeld[kennfeld_len - 1].adc_value)
8
  {
9
      return kennfeld[kennfeld_len - 1].prozent;
10
  }

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.

Beitrag #7433130 wurde von einem Moderator gelöscht.
Beitrag #7433139 wurde von einem Moderator gelöscht.
#7433181
Lesenswert?

Johann L. schrieb:

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 R schrieb:

return (adc_value - kennfeld[max].adc_value) * (kennfeld[max-1].prozent - kennfeld[max].prozent)
/ (kennfeld[max-1].adc_value - kennfeld[max].adc_value) + kennfeld[max].prozent;

1
 r = (a-k[i].a) * (k[i-1].v-k[i].v)/(k[i-1].a-k[i].a)+k[i].v;
2
/* anders arrangiert: */
3

4
 r = k[i].v + (k[i-1].v-k[i].v) * (a-k[i].a)/(k[i-1].a-k[i].a);
5
/* mit 
6
       da = k[i-1].a-k[i].a und 
7
       dv = k[i-1].v-k[i].v */
8
 r = k[i].v + dv * (a-k[i].a)/da;

soweit so gut. Nur ist a nicht zwischen k[i].a und k[i-1].a, sondern "dahinter".

Es liegt daher an dem TO, mal ein update zu geben, was er verstanden hat.

Dass Anfang ODER Ende nicht explizit getestet werden müssen ist ja bereits mehrfach geschrieben (oder beide nicht mit einem weiteren Eintrag)

#7433213
Lesenswert?

Was an:

Bruno V. schrieb:

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.

#7433217
Lesenswert?

Man darf auch in C zwecks besserer Lesbarkeit eine Berechnung in mehrere Schritte und Zeilen zerlegen. Eher so.

1
  // lineare Interpolation
2
  long tmp;
3
  tmp = (adc_value - kennfeld[max].adc_value) * (kennfeld[max-1].prozent - kennfeld[max].prozent);
4
  tmp = tmp / (kennfeld[max-1].adc_value - kennfeld[max].adc_value) + kennfeld[max].prozent;
5
  return tmp;
Persönliche Seite #7433271
Lesenswert?

Kurze Bezeichner gehen ja auch lokal:

1
      int16_t x1 = kennfeld[max-1].adc_value;
2
      int16_t x2 = kennfeld[max].adc_value;
3
      int8_t y1  = kennfeld[max-1].prozent;
4
      int8_t y2  = kennfeld[max].prozent;
5

6
      return y2 + (adc_value - x2) * (y1 - y2) / (x1 - x2);

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.

OP #7434800
Lesenswert?

Hallo,

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):

1
typedef struct {
2
  int16_t adc_value;
3
  int16_t humen_value;
4
} _kf;
5

6
const __flash _kf kf_fuel[] =    {{1024, 0}, {972, 0},{951, 3},{950, 4},{929, 6},
7
                  {907, 7},{886, 10},{864, 12},{843, 13},{842, 15},
8
                  {821, 16},{820, 18},{798, 19},{775, 22},{752, 25},
9
                  {729, 30},{706, 34},{684, 39},{669, 43},{662, 45},
10
                  {639, 48},{616, 52},{594, 57},{571, 63},{552, 70},
11
                  {562, 72},{548, 73},{526, 76},{502, 84},{481, 91},
12
                  {457, 97},{432, 100},{0, 100}};
13
                  
14
const __flash _kf kf_oel_temp[] =  {{1024, 150},{976, 150},{962, 140},{942, 130},
15
                  {930, 125},{917, 120},{889, 110},{858, 100},
16
                  {826, 90},{794, 80},{762, 70},{728, 60},{695, 50},
17
                  {662, 40},{628, 30},{611, 25},{594, 20},{560, 10},
18
                  {527, 0},{0, 0}};
19

20
const __flash _kf kf_h2o_temp[] =  {{1024, 120},{917, 120},{889, 110},{858, 100},
21
                  {826, 90},{794, 80},{762, 70},{728, 60},{695, 50},
22
                  {662, 40},{628, 30},{611, 25},{594, 20},{560, 10},
23
                  {527, 0},{0, 0}};
24
                  
25
int16_t adc_value_2_human(int16_t adc_value, const __flash _kf* kf){
26
  uint8_t max = 0;
27
  while (kf[max].adc_value > adc_value){
28
    max++;
29
  }
30
  return (adc_value - kf[max].adc_value) * (kf[max-1].humen_value - kf[max].humen_value)
31
        / (kf[max-1].adc_value - kf[max].adc_value) + kf[max].humen_value;
32
}
33

34
int main(void){
35
  uint8_t fuel_level;
36
  fuel_level = (uint8_t)adc_value_2_human(0, kf_fuel);        // 100
37
  fuel_level = (uint8_t)adc_value_2_human(450, kf_fuel);        // 98
38
  fuel_level = (uint8_t)adc_value_2_human(960, kf_fuel);        // 2
39
  fuel_level += (uint8_t)adc_value_2_human(1023, kf_fuel);      // 0
40
  
41
  uint8_t oel_temp;
42
  oel_temp = (uint8_t)adc_value_2_human(0, kf_oel_temp);        // 0
43
  oel_temp = (uint8_t)adc_value_2_human(545, kf_oel_temp);      // 5
44
  oel_temp = (uint8_t)adc_value_2_human(969, kf_oel_temp);      // 145
45
  oel_temp += (uint8_t)adc_value_2_human(1023, kf_oel_temp);      // 150
46
  
47
  uint8_t h2o_temp;
48
  h2o_temp = (uint8_t)adc_value_2_human(0, kf_h2o_temp);        // 0
49
  h2o_temp = (uint8_t)adc_value_2_human(577, kf_h2o_temp);      // 15
50
  h2o_temp = (uint8_t)adc_value_2_human(903, kf_h2o_temp);      // 115
51
  h2o_temp += (uint8_t)adc_value_2_human(1023, kf_h2o_temp);      // 120
52
   
53
  while (1){
54
    
55
    }
56
}

Das ist noch nicht fertig, die liebe Zeit.

Gruß und Danke bis hier hin René

#7434880
Lesenswert?

R R schrieb:

Das ist noch nicht fertig,

Wow, dass sieht sehr gut aus, Du scheinst alle verschiedenen Tipps wirklich überdacht und abgewogen zu haben.

Die einzige Frage, die sich mir noch stellt: Wenn nur 8 Bit zurückkommen, würde ich den cast (und ggf. Grenzchecks) in der Funktion machen.

Ich vermeide casten soweit es geht, da man damit Compiler-Warnungen verliert (Überlauf, Pointer statt int, ..)

#7434915
Lesenswert?

R R schrieb:

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.

OP #7435046
Lesenswert?

Hallo,

habe den Cast jetzt mal in die Funktion gelegt und den Rückgabewert geändert.

1
uint8_t adc_value_2_human(int16_t adc_value, const __flash _kf* kf){
2
  uint8_t max = 0;
3
  while (kf[max].adc_value > adc_value){
4
    max++;
5
  }
6
  return (uint8_t)((adc_value - kf[max].adc_value) * (kf[max-1].human_value - kf[max].human_value)
7
        / (kf[max-1].adc_value - kf[max].adc_value) + kf[max].human_value);
8
}

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.

Meinst du das so:

1
uint8_t adc_value_2_human(int16_t adc_value, const __flash _kf* kf){
2
  assert(adc_value >= 0 && adc_value < 1024);
3
  uint8_t max = 0;
4
  while (kf[max].adc_value > adc_value){
5
    max++;
6
  }
7
  int16_t temp;
8
  temp = (adc_value - kf[max].adc_value) * (kf[max-1].human_value - kf[max].human_value)
9
        / (kf[max-1].adc_value - kf[max].adc_value) + kf[max].human_value;
10
  assert(temp >= 0 && temp <= 0xFF);
11
  return (uint8_t)temp;
12
}

Gruß René

#7435125
Lesenswert?

Falk B. schrieb:

Mit der binären Suche reichen 5 Durchläufe.

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.

#7435632
Lesenswert?

Falk B. schrieb:

R R schrieb:

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.

#7435636
Lesenswert?

R R schrieb:

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.

Genau. Deswegen nimmt man unsigned.

Warum ist adc_value ein vorzeichenbehafteter Typ?

Damit in der Funktion adc_value_2_human kein Typwechsel erforderlich wird.

Dann passe _kennfeld entsprechend an.

Falls das so sein soll, gehört da mindestens eine Assertion für den Wertebereich hin. Oder ändere Dein Sentinel.

Meinst du das so:

1
uint8_t adc_value_2_human(int16_t adc_value, const __flash _kf* kf){
2
>   assert(adc_value >= 0 && adc_value < 1024);
3
>   uint8_t max = 0;
4
>   while (kf[max].adc_value > adc_value){
5
>     max++;
6
>   }
7
>   int16_t temp;
8
>   temp = (adc_value - kf[max].adc_value) * (kf[max-1].human_value - 
9
> kf[max].human_value)
10
>         / (kf[max-1].adc_value - kf[max].adc_value) + 
11
> kf[max].human_value;
12
>   assert(temp >= 0 && temp <= 0xFF);
13
>   return (uint8_t)temp;
14
> }

Du scheinst ja schon mal was von Vor- und Nachbedingungen gehört zu haben ;-)

Die Vorbedingung ist bzgl. adc_value ok, wobei Du das wie gesagt mit unsigend verbessern kannst. Es fehlt eine Vorbedingung bzgl. kf.

Und zusätzlich sollten die Parametervariablen const sein.

#7435644
Lesenswert?

Wilhelm M. schrieb:

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.

#7435647
Lesenswert?

Wilhelm M. schrieb:

Und zusätzlich sollten die Parametervariablen const sein.

uint8_t adc_value_2_human(int16_t adc_value, const __flash _kf* kf)

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.

#7435655
Lesenswert?

Falk B. schrieb:

Wilhelm M. schrieb:

Und zusätzlich sollten die Parametervariablen const sein.

uint8_t adc_value_2_human(int16_t adc_value, const __flash _kf* kf)

Der Zeiger auf des Kennfeld ist ein Zeiger auf const.

Korrekt, damit ein Input-Parameter. Das meinte ich aber nicht, s.u.

Bei adc_value ist es egal, denn das ist immer eine Kopie eines Werts.

Für den Aufrufer ist das egal. Aber nicht in der Funktion selbst.

Selbst wenn man da drauf schreiben würde, würde das keinerlei Nebenwirkungen im Programm haben. Ein const ist dort reine Kosmetik.

Nein.

uint8_t adc_value_2_human(const int16_t adc_value, const __flash _kf* const kf);

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.

#7436171
Lesenswert?

Wilhelm M. schrieb:

Falk B. schrieb:

Wilhelm M. schrieb:

Und zusätzlich sollten die Parametervariablen const sein.

uint8_t adc_value_2_human(int16_t adc_value, const __flash _kf* kf)

Der Zeiger auf des Kennfeld ist ein Zeiger auf const.

Korrekt, damit ein Input-Parameter. Das meinte ich aber nicht, s.u.

Bei adc_value ist es egal, denn das ist immer eine Kopie eines Werts.

Für den Aufrufer ist das egal. Aber nicht in der Funktion selbst.

Wieso? Was passiert denn, wenn man versehentlich adc_value beschreibt? Expoldiert dann der Stack?

Selbst wenn man da drauf schreiben würde, würde das keinerlei Nebenwirkungen im Programm haben. Ein const ist dort reine Kosmetik.

Nein.

uint8_t adc_value_2_human(const int16_t adc_value, const __flash _kf* const kf);

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.

Und schlimmen AbKüFi.

#7436224
Lesenswert?

Falk B. schrieb:

Wilhelm M. schrieb:

Falk B. schrieb:

Wilhelm M. schrieb:

Und zusätzlich sollten die Parametervariablen const sein.

uint8_t adc_value_2_human(int16_t adc_value, const __flash _kf* kf)

Der Zeiger auf des Kennfeld ist ein Zeiger auf const.

Korrekt, damit ein Input-Parameter. Das meinte ich aber nicht, s.u.

Bei adc_value ist es egal, denn das ist immer eine Kopie eines Werts.

Für den Aufrufer ist das egal. Aber nicht in der Funktion selbst.

Wieso? Was passiert denn, wenn man versehentlich adc_value beschreibt?

Das hatte ich in meinem Beitrag beschrieben.

Expoldiert dann der Stack?

Das habe ich nicht geschrieben ;-)

Selbst wenn man da drauf schreiben würde, würde das keinerlei Nebenwirkungen im Programm haben. Ein const ist dort reine Kosmetik.

Nein.

uint8_t adc_value_2_human(const int16_t adc_value, const __flash _kf* const kf);

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.

Antwort schreiben

Bitte melde dich an, um einen Beitrag zu schreiben.

oder

Mit Google-Account einloggen

Die Registrierung ist kostenlos und dauert nur eine Minute.

Jetzt registrieren