Skip to main content
added 1167 characters in body
Source Link
RobH
  • 17.1k
  • 6
  • 38
  • 73

Just a minor thing that jumped out at me:

private void rotate(byte[] A, string Test) //Rotate Motor Right
{
    A = MB.TMCL_RMR(Test);
    serialPort3.Write(A, 0, A.Length);
}

Why are you passing in a byte array to just replace it with something else on the first line?

private void SendRotateCommand(string speed)
{
    var commandBytes = MB.TMCL_RMR(speed);
    motorPort.Write(commandBytes, 0, commandBytes.Length); 
}

I've guessed that the Test parameter was actually the rotation speed from a comment in the method that calls it.

You need to improve your naming throughout. Your code is very simple but is very difficult to follow simply because the naming is so poor.


Another small thing:

public byte[] GET_POSI()
{
    byte[] E = new byte[9];
    E[0] = 0x1;
    E[1] = 0x6;
    E[2] = 0x1;
    E[3] = 0x0;
    E[4] = 0x0;
    E[5] = 0x0;
    E[6] = 0x0;
    E[7] = 0x0;
    E[8] = 0x08;
    return E;
}

Could be written more simply as:

public byte[] GET_POSI()
{
    return new byte[] { 1, 6, 1, 0, 0, 0, 0, 0, 8 };
}

I can't really understand what you're trying to save the position for. Or even how you're attempting to do it. You are sending messages to the motor and it's sending things back. You get a message, you figure out what it means and act on it.

You're also mixing DataReceived with SerialPort.ReadLine they both use the same underlying memory stream and you should be using one or the other - not both.

You're also freezing your UI for large chunks of time while you wait for operations to complete. It's fine to accept the command from the user and then update the UI as the action progresses. E.g. show the current position as the motor is rotating, when it gets to the right point, stop it.

Just a minor thing that jumped out at me:

private void rotate(byte[] A, string Test) //Rotate Motor Right
{
    A = MB.TMCL_RMR(Test);
    serialPort3.Write(A, 0, A.Length);
}

Why are you passing in a byte array to just replace it with something else on the first line?

private void SendRotateCommand(string speed)
{
    var commandBytes = MB.TMCL_RMR(speed);
    motorPort.Write(commandBytes, 0, commandBytes.Length); 
}

I've guessed that the Test parameter was actually the rotation speed from a comment in the method that calls it.

You need to improve your naming throughout. Your code is very simple but is very difficult to follow simply because the naming is so poor.

Just a minor thing that jumped out at me:

private void rotate(byte[] A, string Test) //Rotate Motor Right
{
    A = MB.TMCL_RMR(Test);
    serialPort3.Write(A, 0, A.Length);
}

Why are you passing in a byte array to just replace it with something else on the first line?

private void SendRotateCommand(string speed)
{
    var commandBytes = MB.TMCL_RMR(speed);
    motorPort.Write(commandBytes, 0, commandBytes.Length); 
}

I've guessed that the Test parameter was actually the rotation speed from a comment in the method that calls it.

You need to improve your naming throughout. Your code is very simple but is very difficult to follow simply because the naming is so poor.


Another small thing:

public byte[] GET_POSI()
{
    byte[] E = new byte[9];
    E[0] = 0x1;
    E[1] = 0x6;
    E[2] = 0x1;
    E[3] = 0x0;
    E[4] = 0x0;
    E[5] = 0x0;
    E[6] = 0x0;
    E[7] = 0x0;
    E[8] = 0x08;
    return E;
}

Could be written more simply as:

public byte[] GET_POSI()
{
    return new byte[] { 1, 6, 1, 0, 0, 0, 0, 0, 8 };
}

I can't really understand what you're trying to save the position for. Or even how you're attempting to do it. You are sending messages to the motor and it's sending things back. You get a message, you figure out what it means and act on it.

You're also mixing DataReceived with SerialPort.ReadLine they both use the same underlying memory stream and you should be using one or the other - not both.

You're also freezing your UI for large chunks of time while you wait for operations to complete. It's fine to accept the command from the user and then update the UI as the action progresses. E.g. show the current position as the motor is rotating, when it gets to the right point, stop it.

Source Link
RobH
  • 17.1k
  • 6
  • 38
  • 73

Just a minor thing that jumped out at me:

private void rotate(byte[] A, string Test) //Rotate Motor Right
{
    A = MB.TMCL_RMR(Test);
    serialPort3.Write(A, 0, A.Length);
}

Why are you passing in a byte array to just replace it with something else on the first line?

private void SendRotateCommand(string speed)
{
    var commandBytes = MB.TMCL_RMR(speed);
    motorPort.Write(commandBytes, 0, commandBytes.Length); 
}

I've guessed that the Test parameter was actually the rotation speed from a comment in the method that calls it.

You need to improve your naming throughout. Your code is very simple but is very difficult to follow simply because the naming is so poor.