0

So... I'm working with a third-party library (zkemkeeper.dll) to make a winform applicaction to manage attendance devices. I'm trying to get several devices connected and attach some events to listen for some actions they will fire. Everithing it's fine running all sync. I run some methods and for each device I try to connect like this:

public bool connect(Device myDevice)
{
     bool result=false;
     if (!myDevice.isConn)
     {
        myDevice.zkDevice = null;
        myDevice.zkDevice = new CZKEMClass();
        myDevice.zkDevice.MachineNumber = myDevice.id;
        if (myDevice.password.HasValue)
        {
            myDevice.zkDevice.SetCommPassword(myDevice.password.Value);
        }
        try
        {
            result = myDevice.zkDevice.Connect_Net(myDevice.ip, myDevice.port);
            myDevice.error = result ? "" : "Could not connect to device";
        }
        catch (Exception ex)
        {
            myDevice.error = ex.Message;
            result = false;
        }
        if(result)
        {
            //Bind events
            if (myDevice.zkDevice.RegEvent(myDevice.id, 1))
            {
                myDevice.zkDevice.OnAttTransactionEx += new _IZKEMEvents_OnAttTransactionExEventHandler(device_OnTransactionEx);
            }
        }
        myDevice.zkDevice.EnableDevice(myDevice.id, true);
        myDevice.zkDevice.EnableClock(1);
    }
        return result;
}

My main problem is that this take a couple of seconds to connect to each device depending on the state of the network, so if I have 50 or 100 device the interface will freeze every time a device is connected or reconnected and this will make a huge delay.(besides that the application must always be functional)

Ok to solve that I use this:

private async Task connectAllAsync()
{
    List<Task<bool>> lstTasks = new List<Task<bool>>();

    foreach (var device in lstDevices)
    {
        lstTasks.Add(Task.Run(() => connect(device)));
    }

    var arrayComplete =await Task.WhenAll(lstTasks);
    int countErr=arrayComplete.ToList().Where(n => n == false).Count();
    if(countErr>0)
    {
        timerReconnect.Enabled = true;
    }

}

After this the interface doesn't freeze,my connection to all devices is faster,even if I try to interrogate any device for information the device respond BUT when I try to trigger any event these doesnt fire, I dont know where is my mistake I think it could be related to atach the event on another thread or something like that... Help me give me some way to go, thanks in advance.

Edit: Im also tried making the "connect" function async an make it await for the Connect_Net response (which is part of the third party code)

Robert K
  • 130
  • 3
  • 16

1 Answers1

0

You have an uncaptured closure. Try this:

foreach (var device in lstDevices)
{
    var captured = device;
    lstTasks.Add(Task.Run(() => connect(captured)));
}

Or just use LINQ to do it all:

lstTasks = lstDevices.Select
( 
    device => Task.Run
    ( 
        () => connect(device) 
    ) 
);

A totally different approach would get rid of Task.Run() and use Parallel.ForEach instead. If you use this approach, you'd have to store the results in a thread-safe container like ConcurrentBag<>.

var results = new ConcurrentBag<bool>();
Parallel.ForEach
(
    lstDevices, 
    device =>
    {
        results.Add(connect(device));
    }
);
int countErr = results.Count( x => x == false );

Or you could just use a counter:

volatile int countErr = 0;
Parallel.ForEach
(
    lstDevices, 
    device => 
    {
        var ok = results.Add(connect(device));
        if (!ok) Interlocked.Increment(ref countErr);
    }
);
John Wu
  • 50,556
  • 8
  • 44
  • 80
  • Doesn't every loop in C# get its own stack with `device` being the respective device? I guess this would only be a problem in JS... – Legends Jul 19 '18 at 22:51
  • No, not unless you are using `Parallel.For` (which is another option). Anyway, `device` is not stored on the stack; it's stored on the heap, because it is closed. – John Wu Jul 19 '18 at 23:03