Command Class umbenannt (ALARM -> NOTIFICATION), wie in FHEM anpassen?

Begonnen von A.Harrenberg, 15 Mai 2016, 09:48:47

Vorheriges Thema - Nächstes Thema

rudolfkoenig

Hallo Andreas,

Zitat@Rudi: Wenn das so passt werde ich die Doku entsprechend anpassen und Dir dann demnächst mal ein Patch schicken.

Ich bitte dich:
- alarm_XX Readignamen aendern: es darf kein () vorkommen
- ZWave_CC_0x71_01_Get & co in ZWave_Alarm_01_Get usw. umzubennen
- auf TAB zu verzichten (da keiner mehr weiss, wie breit ein TAB ist, stiftet es nur Verwirrung).
- Zeilen auf 80 Zeichen zu beschraenken
- nicht benutzte Funktionen / auskommentierten Code entfernen

Sonst (bitte ignorieren, falls ich mich irre):
-  "for (my $i=0; $i<$numBitMasks; $i++)" klingt falsch, ich wuerde <= erwarten

A.Harrenberg

Hallo Rudi,
Zitat von: rudolfkoenig am 08 Juli 2016, 09:22:43
- alarm_XX Readignamen aendern: es darf kein () vorkommen
ok, war mir nicht bewusst.

Zitat von: rudolfkoenig am 08 Juli 2016, 09:22:43
- auf TAB zu verzichten (da keiner mehr weiss, wie breit ein TAB ist, stiftet es nur Verwirrung).
Hmm, dachte ich hätte meinen Editor so eingestellt das er TAB als zwei Spaces behandelt. Werde die TAB entfernen.

Zitat von: rudolfkoenig am 08 Juli 2016, 09:22:43
- Zeilen auf 80 Zeichen zu beschraenken
- nicht benutzte Funktionen / auskommentierten Code entfernen
Ja, aufhübschen werde ich das noch... ,-)

Zitat von: rudolfkoenig am 08 Juli 2016, 09:22:43
Sonst (bitte ignorieren, falls ich mich irre):
-  "for (my $i=0; $i<$numBitMasks; $i++)" klingt falsch, ich wuerde <= erwarten
Das sollte so passen, da die Schleife bei 0 anfängt und die Zählweise von "numBitMasks" bei 1 anfängt. Musste aber auch gerade überlegen warum ich das so gemacht hatte....

Gruß,
Andreas.
FB 7360, Homematic und ZWave
Support for ZWave-SECURITY

krikan

Hallo Andreas!
Nur eine kurze Zwischenmeldung: Teste derzeit zwischendurch immer wieder Deine oben angehängte Modulversion. Habe nicht wirklich etwas festgestellt; werde aber noch weiter probieren, da ich die Class noch nicht genau durchblicke. Das sollte aber ggfs. nicht an Änderungen/Einchecken hindern.
Gruß, Christian.

A.Harrenberg

Hallo Christian,
Zitat von: krikan am 12 Juli 2016, 20:37:53
Nur eine kurze Zwischenmeldung: Teste derzeit zwischendurch immer wieder Deine oben angehängte Modulversion. Habe nicht wirklich etwas festgestellt; werde aber noch weiter probieren, da ich die Class noch nicht genau durchblicke. Das sollte aber ggfs. nicht an Änderungen/Einchecken hindern.
danke für die RM. Ich habe jetzt ja nur noch die "()" in den Readingnamen entfernt, ansonsten denke ich aber nur internas geändert und funktional nicht nicht mehr wirklich was geändert. Die Dokumentation habe ich auch schon mal soweit fertig, allerdings werde ich es heute und morgen wohl nicht schaffen das als Patch fertig zu machen. Ich peile mal Do. Abend an...

@Rudi: Ich habe ja die auch Texte der Events angepasst (Kommas entfernt), falls jetzt jemand auf den vollständigen String inklusive des Kommas geparst hat, wird das nach dem Update fehlschlagen. Hälst Du das für ein häufiges/wahrscheinliches Szenario? Sollte das dann evtl. angekündigt werden?

Gruß,
Andreas.
FB 7360, Homematic und ZWave
Support for ZWave-SECURITY

rudolfkoenig

Bin unentschlossen. Falls keine andere Meinungen kommen, dann wuerde ich es ankuendigen, zuletzt habe ich einen Rueffel wg. einem unangekuendigten Bugfix mit unerwarteten Nebeneffekten bekommen :)

A.Harrenberg

Hi Rudi,
Zitat von: rudolfkoenig am 13 Juli 2016, 11:50:30
Bin unentschlossen. Falls keine andere Meinungen kommen, dann wuerde ich es ankuendigen, zuletzt habe ich einen Rueffel wg. einem unangekuendigten Bugfix mit unerwarteten Nebeneffekten bekommen :)
den "Rüffel" habe ich gelesen, daher habe ich das hier vorgeschlagen ,-)
Ich denke eine Ankündigung schadet ja nicht, ich denke das Du damit aber noch warten solltest bis ich Dir den Patch schicken kann. Das Einpflegen des Patches kann man dann ja entsprechend einen Tag nach der Ankündigung machen.

Gruß,
Andreas.
FB 7360, Homematic und ZWave
Support for ZWave-SECURITY

A.Harrenberg

Hi Rudi, hallo Christian

angehängt der Patch für ALARM/NOTIFICATION.

@Christian: Ich habe auch eine aktuelle 10_ZWave.pm Version angehängt, wenn Du die vielleicht mal kurz antesten könntest...

@Rudi: Bitte schaue noch mal in den Patch rein und gib mir eine Rückmeldung wenn ich noch was anpassen soll. Im Doku-Teil war mir aufgefallen das dort teilweise "<name>" verwendet wurde, was dann ja als HTML-Tag ausgefiltert wird. Ich hoffe ich habe alle Stellen gefixt. Falls es so in Ordnung ist kannst Du es dann ja ankündigen und danach veröffentlichen.

Gruß,
Andreas.
FB 7360, Homematic und ZWave
Support for ZWave-SECURITY

rudolfkoenig

@Andreas: habe in der Doku ein paar Tippfehler verbessert, und nur den ersten Exemplar des "ALARM was renamend to NOTIFICATION" Paragraphs behalten. Sonst ohne Aenderung eingecheckt. Ich habe die Aenderung in CHANGED erwaehnt. Wer das da nicht liest, wird auch die Ankuendigungen nicht lesen.