Tuesday, August 21, 2007

...in a galaxy far, far away...

My adventures in .NET land have brought me to an interesting cross-roads.
See, most of the actual code modification I do in any environment is refactoring and re-architecture of existing assemblies; my roots in OO leave me in a moral dilemma every time i see something like:
Public Function GetProperty(Optional propName As String = "Any", _
Optional vIndex As Long = -1, _
Optional vType As PropertyType = MI_Value) As String()
Dim I As Integer
Dim j As Integer
Dim Temp() As String
On Error GoTo GET_PROP_Error
ReDim Temp(0) As String
Temp(0) = ""
ErrorState = 0
'check to see if a string or an index has been passed.
If Not propName = "Any" And vIndex = -1 Then
j = 0
For I = 1 To Properties.Count
If UCase(Properties.Item(I).Key) = UCase(propName) Then
j = j + 1
ReDim Preserve Temp(j) As String
If vType = MI_Key Then
Temp(j - 1) = Properties.Item(I).Key
ElseIf vType = MI_Value Then
Temp(j - 1) = Properties.Item(I).Value
ElseIf vType = MI_Both Then
j = j + 1
ReDim Preserve Temp(j) As String
Temp(j - 2) = Properties.Item(I).Key
Temp(j - 1) = Properties.Item(I).Value
End If
End If
Next I
If (j > 0) Then
ReDim Preserve Temp(j - 1)
End If
GetProperty = Temp
ElseIf Not vIndex = -1 And propName = "Any" Then
On Error GoTo GET_PROP_BY_INDEX_Error
ReDim Temp(0) As String
If vType = MI_Key Then
Temp(0) = Properties.Item(vIndex).Key
ElseIf vType = MI_Value Then
Temp(0) = Properties.Item(vIndex).Value
ElseIf vType = MI_Both Then
ReDim Preserve Temp(1) As String
Temp(0) = Properties.Item(vIndex).Key
Temp(1) = Properties.Item(vIndex).Value
End If
GetProperty = Temp
Else
For I = 1 To Properties.Count
If UCase(Properties.Item(I).Key) = UCase(propName) Then
If j = vIndex Then
ReDim Temp(0) As String
If vType = MI_Key Then
Temp(0) = Properties.Item(I).Key
ElseIf vType = MI_Value Then
Temp(0) = Properties.Item(I).Value
ElseIf vType = MI_Both Then
ReDim Preserve Temp(1) As String
Temp(0) = Properties.Item(I).Key
Temp(1) = Properties.Item(I).Value
End If
GetProperty = Temp
Exit Function
Else
j = j + 1
End If
End If
Next I
ErrorState = -10 'item not found in collection
GetProperty = Null
End If
If Temp(0) = "" Then
ErrorState = -10
GetProperty = Temp
Else
ErrorState = 0
End If
Exit Function

GET_PROP_BY_INDEX_Error:
ErrorState = -10 'item not found in collection
Exit Function

GET_PROP_Error:
ErrorState = -9 'Error accessing collection during search

End Function;

It makes me cringe.
Now granted, the above excerpt is (obviously) VB6, which warrants a whole host of implicit, and often required, architecture problems, but I nonetheless develop a certain pang upon sight of such atrocities.

But recently have have run into an issue in my own little framework, something I've codenamed "Project Cloud". It's a base framework for the future implementation of a new ERP system at my company. If all goes well, I will be pitching the idea to the corporate office, whom for whatever reason has chosen our particular division as the source of the next globally unified system, which they have been calling "AQCS".

The dilemma at hand lies in my implementation of event-base asynchronous IO through a serial port. The design consists of a hierarchy of 3 classes:

HardwareDevice - This simply handles the relevant information about the device being communicated with. Things like manufacturer name and model number are stored here (mostly just for better exception information).
ComDevice : HardwareDevice, System.ComponentModel.ISynchronizeInvoke - Though the name suggests something entirely different, this is a device which communicates though a "COM", or Serial port. "COM", of course, comes from the naming scheme of the ports themselves - "COM1", "COM2", etc.
NitonXL800 : ComDevice - This class, as the name suggests, is specific to a single device, it's constructors fill HardwareDevice with the relevant info, as well as provide ComDevice with port setup options.

