0

So I am making a network scanner in c# to show all connected devices to the same network as you. The way I am doing this is by doing a ping command on all IP's from 192.168.0.1 to 192.168.0.255.

private void IPlook_Tick(object sender, EventArgs e)
    {

        Properties.Settings.Default.nextIP += 1;

        if (Properties.Settings.Default.nextIP >= 255)
        {
            IPlook.Stop();
        }

        string IP = "192.168.0." + Properties.Settings.Default.nextIP.ToString();
        CurIP.Text = IP;

        //See if online
        try
        {
            Ping ping = new Ping();
            PingReply pingreply = ping.Send(IP);

            if (pingreply.Status == IPStatus.Success)
            {
                string Hostname = Dns.GetHostByAddress(IP).HostName;
                dataGridView1.Rows.Add(Hostname, IP, "");
            }
            else
            {
            }

        }
        catch (Exception er)
        {
            MessageBox.Show("Something Went Wrong", "Error Alert", MessageBoxButtons.OK, MessageBoxIcon.Error);
        }

    }

This is working fine but the problem is it is taking a very long time and making the program laggy. I have set the interval on the timer to 50.

Any help is appreciated.

Legitimate

Stephen Kennedy
  • 20,585
  • 22
  • 95
  • 108
Legitimat3
  • 41
  • 2
  • 4
  • Using Ping.SendAsync would be faster. As it stands, you're waiting for each device to respond before sending the next ping. – Joe Nov 16 '14 at 17:33
  • 1
    You might also want to ask at https://codereview.stackexchange.com/ – Haedrian Nov 16 '14 at 17:33
  • This is arguably a duplicate of http://stackoverflow.com/q/13492134/397817. At the very least the most upvoted answer there might help. – Stephen Kennedy Nov 16 '14 at 17:48

4 Answers4

2

How about firing them all off at once (from an async method):

IPAddress start = IPAddress.Parse("192.168.1.1");
var bytes = start.GetAddressBytes();
var leastSigByte= start.GetAddressBytes().Last();
var range= 255 - leastSigByte;

var pingReplyTasks = Enumerable.Range(leastSigByte,range)
    .Select(x=>{
        var p = new Ping();
        var bb = start.GetAddressBytes();
        bb[3] = (byte)x;
        var destIp = new IPAddress(bb);
        var pingResultTask = p.SendPingAsync(destIp);
        return new{pingResultTask, addr = destIp};
    })
    .ToList();

await Task.WhenAll(pingReplyTasks.Select(x=>x.pingResultTask));

foreach(var pr in pingReplyTasks)
{
    var tsk = pr.pingResultTask;
    var pingResult = tsk.Result; //we know these are completed tasks
    var ip = pr.addr;

    Console.WriteLine("{0} : {1}",ip,pingResult.Status);
}

You probably don't want to ping addresses ending in 0 or 255.

spender
  • 117,338
  • 33
  • 229
  • 351
0

You are running the pings one after the other. This means that you must wait for the timeout to expire 255 times. Start multiple pings at the same time. Using await would make a lot of sense here.

Also, unblock the UI thread so that the UI is not frozen while this is in progress. Many techniques are available for doing that. await happens to solve this problem as well.

I don't see why you are using a timer. If there is no reason for that simply delete all timer stuff. Use a loop.

usr
  • 168,620
  • 35
  • 240
  • 369
0

Well if I'm reading your code correctly you're allowing the timer to fire 255 times and are pinging exactly one computer in each execution. I'm not sure why you're using a Settings class to store the current iteration number.

You could loop over 1 to 255 and launch each Ping request asynchronously using the Ping.SendAsync method. There is no need for a timer.

See also this question and answers/comments about broadcast pinging.

Stephen Kennedy
  • 20,585
  • 22
  • 95
  • 108
0
using System;
using System.Collections.Generic;
using System.Net;
using System.Net.NetworkInformation;
using System.Threading.Tasks;

namespace ConsoleApplication2
{
    class Program
    {
        static void Main(string[] args)
        {
            // Can build your list of IPs using a for loop
            List<IPAddress> ips = new List<IPAddress>
            {
                new IPAddress(new byte[] {192, 168, 0, 1}),
                new IPAddress(new byte[] {192, 168, 0, 2}),
                // More ips in this list.
            };

            // Exactly what do you do with initiated tasks will depend on your specific scenario.
            List<Task> tps = new List<Task>();

            foreach(var ip in ips)
            {
                tps.Add(InitiatePing(ip));
            }

            // Needed so that console app doesn't exit..
            Console.ReadLine();
        }

        private static async Task InitiatePing(IPAddress ip)
        {
            // Note, this API is different from SendAsync API you are using
            // You may also want to reuse Ping instance instead of creating new one each time.
            var result = await new Ping().SendPingAsync(ip);
            // Process your result here, however you want.
            Console.WriteLine(result.Address + "-" + result.Status + "-" + result.RoundtripTime);
        }
    }
}
Vikas Gupta
  • 4,455
  • 1
  • 20
  • 40