Using Live Charts, I am creating a realtime graph which updates with values read in from the serial port. Now I can get this to work but I don't think I am doing this as efficiently as I could as I am inexperienced using C# and WPF.
I am storing the data I am reading in from the serial port in a SerialCommunication class. I am then using a button to start a new task which opens the serial port and updates my graph.
My issue is that I want to be able to update the graph everytime the Serial class receives a new value, however, my chart is updated in the Read() function which is called from starting a new task and I feel this may cause threading issues. Any advice would be appreciated.
Serial class
public class SerialCommunication
{
private string _value;
SerialPort serialPort = null;
public SerialCommunication()
{
InitializeComms();
}
private void InitializeComms()
{
try
{
serialPort = new SerialPort("COM6", 115200, Parity.None, 8, StopBits.One); // Update this to avoid hard coding COM port and BAUD rate
serialPort.DataReceived += new SerialDataReceivedEventHandler(DataReceivedHandler);
}
catch (Exception ex) { MessageBox.Show(ex.Message); }
}
~SerialCommunication()
{
if(serialPort.IsOpen)
serialPort.Close();
}
public void ReceiveData()
{
try
{
if (!serialPort.IsOpen)
serialPort.Open();
}
catch(Exception ex) { MessageBox.Show(ex.Message); }
}
public void StopReceivingData()
{
try
{
if (serialPort.IsOpen)
serialPort.Close();
}
catch(Exception ex) { MessageBox.Show(ex.Message); }
}
public event EventHandler DataReceived;
private void OnDataReceived(EventArgs e)
{
DataReceived?.Invoke(this, e);
}
// read the data in the DataReceivedHandler
// Event Handler
public void DataReceivedHandler(object sender, SerialDataReceivedEventArgs e)
{
try
{
SerialPort sp = (SerialPort)sender;
_value = sp.ReadLine();
}
catch (Exception ex) { MessageBox.Show(ex.Message); }
OnDataReceived(EventArgs.Empty);
}
}
Time of Flight class which updates chart from sensor values read from serial port, using code taken from LiveCharts by beto-rodriguez
public TimeOfFlight()
{
InitializeComponent();
// attach an event handler to update graph
serial.DataReceived += new EventHandler(UpdateChart);
// Use PlotData class for graph data which will use this config every time
var mapper = Mappers.Xy<PlotData>()
.X(model => model.DateTime.Ticks)
.Y(model => model.Value);
// Save mapper globally
Charting.For<PlotData>(mapper);
chartValues = new ChartValues<PlotData>();
//lets set how to display the X Labels
XFormatter = value => new DateTime((long)value).ToString("hh:mm:ss");
YFormatter = x => x.ToString("N0");
//AxisStep forces the distance between each separator in the X axis
AxisStep = TimeSpan.FromSeconds(1).Ticks;
//AxisUnit forces lets the axis know that we are plotting seconds
//this is not always necessary, but it can prevent wrong labeling
AxisUnit = TimeSpan.TicksPerSecond;
SetAxisLimits(DateTime.Now);
//ZoomingMode = ZoomingOptions.X;
IsReading = false;
DataContext = this;
}
public ChartValues<PlotData> chartValues { get; set; }
public Func<double, string> XFormatter { get; set; }
public Func<double, string> YFormatter { get; set; }
public double AxisStep { get; set; }
public double AxisUnit { get; set; }
public double AxisMax
{
set
{
_axisXMax = value;
OnPropertyChanged("AxisMax");
}
get { return _axisXMax; }
}
public double AxisMin
{
set
{
_axisXMin = value;
OnPropertyChanged("AxisMin");
}
get { return _axisXMin; }
}
public bool IsReading { get; set; }
private void StartStopGraph(object sender, RoutedEventArgs e)
{
IsReading = !IsReading;
if (IsReading)
{
serial.ReceiveData();
}
else
serial.StopReceivingData();
}
public void UpdateChart(object sender, EventArgs e) // new task
{
try
{
var now = DateTime.Now;
// can chartValues.Add be called from INotifyPropertyChanged in
// SerialCommunication.cs and would this cause an issue with the
chartValues.Add(new PlotData
{
DateTime = now,
Value = 0 // update this
});
SetAxisLimits(now);
//lets only use the last 150 values
if (chartValues.Count > 1000) chartValues.RemoveAt(0);
}
catch (Exception ex) { MessageBox.Show(ex.Message); }
}
private void SetAxisLimits(DateTime now)
{
AxisMax = now.Ticks + TimeSpan.FromSeconds(1).Ticks; // lets force the axis to be 1 second ahead
AxisMin = now.Ticks - TimeSpan.FromSeconds(8).Ticks; // and 8 seconds behind
}
public event PropertyChangedEventHandler PropertyChanged;
protected virtual void OnPropertyChanged(string propertyName = null)
{
if (PropertyChanged != null)
PropertyChanged.Invoke(this, new PropertyChangedEventArgs(propertyName));
}
}
PlotData class
public class PlotData
{
public DateTime DateTime { get; set; }
public int Value { get; set; }
}
Yeah, not the best way IMO.
First of all I wouldn't put anything to do with INotifyPropertyChanged or business/view logic in your SerialCommunication class, from an architectural perspective all it should do manage the opening and closing of the serial device, and any data that does arrive should be passed on other parts of your app via an event.
Secondly, why are you using a loop? You've already subscribed to DataReceived, so just read the data in your DataReceivedHandler and pass it on to said event. That handler will be called on a different thread, but the handlers that subscribed to your event can dispatch to the main GUI thread if they need to.
Which brings me to the third point: if you're data binding correctly (as opposed to updating your controls directly) then you shouldn't need to, just update your data values and leave it to the WPF data binding engine to update as needed.
In a real application you may want to do the updates at a certain interval, and for that you'll want to push your data values to a queue and then process them all at once. The correct way to do this is in C# with an asynchronous Task. Don't use threads, and definitely don't use Thread.Sleep....threads are an outdated technology which has no place in C# except for a few very specific circumstances.