UNSOLVED Wattpilot Adapter

[tim2zg]

Hallo zusammen!
Da wir letztens einen Fronius Watt Pilot bekommen haben und es noch keine Integration dafür gibt, habe ich eine erstellt. Ich weiss das der Adapter eher schlecht programmiert ist (bin erst 15 Jahre alt). Ich hoffe deswegen auf Feedback und vielleicht kann man ihn ja gebrauchen.
Das Ganze läuft auf einer Inoffiziellen Websocket API, die durch den Piloten bereitgestellt werden.
https://github.com/joscha82/wattpilot
GitHub Repo: https://github.com/tim2zg/ioBroker.fronius-wattpilot

Freundlich Grüsse,
Tim

[apollon77]

@tim2zg Hi Tim,

herzlich willkommen im kreise der Adapter-Entwickler, wie sagt man so schön "Früh übt sich"

@Homoran verschiebt den Thread mal richtig und fügt dich zur Entwickler-Gruppe hinzu.

Die Entwickler sind neben hier im Forum auch im Discord und Telegram sehr aktiv (auch mit einer gesyncten Gruppe). Die weiteren Details und Dev-Resourcen die wir so haben findest Du unter https://www.iobroker.dev . Kanntest Du die schon?

Sonst können wir gern mal schauen das sich einer der "alten Hasen" den Adapter anschaut und Reviewt - wenn DU dann den Adapter ins Repo aufnehmen willst kommt nochmal ein Review.

Viel Spass mit ioBroker!

Ingo

[tim2zg]

@apollon77
Vielen Dank,
Ich habe momentan eine stressige Woche, aber versuche so bald wie möglich den Adapter für euch Tester bereit zu machen. Er ist zwar schon betriebsbereit aber der Code sieht schrecklich aus.
Freundliche Grüsse,
Tim

[apollon77]

@tim2zg kein Stress, take your time!

[tim2zg]

@apollon77
Adapter wäre jetzt zum Testen bereit.

Freunldiche Grüsse,
Tim

[foxriver76]

@tim2zg Cool, dass du dich an einen Adapter gewagt hast. Eröffne gerne mal einen Thread im Tester Forum, damit User den Adapter funktional testen.

Da du dir etwas Feedback zum Codestyle etc gewünscht hast:

• In https://github.com/tim2zg/ioBroker.fronius-wattpilot/blob/7c6eeeb5114fc676a2d8b5983d9ad937f80d9eb5/main.js#L851 und folgende Zeilen serialisierst du sehr oft das selbe Objekt, was nicht effizient ist, besser am Anfang einmal in einer Konstante speichern.
• Deine Variablen und Funktionsnamen sind oft schlecht lesbar, da du keinen case Style nutzt und somit mehrere Wörter einfach aneinander gereiht sind. In JavaScript wird camelCase genutzt.
• Weiterhin könnten manche Namen sprechender sein z. B. https://github.com/tim2zg/ioBroker.fronius-wattpilot/blob/7c6eeeb5114fc676a2d8b5983d9ad937f80d9eb5/main.js#L846
• JSDoc gerne auch bei eigenen Funktionen auch nutzen, so weißt du in einem halben Jahr schneller, wofür die Funktionen da waren und welche Typen erwartet werden
• Die ganzen auskommentierten Sachen würde ich raus werfen, dank Versionsverwaltung siehst du ja wie es aussah, falls du es nochmal benötigst.
• Bei unload würde ich info.connection ebenfalls auf false setzen
• Ich bin mir nicht sicher über die Logik hier https://github.com/tim2zg/ioBroker.fronius-wattpilot/blob/7c6eeeb5114fc676a2d8b5983d9ad937f80d9eb5/main.js#L914 du kannst auch Wildcards subscriben, hätte den Vorteil dass du es initial ein mal tust, hier wird potentiell mehrfach subscribed, was zwar keine Auswirkungen hat, aber obsolet ist
• Die news in den io-package json existieren nicht für die neusten Versionen, das release-script kann ich dir nur ans Herz legen, dann brauchst du dich darum nicht mehr manuell kümmern. https://github.com/AlCalzone/release-script + ioBroker plugin nicht vergessen.
• Diese Dependencies werden nicht genutzt wie es aussieht https://github.com/tim2zg/ioBroker.fronius-wattpilot/blob/7c6eeeb5114fc676a2d8b5983d9ad937f80d9eb5/package.json#L28-L29
• Engines Feld in package.json könntest du hinzufügen mit minimal nötiger Nodejs Version
• Das Passwort kannst du automatisch verschlüsseln lassen mit encryptedNative https://github.com/tim2zg/ioBroker.fronius-wattpilot/blob/7c6eeeb5114fc676a2d8b5983d9ad937f80d9eb5/io-package.json#L75 und mit protectedNative vor Zugriff durch andere Adapter schützen lassen, benötigt dann mindestens js-controller 3 als dependency in io-package.json sowie admin >=4.0.10
• Offiziell ist https://github.com/tim2zg/ioBroker.fronius-wattpilot/blob/7c6eeeb5114fc676a2d8b5983d9ad937f80d9eb5/main.js#L331 indicator für read only numbers https://github.com/ioBroker/ioBroker/blob/master/doc/STATE_ROLES.md
• empty else https://github.com/tim2zg/ioBroker.fronius-wattpilot/blob/7c6eeeb5114fc676a2d8b5983d9ad937f80d9eb5/main.js#L996
• https://github.com/tim2zg/ioBroker.fronius-wattpilot/blob/7c6eeeb5114fc676a2d8b5983d9ad937f80d9eb5/main.js#L962 besser returnen wenn state.ack true ist, sonst kannst du durch deine eigenen setStates triggern, falls einer davon abonniert ist.
• Error Handling sollte evtl noch nachgezogen werden

Ansonsten schaut das wesentlich sauberer aus als erwartet nach deiner Aussage

beste Grüße und happy coding

fox

[tim2zg]

@foxriver76
Danke vielmal, ich schaue das ich die Punkte bald verbessern kann!

Freundliche Grüsse,
Tim,