views:

263

answers:

3

First question here so hello everyone.

The requirement I'm working on is a small test application that communicates with an external device over a serial port. The communication can take a long time, and the device can return all sorts of errors.

The device is nicely abstracted in its own class that the GUI thread starts to run in its own thread and has the usual open/close/read data/write data basic functions. The GUI is also pretty simple - choose COM port, open, close, show data read or errors from device, allow modification and write back etc.

The question is simply how to update the GUI from the device class? There are several distinct types of data the device deals with so I need a relatively generic bridge between the GUI form/thread class and the working device class/thread. In the GUI to device direction everything works fine with [Begin]Invoke calls for open/close/read/write etc. on various GUI generated events.

I've read the thread here (How to update GUI from another thread in C#?) where the assumption is made that the GUI and worker thread are in the same class. Google searches throw up how to create a delegate or how to create the classic background worker but that's not at all what I need, although they may be part of the solution. So, is there a simple but generic structure that can be used?

My level of C# is moderate and I've been programming all my working life, given a clue I'll figure it out (and post back)... Thanks in advance for any help.

+2  A: 

You can expose a public method on your UI class that the device class can call on the background thread with all the information it needs to pass to the UI. That public method will be executed in the context of the background thread, but since it belongs to the UI class, you can now employ any of the call marshaling techniques you've read about.

Thus, the simplest design then would be:

  • add a method to your UI class (for example MyUIForm)called something like UpdateUI() that takes whatever data structure you are using to pass the data from the device to the UI you use. You can declare that method in an interface (for example IUIForm), if you want to support DI/IoC later, and have the form implement it.
  • on thread A (the UI thread), your UI class creates the device class, initializes all the necessary settings and starts its background thread. It also passes a pointer to itself.
  • on thread B, the device collects the data and calls MyUIForm.UpdateUI() (or IUIForm.UpdateUI()).
  • UpdateUI does Invoke or BeginInvoke as appropriate.

Note that has the side benefit of encapsulating all the UI and presentation logic in your UI class. Your device class can now focus on dealing with the hardware.

Update: To address your scalability concerns -

No matter how much your app grows and how many UI classes you have, you still want to cross the thread boundary using the BeginInvoke for the particular UI class you want to update. (That UI class might be a specific control or the root of a particular visual tree, it does not really matter) The main reason is if you have more than one UI threads, you have to ensure the update of any UI happens on the thread this particualr UI was created on, due to the way Windows messaging and windows work. Hence, the actual logic of crossing the boundary thread should be encapsulated in the UI layer.

You device class should not have to care what UI classes and on which thread need to be updated. In fact, I personally would make the device fully ignorant about any UI and would just expose events on it that different UI classes can subscribe on.

Note that the alternative solution is to make the threading fully encapsulated in the device class and make the UI ignorant about the existence of a bacground thread. However, then thread boundary crossing then becomes responsibility of the device class and should be contained within its logic, so you shouldn't be using the UI way of crossing the threads. That also means your device class is bound to particular UI thread.

Franci Penov
That's probably the simplest method, and it works well in the beginning and I've used it before, but it has the disadvantage that once the GUI becomes bigger and consists of more classes or UserControls (which inevitably seems to happen) that it doesn't scale well.Perhaps I should have specified in my question that the reason I'm looking for a generic solution is I fully expect this application to grow with time and would like something that scales.
Toby Martin
A: 

This is a version with an event handler.
It is simplified so there is no UI controls in the form and no properties in the SerialIoEventArgs class.

  1. Place your code to update the UI under the comment // Update UI
  2. Place your code to read serial IO under the comment // Read from serial IO
  3. Add fields/properties to SerialIoEventArgs class and populate it in method OnReadCompleated.
public class SerialIoForm : Form
{
    private delegate void SerialIoResultHandlerDelegate(object sender, SerialIoEventArgs args);
    private readonly SerialIoReader _serialIoReader;
    private readonly SerialIoResultHandlerDelegate _serialIoResultHandler;

    public SerialIoForm()
    {
        Load += SerialIoForm_Load;
        _serialIoReader = new SerialIoReader();
        _serialIoReader.ReadCompleated += SerialIoResultHandler;
        _serialIoResultHandler = SerialIoResultHandler;
    }

    private void SerialIoForm_Load(object sender, EventArgs e)
    {
        _serialIoReader.StartReading();
    }
    private void SerialIoResultHandler(object sender, SerialIoEventArgs args)
    {
        if (InvokeRequired)
        {
            Invoke(_serialIoResultHandler, sender, args);
            return;
        }
        // Update UI
    }
}
public class SerialIoReader
{
    public EventHandler ReadCompleated;
    public void StartReading()
    {
        ThreadPool.QueueUserWorkItem(ReadWorker); 
    }
    public void ReadWorker(object obj)
    {
        // Read from serial IO

        OnReadCompleated();
    }

