raw-Befehl sendet zu viele Daten

[C_Herrmann]

Hallo,

ich habe einen Fehler in der culfw gefunden. Beim Senden eins CUL-raw-Befehls werden zusätzliche Bits gesendet.

Die Auswirkungen habe ich hier beschrieben:
http://forum.fhem.de/index.php/topic,19930.0.html

Auf der Suche nach dem Fehler habe ich dies Gefunden:

In der Datei aus dem aktuellen SVN:
culfw-code-400-trunk\culfw\clib\rf_send.c

Zeile 130:
    for(i = 7; i > bitoff; i--)                 // broken bytes
      send_bit(msg[j] & _BV(i));

    my_delay_ms(pause);                         // pause

Sollte sein:
    for(i = bitoff; i > 0; i--)                 // broken bytes
      send_bit(msg[j] & _BV(i));

    my_delay_ms(pause);                         // pause

Im Original wird die Schleife 7 mal ausgeführt, wenn nur 1 Bit gesendet werden soll.

Kann ein Entwickler dies bitte mal prüfen und die Datei und eine aktualisierte Firmware ggf. einchecken?

Gruß,
Christian

[rudolfkoenig]

Du hast den Parameter bitoff als NrOfBits interpretiert. Ist es aber nicht.

Hast du nach deiner Aenderungen geprueft, ob FS20/FHT noch funktioniert?

[C_Herrmann]

Hallo Rudi,

Ich habe noch keine Firmware erstellt. Muß mir erst einmal die Programme dafür besorgen. Welche Programme sind unter Win8.1 zu empfehlen?
Gibt es auch Debugger, mit denen man sowas vorher simulieren kann? Ich habe leider keinen zweiten CUL zum testen.

Ich habe jetzt weitere Tests mit 2 und 3 Bits im "broken Byte" sowie 3 Sync ohne broken Byte gemacht.
Mein CUL 868 hat culfw 1.57.

3 Byte + 2 Bits im broken Byte:
set CUL1 raw G0032E15020205054326d2
ergibt:
0101 0100 0011 0010 0110 1101 00010
5       4       3       2       6       d       5 statt 2 Bits

3Byte + 3 Bits im broken Byte:
set CUL1 raw G0033E15020205054326d3
ergibt:
0101 0100 0011 0010 0110 1101 1011
5       4       3       2       6       d       4 statt 3 Bits

3 Sync-Bits + 3 Byte + 0 broken Byte:
set CUL1 raw G0330E15020205054326d
ergibt:
0001 0101 0100 0011 0010 0110 1101 0000111
sync  5       4       3       2       6       d       7 überflüssige Bits

FS20 ist korrekt:
set CUL1 raw F12340111
ergibt:
0000000000001 0001 0010 0 0011 0100 1 0000 0001 1 0001 0001 0 0101 1110 1 0
Sync 12*0 +1     1       2        P 3       4       P 0       1       P 1       1       P Quers.       P E

Obiger FS20 nachgebildet als raw (12 Sync + 5 Byte + 6 broken Bits):
0000000000001 0001 0010 0001 1010 0100 0000 0110 0010 0010 0101 1110 10
Sync 12*0 + 1    1       2       1        A       4       0       6       2       2       5       E       2
set CUL1 raw G0C56E150202050121A406225E2
ergibt:
0000000000001 0001 0010 0 0011 0100 1 0000 0001 1 0001 0001 0 0101 1
Anfang identisch mit obigem Datagramm                                                          hier fehlen 5 von 6 Bit?

rf_send.c
Zeile 246:
  sendraw(hb+7, sync, nby, nbi, repeat, pause);

Zeile 90:
sendraw(uint8_t *msg, uint8_t sync, uint8_t nbyte, uint8_t bitoff,
                uint8_t repeat, uint8_t pause)

*msg = hb+7;  sync = sync;  nbyte = nby;  bitoff = nbi;  repeat = repeat;  pause = pause

Mit "nbi = (hb[1] & 0xf);" in Zeile 239 wird nbi als 3. Byte nach dem "G" übernommen und an bitoff übergeben.

Zeile 130:
for(i = 7; i > bitoff; i--)
Wenn bitoff = 2 wird die Schleife 5 mal durchlaufen.
Wenn bitoff = 3 wird die Schleife 4 mal durchlaufen.
usw.
Beim nachgebildeten FS20-Befehl ist bitoff 6 und die Schleife wird nur ein mal durchlaufen.

