Go Down

Topic: Serial verschluckt Daten ? (Read 640 times) previous topic - next topic

TERWI

Apr 15, 2019, 12:51 pm Last Edit: Apr 15, 2019, 12:53 pm by TERWI
Szenario ist folgendes:
Auf dem PC hab ich ein selbstgebautes Proggie (Delphi), welches ein 16-Byte-Telegramm mit Start- und Endmarker an den Dino (Mega oder Uno) sendet. Je nach dem was im Telegramm steht, kann auch noch eine definierte Menge an Daten folgen.
Der Dino liest das (mit Serial.read), wertet aus und macht je nach Befehl erwas ....
Auf jeden Fall schickt er als eine Art Software-Handshake das gleiche Telegramm (leicht verändert/ergänzt) an den PC zur Kontrolle zurück.
Prinzipiell und auch tatsächlich funzt das super und ist auch recht fix & sicher, ABER.....

Problem:
Während der Testphase, als ich reichlich Serial.println zur Kontrolle drin hatte, lief das einwandfrei.
Nun hab ich das alles so weit auskommntiert, da unnötiger Transfer.
Plötzlich verschluckt der Dino/Serial einige Bytes - das Telegramm kommt zwar komplett zurück, aber nicht mit den erwarteten Werten - wie es mit Serial.println schon der Fall ist...
Es ist auch egal, mit welchem Speed initialisiert wird - bei 9600 kommen ebenso Fehler wie bei 57600.

Die Hautproutine "CMD_Check" wird jeweils in der Loop aufgerufen, sonst ist da im Moment nix weiter drin.
Nach viel Try & Error habe ich mal ein delay() in die Loop gepackt.
Mit einer Verzögerung von min. 20 klappt das dann wieder ....

Sehr eigenartig, kann ich mir nicht erklären.
Jemand eine Idee, warum ist da ein delay nötig ist ?
To young to die - never to old for rock'n roll

combie

#1
Apr 15, 2019, 01:10 pm Last Edit: Apr 15, 2019, 02:48 pm by combie
Quote
Sehr eigenartig, kann ich mir nicht erklären.
Ich mir so auch nicht.


Quote
Jemand eine Idee, warum ist da ein delay nötig ist ?
Nööö...(ich nicht)

Aber ich sehe ja auch nicht, was du da falsch machst.
Der Pessimist sieht die Wolke vor der Sonne.
Der Optimist sieht die Sonne hinter der Wolke.

Mantra: Die Sonne scheint immer!

Tommy56

Wie sollen wir ohne Deinen Sketch etwas Sinnvolles dazu sagen können?
Setze Deinen Code bitte in Codetags (</>-Button oben links im Forumseditor oder [*code] davor und [*/code] dahinter ohne *).

Gruß Tommy
"Wer den schnellen Erfolg sucht, sollte nicht programmieren, sondern Holz hacken." (Quelle unbekannt)

TERWI

#3
Apr 15, 2019, 02:04 pm Last Edit: Apr 15, 2019, 02:05 pm by TERWI
Denke nicht, dass ich was falsch mache.
Es funktioniert ja alles tadellos, sicher und reibungslos - Das Daten-Telegramm wird korrekt hin und zurück gesendet.
Allerdings nur, so lange ich meine Debug-Serial.println's (es müssen min.3 sein !) drinne lasse ODER ich in der Loop ein delay(20) setze.
Anderenfalls werden nur die ersten 5-8 von 16 Byte korrekt gelesen (und genau so falsch zurückgegeben)

Für mich sieht das so aus, als wenn der Dino/Serial "zu schnell für sich selbst" ist und einfach nur für irgendwas ne Pause braucht ?!
Hatte jetzt gehofft, dass hier ggf. irgendwas zeitproblematisches betreff (async Hardware-) Serial bekannt ist.

So eine Frage könnte man auch beantworten, wenn man den Code nicht kennt.
Klar, kann ich hier was posten - das ist allerdings sehr umfangreich und wenn man den (Tricky) Gesamtzusammenhang nicht kennt, wird's etwas schwierig....


To young to die - never to old for rock'n roll

Tommy56

Du kannst ja alles raus lassen, was nicht mit der seriellen Kommunikation zu tun hat, also einen Minisketch bauen, der das Problem nachvollziehbar macht, aber den Rest weg lässt.
Das Problem muss natürlich noch auftreten.

