Why does my loop not allow my labels color to change?

Question

Without the do loop, my code runs fine. As soon as I place it within a do or while loop the code fails to update the color status. Any idea? From what I have gathered from the internet, my loops are written correctly.

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows.Forms;
using System.Net.NetworkInformation;
using System.Threading;



namespace SystemsUpDown
{
    public partial class MainForm : Form
    {

        public MainForm()
        {
            InitializeComponent();
        }

        bool ContinuePing = false;


        private void QuitButton_Click(object sender, EventArgs e)
        {
            this.Close();
        }

        private void StartButton_Click_1(object sender, EventArgs e)
        {
            Ping ping = new Ping();
            ContinuePing = true;
            do
            {
                try ///ping google
                {
                    PingReply reply = ping.Send("8.8.8.8");

                    if (reply.Status == IPStatus.Success)
                    {
                        GoogleStatusLabel.BackColor = Color.Green;
                    }

                }
                catch
                {
                    GoogleStatusLabel.BackColor = Color.Red;
                }

                try ///ping Yahoo!
                {
                    PingReply reply = ping.Send("www.yahoo.com");

                    if (reply.Status == IPStatus.Success)
                    {
                        YahooStatusLabel.BackColor = Color.Green;
                    }

                }
                catch
                {
                    YahooStatusLabel.BackColor = Color.Red;
                }

                try ///ping Reddit.com
                {
                    PingReply reply = ping.Send("www.reddit.com");

                    if (reply.Status == IPStatus.Success)
                    {
                        RedditStatusLabel.BackColor = Color.Green;
                    }

                }
                catch
                {
                    RedditStatusLabel.BackColor = Color.Red;
                }

                try ///ping Chive
                {
                    PingReply reply = ping.Send("www.chive.com");

                    if (reply.Status == IPStatus.Success)
                    {
                        ChiveStatusLabel.BackColor = Color.Green;
                    }

                }
                catch
                {
                    ChiveStatusLabel.BackColor = Color.Red;
                }

                try ///ping CNN
                {
                    PingReply reply = ping.Send("www.cnn.com");

                    if (reply.Status == IPStatus.Success)
                    {
                        CNNStatusLabel.BackColor = Color.Green;
                    }

                }
                catch
                {
                    CNNStatusLabel.BackColor = Color.Red;
                }

            } while (ContinuePing);

        }

        private void StopButton_Click(object sender, EventArgs e)
        {
            GoogleStatusLabel.BackColor = Color.Yellow;
            ChiveStatusLabel.BackColor = Color.Yellow;
            CNNStatusLabel.BackColor = Color.Yellow;
            RedditStatusLabel.BackColor = Color.Yellow;
            YahooStatusLabel.BackColor = Color.Yellow;
            ContinuePing = false;

        }
    }
}

Because you are not giving the UI any time to update, instead of using a do..loop, use a `System.Windows.Forms.Timer` and run it periodically. — Ron Beyer, Oct 20 '15 at 13:02
Agree. The thread will be forever busy with `StartButton_Click_1`, so even if the other event has fired and `StopButton_Click` is "in queue", it will never run because the other method never finishes. When you have fixed it, consider making the field `ContinuePing` into a `volatile` field. — Jeppe Stig Nielsen, Oct 20 '15 at 13:08

score 1 · Answer 1 · answered Oct 20 '15 at 13:01

1

Try forcing a refresh of the label after you change its color:

GoogleStatusLabel.Refresh();

answered Oct 20 '15 at 13:01

snow_FFFFFF

3,235
17
29

That did it. It leads me to another problem I think because of threads like Pieter21 mentioned. But it worked for what I asked. Thanks! – Michael Fhqwhgads Oct 20 '15 at 13:06
Correct - since your loop just continues, the ui never gets a chance to refresh. If you pinged on a separate thread, you wouldn't lock up the UI. – snow_FFFFFF Oct 20 '15 at 13:07

score 1 · Answer 2 · answered Oct 20 '15 at 13:08

1

This is probably because the loops run fast enough and so the updates are not comprehensible.

Try adding some sleep in the Loop like

//Sleep for two seconds. You can add this at end of loop.
//Or, sleep for 2 secs after pinging one site. 
// 2000 miliseconds = 2 seconds.
System.Threading.Thread.Sleep(2000);

And then check.

I would like to suggest that you use BackgroundWorker for this. As this while loop in the main thread will hang the main window form.

BackgroundWorker is suitable for such periodic updates to window form controls so that the main window does not hang. Please refer to the following link - https://msdn.microsoft.com/en-us/library/cc221403(v=vs.95).aspx

