Bugs und Softwarequali

mrtn · December 13, 2023, 12:05pm

Hi,

ich habe gestern angefangen, mich mit dem Backend auseinanderzusetzen und mir sind dabei ein paar Dinge aufgefallen (hatten schon kurz per Mail kontakt, hier etwas ausführlicher). Meine Mission war es, den internen Autotherm Temperatursensor als Innenraumtemperatursensor für die Heizungssteuerung zu verwenden, da ich noch keinen am Raspberry hängen habe. Dazu habe ich das Dropdown Menü bei den Settings um einen siebten Sensor erweitert und anschließend die betreffenden Funktionen bei der Heizung. Es betrifft folgende Funktionen:

Heater Autotherm: check sensor (gibt es dort 2x)
HeaterControls + MaxxFan: get inside tempsensor (gibt es dort auch 2x in sehr ähnlich)
und vermutlich bei jeder anderen Heizung…

zunächst zur Funktion:

01 msg.payload = global.get("heattempsensor");
02 
03 var usb1 = parseInt(global.get("usb1uart"));
04 var usb2 = parseInt(global.get("usb2uart"));
05 var usb3 = parseInt(global.get("usb3uart"));
06 var usb4 = parseInt(global.get("usb4uart"));
07 
08 if (usb1 != 5 && usb2 != 5 && usb3 != 5 && usb4 != 5 && usb1 != 8 && usb2 != 8 && usb3 != 8 && usb4 != 8) {
09     return null
10 }
11
12 if (msg.payload != 1 && msg.payload != 2 && msg.payload != 3 && msg.payload != 4 && msg.payload != 5 && msg.payload != 6 && msg.payload != 7)
13    msg.payload = "no sensor"
14
15 if (msg.payload == 1)
16    msg.payload = global.get("temp1");
17
18 if (msg.payload == 2)
19    msg.payload = global.get("temp2");
20
21 if (msg.payload == 3)
22     msg.payload = global.get("temp3");
23
24 if (msg.payload == 4)
25     msg.payload = global.get("temp4");
26
27 if (msg.payload == 5)
28    msg.global.get("dimmytemp1")
29
30 if (msg.payload == 6)
31     msg.global.get("dimmytemp2")
32
33 if (msg.payload == 7)
34     msg.payload = global.get("heaterroomtemp");
35
36
37 if (global.get("temp1") == "" && global.get("temp2") == "" && global.get("temp3") == "" && global.get("temp4") == "" && global.get("dimmytemp1") == "" && global.get("dimmytemp2") == "" && global.get("heaterroomtemp") == "") {
38     msg.payload = "no TempSensor"
39 }
40
41 if(msg.payload === "no TempSensor")
42 {
43  msg.payload ="TempMode not available, no InsideTempSensor set";
44  return [msg,null];
45 }
46 else
47 {
48  msg.topic="setpoint";
49  return [null,msg];
50 }

(Achtung, in dem Code befindet sich bereits mein 7. Sensor “heaterroomtemp”)

zunächst zum 1. Bug:
in den Zeilen 28 & 31 müsste es nach meiner Auffassung heißen:
msg.payload = global.get(“…”);

nun ein paar Worte zur Softwarequalität:
Ich bin NodeRed Neuling, daher kenne ich mich nicht sonderlich aus, dass jedoch eine ähnliche Funktion mindestens 4 x geschrieben wird, ist aus Gründen der Wartbarkeit problematisch (siehe zweiter Bug später). Ich kenne mich wie gesagt nicht mit NodeRed aus, und welche Möglichkeiten es zur Wiederverwendbarkeit von Funktionen bietet, alternativ könnte man die Prüfung ob und welcher Temperatursensor die Innenraumtemperatur überwacht ggf. zu den Einstellungen und dem Dropdownmenü zentral verfrachten, da würde dann anstelle des Sensor-Index der Variablenname / Key des gewählten Sensors gespeichert.

Bei Bug2 hat dann Tatsächlich die Wartbarkeit zugeschlagen, bei der Erweiterung der beiden Dimmy Sensoren wurde dies in der zweiten “get inside tempsensor” Funktion unter “HeaterControls + MaxFan” vergessen hinzuzufügen.

