Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SD_UT_ added new model for Tedsen | ID 94 for user #550

Merged
merged 7 commits into from
Mar 29, 2019

Conversation

HomeAutoUser
Copy link
Contributor

@HomeAutoUser HomeAutoUser commented Mar 18, 2019

  • Please check if the PR fulfills these requirements
  • Tests for the changes have been added / modified (needed for for bug fixes / features)
  • commandref has been added / updated (needed for bug fixes / features)
  • CHANGED has been updated (needed for bug fixes / features)
  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
  1. revised code for Tedsen
  2. user with many buttons on remote answered on forum https://forum.fhem.de/index.php/topic,39153.0.html
  3. two protocols from one production, revised so we have only one (multiple names for a product)
  4. added Atech wireless weather station (ID94) Atech wireless weather station (vermutlicher Name: WS-308) #547
  • What is the current behavior? (You can also link to an open issue here)
  1. no model
  2. no protocol

- revised code for Tedsen
- user with many buttons on remote answered on forum https://forum.fhem.de/index.php/topic,39153.0.html
- two protocols from one production, revised so we have only one (multiple names for a product)
- added Atech wireless weather station
- remove ID from file
@sidey79
Copy link
Contributor

sidey79 commented Mar 18, 2019

Wieso kommt es denn jetzt zu Änderungen an Protokoll 46?

@HomeAutoUser
Copy link
Contributor Author

Es kommt zur Änderungen des Protokolls, weil eine andere Fernbedienung welche bei ID 46 trifft, auch bei ID78 zutrifft. Die Neuen Modelle welche erarbeitet bzw. zugearbeitet wurden sind 1 Definition mit angepassten "Mittelwert". Wir benötigen nicht 2 Gleiche Definitionen welche stets parallel zutreffen aber das gleiche ist.

@sidey79
Copy link
Contributor

sidey79 commented Mar 18, 2019

@HomeAutoUser

Ich kann noch nicht ganz einschätzen, ob das vorhandene Implementierungen beeinträchtigt.
Leider steht dazu auch nichts im Pullrequest, was ich aber sehe ist, dass Du die Daten für den Test angepasst hast.
Sowohl die rawmsg als auch das erwartete Ergebnis.

Mir wäre erst mal wichtig zu wissen, ob die ursprüungliche raw Nachricht noch zum gleichen Ergebnis wie vorher kommt.

@HomeAutoUser
Copy link
Contributor Author

@sidey79

ich habe nochmal die Daten hervorgekramt und zusammengestellt.
Nachdem das ganze Protokoll @elektron-bbs analysierte mit Usern im Forum welche das ganze auch verifizierten, so nun dein Wunsch

ID 46 org - alte Def
MU;P0=-15829;P1=-3580;P2=1962;P3=-330;P4=245;P5=-2051;D=1234523232345234523232323234523234540023452323234523452323232323452323454023452323234523452323232323452323454023452323234523452323232323452323454023452323234523452323232323452323454023452323234523452323;CP=2;

2019.03.18 22:40:56 4: sduino_dummy: Fingerprint for MU Protocol id 46 -> Berner Garagedoor GA401 matches, trying to demodulate
2019.03.18 22:40:56 4: sduino_dummy: decoded matched MU Protocol id 46 dmsg P46#45048 length 20 dispatch(1/4)
2019.03.18 22:40:56 4: sduino_dummy: SD_UT protocol 46, bitData 01000101000001001000, hlen 5

2019.03.18 22:40:56 4: geiger message converted to tristate code: 0F1FF11F10
2019.03.18 22:40:56 4: sduino_dummy: decoded matched MU Protocol id 78 dmsg u78#2EBEC length 18 dispatch(2/4)

------------------------------------------------
ID 46 org - neue Def