The actual implementation of the classes is quite simple:


public abstract class HardwareDevice
{
private string name;
private string manufacturer;

public HardwareDevice()
: this("Unknown Device", "Unknown Manufacturer") { }

public HardwareDevice(string name)
: this(name, "Unknown Manufacturer") { }
public HardwareDevice(string name, string manufacturer)
{
this.name = name;
this.manufacturer = manufacturer;
}

public string Manufacturer
{
get { return manufacturer; }
set { manufacturer = value; }
}

public string Name
{
get { return name; }
set { name = value; }
}

}
}

public class ComDevice : HardwareDevice
{
#region Fields
private SerialPort prt;
private StringBuilder buffer = new StringBuilder();
private bool closeOnRecieve = true;
#endregion
#region Constructors
protected ComDevice()
: this(string.Empty) { }
protected ComDevice(string name)
: this(name, string.Empty) { }
protected ComDevice(string name, string manufacturer)
: this(name, manufacturer, "COM1") { }
protected ComDevice(string name, string manufacturer, string portName)
: base(name, manufacturer)
{
prt = GetDefaultSetup(portName);
prt.DataReceived += new SerialDataReceivedEventHandler(prt_DataReceived);
}
#endregion
#region Util
protected void OpenPort()
{
OpenPort(true);
}
protected void OpenPort(bool close)
{
prt.Open();
OnPortOpened(new EventArgs());
}
protected void ClosePort()
{
prt.Close();
OnPortClosed(new EventArgs());
}
protected void ClearBuffer()
{
buffer = new StringBuilder();
}
public static SerialPort GetDefaultSetup(string port)
{
return new SerialPort(port, 9600, Parity.None, 8, StopBits.One);
}
#endregion
#region Event Handler Pairs
private void prt_DataReceived(object sender, SerialDataReceivedEventArgs e)
{
OnDataReceived(e);
}
protected virtual void OnDataReceived(SerialDataReceivedEventArgs e)
{
string buf = prt.ReadExisting();
buffer.Append(buf);
if (DataReceived != null)
DataReceived(this, new ComDeviceEventArgs(buf));

if (closeOnRecieve) ClosePort();

}
private void OnPortOpened(EventArgs e)
{
if (PortOpened != null)
PortOpened(this, e);
}
private void OnPortClosed(EventArgs e)
{
if (PortClosed != null)
PortClosed(this, e);
}
#endregion
#region Properties
protected string Buffer
{ get { return buffer.ToString(); } }
protected int BaudRate
{
get { return prt.BaudRate; }
set { prt.BaudRate = value; }
}
//etc...
protected StopBits StopBits
{
get { return prt.StopBits; }
set { prt.StopBits = value; }
}
#endregion
#region Events
protected event ComDeviceDataReceivedEventHandler DataReceived;
protected event EventHandler PortClosed;
protected event EventHandler PortOpened;
#endregion
}
#region Event Stuff
public delegate void ComDeviceDataReceivedEventHandler(object sender, ComDeviceEventArgs e);

[System.Diagnostics.DebuggerStepThrough()]
public class ComDeviceEventArgs
{
private string data;
private SerialDataReceivedEventArgs serialData;

public ComDeviceEventArgs()
: this(string.Empty) { }
public ComDeviceEventArgs(string received)
: this(received, null) { }
public ComDeviceEventArgs(SerialDataReceivedEventArgs e)
: this(string.Empty, e) { }
public ComDeviceEventArgs(string received, SerialDataReceivedEventArgs e)
{
if (e != null) serialData = e;
data = received;
}
public string DataRecieved
{
get { return data; }
}
public SerialData EventType
{
get { return serialData.EventType; }
}
}
#endregion

