[C# .net 4] Serial port communication

[Snuffel]

Beste allemaal,

Onderstaande post zal worden gemaakt in het Engels. Het heeft betrekking op een stuk programmeer code waar mijn vriendin momenteel mee bezig is, en helaas maar waar is zij de Nederlandse taal niet machtig genoeg om in het Nederlands te posten. Vandaar dus de post in het Engels.
Het staat je vrij om in het Nederlands te reageren, Google translate ftw zeggen we dan, en anders ben ik er uiteraard om enige zaken voor haar te vertalen.

En dan nu de probleemstelling:

What the program should do:

Commumnicate between a PC and a peripheral device through a serial port sending commands:
"M1"
"M0"
"MC"
"MT"
"MS"
"PD"
"CL"
(strings, no CR)
and receive and display response to some of the commands (all except M1 and M0).

First the connection is set up, then after sending "M1" (buttonXtend) every 1s a parallel thread should querry the device for some parameters. The device responds with parameters and the program needs to display them. This stops after sending "M0" (buttonStop).

What the program does:

Sending the commands to the device is working. The device responds to them properly (tested with another program). However, my program does not read from the port properly and after some time 'error: timeout of write operation' occurs. The clock_Elapsed method is called once.

That's my code:

C#:

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Windows.Forms;
using System.IO.Ports;
using System.Threading;
using System.Timers;

namespace Endoprosthesis
{
    public partial class Form1 : Form
    {
        //% allocate components
        private static SerialPort port;
        System.Timers.Timer clock;
        delegate void SetTextCallback(string text, Label field);

        public Form1()
        {
            InitializeComponent();

            //% create an instance of serial port
            port = new SerialPort();
              
            //%timer to prompt for status updates every second
            clock = new System.Timers.Timer(1000);
            //% setup not to stop counting after first occurance
            clock.AutoReset = true;
            //% making sure the garbage collector will not kill it too soon
            GC.KeepAlive(clock);

            //% handling for the event of elapsed second
            clock.Elapsed += new ElapsedEventHandler(clock_Elapsed);

        }

        void clock_Elapsed(object sender, ElapsedEventArgs e)
        {
            string value;

            port.Write("CL");
            System.Threading.Thread.Sleep(20);
            value = port.ReadExisting();
            setText(value, length);

            port.Write("MT");
            System.Threading.Thread.Sleep(20);
            value = port.ReadExisting();
            setText(value, temperature);

            port.Write("MC");
            System.Threading.Thread.Sleep(20);
            value = port.ReadExisting();
            setText(value, current);

            port.Write("MS");
            System.Threading.Thread.Sleep(20);
            value = port.ReadExisting();
            setText(value, speed);
        }

        private void buttonXtend_Click(object sender, EventArgs e)
        {
            port.Write("M1");

            clock.Start();

          }

        private void buttonStop_Click(object sender, EventArgs e)
        {
            port.Write("M0");

            clock.Stop();
        }

        private void setText(string text, Label field)
        {
            if (field.InvokeRequired)
            {
                SetTextCallback d = new SetTextCallback(setText);
                this.Invoke(d, new object[] { text, field });
            }
            else
            {
                field.Text = text;
            }
        }

        private void buttonClose_Click(object sender, EventArgs e)
        {
            clock.Stop();
            clock.Dispose();
            if(port.IsOpen) port.Close();
            this.Close();
        }

        private void buttonClear_Click(object sender, EventArgs e)
        {
            length.Text = temperature.Text = current.Text = speed.Text = "--";
        }

        private void buttonConnect_Click(object sender, EventArgs e)
        {
            //% port configuration 
            port.PortName = textBox2.Text;
            port.BaudRate = 38400;
            port.Parity = Parity.None;
            port.DataBits = 8;
            port.StopBits = StopBits.One;
            //% selecting handshake method
            if (radioButton1.Checked) port.Handshake = Handshake.None;
            else if (radioButton2.Checked) port.Handshake = Handshake.RequestToSend;

            port.WriteTimeout = 500;

            port.Open();
            if (!port.IsOpen)
                MessageBox.Show("Error", "Cannot open" + port.PortName + "port", MessageBoxButtons.OK, MessageBoxIcon.Error);

            port.Write("PD");
            System.Threading.Thread.Sleep(20);
            textBox1.Text = port.ReadExisting();
        }

        private void buttonDisconnect_Click(object sender, EventArgs e)
        {
            if (port.IsOpen) port.Close();            
        }

    }
}

I'll be grateful for any suggestions!

[ Voor 0% gewijzigd door RobIII op 26-01-2011 18:44 . Reden: Syntax highlighting FTW \O/ ]

[RobIII]

Wat is het nut van

C#:

GC.KeepAlive(clock); 

De variabele clock is toch gewoon een instance variabele van Form1? Waarom zou de GC daar aan komen dan?

[Snuffel]

RobIII schreef op woensdag 26 januari 2011 @ 18:47:
Wat is het nut van

C#:

GC.KeepAlive(clock); 

De variabele clock is toch gewoon een instance variabele van Form1? Waarom zou de GC daar aan komen dan?

It is a call to Garbage Collector to keep the clock alive until I dispose it manually. It was advised to do so in MSDN help file.
I don't think it has an influence on the port communication.

[RobIII]

MSDN zegt:

quote: http://msdn.microsoft.com.../system.timers.timer.aspx

// If the timer is declared in a long-running method, use
// KeepAlive to prevent garbage collection from occurring
// before the method ends.

// If the timer is declared in a long-running method,
// KeepAlive must be used to prevent the JIT compiler
// from allowing aggressive garbage collection to occur
// before the method ends.

Dat is hier niet aan de orde. Er staat ook:

// Normally, the timer is declared at the class level,
// so that it stays in scope as long as it is needed.

En dat is wél aan de orde. Als je documentatie leest (en zéker MSDN) moet je 't wel goed doen

Wat het communicatieprobleem precies is zie ik zo op 't oog even niet (helaas).

[ Voor 33% gewijzigd door RobIII op 26-01-2011 20:07 ]

[farlane]

Multithreading issues, System.Timer callbacked op een andere thread. Code is trouwens ook ugly as hell met die sleeps erin, en daardoor alleen al waarschijnlijk kapot.

[ Voor 42% gewijzigd door farlane op 26-01-2011 20:18 ]

[barfieldmv]

Windows thread scheduler has a 40 ms resolution, waiting for 20 milliseconds can complete in 40 milliseconds, or on a busy system in 3 40ms steps. Waiting 40 milliseconds is a long period.

Serial port communication should run in a while loop and report an event if an 'entire' message is recieved.

While(notStopped)
if BytesAvailable
ReadBytesAvailableAndPutThemOnSharedList
Thread.Sleep(0)

Another thread should check the shared list and copy out the data to handle it.

While SharedListIsEmpty
CheckIfListIsEmptyAndHandleAvailableItems

Add a Try Catch around the timer_tick. A timer should never have unhandled exceptions.

[ Voor 7% gewijzigd door barfieldmv op 26-01-2011 20:27 ]

[RobIII]

barfieldmv schreef op woensdag 26 januari 2011 @ 20:27:
Windows thread scheduler has a 40 ms resolution

Heb je daar een bron van? De enige resolutie waar ik iets van weet is er een van 15.6ms:

quote: http://webcache.googleuse...D5B237&cd=1&hl=en&ct=clnk

The default timer resolution on Windows 7 is 15.6 milliseconds (ms)

En dan moet je er nog van uit gaan dat 't geen realtime OS is en er dus afwijkingen kunnen zijn. Maar deze 'resolutie' geldt voor timers; de thread-scheduler heeft, AFAIK, al helemaal geen 'resolutie'. En er zijn prima veel hogere resolutie timers beschikbaar als 't nodig is.

Verder zou 't fijn zijn, zoals mijn oud-collega hieronder subtiel opmerkt, als we het gewoon Nederlands houden. Dat betekent voor TS even een vertaalslagje EN->NL te moeten maken voor zaken die hij (of z'n vriendin) kwijt wil en voor de rest iedereen gewoon netjes te antwoorden in NL. We zijn een Nederlandstalig forum en dat willen we graag zou houden

