5
\$\begingroup\$

I am building an app for android to control VLC (mediaplayer) that runs on my computer. At the moment it's just a prototype that works, but I was wondering if I am correctly managing my HandlerThread and Socket.

My fear is that the thread keeps running too long or that I cause memory leaks. I have looked at a lot of examples and questions on Stack Overflow, but I want to be sure before I complete the app in this way.

The Java program that runs on the desktop runs only on the main thread, so I will just post my Android code.

VlcRemoteActivity.java:

package be.zweetinc.PcController;

import android.app.Activity;
import android.os.Bundle;
import android.os.HandlerThread;
import android.os.Message;
import android.view.View;
import android.widget.Button;
import android.widget.EditText;
import android.widget.ImageButton;

public class VlcRemoteActivity extends Activity {

    private ImageButton volumeUpButton;
    private ImageButton volumeDownButton;
    private EditText messageText;
    private Message message;
    private Button sendButton;
    private Button killServerButton;

    private HandlerThread connectionHandlerThread;
    private ConnectionHandler connectionHandler;

    public void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.main);
        volumeDownButton = (ImageButton) findViewById(R.id.downButton);
        volumeUpButton = (ImageButton) findViewById(R.id.upButton);
        messageText = (EditText) findViewById(R.id.message);
        sendButton = (Button) findViewById(R.id.sendButton);
        killServerButton= (Button) findViewById(R.id.killServerButton);

        connectionHandlerThread = new HandlerThread("ConnectionThread");
        connectionHandlerThread.start();

        connectionHandler = new ConnectionHandler(connectionHandlerThread.getLooper());

        message = Message.obtain(connectionHandler);
        message.what = MessageCode.CLASS_CONNECTION; // EventClass CONNECTION
        message.arg1 = MessageCode.CONNECTION_CONNECT; // EventAction CONNECT

        connectionHandler.sendMessage(message);

        volumeUpButton.setOnClickListener(new View.OnClickListener() {
            @Override
            public void onClick(View view) {
                message = Message.obtain(connectionHandler, MessageCode.CLASS_VLC, MessageCode.VLC_VOLUME_UP, 0);
                connectionHandler.sendMessage(message);
            }
        });

        volumeDownButton.setOnClickListener(new View.OnClickListener() {
            @Override
            public void onClick(View view) {
                message = Message.obtain(connectionHandler, MessageCode.CLASS_VLC, MessageCode.VLC_VOLUME_DOWN, 0);
                connectionHandler.sendMessage(message);
            }
        });

        sendButton.setOnClickListener(new View.OnClickListener() {
            @Override
            public void onClick(View view) {
                String textToSend = messageText.getText().toString();

                Bundle data = new Bundle();
                data.putString(ConnectionHandler.MESSAGE_DATA, messageText.getText().toString());
//                TODO: change arg1 and arg2 parameter when feature is not limited to logging only
                message = Message.obtain(connectionHandler, MessageCode.CLASS_VLC, MessageCode.VLC_VOLUME_UP, 0, data);
                connectionHandler.sendMessage(message);
            }
        });

        killServerButton.setOnClickListener(new View.OnClickListener() {
            @Override
            public void onClick(View view) {
                message = Message.obtain(connectionHandler, MessageCode.CLASS_SERVICE, 0, 0);
                connectionHandler.sendMessage(message);
            }
        });
    }

    @Override
    protected void onPause() {
        super.onPause();
        message = Message.obtain(connectionHandler, MessageCode.CLASS_CONNECTION, MessageCode.CONNECTION_DISCONNECT, 0);
        connectionHandler.sendMessage(message);
//        connectionHandlerThread.quit();
    }

}

ConnectionHandler.java:

package be.zweetinc.PcController;

import android.os.Handler;
import android.os.Looper;
import android.os.Message;
import android.util.Log;
import org.json.JSONException;
import org.json.JSONObject;

import java.io.IOException;
import java.io.PrintWriter;
import java.net.Socket;

public class ConnectionHandler extends Handler {
    public static final String MESSAGE_DATA = "be.zweetinc.PcController.Message_Data";

    private Socket client;
    private PrintWriter printwriter;
    private Message message;

    public ConnectionHandler(Looper looper){
        super(looper);
    }

    @Override
    public void handleMessage(Message msg) {
        super.handleMessage(msg);    //To change body of overridden methods use File | Settings | File Templates.
        message = msg;
        if(message.what != MessageCode.CLASS_CONNECTION){
            sendMessageToServer();
        } else {
            handleConnection();
        }
    }
    private void handleConnection(){
        if(message.arg1 == MessageCode.CONNECTION_CONNECT) {
            makeConnection();
        } else {
            closeConnection();
        }
    }

    private void sendMessageToServer(){
        JSONObject jsonObject = new JSONObject();
        try {
            jsonObject.put("eventClass", message.what);
            jsonObject.put("eventAction", message.arg1);
            jsonObject.put("eventActionType", message.arg2);
            jsonObject.put("eventText", message.getData().getString(MESSAGE_DATA));
        } catch (JSONException e) {
            e.printStackTrace();  //To change body of catch statement use File | Settings | File Templates.
        }



        printwriter.println(jsonObject.toString());

        Log.d("ConnectionInfo", "printwriter error: " + printwriter.checkError());

//        printwriter.write(jsonObject.toString()+"\n");
    }

    protected void quit(){
        getLooper().quit();
    }

    private void makeConnection(){
        try {
            client = new Socket("192.168.1.195", 4444);
            Log.d("ConnectionInfo", client.toString());
            printwriter = new PrintWriter(client.getOutputStream(), true);
        } catch (IOException e) {
            e.printStackTrace();  //To change body of catch statement use File | Settings | File Templates.
        }
    }

    private void closeConnection(){
        printwriter.close();
        try {
            client.close();
        } catch (IOException e) {
            e.printStackTrace();  //To change body of catch statement use File | Settings | File Templates.
        }
        quit();
    }
}
\$\endgroup\$
6
\$\begingroup\$

From what I can see, it seems like you are doing it correctly.

On some exceptions, you might want to alert your user with a message about what went wrong (for example, "Connection failed"). Few users can read stack traces while using an app.

I only have a few comments regarding your code style:

You're inconsistent in putting spaces around {-characters, and also with if-statements. There should always be a space after if and before a {
As in, private void makeConnection(){ <-- insert space here.

\$\endgroup\$
  • \$\begingroup\$ Thanks for the answer, I will indeed be adding user alerts, the code I posted was a simple proof of concept to test what the best way for the use of a socket would be. As for my coding style, fair points, I will keep them in mind for my future coding/refactoring. \$\endgroup\$ – Koen Gabriels Nov 22 '13 at 10:22

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