Gruß Tommy
"Wer den schnellen Erfolg sucht, sollte nicht programmieren, sondern Holz hacken." (Quelle unbekannt)

Whandall

Denke nicht, dass ich was falsch mache.
Ich bin ziemlich sicher dass du dich irrst.
Ah, this is obviously some strange usage of the word 'safe' that I wasn't previously aware of. (D.Adams)

uwefed

Denke nicht, dass ich was falsch mache.
Wieso fragst Du denn?

Arduino macht sicher nur das was Du programmiert hast. Ich tippe mal auf Indexüberschreitung eines Arrays.

Also zeig uns den Sketch.

Grüße Uwe

TERWI

#7
Apr 16, 2019, 07:23 am Last Edit: Apr 16, 2019, 07:26 am by TERWI
Muss ich leider in 2 Posts stückeln weil nir 9K reinpassen.

Definitionen:
Code: [Select]

#define DINO_VERSION    33            // "1.1" - HighNibble=Major, LowNibble=Minor
#define DINO_MODUL      1             // Modul-Number - 0=Master, 1++=Slave(s)
#define DINO_MAXCH      1             // standard is 1 - max 4 on MEGA
#define DINO_ID_STRING  "MULTI_TENS"  // string to identify

// Inputs:

// Outputs:

// communication Header
const byte HEADERSIZE = 16;
byte       header[HEADERSIZE];
const byte MARKERSTART =  62;           // ">" Start-marker, transmission begins
const byte MARKERDATA  = 124;           // "|" Header ending, start data
const byte MARKEREND   =  60;           // "<" End-marker, transmisson finished
const byte IDXHEADLEN  =   0;           // real length of header - MARKERDATA should follow immedeatly !!!
const byte IDXMODNUM   =   1;           // Modul-number to talk to
const byte IDXMODCHL   =   2;           // Modul-Channel - supported (Std. 1, 2-4 MEGA only)
const byte IDXMODVOL   =   3;           // "Master-Volume" 0-100%
const byte IDXREPLY    =   4;           // - 0 = return mode info only (no DATA)
                                        // - 1 = return + modul-info
                                        // - 2 = return + sequence-data
                                        // - 3 = return + actual external pulse-data (potis - if any)
const byte IDXPLAY     =   5;           // - 0 = ... Stop everything ...
                                        // - 1 = Manually external --- Freg, Pulse, Rate & Intens (with poti ... if available)
                                        // - 2 = Manually internal --- Freg, Pulse, Rate & Intens (values by block_num[0])
                                        // - 3 = single sequence   --- ... see below ...
                                        // - 4 = sequence PlayList --- ... see below ...
const byte IDXSTATUS   =   6;           // ... to be defined
const byte IDXERROR    =   7;           // ... to be defined
const byte IDXDATATYP  =   8;           // same as IDXREPLY ???!!!
const byte IDXDATALEN  =   9;           // length of one DATA-BLOCK
const byte IDXDATABLK  =  10;           // number of DATA-BLOCKs

byte   pulserate   = 0;   // 0-200, 0 = no repetition
byte   pulsewidth  = 0;   // 0-100, 0 = no pulse
byte   pulseintens = 0;   // 0-255, 0 = no intensity


Die Schleife:
Code: [Select]

void loop()
{
  CMD_check();
  delay(20);
}


SetUp:
Code: [Select]

void setup()
{
  // Setup input-pins:
 
  // Setup output-pins:

  noInterrupts();         

  // initialize timers & ISR

  interrupts();           

  Serial.begin(57600, SERIAL_8N1);       

  while (!Serial)               // also check timeout !
  { ;                // wait for serial port to connect. Needed for native USB port only
  }
  delay(10);

  // ---------- Send Modul-Info on start
  for (int i = 0; i < HEADERSIZE; i++) header[i] = 0;  // clear header
  header[IDXREPLY]     = 1;           // send Info
  CMD_echo();                   // return DATA: INFO
}



To young to die - never to old for rock'n roll

TERWI

#8
Apr 16, 2019, 07:24 am Last Edit: Apr 16, 2019, 07:27 am by TERWI
Prüfe Telegramm:
Code: [Select]

