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

unittest to verify set command sendMsg a little bit #539

Merged
merged 5 commits into from
Mar 8, 2019

Conversation

sidey79
Copy link
Contributor

@sidey79 sidey79 commented Mar 5, 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, ...)

It will test if the set command sendMsg will generate the desired command

  • What is the current behavior? (You can also link to an open issue here)

There is currently no test to verify this section of our code

  • What is the new behavior (if this is a feature change)?

some conditons of our code a verified

Test will verify via following protocol IDs:
0
17
43
7

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)

  • Other information:

simple sendMsg Test with protocols 0 17 43 72
@sidey79 sidey79 changed the base branch from master to dev-r34 March 5, 2019 22:38
@sidey79 sidey79 changed the title Dev r34 send msg unittest unittest to verify set command sendMsg a little bit Mar 5, 2019
@sidey79 sidey79 marked this pull request as ready for review March 5, 2019 22:42
@coveralls
Copy link

coveralls commented Mar 5, 2019

Pull Request Test Coverage Report for Build 1335

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 177 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+2.9%) to 13.194%

Files with Coverage Reduction New Missed Lines %
FHEM/lib/SD_ProtocolData.pm 1 66.67%
FHEM/14_SD_WS07.pm 19 67.86%
FHEM/14_SD_WS.pm 157 42.95%
Totals Coverage Status
Change from base Build 1308: 2.9%
Covered Lines: 803
Relevant Lines: 6086

💛 - Coveralls

added protocol 29 with hexdata
@sidey79
Copy link
Contributor Author

sidey79 commented Mar 6, 2019

@elektron-bbs
@HomeAutoUser

Mir fehlt vom Prinzip nur noch ein sendMsg Test der die Frequenz verstellen möchte. Ansonsten sieht die Testabdeckung sehr gut für diesen kleinen Teil aus.

@HomeAutoUser
Copy link
Contributor

@sidey79

Mir fehlt vom Prinzip nur noch ein sendMsg Test der die Frequenz verstellen möchte.

Was meinst du direkt damit? Möchtest du die Syntax testen oder inwieweit ist das "Frequenz verstellen" zu verstehen mit sendMsg?

@sidey79
Copy link
Contributor Author

sidey79 commented Mar 6, 2019

MitCode coverage wird grafisch angezeigt, welche Teile vom quellcode mit den Tests verifiziert werden.

rot = wird nicht verifiziert.

image

In der Grafik ist zu sehen, dass die Frequenz hier nicht verifiziert wird.
Wenn Du zufällig eine Sendenachricht parat hast, bei der die Frequenz verstellt wird, würde ich das noch aufnehmen. Ich habe gestern auf Anhieb keine griffbereit gehabt.

Ansonsten wird alles andere mit dem Test verifiziert,

@HomeAutoUser
Copy link
Contributor

Ich verstehe das. Das was ich nicht verstehe

Wenn Du zufällig eine Sendenachricht parat hast, bei der die Frequenz verstellt wird, würde ich das noch aufnehmen.

Wie kann ich mit einer Nachricht welche ich sende, die Frequenz verstellen? grübel
Ich kenne nur den Unterschied, eine Nachricht welche auf dem Frequenzband 1 zu senden ist und eine weitere auf Frequenzband 2.

@sidey79
Copy link
Contributor Author

sidey79 commented Mar 6, 2019

Ich verstehe das. Das was ich nicht verstehe

Wenn Du zufällig eine Sendenachricht parat hast, bei der die Frequenz verstellt wird, würde ich das noch aufnehmen.

Wie kann ich mit einer Nachricht welche ich sende, die Frequenz verstellen? grübel
Ich kenne nur den Unterschied, eine Nachricht welche auf dem Frequenzband 1 zu senden ist und eine weitere auf Frequenzband 2.

Es ist möglich dem sendMsg Befehl ein #F mitzugeben.
Leider haben wir das wohl nicht dokumentiert :(

@HomeAutoUser
Copy link
Contributor

Es ist möglich dem sendMsg Befehl ein #F mitzugeben.
Leider haben wir das wohl nicht dokumentiert :(

Das ist mir NEU und ja, das sollten wir mal fix dokumentieren.

sidey79 added 3 commits March 8, 2019 22:04
Added frequency to testprotocol 43
Added second test with protocol 43
Test with cc1101 capability to cover some more lines of code
@sidey79
Copy link
Contributor Author

sidey79 commented Mar 8, 2019

@HomeAutoUser
Also nach F scheinen die Register für den cc1101 zu kommen. Das was im sendMsg Kommando angegeben wird, das wird direkt an den uC übergeben.
Ehrlich gesagt, weiss ich jetzt nicht wie man das verständlich dokumentieren soll.

Wir müssten an dieser Schnittstelle eigentlich eine Eingabe wie 434.00Mhz einbauen die es dann in Register umrechnet.

Der Test sendMsg ist fertig. an diesem PR würde ich nichts mehr machen wollen

@HomeAutoUser
Copy link
Contributor

Wir müssten an dieser Schnittstelle eigentlich eine Eingabe wie 434.00Mhz einbauen die es dann in Register umrechnet.

Können wir nicht die Frequenz als Zahlenwert auswerten und dann umwandeln in der internen Verarbeitung.

BsP: #F433.000 wird zu #F0x23 ... (also die Registerwerte) oder stelle ich mir das falsch vor?

@sidey79
Copy link
Contributor Author

sidey79 commented Mar 8, 2019

Wir müssten an dieser Schnittstelle eigentlich eine Eingabe wie 434.00Mhz einbauen die es dann in Register umrechnet.

Können wir nicht die Frequenz als Zahlenwert auswerten und dann umwandeln in der internen Verarbeitung.

BsP: #F433.000 wird zu #F0x23 ... (also die Registerwerte) oder stelle ich mir das falsch vor?

Das wäre eine tolle Erweiterung, aber stand heute ist das ja nicht implementiert

@sidey79
Copy link
Contributor Author

sidey79 commented Mar 8, 2019

Wir müssten an dieser Schnittstelle eigentlich eine Eingabe wie 434.00Mhz einbauen die es dann in Register umrechnet.

Können wir nicht die Frequenz als Zahlenwert auswerten und dann umwandeln in der internen Verarbeitung.

BsP: #F433.000 wird zu #F0x23 ... (also die Registerwerte) oder stelle ich mir das falsch vor?

Das wäre eine tolle Erweiterung, aber stand heute ist das ja nicht implementiert
Ich ergänze. Wir haben den Befehl freq der macht schon genau so eine Umwandlung.

@HomeAutoUser
Copy link
Contributor

Ich ergänze. Wir haben den Befehl freq der macht schon genau so eine Umwandlung.

Dann nutzen wir das gleich. So werden interne Dinge genutzt die es eh schon umsetzen.
Ich kenne den Umfang nicht aber stelle mit es eigendlich einfach vor.

Leihenhaft würde ich sagen, aus "Variable 1" wird der "Teil abc" durch die "Funktion Wandel" in "Variable 2" geschrieben.

@sidey79
Copy link
Contributor Author

sidey79 commented Mar 8, 2019

ja, wir können die 3-4 Zeilen in eine sub stecken und die dann einfach an den benötigten Stellen aufrufen... aber nicht in diesem PR :)

@sidey79 sidey79 requested review from HomeAutoUser and Ralf9 March 8, 2019 22:47
Copy link
Contributor

@HomeAutoUser HomeAutoUser left a comment

Choose a reason for hiding this comment

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

BsP: #F433.000 wird zu #F0x23 ... (also die Registerwerte) oder stelle ich mir das falsch vor?

Das wäre eine tolle Erweiterung, aber stand heute ist das ja nicht implementiert

--> neuer PR

@sidey79 sidey79 merged commit 376739f into dev-r34 Mar 8, 2019
@sidey79 sidey79 deleted the dev-r34_sendMsg-Unittest branch March 8, 2019 23:07
@HomeAutoUser
Copy link
Contributor

@sidey79
ich habe für dich eine frequency Angabe gefunden.

frequency => '10AB85550A',

suchtest du soetwas?

@sidey79
Copy link
Contributor Author

sidey79 commented Mar 15, 2019

Danke, Ich hatte in dem Test ja eine Angabe eingebaut, ich glaube es war das Somfy Protokoll :)

@HomeAutoUser
Copy link
Contributor

Richtig :-)
Wenn du mir nun noch sagen kannst wie der Syntax hinter F erfolgen muss, so können wir das

<ul><li>P<protocol id>#0xhexdata#R<anzahl der wiederholungen>#C<optional taktrate>#F<optional Frequenz>    (#C #F is optional) 
<br>Beispiel 0xhexdata: <code>set sduino sendMsg P29#0xF7E#R4#F</code>
<br>Wird eine sende Kommando fuer die Hexfolge F7E anhand der protocol id 29 erzeugen. Die Nachricht soll 4x gesenset werden mit einer Frequenz von ... Mhz.
<br>SR;R=4;P0=-8360;P1=220;P2=-440;P3=-220;P4=440;D=01212121213421212121212134;F=........;</ul>

irgendwie in die Dokumentation einbringen :-D

@sidey79
Copy link
Contributor Author

sidey79 commented Mar 15, 2019

Hmm, das wird schwierig. Das sind direkt die Registerwerte die geschrieben werden

@HomeAutoUser
Copy link
Contributor

Sehe bzw. denke ich richtig, das das Register immer komplett geschrieben werden muss.
Also Schlussfolgerung, wenn Bsp: 8 Reg. geschrieben werden müssen, so MUSS man es 8stellig angeben!?

@HomeAutoUser
Copy link
Contributor

Ein Bsp: wäre

<ul><li>P<protocol id>#0xhexdata#R<anzahl der wiederholungen>#C<optional taktrate>#F<optional Frequenz>    (#C #F is optional) 
<br>Beispiel 0xhexdata: <code>set sduino sendMsg P36#0xF7#R6#F(Registerwert des CC1101)</code>
<br>Wird eine sende Kommando fuer die Hexfolge F7 anhand der protocol id 29 erzeugen. Die Nachricht soll 6x gesenset werden mit der angegebenen Frequenz.
<br>SR;R=6;P0=-8360;P1=220;P2=-440;P3=-220;P4=440;D=012323232324232323;F=(Registerwert des CC1101);</ul>

so sollte jeder Wissende bescheid wissen. Die Register kann man ja entnehmen aus dem Datenblatt.

@elektron-bbs
Copy link
Contributor

Mhmm, klar ist die Angabe nicht. Es müssen ja drei Register geschrieben werden:

0x10, // 0D FREQ2 1E Reg Pos 0C
0xB0, // 0E FREQ1 C4 Reg Pos 0D
0x71, // 0F FREQ0 EC Reg Pos 0E

Wie muss das dort angegeben werden?

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.

4 participants