andi hat geschrieben:@Richy
Servus, hast schon mal Zeit gehabt dir den Code anzusehen ?
Hallo Andi,
ich habe mal fix reingeschaut.
Möchtest du ehrliche Kritik oder eine, die der Seele nicht wehtut?
Na, ich probiere es mit konstruktiver Kritik, auch wenns mir auf den Nägeln brennt.
Fangen wir mit der Android-App aus dem git-Repo an:
Ich habe bisher noch keine Android-App geschrieben, daher bleibe ich mal allgemein.
1. Von allen Sprachen dieser Welt, musste es unbedingt Pascal sein? Java oder C++ wären halt etwas zeitgemäßer und Objektorientiert.
2. Kommentare wären fein.
3. Gui und Funktion trennen auch.
4. Aufteilen auf mehrere Dateien.
Der eigentliche Code für den µC gefällt mir besser. Da müsste man aber auch genau reinschauen, um den zu prüfen. Hast du ein Platinenlayout, welcher AVR wurde genau verwendet?
Folgendes ist mir aufgefallen:
1. In der Interrupt-Routine "handleSensor" führst du die kompletten Berechungen inkl. vieler if/else-Verzweigungen durch. Diese Funktion sollte jedoch so kurz wie irgendwie möglich gehalten werden, d.h. nur den Timer abrufen, dessen Wert speichern und die Funktion verlassen. Die eigentliche Berechnung führst du dann in einer Funktion auf, die vom main-loop zyklisch aufgerufen wird.
Der Hintergrund ist folgender: Im Interrupt blockiert man den Rest vom µC ziemlich gewaltig. if/else-Anweisungen gehören mit den zu "kostspieligsten" Aufrufen überhaupt, d.h. sie kosten massiv Rechenzeit. Das beides kombiniert kann einem später
ziemlich den Programmlauf kaputt machen.
2. In der Main-Loop führt du, egal ob der Motor läuft oder nicht, die Konfiguration durch. Das halte ich für fehleranfällig und ist eigentlich nicht nützlich.
Daher folgender Vorschlag:
- Konfiguration in eigene Routine auslagern.
- diese in der Main-Loop nur ausführen, wenn für eine bestimmte Zeit (vllt. 5s) kein Impuls mehr vom Geber gekommen ist.
Sauberer und verhindert, dass wenn mal im laufenden Betrieb Mist über die serielle Schnittstelle kommt, auf einmal die Zündung Unsinn macht.
Generell würde ich die Konfiguration noch etwas mehr absichern als über den Empfang von 3 ASCII-Zeichen. Ein kleines Protokoll mit Prüfsumme macht das sicherer und verhindert ebenfalls ungewollte Verstellung der Zündung.
Ebenfalls würde ich noch eine Prüfung der empfangenen Zündungsimpulse auf Plausibilität empfehlen (anhand der Drehzahl und einer konfigurierbaren Winkelbeschleunigung am Motor).
Als Geber sollte man einen direkt aus dem Automotive-Bereich verwenden, die funktionieren und sind billig.
Nach meinen Erfahrungen mit meinem Tacho an der ES kann ich sagen, dass man sich dort noch auf einige Überraschungen gefasst machen sollte. KFZ-Umgebungen sind mit die dreckigsten, die es so gibt.