Serielle Schnittstelle Empfangsbuffer

[FlorianKr]

Hallo Community,

ich bin gerade dabei einen GPS-Empfänger über die Serielle schnittstelle auszulesen.

Immer wenn an der seriellen Schnitstelle Daten ankommen wird ein Event ausgelöst und die Schnittstelle abgefragt.
Das Problem ist hierbei jetzt, dass zwar Daten ankommen, aber die neuen Daten immer nur an die alten Daten angehängt werden.
Die alten Daten werden nicht aus dem Buffer gelöscht.

Im angehängten Bild ist die Consolen-Ausgabe zu sehen.
Immer bei Characters Recieved kommt ein neuer Datensatz.
Die erste Zahl nach GPGGA ist die Uhrzeit, die daten kommen alle 200ms.

Ich habe es mit
PurgeComm(hComm, PURGE_RXCLEAR)
versucht was allerdings nicht funktioniert hat.

Hat jemand sonst noch eine Idee?

MfG Florian

31542

[Peter(TOO)]

Hallo Florian,

Tja, etwas mager was du da als Code offenbarst!
Wie wäre es mit dem ganzen Code den du geschrieben hast? Dann kann man sehen wo du den Fehler machst.

MfG Peter(TOO)

[FlorianKr]

Hallo Peter(TOO),

sorry das habe ich gestern leider in meinem Frust vergessen.


BOOL CommSeriell::initComPort()
{
printf("\n\n +==========================================+");
printf("\n | Serial Port Reception (Win32 API) |");
printf("\n +==========================================+\n");
/*---------------------------------- Opening the Serial Port -------------------------------------------*/

TCHAR *ComPortName = TEXT("COM5");
hComm = CreateFile(ComPortName, // Name of the Port to be Opened
GENERIC_READ | GENERIC_WRITE, // Read/Write Access
0, // No Sharing, ports cant be shared
NULL, // No Security
OPEN_EXISTING, // Open existing port only
0, // Non Overlapped I/O
NULL); // Null for Comm Devices

if (hComm == INVALID_HANDLE_VALUE)
std::wcout<<"\n Error! - Port "<< ComPortName <<" can't be opened\n";
else
std::wcout <<"\n Port "<<ComPortName<<" Opened\n ";

/*------------------------------- Setting the Parameters for the SerialPort ------------------------------*/

DCB dcbSerialParams = { 0 }; // Initializing DCB structure
dcbSerialParams.DCBlength = sizeof(dcbSerialParams);

Status = GetCommState(hComm, &dcbSerialParams); //retreives the current settings

if (Status == FALSE){
printf("\n Error! in GetCommState()");
error = 1;
}
dcbSerialParams.BaudRate = CBR_19200; // Setting BaudRate = 9600
dcbSerialParams.ByteSize = 8; // Setting ByteSize = 8
dcbSerialParams.StopBits = ONESTOPBIT; // Setting StopBits = 1
dcbSerialParams.Parity = NOPARITY; // Setting Parity = None

Status = SetCommState(hComm, &dcbSerialParams); //Configuring the port according to settings in DCB

if (Status == FALSE)
{
printf("\n Error! in Setting DCB Structure");
error = 1;
}
else //If Successfull display the contents of the DCB Structure
{
printf("\n\n Setting DCB Structure Successfull\n");
printf("\n Baudrate = %d", dcbSerialParams.BaudRate);
printf("\n ByteSize = %d", dcbSerialParams.ByteSize);
printf("\n StopBits = %d", dcbSerialParams.StopBits);
printf("\n Parity = %d", dcbSerialParams.Parity);
}

/*------------------------------------ Setting Timeouts --------------------------------------------------*/

COMMTIMEOUTS timeouts = { 0 };
timeouts.ReadIntervalTimeout = 50;
timeouts.ReadTotalTimeoutConstant = 50;
timeouts.ReadTotalTimeoutMultiplier = 10;
timeouts.WriteTotalTimeoutConstant = 50;
timeouts.WriteTotalTimeoutMultiplier = 10;

if (SetCommTimeouts(hComm, &timeouts) == FALSE)
printf("\n\n Error! in Setting Time Outs");
else
printf("\n\n Setting Serial Port Timeouts Successfull");

return Status;
}

BOOL CommSeriell::setRecieveMask() {
/*------------------------------------ Setting Receive Mask ----------------------------------------------*/

Status = SetCommMask(hComm, EV_RXCHAR); //Configure Windows to Monitor the serial device for Character Reception

if (Status == FALSE)
printf("\n\n Error! in Setting CommMask");
else
printf("\n\n Setting CommMask successfull");
return Status;
}

BOOL CommSeriell::recieveComPort(){
/*------------------------------------ Setting WaitComm() Event ----------------------------------------*/

printf("\n\n Waiting for Data Reception");

do {

Status = WaitCommEvent(hComm, &dwEventMask, NULL); //Wait for the character to be received


/*-------------------------- Program will Wait here till a Character is received ------------------------*/

if (Status == FALSE)
{
printf("\n Error! in Setting WaitCommEvent()");
error = 10;
}
else //If WaitCommEvent()==True Read the RXed data using ReadFile();
{
printf("\n\n Characters Received");
do
{
Status = ReadFile(hComm, &TempChar, sizeof(TempChar), &NoBytesRead, NULL);
SerialBuffer[i] = TempChar;
i++;
} while (NoBytesRead > 0);


/*------------Printing the RXed String to Console----------------------*/

printf("\n\n ");
int j = 0;
for (j = 0; j < i - 1; j++) // j < i-1 to remove the dupliated last character
printf("%c", SerialBuffer[j]);

//PurgeComm(hComm, PURGE_RXCLEAR); //lehren des Empfangsbuffer

}
error++; //nur 10 mal ausführen
} while (error <10);
return Status;
}

void CommSeriell::theThread()
{
initComPort(); //Initialisieren des ComPort
setRecieveMask(); //Event setzen
recieveComPort(); //In einer Dauerschleife auf Nachrichten warten

CloseHandle(hComm);//Closing the Serial Port
printf("\n +==========================================+\n");
};


Die Funktion "theThread" wird als eigener Thread gestartet, der die Funktionen nacheinander aufruft.
Der Inhalt der Funktion "recieveComPort" läuft eigentlich in einer Dauerschleife bis ein Fehler auftritt.
Dies ist aktuell zum Testen allerdings auf 10 durchläufe begrenzt.

MfG Florian

[Mxt]

Hallo,

ich will Peter nicht vorgreifen, vielleicht kennt er sich da besser aus. Das Problem ist, dass die Windows Funktionen für die serielle Schnittstelle so vielfältig sind, dass man sein Programm sehr unterschiedlich aufbauen kann.

Ist der Code irgendwie zusammenkopiert oder selber geschrieben ?

Das zeichenweise Lesen über ReadFile ist sehr ineffizient. Nicht umsonst hat ReadFile einen Parameter, der angibt wie viele Zeichen gelesen wurden. Wenn man schon die COMMTIMEOUTS setzt, kann man sie auch gleich so setzen, dass ReadFile mit dem zurückkommt, was im Eingangspuffer steht.

Ein funktionsfähiges Beispiel für Eventbasiertes Lesen ohne asynchrones IO habe ich mit Google auf die Schnelle nicht gefunden. Ich hätte erwartet, dass man die Schnittstelle dann mit Overlapped öffnet, so wie hier
http://www.codeguru.com/cpp/i-n/network/serialcommunications/article.php/c5425/Serial-Communication-in-Windows.htm
Das mache ich normalerweise nicht, ist mir zu kompliziert. Sollte ich das mal brauchen, würde ich wohl eine fertige Klasse dafür nehmen, so wie die hier
http://www.naughter.com/serialport.html

