Bugs in aktueller c't Bot Software

Die Programmierung des c't-Bot
Antworten
Sheridan

Bugs in aktueller c't Bot Software

Beitrag von Sheridan » 08 Feb 2009, 00:21

Hallo Leute,
ich habe mich heute nachmittag mal etwas mit der neuesten software aus dem Repositorium beschäftigt und dabei erstaunlicherweise noch Bugs gefunden, die sogar die Compilierung verhindert haben.

Hier die Liste der Änderungen, die ich vornehmen mußte:

Datei rc5.c

Hier mußte noch #include timer.h hinzugefügt werden, sonst gabs Compiler - Fehler.
In der Funktion rc5_control() findet sich

#ifndef DISPLAY_AVAILABLE // B.J.
default_key_handler(); // Falls Display aus ist, ist auch GUI aus => Tastenbehandlung hier abarbeiten
#endif

Das ist Quatsch. In der Funktion default_key_handler(); wird der Screen eingestellt. Es gibt keine andere Funktion in der das sonst noch eingestellt werden kann.

Datei: display.h

Hier steht uint8 display_screen; obwohl es extern uint8 display_screen; heißen müßte. Erstaunlicherweise wurde das beim Compilieren nicht angemeckert.

Zuletzt noch eine Sache, die nicht falsch ist, die ich aber trotzdem nicht verstehe. Warum heißt es

#define MS_TO_TICKS(ms) ((ms)*(1000/8)/(TIMER_STEPS/8))

In dieser Formel kürzt sich die 8 ja wieder raus. Aus welchem Grund wurde sie überhaupt hineingeschrieben.

Gruß

Bernhard

eax
Friends of Robby
Friends of Robby
Beiträge: 755
Registriert: 18 Jan 2006, 16:16
Wohnort: Karlsruhe

Re: Bugs in aktueller c't Bot Software

Beitrag von eax » 08 Feb 2009, 12:20

Hi,
Sheridan hat geschrieben:Hallo Leute,
ich habe mich heute nachmittag mal etwas mit der neuesten software aus dem Repositorium beschäftigt und dabei erstaunlicherweise noch Bugs gefunden, die sogar die Compilierung verhindert haben.
na ja, da der Code erfreulicher Weise ständig erweitert wird, kommt sicherlich auch ab und zu mal ein Bug mit hinein.
Sheridan hat geschrieben:Datei rc5.c

Hier mußte noch #include timer.h hinzugefügt werden, sonst gabs Compiler - Fehler.
Ja, den Fehler bekommt man aber nur, wenn BEHAVIOUR_AVAILABLE aus ist und das dürfte nur sehr selten gewünscht sein. Daher fiel das wohl bisher noch niemandem auf. Allerdings knallt es dann auch noch an anderer Stelle.
Sheridan hat geschrieben:In der Funktion rc5_control() findet sich

#ifndef DISPLAY_AVAILABLE // B.J.
default_key_handler(); // Falls Display aus ist, ist auch GUI aus => Tastenbehandlung hier abarbeiten
#endif

Das ist Quatsch.
Das ist überhaupt kein Quatsch, wenn der Bot kein Display hat und daher DISPLAY_AVAILABLE aus ist, erfolgt die Tastenbehandlung der RC5-Fernbedienung logischer Weise nicht per Display-Screen und muss deshalb dort explizit aufgerufen werden.
Sheridan hat geschrieben:In der Funktion default_key_handler(); wird der Screen eingestellt. Es gibt keine andere Funktion in der das sonst noch eingestellt werden kann.
Die Anmerkung macht irgendwie keinen Sinn, default_key_handler() wertet die Tastenkommandos der Fernbedienung aus, sofern sie nicht für einen speziellen Screen überladen wurden. Wer kein Display an seinem Bot hat, der möchte ihn aber vermutlich trotzdem per Fernbedienung steuern können und braucht daher auch den Funktionsaufruf dort.
Sheridan hat geschrieben:Datei: display.h

Hier steht uint8 display_screen; obwohl es extern uint8 display_screen; heißen müßte. Erstaunlicherweise wurde das beim Compilieren nicht angemeckert.
Das extern macht in diesem Fall wohl keinen Unterschied, es läuft so oder so auf external linkage hinaus, darum meldet der Compiler dort auch keinen Fehler.
Sheridan hat geschrieben:Zuletzt noch eine Sache, die nicht falsch ist, die ich aber trotzdem nicht verstehe. Warum heißt es