answered Oct 20 '15 at 13:08

Aman Sura

246
1
7

1

Sleeping in the UI thread will not allow the UI to update, in fact you may get a "not-responding" if the delay is long enough (5 seconds), and it doesn't need to be a single delay, the loop will cause it. `BackgroundWorker` has cross-thread issues that you need to be aware of. – Ron Beyer Oct 20 '15 at 13:14
1

The problem is that the UI message pump isn't running, so no actual updates are being done on the UI at all. The messages that would cause the display to be updated are queuing up waiting for the loop to exit, at which point the last change will be displayed once the message gets processed. – Corey Oct 20 '15 at 13:39

Pieter21 · Answer 3 · 2015-10-20T13:05:43.470

0

Your process is continuously running on a different thread than the UI thread, that does not get time to update the UI items.

Use Update/Refresh to force the UI some update time, or use sleep to let the UI thread have some time

edited Oct 20 '15 at 13:05

answered Oct 20 '15 at 13:02

Pieter21

1,765
1
10
22

I don't think this is true, no threads are started anywhere - it all occurs within a `button_click` started from the UI thread - most likely it is just the case that the UI is not updated as it is constantly running and has no time to updated. – Ric Oct 20 '15 at 13:05
What I have is the updated color, however I cannot interact with the UI because it is stuck in a constant loop now. It's another problem I'm working through now. – Michael Fhqwhgads Oct 20 '15 at 13:08
Sorry, you were right, I was too much preoccupied with what happened when I had similar issue. Who hasn't ;) . It is still a question of not having the time to update, but not on different threads – Pieter21 Oct 20 '15 at 13:09
For interacting with the UI, use the BackgroundWorker from a different answer. – Pieter21 Oct 20 '15 at 13:10
1

@MichaelFhqwhgads - ofcourse, maybe look at something like `BackGroundWorker` as has been suggested to perform the updates and consider not constantly pinging the same address! – Ric Oct 20 '15 at 13:10
Indeed, I don't think Google at 8.8.8.8 likes everybody constantly pinging, though it will surely be able to handle it. – Pieter21 Oct 20 '15 at 13:12

score 0 · Answer 4 · answered Oct 20 '15 at 15:10

Mark the Click() handler with async, then throw your loop into an await Task.Run() with an anonymous delegate. Update the UI using this.Invoke(), which I've done below with a helper method:

    private async void StartButton_Click_1(object sender, EventArgs e)
    {
        await Task.Run(() =>
        {
            Ping ping = new Ping();
            ContinuePing = true;
            do
            {
                try ///ping google
                {
                    PingReply reply = ping.Send("8.8.8.8");

                    if (reply.Status == IPStatus.Success)
                    {
                        SetLabelColor(GoogleStatusLabel.BackColor, Color.Green);
                    }

                }
                catch
                {
                    SetLabelColor(GoogleStatusLabel.BackColor, Color.Red);
                }

                try ///ping Yahoo!
                {
                    PingReply reply = ping.Send("www.yahoo.com");

                    if (reply.Status == IPStatus.Success)
                    {
                        SetLabelColor(YahooStatusLabel.BackColor, Color.Green);
                    }

                }
                catch
                {
                    SetLabelColor(YahooStatusLabel.BackColor, Color.Red);
                }

                try ///ping Reddit.com
                {
                    PingReply reply = ping.Send("www.reddit.com");

                    if (reply.Status == IPStatus.Success)
                    {
                        SetLabelColor(RedditStatusLabel.BackColor, Color.Green);
                    }

                }
                catch
                {
                    SetLabelColor(RedditStatusLabel.BackColor, Color.Red);
                }

                try ///ping Chive
                {
                    PingReply reply = ping.Send("www.chive.com");

                    if (reply.Status == IPStatus.Success)
                    {
                        SetLabelColor(ChiveStatusLabel.BackColor, Color.Green);
                    }

                }
                catch
                {
                    SetLabelColor(ChiveStatusLabel, Color.Red);
                }

                try ///ping CNN
                {
                    PingReply reply = ping.Send("www.cnn.com");

                    if (reply.Status == IPStatus.Success)
                    {
                        SetLabelColor(CNNStatusLabel, Color.Green);
                    }

                }
                catch
                {
                    SetLabelColor(CNNStatusLabel, Color.Red);
                }

            } while (ContinuePing);
        });
    }

    private void SetLabelColor(Label lbl, Color clr)
    {
        this.Invoke((MethodInvoker)delegate {
            lbl.BackColor = clr;
        });
    }

Why does my loop not allow my labels color to change?

4 Answers4