Android Bluetooth ConnectThread herschrijven

[redesign]

Hallo allemaal,

Ik ben bezig om een goed werkende Bluetooth app te bouwen in Android, die meerdere Bluetooth apparaten tegelijkertijd bediend. De Android voorbeeld code (BluetoothChatService.class) laat nogal te wensen over qua implementatie, vandaar dat ik e.e.a. zelf aan het ontwerpen/herschrijven ben.

Hieronder de code van een connectThread die er voor zorgt dat je met een ander BT apparaat kunt verbinden. Zodra de verbinding is gelukt - en men een BluetoothSocket heeft verkregen die de verbinding tussen de twee BluetoothDevices regelt - wordt deze thread gestopt en wordt er verder gegaan in een andere thread, namelijk de connectedThread. Deze handelt al het onderlinge verkeer tussen de BT apparaten af.

Ik wil de code van de connectThread herschijven (want spagetti code). De MY_UUID_SECURE en MY_UUID_INSECURE zijn gewoon gegenereerde UUIDs van mijn app, verder niet belangrijk voor het voorbeeld.

Java:

private class ConnectThread extends Thread {
    private static final String TAG = "ConnectThread"; 
    private final BluetoothAdapter mAdapter;
    private final BluetoothSocket mSocket;
    private final BluetoothDevice mDevice;
    private final String mSocketType;

    public ConnectThread(BluetoothDevice device, boolean secure) {
        BluetoothSocket tmp = null;
        mAdapter = BluetoothAdapter.getDefaultAdapter();
        mDevice = device;
        mSocketType = secure ? "Secure" : "Insecure";

        // Get a BluetoothSocket for a connection with the given BluetoothDevice
        try {
            if (secure) {
                tmp = device.createRfcommSocketToServiceRecord(
                        MY_UUID_SECURE);
            } else {
                tmp = device.createInsecureRfcommSocketToServiceRecord(
                        MY_UUID_INSECURE);
            }
        } catch (IOException e) {
            Log.e(TAG, "Socket Type: " + mSocketType + "create() failed", e);
        }
        mSocket = tmp;
    }

    @Override
    public void run() {
        Log.i(TAG, "BEGIN mConnectThread SocketType:" + mSocketType);
        setName("ConnectThread" + mSocketType);

        // Always cancel discovery because it will slow down a connection
        mAdapter.cancelDiscovery();

        // Make a connection to the BluetoothSocket
        try {
            // This is a blocking call and will only return on a
            // successful connection or an exception
            mSocket.connect();
        } catch (IOException e) {
            // Close the socket
            try {
                mSocket.close();
            } catch (IOException e2) {
                Log.e(TAG, "unable to close() " + mSocketType +
                        " socket during connection failure", e2);
            }
            return;
        }

        // Start the connected thread
        connected(mSocket, mDevice, mSocketType);
    }

    public void cancel() {
        try {
            mSocket.close();
        } catch (IOException e) {
            Log.e(TAG, "close() of connect " + mSocketType + " socket failed", e);
        }
    }
}

De connected(mSocket, mDevice, mSocketType); aanroep in de connectThread leidt naar de volgende code binnen de BluetoothChatService.class:

Java:

public synchronized void connected(BluetoothSocket socket, BluetoothDevice
            device, final String socketType) {
        if (D) Log.d(TAG, "connected, Socket Type:" + socketType);

        // Cancel the thread that completed the connection
        if (mConnectThread != null) {mConnectThread.cancel(); mConnectThread = null;}

        // Cancel any thread currently running a connection
        if (mConnectedThread != null) {mConnectedThread.cancel(); mConnectedThread = null;}

        // Cancel the accept thread because we only want to connect to one device
        if (mSecureAcceptThread != null) {
            mSecureAcceptThread.cancel();
            mSecureAcceptThread = null;
        }
        if (mInsecureAcceptThread != null) {
            mInsecureAcceptThread.cancel();
            mInsecureAcceptThread = null;
        }

        // Start the thread to manage the connection and perform transmissions
        mConnectedThread = new ConnectedThread(socket, socketType);
        mConnectedThread.start();

        // Send the name of the connected device back to the UI Activity
        Message msg = mHandler.obtainMessage(BluetoothChat.MESSAGE_DEVICE_NAME);
        Bundle bundle = new Bundle();
        bundle.putString(BluetoothChat.DEVICE_NAME, device.getName());
        msg.setData(bundle);
        mHandler.sendMessage(msg);

        setState(STATE_CONNECTED);
    }