[ Voor 84% gewijzigd door RobIII op 26-01-2011 20:57 ]

[whoami]

Pourquoi est-ce-que la question est-elle posé en français ?
C'est un forum néerlandophone ici, non ?

[farlane]

Serial port communication should run in a while loop and report an event if an 'entire' message is recieved.

While(notStopped)
if BytesAvailable
ReadBytesAvailableAndPutThemOnSharedList
Thread.Sleep(0)

Another thread should check the shared list and copy out the data to handle it.

While SharedListIsEmpty
CheckIfListIsEmptyAndHandleAvailableItems

Add a Try Catch around the timer_tick. A timer should never have unhandled exceptions.

Als je alleen maar bytes leest die toch al aanwezig zijn ( ReadExisting blocked niet ) heb je helemaal geen andere thread nodig.

code:

Lees beschikbare bytes
Plak aan reeds ontvangen bytes vast
Kijk of er een volledig bericht is
Is het bericht geldig? Verwerk bericht anders flush buffers en begin opnieuw
Zo nee kijk hoe lang er al geen volledig bericht is
Te lang al geen volledig bericht ?  Flush buffers en begin opnieuw
Schoonspoelen && Herhalen

[Snuffel]

farlane schreef op woensdag 26 januari 2011 @ 20:17:
Multithreading issues, System.Timer callbacked op een andere thread. Code is trouwens ook ugly as hell met die sleeps erin, en daardoor alleen al waarschijnlijk kapot.

Zo te horen weet jij in ieder geval hoe het wél moet, dus laat eens zien?

[RobIII]

Snuffel schreef op woensdag 26 januari 2011 @ 21:40:
Zo te horen weet jij in ieder geval hoe het wél moet, dus laat eens zien?

Ja,
We zitten hier niet om voor jan-en-alleman op een zilveren presenteerblaadje kant-en-klare oplossingen te schrijven: Kan iemand even...?

Ik laat 't topic open maar ik zou 't doodzonde vinden als iemand nu complete code gaat lopen voorkauwen.

Give a man a fish and feed him for a day. Teach a man how to fish and feed him for a lifetime.

[Snuffel]

Kijk, als ik het wist zou ik haar al lang hebben geholpen, maar ik weet het zelf absoluut niet.
Ze heeft een probleem, en kan niet achter komen wat er nu precies mis gaat, en ik heb haar aangeraden hier eens te posten wat een (of de) oplossing zou kunnen zijn.

Dus als iemand haar in een bepaalde richting kan duwen, dan zou dat perfect zijn. Maar als je iets hebt van: daar zitten we hier niet voor, ook prima, dan gooi er maar een slotje op.

Wat ik overigens vreemd zou vinden, want het is toch een duidelijke hulp vraag, en daar is het forum toch voor?

[RobIII]

Snuffel schreef op woensdag 26 januari 2011 @ 21:51:
Wat ik overigens vreemd zou vinden, want het is toch een duidelijke hulp vraag, en daar is het forum toch voor?

Je topicstart wel, "Zo te horen weet jij in ieder geval hoe het wél moet, dus laat eens zien?" niet

[farlane]

Snuffel schreef op woensdag 26 januari 2011 @ 21:40:
Zo te horen weet jij in ieder geval hoe het wél moet, dus laat eens zien?

Ja ik weet hoe het moet, maar ik vind het zo vervelend om mezelf te quoten, dus als je wilt scroll een stukkie omhoog.

[Snuffel]

RobIII schreef op woensdag 26 januari 2011 @ 22:00:
[...]

Je topicstart wel, "Zo te horen weet jij in ieder geval hoe het wél moet, dus laat eens zien?" niet

Nee, zo start dit topic helemaal niet, misschien moet je het nog eens nalezen?
Overigens vraag ik mij af waarom je zo off-topic bezig bent.
De vraagstelling is duidelijk lijkt me, en op één enkel antwoord na zie ik alleen maar reacties waar de vraagsteller absoluut niets mee kan.

[Verwijderd]

Snuffel schreef op donderdag 27 januari 2011 @ 07:37:
[...]


Nee, zo start dit topic helemaal niet, misschien moet je het nog eens nalezen?
Overigens vraag ik mij af waarom je zo off-topic bezig bent.
De vraagstelling is duidelijk lijkt me, en op één enkel antwoord na zie ik alleen maar reacties waar de vraagsteller absoluut niets mee kan.

Dude heb je de komma wel gezien?

[Spiral]

http://msdn.microsoft.com...timers.timer.elapsed.aspx

If the SynchronizingObject property is null, the Elapsed event is raised on a ThreadPool thread. If the processing of the Elapsed event lasts longer than Interval, the event might be raised again on another ThreadPool thread. In this situation, the event handler should be reentrant.

Je interval staat nu op 1000 ms. Het kan zijn dat de methode clock_Elapsed er langer over doet dan 1000ms. Dit betekend dat de timer op een andere thread clock_Elapsed ook aanroept. En dan kan je problemen krijgen die jij beschrijft

[Verwijderd]

Misschien snap ik het niet helemaal maar waarom wordt het DataReceived event niet gewoon gebruikt?

[farlane]