Weiter ein paar ein paar Worte zur Softwarequalität:
Die Funktion selbst ließe sich deutlich leichter schreiben, wenn die If- Statements der Zeilen 12-31 mit Elseif verknüpft wären. Das If-Statement aus Zeile 8 könnte dann durch ein einfaches abschließendes else ersetzt werden. Das verbessert die Lesbarkeit und würde auch das Erweitern vereinfachen. Allgemein würde ich in anderen Programmiersprachen dafür sorgen, dass die verschiedenen Temperatursensor Optionen aus einer Liste geladen werden und nicht in jeder Funktion hartgecoded auftauchen.
In Zeile 37 würde ich dann nicht überprüfen, ob irgendeiner aller Sensoren einen Wert liefert, sondern ob explizit der ausgewählte und abgefragte Innenraumsensor (an der Stelle ist dessen Wert bereits in msg.payload geschrieben) einen Wert liefert.

Da ich noch keinen Gesamtüberblick über das Backend habe, wollte ich die Änderungen noch nicht “fertig” bereitstellen. Nicht dass ich an anderer Stelle etwas zerschieße…

Nun ein paar Worte für Leute die nicht im Thema Software stecken:
Software wächst in der Regel “historisch”, alle Mitarbeitenden lernen dazu und insbesondere in kleinen Teams muss der Fokus der Entwicklung häufig auf Funktionalität und neuer Funktionalität liegen, da Softwarequalität kurzfristig keinen Mehrwert bietet und lauffähige Software mit Funktionalitäten gerade am Anfang definitiv wichtiger ist. Was bringt schöne Software, die nichts kann. Ich habe daher riesigen Respekt vor eurer bisherigen Arbeit und möchte dies hier (abgesehen von den beiden Bugs) eher als Anregung verstehen, da mir bewusst ist, dass man dafür Zeit / Resourcen benötigt.

Kurz noch als Ausblick, ich plane für meinen Bus folgendes, und kann das bei Gelegenheit bereitstellen:

Einbindung des Raumtemperatursensors der Standheizung (erledigt)
Verbesserter Temperaturmodus, aktuell gibt es von / für Autotherm
Thermostatmodus (vereinfacht: Temeratur > soll = Aus / Temperatur < soll = An), das ständige an/aus ist schlecht für die Heizung und benötigt viel Strom
Temperaturmodus (Heizung ballert bis zur Zieltemp und versucht diese dann auf Sufe 1 zu halten), bei manchen wird es dann aber weiter zu warm, bei mir zu kalt.
Ich will eine Regelung (vermutlich PI Regler), die je nach Temperatur die Heizungsstufe über den Powermode verändert, und falls die kleineste Sufe zu groß ist auch abschaltet. Das verbindet und erweitert die Vorteile der beiden obigen Varianten. (Danke auch an der Stelle schon mal für die Info per Mail)
eine Android App nach meinen Vorstellungen, angepasst auf meinen Van
Verschiedene Datenverbindungen für die App, die automatisch nach gewissen Regeln gewählt werden

Wlan, wenn sich Handy und Pi im selben Netz befinden und unlimited Internet im Netzwerk verfügbar ist
Bluetooth, wenn kein unlimited Internet verfügbar ist, so nutzt das Handy weiter sein eigenes Internet
Internet, wenn weder Bluetooth verfügbar ist, noch Pi und Handy im selben Netz sind. Kann entweder über die mobilen Daten des Bus gehen, oder wenn der Bus zuhause im Wlan hängt.

Mal schauen, wie weit ich komme, ich bin sehr Dankbar für euer Projekt, da es bereits eine gute Grundlage bietet und man schnell erste eigene Ideen umsetzen kann und Funktionen sieht, ohne Ewigkeiten in die Grundlagen zu stecken.

Viele Grüße

ahtdf · December 13, 2023, 4:09pm