void CMD_check()
{
  boolean doread   = true;
  boolean isheader = false;
  boolean isdata   = false;
  boolean doecho   = false;
  byte    rb       = 0;         // received byte by Serial.read
  byte    idx      = 0;
  byte    blk      = 0;
  byte    len      = 0;
  int     datamax  = 0;

  if (Serial.available() < 14) return;           // enough bytes in queue ?
  for (int i = 0; i < HEADERSIZE; i++) header[i] = 0;  // clear header
  // ----- evaluate incoming bytes:
  while ((Serial.available() > 0) && doread)     // ... bytes available  and NOT header/data done ?
  {
    rb = Serial.read();                        // YES, read a byte ...
    switch (rb)
    {
      case MARKERSTART:               // ">" Start-marker, transmission begins
        isheader = true;              //
//        Serial.println("CMD_CKECK - StartByte OK, read HEADER....");
        continue;                     // ignore Marker, next byte in loop
        break;
      case MARKERDATA:                // "|" Header ending, start data ?!
        if ( (idx == header[IDXHEADLEN]) || !isheader )
        {
          isheader = false;             //
          doecho   = true;
          idx      = 0;
          datamax  = header[IDXDATALEN] * header[IDXDATABLK];
          if (datamax > 0) isdata  = true;
        }
//        Serial.println("CMD_CKECK - HEADER OK, read DATA....");
        continue;                     // ignore Marker, next byte in loop
        break;
      case MARKEREND:                 // "<" End-marker, transmisson finished
        doread   = false;             
        isheader = false;           
        isdata   = false;
        doecho   = true;
//        Serial.println("CMD_CKECK -> DATA OK, finished !");
//        continue;                   // ignore Marker, next byte in loop (and stop it !)
        break;
    }

    // ---------- read Header:
    if (isheader == true)
    {
/*
      Serial.print("-> read HDR: ");
      Serial.print(ndx);
      Serial.print(" - ");
      Serial.println(rb);
*/
      if (idx < HEADERSIZE)  // ??? && (idx < header[IDXHEADLEN])
      {
        header[idx] = rb;   // store header-byte
      }
      else
      {
        isheader = false;             
        if (header[IDXMODNUM] != DINO_MODUL) // this header is for us ?
        {                                    // ... NO !
          doread = false;                    // break the loop
          doecho = false;                    // no echo !
        }
      }
      idx++;
    } // end if (isheader) ...

    // ---------- read Data: (... if any)
    if (isdata == true)
    {
/*     
      Serial.print("-> read DAT: ");
      Serial.print(len);
      Serial.print(" - ");
      Serial.print(blk);
      Serial.print(" / ");
      Serial.println(rb);
*/     
      switch (header[IDXDATATYP])
      { case 1:     // pulse manual external
          if (len == 0) pulserate   = rb;        // 0-200, 0 = no repetition
          if (len == 1) pulsewidth  = rb;        // 0-100, 0 = no pulse
          if (len == 2) pulseintens = rb;        // 0-255, 0 = no intensity
          break;
        case 2:     // set single sequence
//
        case 3:     // set Playlist section
//
          break;
        case 4:     // new Sequence-Block
          break;
        case 5:     // new Sequence-Control
          break;
        case 6:     // new Sequence-PlayList
          break;
      }   
      idx = (len + 1) * (blk + 1);
      len++;
      if (len >= header[IDXDATALEN])
      { len = 0;
        blk++;
        if (blk >= header[IDXDATABLK])
        {
          isdata = false;
        }
      }
    } // end if (isdata) ...

  } // while

  while (Serial.available() > 0) Serial.read();  // clear RX-buffer

  if (doecho == true)
  {
    CMD_echo();                   // return DATA
  }
}


Sende Telegramm:
Code: [Select]