2019.03.18 22:43:14 4: sduino_dummy: Fingerprint for MU Protocol id 46 -> SKXxxx, GF0x0x matches, trying to demodulate
2019.03.18 22:43:14 4: sduino_dummy: last part pair=4 reconstructed, bit=0
2019.03.18 22:43:14 4: sduino_dummy: decoded matched MU Protocol id 46 dmsg P46#BAFB0 length 20 dispatch(1/4)
2019.03.18 22:43:14 4: sduino_dummy: SD_UT protocol 46, bitData 10111010111110110000, hlen 5
2019.03.18 22:43:14 1: sduino_dummy: SD_UT_Parse UNDEFINED sensor unknown detected, protocol 46, data BAFB0, code F1FF11F
2019.03.18 22:43:14 4: sduino_dummy: last part pair=4 reconstructed, bit=0

Als Anmerkung muss man auch sagen, sämtliche Modelle des Protokolls "identisch" sind von der Hardware und nur von anderen Vertrieben verkauft werden. Wir haben auch Herstellerdoku´s und Kataloge danch gegoogelt.

@sidey79
Copy link
Contributor

sidey79 commented Mar 18, 2019

Annahme @HomeAutoUser

Wenn vor de Anpassung P46#45048 und nach der Anpassung P46#BAFB0 an das SD_UT Modul übergeben wird. Funktionieren dann die vorhandenen Definitionen noch oder muss der Anwender diese anpassen?

@HomeAutoUser
Copy link
Contributor Author

HomeAutoUser commented Mar 18, 2019

Das Model was beim alten ausgewertet wurde, wurde umbenannt uns somit wird beim User jetzt ein eues Gerät angelegt.

Erklärung:

  • damals nannte sich das Model SKX1MD

Nach Recherchen und diversen Sichtungen der Kataloge, stellte sich heraus, SKX1... steht für 1 Kanal bzw. Taste auf der Remote. Das hat auch der User bestätigt. Das MD sind interne Bezeichnungen der Hersteller für verschiedene Formen. Das Variiert zu DD - MD oder ND. Alle Mehrkanal Remotes werden dann sortiert nach
SKX2xx, 2 Tasten (GEIGER_GF0x01) - Modulmodel: Tedsen_SKX2xx
SKX4xx, 4 Tasten (GEIGER_GF0x02) - Modulmodel: Tedsen_SKX4xx
SKX6xx, 6 Tasten (GEIGER_GF0x03) - Modulmodel: Tedsen_SKX6xx

Da alle nach dem selben Prinzip von 9 Dilschalter arbeiten, so wurde zusätzlich das "alte" Model in der Verarbeitung angepasst an die Tasten Modelle 2-6.

Hinzu kommt nun das passendesnste. Die MU Nachricht ist von #91 hier genommen wurden. Die damalige Firmware ist nun 2 Jahre alt und daher wird auch die Änderung der Bits sein. (Thematik, alte Nachrichten und nun neue Firmware).

Da Der User (1Kanal) sich bis heute nicht zu Wort gemeldet hat, so haben wir es nach 85% oder mehr nach angaben des Herstellers und Test mit der Mehrkanal remote angepasst.

@sidey79
Copy link
Contributor

sidey79 commented Mar 19, 2019

@HomeAutoUser

Ich weiss leider immer noch nicht so recht, woran wir nun sind. Aus welchem Grund hast Du denn die RMSG im Test angepasst?

Was muss der Anwender machen, wenn er so eine Definition bereits in Verwerndung hatte?
Passiert das Anpassen der Definition automatisch?

@HomeAutoUser
Copy link
Contributor Author

@sidey79

  1. Die Anpassung der RMSG im Test habe ich gemacht weil ich gesehen hatte, das was fehl schlug und ich der Annahme war, dort muss nun die jetzige DMSG rein.

  2. Die Anpassung erfolgt nicht automatisch. Wir haben damals eine alte RAWMSG genutzt und der Einbau erfolgte auch nur, weil der PR offen umher lag. Eine Rückmeldung von der Benutzung haben wir nie erhalten. Ich bin der Meinung, wenn wir solche Anpassungen nun vornehmen mit Usern im Forum auf den jetzigen Stand, so wird sich schon jemand melden, wenn etwas nicht geht. Wie will ich sonst solche Anpassungen vornehmen, wenn ich sehe im laufe der zeit, das die Modellbezeichnung total verkehrt war weil der user nur sein Model hatte.