@mrtn
Willkommen im Club,
erstmal finde ich das super das sich einer so mit der Software auseinandersetzt.
Und ich binn auch der Meinung das Fehler aufgedeckt und beseitigt werden sollten.
Auch hast du damit recht das ein Programm wächst und die Programmierer dazu lernen.
Ich begleite das Projekt jetzt seid 2 oder 3?? Jahren, allso Quasi von Anfang an.
mit Node red hatte ich bis dato auch keinen Kontakt, aber ich habe mich da hereingefucht,
eine Funktion-Node, ist jedes mal eine herrausforderung für mich, da weder c, noch java oder ähnliches
meine Programmierspache ist. Ich programmiere nur im FUP, und das ist ja mehr oder weniger, wie auch bei Node_red,
Bildchen aneinander schieben. Aber ich finde auch viele Wege führen nach Rom,
auch der Weg den Pekaway eingeschlagen hat. Das Problem ist einfach, wenn zu viel geändert wird,
kann das auf die Programmierung, die von anderen hinzugefügt worden ist zurückschlagen.
Ist auf jeden Fall schon bein umstieg von Relayboard 1 auf die 2. Version passiert,
da musste ich meine ganzen Schaltereingänge ändern.
was die Temperaturfühler angeht, fehlt nicht nur der Temperaturfühler aus der Autotherm, sondern der Temperaturfühler aus dem NS Panel,
oder die 4 Fühler die ich am Dimmi habe, und der DHT31, oder BMP280 am i2Cbus…
Das gleiche gilt für Relays.
Das kann Pekaway nicht abbilden.
Das System sollte so genutzt werden können wie es angeboten wird, und das Fehlerfrei.
All das was selber gestrickt wird, sollte jeder selber für verantwortlich sein.
Wenn ich mir einen Zusätzliche Fühler anlege, dann zu einem bestimmten Zweck,
und dann kann ich den auch direkt mit der Funktion verknüpfen. Dazu brauche ich dann kein Dropdown Menü.
Es ist auf jeden Fall toll wenn hier noch ein bischen frischer Wind reinkommt,
und auch ein paar Flows die man nutzen kann.
Z.B eine “PI” geregelte Heizung ist sicher nicht verkehrt
und für eine APP ist sicher auch großes Interesse da. Haupsache alles nicht zu komliziert
Gruß Arno

Vincent · December 18, 2023, 4:30pm

Hey @mrtn

du sagst es ja schon, wir alle lernen dazu und die Software entwickelt sich ständig weiter. Wir versuchen dabei immer alles möglichst so zu halten, dass es auch Laien verstehen können. Damit einher geht dann der Kompromiss, dass es Softwaretechnisch leider nicht das Schönste ist.
Die Frage stellt sich dann immer, ob Funktionen - also Code - geschrieben werden sollen oder ob man es auch ohne Code abbilden kann… Eine komplette No-Code Version wäre bestimmt nicht schlecht, allerdings tendiere ich selbst immer mehr dazu einen Low-Code Ansatz zu wählen, um das Backend einigermaßen übersichtlich zu halten (was gar nicht so einfach ist) und Funktionen stärker zusammenfassen zu können.
Viele Stellen benötigen da Verbesserungen/Überarbeitungen, sowohl von der Syntax auch als von der Logik her. Aber wie du bereits geschrieben hast, Funktionalität geht erstmal vor. Wenn ich es jetzt komplett neu machen würde, würde ich einiges anders machen.

Auf jeden Fall ist es wichtig, dass wenn Fehler gefunden werden, diese kommuniziert werden, damit wir nachbessern können. Bzw. durch die Open-Source Thematik kann ja schon viel selbst erledigt werden, bis ein offizielles Update kommt.

Hab übrigens mal deinen Code oben durch ChatGPT laufen lassen, da kann man noch einiges dran machen

msg.payload = global.get("heattempsensor");
const usbUarts = Array.from({ length: 4 }, (_, i) => parseInt(global.get(`usb${i + 1}uart`)));

if (!usbUarts.includes(5) && !usbUarts.includes(8)) return null;

const sensorMappings = {1: "temp1", 2: "temp2", 3: "temp3", 4: "temp4", 5: "dimmytemp1", 6: "dimmytemp2", 7: "heaterroomtemp"};
msg.payload = (msg.payload >= 1 && msg.payload <= 7) ? global.get(sensorMappings[msg.payload]) : "no sensor";

const hasTempSensors = ["temp1", "temp2", "temp3", "temp4", "dimmytemp1", "dimmytemp2", "heaterroomtemp"].some(sensor => global.get(sensor) !== "");

if (!hasTempSensors) {
    msg.payload = "no TempSensor";
    msg.topic = "TempMode not available, no InsideTempSensor set";
    return [msg, null];
}

msg.topic = "setpoint";
return [null, msg];

Ist nicht getestet, aber da geht schon einiges.

Die App für iOS und Android wird ebenfalls eine Option bekommen, dass man einfach per HTTP die Daten austauschen kann, wenn man im selben Netzwerk ist. Ansonsten müssen wir die App aber “grundfunktional” halten, da können wir leider nicht all zu viel Rücksicht auf individuelle Wünsche nehmen.