void CMD_echo()
{
  header[IDXHEADLEN]   = 11;          // idx = 0 - .... following data only
  header[IDXMODNUM]    = DINO_MODUL;  // idx = 1 - Modul-number to talk to
  header[IDXMODCHL]    = DINO_MAXCH;  // idx = 2 - Modul-Channel - supported (Std. 1, 2-4 MEGA only)
  header[IDXMODVOL]    = 100;         // idx = 3 - "Master-Volume" 0-100%
  header[IDXPLAY]      = 0;           // idx = 5 - STOP
  header[IDXSTATUS]    = 99;          // idx = 6 - ... to be defined
  header[IDXERROR]     = 88;          // idx = 7 - ... to be defined
  switch (header[IDXREPLY])           // idx = 4
  { case 1:                                        // - 1 = return + modul-info
      header[IDXDATATYP] = 1;
      header[IDXDATALEN] = 2 + sizeof(DINO_ID_STRING)-1;  // b2f - num of extra-bytes following
      header[IDXDATABLK] = 1;      // only 1 data-block of size = len
      break;
    case 2:                                        // - 2 = return + sequence-data
      header[IDXDATATYP] = 2;     
      header[IDXDATALEN] = 0;      // not yet defined
      header[IDXDATABLK] = 0;      // not yet defined
      break;
    case 3:                                        // - 2 = return + sequence-data
      header[IDXDATATYP] = 3;     
      header[IDXDATALEN] = 3;      // Puls-Rate, -Width, -Val
      header[IDXDATABLK] = 1;      // only 1 data-block of size = len
      break;
  }
  // ---------- send Header
  Serial.write(">>");                         // Marker HEADER BEGIN
  Serial.write(header, header[IDXHEADLEN]);   // the header itself
  Serial.write("||");                         // Marker HEADER END - DATA BEGIN
  // ---------- send Data
  switch (header[IDXREPLY])
  { case 1:                                        // - 1 = return + modul-info
      Serial.write(DINO_VERSION);                  // Version byte - HighNibble=Major, LowNibble=Minor
      Serial.write(DINO_MODUL);                    // Modul-Number - 0=Master, 1++=Slave(s)
      Serial.write(DINO_ID_STRING);                // DINO_ID_STRING for compare
      break;
    case 2:                                        // - 2 = return + sequence-data
/*
*/
      break;
    case 3:                                        // - 3 = return + actual external data (potis - if any)
      Serial.write(pulserate);                 
      Serial.write(pulsewidth);               
      Serial.write(pulseintens);               
      break;
    default:                                       // - 0 = return mode info only 
      break;
  }   
  Serial.write("<<");                         // Marker TRANSMISSION END
  Serial.flush();
}


Dann schaut mal durch. Hab nur das nötigste drin gelassen.
Basiert übrigends auf dieser Idee.
Problem ist wie gesagt, dass hier z.B. im zurückgsendeten Telegramm im Header offensichtlich "switch (header[IDXREPLY])" nicht richtig ausgewertet wird (alles 0 ?!).

Interessanterweise passiert das nicht regelmäßig, sondern sporadisch.
Vor allem wenn in CMD_Check auch nur eines der "Serial.println("CMD_CKECK" oder das delay(20) in der Loop weggelassen wird.
To young to die - never to old for rock'n roll

combie

#9
Apr 16, 2019, 07:37 am Last Edit: Apr 16, 2019, 07:39 am by combie
Quote
Problem ist wie gesagt, dass hier z.B. im zurückgsendeten Telegramm im Header offensichtlich "switch (header[IDXREPLY])" nicht richtig ausgewertet wird (alles 0 ?!).
Dann steht da nicht das richtige drin.


------------


Ein kleines wenig, fühle ich mich *******...

Da mache ich mir schon die Arbeit, erzeuge eine neues Projekt, und dann:
Der Code ist nicht testbar

Nirgendwo ist zu sehen, wo header[IDXREPLY] gefüllt wird.

Ich gebe auf....

Der Pessimist sieht die Wolke vor der Sonne.
Der Optimist sieht die Sonne hinter der Wolke.

Mantra: Die Sonne scheint immer!

Whandall

Basiert übrigends auf dieser Idee.
Das ist nicht so, es sei denn die Idee wird auf Start- und Endmarker reduziert.

Robin2s Kode enthält keine delays, Tests auf Tiefe des RX-Buffers und keine whiles.
Ah, this is obviously some strange usage of the word 'safe' that I wasn't previously aware of. (D.Adams)

TERWI

@ combie
.... das meinte ich mit der Komplexität.
Der zu empfangende Header wird vom PC aus gesendet und auch jeweils richtig gefüllt.
Wenn ich in CMD_Check nach " if (isheader == true)" die Ausmarkierung weglasse, sehe ich die Header-Daten die kommen und die sind dann richtig.

