Come posso migliorare questo terribile codice della porta seriale?

Question

Ho un brutto pezzo di codice di porta seriale che è molto instabile.

void port_DataReceived(object sender, SerialDataReceivedEventArgs e)
{
    Thread.Sleep(100);
    while (port.BytesToRead > 0)
    {
        var count = port.BytesToRead;

        byte[] buffer = new byte[count];

        var read = port.Read(buffer, 0, count);

        if (DataEncapsulator != null)
            buffer = DataEncapsulator.UnWrap(buffer);


       var response = dataCollector.Collect(buffer);

       if (response != null)
       {
           this.OnDataReceived(response);
       }

       Thread.Sleep(100);
    }    
}

Se rimuovo una delle chiamate Thread.Sleep (100), il codice smette di funzionare.

Naturalmente questo rallenta davvero le cose e se molti flussi di dati entrano, smette anche di funzionare a meno che non renda il sonno ancora più grande. (Smette di funzionare come in deadlock puro)

Nota: DataEncapsulator e DataCollector sono componenti fornito da MEF, ma le loro prestazioni sono abbastanza buone.

La classe ha un metodo Listen () che avvia un lavoratore in background ricevere dati.

public void Listen(IDataCollector dataCollector)
{
    this.dataCollector = dataCollector;
    BackgroundWorker worker = new BackgroundWorker();

    worker.DoWork += new DoWorkEventHandler(worker_DoWork);
    worker.RunWorkerAsync();
}