public class NitonXL800 : ComDevice
{
public NitonXL800()
: this("COM1") { }
public NitonXL800(string port)
: base("XL800", "Niton", port)
{
base.Encoding = System.Text.Encoding.ASCII;
base.ReadTimeout = 2000;
base.DataReceived += new ComDeviceDataReceivedEventHandler(NitonXL800_DataReceived);
base.CloseOnRecieve = false;
base.PortClosed += new EventHandler(NitonXL800_PortClosed);
base.PortOpened += new EventHandler(NitonXL800_PortOpened);
}

#region Event Handler Pairs
private void NitonXL800_DataReceived(object sender, ComDeviceEventArgs e)
{
OnDataReceived(e);
}
private void OnDataReceived(ComDeviceEventArgs e)
{
if (DataReceived != null)
DataReceived(this, e);
}
void NitonXL800_PortOpened(object sender, EventArgs e)
{
OnPortOpened(e);
}
private void OnPortOpened(EventArgs e)
{
if (PortOpened != null)
PortOpened(this, e);
}
void NitonXL800_PortClosed(object sender, EventArgs e)
{
OnPortClosed(e);
}
private void OnPortClosed(EventArgs e)
{
if (PortClosed != null)
PortClosed(this, e);
}
#endregion
#region Properties
public new string Buffer
{ get { return base.Buffer; } }
public bool PortOpen
{
get { return base.IsOpen; }
}
public int Timeout
{
get { return base.ReadTimeout; }
set { base.ReadTimeout = value; }
}
#endregion
#region Util
public void Read(bool close)
{
CloseOnRecieve = close;
base.OpenPort();
}
public void Read()
{
Read(true);
}
public void CancelRead()
{
// TDODO: Cancel read
}
#endregion
#region Overrides
public override string ToString()
{
return string.Format("{0} {0} in port {2}", base.Manufacturer, base.Name, base.PortName);
}
#endregion
#region Events
public new event ComDeviceDataReceivedEventHandler DataReceived;
public new event EventHandler PortClosed;
public new event EventHandler PortOpened;
#endregion
}


It's an incomplete implementation to say the least. Criticism aside, my problems arise below when I attempt to post e.DataRecevied to a control.
public partial class frmTestBed : Form
{
NitonXL800 tester;

public frmTestBed()
: this(null) { }
public frmTestBed(NitonXL800 gun)
{
InitializeComponent();

if (gun != null) tester = gun;
else tester = new NitonXL800();

tester.DataReceived += new ComDeviceDataReceivedEventHandler(tester_DataReceived);
tester.PortClosed += delegate(object sender, EventArgs e) { StopProgress(); };
tester.PortOpened += delegate(object sender, EventArgs e) { StartProgress(); };
}

void tester_DataReceived(object sender, ComDeviceEventArgs e)
{
string received = string.Empty;
GetData updater = delegate(string data) { txtOutput.Text = data; StopProgress(); };
txtOutput.BeginInvoke(updater, e.DataRecieved);
}
private void StartProgress()
{
pgReading.Style = ProgressBarStyle.Marquee;
pgReading.Value = 10;
}
private void StopProgress()
{
pgReading.Style = ProgressBarStyle.Blocks;
pgReading.Value = 0;
}

delegate void GetData(string data);
}

Oddly enough, getting the data to a control is easy, BeginInvoke works like a charm. But there in lies my moral dilemma: I'm an architect; a framework developer; I can't force those who write applications that target my framework to understand asynchronous programming, especially if my own knowledge of it is still not fully developed. Ideally, all any developer would have to do is hook up the DataReceived event of the NitonXL800 class, call Read(), and wait for DataReceived to fire so s/he can drop that info into whatever container requires it. I can't have some random threading exception when they don't use BeginInvoke.
As far as I can tell at the moment, I need some sort of liaison--Something that implements ISynchronizeInvoke that can marshal over the data into the executing thread before the event exits, so that the form below could care less about where the data actually came from.
The reason for this is (at least to me) somewhat interesting. As it turns out, even calls that have nothing to do with the data being retrieved by the event throw exceptions. Of course it's mostly assumption, but I have a sneaking suspicion that it's not the data that is owned by another thread, but the method that is executing in another thread. What this means is that with the design lined out above, I can get the data to the TextBox, but the StopProgress throws an InvalidOperationException since it's being called by the DataReceived event's thread.
After slapping myself in the head for my ignorance of such a simple concept, I determined that the assistance of someone more versed in this than me was required.

...And as soon as I find this person, I'll be sure to post the solution right way...

No comments: