I'm writing a C# WPF project built with a MVVM pattern. This software is written to use a medical diagnostic device, so in our planning it will be certified IEC 62304. Since I've been mainly writing only C++ software for the last few years, I don't have much exeperience with C# programming. So I would like to receive a feedback on my code and understand if there are some weaknesses.
The software completely relies on a serial port communication with the device, so my serial port related code is very crucial. Since I read that there are a lot of problem when speakiing of the .NET serial port class, I would like to ask you if you think that my approach is right or not.
To costumize the serial port usage to my needs, I decided to implement a wrapper of the standard class, that hence inherits from SerialPort. Since I only need to communicate with one device, I decided to make my class static and call its public methods from my app. Is it a righ way to handle the communication? I also have some methods related to searching my serial port and check if my device (identified by USB VID and PID) is connected or not. I only need to receive data from the device when I ask for something, so I don't need to handle data indipendently sent from the device. I think I don't need a "DataReceived" event handler then.
In my MVVM pattern I call the serial port public methods from a Task launched from my ViewModel. Actually I have a second static class (called DeviceCommunication) with all the commands I have to send to my device, which calls my serial port methods like read and write. Do you see any problem in doing so? I'm posting my entire serial port class and some snippets from my ViewModel.
Any feedback is appreciated! Thanks!
/// <summary>
/// This class implements the Com Port Driver
/// </summary>
public static class ComPort
{
/// <summary>
/// Serial Port public handler
/// </summary>
static SerialPort devicePort = new SerialPort();
/// <summary>
/// Flag that tells if the device il connected
/// </summary>
public static bool DeviceConnected;
/// <summary>
/// COM port searching main method.
/// </summary>
public static void SearchCOM()
{
var QueryCmd = new byte[] { /* Command buffer content to query the device */ };
var Reply = new byte[] { 0, 0, 0, 0 };
var BytesRead = 0;
if (DeviceConnected)
{
devicePort.Close();
DeviceConnected = false;
}
while (!DeviceConnected)
{
var SerailPortsNames = ComPortNames("0483", "5740");
if (SerailPortsNames.Count > 0)
{
foreach (string s in SerialPort.GetPortNames())
{
if (SerailPortsNames.Contains(s))
{
devicePort.PortName = s;
devicePort.BaudRate = 115200;
devicePort.Parity = Parity.None;
devicePort.DataBits = 8;
devicePort.StopBits = StopBits.One;
devicePort.RtsEnable = false;
devicePort.DtrEnable = false;
devicePort.ReadTimeout = 10000;
devicePort.WriteTimeout = 1000;
devicePort.ReadBufferSize = 60000;
devicePort.Handshake = Handshake.None;
try
{
devicePort.Open();
if (devicePort.IsOpen)
{
devicePort.Write(QueryCmd, 0, QueryCmd.Length);
BytesRead = Read(Reply, 4);
if (BytesRead != 4)
{
devicePort.Close();
}
else
{
DeviceConnected = true;
}
}
}
catch(Exception e)
{
Console.WriteLine("{0} Exception caught.", e);
}
}
}
}
System.Threading.Thread.Sleep(250);
}
}
/// <summary>
/// Public method to use for reading from COM port. It's an override of the already existing method from SerialPort class.
/// </summary>
/// <param name="buffer"> Buffer to use for storing data </param>
/// <param name="lenght"> Number of bytes to read </param>
/// <returns></returns>
public static int Read(byte[] buffer, int lenght)
{
var i = 0;
while (i < lenght)
{
i += devicePort.BaseStream.Read(buffer, i, (lenght - i));
}
return i;
}
/// <summary>
/// Public method to use for writing to COM port. It's an override of the already existing method from SerialPort class.
/// </summary>
/// <param name="buffer"> Buffer to write </param>
/// <param name="lenght"> Number of bytes to write </param>
public static void Write(byte[] buffer, int lenght)
{
devicePort.BaseStream.Write(buffer, 0, lenght);
}
/// <summary>
/// Public method to close the COM port.
/// </summary>
public static void Close()
{
devicePort.Close();
}
/// <summary>
/// Static method that returns the list of connected COM ports with given VID and PID.
/// </summary>
/// <param name="vid"> Vendor ID </param>
/// <param name="pid"> Product ID </param>
/// <returns> List of connected COM ports with given VID and PID </returns>
static List<string> ComPortNames(string vid, string pid)
{
int ID;
var pattern = string.Format("^VID_{0}.PID_{1}", vid, pid);
var _rx = new Regex(pattern, RegexOptions.IgnoreCase);
var comports = new List<string>();
var rk1 = Registry.LocalMachine;
var rk2 = rk1.OpenSubKey("SYSTEM\\CurrentControlSet\\Enum");
foreach (string s3 in rk2.GetSubKeyNames())
{
var rk3 = rk2.OpenSubKey(s3);
foreach (string s in rk3.GetSubKeyNames())
{
if (_rx.Match(s).Success)
{
var rk4 = rk3.OpenSubKey(s);
foreach (string s2 in rk4.GetSubKeyNames())
{
if (int.TryParse(s2, out ID))
{
var rk5 = rk4.OpenSubKey(s2);
var rk6 = rk5.OpenSubKey("Device Parameters");
if (IsDeviceConnected((string)rk6.GetValue("PortName")))
{
comports.Add((string)rk6.GetValue("PortName"));
}
}
}
}
}
}
return comports;
}
/// <summary>
/// This method checks whether the device is connected or not.
/// </summary>
/// <param name="comName"> Name of the port </param>
/// <returns> true if device is connected, fals if not. </returns>
static bool IsDeviceConnected(string comName)
{
string GetComName;
var Rk1 = Registry.LocalMachine;
var Rk2 = Rk1.OpenSubKey("HARDWARE\\DEVICEMAP\\SERIALCOMM");
foreach (string S1 in Rk2.GetValueNames())
{
GetComName = (string)Rk2.GetValue(S1);
if (comName.Equals(GetComName))
{
return true;
}
}
return false;
}
}
In my ViewModel contructor I start two tasks:
var TaskSearchCOM = Task.Run(() => ComPort.SearchCOM());
var TaskStartup = Task.Run(() => Startup());
...
/// <summary>
/// This method asks the device for startup info.
/// </summary>
public async Task Startup()
{
while (!ComPort.DeviceConnected)
await Task.Delay(250);
Info = "Device found, asking for info...";
while(taskState != TaskStates.Terminate)
{
switch(taskState)
{
case TaskStates.AskInfo:
{
var DeviceID = DeviceCommunication.GetDeviceSerialNumber();
... break;
}
case ...