Das würde meine Vermutung bestätigen.

Kannst Du das bitte noch mal prüfen? Meine Beobachtungen lassen sich jederzeit reproduzieren.
Muss die Schleife ggf. nur für raw-Befehle angepasst werden?

Gruß,
Christian

[rudolfkoenig]

FS20 ist korrekt:

Ok, dann sollte schonmal sendraw in Ruhe gelassen werden.

Du hast den Parameter bitoff als NrOfBits interpretiert. Ist es aber nicht.

Ich versuche es mit anderen Worten: Die Doku zu G ist falsch oder nicht vollstaendig, n bezeichnet nicht die Anzahl der Bits, die gesendet werden, sondern den Offset des Bits, das nicht mehr gesendet werden soll.

[C_Herrmann]

Hallo Rudi,

danke für die Antwort. Anders herum hätte ich es logischer gefunden.

Gruß,
Christian

[C_Herrmann]

Hallo Rudi,

ich habe mir die Datei rf_sendraw.c noch mal genau angesehen. Wenn ich es richtig verstehe, gelten Anweisungen, die mit "#ifdef HAS_RAWSEND" definiert sind, nur für raw-Befehle. Dann müsste doch die Funktionalität, die in der commandref auf culfw beschrieben ist, mit der u.g. Änderung für raw-Befehle möglich sein, ohne die FS20-Funktionen zu beeinflussen.

Zeile 130,131:
    for(i = 7; i > bitoff; i--)                 // broken bytes
      send_bit(msg[j] & _BV(i));

Ersetzen durch:
#ifdef HAS_RAWSEND
    for(i = bitoff; i > 0; i--)                 // broken bytes raw
      send_bit(msg[j] & _BV(i));
#else
    for(i = 7; i > bitoff; i--)                 // broken bytes
      send_bit(msg[j] & _BV(i));
#endif

Kannst Du das bitte noch mal überdenken? So wäre die Eingabe logischer, da nicht eine 7 eingegeben werden muss, wenn keine broken Bits gesendet werden sollen.

Gruß,
Christian

[rudolfkoenig]

Den Vorschlag kann ich natuerlich nicht akzeptieren, #ifdef ist _COMPILE_ time, d.h. ein CUL_V3 wuerde damit kein FS20/FHT mehr senden koennen.

[C_Herrmann]

Hallo Rudi,

Sorry, ich habe wohl doch nicht genau genug hingeschaut und mich die Zeilen 49 bis 83 irritieren lassen.

Würdest Du es denn grundsätzlich befürworten, die sendraw-Routine nach meinen Vorstellungen zu ändern?

Nach weiteren Überlegungen habe ich eine ganz einfache Lösung gefunden.

Zeile 239:
  nbi = (hb[1] & 0xf);

Ändern in:
  nbi = 7 - (hb[1] & 0xf);

Ergebnis bisher:
Gesendet mit G003x...  x=bitoff=nbi
0 1 2 3 4 5 6 7 8 9 a b c d e f
Empfangene broken Bits:
7 6 5 4 3 2 1 0 0 0 0 0 0 0 0 0

Nach Änderung:
Gesendet mit G003x...  x=bitoff
0 1 2 3 4 5 6 7 8 9 a b c d e f
sendraw wird aufgerufen mit nbi = 7 - bitoff:
7 6 5 4 3 2 1 0
Empfangene broken Bits:
0 1 2 3 4 5 6 7

Eventuell muss es "7 - (hb[1] & 0x7)" sein, um Werte größer 7 auszuschließen. Bislang werden ja auch nur maximal 7 Bits akzeptiert.

Könntest Du Dich damit anfreunden? Es beträfe nur raw-Befehle.

Gruß,
Christian

[rudolfkoenig]

Deine neue Loesung ist aequivalent mit der alten, und macht FS20/FHT kaputt.

Eine korrekte Loesung besteht entweder darin, die G-Doku zu fixen, oder FS20/FHT umzubauen. Wenn ueberhaupt, dann wuerde ich nur Doku-Fixen, da ich keine Lust habe 1000+ Fehlermeldungen zu haben, falls beim Umbau auch nur minimal was schiefgeht.

[C_Herrmann]

Hallo Rudi,

mich hat die Geschichte mit dem raw-Befehl nicht ruhen lassen. Daher habe ich jetzt selbst mal die Firmware geändert. Der Ansatz, den ich beschrieben hatte, war richtig. Ich habe noch eine Abfrage des ersten Byte des raw-Befehls eingefügt. Wenn dort ein "G" für den raw-Befehl kommt, wird die geänderte Zeile ausgeführt.

Meine FS20-, FHT- und IT-Geräte arbeiten damit problemlos und der raw-Befehl sendet die bis zu 7 höchsten Bits des broken Byte wie gewünscht.

rf_send Zeile 230ff:

Code Auswählen Erweitern

rawsend(char *in)
{
  uint8_t hb[16]; // 33/2: see ttydata.c
  uint8_t nby, nbi, pause, repeat, sync;

  fromhex(in+1, hb, sizeof(hb));
  sync = hb[0];
  nby = (hb[1] >> 4);
  nbi = (hb[1] & 0xf);
  pause = (hb[2] >> 4);
  repeat = (hb[2] & 0xf);
  zerohigh = hb[3];
  zerolow  = hb[4];
  onehigh  = hb[5];
  onelow   = hb[6];
  sendraw(hb+7, sync, nby, nbi, repeat, pause);
}

Geänderte Subroutine:

Code Auswählen Erweitern

rawsend(char *in)
{
  uint8_t hb[16]; // 33/2: see ttydata.c
  uint8_t nby, nbi, pause, repeat, sync;
  char *rst;

  rst = in;
  fromhex(in+1, hb, sizeof(hb));
  sync = hb[0];
  nby = (hb[1] >> 4);
  if((rst = "G")) {
    nbi = 7 - (hb[1] & 0xf);
  } else {
    nbi = (hb[1] & 0xf);
  }
  pause = (hb[2] >> 4);
  repeat = (hb[2] & 0xf);
  zerohigh = hb[3];
  zerolow  = hb[4];
  onehigh  = hb[5];
  onelow   = hb[6];
  sendraw(hb+7, sync, nby, nbi, repeat, pause);
}


Dann habe ich noch einen Send-Befehl "U" für UNIRoll eingebaut. Dazu habe ich nach der ks_send-Routine diesen Code am Ende der HAS_RAWSEND Definition angehängt:

Code Auswählen Erweitern

// UNIRoll-Send
//G0036E368232368hhhhdc1
//16 bit group-address 4 bit channel-address 4 bit command 1 1-bit

void
ur_send(char *in)
{
  uint8_t hb[7];

  fromhex(in+1, hb, 6);
  zerohigh = TDIV(1700);
  zerolow  = TDIV(590);
  onehigh  = TDIV(540);
  onelow   = TDIV(1700);
  hb[6] = 15;
  sendraw(hb, 0, 3, 6, 3, 15);
}


In rf_send.h (Zeile 9ff.) ist die mittlere Zeile eingefügt.

Code Auswählen Erweitern

void ks_send(char *in);
void ur_send(char *in);
void addParityAndSend(char *in, uint8_t startcs, uint8_t repeat);


In cul.c (Zeile 67ff.) ist die letzte Zeile angehängt.

Code Auswählen Erweitern

#ifdef HAS_RAWSEND
  { 'G', rawsend },
  { 'M', em_send },
  { 'K', ks_send },
  { 'U', ur_send },
#endif


Leider habe ich nicht gefunden, wo der neue Befehl "U" eingetragen werden muss, damit der CUL ihn bei den Kommandos anzeigt und wo "UNIROLL" für die Anzeige der unterstützten Clients eingetragen werden muss.

Die obigen Änderungen beim broken Bit wären dadurch eigentlich nicht mehr erforderlich, aber so entspräche es der commandref.

Gruß,
Christian

[rudolfkoenig]

