Worker thread and controls

[Cralis]

Hi guys,

I write a small winforms application that reads data from a device via
a comms cable, and displays data in a grid, labels and other controls.
So, I send a query, such a $Param1, and the device returns a value for
Param1, which I then display in either a cell in a grid view, or else
a label.

Problem is, there's a small split second delay while I do the query at
the moment, as I am doing it in the main thread.

I have split my application into layers, so to make the call, I call a
static class method:

public static string GetParametersValue(string ParamName);

That then builds up a string and sends that to the device, and gets a
reply, which it returns to my GUI.

I use BackGround worker threads for a few WebService calls I make,
which work fine. But now.. I need to use background threads for my
calls to my device. That should be fine - my problem is, I don't think
the thread can access the controls on my form. Certain parameter
types (calls) get populated in certain controls. I think I need to
make an 'UpdateControl type method, which is somehow threadsafe (??) -
but how can I make it nice and genertic allowing it to recieve maybe a
value and a control name (?) which the read can call, to safely update
the GUI?

 

[Arne Vajhøj]

I write a small winforms application that reads data from a device via
a comms cable, and displays data in a grid, labels and other controls.
So, I send a query, such a $Param1, and the device returns a value for
Param1, which I then display in either a cell in a grid view, or else
a label.

Problem is, there's a small split second delay while I do the query at
the moment, as I am doing it in the main thread.

I have split my application into layers, so to make the call, I call a
static class method:

public static string GetParametersValue(string ParamName);

That then builds up a string and sends that to the device, and gets a
reply, which it returns to my GUI.

I use BackGround worker threads for a few WebService calls I make,
which work fine. But now.. I need to use background threads for my
calls to my device. That should be fine - my problem is, I don't think
the thread can access the controls on my form. Certain parameter
types (calls) get populated in certain controls. I think I need to
make an 'UpdateControl type method, which is somehow threadsafe (??) -
but how can I make it nice and genertic allowing it to recieve maybe a
value and a control name (?) which the read can call, to safely update
the GUI?

If you need to update a control from a another thread then
you need to use Invoke.

Arne

 

[Cralis]

So I don't need complicated delegates and all that?

I can create a method in the GUI class that accept an object, and a
value.

Within that method, I can then get the type of the object.. and if
it's a Label, cast it as a label, and do ((Label)myObject).Text =
incomingText; ?

 

[Arne Vajhøj]

So I don't need complicated delegates and all that?

With newer versions of .NET then you do it rather simple.

But you still need to call Invoke with something.

I can create a method in the GUI class that accept an object, and a
value.

Within that method, I can then get the type of the object.. and if
it's a Label, cast it as a label, and do ((Label)myObject).Text =
incomingText; ?

It is not clear to me how this relates to the thread problem.

Arne

 

[Cralis]

Thanks guys.

Pete - I'm trying to see how I can fit your example to my situation.

Here is some code I am going to be using to try the threading out.
It's a method the gets called on connection to the device, to show a
few bits of info to the user. It also gets called by a timer if the
user has selected an AutoUpdate checkbox.

