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

signalDecoder: Bugfixing und Verbesserungen #67

Open
Ralf9 opened this issue Oct 19, 2017 · 79 comments
Open

signalDecoder: Bugfixing und Verbesserungen #67

Ralf9 opened this issue Oct 19, 2017 · 79 comments

Comments

@Ralf9
Copy link
Contributor

Ralf9 commented Oct 19, 2017

Ich habe in der doDetect() einige Verbesserungen vorgenommen. Die gleichen patterrnNr in Folge im message Puffer sind jetzt weg.

Damit waren die meisten gleichen patterrnNr in Folge weg.

if (success == false || (messageLen > 0 && (*first ^ *last) > 0)) {

Es blieben aber noch einige gleiche patterrnNr in Folge übrig, die schwer zu finden waren.
Damit wurde der last pulse mit dem aktuellen first überschrieben. Der Fehler in der calcHisto Routine konnte sich da auch ausgewirkt haben

			for (uint8_t i = messageLen - 1; i >= 0 && histo[pattern_pos] > 0; --i)
			{
				if (message[i] == pattern_pos) // Finde den letzten Verweis im Array auf den Index der gleich ueberschrieben wird
				{
					i++; // i um eins erhoehen, damit zukuenftigen Berechnungen darauf aufbauen koennen
					bufferMove(i);
					break;
				}

			}

Ich habe die Routine abgeändert. Nun wird zuerst ein compress_pattern versucht, falls kein compress möglich war, entferne ich Zeichen am Anfang des message Puffers:

					uint8_t idx;
					for (uint8_t i = 0; i<messageLen; i++)
					{
						idx = message[i];
						if (histo[idx] == 1)
						{
							pattern[idx] = 0;
							bufferMove(i+1);
							pattern_pos = idx;
							break;
						}
						else
						{
							histo[idx]--;
						}
					}

Diese Routine gefällt mir noch nicht so richtig:
Damit wird processMessage() auch aufgerufen, wenn die mindest messageLen noch nicht erreicht wurde. Wenn beim Aufruf von processMessage() das "m_truncated = false" ist, dann wird in der processMessage() ein reset() durchgeführt!

		// Add pattern
		if (patternLen == maxNumPattern)
		{
			calcHisto();
			if (histo[pattern_pos] > 2)
			{
				processMessage();
				calcHisto();
@Ralf9 Ralf9 mentioned this issue Oct 19, 2017
@sidey79
Copy link
Contributor

sidey79 commented Oct 20, 2017

Hi Ralf,

ich komme da leider noch nicht ganz mit.
Ich würde auch in der SIGNALDetector Klasse vorerst noch nichts ändern, bis nicht die Fehler in der Bitstore Klasse beseitigt wurden.

Einige Fehler konnte ich schon identifizieren und ausbessern. Einen habe ich aber noch nicht gefunden. Da passen auf einmal die Variable valcount unt bitcount nicht mehr zusammen. Das führt dann auch zu Einträgen mit fehlerhaftem Vorzeichen.

@Ralf9
Copy link
Contributor Author

Ralf9 commented Oct 20, 2017

ich konnte außer dem bcnt Fehler in der moveLeft und der etwas unsauberen Variablen Definitionen keine weiteren Fehler in der Bitstore Klasse finden.
Nach dem korrigieren des bcnt in der moveLeft und änderen einiger Variablen Definitionen, konnte ich in der Bitstore Klasse keine Fehler mehr finden.
Kannst Du ausschliessen, daß die z.T. unsaubere Variablen Definitionen ein Teil deiner Probleme verursacht?

@sidey79
Copy link
Contributor

sidey79 commented Oct 20, 2017

Hi Ralf,

Ich mache es aktuell so:
Wenn ich einen Fehler anhand von Debug Ausgaben finde, dann Stelle ich die Situation in einem Testprogramm nach.

Mit diesem Testprogramm prüfe ich dann auch meine Fehlerbehebung und passe so lange an, bis alle Tests erfolgreich sind.

Dadurch habe ich nun schon einige Anpassungen verifiziert. Die Berechnung von valcount habe ich so angepasst, dass es relativ zum vorherigen Wert berechnet wird und nicht mehr absolut.

Irgendwo hapert es aber noch mit der Berechnung von bitcount . Da habe ich leider noch nicht genug Ausgaben eingebaut, um diesen Fehler reproduzieren zu können.

@sidey79
Copy link
Contributor

sidey79 commented Oct 20, 2017

Uups, ich meinte die Berechnung von bytecount ist fehlerhaft und nicht bitcount

@Ralf9
Copy link
Contributor Author

Ralf9 commented Oct 22, 2017

Ich habe mal meine Verbesserungen und Überarbeitungen von doDetect() und bitstore.h in github commited. Es kann sein, das es noch nicht ganz passt
https://github.com/Ralf9/SIGNALDuino/tree/dev-r332_cc1101

@HomeAutoUser
Copy link
Contributor

Hallo @Ralf9 , deine Version empfängt auf jedenfall im Gegensatz zu @sidey79 seiner aktuellen Version die Hideki Protokolle wie von mir hier schon erwähnt. Im Groben kann ich erstmal sagen, läuft. Langzeit werde ich verfolgen.

@sidey79
Copy link
Contributor

sidey79 commented Oct 22, 2017

Tia, ich weiss jetzt leider nicht , ob mir das hilft.

Ich weiss nicht genau, was der Fehler ist. Ich sehe nur sehr viele Änderungen, die ich so erst Mal nicht nachvollziehen kann.

Wieso damit jetzt Hideki besser sein soll verstehe ich auch nicht.

@Ralf9
Copy link
Contributor Author

Ralf9 commented Oct 26, 2017

Hallo Sidey,

ich habe versucht die doDetect Routine zu optimieren. Bei der jetztigen Struktur tue ich mich dabei etwas schwer.
Mit der folgenden Struktur müsste es einfacher sein. Ich werde es mal so versuchen:

bool valid;
valid = (messageLen == 0 || last == NULL || (*first ^ *last) < 0); // true if a and b have opposite signs
valid &= (*first > -maxPulse); // if low maxPulse detected, start processMessage()

!valid bedeuted, daß ein Signalende erkannt wurde. Ein voller message Puffer ist kein Signalende.
Stark vereinfacht stelle ich mir die Verarbeitung so ungefähr vor:
Wenn !valid dann:

processMessage();
wenn messageLen < minMessageLen dann reset()
wenn nach dem processMessage messageLen > minMessageLen ist, dann gibt es noch Wiederholungen die mit einem weiteren processMessage() ausgegeben werden
last = NULL

Wenn valid dann:

wenn messagePuffer voll, dann processMessage()
wenn der PatternPuffer voll ist, dann compress_pattern()
wenn es danach immer noch kein Platz im PatternPuffer hat, dann werden Einträge vom messagePuffer entfernt

@sidey79
Copy link
Contributor

sidey79 commented Oct 26, 2017

Ja ich denke ich verstehe deinen Ansatz.

Ich hatte das auch mal so ähnlich angedacht, aber wenn der Puffer voll ist und man kein processMessage probiert, dann verliert man alle Wiederholungen.

@Ralf9 Ralf9 changed the title doDetect(): Bugfixing und Verbesserungen signalDecoder: Bugfixing und Verbesserungen Oct 29, 2017
@Ralf9
Copy link
Contributor Author

Ralf9 commented Oct 29, 2017

Ich habe das mit dem !valid = Signalende erkannt, mal bei mir eingebaut.
https://github.com/Ralf9/SIGNALDuino/tree/dev-r332_cc1101

Ich hatte vor die Add pattern Routine vom doDetect() zu optimieren, dies hat aber nicht so funktioniert wie ich es vor hatte. Dies ist doch sehr viel komplexer als ich dachte. Ich verwende nun wieder Deine Add pattern Routine.

Gefühlsmäßig müsste in der Add pattern Routine noch Verbesserungspotential sein.
Z.B. bei den MU-Nachrichten ist mir aufgefallen, daß am Anfang was fehlt, beim decodieren wird dann die Wiederholung verwendet.

Mir ist auch aufgefallen, daß mein hideki Sensor mit dem RXB6 Empfänger sehr viel besser erkannt wird.

Beim RXB6 Empfänger werden in der Regel alle 3 Wiederholungen vollständig erkannt:

MC;LL=-1062;LH=890;SL=-569;SH=408;D=AE0B174A6183EBB1543B280;C=488;L=89;
MC;LL=-1010;LH=938;SL=-527;SH=449;D=A8FA745AEF3E0A2755E37CE;C=487;L=91;
MC;LL=-1005;LH=947;SL=-522;SH=456;D=AE0B174A4183EBB1543A320;C=488;L=90;

MC;LL=-1051;LH=898;SL=-567;SH=407;D=A8FA745ACA3E0A2755B24D6;C=487;L=91;
MC;LL=-1011;LH=941;SL=-523;SH=450;D=AE0B174A2B83EBB154994B0;C=487;L=90;
MC;LL=-1004;LH=947;SL=-517;SH=459;D=AE0B174A4B83EBB1549A7F0;C=487;L=90;

MC;LL=-1052;LH=896;SL=-564;SH=410;D=A8FA745ACF3E0A2755E26BE;C=486;L=91;
MC;LL=-1009;LH=941;SL=-523;SH=454;D=AE0B174A2183EBB15439060;C=487;L=90;
MC;LL=-1007;LH=941;SL=-522;SH=456;D=AE0B174A4183EBB1543A320;C=487;L=90;
Beim cc1101 werden in der Regel nur max 2 Wiederholungen vollständig erkannt:

MC;LL=-1028;LH=929;SL=-548;SH=422;D=AE0B174A6B83EBB1549B650;C=487;L=90;R=61;
MC;LL=-1013;LH=953;SL=-516;SH=458;D=A8FA745AEA3E0A2755B35A6;C=489;L=91;R=61;
MC;LL=-1023;LH=939;SL=-526;SH=443;D=A2D6D1F0513AAD9603;C=488;L=72;R=61;

MC;LL=-1011;LH=942;SL=-531;SH=448;D=ACA3E0A2755B24D6;C=488;L=63;R=67;
MC;LL=-1022;LH=931;SL=-548;SH=434;D=A8FA745AEA3E0A2755B35A6;C=489;L=91;R=67;
MC;LL=-1018;LH=944;SL=-531;SH=446;D=AE0B174A4B83EBB1549A7F0;C=489;L=90;R=67;

MC;LL=-1028;LH=917;SL=-532;SH=452;D=A8FA745ACA3E0A2755B24D6;C=488;L=91;R=71;
MC;LL=-1020;LH=938;SL=-524;SH=444;D=AEA3E0A2755B35A6;C=487;L=63;R=71;
MC;LL=-1011;LH=950;SL=-516;SH=455;D=A8FA745ADA3E0A2755B2C06;C=488;L=91;R=71;

Beim RXB6 Empfänger werden die beiden Wiederholungen vom FS10 Sender zuverlässig erkannt.
Beim cc1101 werden die Tastendrücke vom FS10 Sender nicht zuverlässig erkannt.

@sidey79
Copy link
Contributor

sidey79 commented Oct 29, 2017

Vielleicht liegt es daran, dass beim cc1101 kein rauchen empfangen wird und dadurch der Pin nicht permanent toggelt.

@HomeAutoUser
Copy link
Contributor

HomeAutoUser commented Oct 30, 2017

Hallo,
ich habe soeben mal @Ralf9 Firmware am laufen. Die Widerholung sieht schonmal gut aus. Diese funktioniert auch bei den 868Mhz - FHT Protokollen. Bei den anderen Protokollen muss ich dies noch verifzieren.

Mal "dahin" gefragt, was ist der große Unterschied in dem jetzigen Code zum "damaligen" Stand juni von @sidey79. Eines ist auffällig. Seitdem wir versuchen das ganze zu Debugen mit addData, ist der Empfang der Hideki Protokolle schlechter. Ich habe das mal durchgespielt als ich die alte FW nutzte und nun diese von Dir Ralf oder von Sidey. Der Count der Nachrichten ist immer mindestens ca. 1/3 geringer als bei der noch fehlerhaften Firmware vom Juni.

@elektron-bbs
Copy link
Contributor

elektron-bbs commented Oct 30, 2017

Mhmm, irgend etwas muss ich falsch machen :-(
Bei mir splittet es die Nachrichten vom FHT80 nicht.

SIGNALDuino-dev-r332_cc1101_2017-10-30_Ralf9:

2017.10.30 13:42:20 4: sduino868/msg READredu: MU;P0=-32001;P1=379;P2=-409;P3=579;P4=-605;P5=-9312;D=0121212121212121212121212341234123412123412341212123434121234341212121212121212123412341212343412121212121212121212121212123434341234121512121212121212121212121234123412341212341234121212343412123434121212121212121212341234121234341212121212121212121212;CP=1;R=33;O;
2017.10.30 13:46:11 4: sduino868/msg READredu: MU;P0=-32001;P1=388;P2=-398;P3=581;P4=-597;P5=-9304;D=0121212121212121212121212341234123412123412341212123434121234341212121212121212123412341212343412121212121212121212121212123434341234121512121212121212121212121234123412341212341234121212343412123434121212121212121212341234121234341212121212121212121212;CP=1;R=33;O;

Die Wiederholung nach der Pause von ca. 9 mSek wird von FHEM an FHT80TF übergeben und dort natürlich nicht dekodiert, was ja auch so richtig ist.

Mit dem Parameter probiere ich jetzt gerade herum (war ursprünglich 0):
CSmuthresh= -> Schwellwert für den split von MU Nachrichten (0=aus)

Werte von 1 bis 10 splitten die Nachrichten so:

2017.10.30 16:14:54 4: sduino868/msg READ: MS=1;MU=1;MC=0;Mred=1;Mdebug=0_MScnt=3;MuSplitThresh=10
2017.10.30 16:18:16 4: sduino868/msg READredu: MU;P0=-32001;P1=399;P2=-398;P3=575;P4=-611;P5=-9312;D=012121212121212121212121234123412341212;CP=1;R=33;O;
2017.10.30 16:18:16 4: sduino868/msg READredu: MU;P1=393;P2=-402;P3=584;P4=-595;P5=-9312;D=412341212123434121234341212121212121212;CP=1;R=33;e;
2017.10.30 16:18:16 4: sduino868/msg READredu: MU;P1=393;P2=-402;P3=584;P4=-595;P5=-9312;D=234123412123434121212121212121212121212;CP=1;R=33;e;
2017.10.30 16:18:16 4: sduino868/msg READredu: MU;P1=393;P2=-402;P3=584;P4=-595;P5=-9312;D=212343434123412151212121212121212121212;CP=1;R=33;e;
u.s.w.

Was versteckt sich hinter dem Parameter "CSmuthresh" genau?
Durch probieren habe ich jetzt herausgefunden, das da wahrscheinlich die minimale Pausenzeit im mSek hin gehört. Mit einem Wert von 8000 trennt es mir jedenfalls jetzt die Nachrichten:

2017.10.30 16:29:55 4: sduino868/msg READ: MS=1;MU=1;MC=0;Mred=1;Mdebug=0_MScnt=3;MuSplitThresh=8000
2017.10.30 16:31:44 4: sduino868/msg READredu: MU;P0=-13904;P1=396;P2=-400;P3=590;P4=-598;P5=-9296;D=012121212121212121212121234123412341212341234121212343412123434121212121212121212341234121234341212121212121212121212121212343434123412;CP=1;R=33;O;
2017.10.30 16:31:44 4: sduino868/msg READredu: MU;P1=387;P2=-405;P3=577;P4=-604;P5=-9296;D=512121212121212121212121234123412341212341234121212343412123434121212121212121212341234121234341212121212121212121212121212343434123412;CP=1;R=33;e;

@Ralf9
Copy link
Contributor Author

Ralf9 commented Oct 30, 2017

Ja das muthresh ist der Patternwert ab dem eine Pause erkannt wird. Hier wäre es ca 9000.
Das passt so jetzt, es sind 2 gleiche Nachrichten.
Welche Vorteile habt Ihr, wenn Ihr bei dem MU Nachrichten die Wiederholungen habt?
Wird bei diesem Protokoll die Nachricht einmal wiederholt?
Ein "O" an Ende bedeutet übrigens, das der Puffer voll ist, und ein "e" bedeutet, daß ein Nachrichten ende erkannt wurde.

@elektron-bbs
Copy link
Contributor

elektron-bbs commented Oct 30, 2017

Da die Auswertung ja auf ">= MuSplitThresh" prüft, würde ich hier z.B. 5000 einstellen. Aber wie Sidey schon bemerkt hatte, wäre es sicher besser, diesen Wert nicht benutzerdefiniert zu machen. Das ist zum einen keinem Nutzer zumutbar, die Protokolle zu kennen und zum anderen braucht man ja u.U. bei verschiedenen Protokollen unterschiedlich Werte. Gefühlsmäßig würde ich den Wert so auf 5 bis 10 mal größte Pulslänge setzen.

Welche Vorteile habt Ihr, wenn Ihr bei dem MU Nachrichten die Wiederholungen habt?

Der Vorteil ist höhere Übertragungssicherheit, wenn der erste Teil vielleicht nicht erkannt wurde, klappt es halt mit dem zweiten :-) Außerdem erinnere ich mich an ein anderes Protokoll (Siro m.E.), da wurde ein bestimmter Befehl erst in einer Wiederholung übertragen.

Wird bei diesem Protokoll die Nachricht einmal wiederholt?

Beim FHT80 einmal, bei FS20 bis zu zweimal.

@Ralf9
Copy link
Contributor Author

Ralf9 commented Oct 31, 2017

Hallo Sidey,

ich verstehe noch nicht so richtig wie die Add pattern Routine funktioniert.

		// Add pattern
		if (patternLen == maxNumPattern)
		{
			calcHisto();
			if (histo[pattern_pos] > 2)
			{
				processMessage();
				calcHisto();


			}
			for (uint8_t i = messageLen - 1; i >= 0 && histo[pattern_pos] > 0; --i)
			{
				if (message[i] == pattern_pos) // Finde den letzten Verweis im Array auf den Index der gleich ueberschrieben wird
				{
					i++; // i um eins erhoehen, damit zukuenftigen Berechnungen darauf aufbauen koennen
					bufferMove(i);
					break;
				}
	
			}

		}

Wenn ich das richtig verstehe, wird bei einem neuen Pattern das älteste Pattern im Patternpuffer überschrieben.

Mir ist aber nicht klar zu was dies nötig ist?

			if (histo[pattern_pos] > 2)
			{
				processMessage();
				calcHisto();
			}

@sidey79
Copy link
Contributor

sidey79 commented Oct 31, 2017

Der Abschnitt dient dazu, processMessage aufzurufen, wenn das Pattern, welches überschrieben werden soll mehr als 2 Mal in der Nachricht vorkommt

@Ralf9
Copy link
Contributor Author

Ralf9 commented Oct 31, 2017

in findpatt() wird ein tolfact von 0,2 verwendet
tol = abs(val)*0.2;
und in reset() steht
tolFact = 0.25;

ist dies so ok?

@Ralf9
Copy link
Contributor Author

Ralf9 commented Nov 1, 2017

Hallo Sidey,

ich habe in der calcHisto Routine noch einige Fehler gefunden. Eine ungerade startpos wurde nicht berücksichtigt und das endpos hat auch noch nicht ganz gepasst:

void SignalDetectorClass::calcHisto(const uint8_t startpos, uint8_t endpos)
{
	for (uint8_t i = 0; i<maxNumPattern; ++i)
	{
		histo[i] = 0;
	}

	if (endpos == 0) endpos = messageLen-1;
	uint8_t bstartpos = startpos/2;       // *4/8;
	uint8_t bendpos = endpos/2;           // *4/8;
	uint8_t bval;
	if (startpos % 2 == 1)  // ungerade
	{
		message.getByte(bstartpos,&bval);
		histo[bval & B00001111]++;
		bstartpos++;
	}
	for (uint8_t i = bstartpos; i <= bendpos; ++i)
	{
		message.getByte(i,&bval);
		histo[bval >> 4]++;
		histo[bval & B00001111]++;
	}
	if (endpos % 2 == 0)  // gerade
	{
		message.getByte(bendpos, &bval);
		histo[bval & B00001111]--;
	}
}

Nun ist mir aufgefallen, daß sich die Add pattern Routine jetzt etwas anders verhält. Bei MS Nachrichten ist nun das mStart viel größer als vorher:

Sync: -3894 -> SyncFact: -7.91, Clock: 492, Tol: 198, PattLen: 4 (4), Pulse: F/L 492, -980, mStart: 42, mend: 115, valcnt: 254, MCD: 0, mtrunc: 1, state: 2
Signal: 01020102020202020202020202010201010101020103010202010201020202010201010101010201020202020202020202020102010101010201030102020102010202020102010101010102010202020202020202020201020101010102010301020201020102020201020101010101020102020202020202020202010201.  [254]
Pattern:  P0: 37*[492] P1: 16*[-980] P2: 20*[-1938] P3: 1*[-3894]

MS;P0=492;P1=-980;P2=-1938;P3=-3894;D=03010202010201020202010201010101010201020202020202020202020102010101010201;CP=0;SP=3;R=11;O;
Sync: -9028 -> SyncFact: -17.07, Clock: 529, Tol: 6282, PattLen: 6 (6), Pulse: F/L -9168, -31412, mStart: 34, mend: 101, valcnt: 102, MCD: 0, mtrunc: 0, state: 2
Signal: 012121232321232321232123212323242124212323232123212121212123212121212323212321212121212323212323212325.  [102]
Pattern:  P0: 0*[180] P1: 19*[-2081] P2: 34*[529] P3: 13*[-4133] P4: 1*[-9028] P5: 1*[-31412]

MS;P1=-2081;P2=529;P3=-4133;P4=-9028;P5=-31412;D=24212323232123212121212123212121212323212321212121212323212323212325;CP=2;SP=4;R=249;
Sync: -8964 -> SyncFact: -15.30, Clock: 586, Tol: 1379, PattLen: 8 (0), Pulse: F/L -6896, 586, mStart: 45, mend: 71, valcnt: 72, MCD: 0, mtrunc: 1, state: 2
Signal: 012304540454645454645454645454545454545454645407645454545464646454646464.  [72]
Pattern:  P0: 1*[-8964] P1: 0*[204] P2: 0*[-116] P3: 0*[112] P4: 13*[586] P5: 5*[-2096] P6: 7*[-4144] P7: 1*[292]

MS;P0=-8964;P4=586;P5=-2096;P6=-4144;P7=292;D=407645454545464646454646464;CP=4;SP=0;R=249;

In der processMessage hat bei MS Nachrichten bei der Abfrage "(messageLen - mstart) >= minMessageLen" gefehlt:
if ((m_endfound && (mend - mstart) >= minMessageLen) || (!m_endfound && messageLen < maxMsgSize && (messageLen - mstart) >= minMessageLen))

@sidey79
Copy link
Contributor

sidey79 commented Nov 5, 2017

@Ralf9

Ich habe mir mal die Laufzeit einiger Passagen angesehen...

void SignalDetectorClass::processMessage()
{
	unsigned long t = micros();
	yield();


	if (mcDetected == true || messageLen >= minMessageLen) {
		success = false;
		m_overflow = (messageLen == maxMsgSize) ? true : false;

#if DEBUGDETECT >= 1
		DBG_PRINTLN("Message received:");
#endif

		compress_pattern();
		d = micros() - t;

compress_pattern kann durchaus mal 10 ms andauern. Da läuft der Puffer durchaus mal voll.
:(

@Ralf9
Copy link
Contributor Author

Ralf9 commented Nov 5, 2017

Ja, das habe ich inzwischen auch festgestellt, daß in der compress_pattern anscheinend etwas nicht zu passen scheint.
Der bug tritt aber recht selten auf.
Ich denke, daß dies aber nichts mit der Laufzeit zu tun hat.

@Ralf9
Copy link
Contributor Author

Ralf9 commented Nov 5, 2017

Kann es sein, daß in dieser Zeile was nicht passt?
pattern[idx] = ((long(pattern[idx]) * histo[idx]) + (pattern[idx2] * histo[idx2])) / sum;

Ich habe ein paar Debugausgaben eingefügt:

				int  sum = histo[idx] + histo[idx2];
				int lPatternIdx = pattern[idx];
				pattern[idx] = ((long(pattern[idx]) * histo[idx]) + (pattern[idx2] * histo[idx2])) / sum;
				if (abs(pattern[idx]) < 90) {
					DBG_PRINT("upfirst<90 ");DBG_PRINT(idx);DBG_PRINT(" ");DBG_PRINT(idx2);DBG_PRINT(" ");DBG_PRINTLN(lPatternIdx);
					printOut();
				}

und bekomme diese Ausgabe!

upfirst<90 0 4 724
Signal: 12305060606050505060605050605050505060506060605050605060606060505060605060505070507050606060505050606050506050505050605060606050506050606060605050606050605050705070506060605050506060505060505050506050606060505060506060606050506060506050507050705060606054.  [254]
Pattern:  P0: 1*[45] P1: 1*[-160] P2: 1*[208] P3: 1*[-92] P4: 125*[564] P5: 62*[-2094] P6: 57*[-4090] P7: 6*[-9019]

P0:1*[724] + P4: 125*[564] = 45 passt nicht so richtig!

oder

upfirst<90 0 3 -2084
Pattern:  P0: 9*[-53] P1: 2*[-764] P2: 125*[554] P3: 56*[-2068] P4: 53*[-4110] P5: 6*[-9015] P6: 2*[774] P7: 1*[-1008]

P0: 9*[-2084] + P3: 56*[-2068] = -53

@sidey79
Copy link
Contributor

sidey79 commented Nov 5, 2017

Der Wertebereich läuft über, da pattern[idx2] * histo[idx2] in der Klammer steht und nur ein int ist (16 Bit)

@Ralf9
Copy link
Contributor Author

Ralf9 commented Nov 5, 2017

ist es so besser?
pattern[idx] = ((long(pattern[idx]) * histo[idx]) + (long(pattern[idx2]) * histo[idx2])) / sum;

@sidey79
Copy link
Contributor

sidey79 commented Nov 5, 2017

Ich würde Mal darauf tippen, dass es dann klappt ;)

@Ralf9
Copy link
Contributor Author

Ralf9 commented Nov 5, 2017

nun scheint es zu klappen, bis jetzt hatte ich keine Wrong Data after compress_pattern mehr.
Kann sich durch den Wertebereich überlauf auch das Vorzeichen geändert haben?

@Ralf9
Copy link
Contributor Author

Ralf9 commented Nov 6, 2017

Zu was ist ein nach einem compresspattern noch ein calcHisto notwendig?

@Ralf9
Copy link
Contributor Author

Ralf9 commented Nov 18, 2017

ok, dann baue ich es in die 00_SIGNALduino ein.

Ich möchte beim Hideki auch die Möglichkeit haben, zu testen und auszugeben ob und wie oft es vorkommt, daß die MC-Nachrichten invertiert sind.

Kann ich es so machen?

sub	SIGNALduino_Hideki()
{
	my ($name,$bitData,$id,$mcbitnum) = @_;
	my $debug = AttrVal($name,"debug",0);
	
    Debug "$name: search in $bitData \n" if ($debug);
	my $message_start = index($bitData,"10101110");
	if ($message_start >= 0 )   # 0x75 but in reverse order
	{
		Debug "$name: Hideki protocol detected \n" if ($debug);
	} else if (SDUINO_MC_TESTINVERT == 1) {
		$message_start = index($bitData,"01010001");      #  pruefen ob das Signal invertiert ist
		if ($message_start >= 0 )   # 0x75 invertiert
		{
			#Debug "$name: Hideki inverted protocol detected \n" if ($debug);
			Log3 $name, 3, "$name: Hideki inverted protocol detected";
			$bitData =               # $bitData invertieren
		} else {
			return (-1,"Start pattern (10101110) not found");
		}
	} else {
		return (-1,"Start pattern (10101110) not found");
	} 
 ...

Wie kann ich die $bitData invertieren?

@sidey79
Copy link
Contributor

sidey79 commented Nov 18, 2017

Verstehe ich nicht ganz was Du da ausgeben möchtest.
Jede Nachricht ist doch invertiert im Vergleich zu vorher.

Und in parse_MC ist das invertieren der Nachrichten bereits enthalten.

			if (exists($ProtocolListSIGNALduino{$id}{polarity}) && ($ProtocolListSIGNALduino{$id}{polarity} eq 'invert') && (!defined($hash->{version}) || substr($hash->{version},0,6) ne 'V 3.2.'))
			# todo  && substr($hash->{version},0,6) ne 'V 3.2.')   # bei version V 3.2. nicht invertieren 
			{
		   		$bitData= unpack("B$blen", pack("H$hlen", $rawDataInverted)); 
			} else {
		   		$bitData= unpack("B$blen", pack("H$hlen", $rawData)); 
			}

Und in SIGNALduino_Split_Message müsste man halt noch auf den Typ Mc Zusätzlich zu MC matchen
if ($_ =~ m/^MS/ or $_ =~ m/^MC/ or $_ =~ m/^Mc/ or $_ =~ m/^MU/)

Dann könnte man mithilfe "messagetype" abfragen ob es MC oder Mc ist.

Die hinterlegte Polarität könnte man in etwa so umdrehen:


if (exists($ProtocolListSIGNALduino{$id}{polarity})
{
   ($ProtocolListSIGNALduino{$id}{polarity};
} else {
 $ProtocolListSIGNALduino{$id}{polarity} = "inverted"
}

Habe ich jetzt so noch nicht ausprobiert, müsste aber im groben so gehen.

@Ralf9
Copy link
Contributor Author

Ralf9 commented Nov 18, 2017

Ich möchte langfristig von der ID 12.1 wegkommen.
Durch den neuen MC Pattern Decoder der nicht mehr invertiert, dürfte das invertieren der Nachrichten eigentlich nicht mehr notwendig sein. Bei einem ersten test gestern Abend hatte ich den Eindruck, daß die Hideki Nachrichten nur noch recht selten invertiert sind.
Bei "Mc" dürfte eigentlich keine Inventierung notwendig sein.

my $messagetype=$msg_parts{messagetype};
..
			if ($messagetype eq 'MC' && exists($ProtocolListSIGNALduino{$id}{polarity}) && ($ProtocolListSIGNALduino{$id}{polarity} eq 'invert') && (!defined($hash->{version}) || substr($hash->{version},0,6) ne 'V 3.2.'))
			{
		   		$bitData= unpack("B$blen", pack("H$hlen", $rawDataInverted)); 
			} else {
		   		$bitData= unpack("B$blen", pack("H$hlen", $rawData)); 
			}

@sidey79
Copy link
Contributor

sidey79 commented Nov 18, 2017

Naja, das verstehe ich nicht.

Jetzt wird ja im Gegensatz zu vorher die Polarität zu vorher invertiert erfasst.
Der Decoder weiss ja nichts davon, wie es der sender gemeint hat.

@Ralf9
Copy link
Contributor Author

Ralf9 commented Nov 18, 2017

Heißt das, daß wir bei den Protokollen, bei denen momentan keine polarity 'invert' definiert ist, jetzt invertieren müssen?

11 AS
43 Somfy RTS
47 Maverick protocol

Kann ich damit $bitData invertieren?
$bitData =~ tr/01/10/;

@sidey79
Copy link
Contributor

sidey79 commented Nov 18, 2017

Davon gehe ich aus, dass wir die Polarität genau umgekehrt benötigen wie davor.

Mit tr/01/10 könntest Du bits invertieren, aber die Daten liegen schon in beiden Formen im Speicher, da die Hex Digits bereits invertiert in einer Variabe gespeichert werden.

$rawDataInverted und $rawData

Meiner Meinung nach, müssen wir eigentlich die Polarität nur umdrehen, wenn Mc übergeben kommt, als es aktuell in den Protokollen vermerkt ist.

Wenn Polarity gesetzt ist, dann werden aktuell die invertierten Werte genommen. Ist dies nicht gesetzt, dann wird RawData verwendet. Dieses Verhalten müssen wir nun umdrehen.

Wenn Polarity gesetzt ist, dann wird der nicht invertiere Werte genommen. Ist dies nicht gesetzt, dann wird rawDataInverted verwendet.

Pro Protokoll würde ich so Änderungen nicht anfangen, dafür ist ja Polarity gedacht.

@Ralf9
Copy link
Contributor Author

Ralf9 commented Nov 18, 2017

Ich möchte ohne dafür eine 2.Protokol (ID 12.1) definieren zu müssen, die Möglichkeit haben, zu testen ob und wie häufig es vorkommt daß es dem MC Pattern decoder nicht gelingt das MC Signal nicht invertiert zu erkennen und es dann invertiert decodiert und übertragen wird.
Wenn dies nur sehr selten vorkommt können wir auf die ID 12.1 verzichten

@sidey79
Copy link
Contributor

sidey79 commented Nov 18, 2017

Das sieht man, je nachdem ob die Definition für Protokoll 12 oder 12.1 passt dachte ich.

@Ralf9
Copy link
Contributor Author

Ralf9 commented Nov 18, 2017

Ich möchte es als log Ausgabe haben

2017.11.19 00:34:40.153 3 : sduino: Hideki inverted protocol detected
2017.11.19 00:34:40.686 3 : sduino 12, Mc;LL=-1004;LH=944;SL=-515;SH=460;D=570C8BA527DBF5188A1E928;C=487;L=91;
2017-11-19 00:34:40.688 Hideki Hideki_30_5 T: 16.1 H: 53
2017-11-19 00:34:40.688 Hideki Hideki_30_5 battery: ok
2017-11-19 00:34:40.688 Hideki Hideki_30_5 channel: 5
2017-11-19 00:34:40.688 Hideki Hideki_30_5 temperature: 16.1
2017-11-19 00:34:40.688 Hideki Hideki_30_5 package_number: 3
2017-11-19 00:34:40.688 Hideki Hideki_30_5 humidity: 53
2017-11-19 00:34:40.688 Hideki Hideki_30_5 comfort_level: Hum. OK. Temp. uncomfortable (>24.9 or <20)

@Ralf9
Copy link
Contributor Author

Ralf9 commented Nov 19, 2017

Dies ist so evtl etwas kompliziert aber ich weis nicht wie ich es einfacher hinbekomme:

			my $polarityInvert = 0;
			if (exists($ProtocolListSIGNALduino{$id}{polarity}) && ($ProtocolListSIGNALduino{$id}{polarity} eq 'invert'))
			{
				$polarityInvert = 1;
			}
			if (($messagetype eq 'Mc') || (defined($hash->{version}) && substr($hash->{version},0,6) eq 'V 3.2.'))
			{
				$polarityInvert = $polarityInvert ^ 1;
			}
			if ($polarityInvert == 1)
			{
		   		$bitData= unpack("B$blen", pack("H$hlen", $rawDataInverted)); 
			} else {
		   		$bitData= unpack("B$blen", pack("H$hlen", $rawData)); 
			}

@sidey79
Copy link
Contributor

sidey79 commented Nov 19, 2017

Naja ich finde es nicht kompliziert.
Meine Idee mit dem deleted ist ja nicht so gut :)

Nach dem oder
if (($messagetype eq 'Mc') ||

Würde ich die darauf Folgenden Bedingungen in eine Klammer setzen.
Sonst müsste Mc und V3.2. gefunden werden. Das halte ich für relativ unwahrscheinlich, dass es dazu kommt :)

@Ralf9
Copy link
Contributor Author

Ralf9 commented Nov 19, 2017

Das müsste so passen, da ja || und && einer höhere prio hat als eq
if ($messagetype eq 'Mc' || (defined($hash->{version}) && substr($hash->{version},0,6) eq 'V 3.2.'))

Das testen von ob und wie oft es vorkommt daß die MC-Nachrichten invertiert gesendet werden,
habe ich mit diesen beiden Konstanten gelöst

	SDUINO_MC_DISPATCH_VERBOSE  => 5,      # wenn kleiner 5, z.B. 3 dann wird vor dem dispatch mit loglevel 3 die ID und rmsg ausgegeben
	SDUINO_MC_DISPATCH_LOG_ID   => '12.1'  # die o.g. Ausgabe erfolgt nur wenn der Wert mit der ID übereinstimmt

@sidey79
Copy link
Contributor

sidey79 commented Nov 19, 2017

Hmm, ich würde das so auswerten:

Bedingung 1
oder Bedingung 2 und Bedingung 3

Also Bedingung 1 und Bedingung 3 oder Bedingung 2 und 3 müssen erfüllt sein.

Ob es Perl in allen Versionen auch nicht weiss ich nicht.

Eindeutig und gewollt war doch bestimmt

Bedingung 1 oder (Bedingung 2 und Bedingung 3)

Also 1 oder 2 und 3
1 und 3 ist keine gewollte Kombination.

@Ralf9
Copy link
Contributor Author

Ralf9 commented Nov 19, 2017

if ($messagetype eq 'Mc' || (defined($hash->{version}) && substr($hash->{version},0,6) eq 'V 3.2.'))

Durch die Klammer vor defined "'Mc' || (defined" habe ich doch:
Bedingung 1 oder (Bedingung 2 und Bedingung 3)

@sidey79
Copy link
Contributor

sidey79 commented Nov 19, 2017

Irgendwie war ich der Meinung die Bedingung 2 steht alleine in einer Klammer. Auf dem Handy finde ich das immer etwas schwieriger zu erkennen :(

Ist aber wohl doch nicht so, die Klammer ist un Bedingung 2 und Bedingung 3.

@sidey79
Copy link
Contributor

sidey79 commented Nov 19, 2017

@Ralf9

Ich habe die Ausgabe (Mc) gepusht:
0557450

Kannst Du das angepasste Modul schon pushen?

@Ralf9
Copy link
Contributor Author

Ralf9 commented Nov 19, 2017

was meinst Du mit pushen? Ich habe es heute Nachmittag commitet
RFD-FHEM/RFFHEM@4376335

@sidey79
Copy link
Contributor

sidey79 commented Nov 19, 2017

Hatte ich irgendwie nicht so richtig gesehen.

sidey79 added a commit that referenced this issue Nov 26, 2017
Fehler in compress_pattern (cast auf long für 2. puls fehlte)
#78 #71 #67
@Ralf9
Copy link
Contributor Author

Ralf9 commented Nov 27, 2017

in der SignalDetectorClass::getSync() ist mir was aufgefallen, das für mich nicht ganz sauber aussieht.
Die for Schleife wird mit einem "return true;" verlassen.
Wäre "ret=true;" und "break;" nicht sauberer?
Oder sind die aktuellen c-Compiler so gut, daß diese Unsauberkeit nichts ausmacht?

@sidey79
Copy link
Contributor

sidey79 commented Nov 27, 2017

@Ralf9

Was ist daran unsauber eine Funktion mit Return zu verlassen?

@Ralf9
Copy link
Contributor Author

Ralf9 commented Nov 27, 2017

Ich habe mal nachgeschaut, demnach kann auch mit return eine Schleife sauber verlassen werden.
Dies war mir bis jetzt nicht klar.
https://msdn.microsoft.com/de-de/library/b80153d8.aspx

Eine for-Schleife wird beendet, wenn break, return oder goto (an eine Anweisung mit Bezeichnung außerhalb der for-Schleife) innerhalb von statement ausgeführt wird. Mit einer continue-Anweisung in einer for-Schleife wird nur die aktuelle Iteration beendet.

Dann kann auch eine verschachtelte Schleife mit return beendet werden:

for (int a=0; ...
{
	for (int b=0; ...
	 {
		....
		return true;
	 }
}

@sidey79
Copy link
Contributor

sidey79 commented Nov 27, 2017

Jo, dann sind wir ja auf dem gleichen Stand :)

@Ralf9
Copy link
Contributor Author

Ralf9 commented Feb 24, 2018

@sidey79
Mir ist aufgefallen, daß bei "compress_pattern()" für die tol ein tolFact = 0.25 verwendet wird.
Bei "findpatt" wird aber bei tol ein fester Faktor von 0,2 verwendet. Wäre es nicht besser hier auch den tolFact zu verwenden?

sidey79 added a commit that referenced this issue Feb 25, 2018
#67

Erkennung des 1. Bits angepasst, wenn eine preamble erkannt wurde
sidey79 added a commit that referenced this issue Feb 25, 2018
Fehler in compress_pattern (cast auf long für 2. puls fehlte)
#78 #71 #67
@sidey79
Copy link
Contributor

sidey79 commented Feb 25, 2018

Zu Beginn lief alles über tolfact.

Der Faktor in findpatt stand auch Mal auf 0,3.

Leider weiss ich nicht mehr genau welche Probleme es damit gab, allerdings habe ich es nicht ganz ohne Grund geändert.

Grundlegend wäre es aber wünschenswert überall mit den gleichen Toleranz Werten zu arbeiten.

@Ralf9
Copy link
Contributor Author

Ralf9 commented Apr 2, 2018

Beim Einbauen vom empfangen von HE_EU ist mir aufgefallen, daß "syncLenMax = 100;" in getsync hierfür zu klein ist.
Das HE_EU hat 57 Datenbits, dies ergibt 2 + 57 x 2 = 116 Pattern.
Ich habe bei mir syncLenMax auf 120 erhöht, damit passt es.

Ich habe mir nochmals Gedanken über das histo[p] >= 1 gemacht:

if ((pattern[p] < 0) &&
				(syncabs < syncMaxMicros && syncabs / pattern[clock] <= syncMaxFact) &&
				(syncabs > syncMinFact*pattern[clock]) &&
				(histo[p] < messageLen*0.08) && (histo[p] >= 1)
)

Wir haben histo[p] > 1 durch histo[p] >= 1 ersetzt.
Ein Grund war, daß es vor kam, daß die letzte Wiederholung als MU Nachricht erkannt wurde.
Dieser Grund hat sich bei mir durch eine Optimierung der MS-Ausgabe erledigt.

Ein anderer Grund könnte sein, daß es MS Nachrichten gibt bei denen jede Wiederholung eine eigene Nachricht ohne ein sync am Ende ist.

Wir hatten bei einem Issue oder Commit schon mal was darüber geschrieben, ich kann es aber nicht mehr finden.

Nachtrag;
Ich habe das Gefühl, daß das histo[p] >= 1 auch Nachteile hat, u.a. wird dadurch mehr Zeit benötigt.

@Ralf9
Copy link
Contributor Author

Ralf9 commented Apr 3, 2018

Eine Möglichkeit ist auch:

...
(histo[p] < messageLen*0.08) && (histo[p] >= 1)
)
   if (histo[p] == 1 && messageLen == maxMsgSize)
       continue;

Ralf9 added a commit to Ralf9/SIGNALDuino_Forked that referenced this issue Apr 14, 2018
von 100 auf 120 erhöht. Dies ist zum Empfangen von Homeeasy HE_EU notwendig
RFD-FHEM#67 (comment)

Beim if   histo[p] => 1 durch histo[p] = 1 ersetzt
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

No branches or pull requests

4 participants