@ Whandall
Ich sagte ja auch "basiert".....
Und ein while hat der Robin sehr wohl darin - siehe Code #1.
Und betreff delay schau mal ab #36.

Hier geht es um die Frage:
Warum gehts das mit delay / div Serial.print und warum nicht ohne ?
To young to die - never to old for rock'n roll

Whandall

#12
Apr 16, 2019, 08:08 am Last Edit: Apr 16, 2019, 08:18 am by Whandall
Robin2 wartet vorher nicht auf dem Empfang der vollständigen Nachricht,
die Verwendung eines while statt eines ifs halte ich an der Stelle für didaktisch falsch,
dich hat es jedenfalls auf den Holzweg geführt.
Ein delay findet Firefox auf der ersten Seite des verlinkten Beitrags nicht.

Delays gehören nicht in ein Programm das asynchron kommunizieren soll.
Warte nicht auf einen Füllstand im RX-Puffer, verarbeite jedes Zeichen wenn es empfangen wird.

Den Array vor einem Empfang zu löschen ist völlig überflüssig, wenn du einen Terminator brauchst,
hänge den nach der Nachricht an.
Auch das Löschen des Puffers nach einer Nachricht erscheint mir kontraproduktiv,
ich denke da verschluckst du die Daten.

[OT] Ich empfinde Start- und Endmaker als unnötig kompliziert, ein Endmarker reicht (mir) völlig. [/OT]
Ah, this is obviously some strange usage of the word 'safe' that I wasn't previously aware of. (D.Adams)

TERWI

#13
Apr 16, 2019, 08:25 am Last Edit: Apr 16, 2019, 08:30 am by TERWI
@ Whandall
Mal richtig lesen: while in Post #1 auf Seite 1 und delay in/ab Post #36 auf Seite 4.
Mir scheint, meinen Code hast du auch noch nicht wirklich gelesen.

Ich lese byte für byte so bald etwas vorhanden ist. while ... Serial.read ....
Das löschen des Header-Arrays hat den Sinn, definierte Daten zu haben, wenn z.B. eine unvollständige Nachricht ankommt.
In Kombination eben mit einen Start- und End-Marker (hier zusätzlich ein Marker, welcher das Ende des Headers angibt) soll zusätzlich definiert den Begin und das Ende von erwünschten Daten anzeigen - egal was da ggf. für ein Datensalat ankommt.

Das Löschen des RX-Puffers ist nur "safety" und optional. Könnte man auch weglassen ....
Mir ist Anfangs nach einem Fehler aufgefallen, dass wenn z.B. der Endmarker nicht richtig gelesen (und gelöscht) wurde, dieser ja im Puffer verbleibt und dann beim nächsten Schwung Daten (> 14 Bytes !) für "Verwirrung" sorgt.
Das verschluckt hier m.E.n. keinerlei Daten, da schon alle (insbesondere Header) gelesen wurden.

Recht gebe ich dir, dass ein delay in Ser.-Kommunikation normalerweise nix zu suchen hat.
Deshalb frage ich ja auch nach einem eventuellem Grund für das eigenartige Verhalten - ohne delay.....

PS: Alle Marker-Werte können logo auch in möglichen Daten vorkommen. Die Prüfung, ob ein Endmarker auch an der erwarteten Position kommt, fehlt noch.
Zu dem haben die Marker (hier) den Sinn, zu prüfen ob die Übertragung auch wirklich korrekt ist.
Böse Zungen behaupten, ein CRC täte es auch .... :-)
To young to die - never to old for rock'n roll

Whandall

Ich finde imer toll dass andere besser wissen was ich gelesen und verstanden habe.

Das verschluckt hier m.E.n. keinerlei Daten, da schon alle (insbesondere Header) gelesen wurden.
Code: [Select]
  while (Serial.available() > 0) Serial.read();  // clear RX-buffer

Da du ja alles besser weisst wünsche ich dir viel Erfolg bei deinem Projekt,
dafür ist mir meine Zeit zu schade.
Ah, this is obviously some strange usage of the word 'safe' that I wasn't previously aware of. (D.Adams)

Go Up