    private void OnReadCompleated()
    {
        var readCompleated = ReadCompleated;
        if (readCompleated == null) return;
        readCompleated(this, new SerialIoEventArgs());
    }
}

public class SerialIoEventArgs : EventArgs
{
}
Jens Granlund
A: 

So, after some research based on the answers above, further Google searching and asking a colleague who knows a bit about C# my chosen solution to the problem is below. I remain interested in comments, suggestions and refinements.

First some further detail about the problem, which is actually pretty generic in the sense that the GUI is controlling something, that must remain wholly abstract, through a series of events to whose responses the GUI must react. There a a few distinct problems:

  1. The events themselves, with different data types. Events will get added, removed, changed as the program evolves.
  2. How to bridge several classes that comprise the GUI (different UserControls) and the classes that abstract the hardware.
  3. All classes can produce and consume events and must remain decoupled as far as possible.
  4. The compiler should spot coding cockups as far as possible (eg. an event that sends one data type but a comsumer that expects another)

The first part of this is the events. As the GUI and the device can raise several events, possibly having different data types associated with them, an event dispatcher is handy. This must be generic in both events and data, so:

    // Define a type independent class to contain event data
    public class EventArgs<T> : EventArgs
    {
    public EventArgs(T value)
    {
        m_value = value;
    }

    private T m_value;

    public T Value
    {
        get { return m_value; }
    }
}

// Create a type independent event handler to maintain a list of events.
public static class EventDispatcher<TEvent> where TEvent : new()
{
    static Dictionary<TEvent, EventHandler> Events = new Dictionary<TEvent, EventHandler>();

    // Add a new event to the list of events.
    static public void CreateEvent(TEvent Event)
    {
        Events.Add(Event, new EventHandler((s, e) => 
        {
            // Insert possible default action here, done every time the event is fired.
        }));
    }

    // Add a subscriber to the given event, the Handler will be called when the event is triggered.
    static public void Subscribe(TEvent Event, EventHandler Handler)
    {
        Events[Event] += Handler;
    }

    // Trigger the event.  Call all handlers of this event.
    static public void Fire(TEvent Event, object sender, EventArgs Data)
    {
        if (Events[Event] != null)
            Events[Event](sender, Data);

    }
}

Now we need some events and coming from the C world, I like enums, so I define some events that the GUI will raise:

    public enum DEVICE_ACTION_REQUEST
    {
    LoadStuffFromXMLFile,
    StoreStuffToDevice,
    VerifyStuffOnDevice,
    etc
    }

Now anywhere within the scope (namespace, typically) of the static class of the EventDispatcher it is possible to define a new dispatcher:

        public void Initialize()
        {
        foreach (DEVICE_ACTION_REQUEST Action in Enum.GetValues(typeof(DEVICE_ACTION_REQUEST)))
            EventDispatcher<DEVICE_ACTION_REQUEST>.CreateEvent(Action);
        }

This creates an event handler for each event in the enum.

And consumed by subscribing to the event like this code in the constructor of the consuming Device object:

        public DeviceController( )
    {
        EventDispatcher<DEVICE_ACTION_REQUEST>.Subscribe(DEVICE_ACTION_REQUEST.LoadAxisDefaults, (s, e) =>
            {
                InControlThread.Invoke(this, () =>
                {
                    ReadConfigXML(s, (EventArgs<string>)e);
                });
            });
    }

Where the InControlThread.Invoke is an abstract class that simply wraps the invoke call.

Events can be raised by the GUI simply:

        private void buttonLoad_Click(object sender, EventArgs e)
        {
            string Filename = @"c:\test.xml";
            EventDispatcher<DEVICE_ACTION_REQUEST>.Fire(DEVICE_ACTION_REQUEST.LoadStuffFromXMLFile, sender, new EventArgs<string>(Filename));
        }

This has the advantage that should the event raising and consuming types not match (here the string Filename) the compiler will grumble.

There are many enhancements that can be made but this is the nuts of the problem. I'd be interested, as I said in comments, especially if there are any glaring omissions/bugs or deficiencies. Hope this helps someone.

Toby Martin
It seems to me that you're trying to bolt C semantics onto C#. It's not productive. You could've simply added events to ComSer class and .BeginInvoke (asynchronous) them and the form would've subscribed to them etc. Now you have this hideous thing with your own semantics that nobody coming natively from C# background sees any reason for. I'm sorry this is harsh and no offence meant.
Pasi Savolainen
Thanks for the comment. Although you can evidently provide reviews in the name of the entire C# world I have Googled the pertinent points from your suggestion I'm still none the wiser as to how they would help.
Toby Martin