HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Server that supports two types of connections

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
supportstwothattypesserverconnections

Problem

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 progress;
public GUI()
{
InitializeComponent();
progress = new Progress(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)

Solution

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();
await Task.Run(() =>
{
    try
    {
        …
        tcs.SetResult(true);
    }
    catch (Exception e)
    {
        tcs.SetException(e);
    }
});

Task 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.

Code Snippets

static readonly string BT = "BT";
static readonly string LAN = "LAN";
rtbDebug.ForeColor = Color.Blue;
rtbDebug.AppendText(msg + System.Environment.NewLine);
rtbDebug.ScrollToCaret();
if (btStarted == true)
if (btStarted)
catch (Exception eee)
{
    btStarted = false;
    btConnected = false;
    progress.Report("BT connection failed");
}

Context

StackExchange Code Review Q#68084, answer score: 3

Revisions (0)

No revisions yet.