BackgroundWorker thread and Timer logic

Question

I've been trying to get the logic right for my timer and backgroundworker thread. Granted I don't fully understand the whole system despite all my reading. the following are excerpts of code concerned: My polling button :

private void pollStart_Click(object sender, EventArgs e)
    {
        tst_bgw = new BackgroundWorker();
        //mandatory. Otherwise will throw an exception when calling ReportProgress method  
        tst_bgw.WorkerReportsProgress = true;
        //mandatory. Otherwise we would get an InvalidOperationException when trying to cancel the operation  
        tst_bgw.WorkerSupportsCancellation = true;
        tst_bgw.DoWork += tst_bgw_DoWork;
        tst_bgw.ProgressChanged += tst_bgw_ProgressChanged;
        tst_bgw.RunWorkerCompleted += tst_bgw_RunWorkerCompleted;
        tst_bgw.RunWorkerAsync();

    }

which I think is right so far

my Background worker thread:

private void tst_bgw_DoWork(object source, DoWorkEventArgs e)
    {
        m_timer = new System.Timers.Timer();
        m_timer.Interval = 1000;
        m_timer.Enabled = true;
        m_timer.Elapsed += new ElapsedEventHandler(OnTimedEvent);
        if (tst_bgw.CancellationPending)
        {
            e.Cancel = true;
            return;
        }

    }

and the elapsed tier event code:

private void OnTimedEvent(object source, ElapsedEventArgs e)
    {              
        if (powerVal > 3250)
        {
            m_timer.Stop();
            tst_bgw.CancelAsync();
        }
        else
        {
            string pow;                
            int progressVal = 100 - ((3250 - powerVal) / timerVal);
            uiDelegateTest tstDel = new uiDelegateTest(recvMessage);// the recvMessage function takes a textbox as an argument and directs output from socket to it.

            pow = construct_command("power", powerVal); 
            sData = Encoding.ASCII.GetBytes(pow);

            if (active_connection)
                try
                {
                    m_sock.Send(sData);
                    Array.Clear(sData, 0, sData.Length);
                    tstDel(ref unit_Output);// Read somewhere that you can only modify UI elements in this method via delegate so I think this is OK.
                    m_sock.Send(time_out_command);
                    tstDel(ref unit_Output);
                    tst_bgw.ReportProgress(progressVal);
                }
                catch (SocketException se)
                {
                    MessageBox.Show(se.Message);
                }
            tst_bgw.ReportProgress(powerVal, progressVal);
            powerVal = powerVal + pwrIncVal;
        }

I'd just like to know a few other things; am I using the right timer (not that I think it should matter greatly but it was suggested that this might be the best timer for what I want to do) and canI really modify UI elements in the DoWork method only through delegates and if yes are there sepcial considerations to doing so. Sorry about the long posting and thank you for your time.

Answer 1

Actually I'm not sure what the point of the background worker here is - you are using a background thread to start a timer - which by definition starts its own thread also. - so what I see happening is

• background worker thread starts
  • setting up a timer thread
  • starting the timer
• background worker thread ends
• background worker completed event fires
• Timer thread fires
  • Timer thread accesses the background worker's progress?

Yes - you have to submit delegates to the UI thread queue in order to access UI elements. I'm not sure what uiDelegateTest is, but traditionally I've always seen people use the "Invoke" method off of any UI element.

So if you have a textbox it is perfectly okay to do the following:

myTextBox.Invoke(
    new MethodInvoker(
        delegate
        {
            // Whatever code you need to do here
            myTextBox.Text = "Here is some text";
        }

I am pretty sure I am not fully understanding what you are trying to do here tho as there appears to be some code missing from your post.

Also - correct me if I'm wrong - isn't the System.Threading.Timer more ideal to use for this since it uses thread pooling?

Answer 2

There is lots wrong with this code.

1) You aren't disposing of your background worker. BackgroundWorkers must be disposed of after use. They are designed to be used as winforms components and would normally be added to a window via the designer. This will ensure it is created with the form and disposed of when the form is.
2) All you are doing in your dowork method is creating a new timer and running it. There is no point of doing this in a background worker because it will happen so quickly anyway.
3) You will recreate the timer every time you run the background worker again. But you aren't ever stopping or disposing of the old timer, you are just overwriting the member.

I recommend you get rid of the BackgroundWorker completely and just use a timer. Create the timer in the forms constructor and make sure you dispose of it in the forms dispose method. (Or use the designer to add it to the form). In the pollstart_click method just start the timer. (If you have a poll stop method, you can stop the timer in that)

Answer 3

You don't need both a BackgroundWorker and a Timer to accomplish your goal. From what you have posted it looks like you want to have the user click a button which starts a polling process that quits at a certian point.

Your polling model really suggests a timer would work just fine.

If you use a Timer I would Initialize the timer after the InitializeComponent() call with something like

private void InitializeTimer()
{
    this.timer = new Timer();
    int seconds = 1;
    this.timer.Interval = 1000 * seconds; // 1000 * n where n == seconds
    this.timer.Tick += new EventHandler(timer_Tick);
    // don't start timer until user clicks Start
}

The button_click will simply

private void button_Click(object sender, EventArgs e)
{
    this.timer.Start();
}

Then on the timer_Tick you will need to do your polling and you should be able to update your UI from there if the timer is on the UI thread like this

void timer_Tick(object sender, EventArgs e)
{
   if( determineIfTimerShouldStop() )
   {
       this.timer.Stop();
   }
   else
   {
       // write a method to just get the power value from your socket
       int powerValue = getPowerValue();

       // set progressbar, label, etc with value from method above
   }
}

However if the timer thread is not on the same thread as the UI you well get an exception while trying to update the UI. In that case you can use the Invoke that DataDink mentions and do something like this

void timer_Tick(object sender, EventArgs e)
{
   if( determineIfTimerShouldStop() )
   {
       this.timer.Stop();
   }
   else
   {
       // write a method to just get the power value from your socket
       int powerValue = getPowerValue();

       // set a label with Invoke
        mylabel.Invoke( 
            new MethodInvoker( delegate { mylabel.Text = "some string"; } )
                      );
   }
}

Given the code you posted you really didn't need to do both a BackgroundWorker and a Timer, but I have had instances where I have used a BackgroundWorker to do work when a timer is called so that I could have a timer update UI periodically and have a manual button to Refresh the UI. But I wasn't updating my UI quite the way you are.

If you still have the need to do both, here is, roughly, how you can flow your app...

• Create an InitailizeBackgroundWorker() method along with the InitializeTimer so you have it already initalized before the Timer fires.
• Then set the Timer.Tick to call the BackgroundWorker.RunWorkerAsync()
• Then you can do all the UI updates from within the RunWorkerAsync by using the BackgroundWorker.ReportProgress().