6
\$\begingroup\$

I'm wonder if my approach is correct. I'm trying to make a WinForms server that processes data from 3 sides:

  • from bluettoh (InTheHand library) as source data which I need to process in server
  • to local network (TcpListener/TcpClient) to send processed result from Bluetooth data
  • from local network (TcpListener/TcpClient) to be able to set server processing method

I want to use portable devices, such as a tablet, as controllers to send sensor data to the server. The server read data continuously, processes data and sends a short message to a local network client (some game) to make a move/action in the game. The server is the only link between two clients: the controller (portable device) and game.

I'd like to use tasks and the await/async approach.

public partial class GUI : Form
    {
    // Bluetooth
    static readonly string BT = "BT";
    static readonly string LAN = "LAN";

    CancellationTokenSource cancelTokenSource;
    CancellationToken cancelToken;

    private BluetoothListener btListener = null;
    private BluetoothClient btConnection = null;
    private Stream btStream;

    private bool btStarted = false;
    private bool btConnected = false;

    Progress<string> progress;
    public GUI()
    {
        InitializeComponent();
        progress = new Progress<string>(msg =>
        {
            rtbDebug.ForeColor = Color.Blue;
            rtbDebug.AppendText(msg + System.Environment.NewLine);
            rtbDebug.ScrollToCaret();
        });

        cancelTokenSource = new CancellationTokenSource();
        cancelToken = cancelTokenSource.Token;
    }

    private async void btnStartBT_Click(object sender, EventArgs e)
    {
        if (btStarted == true)
        {
            rtbDebug.AppendText("BTServer already started..." + System.Environment.NewLine);
            return;
        }

        rtbDebug.AppendText("BTServer started..." + System.Environment.NewLine);
        btStarted = true;

        await StartBluetoothServer(progress, cancelToken);
    }

    public async Task StartBluetoothServer(IProgress<string> progress, CancellationToken tk) 
    {
        try
        {
            await MakeBluetoothConnection(progress, cancelToken);
            btConnected = true;
            progress.Report("BT connected");
            Task.Run(() => ProcessBluetooth(progress, cancelToken));
        }
        catch (Exception eee)
        {
            btStarted = false;
            btConnected = false;
            progress.Report("BT connection failed");
        }
    }

    async Task<bool> MakeBluetoothConnection(IProgress<string> progress, CancellationToken ct)
    {
        var tcs = new TaskCompletionSource<bool>();
        await Task.Run(() =>
        {
            try
            {
                Guid guid = new Guid("04c6093b-0000-1000-8000-00805f9b34fb");
                btListener = new BluetoothListener(guid);
                btListener.Start();
                btConnection = btListener.AcceptBluetoothClient();
                btStream = btConnection.GetStream();
                tcs.SetResult(true);
            }
            catch (Exception e)
            {
                tcs.SetException(e);
            }
        });

        Task<bool> task = tcs.Task;
        return task.Result;
    }

    async Task ProcessBluetooth(IProgress<string> progress, CancellationToken ct)
    {
        try
        {
            string result = await ProcessBluetoothData(progress, cancelToken);
            await SendDataToLan(progress, cancelToken, result);
        }
        catch (Exception e)
        {
            // ISSUE WITH PROCEESSING DATA FROM BLUETOOTH
        }
    }

    async Task<string> ProcessBluetoothData(IProgress<string> progress, CancellationToken ct)
    {
        // simulate processing
        await Task.Delay(2000, cancelToken);
        string temp = "Value";
        return temp;
    }

    async Task SendDataToLan(IProgress<string> progress, CancellationToken ct, string message)
    {
        try
        {
            await TrySend(progress, cancelToken, message);
        }
        catch (Exception e)
        {
            progress.Report(e.ToString());
        }
    }

    async Task<bool> TrySend(IProgress<string> progress, CancellationToken ct, string message)
    {
        var tcs = new TaskCompletionSource<bool>();

        Task.Run(() =>{
            // sending to local network client which I didn't initialize yet ;)
        }, cancelToken);

        return tcs.Task.Result;
    }

}

Should I use the producer/consumer approach with some kind of blocking connection or direct sending to the connected client? It will be possible to reconnect so I'd like avoid situations where a reconnected client will use buffered data after reconnecting.

Please look and tell me if my approach is ok or what I could improve.

\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

I'm trying to make a WinForms server

This is suspicious. Servers shouldn't be GUI application, they should be services, so that it can run when nobody is logged into the server machine. If you want a GUI application that manages the server, create it separately.


In general, your code is too tightly coupled to your GUI. If you decided later on to have no GUI or to use the same code from another application, you couldn't easily do that. Instead, you should separate your concerns: create one class that deals only with BT communication, one for LAN sending and one for LAN listening, or something like that. You can then use these classes from your GUI.


static readonly string BT = "BT";
static readonly string LAN = "LAN";

This looks like it should be an enum instead of a pair of strings. But it's hard to say, because they are not used anywhere in the code you posted.


rtbDebug.ForeColor = Color.Blue;
rtbDebug.AppendText(msg + System.Environment.NewLine);
rtbDebug.ScrollToCaret();

Why do you set ForeColor and call ScrollToCaret() only here and not in other cases where you log something? I think you should encapsulate this code into a method, so that you can call it from anywhere in this class.


if (btStarted == true)

You can remove the == true part:

if (btStarted)

catch (Exception eee)
{
    btStarted = false;
    btConnected = false;
    progress.Report("BT connection failed");
}

You should log somewhere what kind of exception this was. Otherwise there is no way to figure out what caused the connection to fail.


var tcs = new TaskCompletionSource<bool>();
await Task.Run(() =>
{
    try
    {
        …
        tcs.SetResult(true);
    }
    catch (Exception e)
    {
        tcs.SetException(e);
    }
});

Task<bool> task = tcs.Task;
return task.Result;

You don't need all this boilerplate code. If your Task returns the result, you can just return it back:

return await Task.Run(() =>
{
    …
    return true;
});

This will also propagate exceptions from your Task correctly (and won't hide them inside an AggregateException, like your current code does).


catch (Exception e)
{
    // ISSUE WITH PROCEESSING DATA FROM BLUETOOTH
}

Again, exceptions shouldn't be ignored, at the very least, they should be logged somewhere.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.