Wenn man schon in einem Thread arbeitet, das mache ich bei seriellen Schnittstellen normalerweise auch, dann kann man da auch einfach pollen, ob was da ist, sonst eine Weile schlafen. Ist was da, liest man alles was da ist und nimmt das dann auseinander.

Ungefähr so


//
// Anzahl der Bytes im Eingangspuffer
//
DWORD SensorInterface::InputSize()
{
DWORD PortErrors;
COMSTAT PortStatus;

if ( ClearCommError(m_hComm, &PortErrors, &PortStatus) )
{
return PortStatus.cbInQue;
}
else
{
return 0;
}
}

und so


std::string SensorInterface::Receive()
{
if ( !m_bOpen )
{
throw std::runtime_error("port not open");
}

const int nReadSize = 128;
std::vector<char> Buffer(nReadSize);
unsigned long nDataRead;

if ( !ReadFile( m_hComm, &Buffer[0], nReadSize, &nDataRead, NULL) )
{
throw std::runtime_error("General Read Error");
}

return std::string(Buffer.begin(), Buffer.begin() + nDataRead);
}

[FlorianKr]

Hallo Mxt,

den Code hatte ich von folgender Seite: http://xanthium.in/Serial-Port-Programming-using-Win32-API

Ich habe deine Funktion "SensorInterface::Receive()" einfach mal per Copy&Paste bei mir eingefügt und es funktioniert so wie ich mir es erhofft hatte.

Die Daten kommen mit 5 Hertz an. Wenn ich polle mit 10 Hertz kommen die Daten so an wie ich sie gern hätte.
Mit dem WaitCommEvent ganau so. Was sind hier die Vorteile?

Das mit den TimeOuts habe ich noch nicht ganz verstanden. Habe sie einfach mal auskommentiert und das Ergebnis war jeweils das gleiche.

Anbei mal mein aktueller Code:

#include "stdafx.h"
#include "CommSeriell.h"


CommSeriell::CommSeriell(int a)
{
m_run.store(true);
m_thread = std::thread(std::bind(&CommSeriell::theThread, this));
}


CommSeriell::~CommSeriell()
{
m_run.store(false);
if (m_thread.joinable())
{
m_thread.join();
}
}

BOOL CommSeriell::initComPort()
{
printf("\n\n +==========================================+");
printf("\n | Serial Port Reception (Win32 API) |");
printf("\n +==========================================+\n");
/*---------------------------------- Opening the Serial Port -------------------------------------------*/

ComPortName = TEXT("COM5");
hComm = CreateFile(ComPortName, // Name of the Port to be Opened
GENERIC_READ | GENERIC_WRITE, // Read/Write Access
0, // No Sharing, ports cant be shared
NULL, // No Security
OPEN_EXISTING, // Open existing port only
0, // Non Overlapped I/O
NULL); // Null for Comm Devices

if (hComm == INVALID_HANDLE_VALUE)
std::wcout<<"\n Error! - Port "<< ComPortName <<" can't be opened\n";
else
std::wcout <<"\n Port "<<ComPortName<<" Opened\n ";

/*------------------------------- Setting the Parameters for the SerialPort ------------------------------*/

DCB dcbSerialParams = { 0 }; // Initializing DCB structure
dcbSerialParams.DCBlength = sizeof(dcbSerialParams);

Status = GetCommState(hComm, &dcbSerialParams); //retreives the current settings

if (Status == FALSE){
printf("\n Error! in GetCommState()");
error = 1;
}
dcbSerialParams.BaudRate = CBR_19200; // Setting BaudRate = 9600
dcbSerialParams.ByteSize = 8; // Setting ByteSize = 8
dcbSerialParams.StopBits = ONESTOPBIT; // Setting StopBits = 1
dcbSerialParams.Parity = NOPARITY; // Setting Parity = None

Status = SetCommState(hComm, &dcbSerialParams); //Configuring the port according to settings in DCB

if (Status == FALSE)
{
printf("\n Error! in Setting DCB Structure");
error = 1;
}
else //If Successfull display the contents of the DCB Structure
{
printf("\n\n Setting DCB Structure Successfull\n");
printf("\n Baudrate = %d", dcbSerialParams.BaudRate);
printf("\n ByteSize = %d", dcbSerialParams.ByteSize);
printf("\n StopBits = %d", dcbSerialParams.StopBits);
printf("\n Parity = %d", dcbSerialParams.Parity);
}

/*------------------------------------ Setting Timeouts --------------------------------------------------*/

COMMTIMEOUTS timeouts = { 0 };
timeouts.ReadIntervalTimeout = 50;
timeouts.ReadTotalTimeoutConstant = 50;
timeouts.ReadTotalTimeoutMultiplier = 10;
timeouts.WriteTotalTimeoutConstant = 50;
timeouts.WriteTotalTimeoutMultiplier = 10;

if (SetCommTimeouts(hComm, &timeouts) == FALSE)
printf("\n\n Error! in Setting Time Outs");
else
printf("\n\n Setting Serial Port Timeouts Successfull");

return Status;
}

BOOL CommSeriell::setRecieveMask() {
/*------------------------------------ Setting Receive Mask ----------------------------------------------*/

Status = SetCommMask(hComm, EV_RXCHAR); //Configure Windows to Monitor the serial device for Character Reception

if (Status == FALSE)
printf("\n\n Error! in Setting CommMask");
else
printf("\n\n Setting CommMask successfull");
return Status;
}

BOOL CommSeriell::recieveComPort(){
/*------------------------------------ Setting WaitComm() Event ----------------------------------------*/

printf("\n\n Waiting for Data Reception");

do {

Status = WaitCommEvent(hComm, &dwEventMask, NULL); //Wait for the character to be received

/*-------------------------- Program will Wait here till a Character is received ------------------------*/

if (Status == FALSE)
{
printf("\n Error! in Setting WaitCommEvent()");
error = 10;
}
else //If WaitCommEvent()==True Read the RXed data using ReadFile();
{
readbuf = Receive();
std::cout << "\nPrint input\n";
std::cout << readbuf << std::endl;
}
error++; //nur 10 mal ausführen
} while (error <10);

CloseHandle(hComm);//Closing the Serial Port
return Status;
}
BOOL CommSeriell::readNMEA(){
return Status;
}


void CommSeriell::theThread()
{
initComPort();
setRecieveMask();

recieveComPort();


printf("\n +==========================================+\n");
};



std::string CommSeriell::Receive()
{
const int nReadSize = 256;
std::vector<char> Buffer(nReadSize);
unsigned long nDataRead;

if (!ReadFile(hComm, &Buffer[0], nReadSize, &nDataRead, NULL))
{
std::cout<<"General Read Error";
}

return std::string(Buffer.begin(), Buffer.begin() + nDataRead);
}

//DWORD CommSeriell::InputSize()
//{
// DWORD PortErrors;
// COMSTAT PortStatus;
//
// if (ClearCommError(hComm, &PortErrors, &PortStatus))
// {
// return PortStatus.cbInQue;;
//}
//else
// return 0;
//}

Kann ich das so stehen lassen oder habt ihr noch Tipps zur optimierung?

Danke für die schnelle Hilfe
Florian

[Mxt]

Mit dem WaitCommEvent ganau so. Was sind hier die Vorteile?

Wenn das mit dem Event geht, dann nimm das Event. Das spart etwas CPU-Zeit, allerdings fällt serielle Kommunikation auf heutigen PCs nicht mehr wirklich ins Gewicht.



Das mit den TimeOuts habe ich noch nicht ganz verstanden. Habe sie einfach mal auskommentiert und das Ergebnis war jeweils das gleiche.

Wahrscheinlich müsste man die Zeiten deutlich reduzieren, um was zu merken.

Ganz grob gesagt, kommt ReadFile bei einer seriellen Schnittstelle zurück, wenn
- die zu lesende Anzahl der Zeichen gelesen wurde
- die Zeit ReadTotalTimeoutConstant + Anzahl zu lesender Zeichen * ReadTotalTimeoutMultiplier um ist (grob)
- nach dem letzten empfangenen Zeichen seit ReadIntervalTimeout nichts mehr gekommen ist
- im Overlapped Modus immer sofort (mit 0 Zeichen), das eigentliche Lesen erfolgt dann später mit GetOverlappedResult



Kann ich das so stehen lassen oder habt ihr noch Tipps zur optimierung?

Naja, ist halt ein gewisses Sammelsurium ;) Aber das würde jetzt zu lange dauern ...

[Peter(TOO)]

Hallo Florian,


/*------------Printing the RXed String to Console----------------------*/

printf("\n\n ");
int j = 0;
for (j = 0; j < i - 1; j++) // j < i-1 to remove the dupliated last character
printf("%c", SerialBuffer[j]);


Hier fehlt

i = 0;

Nach der Ausgabe der Daten.

Grundsätzlich: Wieso gibt man die ganzen Variablen-Deklarationen nicht auch an?
Da können schon viele Fehler entstehen, z.B. durch signed/unsigned.

Dein Code ist aber eher etwas kriminell!

SerialBuffer[i] = TempChar;
i++;

Hier überprüfst du nicht ob i grösser als der Buffer ist!
Kann gut gehen, aber wenn da mal eine Störung bei der Übertragung ist überschreibst du alles was hinter SerialBuffer im Speicher liegt...

Übrigens war genau dieser Fehler bei MS schuld daran, dass SASSER sich verbreiten konnte.
Hinter deren Buffer lag irgendwo Code und das Protokoll war festgelegt auf maximal 4kB pro Paket. Der Hacker hat dann einfach ein zu grosses Datenpaket verschickt, welches dann hinten seinen Code enthalten hat. Somit wurde dann das normale Programm überschrieben und bei einem Aufruf der normalen Funktionen wurde der Code des Hackers ausgeführt! Braucht nur etwas Analyse, damit die Einsprungspunkte der Hackerroutinen an die Stellen der originalen Einsprungspunkte zu liegen kommen.

MfG Peter(TOO)

[FlorianKr]

Hallo,

vielen Dank für die Hinweise und Erklärungen.

Ich habe jetzt versucht den Code etwas aufzuräumen:



//CommSeriell.cpp

#include "stdafx.h"
#include "CommSeriell.h"


CommSeriell::CommSeriell(int a)
{
m_run.store(true);
m_thread = std::thread(std::bind(&CommSeriell::theThread, this));
}


CommSeriell::~CommSeriell()
{
m_run.store(false);
if (m_thread.joinable())
{
m_thread.join();
}
}

BOOL CommSeriell::initComPort()
{
std::cout <<std::endl<<"Serial Port Reception (Win32 API)"<< std::endl;

/*---------------------------------- Opening the Serial Port -------------------------------------------*/

ComPortName = TEXT("COM2");
hComm = CreateFile(ComPortName, // Name of the Port to be Opened
GENERIC_READ | GENERIC_WRITE, // Read/Write Access
0, // No Sharing, ports cant be shared
NULL, // No Security
OPEN_EXISTING, // Open existing port only
0, // Non Overlapped I/O
NULL); // Null for Comm Devices

if (hComm == INVALID_HANDLE_VALUE)
std::wcout <<"\n Error! - Port "<<ComPortName<<" can't be opened\n";
else
std::wcout <<"\n Port "<<ComPortName<<" Opened\n ";

/*------------------------------- Setting the Parameters for the SerialPort ------------------------------*/

DCB dcbSerialParams = { 0 }; // Initializing DCB structure
dcbSerialParams.DCBlength = sizeof(dcbSerialParams);

Status = GetCommState(hComm, &dcbSerialParams); //retreives the current settings

if (Status == FALSE){
std::cout <<"\n Error! in GetCommState()";
error = 10;
}
dcbSerialParams.BaudRate = CBR_19200; // Setting BaudRate = 9600
dcbSerialParams.ByteSize = 8; // Setting ByteSize = 8
dcbSerialParams.StopBits = ONESTOPBIT; // Setting StopBits = 1
dcbSerialParams.Parity = NOPARITY; // Setting Parity = None

Status = SetCommState(hComm, &dcbSerialParams); //Configuring the port according to settings in DCB

if (Status == FALSE)
{
std::cout <<std::endl<<"\n Error! in Setting DCB Structure"<<std::endl;
error = 1;
}
else //If Successfull display the contents of the DCB Structure
{
std::cout << "\n\n Setting DCB Structure Successfull" << std::endl
<<"\n Baudrate = " << dcbSerialParams.BaudRate
<<"\n ByteSize = %d" << dcbSerialParams.ByteSize
<<"\n StopBits = %d" << dcbSerialParams.StopBits
<<"\n Parity = %d" << dcbSerialParams.Parity;
}
return Status;
}

BOOL CommSeriell::setRecieveMask() {
/*------------------------------------ Setting Receive Mask ----------------------------------------------*/

Status = SetCommMask(hComm, EV_RXCHAR); //Configure Windows to Monitor the serial device for Character Reception

if (Status == FALSE)
std::cout << "\n\n Error! in Setting CommMask";
else
std::cout << "\n\n Setting CommMask successfull";
return Status;
}

BOOL CommSeriell::recieveComPort(){
/*------------------------------------ Setting WaitComm() Event ----------------------------------------*/

std::cout <<"\n\n Waiting for Data Reception";

do {

Status = WaitCommEvent(hComm, &dwEventMask, NULL); //Wait for the character to be received

/*-------------------------- Program will Wait here till a Character is received ------------------------*/

if (Status == FALSE)
{
std::cout <<"\n Error! in Setting WaitCommEvent()";
error = 10;
}
else //If WaitCommEvent()==True Read the RXed data using ReadFile();
{
readbuf = ReceiveData();
std::cout <<"\n Print input\n";
std::cout << readbuf << std::endl;
}
error++; //nur 10 mal ausführen
} while (error <10);
return Status;
}

std::string CommSeriell::ReceiveData()
{
const int nReadSize = 256;
std::vector<char> Buffer(nReadSize);
unsigned long nDataRead;

if (!ReadFile(hComm, &Buffer[0], nReadSize, &nDataRead, NULL))
{
std::cout<<"General Read Error";
}

return std::string(Buffer.begin(), Buffer.begin() + nDataRead);
}

void CommSeriell::theThread()
{
initComPort();
setRecieveMask();
recieveComPort();

CloseHandle(hComm);//Closing the Serial Port
printf("\n +==========================================+\n");
}




// CommSeriell.h

#pragma once
class CommSeriell
{
public:
CommSeriell(int a);
~CommSeriell();

private:
std::atomic<bool> m_run;
std::thread m_thread;

//Variablendeklarationen
HANDLE hComm; // Handle to the Serial port
TCHAR *ComPortName; // Name of the Serial port(May Change) to be opened,
BOOL Status; // Status of the various operations
DWORD dwEventMask; // Event mask to trigger
int error = 0; //
std::string readbuf; //

//Funktiosdeklarationen
void theThread();

BOOL initComPort();
BOOL setRecieveMask();
BOOL recieveComPort();
std::string ReceiveData();
};


Anbei auch die Variablen-Deklarationen.

Die von dir einen Thread vorher genannten Punkte habe ich hoffentlich soweit alle herausgenommen.

Anbei noch ein Bild wie die Daten jetzt ausgegeben werden.

MfG Florian

31549