@HomeAutoUser HomeAutoUser requested a review from sidey79 March 21, 2019 21:22
@sidey79
Copy link
Contributor

sidey79 commented Mar 21, 2019

Ich verstehe das so, wir haben es eingebaut aber wissen noch ob das was wir gemacht haben klappt.
Jetzt wissen wir aber mehr und daher haben wir (Du und Udo) es angepasst.

Wegen dem Test schau ich mal schnell :)

Copy link
Contributor

@sidey79 sidey79 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ich habe mir die Daten und auch den Test angesehen. Die RMSG habe ich wieder auf die Ursprüngliche zurück geändert. Ich weiss jetzt woran es liegt.

01230121212301230121212121230121230
351230121212301230121212121230121230
351230121212301230121212121230121230
351230121212301230121212121230121230
3512301212123012301212121212301212303
51230

Der Sync ist die Sequenz 35. Die 4 Wiederholungen sind schön zu sehen.

2019.03.21 23:26:44 5: part is 12301212123012301212121212301212303 starts at position 0 and ends at 37
2019.03.21 23:26:44 5: dummyDuino: Starting demodulation (StartStr: 35 first found at 35 regex: (?:35)((?:12|30){14,}(?:3|1)?) Pos 35) length_min_max (14..18) length=18

Die Zusammengebaute Regex enthält den recionstructlastBit Teil: (?:3|1)?)
Die 3 passt aber auch zu unserem start. Aber dass es zum start und nicht zu zero gehört, das kann reconstruct lastbit nicht feststellen.

2019.03.21 23:26:44 4: dummyDuino: last part pair=3 reconstructed, bit=0

Da die Sequenz mit 35 beginnt, endet sie sozusagen auch mit 3 am Ende.

2019.03.21 23:26:44 1: DEBUG>dummyDuino: demodulated message raw (1 0 1 1 1 0 1 0 1 1 1 1 1 0 1 1 0 0), 18 bits


Meiner Meinung nach, habe wir hier die Startsequenz als Teil von zero interpretiert und es halt rekonstruiert.

Wir suchen dann nach dem Nächsten Vorkommen, aber leider haben wir die 3 schon verarbeitet und berücksichtigen nun nicht doppelt.
`2019.03.21 23:26:44 5: Starting notify loop for dummyDuino, 1 event(s), first is dummyDuino 5: part is 12301212123012301212121212301212303 starts at position 72 and ends at 109
`

@HomeAutoUser HomeAutoUser requested a review from sidey79 March 25, 2019 16:14
@HomeAutoUser
Copy link
Contributor Author

Jo, du hast es richtig durchleuchtet.

Ich verstehe das so, wir haben es eingebaut aber wissen noch ob das was wir gemacht haben klappt.
Jetzt wissen wir aber mehr und daher haben wir (Du und Udo) es angepasst.

Richtig, es ist eine offene Sache gewesen welche damals lange ohne Verarbeitung hier in Git lag. Dann hatten wir uns der gewidemt um sie einzuarbeiten und via der aktueleln Firmware einzuarbeiten nachdem wir diese mit senden und co durchspielten.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 1378

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at ?%

Totals Coverage Status
Change from base Build 1376: 0.0%
Covered Lines:
Relevant Lines: 0

💛 - Coveralls

@coveralls
Copy link

coveralls commented Mar 25, 2019

Pull Request Test Coverage Report for Build 1398

  • 0 of 34 (0.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-5.3%) to 3.571%

Changes Missing Coverage Covered Lines Changed/Added Lines %
FHEM/14_SD_UT.pm 0 34 0.0%
Totals Coverage Status
Change from base Build 1376: -5.3%
Covered Lines: 25
Relevant Lines: 700

💛 - Coveralls

46:  reconstructLastBit deaktiviert, da es sonst zu einer  Fehlerkennung der Startsequenz kommt.
Änderung der Testdaten rückgängig gemacht
@HomeAutoUser
Copy link
Contributor Author