#define MS_TO_TICKS(ms) ((ms)*(1000/8)/(TIMER_STEPS/8))

In dieser Formel kürzt sich die 8 ja wieder raus. Aus welchem Grund wurde sie überhaupt hineingeschrieben.
Da es einen Überlauf geben kann (Restklassenring), kürzt sich die 8 nicht zwangsläufig raus. Weil ein int beim avr-gcc standardmäßig 16 Bit groß ist, kommt es hier für gängige Werte von ms schnell zu einem Überlauf bei der Multiplikation mit 1000. Es wird durch 8 geteilt, weil 8 der ggT von 1000 und 176 ist, somit erhält man den größten Wertebereich in 16 Bit ohne Genauigkeitsverlust.

Gruß,
Timo

Sheridan

Beitrag von Sheridan » 08 Feb 2009, 15:24

Hallo Timo,
vielen Dank für Deine Antwort. Mein Beitrag klang vielleicht etwas launiger als beabsichtigt. Aber nach ein paar Stunden intensiver Beschäftigung mit einem komplexen Sourcecode ist man schon mal leicht genervt. Letzlich ist dieses Projekt insgesamt natürlich eine tolle Sache.

Ich habe mich gerade nochmal intensive mit den Themen auseinandergsetzt. Trotzallem verstehe ich einiges immer noch nicht.
Ja, den Fehler bekommt man aber nur, wenn BEHAVIOUR_AVAILABLE aus ist und das dürfte nur sehr selten gewünscht sein. Daher fiel das wohl bisher noch niemandem auf. Allerdings knallt es dann auch noch an anderer Stelle.
Okay, das stimmt. Ich hab tatsächlich BEHAVIOUR_AVAILABLE abgeschaltet und einfacher den Rest leichter ausprobieren zu können. Und es stimmt, wenn BEHAVIOUR_AVAILABLE aktiv ist, funktioniert es.

Nur: Im anderen Fall scheitert das Linken, da die Funktion timer_reset() nicht gefunden wird. Die wird in "timer.h" definiert. Diese Funktion muß als Deklaration aber auf jeden Fall im Kopf rc5.c landen. Nur kann ich nicht erkennen, wie das mit BEHAVIOUR_AVAILABLE zusammenhängt.

Die Anmerkung macht irgendwie keinen Sinn, default_key_handler() wertet die Tastenkommandos der Fernbedienung aus, sofern sie nicht für einen speziellen Screen überladen wurden. Wer kein Display an seinem Bot hat, der möchte ihn aber vermutlich trotzdem per Fernbedienung steuern können und braucht daher auch den Funktionsaufruf dort.
Als wenn ich den Screen wechseln will, funktioniert das doch so:

1. Mit der Funktion rc5_control(void) muß mit ir_read() das Kommando der Fernbedienung gelesen werden.

2. Dann muß aber die Funktion default_key_handler(); aufgerufen, um dort mit der Funktion rc5_screen_set(); die Variable display_screen zu setzen.

3. Erst dann wird mit der Funktion gui_display(int8 screen) auf den jeweiligen Screen umgeschaltet. Ich muß also die Funktion default_key_handler(); in jedem Fall aufrufen. Deswegen verstehe ich den Compiler - Switch an der Stelle nicht.

Zu
Da es einen Überlauf geben kann (Restklassenring), kürzt sich die 8 nicht zwangsläufig raus. Weil ein int beim avr-gcc standardmäßig 16 Bit groß ist, kommt es hier für gängige Werte von ms schnell zu einem Überlauf bei der Multiplikation mit 1000. Es wird durch 8 geteilt, weil 8 der ggT von 1000 und 176 ist, somit erhält man den größten Wertebereich in 16 Bit ohne Genauigkeitsverlust.
Okay, das habe ich verstanden. Vielen Dank.

Schönen Sonntag noch.
Gruß Bernhard

eax
Friends of Robby
Friends of Robby
Beiträge: 755
Registriert: 18 Jan 2006, 16:16
Wohnort: Karlsruhe

Beitrag von eax » 09 Feb 2009, 17:16

Hi,
Sheridan hat geschrieben:Nur: Im anderen Fall scheitert das Linken, da die Funktion timer_reset() nicht gefunden wird. Die wird in "timer.h" definiert. Diese Funktion muß als Deklaration aber auf jeden Fall im Kopf rc5.c landen. Nur kann ich nicht erkennen, wie das mit BEHAVIOUR_AVAILABLE zusammenhängt.
bot-logik.h bindet available_behaviours.h ein und available_behaviours.h bindet behaviour_delay.h ein, welche letztlich timer.h einbindet. Fehlt halt nur, wenn BEHAVIOUR_AVAILABLE aus ist, von daher sollte da schon ein #include "timer.h" rein.
Sheridan hat geschrieben:Als wenn ich den Screen wechseln will, funktioniert das doch so:

1. Mit der Funktion rc5_control(void) muß mit ir_read() das Kommando der Fernbedienung gelesen werden.

2. Dann muß aber die Funktion default_key_handler(); aufgerufen, um dort mit der Funktion rc5_screen_set(); die Variable display_screen zu setzen.

3. Erst dann wird mit der Funktion gui_display(int8 screen) auf den jeweiligen Screen umgeschaltet. Ich muß also die Funktion default_key_handler(); in jedem Fall aufrufen. Deswegen verstehe ich den Compiler - Switch an der Stelle nicht.
Nicht ganz:
1. bot_behave() wird in jedem Bot-Zyklus genau einmal aufgerufen. bot_behave() ruft rc5_control() auf, das mit ir_read() die Fernbedienung abfragt, das Toggle-Bit auswertet und die zuletzt gedrückte Taste in RC5_Code speichert.
2. Alle 100 ms wird gui_display() aufgerufen, um das Display neu zu zeichnen. Je nach eingestelltem Screen wird dazu eine Funktion aufgerufen. Diese Display-Funktion kann nun den in RC5_Code gespeicherten Tastencode auswerten (wenn gewünscht) und anschießend löschen. Die Display-Funktionen gibt es deshalb, damit z.B. ein Verhalten selbst bestimmen kann, was auf dem Display angezeigt wird, falls dieser Screen aktiv ist. Außerdem lässt sich so pro Screen eine Tastenbelegung einstellen.
Hat die Display-Funktion den RC5_Code aber gar nicht gelöscht (weil es kein Code war, der sie interessiert), wird er anschließend in gui_display() mit dem Aufruf von default_key_handler() ausgewertet. Dort werden also alle Tasten abgehandelt, die zum Zuge kommen, falls der aktuelle Screen für sie keine spezielle Belegung hat.

Für den Fall, dass der Bot kein Display hat, gibt es natürlich auch keine Display-Screens und der Aufruf von gui_display() findet nicht statt. Deshalb kann nur die Standard-Tastenbelegung greifen und genau das passiert dann in rc5_control() mit dem Aufruf von default_key_handler(). Mit Display wären so keine Tastenbelegungen pro Display-Screen möglich, ohne Display gibt es die aber ja eh nicht.


Btw: Falls du Eclipse als Editor benutzt, hilft es sehr mit einem Rechtsklick auf eine Funktion oder einen Funktionsaufruf im Code mit "Open Declaration", "References -> Project" oder "Declarations -> Project" durch den ganzen Code zu browsen.

Grüße,
Timo

Sheridan

Beitrag von Sheridan » 09 Feb 2009, 23:16

Hallo Timo,
vielen Dank für Deine ausführliche Antwort. Deine Ausführungen konnte ich gut nachvollziehen, so daß ich diese Punkte jetzt verstanden habe. Ich arbeite auch mit Eclipse. Bisher habe ich lediglich alles exakt so nachvollzogen, wie es von C't vorgegeben wurde. Die Funktionen Open Declaration/Definition bzw. References -> Project funktionieren bei mir nur teilweise. D.h. in ca. 50% der Fälle funktionierts, in allen anderen Fällen wird gar nichts gefunden. Am wenigsten funktioniert References. Hier wird meist gar nichts gefunden. Deshalb bin ich gezwungen hauptsächlich mit Search -> Project zu arbeiten.

In meiner Abteilung arbeiten wir an einer umfangreichen Software, die wir ebenfalls in C entwickeln. Und dafür haben wir die strikte Designregel aufgestellt, daß KEINE *.h - Datei eine andere includen darf. Sonst wird das ganze unüberschaubar.

Wenn ich erst mal mit dem gesamten Source - Code vertraut bin, werde ich sicher versucht sein, hier einiges komplett umstricken zu wollen.

Trotzallem bin ich natürlich dankbar, daß einem mit diesem Projekt mal gezeigt wird, wie man so eine Roboterprogrammierung komplett mit allem drum und dran aufsetzt. Allein hätte ich nie die Zeit so weit zu kommen.

Bis dann

Gruß Bernhard

Antworten