Nu zie ik in de BluetoothChatService.class dat vanuit de connectThread een aanroep wordt gedaan naar connected() methode in de mainThread (deze wordt dus op de mainThread uitgevoerd!). Deze connected() methode start dan de connectedThread met de eerder verkregen BluetoothSocket, en zo verder.
Ergens vind ik dit behoorlijk kris-kras door elkaar heen lopende code.

Wat ik wil is dat de mainThread altijd de thread is die de volledige regie/controle in handen heeft, ipv de ondersteunende threads (de connectThread moet NIET de connected() method aanroepen in de mainThread bijvoorbeeld).

Ik zat er aan te denken om een verbinding op te bouwen via een Callable task die in een thread wordt uitgevoerd die door een ExecutorService/ThreadPool wordt beheerd/aangestuurd en die vervolgens een Future<BluetoothSocket> object terug geeft zodra de verbinding is opgebouwd. Hierdoor is de connectThread alleen nog maar verantwoordelijk voor het verkrijgen van een BluetoothSocket object en meer niet. Zo zou het ook moeten is mijn idee, want het opbouwen van een verbinding met een ander BluetoothDevice is namelijk een blocking call. Dus deze thread blijft "hangen" totdat een verbinding is opgebouwd, of een exception heeft plaatsgevonden. De huidige connectThread slikt deze exception in en doet er verder niks mee, want de thread stopt zonder resultaat. De mainThread weet dus niet of er een exception heeft plaatsgevonden.

Mijn voorstel voor een andere aanpak mbv een Callable task is als volgt:

Java:

private class ConnectTask implements Callable<BluetoothSocket> {
    private static final String TAG = "ConnectTask";
    private final BluetoothAdapter mAdapter;
    private final BluetoothSocket mSocket;
    private final BluetoothDevice mDevice;
    private final String mSocketType;

    public ConnectTask(BluetoothDevice device, boolean secure) {
        BluetoothSocket tmp = null;
        mAdapter = BluetoothAdapter.getDefaultAdapter();
        mDevice = device;
        mSocketType = secure ? "Secure" : "Insecure";

        // Get a BluetoothSocket for a connection with the given BluetoothDevice
        try {
             if (secure) {
                tmp = device.createRfcommSocketToServiceRecord(MY_UUID_SECURE);
            } else {
                tmp = device
                        .createInsecureRfcommSocketToServiceRecord(MY_UUID_INSECURE);
            }
         } catch (IOException e) {
            Log.e(TAG, "Socket Type: " + mSocketType + "create() failed", e);
         }
        mSocket = tmp;
    }

    @Override
    public BluetoothSocket call() throws Exception {
        // Always cancel discovery because it will slow down a connection
        mAdapter.cancelDiscovery();

        // Make a connection to the BluetoothSocket
        try {
            // This is a blocking call and will only return on a
            // successful connection or an exception
            mSocket.connect();
         } catch (IOException e) {
            // Close the socket
            try {
                 mSocket.close();
            } catch (IOException e2) {
                Log.e(TAG, "unable to close() " + mSocketType
                         + " socket during connection failure", e2);
            }
            throw new Exception(
                    "Unable to establish a connection with Bluetooth device: "
                             + mDevice.getName() + " at address: "
                            + mDevice.getAddress());
         }
        return mSocket;
    }

    public void cancel() {
        try {
            mSocket.close();
        } catch (IOException e) {
            Log.e(TAG, "close() of connect " + mSocketType + " socket failed", e);
        }
    }
}

Zoals je kan zien is de code enigsinds anders geschreven:
- bij een exception wordt eerst de rommel intern opgeruimd en daarna netjes (dmv een 'gewone' exception) gemeldt aan de aanroepende partij.
- bij een succesvolle verbinding wordt de verkregen BluetoothSocket geretourneerd als resultaat van de call() method.

Voordeel hiervan is dat het opzetten van een verbinding alleen binnen deze task gebeurt, en nergens anders. Er wordt ook geen code aangeroepen die niks met deze taak te maken heeft, zoals in het eerdere voorbeeld.

Vanuit de mainThread kan men dan het volgende doen:
- een nieuwe connectTask aanmaken voor een andere verbinding en deze laten uitvoeren binnen een thread die door de threadpool beheert wordt
- een connectTask annuleren d.m.v. de method aan te roepen cancel() methode.
- controleren of de connectTask inmiddels een BluetoothSocket heeft verkregen door de Future.isDone() methode aan te roepen.
- een time-out te specificeren waar binnen de Future.get() methode dient te reageren om de BluetoothSocket te overhandigen. Dit is een blocking call totdat de time-out periode is overschreden.