Wie verfahren wir hier weiter?

@sidey79
Copy link
Contributor

sidey79 commented Mar 27, 2019

@HomeAutoUser
Schau dir bitte meinen Commit an. Ich habe das reconstruct lastbit deaktiviert, da es hier zu fehlern führt.

@HomeAutoUser
Copy link
Contributor Author

Angesehen habe ich es mir und das noch ein Review offen ist.
@sidey79 was requested for review
Du hast es ja direkt schon geändert.

@Ralf9
Copy link
Contributor

Ralf9 commented Mar 27, 2019

Das reconstructlastBit macht ja hier bei dieser ID auch keinen Sinn

@sidey79
Copy link
Contributor

sidey79 commented Mar 27, 2019

@HomeAutoUser

Ich habe es angepasst, da ich seit dem 21. März keine Reaktion mehr erhalten hatte.
Da ich wissen wollte, ob das Problem nur an reconstruct liegt habe ich es verifiziert.

…D_UT.txt

- added sub in 14_SD_UT.pm
- SD_ProtocolData.pm, id 46 set reconstructBit and revised start
- SIGNALduino_TOOL_Dispatch_SD_UT.txt, revised message with last bit message
@HomeAutoUser
Copy link
Contributor Author

@sidey79 @Ralf9
das Ganze hier wurde heute nochmal beleuchtet mit @elektron-bbs zusammen.

Die Option reconstruct ist notwendig, da sonst eine Taste nicht erkannt wird. (Button 6)
Aus diesem Grund habe ich einen neuen commit hinterhergeschoben. Wenn nun dein Test fehl schlägt @sidey79, dann sollten wir den Grund herausfinden wieso dies so ist. Fakt ist, ohne die Option reconstruct wird eine Taste nicht erkannt.

@elektron-bbs wird sich dazu betimmt selbst nochmal melden.
Ich verdrücke mich nun mal die Tage und werde Euch nur über mobil verfolgen.

@elektron-bbs
Copy link
Contributor

Sorry, war mein Fehler. Mit der Definition für Start
start => [-55],
funktioniert reconstructBit auch ordnungsgemäß.

@sidey79

Die Zusammengebaute Regex enthält den recionstructlastBit Teil: (?:3|1)?)
Die 3 passt aber auch zu unserem start. Aber dass es zum start und nicht zu zero gehört, das kann reconstruct lastbit nicht feststellen.

Wieso sollte bei der Rekonstruktion eines BIT die Startsequenz mit einbezogen werden? Da reicht es doch auf one und zero zu prüfen.

clockabs => 290,
# reconstructBit => '1', TODO: Festlegen ob reconstruct ben�tigt wird und wie mit den Startsequenzen zuk�nftig gearbeitet werden soll
reconstructBit => '1', # TODO: Festlegen ob reconstruct ben�tigt wird und wie mit den Startsequenzen zuk�nftig gearbeitet werden soll
format => 'tristate', # not used now
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wie kommt Ihr auf Format tristate?
one und zero sind nur zwei state.
one, zero und float wäre tristate

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Um ein einheitliches Format zur Übergabe an das Modul beizubehalten, wird erst im Modul in Tristate umgewandelt. Definiert werden die Geräte dann z.B. so:
Tedsen_SKX6xx_1F10110

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tristate kann dann ja an dieser Stelle entfallen, wenn es im SIGNALduino Modul nicht umgewandelt wird, was meines Erachtens auch nicht zielführend wäre.