Code Auswählen Erweitern

  if((rst = "G")) {
Das ist zwar syntaktisch korrekt, aber semantisch "grober Unfug",
d.h. wenn FS20&co funktioniert, dann ist es wohl Glueck.

Auf Ruecksicht auf bisherige Anwender sollte die Doku und nicht der Code geaendert werden.

Dann habe ich noch einen Send-Befehl "U" für UNIRoll eingebaut.

Sowas bitte nur mit "HAS_UNIROLL", was wiederum in der entsprechenden board.h eingetragen wird.
Fuer CUL_V2 waere das z.Bsp. wg. Speichermangel sicher ein Problem. Warum das U nicht in der Liste der bekannten
Befehle angezeigt wird, ist mir z.Zt auch ein Raetsel.

[C_Herrmann]

Hallo Rudi,

Beim UNIROLL-Send hatte ich mich an der Routine "ks_send" orientiert. Jetzt habe ich die UNIROLL-Routine eigenständig gemacht. Die erforderlichen Änderungen habe ich anschließend noch einmal zusammengefasst.

clib:rf_send.c
Ganz am Ende einfügen nach der RAWSEND-Definition:

Code Auswählen Erweitern

#ifdef HAS_UNIROLL
//G0031E368232368hhhhdc1
//16 bit group-address 4 bit channel-address 4 bit command 1 1-bit

void
ur_send(char *in)
{
  uint8_t hb[4];

  fromhex(in+1, hb, 3);
  zerohigh = TDIV(1700);
  zerolow  = TDIV(590);
  onehigh  = TDIV(540);
  onelow   = TDIV(1700);
  hb[3] = 0x80;     //10000000
  sendraw(hb, 0, 3, 6, 3, 15);
}
#endif

clib:rf_send.h
Mittlere Zeile

Code Auswählen Erweitern

void ks_send(char *in);
void ur_send(char *in);
void addParityAndSend(char *in, uint8_t startcs, uint8_t repeat);


CUL:cul.c
Bedingte Definition hinter HAS_RAWSEND

Code Auswählen Erweitern

#ifdef HAS_RAWSEND
  { 'G', rawsend },
  { 'M', em_send },
  { 'K', ks_send },
#endif
#ifdef HAS_UNIROLL
  { 'U', ur_send },
#endif

CUL:board.h
Mittlere Zeile

Code Auswählen Erweitern

#  define HAS_INTERTECHNO
#  define HAS_UNIROLL
#  define HAS_HOERMANN

commandref.html

Code Auswählen Erweitern

    <a name="cmd_U"></a>
    U&lt;hex&gt;
    <ul>
      Send out an UNIRoll message. &lt;hex&gt; is a hex string of the following form:<br>
      ggggdc, where
      <ul>
      <li> gggg is the UNIRoll group address,
      </li><li> d is the UNIRoll device address,
      </li><li> c is the UNIRoll command (b - down, d - stop, e - up)
      </li></ul>
      Example: U12340b
    </ul><br><br>

Wärst Du bereit, das bei Gelegenheit einzupflegen?


Im UNIRoll-Modul habe ich eine Abfrage nach der CUL-Version eingebaut, mit der ich entweder den raw-Befehl oder den UNIRoll-Befehl senden kann.

Code Auswählen Erweitern

# CUL-Version ermitteln und Sendestrings anpassen
  my $iod = AttrVal($name, "IODev", "0");
  my $culget = CommandGet(undef, "$iod version"); # CUL1 version => V 1.58 CUL868
#  $culget = "CUL1 version => V 1.59 CUL868";
  (undef, $culget) = split("V ", $culget);
  my ($culver, $culdev) = split(" ", $culget) if(defined($culget));
  my $vergl = "1.58";
  if(defined($culver) && ($culver =~ m/^\d*\.?\d*$/) && ($culver > $vergl)) {
    $rawpre = $rawpre_old;
    $rawpost = $rawpost_old;
  }

Lässt sich das besser lösen?

Gruß,
Christian

[rudolfkoenig]

Hallo Christian,

ein einziger Patch waere mir lieber, aber ich werde es versuchen auch so einzuchecken.

Die Abfrage im UNIRoll Modul darf nicht auf die Versionsnummer lauten, sondern auf die Liste der angebotenen Befehle ($iodev->{CMDS}), da z.Bsp. viele aktuelle CUL Derivate wie COC,CUNO und SSC ja diesen Befehl auch nicht beinhalten.

Gruss,
  Rudi

[C_Herrmann]

Hallo Rudi,

ich habe einen diff-Patch mit WinMerge erzeugt. Basis ist culfw-code-413-trunk.

Gruß,
Christian

[C_Herrmann]

Hallo Rudi,

Die Abfrage im UNIRoll Modul darf nicht auf die Versionsnummer lauten, sondern auf die Liste der angebotenen Befehle ($iodev->{CMDS}), da z.Bsp. viele aktuelle CUL Derivate wie COC,CUNO und SSC ja diesen Befehl auch nicht beinhalten.

Das ist ja viel einfacher.

Die Version möchte ich aber auch benutzen wegen der Änderungen bei den Sync-Bits ab culfw 1.49. Wenn eine ältere Version vorliegt, wird beim Definieren eines Geräts eine Hinweis ausgegeben, dass eine Aktualisierung erforderlich ist.

Gruß,
Christian