Het voordeel zit hem nu in het feit dat de mainThread gewoon door kan blijven gaan met andere zaken en af en toe moet kijken of de verbinding is tot stand gebracht. Daarna kan de mainThread een andere thread de opdracht geven de verbinding te beheren.

Mijn vraag aan jullie: wat vinden jullie van deze oplossing? Beter/Slechter? Heb je je bedenkingen over deze oplossingsrichting? Gaarne jullie feedback, eventueel met aanvullende/verbeterende code voorbeelden.

Alvast bedankt!

[Janoz]

Hele lap tekst, maar ik wil iig alvast op een paar dingetjes inspringen.

Nu zie ik in de BluetoothChatService.class dat vanuit de connectThread een aanroep wordt gedaan naar connected() methode in de mainThread (deze wordt dus op de mainThread uitgevoerd!).

Dit is niet correct. De connected() methode wordt inderdaad op het object BluetoothChatService uitgevoerd, maar zeker niet op de main thread. Dat een aanroep op hetzelfde object gebeurt betekent niet dat het dan ook in dezelfde thread gebeurt.

Ik vermoed namelijk dat de denkfout die je maakt is dat je denkt dat executie pad thread gelijk is aan object Thread. Dit is niet het geval. Een Thread object is niet anders dan andere objecten, Het enige verschil is dat een aanroep naar start() niet blokt en vervolgens in een andere thread de code in de run() methode uitvoert.

Tot slot. Er zijn twee manieren om met threads te werken in java. Threads extenden of runnable implementeren. De standaard doet de eerste, maar de tweede manier is juist de te prefereren manier. Zie ook http://stackoverflow.com/...unnable-vs-extends-thread

[ Voor 17% gewijzigd door Janoz op 13-11-2013 16:30 . Reden: Na wat nazoeken blijkt het eerste bestaande code te zijn ]

[redesign]

@Janoz: Ik ben inderdaad aan het stoeien met mijn hersenspinsels over threads ( die blijken niet altijd te kloppen).

Zoals de code nu is, is de connected() methode een methode van de BluetoothChatService.class, Mijn gedachtensprong is dan als volgt: BluetoothChatService extends Service. Een Service in Android wordt gestart met startService(new serviceIntent), en vervolgens wordt deze service op de ui/main-thread uitgevoerd.
De ConnectThread wordt binnen de service uitgevoerd en zodra er een BluetoothSocket is verkregen, wordt de connected() methode aangeroepen die in de BluetoothChatService gedefinieerd is.

Dus de connected() methode wordt NIET uitgevoerd op de mainThread, maar in de ConnectThread. Wat ik juist wil bereiken is dat de ConnectThread geen andere code in een andere thread aanroept, maar netjes aan de aanroepende thread (in dit geval de mainThread) meedeeld: ik ben klaar met mijn blocking call, ik heb een BluetoothSocket voor je, kom je die even ophalen?

Mijn oplossing zou zijn de code zoals weergegeven in de ConnectTask, die in een willekeurige thread wordt uitgevoerd (niet zijnde de mainThread) en direct een toekomstig resultaat teruggeeft aan de aanroepende thread, de mainThread. Dan kan de mainThread zelf bepalen wanneer deze het resultaat van de ConnectTask uitleest d.m.v. een Futute.isDone() aanroep (om de zoveel seconde pollen van deze methode).
Wat ik wil voorkomen is dat de ConnectThread zelf bepaalt welke code uitgevoerd wordt in de mainThread (ik bedoel te zeggen: niet OP de mainThread, maar VAN het mainThread object). Ik heb zelf het idee dat de ConnectTask dit zou kunnen oplossen: het verschuiven van verantwoordelijkheid.

Ik heb wel het idee dat ik het concept Runnable vs Thread onder de knie heb, alleen het werken met Callable/Future is nieuw voor me. Het voordeel van de ConnectTask is dat ik meerdere ConnectTasks kan uitvoeren (zowel achter elkaar alsook naast elkaar). Dat kan met de ConnectThread ook, maar dan krijg je ook zoveel aantal ConnectThreads tegelijktijd. Dat is niet het geval bij een Executor/ThreadPool het geval.

Graag ook je visie over de ConnectTask aub... ik ben wel benieuwd of mijn redenering juist is of in de buurt komt van dezelfde werking/functionaliteit als de ConnectThread.