@@ -88,5 +88,8 @@ RH787T,1_fan_minimum_speed,MU;P0=388;P1=-112;P2=267;P3=-378;P5=585;P6=-693;P7=-1
SA_434_1_mini,receive,MU;P0=-1756;P1=112;P2=-11752;P3=496;P4=-495;P5=998;P6=-988;P7=-17183;D=0123454545634545456345634563734545456345454563456345637345454563454545634563456373454545634545456345634563734545456345454563456345637345454563454545634563456373454545634545456345634563734545456345454563456345637345454563454545634563456373454545634545456;CP=3;R=0;
SF01_01319004,minus,MU;P0=-32001;P1=326;P2=-721;P3=-385;P4=656;P5=-15267;D=01213421343434342134213421343421342134512134213434343421342134213434213421345121342134343434213421342134342134213451213421343434342134213421343421342134512134213434343421342134213434213421345121342134343434213421342134342134213451213421343434342134213421;CP=1;R=10;O;
SF01_01319004_Typ2,plus,MU;P0=-15222;P1=379;P2=-329;P3=712;P6=-661;D=30123236123236161232323616161232361232301232361232361612323236161612323612323012323612323616123232361616123236123230123236123236161232323616161232361232301232361232361612323236161612323612323012323612323616123232361616123236123230123236123236161232323616;CP=1;O;
TEDSEN_SKX1MD,receive,MU;P0=-15829;P1=-3580;P2=1962;P3=-330;P4=245;P5=-2051;D=1234523232345234523232323234523234540023452323234523452323232323452323454023452323234523452323232323452323454023452323234523452323232323452323454023452323234523452323232323452323454023452323234523452323;CP=2;
P46_TEDSEN_SKX1xx,Button:1,MU;P0=-15829;P1=-3580;P2=1962;P3=-330;P4=245;P5=-2051;D=1234523232345234523232323234523234540023452323234523452323232323452323454023452323234523452323232323452323454023452323234523452323232323452323454023452323234523452323232323452323454023452323234523452323;CP=2;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wo habt Ihr diese fehlerhafte MU-Nachricht her?
Es sieht so aus als wäre sie von einer älteren fehlerhaften firmware.
D=1234523232345234523232323234523234540023452323
00 kann nicht sein

Copy link
Contributor

@elektron-bbs elektron-bbs Mar 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sooo lange ist das noch nicht her mit diesem Fehler...
Die Nachrichten stammen alle aus dem Forum: https://forum.fhem.de/index.php/topic,39153.0.html und aus diesem Issue: #91

@Ralf9
Copy link
Contributor

Ralf9 commented Mar 28, 2019

Die Option reconstruct ist notwendig, da sonst eine Taste nicht erkannt wird. (Button 6)

Ok, mit start => [-55], passt es und mit der rawmsg von Button 6 macht auch das recionstructlastBit sinn

@sidey79
Copy link
Contributor

sidey79 commented Mar 28, 2019

Die Zusammengebaute Regex enthält den recionstructlastBit Teil: (?:3|1)?)
Die 3 passt aber auch zu unserem start. Aber dass es zum start und nicht zu zero gehört, das kann reconstruct lastbit nicht feststellen.

Wieso sollte bei der Rekonstruktion eines BIT die Startsequenz mit einbezogen werden? Da reicht es doch auf one und zero zu prüfen.

Es wird auf one und zero geprüft. Die Startsequenz hat aber auch zu 1/2 zero gepasst

@HomeAutoUser
Copy link
Contributor Author

@sidey79 muss ich hier noch ein request beantworten? Habe nichts gefunden, das ich reagieren könnte oder ist es bei dir nur noch als "offen" markiert?

@sidey79
Copy link
Contributor

sidey79 commented Mar 29, 2019

Ich habe den Eindruck, dass die Protokolldefinition angepasst werden muss.
Wenn ich etwas falsch verstanden habe, dann korrigiert mich bitte.

@HomeAutoUser
Copy link
Contributor Author

Die Definition wurde im letzten Commit auf die 55 genommen. Somit sollte alles angepasst wurden sein. @elektron-bbs kann dies gern gehen prüfen nochmal aber alles sollte somit erfasst wurden sein.

@elektron-bbs
Copy link
Contributor

Meines Erachtens sollte jetzt alles passen.

@HomeAutoUser HomeAutoUser merged commit 0f6fc8c into RFD-FHEM:dev-r34 Mar 29, 2019
@HomeAutoUser HomeAutoUser deleted the dev-r34_SD_UT_added branch April 8, 2019 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants