verzeichnisauswahl geht nimmer

[mb405]

@seife
seit deinen menu.cpp menu.h einchecken geht keine verzeichniswahl mehr.
http://cvs.tuxbox-cvs.sourceforge.net/l ... 00019.html
bitte gugste da nochma drüber ?

[dbt]

Mmh...welches Verzeichnis? Aufnahme etc. Welches Menü? Kann keinen Fehler feststellen.

[seife]

Aufnahmeverzeichnis. Die Auswahl funktioniert schon, nur die anzeige nicht. Wenn man ganz rausgeht aus dem menü und wieder rein, dann wirds angezeigt. Bin dran. Braucht wohl auch einen notifier...

[micha-bbg]

Guten Abend,

es scheinen wohl ziemlich alle Menüs betroffen zu sein, z.B. Zeiteingabe im Timermenü oder IP-Nummern in den Netzwerkeinstellungen.

Bei mir baut Neutrino zu Zeit gar nicht mehr:

Code: Alles auswählen

lcdd.cpp:194: error: 'class LcdFontRenderClass' has no member named 'getFamily'
lcdd.cpp:203: error: 'class LcdFontRenderClass' has no member named 'getFamily'
lcdd.cpp:214: error: 'class LcdFontRenderClass' has no member named 'getFamily'
make[5]: *** [lcdd.o] Error 1

[seife]

Bei mir baut Neutrino zu Zeit gar nicht mehr:

Das hatten wir schon. "make tuxbox_libs"

[micha-bbg]

Das hatten wir schon. "make tuxbox_libs"

Danke, baut wieder - da war mir wohl etwas entgangen

[seife]

Bin dran. Braucht wohl auch einen notifier...

Dummerweise geht das nicht so einfach. Das ganze menügedöns ist wohl broken by design...

Ich erwäge, die letzten commits einfach zu reverten und es kaputt zu lassen.

Das Problem ist IMHO, dass den Menuwidgets (z.B. Verzeichnisauswahl oder "Welche Sats scannen?") ein Pointer/eine Referenz auf den Startwert übergeben wird, und der dann "inplace" verändert wird. Die aufrufende Funktion weiss das aber nicht, denkt der wird nicht mehr benötigt und verwendet denselben Pointer schon wieder für was anderes (das war das Problem mit der Sat/Transponderauswahl beim scan, konnte man sehr schön mittels 'printf("pointer value: %p\n", variable);' an verschiedenen stellen sehen).

Dann gibt es designprobleme, dass z.B. strings in den ursprünglich übergebenen Pointer reinkopiert werden, ohne zu wissen, ob da überhaupt genug platz ist etc.

Alles sehr dubios, und valgrind kommt auch kaum mit meckern nach. Nur das jetzt zu fixen bedeutet die menüklassen neu zu schreiben (mehr oder weniger), und da habe ich
a) keinen bock zu
b) ist mein C++-fu dafür zu schwach. Es würde also eher C-Code werden...

Aber vielleicht sieht ja jemand was, was ich übersehen habe, einen einfachen workaround oder so.

[mb405]

mach einfach die menu.cpp + .h änderungen wieder rückgängig, dann passt das wieder.
dann kann man sich was andres einfallen lassen mit der zeit.

[seife]

Naja, der code vorher war eindeutig kaputt, und "neutrino crashed nach ein paar wochen" wäre ein wahrscheinliches szenario. Deswegen wäre es mir schon recht, wenn jemand mit mehr C++-Hintergrund da eine zündende Idee hätte.

Die Änderung einfach rückgängig machen sorgt ja nur dafür, dass es so aussieht, als ob es funktioniert...

[mb405]

klär mich mal als nicht(oder nur wenig) codeversteher auf warum die änderungen ?
der fehler tritt ja nur im scan menü auf bei der satauswahl. in allen anderen CMenuForwardern gehts ja.
mach mal im scanmenü 2 oder 3 zusätzliche menüpunkte(über den eintrag Scan Modi, und schalte mal paar scan modis durch)dann haste ein komisches phenomen, welches ich schon mal gepostet hatte.

[seife]

Dass es in anderen Menüs funktioniert ist aber auch nur Zufall, soweit ich das beurteilen kann.

[dbt]

Zugegebenermassen ist es nicht wirklich sinnvoll, deswegen alles zurückzuehmen. Wenn man schon mal dabei ist, dass zu fixen, sollte man das schon machen. Seife hat da schon recht.

...
mach mal im scanmenü 2 oder 3 zusätzliche menüpunkte(über den eintrag Scan Modi, und schalte mal paar scan modis durch)dann haste ein komisches phenomen, welches ich schon mal gepostet hatte.

Das hat einen anderen Grund. Weil die hinteren Items nach der Änderung des Scanmodes neu gemalt werden, bleiben die stehen. Das könnte man eleganter im paint oder über ein erweitertes hide regeln, damit der entsprechende Bereich aufgeräumt wird. Bei dir liegts aber an deinen nicht cvs-konformen Änderungen, auf die man nicht unbedingt Rücksicht nehmen muss, aber wenn man schon dabei ist, kann man das schon mal ausbessern.

[mb405]

jo
ich habs einfach umgangen.
was nich passt wird passend gemacht

[seife]

Ich reverte den commit erstmal. Es hat 10 Jahre mit dem kaputten code mehr oder weniger funktioniert, da kommt es auf ein paar Jahre mehr oder weniger auch nicht mehr an.

Das Redesign würde ich schrittweise machen:
* erst die ganzen CMenu*-Konstruktoren loswerden, die mit std::string arbeiten, nur noch die mit char * übriglassen.
* dann denen ein vorher allozierten char[xxx] string übergeben, damit auch sicher genug platz drin ist.

Der Vorteil ist dann, dass der pointer dann konstant ist. Der wird dann in der CMenu-Klasse auch nicht umgebogen, sondern das Ergebnis wird wieder mit strcpy dort hinkopiert.

Alternativ das Ergebnis per "getResult()"-Methode zurückgeben, anstatt den Eingabestring inplace zu editieren.
Das inplace-Editieren hat aber den Vorteil, dass man evlt. auf so einige ChangeNotifier verzichten kann...

Jedenfalls ist das "wir biegen die pointer hin und her und hoffen, dass inzwischen keiner was anderes an die Stelle gelegt hat", was bisher praktiziert wird, nicht sonderlich korrekt, wie man beim scansetup auch sehr schön sieht.

[mb405]

danke dir.
eigentlich ist es ja nur eine auswahl von optionen,wenn ich das richtig verstanden habe.
jeder option ist eine zahl zugeordnet, der wiederum ein text(char, string) zugeordnet ist. wie das bei den verzeichnissen ist versteh ich net, das is mir zu hoch.
wenn ich das jetzt richtig lese ist dein problem, das einmal mit char* und einmal std::string gearbeitet werden muss ?
kann man das erstmal umgehen, wenn man 2 klassen dafür schreibt ??

[seife]

Nein. Das Problem ist, dass da ein Pointer übergeben wird. Zur zeit der Übergabe zeigt der auf einen char[] oder einen std::string. Der Pointer wird in der Routine umgebogen oder auch nicht, oder an seine Stelle wird ein neuer inhalt kopiert.
Weiter "oben" in der Menüebene wird dann wieder auf den Inhalt des originalen pointers zugegriffen, der sich geändert haben kann oder auch nicht.

Hinzu kommt, dass der Compiler auch feststellen kann, dass die Variable ja nirgends mehr benutzt wird, und deswegen den Platz für was anderes verwendet, so dass "weiter oben" zwar noch auf denselben Pointer zugegriffen wird, aber der Platz, auf den der Pointer zeigt, nun anderen Inhalt hat. Das ist das, was gerade im scansetup im "Ein Sat" -> "Alle Sats" -> "Ein Transponder" Fall passiert.

Dass die Klassen sowohl std::string als auch char[] verarbeiten können, macht es halt (IMHO) noch schwieriger, das zu fixen.
Ansonsten würde man vermutlich mit (pseudocode)

Code: Alles auswählen

char text[123];
strcpy(text, "Meine Option");
MenuChooser *foo = new MenuChooser(&text, xxx1, xxx2);
MenuForwarder *mf = new MenuForwarder(&text, option, option2, foo);
mf->exec();

Auskommen, wenn MenuChooser den &text inplace editiert. Dann hat nämlich der MenuForwarder und der MenuChooser dieselbe Adresse übergeben bekommen und dem Compiler ist auch (hoffentlich klar, dass er bis ans Ende der Routine den Platz nicht für was anderes benutzen darf.

Evtl. könnte man sogar auf den einen oder anderen ChangeNotifier verzichten...

[GetAway]

@dbt

Die Menüklasse macht immer noch Mist, und zwar beim Menü Settings/Tasteneinstellung/Benutzermenü(rt,gn,ge,bl)/Name.
Bis zu einer Eingabe von 6 Zeichen für den Namen ist alles OK, ab dem 7. werden da auch *.raw Namen übergeben. Ich will nicht
ausschließen das es sich bei dem ein oder anderen anders verhalten kann. Broken ist es trotzdem.

[dbt]

An der Stelle wurde ja auch noch nichts geändert. Das Scansetup wollte ich nur nicht so stehen lassen. Das Usermenü ist ohnehin um das Thema bisher noch herumgekommen.

[dbt]

@dbt

Die Menüklasse macht immer noch Mist, und zwar beim Menü Settings/Tasteneinstellung/Benutzermenü(rt,gn,ge,bl)/Name.
Bis zu einer Eingabe von 6 Zeichen für den Namen ist alles OK, ab dem 7. werden da auch *.raw Namen übergeben. Ich will nicht
ausschließen das es sich bei dem ein oder anderen anders verhalten kann. Broken ist es trotzdem.

Probier das mal: neutrino-diff-2010-07-27-23-21-10.patch Damit hatte ich kein solches *raw mehr nach der Eingabe gesehen. Bitte mal testen, wenn das auch geht, wäre das auch mal wech...

[GetAway]

Positiv getestet.

[dbt]

Ok, dann kann man auch noch die Restlichen Setup-Apps aufräumen, die habe ich hier inzwischen auch noch fürs LCD-Setup, Driver/Boot-Setup da liegen. Dann fehlt noch der ganze OSD-Setupkram für Farben, Schrift, aber das gehört dann wieder hier hin. http://www.tuxbox-cvs.sourceforge.net/f ... 14#p377014
Gabs beim Usermenü denn auch keine Probleme mit dem Patch? Wenn nicht, setzt ich das mal fort.

[GetAway]

Der Patch lief sauber durch. Nur mal eine Frage nebenbei.
Warum werden eigentlich überall relative Pfadangaben für lokale Headerdateien angegeben.

Code: Alles auswählen

#include "gui/user_menue_setup.h"

So geht's doch auch.

Code: Alles auswählen

#include "user_menue_setup.h"

[dbt]

KA, vlt. auch Relikte, die der Form halber weiter mitgeschleppt wurden. Würde sagen, dass man das unabhängig davon wer da mal dran werkelt hier und da ändert, spricht auch nichts dagegen wenn da jemand einen kompletten Patch anbietet, andererseits, machts auch nicht kaputt, wenn's so bleibt.