private void DisplayLiveSummary(bool incServiceCalls)
{

lblSoftwareVersion.Text = C_VERSION_NUMBER;
if (DeviceServices.IsConnected())
{
ReplyString rep = DeviceServices.GetFirmwareVerison();
lblFirmwareVersion.Text = rep.Result ?
rep.ReplyValue : "Unable to get Version";
Common.DeviceVersion ver =
DeviceServices.GetDeviceVersion();
switch (ver)
{
case Common.DeviceVersion.V1:
lblHardwareVersion.Text = "DOSD+ V1";
break;
case Common.DeviceVersion.V2:
lblHardwareVersion.Text = "DOSD+ V2";
break;
default:
lblHardwareVersion.Text = "Unable to get
Version";
break;
}

ReplyFloat v1 = DeviceServices.GetVoltage(1);
lblV1.Text = v1.Result ? string.Format("{0:f}",
v1.ReplyValue) : "Unable to read Voltage";

ReplyString gps =
DeviceServices.GetParameterFromDevice("HAVEGPS");
imgGPS.Image = imageList1.Images[gps.Result ?
(gps.ReplyValue == "1" ? C_IMAGE_YES : C_IMAGE_NO) : C_IMAGE_UNKNOWN];
bool haveGps = gps.ReplyValue == "1" ? true : false;

ReplyFloat mem = DeviceServices.GetMemoryUsage();
lblMem.Text = mem.Result ? String.Format("{0:f}%
Free", mem.ReplyValue) : "Unable to detect";


}

In that code, you can see the issues. I am setting
lblSoftwareVersion.Text directly (for example), where
lblSoftwareVersion is a Label.
I also set an image object based on a reply: imgGPS.Image =
imageList1.Images

Both of these examples need to be thread safe.

Calls to my static device service (The class I use that has all the
functions I can call on my device) are made like this:

Common.DeviceVersion ver = DeviceServices.GetDeviceVersion();

(Common.DeviceVersio is a custom type I use)


Now, looking at your code, and trying to see how I can impliment that.
I think it's something in these lines:


void MethodThatWillRefreshScreen() // This is the method that I will
call to do a screen refresh.
{
MySpecialDataType result;
BackgroundWorker worker = new BackgroundWorker();

worker.DoWork += (senderWorker, eWorker) =>
{
// do something that eventually sets "result"
// For example, set it to the return value of
// some method that does the actual background
// work:
result = DoBackgroundWork();
};

worker.RunWorkerCompleted += (senderWorker, eWorker) =>
{
// do something that uses "result". E.g. just call
// some method that does the actual GUI update:
FinishedBackgroundTask(result);
}
}

Actually, I have no clue what to do with this, and how it can be made
to suite my needs. Hope you can assist.

 

[Cralis]

Hey Peter - adding to this, I have 180 parameters I query from the
device. Initially, they are all queried via a DumpAll type call, which
returns a large string. I then populate all values on the screen
(Which consists of a few tabs). This is very quick, and is a one-off
call. The re-queryable parameters are parameters the users ask for, to
be displayed on a 'Live info' type tab. This is a subset of the 180
parameters (At the moment, around 15 parameters are live).

So, would it be acceptable design to create a LiveData object, which
simply has all the paramerters that are Live.. so, for now, 15
properties, such as 'Altitude', 'Latitude' etc etc. The class can then
have a 'LoadMe' type method, which populates all the values. Once
loaded, I then go though them and update the GUI controls with their
values.

So...

- Create the object with the 15 parameters. Something like:

public class LiveDataObject()
{
float Altitude {Get; Set;}
float Latitude {Get; Set; }
bool hasGpsModuleConnected {get; set;}

...
}

- Add a LoadMe method that loads all the parameters

public class LiveDataObject()
{
float Altitude {Get; Set;}
float Latitude {Get; Set; }
bool hasGpsModuleConnected {get; set;}

...


public void LoadMe()
{
ReplyString rep=
DeviceServices.GetParameterFromDevice("HAVEGPS");
hasGpsModule = rep.ReplyValue;

ReplyString rep=
DeviceServices.GetParameterFromDevice("ALTITUDE");
Altitude = rep.ReplyValue;

... etc setting all properties.
}
}

So, when ever the user decides they want a new parameter displayed
live, I simply add the label/image, and then add the parameter
property and LoadMethod call.

Does this seem like the correct direction?

 

[Cralis]

Thanks Peter.

I have now refactored, and have created a class which on creation,
populates all it's own properties with data from the device:


class LiveItems
{
private string _voltage1;
private string _voltage2;
private GpsPosition _currentPosition;
private GpsPosition _homePosition;
private string _currentAltitude;
private string _homeAltitude;
private string _satCount;
private bool? _homeSet;
private string _memory;


public string Voltage1
{
get { return _voltage1; }
}

etc etc. Each property has a publig getter.

The constructor then goes and get's all the data from the device:

public LiveItems()
{

_currentPosition = new GpsPosition();
_homePosition = new GpsPosition();

ReplyFloat v1 = DeviceServices.GetVoltage(1);
_voltage1 = v1.Result ? string.Format("{0:f}",
v1.ReplyValue) : "Unable to read Voltage";

ReplyFloat v2 = DeviceServices.GetVoltage(2);
_voltage2 = v2.Result ? string.Format("{0:f}",
v2.ReplyValue) : "Unable to read Voltage";

etc etc for each property I need,

Then, my GUI hasb't been threaded yet. But this is what I have...

This is the method that I have created to populate the live values:

private void PopulateLiveDate()
{

LiveItems li = new LiveItems();

lblV1.Text = li.Voltage1;
lblV2.Text = li.Voltage2;
lblSats.Text = li.SatCount;
lblAlt.Text = li.CurrentAltitude;
lblMem.Text = li.Memory;

imgHomeSet.Image = imageList1.Images[li.HomeSet != null ?
(li.HomeSet.Value ? C_IMAGE_YES : C_IMAGE_NO) : C_IMAGE_UNKNOWN];

}


Does this seem OK so far?

 

[Cralis]

And here's my attempt at the thread, but it's freezing up.. and I have
no idea why..

private void PopulateLiveDate()
{

BackgroundWorker worker = new BackgroundWorker();
worker.DoWork += PopulateLiveDateWorkerThread;
worker.RunWorkerCompleted +=
PopulateLiveDataCompleteThread;
worker.WorkerReportsProgress = true;
worker.RunWorkerAsync();
}

private void PopulateLiveDateWorkerThread(object sender,
DoWorkEventArgs e)
{
LiveItems li = new LiveItems();
e.Result = li;
}


private void PopulateLiveDataCompleteThread(object sender,
RunWorkerCompletedEventArgs e)
{
LiveItems li = (LiveItems)e.Result;

lblV1.Text = li.Voltage1;
lblV2.Text = li.Voltage2;
lblSats.Text = li.SatCount;
lblAlt.Text = li.CurrentAltitude;
lblMem.Text = li.Memory;

imgHomeSet.Image = imageList1.Images[li.HomeSet != null ?
(li.HomeSet.Value ? C_IMAGE_YES : C_IMAGE_NO) : C_IMAGE_UNKNOWN];

}

 

[Cralis]

Pete - thanks again for your time. My Internet dropped for a long
time, so I had time to play about. The freeze seemed to be related to
my log file logging. I think I was logging from the Background
thread,a s well as some GUI thread... and it seemed I was locked while
trying to write to my log file. My logging method is far from thread
safe:

public static void WriteLog(string input)
{
try
{
if (!Directory.Exists(LogFilePath))
Directory.CreateDirectory(LogFilePath);
using (StreamWriter w = File.AppendText(LogFilePath +
@"\" + LogFileName))
{

w.WriteLine(string.Format("{0}\t{1}",
DateTime.Now, input));
w.Flush();
}
}
catch (Exception)
{

}
}

I'll need to work on that, but for now, I am disabling the logging.

Since doing that - no more freezing.

However, the issue I am left with is that even when I exit the app
gracefully, the debugger remains 'Running'. When I try break out,
Visual Studio (2010 Ultimate) freezes up, and then hits me with a
'Debugger still busy' but offers me a 'Stop' option. I stop it, and
the debugger returns, but I find that a thead (I assume the one I have
created) is still alive. Reason is, I can no longer connect to the COM
Port (As it's being used by another process). I then check my
processes, and see a myapp.vshost.exe running - and I can't kill it
(Access Denied). Even pskill -t won't do it.

I have to reboot...

So, looks like I am starting a thread, but it's not dying when I exit
the app....

 

[Cralis]

Also, when it freezes - how do I see the threads? That would be
helpful, and I am getting lockups a lot now - even after5 commenting
out the Logging.

 

[Cralis]

Thanks Peter - I'm absorbing a few lessons of you here, if that's OK.

FWIW, there's no need to call Directory.Exists().  Just call
CreateDirectory().  It will do nothing if the directory already exists.


As you may realize, the above is dangerous in non-thread-safe code.  In
particular, if you have multiple threads using the same log file, they
will interfere with each other, as only one thread can have the file
open at a time using File.AppendText() (and even if you changed it to
something that allowed file sharing, you wouldn't actually want to have
multiple threads writing to the same file at once).

I have refactored this code now, which should be more suitable.

public static void WriteLog(string input)
{
Directory.CreateDirectory(LogFilePath);
StreamWriter w = File.AppendText(LogFilePath + @"\" +
LogFileName);
try
{
w.WriteLine(string.Format("{0}\t{1}", DateTime.Now,
input));
}
catch (Exception e)
{
System.Console.WriteLine("Error writing to log
file!");
System.Console.WriteLine("Tried to write: [" + input +
"]");
System.Console.WriteLine("Failed with error: [" +
e.Message + "]");
}
finally
{
w.Close();
}
}

This, however, doesn't address the threading issue. But would this
just be a case of putting a lock(object) over the whol Try block?

 

[Cralis]

About the threads locking up:

That usually means that you've got a non-background worker thread that
you haven't terminated properly.  Any threads you create, you need to
ensure that they will be terminated when the application exits.

At the moment, I use a normal thread to handle a splash screen:

Thread th = new Thread(DoSplash);
th.Name = "SplashScreenThread";

th.Start();
Common.WriteLog("Splash Shown");

Thread.Sleep(3000);
th.Abort();
Common.WriteLog("Splash Hidden");
Thread.Sleep(1000);

where DoSplash is simply a method that does:

private static void DoSplash()
{
Splash sp = new Splash();
sp.SetVersion(C_VERSION_NUMBER, C_BUILD_DATE,
C_DEVELOPER);
sp.ShowDialog();
}


That works pretty well... so far, no hassles.

All the rest of my threads are BackGroundWorkers... Would it be useful
to change these to normal threads, as as you say, set the IsBackground
property to true? Do I still have the same DoWork, Complete and
Progress ways of doing things, or would it be quite a major change?
Also, is setting IsBackground not the same as simply having a
BackgroundWorker thread?

Sorry, that problem description is unfamiliar to me.  I'm not sure what
you mean by "freezes up" and "hits me…".  A more specific transcript of
the exact wording of all messages the IDE provides would be useful.

I need to reboot my PC as a thread is locked. I am an admin, but just
can't kill the process. pskill reports it killed, but when I try run
my application again, my COM Port is 'In use by another application'.
TaskManager shows me a a process called MyApplication.vshost.exe
(PID1408, and the username is mine). I try and End Process it, and it
asks me if I want to stop that process. I select Yes, but it won't go
away. (I used to get Access Denied, but now it simply doesn't go
away).

Let me try get the Visual Studio error to happen again when I try
terminate my application (SHIFT+F5). Once I get it, I will report.

 

[Cralis]

[...]
This, however, doesn't address the threading issue. But would this
just be a case of putting a lock(object) over the whol Try block?

I would protect the entire method, not just the try/catch/finally block.
  Otherwise, you can still get file access exceptions just creating the
StreamWriter.

But yes, using a "lock" to synchronize the method should address the
thread-safety issue.

Pete

Thanks Pete. I took your advice, I think, and now have this:

public static void WriteLog(string input)
{
object o = new object();
lock (o)
{
WriteLogThreadSafe(input);
}
}

private static void WriteLogThreadSafe(string input)
{
Directory.CreateDirectory(LogFilePath);
StreamWriter w = File.AppendText(LogFilePath + @"\" +
LogFileName);
try
{
w.WriteLine(string.Format("{0}\t{1}", DateTime.Now,
input));
}
catch (Exception e)
{
System.Console.WriteLine("Error writing to log
file!");
System.Console.WriteLine("Tried to write: [" + input +
"]");
System.Console.WriteLine("Failed with error: [" +
e.Message + "]");
}
finally
{
w.Close();
}

}

However, I get this:
The process cannot access the file 'C:\ProgramData\DragonLabs
\PCCommander\PCCommander.Debug.txt' because it is being used by
another process.

Seems the lock isn't working. I may misunderstand it's use? I know
that more than one thread is active at this point ... but thought the
lock would stop an issue?

 

[Cralis]

It's a static class which holds this method - does that effect it?

 

[Cralis]

The message I get, when I try break out of a locked application is:

"Stop Debugging in Progress - Debugging is being stopped but is not
yet complete. You can force debugging to stop immediatly, but any
prococesses being detached may be terminated instead.

This window will automatically close when the debugging has completely
stopped"

This, of course, never happens.

 

[Cralis]

Thanks Peter!
After a few minutes of testing using my private static object
_lockObject = new object();, and then using that... I have had zero
lockups. I've applied that to both my Logging method, as well as the
shared method I use to send and recieve data to my device! And no more
freezing debug sessions... no more lingering processes... I'm going
to test a lot more... and hammer it a bit, and see how we go. Please
keep an eye here for more tears, but so far - looking great. You've
been a fantastic help, and highly patient with me. Thanks!

 

[Cralis]

Thanks. Busy hammering it a bit. Just for some background, I do some
free work for these guys who make auto pilot and onboard diagnostics
for remote controlled planes, which then overlays the data on screen.
I don't do the boards, but the app I work on basically configures the
boards, handles the GPS download data which is transmitted video video
signal (Embedded somehow), and sets up the parameters for the plane to
fly nicely. Here's a video of mine flying with the software. Note: I
fly with goggles, so what you see, I see.

You're helping me get that right.