Return to Answer

Fix motorPort to MotorPort in constructor.

Source Link

edited Dec 13, 2016 at 11:27

public sealed class Motor : IDisposable
{
    public static readonly Motor Instance = new Motor();
      
    private Motor()
    {
        this.motorPortMotorPort = new SerialPort();
        motorPortMotorPort.PortName = "COM6";
        motorPortMotorPort.Handshake = Handshake.RequestToSend;
        motorPortMotorPort.ReceivedBytesThreshold = 8;
        motorPortMotorPort.DataReceived += new SerialDataReceivedEventHandler(dataRecievedHandler);
        motorPortMotorPort.Open();
    }

    public SerialPort MotorPort { get; private set; }
    public decimal Position { get; private set; }
    public void StopRotate()    { }
    public void StartRotate(string speed) { }
    public void MoveToPosition(int position) { }
    private void dataRecievedHandler(object sender, SerialDataReceivedEventArgs e)
    {
    }
    public void Dispose()
    {
        if (MotorPort.IsOpen)
        {
            MotorPort.Close();
        }
    }
}

public sealed class Motor : IDisposable
{
    public static readonly Motor Instance = new Motor();
      
    private Motor()
    {
        this.motorPort = new SerialPort();
        motorPort.PortName = "COM6";
        motorPort.Handshake = Handshake.RequestToSend;
        motorPort.ReceivedBytesThreshold = 8;
        motorPort.DataReceived += new SerialDataReceivedEventHandler(dataRecievedHandler);
        motorPort.Open();
    }

    public SerialPort MotorPort { get; private set; }
    public decimal Position { get; private set; }
    public void StopRotate()    { }
    public void StartRotate(string speed) { }
    public void MoveToPosition(int position) { }
    private void dataRecievedHandler(object sender, SerialDataReceivedEventArgs e)
    {
    }
    public void Dispose()
    {
        if (MotorPort.IsOpen)
        {
            MotorPort.Close();
        }
    }
}

public sealed class Motor : IDisposable
{
    public static readonly Motor Instance = new Motor();
      
    private Motor()
    {
        MotorPort = new SerialPort();
        MotorPort.PortName = "COM6";
        MotorPort.Handshake = Handshake.RequestToSend;
        MotorPort.ReceivedBytesThreshold = 8;
        MotorPort.DataReceived += new SerialDataReceivedEventHandler(dataRecievedHandler);
        MotorPort.Open();
    }

    public SerialPort MotorPort { get; private set; }
    public decimal Position { get; private set; }
    public void StopRotate()    { }
    public void StartRotate(string speed) { }
    public void MoveToPosition(int position) { }
    private void dataRecievedHandler(object sender, SerialDataReceivedEventArgs e)
    {
    }
    public void Dispose()
    {
        if (MotorPort.IsOpen)
        {
            MotorPort.Close();
        }
    }
}

Source Link

answered Dec 13, 2016 at 0:21

Tomas Paul

I guess you thought about avoiding of Thread.Sleep() method, not Wait() you mentioned in title. I can't test your code because I don't have either COM port nor motor. I would like to give you some general tips how to improve your code. Let's implement some SOLID and DRY principles. I would start with class Motor which will represent your device. You can implement for example singleton class for this purpose. This helps you to separate your motor logic from other logic such is user interface. I can also imagine your port settings as a part of application configuration.

Motor.cs

public sealed class Motor : IDisposable
{
    public static readonly Motor Instance = new Motor();
      
    private Motor()
    {
        this.motorPort = new SerialPort();
        motorPort.PortName = "COM6";
        motorPort.Handshake = Handshake.RequestToSend;
        motorPort.ReceivedBytesThreshold = 8;
        motorPort.DataReceived += new SerialDataReceivedEventHandler(dataRecievedHandler);
        motorPort.Open();
    }

    public SerialPort MotorPort { get; private set; }
    public decimal Position { get; private set; }
    public void StopRotate()    { }
    public void StartRotate(string speed) { }
    public void MoveToPosition(int position) { }
    private void dataRecievedHandler(object sender, SerialDataReceivedEventArgs e)
    {
    }
    public void Dispose()
    {
        if (MotorPort.IsOpen)
        {
            MotorPort.Close();
        }
    }
}

Instead of using global fields (M, buf), encapsulate them into object and set the proper access modifier or better create properties. Use meaningful names your fields and properties. Use Capitalization Convention.

You can isolate your repeating strings into separate static class to make your string constant repository which would be more easily maintainable against direct using of particular string.

StringConstants.cs

static class StringConstants
{
    public const string Prefix = "SR,00,002,";
    public const string PrefixCrLf = "SR,00,002\r\n";
}

Extension methods can simplify your code and make it more readable.

Extensions.cs

static class Extensions
{
    public static decimal GetDecimalOutput(this SerialPort port)
    {
        return decimal.Parse(Regex.Split(port.ReadLine(), StringConstants.Prefix)[1]);
    }
    public static void SendCommand(this SerialPort port)
    {
        port.WriteLine(StringConstants.PrefixCrLf);
    }
}

If you have implemented Motor.cs methods, you can use it or extend it.

Form1.cs

private void btnTest_Click(object sender, EventArgs e)
{
    Motor.Instance.StartRotate(speed);
    Thread.Sleep(5000);
    Motor.Instance.StopRotate();
    Motor.Instance.MoveToPosition(0);

    //read sensor
    serialPort2.SendCommand();
    decimal caliber = serialPort2.GetDecimalOutput();

    do
    {
        serialPort2.SendCommand();
        actualValue = serialPort2.GetDecimalOutput();
    }
    while (actualValue <= caliber);
}

I am not experienced in serial port programming, but I am pretty sure that event SerialPort.DataReceived should be used to invoke subscribed method as you do with your serialPort3. I don't know exactly why you are using Thread.Sleep(), because I cannot see getPosi(C); method.