void worker_DoWork(object sender, DoWorkEventArgs e)
{
    port = new SerialPort();

    //Event handlers
    port.ReceivedBytesThreshold = 15;
    port.DataReceived += new SerialDataReceivedEventHandler(port_DataReceived);

    ..... remainder of code ...

I suggerimenti sono benvenuti!

Aggiornamento: * Solo una breve nota di ciò che fanno le classi IDataCollector. Non c'è modo di sapere se tutti i byte dei dati che sono stati inviati vengono letti in un'unica operazione di lettura. Quindi ogni volta che vengono letti i dati lo è passato a DataColllector che restituisce true quando completo e è stato ricevuto un messaggio di protocollo valido. In questo caso qui solo controlla un byte di sincronizzazione, lunghezza, crc e byte di coda. Il vero lavoro viene fatto successivamente da altre classi. *

Aggiornamento 2: Ho sostituito il codice ora come suggerito, ma c'è ancora qualcosa che non va:

void port_DataReceived(object sender, SerialDataReceivedEventArgs e)
{
        var count = port.BytesToRead;

        byte[] buffer = new byte[count];

        var read = port.Read(buffer, 0, count);

        if (DataEncapsulator != null)
            buffer = DataEncapsulator.UnWrap(buffer);

        var response = dataCollector.Collect(buffer);

        if (response != null)
        {
            this.OnDataReceived(response);
        }     
}

Vedi che funziona benissimo con una connessione veloce e stabile. Ma OnDataReceived NON viene chiamato ogni volta che vengono ricevuti i dati. (Vedi i documenti MSDN per ulteriori informazioni). Quindi se i dati vengono frammentati e hai letto solo una volta all'interno dell'evento i dati vengono persi.

E ora ricordo perché avevo il loop in primo luogo, perché in realtà deve leggere più volte se la connessione è lenta o instabile.

Ovviamente non posso tornare alla soluzione del ciclo while, quindi cosa posso fare?

Answer 1

La mia prima preoccupazione per il frammento di codice originale basato su while è l'allocazione costante di memoria per il buffer di byte. Mettere un "nuovo" dichiarazione qui specificatamente andando al gestore della memoria .NET per allocare memoria per il buffer, prendendo la memoria allocata nell'ultima iterazione e rinviandola nel pool inutilizzato per l'eventuale garbage collection. Sembra un lavoro terribile da svolgere in un ciclo relativamente ristretto.

Sono curioso del miglioramento delle prestazioni che otterresti creando questo buffer in fase di progettazione con una dimensione ragionevole, diciamo 8K, quindi non hai tutta questa allocazione di memoria, deallocazione e frammentazione. Sarebbe di aiuto?

private byte[] buffer = new byte[8192];

void port_DataReceived(object sender, SerialDataReceivedEventArgs e)
{
    Thread.Sleep(100);
    while (port.BytesToRead > 0)
    {
        var count = port.BytesToRead;

        var read = port.Read(buffer, 0, count);

        // ... more code   
    }
}

L'altra mia preoccupazione riguardo alla riassegnazione di questo buffer su ogni iterazione del ciclo è che la riallocazione potrebbe non essere necessaria se il buffer è già abbastanza grande. Considera quanto segue:

• Iterazione loop 1: 100 byte ricevuti; allocare buffer di 100 byte
• Iterazione loop 2: 75 byte ricevuti; allocare buffer di 75 byte

In questo scenario, non è necessario riassegnare il buffer, poiché il buffer di 100 byte allocati nell'Iterazione 1 del ciclo è più che sufficiente per gestire i 75 byte ricevuti nell'Iterazione 2 del ciclo. Non è necessario per distruggere il buffer da 100 byte e creare un buffer da 75 byte. (Questo è discutibile, ovviamente, se si crea staticamente il buffer e lo si sposta completamente fuori dal ciclo.)

In un'altra tangente, potrei suggerire che il loop DataReceived si occupa solo della ricezione dei dati. Non sono sicuro di cosa stiano facendo quei componenti MEF, ma mi chiedo se il loro lavoro debba essere svolto nel ciclo di ricezione dei dati. È possibile che i dati ricevuti vengano inseriti in una sorta di coda e i componenti MEF possano prenderli lì? Sono interessato a mantenere il loop DataReceived il più veloce possibile. Forse i dati ricevuti possono essere messi in coda in modo che possano tornare al lavoro ricevendo più dati. È possibile impostare un altro thread, forse, per controllare i dati in arrivo sulla coda e fare in modo che i componenti MEF raccolgano i dati da lì e facciano il loro lavoro da lì. Potrebbe essere più codifica, ma può aiutare il loop di ricezione dei dati a essere il più reattivo possibile.

Answer 2

E può essere così semplice ...

O usi il gestore DataReceived ma senza un ciclo e certamente senza Sleep (), leggi quali dati sono pronti e spingili da qualche parte (in una coda o MemoryStream),

o

Avvia un thread (BgWorker) ed esegui (bloccando) serialPort1.Read (...) e, di nuovo, spingi o assembla i dati che ottieni.

Modifica:

Da quello che hai pubblicato direi: rilasciare il gestore eventi e basta leggere i byte all'interno di Dowork (). Questo ha il vantaggio che puoi specificare quanti dati vuoi, purché siano (molto) più piccoli di ReadBufferSize.

Modifica2, per quanto riguarda l'aggiornamento2:

Sarai ancora molto meglio con un ciclo while all'interno di un BgWorker, non usando affatto l'evento. Il modo semplice:

byte[] buffer = new byte[128];  // 128 = (average) size of a record
while(port.IsOpen && ! worker.CancelationPending)
{
   int count = port.Read(buffer, 0, 128);
   // proccess count bytes

}

Ora forse i tuoi record sono di dimensioni variabili e non vuoi aspettare che arrivino i successivi 126 byte per completarne uno. È possibile ottimizzare ciò riducendo la dimensione del buffer o impostare un ReadTimeOut. Per ottenere una grana molto fine è possibile utilizzare port.ReadByte (). Dal momento che legge dal ReadBuffer non è affatto più lento.

Answer 3

Se vuoi scrivere i dati in un file e la porta seriale si ferma ogni tanto, questo è un modo semplice per farlo. Se possibile, rendere il buffer sufficientemente grande da contenere tutti i byte che si prevede di inserire in un singolo file. Quindi scrivi il codice nel gestore dell'evento non ricevuto come mostrato di seguito. Quindi, quando si ottiene un'opportunità, scrivere l'intero buffer in un file come mostrato di seguito. Se devi leggere FROM dal tuo buffer mentre la porta seriale sta leggendo TO nel tuo buffer, prova a utilizzare un oggetto stream bufferizzato per evitare deadlock e condizioni di competizione.

private byte[] buffer = new byte[8192]; 
var index = 0;
void port_DataReceived(object sender, SerialDataReceivedEventArgs e)
{    
    index += port.Read(buffer, index, port.BytesToRead);
} 

void WriteDataToFile()
{
    binaryWriter.Write(buffer, 0, index); 
    index = 0;
}