0
\$\begingroup\$

I am having a serial port communication application using 3 serial ports DMX (m_port,m_port1,m_port2) which will run 24 *7 , so i just need to ensure memory usage doesn't go above limit with using(), Idisposable and minimize use of global variables.

Start method to start sending hex data continuously till i stop it using flag Global.tmrStarted on stop method

Below is my code:

DMX controller for serial port baud rate 250000

         public class Dmx
         {
       public byte R { get; protected set; }
       public byte G { get; protected set; }
       public byte B { get; protected set; }

        private SerialPort m_port;

        private SerialPort m_port1;

        private SerialPort m_port2;

       bool OpenFlag = true;

DMX constructor for initlizing 3 serial ports

          public Dmx(int portNo, int R)
          {
          using (m_port = new SerialPort(string.Format("COM2", portNo), 250000,                             Parity.None, 8, StopBits.Two)) ;
          {

        m_port.Handshake = Handshake.None;
        m_port.RtsEnable = true;

        if (OpenFlag == false)
        {
            m_port.Close();
            m_port.DtrEnable = false;
            m_port.RtsEnable = false;

        }
        else
        {
            m_port.Open();
            m_port.Parity = Parity.None;
            m_port.StopBits = StopBits.Two;
            m_port.Handshake = Handshake.None;
            m_port.ReadTimeout = 2000;
            m_port.WriteTimeout = 500;
            m_port.DtrEnable = true;
            m_port.RtsEnable = true;

        }
    }


  using (m_port1 = new SerialPort(string.Format("COM3", portNo), 250000, Parity.None, 8, StopBits.Two)) ;
    {
        m_port1.Handshake = Handshake.None;
        m_port1.RtsEnable = true;

        if (OpenFlag == false)
        {
            m_port.Close();
            m_port.DtrEnable = false;
            m_port.RtsEnable = false;

        }
        else
        {
            m_port1.Open();
            m_port1.Parity = Parity.None;
            m_port1.StopBits = StopBits.Two;
            m_port1.Handshake = Handshake.None;
            m_port1.ReadTimeout = 2000;
            m_port1.WriteTimeout = 500;
            m_port1.DtrEnable = true;
            m_port1.RtsEnable = true;
        }



     using (m_port2 = new SerialPort(string.Format("COM4", portNo), 250000, Parity.None, 8, StopBits.Two)) ;
    {

        m_port2.Handshake = Handshake.None;
        m_port2.RtsEnable = true;

        if (OpenFlag == false)
        {
            m_port2.Close();
            m_port2.DtrEnable = false;
            m_port2.RtsEnable = false;

        }
        else
        {
            m_port2.Open();
            m_port2.Parity = Parity.None;
            m_port2.StopBits = StopBits.Two;
            m_port2.Handshake = Handshake.None;
            m_port2.ReadTimeout = 2000;
            m_port2.WriteTimeout = 500;
            m_port2.DtrEnable = true;
            m_port2.RtsEnable = true;

        }

    }
}

Global variable and start method to send data to 3 ports in continuous while loop which calls SendData() method

              List<byte> gRGBdata;
         bool stopRequested = false; //signaler flag
       //Thread st;
          [WebMethod]
           public void Start(List<byte> RGBdata)
            {

    gRGBdata = RGBdata; //copy to "global" variable
    int portNo = 2;
    stopRequested = false;


    Global.tmrStarted = true;
    SendDataLoop();

            }





         List<byte> value;



         public void SendDataLoop()
           {
         value = gRGBdata;

    while (Global.tmrStarted)
     {
        Break();
        Thread.Sleep(50); //uncommect this is you need, test
        if (m_port == null || m_port.IsOpen == false) return;
        m_port.Write(new byte[] { 0 }, 0, 1);
        SendData();

        Break();
        Thread.Sleep(50); //uncommect this is you need, test
        if (m_port1 == null || m_port1.IsOpen == false) return;
        m_port1.Write(new byte[] { 0 }, 0, 1);
        SendData();

         Break();
        Thread.Sleep(50); //uncommect this is you need, test
        if (m_port2 == null || m_port2.IsOpen == false) return;
        m_port1.Write(new byte[] { 0 }, 0, 1);
        SendData();

          }

          Stop();

           }

Stop method to stop data

        public void Stop()
         {
       timer.Stop();

       Global.tmrStarted = false;
       m_port.Close();

    System.Threading.Thread.Sleep(500);
    if (m_port.IsOpen)
    {
        try
        {
            OpenFlag = false;
            m_port.Close();
        }
        catch (Exception ex)
        {
            System.Diagnostics.Debug.WriteLine(ex.Message + ex.StackTrace);
        }
            }
            }

Break method for break state

          private void Break()
          {
    m_port.BreakState = true;
     System.Threading.Thread.Sleep( 1 );
    m_port.BreakState = false;

          }

            public void SendData()
    {  
      m_port.Write(gRGBdata.ToArray(), 0, gRGBdata.Count);
    }
      }

Please let me know whether i need to implement Idisposable or using() is sufficient to release resources and i am concerned about below code as it will run 24 * 7 and won't throw memory exception.Thanks

\$\endgroup\$
  • 2
    \$\begingroup\$ As I asked on your previous question does your code work in its current state? It looks like the constructor for Dmx disposes of the serial ports, however you then attempt to use them from your SendDataLoop without opening it again. \$\endgroup\$ – forsvarir Jul 14 '16 at 6:10
  • \$\begingroup\$ @forsvarir Yes , it certainly works . In dmx constructor 3 serial ports get initilaized , ohh ok , so u mean as i have used using() so it will be available till that scope... but it works and u see in SendDataLoop i check if port is opened then send data \$\endgroup\$ – Ajay S sharma Jul 14 '16 at 6:17
  • \$\begingroup\$ My conccern is memory utilization as i need to have this application work continuously , i need to ensure that it memory limit should not hang it. Should i use idisposable or what improvable changes i do for less memory utilization . Thanks for your help \$\endgroup\$ – Ajay S sharma Jul 14 '16 at 6:18
2
\$\begingroup\$

Formatting

The first impression your code makes is not a good one. The formatting is very inconsistent, which makes it difficult to follow and is likely to introduce bugs. If it looks like this in your IDE, then you really should expend some effort in fixing the formatting (any version of visual studio will significantly help with this). Copying and pasting the code into your questions, then selecting it and pressing Ctrl+k will format the block.

Nested using statements

Looking at this code below, which I've formatted:

using (m_port1 = new SerialPort(string.Format("COM3", portNo), 250000, Parity.None, 8, StopBits.Two)) ;
{
    m_port1.Handshake = Handshake.None;
    m_port1.RtsEnable = true;

    if (OpenFlag == false)
    {
        m_port.Close();
        m_port.DtrEnable = false;
        m_port.RtsEnable = false;

    }
    else
    {
        m_port1.Open();
        m_port1.Parity = Parity.None;
        m_port1.StopBits = StopBits.Two;
        m_port1.Handshake = Handshake.None;
        m_port1.ReadTimeout = 2000;
        m_port1.WriteTimeout = 500;
        m_port1.DtrEnable = true;
        m_port1.RtsEnable = true;
    }

    using (m_port2 = new SerialPort(string.Format("COM4", portNo), 250000, Parity.None, 8, StopBits.Two)) ;
    {

You can see that the using statement that you're initialising m_port2 in is within the statement that you're initialising m_port1, however neither of them is within the using where you initialise m_port. This inconsistency suggests that it is a bug, which is harder to spot because of the code formatting.

Numbered variable names

Adding numbers to variable names to differentiate between them isn't a great naming strategy. In your case, it could actually work as a way of pairing the variable with the COM port that it's associated with. However, you're assigning m_port1 to COM3 etc, which is just confusing.

Disposing & member variables

To answer your specific concern, disposing the SerialPort should be enough to clean up after it's resources, however the way you're using it is odd (and I'm surprised it is working). Your ports are member variables, but you're wrapping your constructor calls in using statements. This should mean that the SerialPort is being disposed of in your constructor, and isn't in an appropriate state for usage from that point on, certainly not without opening it again. It would be better to either hold the serial port open (and have DMX implement IDisposable), or not have the serial port as member variables, so that you can wrap calls in using statements. The right approach really depends on your usage and expectations, however it's worth noting that you shouldn't immediately be calling open again on a closed port because there is an expectation that it may take some time to close.

\$\endgroup\$
  • \$\begingroup\$ Thanks and i will make myself formatting code more better and to standard level.So by your words "Your ports are member variables, but you're wrapping your constructor calls in using statements. This should mean that the SerialPort is being disposed of in your constructor". So how that port remains open even after getting disposed and my code works. I will also check my code debugging . \$\endgroup\$ – Ajay S sharma Jul 14 '16 at 10:09
  • \$\begingroup\$ And one more thing if i implement Idispoable on class DMX will it dispose all 3 objects of Serial port m_port,m_port1,m_port2 \$\endgroup\$ – Ajay S sharma Jul 14 '16 at 10:24
  • \$\begingroup\$ @AjaySsharma If you implement IDisposable on DMX, your dispose would need to call dispose on each of the ports, if required, it won't be wired up for you. \$\endgroup\$ – forsvarir Jul 14 '16 at 10:28
  • \$\begingroup\$ Yes i did it but since i have while continuous loop to run for 12- 15 hours, i get an error pop up An unhandled .Net Framework exception occured. When i checked System logs in Event viewer , it came as it got fatal from Windows Activation Service.So is it memory storage or RAM problem with while loop \$\endgroup\$ – Ajay S sharma Jul 29 '16 at 6:40

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.