Verwijderd schreef op donderdag 27 januari 2011 @ 11:58:
Misschien snap ik het niet helemaal maar waarom wordt het DataReceived event niet gewoon gebruikt?

Omdat pollen met ReadExisting ook werkt. Je moet toch een timeout mechanisme hebben.

[Snuffel]

thank you everyone for your input and suggestions.
I managed to get the reading working after changing the reading to event-based. On virtual null modem it works great both ways, bu when connected to a real device, it still gives the write timeout. What can still be the problem?

C#:

using System.IO.Ports;
using System.Threading;
using System.Timers;

namespace Endoprosthesis
{
    public partial class Form1 : Form
    {
        //% allocate components
        private static SerialPort port;
        System.Timers.Timer clock;
        delegate void SetTextCallback(string text);

        public Form1()
        {
            InitializeComponent();

            //% create an instance of serial port
            port = new SerialPort();

            port.DataReceived += new SerialDataReceivedEventHandler(port_DataReceived);

            //%timer to prompt for status updates every second
            clock = new System.Timers.Timer(1000);
            //% setup not to stop counting after first occurance
            clock.AutoReset = true;
            //% making sure the garbage collector will not kill it too soon
            GC.KeepAlive(clock);

            //% handling for the event of elapsed second
            clock.Elapsed += new ElapsedEventHandler(clock_Elapsed);
        }

        void clock_Elapsed(object sender, ElapsedEventArgs e)
        {
            port.Write("CL");
            System.Threading.Thread.Sleep(20);

            port.Write("MT");
            System.Threading.Thread.Sleep(50);

            port.Write("MC");
            System.Threading.Thread.Sleep(50);

            port.Write("MS");
            System.Threading.Thread.Sleep(50);
        }

        private void buttonXtend_Click(object sender, EventArgs e)
        {
            port.Write("M1");

            clock.Start();
        }

        private void buttonStop_Click(object sender, EventArgs e)
        {
            port.Write("M0");

            clock.Stop();
        }

        private void buttonClose_Click(object sender, EventArgs e)
        {
            clock.Stop();
            clock.Dispose();
            if(port.IsOpen) port.Close();
            this.Close();
        }

        private void buttonClear_Click(object sender, EventArgs e)
        {
            length.Text = temperature.Text = current.Text = speed.Text = "--";
        }

        private void buttonConnect_Click(object sender, EventArgs e)
        {
            //% port configuration
            port.PortName = textBox2.Text;
            port.BaudRate = 38400;
            port.Parity = Parity.None;
            port.DataBits = 8;
            port.StopBits = StopBits.One;
            //% selecting handshake method
            if (radioButton1.Checked) port.Handshake = Handshake.None;
            else if (radioButton2.Checked) port.Handshake = Handshake.RequestToSend;

            port.WriteTimeout = 500;
            port.ReadTimeout = 500;

            port.Open();
            if (!port.IsOpen)
                MessageBox.Show("Error", "Cannot open" + port.PortName + "port", MessageBoxButtons.OK, MessageBoxIcon.Error);

            port.Write("PD");
        }

        private void buttonDisconnect_Click(object sender, EventArgs e)
        {
            if (port.IsOpen) port.Close();           
        }

        private void port_DataReceived(object sender, SerialDataReceivedEventArgs e)
        {
            string value = port.ReadExisting();
            this.SetText(value);
        }

        void SetText(string text)
        {
            string fieldname = text.Substring(0, 2);
            if (fieldname == "MC") setName(current, text);
            else if (fieldname == "MT") setName(temperature, text);
            else if (fieldname == "MS") setName(speed, text);
            else if (fieldname == "CL") setName(length, text);
            else if (fieldname == "PD") setName(textBox1, text);
        }

        void setName(Label field, string text)
        {
            if (field.InvokeRequired)
            {
                SetTextCallback d = new SetTextCallback(SetText);
                this.Invoke(d, new object[] { text });
            }
            else
            {
                text = text.Substring(3, text.Length - 4);
                field.Text = text;
            }
        }

        void setName(TextBox field, string text)
        {
            if (field.InvokeRequired)
            {
                SetTextCallback d = new SetTextCallback(SetText);
                this.Invoke(d, new object[] { text });
            }
            else
            {
                text = text.Substring(3, text.Length - 4);
                field.Text = text;
            }
        }
    }

}

[farlane]

Welke garantie heb je dat bij een DataReceived event ook daadwerkelijk het hele bericht binnen is?

Anyway, je hebt een behoorlijk selectief begrip dus ik ben er wel klaar mee.