-1

I have a background task that sits in a loop reading data from an external hardware device. The device exposes a "counter" that increments every 100ms - when this value changes I fetch the next data value from the device and raise an event. Psuedo-code:-

public event HasDataEventArgs DataReceived;

while (some condition)
{
    // Wait for device counter to increment
    while (DeviceCounterValue == _lastDeviceCounterValue)
    {
       Thread.Sleep(3);
    }
    _lastDeviceCounterValue = DeviceCounterValue;

    // Read data value from device
    var data = GetDataFromDevice();

    // Raise my event
    DataReceived(this, new HasDataEventArgs(data)); 
}

I also have a UI view that subscribes to this event. The event handler plots the data value on a chart and sets a number of bound properties.

Most of the time this all works fine, but if I (say) drag a window around or open a modal dialog, it can occasionally result in data being missed. What seems to happen is that the external device counter carries on incrementing, but the 'while' loop has effectively stalled briefly, so misses those changes. Very occasionally I'll see the same effect even when I'm not messing around with anything in the UI.

The event handler in the view isn't doing much, but this is a complex desktop application with other background threads updating other bound controls. Perhaps this particular process is just tipping things over the edge performance-wise, especially once I start dragging windows around?

I was wondering if I could wrap the event handler code in a Task.Run(), which (I assume) would result in it returning control to the while loop immediately, rather than have to wait for the event handler to do its thing. It sounds hacky though - am I asking for trouble with something like this, especially given the frequency that the event handler will be called (every 100ms)?

Andrew Stephens
  • 9,413
  • 6
  • 76
  • 152
  • Do you look for `Invoking` the UI? https://stackoverflow.com/questions/11625208/accessing-ui-main-thread-safely-in-wpf?utm_medium=organic&utm_source=google_rich_qa&utm_campaign=google_rich_qa – Smartis has left SO again May 09 '18 at 11:46
  • A very standard bug in WPF is to use Dispatcher.Invoke() instead of BeginInvoke(). Bit of a design mistake to give the method that is wrong 99% of the time the shortest name. – Hans Passant May 09 '18 at 11:49
  • @Smartis I'm only updating bound properties so Dispatcher.Invoke() isn't necessary – Andrew Stephens May 09 '18 at 11:49
  • @HansPassant I'm not using Dispatcher at all as I'm only updating bound properties. My understanding is that WPF takes care of dispatching to the UI thread itself in this scenario. It's all working fine in that respect, and I'm not getting the typical "the calling thread cannot access this object..." errors. – Andrew Stephens May 09 '18 at 11:51
  • 1
    You must use BeginInvoke() to ensure that the worker thread cannot be bogged down by the UI thread. – Hans Passant May 09 '18 at 11:52
  • @HansPassant that's interesting, I had always assumed Invoke/BeginInvoke was only used to avoid the "calling thread cannot access..." issue and hadn't considered it here. – Andrew Stephens May 09 '18 at 11:54
  • Possible duplicate of [How do I update the GUI from another thread?](https://stackoverflow.com/questions/661561/how-do-i-update-the-gui-from-another-thread) – Lucifer May 09 '18 at 11:59

2 Answers2

2

You left out some details, mainly what the eventhandler does to get on the GUI thread. Invoke or BeginInvoke etc.

But there is another option, if safeguarding your data matters most: push the new data into a ConcurrentQueue. Raising a Received event is then OK but optional, you probably won't need it.

The Main thread can empty the queue in its own time. For example with a Timer.

Your screen updates will still stutter but you shouldn't lose data anymore.

bommelding
  • 2,969
  • 9
  • 14
  • See comments above re Invoke. I had thought about implementing a queue but looking for a simpler solution first if possible. – Andrew Stephens May 09 '18 at 11:53
  • 1
    A queue is relatively simple. Decoupling stuff is always a good idea, especially when the MessageLoop meets background threads. – bommelding May 09 '18 at 11:58
  • 2
    "The Main thread can empty the queue in its own time" is an important point here. While the producer may add single items at 10 Hz, the consumer is free to read 10 items each second. – Clemens May 09 '18 at 12:05
  • Rather than a `ConcurrentQueue` with a timer, consider a `BlockingCollection`. – mjwills May 09 '18 at 12:38
  • `For example with a Timer.` Using a `ConcurrentQueue` means you need something like a `Timer` to be polling the queue. `Got anything for me? What about now? What about now? What about now? What about now?` `BlockingCollection` avoids that issue. https://stackoverflow.com/a/40344152/34092 – mjwills May 15 '18 at 06:55
  • Before we continue with this discussion, can you just clarify whether you've used a `BlockingCollection` before? I am worried you are understanding `Blocking` in the name to mean something slightly different to my understanding. – mjwills May 15 '18 at 07:40
1

You have to split (thread-wise) two things: the background work and the plotting work. This is, in general, the way how to do such things, but to be concrete - if your plotting requires time, then there are chances that your working thread is not able to handle the incoming data on time and you can get some data lost/omitted (that's what you're actually observing).

Here's one way how to do it (the method must be a member of a UI class - Window, user control, etc.):

void OnDataReceived(object sender, DataEventArgs e)
{
    // here we're in the context of the working thread

    // this call will return immediately giving control back to the working thread
    Dispatcher.BeginInvoke(
        DispatcherPriority.Normal,
        (Action)delegate
        {
            // here we are in the context of the UI thread
        });
}
armenm
  • 922
  • 6
  • 9