[Java] Bug in IPConnection.enumerate?

[gagahhag]

Ich hätte bei GIT-Hub auch eine Issue eröffnen können, aber wollte nicht dafür einen Account eröffnen, sorry 

 

Ich habe beim Enumerate etwas 'komisches' festgestellt.

 

Beim folgenden Java-Code erwarte ich eigentlich eine Ausgabe mit Namen und UID in einer Zeile.

 

public class TinkerTest {

public static void main(String[] args) throws IOException {
	IPConnection ipCon = null;

	ipCon = new IPConnection("localhost", 4223);

	ipCon.enumerate(new IPConnection.EnumerateListener() {

		@Override
		public void enumerate(String uid, String name, short stackID, boolean isNew) {
			System.out.println("name: ["+name.trim()+"], uid: ["+uid+"]");
		}
	});

	new Scanner(System.in).nextLine();

	ipCon.destroy();
}
}

 

Die Ausgabe ist:

name: [Master Brick 1.0
name: [Rotary Poti Bricklet 1.0
name: [LCD 20x4 Bricklet 1.0

 

Jedoch sollte sie folgendermassen aussehen:

name: [Master Brick 1.0], uid: [9U4XL68z4gG]
name: [Rotary Poti Bricklet 1.0], uid: [aDw]
name: [LCD 20x4 Bricklet 1.0], uid: [bjy]

 

Der Fehler selbst kann umgangen werden, wenn ich den Namen mittel 'name.trim()' ausgebe.

 

Die Länge von 'name' ist komischerweise immer 40 Zeichen. Für mich sieht es so aus, als würden da noch einige NUL-Zeichen (ASCII 0) mitgeschickt...

 

Ich habe dies nur mit Java ausprobiert und weiss nicht, ob das in den anderen Bindings auch der Fall ist.

[M4ST3R]

Hey,

versuch mal statt println einfach nur ein print. Weil println schreibt dir alles in eine Zeile. Möglicherweise musst du auch nur ganz weit nach rechts scrollen. Weiß nicht wieso, aber bei enumerate bekomme ich auch einen ganzen Haufen Leerzeichen hinter dem Namen.

[AuronX]

Also bei mir (C#) ist alles schön knackig kurz. Allerdings wird in C# im LEConverter auch beim ersten NUL nicht weitergebaut. Ich schau gleich aml in die Java_Bindings... in 15 Minuten oder so ^^

 

LG

Jan

 

edit: Jo, also Java ist definitiv liebloser implementiert als C#

String name = "";
for(int i = 0; i < 40; i++) {
    name += (char)bb.get();
}

 

Ich werde gleich mal nen Pull Request hochschieben.

[photron]

AuronX, ich arbeite schon dran.

 

Fix wird gleich commited. Neue Bindings gibts dann heute Abend, es kommen voraussichtlich auch noch Änderungen für WIFI.

[AuronX]

AuronX, ich arbeite schon dran.

 

Das habe ich wohl zu spät gelesen ^^

 

edit:

The basic idea is good [...] but you change can break proper derserialization.

okay... mein Fix war wohl wirklich ein Schnellschuss ^^ Sorry dafür Das passiert wenn man seine Änderungen nicht testet... *selbstgeißel*

[gagahhag]

Ihr seid super. Hatte leider gestern Abend / Nacht keine Lust mehr, das Ganze im Source zu analysieren 

[M4ST3R]

Wir arbeiten im Netbeans Training gerade an der Software. Allerdings sind manche Dinge etwas blöd. Man muss sich im Enumerate immer alles zusammen schneiden und dann ein switch(string) machen. Gibt´s nicht eine schönere Möglichkeit zu erfahren was für ein Device das Brick ist?!

[AuronX]

Sollte zwar in einen anderen Thread, aber ich meine mich zu erinnern, dass auch die offizielle TF-Aussage war: "String parsen". Die TF-Bindings behelfen sich zumindest auch genau damit.

 

Ich denke noch immer, dass das enumerate um etwas wie eine DeviceClass erweitert werden sollte, also eine ID, die nur spezifiziert was für ein Ding da vorliegt.

Ich denke 2 Byte (also ein ushort) sollten als Namensraum reichen, aber auch nötig sein. Das könnte in den Programmiersprachen dann in der Regel als enum dargestellt werden.

Wenn man die Idee weiterspinnt wird gleich jemand kommen und fragen "was ist wenn ich selbst ein Bricklet baue/die Firmware meines Bricks verändere?"

Die Antwort darauf wäre meines erachtens, dass man einen "User-Defined" Addressraum hat. Das heißt z.B. alles was mit Bit 1 beginnt (also alle IDs ab ~32000) würde niemals von TF in Produkten verwendet werden und wäre frei für eigen-Kreationen der Nutzer.

 

Der Haken an meiner Idee?

Ich habe keine Ahnung wie/ob das abwärtskompatibel geht.

[M4ST3R]

Ja also das große Problem was wir momentan haben ist, dass wir zwar per Enum alle Devices erreichen können (ja bei nem kleinen Programm kann ich auch die entsprechenden Bricks verarbeiten und benutzen)

aber jetzt in einer komplexeren Anwendung mit den entgültigen Bricks (z.B. Masterbrick) nichts mehr anfangen können, weil wir kein UID etc. haben.

Mit Device können wir allerdings auch nicht arbeiten, weil wir nicht wissen was für ein Device vorliegt. Und das Parsen von einem String ist viel zu Fehleranfällig (Rechtschreibfehler, Änderungen an der Namensgebung ... etc.)

[AuronX]

Ich hab mal gerade nachgedacht was so geht...

 

Ich könnte mir vorstellen so eine Funktion auf der IPConnection zu implementieren:

public Device CreateDevice(String uid)

 

Ich rede mal gerade nciht über Details der Implementierung, weil da kann man an vielen Stellen streiten. Aber die Semantik wäre, dass ich eine uid reingebe und ein Device rausbekomme das diese uid trägt. Also damit meine ich natürlich, dass ich beispielsweise ein BrickletTemperature in der Hand halte, aber auch das ist ja ein spezielles Device.

 

Ich bin mir nur gerade nicht sicher, wie weit einen das bringt. Weil jetzt habe ich ein Device in der Hand von dem ich nicht weiß was es kann und ich muss wieder switchen (per instanceof o.ä.).

 

Würde dir sowas helfen? Oder ist dein Problem noch ganz woanders zu suchen und ich verstehe es noch nicht?

[gagahhag]

Wenn ich das bis jetzt richtig verstanden habe, braucht es grundsätzlich immer eine Fallunterscheidung, was für ein Device ich jetzt gerade habe.

Das Hauptproblem dabei ist IMHO, auf was für Daten man die Unterscheidung macht. Bis jetzt bleibt uns nur der Name, welchen wir von Enumerate zurückkriegen. Das dies ein String ist, ist die Wahrscheinlichkeit gross, dass man Schreibfehler macht. Wenn jetzt jedes Device eine eindeutige 'Typ-Nummer' hat (wie AuronX unten schon vorgeschlagen hatte) wäre das natürlich einiges 'sicherer'.

 

Für OO-Sprachen wäre das natürlich schön, so ein public Device getDevice(<UID>), da würde halt IPConnection die Fallunterscheidung machen, wir dann aber wieder das 'instance of'...

 

Ich wäre mal für den Vorschlag von AuronX mit der ID

[kuli]

Statt einer eigenen ID wärs doch toll den device typ in die UID einzubauen. Aber da haben wir wohl das Problem der Abwärtskompatibilität.

 

Was jedoch wirklich sinnvoll wäre, ist analog zum EnumerateListener für jedes device ein eigener listener. Z.B. einen MasterListener wo dann eine Instanz vom MasterBrick übergebn wird.

 

Add: Natürlich sollte jeder Listener statt nur einer Methode zwei haben, deviceConnected und deviceDisconnected. Soviel zum Thema OO ;-)

[gagahhag]

Natürlich sollte jeder Listener statt nur einer Methode zwei haben, deviceConnected und deviceDisconnected. Soviel zum Thema OO ;-)

 

<klugscheisser>Nö ein 'isConnected()' reicht völlig aus</klugscheisser>

[M4ST3R]

Also das Problem ist eigentlich, dass du nur beim enumerate den Namen und die UID + StackID hast.

Man müsste nur die abstract Class vollständig in die einzelnen Bricklets übernehmen. Dann hätten alle Bricks und Bricklets (gehen wir mal von einem Master hier aus) auch die UID.

So könnte man ein enumerate fahren und bekäme eine Liste mit Devices, die man dann separieren müsste. Deswegen fände ich eine ID für alle Bricks auch super dann hätte man nicht das Problem mit Strings und dem zuschneiden etc.

Dann könnte man nach dem Enumerate einfach die einzelnen Bricks generieren und in in seinem Code verarbeiten.

Momentan ist ja das Problem, dass man sobald man das Enumerate mit entsprechendem Device verwirft keine Ahnung mehr hat was sein Masterbrick für eine UID hat. Momentan müsste (ich werds nicht tun) ich mir die Mühe machen und eine Map mit Device und Masterbrick schreiben um an alle Informationen zu kommen. Aber das wäre natürlich bei einer vollständigen Implementierung des Masterbricks völlig unnötig! Aber scheinbar sehen das ja mehrere Leute hier so. Was sagt die TF Crew dazu?

Abwärtskompatibel wäre es trotzdem, weil man einfach nur zusätzliche Felder generiert und somit keine Änderung an vorhandenen hätte.

 

Edit: kann es sein, dass die Enumerate nicht threadsicher ist?

[borg]

Kann mir mal einer ein bisschen genauer den Anwendungsfall darstellen?

 

Der ganze Enumerate Kram ist nicht so gut gelungen und sollte auf Dauer definitiv gegen irgendetwas besseres ersetzt werden. Aber wofür genau benutzt ihr Enumerate eigentlich?

 

Warum nicht einfach eine Liste von UIDs und Devices vorhalten die ihr habt und diese einfach abarbeiten?

 

[M4ST3R]

Naja es geht ja gerade darum, dass man das was angeklemmt wird einfach erkennt und verarbeiten kann.

Gerade für den GUI Designer den wir gerade bauen brauchen wir alle Informationen über das Brick.

Das Enumerate ist allerdings nicht threadsicher. Wenn man zwei Java Programme öffnet direkt hintereinander, dann läuft das Enumerate zwei mal durch. Sollte ja eigentlich nicht sein.

Das größte Problem ist halt, dass man nur durch Enumerate alle Informationen über das Brick/let bekommt. Aber das sollte eigentlich im gesamten Programm zur Verfügung stehen. Man sollte ja von jeder Stelle im Programm auf die Informationen zugreifen können ohne ein kompliziertes Mapping zu veranstalten. Gerade UID etc. sind ja Basisinformationen die man für Connection etc benötigt

[AuronX]

Was meinst du mit "nicht threadsicher"?

 

Meinst du den Fakt, dass wenn du zweimal fragst zwei mal geantwortet wird? Das Grundprinzip aller Callbacks in TF ist ja, dass die TF-Hardware nciht den Absender kennt, deswegen wird so ein Callback (wie etwa Enumerate) einfach allen geschickt, die gerade auf Enumerate hören. Das heißt wenn ein Programm läuft das auf enumerate hört und jetzt ein zweites startet und auch enumerieren lässt, dann werden beide Programme die enumerate-nachrichten empfangen. Ist es dieses Phänomen das du meinst?

 

Dann schreibst du noch

Man müsste nur die abstract Class vollständig in die einzelnen Bricklets übernehmen

Wie meinst du das?

 

Sorry für die vielen Fragen, aber ich möchte dich gut verstehen

[M4ST3R]

Sorry für die vielen Fragen, aber ich möchte dich gut verstehen

Kein Problem ist noch früh am morgen und ich sitze in der Trainingssession, deswegen immer nur so Sachen die uns hier direkt auffallen.

Es gibt ja die Abstract Class Device wenn ich das richtig gesehen habe. Dort ist UID etc. vorhanden. Nur das wird nicht an die Erbenden Bricks/Bricklets übergeben.

 

Ja das mit Enumerate erklärt das natürlich. Allerdings sollte man sich da irgendwas überlegen (fällt mir spontan nichts ein) um das zu ändern. Weil so teilweise die Bricks/lets doppelt angelegt werden

[AuronX]

Es gibt ja die Abstract Class Device wenn ich das richtig gesehen habe. Dort ist UID etc. vorhanden. Nur das wird nicht an die Erbenden Bricks/Bricklets übergeben.

 

Ah, jetzt sind die Tomaten auf meinen Augen weg ^^

Diese Felder sind alle nur package-visible, das ist auch okay so.

Aber es sollte definitiv public getter dafür geben (so ist ja zumindst der Java-Stil ^^)

 

Wenn TF nicht schneller ist mache ich dazu später noch nen Pull Request.

 

Ja das mit Enumerate erklärt das natürlich. Allerdings sollte man sich da irgendwas überlegen (fällt mir spontan nichts ein) um das zu ändern. Weil so teilweise die Bricks/lets doppelt angelegt werden

Ich denke sobald die UID leichter zugänglich ist sollte es zumindest leichter möglich sein, diese Dopplung einfach selbst zu vermeiden.

[M4ST3R]

Diese Felder sind alle nur package-visible, das ist auch okay so.

Aber es sollte definitiv public getter dafür geben (so ist ja zumindst der Java-Stil ^^)

 

Wenn TF nicht schneller ist mache ich dazu später noch nen Pull Request.

 

Ja so eilig ist es nicht. Das Programmiertraining ist jetzt vorbei. Werde demnächst ein paar Bilder der "Software" hier zeigen oder verlinken. Ist leider aus Zeit und zum Teil auch aus gründen wie diesen nicht soo viel raus gekommen wie ich erwartet hatte aber ist ein guter Anfang!

 

Ich denke sobald die UID leichter zugänglich ist sollte es zumindest leichter möglich sein, diese Dopplung einfach selbst zu vermeiden.

 

Ja das schon aber das doppelte Enumerate bzw. n-fache ist ansonsten auch nicht sinnvoll.. muss man sich mal ansehen!