Best Practise anhand Codebeispiel

[Matthias (matjoe)]

Guten Tag,

ich habe eine Frage in Richtung Programmierung / Best Practice.
Ich möchte am RaspberryPi einen Heizstab ansteuern. Dafür würde ich 
gerne ein PWModuliertes Signal nutzen. Ich möchte hinterher die Vorgabe 
der Heizleistung aus einer Regelung erhalten, hier zum Test mit 
Konstante. Die Heizleistungsvorgabe kann durch die Regelung über die 
Stellwertgrenzen gehen, daher die Begrenzung.

#include <wiringPi.h>

#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>


#define PWM_PIN01 = 1
#define MAX_LOAD_POWER = 3000
#define PWM_CLK = 4
#define MAIN_FREQ = 192000000
#define PWM_RANGE = 512


void pwm_setup(void);
int check_heatpower (int heat2pwm);
int heat2pwm (int heat2pwm);


void pwm_setup(void)
{
  if (wiringPiSetupGpio() == -1)
    exit (1) ;
    
  pinMode (PWM_PIN01, PWM_OUTPUT);
  pwmSetClock (MAIN_FREQ / PWM_RANGE / PWM_CLK);
  pwmSetRange (PWM_RANGE);
 
  return 0;
}


int check_heatpower (int heat2pwm)
{
  if (heat2pwm > MAX_LOAD_POWER)
  {
    return MAX_LOAD_POWER;
  }
  return heat2pwm;
}

int heat2pwm (int heat2pwm)
{
  return heat2pwm / MAX_LOAD_POWER * (PWM_RANGE-1);  
}

int main()
{
  pwm_setup();
  pwmWrite(heat2pwm(check_heatpower(2000)));
  return;
}

Ich habe versucht, die Funktionen zu trennen und möglichst wenig Inhalt 
reinzupacken. Ist dies so wünschenswert (zB heat2pwm könnte ich auch 
direkt ind pwm write reinschreiben oder check_heatpower in heat2pwm)? 
Ist der Aufruf auch so ok (mit den funktionen verschachtelt, kommt es 
mir merkwürdig vor).
Bin leider lange nicht mehr am programmieren gewesen :)
Für ein paar Hinweise wäre ich sehr dankbar ;)

[Michael B. (laberkopp)]

Theoretisch ok, pwmWrite will wohl noch die einzig verfügbare Pinnummer.
Praktisch würde ich heat2pwm nicht als Parameternamen in check_heatpower 
verwenden wenn er schon als Funktion anderweitig auftaucht.

[Lothar M. (Firma: Titel) (lkmiller) (Moderator)]

Irgendwie mutet es ein wenig strange an, wenn eVariablen wie Funktionen 
z.B. "heat2pwm" heißen...

Du solltest übrigens generell deine Rechnung ändern, denn wenn der 
Compiler von links nach rechts rechnet, was ist dann 
heat2pwm/MAX_LOAD_POWER in 2999 von 3000 Fällen? Richtig: es ist 0 und 
deshalb kommt in dieser Rechnung von 0 bis 2999 das Ergebnis 0 heraus 
und bei nur 3000 das Ergebnis 511.

Abhilfe schafft eine Umstellung (und zur Sicherheit Klammern):

  return (heat2pwm * (PWM_RANGE-1)) / MAX_LOAD_POWER;

Die Formel ist aber im Grunde falsch (wie so viele ADC-Berechnungen mit 
Faktoren wie 255, 511, 1023, usw):

  return heat2pwm / MAX_LOAD_POWER * (PWM_RANGE-1);

Denn hier bekommst du nur und genau bei heat2pwm = 3000 den PWM-Wert 
511, aber dafür von heat2pwm = 0..5 den PWM-Wert 0.

Korrekt wäre es, den Wertebereich des Sollwerts auf 0..2999 zu begrenzen 
und mit 512 zu skalieren:

  if (heat2pwm > MAX_LOAD_POWER-1)
  {
    return MAX_LOAD_POWER-1;

:
:
  return (heat2pwm * PWM_RANGE) / MAX_LOAD_POWER;

Ich würde da übrigens deutsche Namen verwenden, so lange das kein 
internationales Projekt wird. Denn es gibt im Englischen keine 
"heatpower" in dem Sinne, wie sie hier verwendet wird. Und mir ist auch 
rätselhaft, wie man eine "heat" in eine "PWM" umrechnen kann...

[Monk (Gast)]

Matthias schrieb:
> Ich habe versucht, die Funktionen zu trennen und möglichst wenig Inhalt
> reinzupacken. Ist dies so wünschenswert?

Ja, es verbessert die Lesbarkeit des Codes.

Nur wenn du wirklich einen Performance-Engpass hast, der die Funktion 
beeinträchtigt, würde ich versuchen, den Quelltext für die Maschine zu 
optimieren.

Falls dein Gerät Strom in Größenordnung von Kilowatt verheizt, beachte 
die Vorschriften des Netzbetreibers. Man darf nämlich nicht beliebig 
große Leistungen per PWM steuern.

[Lothar M. (Firma: Titel) (lkmiller) (Moderator)]

Steve van de Grens schrieb:
> Falls dein Gerät Strom in Größenordnung von Kilowatt verheizt
Könnten natürlich auch 3000 mW sein. Denn diese Information bzw. die 
Skalierung ist in irgendeinem anderen Teil der Doku zu finden.

[Matthias (matjoe)]

Danke jedenfalls für die vielen Anmerkungen zum besagten Codeausschnitt.

Und keine Sorge, ich möchte einen gekauften Leistungssteller 
(Phasenanschnittsteuerung) mit PWM EINGANG ansteuern. Die 
Leistungselektronik und Netzverträglichkeit habe ich somit eingekauft.

LG

[Lothar M. (Firma: Titel) (lkmiller) (Moderator)]

Matthias schrieb:
> ich möchte einen gekauften Leistungssteller (Phasenanschnittsteuerung)
> mit PWM EINGANG ansteuern.
Nicht alles, was man kaufen kann, darf so ohne weiteres auch betrieben 
werden. Und zum Thema gibt es durchaus Regelwerke, die simple 
Phasenanschnittgeräte über 200W verbieten:
- VDE-AR-N+4100
- TAB 2007
- TAEV 2016

> Die Leistungselektronik und Netzverträglichkeit habe ich somit
> eingekauft.
Welches Gerät ist es denn? Kann das evtl. mehr als nur simplen 
Phasenanschnitt?

[Vancouver (vancouver)]

Ehrlich gesagt hätte ich diesen Code komplett in die Mainfunktion 
gepackt. Du hast einige extrem einfache Funktionen definiert, die Du nur 
an einer einzigen Stelle aufrufst. Dadurch produzierst du einen ziemlich 
substanzlosen Codeschaum (damit meine ich die Syntax, nicht die 
Funktion). Best Practice: Wenn du etwas kurz formulieren kannst, ohne 
dass die Verständlichkeit verloren geht, dann mach es auch. Funktionen 
lohnen sich erst ab einer gewissen Komplexität und/oder mehreren 
Aufrufpunkten. Hingegen lohnen sich Sourcecode-Kommentare immer :-)

Was die Sprache angeht, möchte ich Lothar widersprechen. Deutsche 
Variablennamen klingen irgendwie immer nach Volkshochschul-Basic-Kurs 
aus den 80ern. Bei dem Mischmasch aus englisch-stämmigen 
C-Schlüsselwörtern und deutschen Namen und Kommentaren sträuben sich mir 
die Haare, aber das ist natürlich nur mein persönliches Empfinden. Aber 
es gibt immer eine gewisse Wahrscheinlichkeit, dass dein Code irgendwann 
mal in einem englischsprachigen Forum landet. Wir lesen hier ja oft 
genug Sourcecodes mit französischen oder spanischen Variablennamen, und 
dann geht die Raterei los.

Und btw, du willst wirklich einen RPi dafür nehmen? Mehrere CPU-Cores, 
Gigabyte Speicher und ein Multiuser-Multitasking-Betriebssystem, um 
einen Tauchsieder zu regeln? Ein Arduino-Nano-Klon vom Chinesen könnte 
das mit einem Bruchteil der Ressourcen, und der C-Code wäre fast der 
selbe.

[Sebastian S. (dsebastian)]

Matthias schrieb
> int heat2pwm (int heat2pwm)
> {
>   return heat2pwm / MAX_LOAD_POWER * (PWM_RANGE-1);
> }

Kompiliert das überhaupt? Die Funktion heat2pwm bekommt eine int 
gleichen Namens übergeben. Spätestens bei

return heat2pwm / …

wüßte ich ja nicht mehr so recht, ob hier eine Funktion rekursiv 
aufgerufen werden soll (dann wäre aber die Zeile fehlerhaft), oder ob 
hier die übergebene int gemeint ist…

[Klaus H. (hildek)]

Vancouver schrieb:
> Ehrlich gesagt hätte ich diesen Code komplett in die Mainfunktion
> gepackt.
Ja, da stimme ich dir zu! Ein paar sinnvolle Kommentarzeilen zusammen 
mit einer Leerzeile strukturieren auch.

> Was die Sprache angeht, möchte ich Lothar widersprechen. Deutsche
> Variablennamen klingen irgendwie immer nach Volkshochschul-Basic-Kurs
> aus den 80ern. Bei dem Mischmasch aus englisch-stämmigen
> C-Schlüsselwörtern und deutschen Namen und Kommentaren sträuben sich mir
> die Haare, aber das ist natürlich nur mein persönliches Empfinden.

Naja, wie du sagst: persönliches Empfinden.
Jemand der die Sprache gerade lernt ist vermutlich dankbar, wenn er 
zwischen den (englischen) Schlüsselwörtern und Bibliotheksfunktionen und 
den deutsch geschriebenen Variablennamen und eigenen Funktionen 
einfach(er) unterscheiden kann. Ich hätte mich damals jedenfalls bei den 
Codebeispielen deutlich leichter getan.
Dem Compiler jedenfalls ist es völlig egal ... 😀

[M. K. (sylaina)]

Sebastian S. schrieb:
> Matthias schrieb
>> int heat2pwm (int heat2pwm)
>> {
>>   return heat2pwm / MAX_LOAD_POWER * (PWM_RANGE-1);
>> }
>
> Kompiliert das überhaupt? Die Funktion heat2pwm bekommt eine int
> gleichen Namens übergeben. Spätestens bei
>
> return heat2pwm / …
>
> wüßte ich ja nicht mehr so recht, ob hier eine Funktion rekursiv
> aufgerufen werden soll (dann wäre aber die Zeile fehlerhaft), oder ob
> hier die übergebene int gemeint ist…

Stell dir mal folgendes vor:

uint8_t myFunkyVariable = 10;
...
uint8_t myFamousFunction(uint8_t myFunkyVariable){
  return myFunkyVariable/2;
}

...

int main(void){
  ...
  uint8_t aHalfOfFunkyStuff = myFamousFunction(38);
  ...
}

Welchen Wert wird aHalfOfFunkyStuff haben am Ende? 5 oder 19? Auch das 
wird fehlerfrei compilieren, genauso wie das von dir angesprochene ;)

[Sebastian S. (dsebastian)]

M. K. schrieb:
>
> Welchen Wert wird aHalfOfFunkyStuff haben am Ende? 5 oder 19? Auch das
> wird fehlerfrei compilieren, genauso wie das von dir angesprochene ;)

Danke für das kleine Rätsel, kann es so aus dem Stehgreif nicht 
beantworten. Was sich vielleicht auch durch meine (Rück-) Frage erahnen 
ließ … mein Bauchgefühl und etwas Kopf sagt mir 19.

Aber den Punkt den ich machen wollte ist der, dass die Lesbarkeit des 
Codes leidet, wenn Funktionen und Variable den gleichen Namen und 
Schreibweise (groß/klein) haben. Und umschifft, wenn konsequent 
angewendet, auch die Notwendigkeit die Frage 5 oder 19 zu beantworten. 
Eine mögliche Fehlerquelle weniger .

[Bruno V. (bruno_v)]

Sorry, aber die Benennung ist doch irreführend. Und damit meine ich 
**falsch**.


check_heatpower bekommt eine heatpower und gibt eine zurück. Da ist kein 
"pwm" in der Funktion.

heat2pwm bekommt ebenfalls eine heatpower. Die sollte ebenfalls nicht 
2pwm heißen, geht aber noch irgendwie.

Über den Rest kann man sicher geteilter Meinung sein. Ich finde 
einerseits check_heatpower eher klein/klein,
pwmWrite(heat2pwm(check_heatpower(2000))) das Gegenteil. pwmWrite macht 
ja keine generelle PWM. Sondern steuert die Heizung an. Ich hatte da 
eine Funktion (z.b. SetHeat) die mit 2000 aufgerufen wird, die Grenzen 
(auch 0) testet und dann pwmWrite aufruft. Das hängt aber stark vom 
restlichen Code und deinem Style ab

[M. K. (sylaina)]

Sebastian S. schrieb:
> Danke für das kleine Rätsel, kann es so aus dem Stehgreif nicht
> beantworten. Was sich vielleicht auch durch meine (Rück-) Frage erahnen
> ließ … mein Bauchgefühl und etwas Kopf sagt mir 19.

Richtig. idR haben nämlich lokale Bezeichner das höhere Gewicht als 
(globale) Bezeichner, die darüber stehen.

Sebastian S. schrieb:
> Aber den Punkt den ich machen wollte ist der, dass die Lesbarkeit des
> Codes leidet, wenn Funktionen und Variable den gleichen Namen und
> Schreibweise (groß/klein) haben. Und umschifft, wenn konsequent
> angewendet, auch die Notwendigkeit die Frage 5 oder 19 zu beantworten.
> Eine mögliche Fehlerquelle weniger .

Dann schreib das auch so, dann weiß man direkt was du meinst ;)
Ich sehe das übrigens ähnlich. Bezeichner mit identischen Namen sollte 
man vermeiden. Übrigens, mein Beispiel war noch etwas kniffliger als 
deines: Der Aufruf einer Funktion unterscheidet sich ja doch schon 
erheblich vom Aufruf einer Variablen in C. Spannender ist da der Aufruf 
zweier Variablen mit selben Namen. So wie ich es geschrieben habe ist 
die globale Version innerhalb der Funktion gar nicht erreichbar ;)

[Frank E. (Firma: Q3) (qualidat)]

a) Funktionen mache ich nur dann, wenn ich den enthaltenen Code 
tatsächlich oder vermutlich zukünftig mehr als einmal benötige

b) wenn ich das Resultat einer Funktion/Methode innerhalb eines 
Programmblocks oder einer eigenen Funktion mehrfach benötige, kopiere 
ich deren Resultat in eine lokale Variable und nutze diese so oft wie 
notwendig

[M. K. (sylaina)]

Frank E. schrieb:
> a) Funktionen mache ich nur dann, wenn ich den enthaltenen Code
> tatsächlich oder vermutlich zukünftig mehr als einmal benötige

Ich mach das in erster Linie der Übersichtlichkeit halber. Wenn ich 
Funktionen zukünftig auch verwenden will oder dies als sinnvoll erachte 
mache ich sogar Klassen daraus ;)

[Christoph S. (mr9000)]

Frank E. schrieb:
> a) Funktionen mache ich nur dann, wenn ich den enthaltenen Code
> tatsächlich oder vermutlich zukünftig mehr als einmal benötige
>
> b) wenn ich das Resultat einer Funktion/Methode innerhalb eines
> Programmblocks oder einer eigenen Funktion mehrfach benötige, kopiere
> ich deren Resultat in eine lokale Variable und nutze diese so oft wie
> notwendig

Klingt wie etwas, was der Compiler für mich macht.