Can't send data back with the UI from an AsyncTask

Question

I have this AsyncTask which I use to send a chat message over the internet. The problem is that when I execute the task nothing happens - at least not on the UI. I suspect that onProgressUpdate() does not execute at all. The idea is that when the task is started, a message will be sent over the internet and an EditText on the UI will be updated with the new text. Here is the whole class:

import java.io.IOException;
import java.net.DatagramPacket;
import java.net.InetAddress;
import java.net.MulticastSocket;

import android.os.AsyncTask;
import android.widget.EditText;

public class Messager extends AsyncTask<SocketAndEditText, Void, Void> {

    private MulticastSocket socket;
    private EditText host;
    private EditText port;
    private EditText sendMessage;
    private EditText messageBoard;
    private InetAddress serverAddress;
    private int pt;
    private String newConverstion;
    private String message;

    @Override
    protected Void doInBackground(SocketAndEditText... soEd) {
        // get the text that they contain and add the new messages to the old ones
        //host = soEd[0].getHost();
        //port = soEd[0].getPort();
        messageBoard = soEd[0].getMessageBoard();
        sendMessage = soEd[0].getSendMessage();

        message = sendMessage.getText().toString();
        String conversation = messageBoard.getText().toString();

        newConverstion = conversation.concat("\n[You] ").concat(message);

        return null;
    }

    protected void onProgressUpdate(Integer... progress) {
        // make the messages text view editable
        messageBoard.setFocusable(true);
        messageBoard.setText(newConverstion);   // add the new message to the text view
        messageBoard.setFocusable(false);   // make the messages text view not editable

        // erase the text on the second text view that has just been sent
        sendMessage.setText("");

        sendMessage(message);
    }

    public void sendMessage(String message) {
        // convert the host name to InetAddress
        try {
            serverAddress = InetAddress.getByName("localhost");
        } catch (Exception e) {}
            pt = 4456;

        // create socket and start communicating
        try {
            socket = new MulticastSocket(pt);
            socket.joinGroup(serverAddress);
        } catch (IOException e) {}

        // Send message to server

        // convert message to bytes array
        byte[] data = (message).getBytes();

        // create and send a datagram
        DatagramPacket packet = new DatagramPacket(data, data.length, serverAddress, pt);

        try {
            socket.send(packet);
        } catch (IOException e) {}
    }

}

What could be wrong?

Answer 1

onProgressUpdate should be invoked explicitly from within doInBackground as seen here. It is not the correct method to use in your case. I would rather expect that the setting of the text field should be done in onPostExecute. Reason being that the value of newConverstion is determined just after the remote call and it might take a while to complete. If you do it before the asynctask has finished execution you risk NPE.

Edit Adding some code:

    public class Messager extends AsyncTask<SocketAndEditText, Void, Void> {

   //skipping some field declaration

    @Override
    protected Void doInBackground(SocketAndEditText... soEd) {
        // get the text that they contain and add the new messages to the old ones
        //host = soEd[0].getHost();
        //port = soEd[0].getPort();
        messageBoard = soEd[0].getMessageBoard();
        sendMessage = soEd[0].getSendMessage();

        message = sendMessage.getText().toString();
        sendMessage(message); //NOTE: added the remote call in the background method. This is the only thing that really SHOULD be done in background. 

        String conversation = messageBoard.getText().toString();

        newConverstion = conversation.concat("\n[You] ").concat(message);

        return null;
    }

    protected void onPostExecute(Void result) {
        // make the messages text view editable
        messageBoard.setFocusable(true);
        messageBoard.setText(newConverstion);   // add the new message to the text view
        messageBoard.setFocusable(false);   // make the messages text view not editable

        // erase the text on the second text view that has just been sent
        sendMessage.setText("");
     }

Basically the most important thing is to place the most time consuming calls in the background of the task. In your case this is sendMessage. From then on you can do whatever fixes you wish in the postExecute and the preExecute. I am not quite sure what your intention for the onProgressUpdate was. Holever I just translated it to using onPostExecute. If you need to temporarily disable the field you can disable it in onPreExecute and enable it onPostExecute.

Answer 2

The onProgressUpdate() won't be called if you don't call publishProgress() yourself. See the 4 steps of AsyncTask.

As Boris pointed out. You should call sendMessage() in doInBackground() and update UI in onPostExecute().