CODEWITHSUNDEEP
android-asynctaskandroid
Sibbs Gambling
Sibbs Gambling

Reputation: 20355

How to use AsyncTask properly to avoid a slow UI?

On my UI, I have an ImageView (an arrowhead image) that needs to be updated in real time.

There are 2 possible movements of the arrowhead:

I have already got these 2 methods to function properly.

The only problem is that my UI is very slow and sometimes gets stuck. Meanwhile, my phone always gets extremely hot while the app is being run on it. The Logcat also sometimes tells me

Skipped * frames. The application may be doing too much work on its main thread.

I am told to use AsyncTask so as not to stress my UI. So I do my arrowhead updating work in the AsyncTask. However, the problem remains. My UI is still very slow.

I guess there should be something wrong in my AsyncTask implementation. I paste them here as follows:

    public class ArrowheadUpdater extends AsyncTask<Float, Integer, Float> { // Float: azimuth, Integer: state

        private ImageView arrowheadToRotate;
        private float rotationAngle; // also in radians

        // constructor
        public ArrowheadUpdater(ImageView _arrowheadToRotate) {

            arrowheadToRotate = _arrowheadToRotate;
            rotationAngle = -1;
        }

        protected void onPreExecute(Float _azimuth) {
            super.onPreExecute();
        }

        @Override
        // executed first to get the angle to rotate
        protected Float doInBackground(Float... arg0) {

            rotationAngle = (float) (Constant.MAP_ORIENTATION_OFFSET + arg0[0]);

            return rotationAngle;
        }

        protected void onProgressUpdated(Integer... progress) {
            super.onProgressUpdate(progress);
        }

        protected void onPostExecute(Float result) {
            super.onPostExecute(result);

              \\ rotation happens here
            rotateImageView(ShowPathActivity.this, arrowheadToRotate, R.drawable.marker, result);

              \\ moving happens here
            movaImageView(arrowhead, MapView.historyXSeries, MapView.historyYSeries);
        }

I call the AsyncTask like this:

// called when sensor values change
    public void onSensorChanged(SensorEvent event) { // is roughly called 350 times in 1s
                //...
        if (event.sensor.getType() == Sensor.TYPE_MAGNETIC_FIELD) {

            compassChangedTimes++;

            magneticField[0] = event.values[0];
            magneticField[1] = event.values[1];
            magneticField[2] = event.values[2];

            SensorManager.getRotationMatrix(RotationM, I, gravity, magneticField);
            SensorManager.getOrientation(RotationM, direction);

            if (compassChangedTimes % 50 == 0) {
                         // HERE!!!!!!!!!!
                new ArrowheadUpdater(arrowhead).execute(direction[0]);
            }
        }

        if (startFlag)
            dataCollector.saveDataShowPath(acceleration, magneticField, startTime, currentTime);
    }

Should I put the 2 arrowhead-updating methods in doInBackground() instead of onPostExecute()?

But can the lines in doInBackground() update the UI? I am not sure.

Is there anything wrong in my AsyncTask?

Other guesses or comments are well welcomed!

MORE CLUES:

I just notice that when I just get into this activity, the UI is super slow and gets stuck a lot. But after a while, say 10 seconds, it somewhat becomes acceptably smooth.

Upvotes: 3

Views: 3598

Answers (4)

amalBit
amalBit

Reputation: 12181

There are good answers above.. But Async task has many loop-hole which I came to know after following the robo-spice app.

The app is open source. Have a look at it .

Upvotes: 0

srain
srain

Reputation: 9082

The UI is very slow and sometimes gets stuck because you maybe did something take a lot of time in your Main UI thread.

You can not update UI in doInBackground() method. It is suggest that do something take a lot of time in this method. For example, downloading data, or a picture.

After the data is prepared, onPostExecute() will be called, You can Update Your UI in these method.

++++++++++++++++++++++++++++++++++++++

Your AsyncTask is correct.

You can no update UI in doInBackground(). So, the 2 arrowhead-updating methods can not put into doInBackground() method.

In fact, I think there is no need for you to use AsyncTask.

The problem is onSensorChanged() call 350 times in 1s and you do too much work in it.

        SensorManager.getRotationMatrix(RotationM, I, gravity, magneticField);
        SensorManager.getOrientation(RotationM, direction);

and if startFlag is true:

        dataCollector.saveDataShowPath(acceleration, magneticField, startTime, currentTime);

You can change you onSensorChanged() method like this:

  public void onSensorChanged(SensorEvent event) {

        compassChangedTimes++;

        if (compassChangedTimes % 50 == 0) {
            if (event.sensor.getType() == Sensor.TYPE_MAGNETIC_FIELD) {


                magneticField[0] = event.values[0];
                magneticField[1] = event.values[1];
                magneticField[2] = event.values[2];

                SensorManager.getRotationMatrix(RotationM, I, gravity, magneticField);
                SensorManager.getOrientation(RotationM, direction);

                new ArrowheadUpdater(arrowhead).execute(direction[0]);
            }
            if (startFlag)
                dataCollector.saveDataShowPath(acceleration, magneticField, startTime, currentTime);
        }
    }

Upvotes: 0

njzk2
njzk2

Reputation: 39397

AsyncTasks are used to perform heavy/lengthy operations in background and have the result pushed on the UI.

In your case, your AsyncTask is not performing a heavy operation, so you probably can discard it.

Also, there is no limit in your current code as to the frequency of UI updates. You could implement such a limit with a Handler.

public static final int ARROW_MESSAGES = 0;

private Float angle;

Handler handler = new Handler() {
    @Override
    public void handleMessage(Message msg) {
        // Discard other messages
        removeMessages(ARROW_MESSAGES);
        // UI update
        rotateImageView(ShowPathActivity.this, arrowheadToRotate, R.drawable.marker, angle);
        movaImageView(arrowhead, MapView.historyXSeries, MapView.historyYSeries);

    }
}


// (...)
if (compassChangedTimes % 50 == 0) {
    float res = (float) (Constant.MAP_ORIENTATION_OFFSET + direction[0]);
    if (Math.abs(res - angle) > 1) {
        angle = res;
        handler.sendEmptyMessage(ARROW_MESSAGES);
    }
}

The principle is that you send messages to a handler, but the handler will only pick the first message and discard the rest (it is also possible to test the handler to know if there are already any messages before posting, I'm not sure which is more efficient).

This way, the UI will be updated with the latest angle value and will not try to display all intermediate stages.

Also, the diff test avoids unnecessary updates (I assume 1 degree is precise enough, you may even want to put a bigger value there).

Upvotes: 2

Glitch
Glitch

Reputation: 2785

Your problem is that you're getting 350 calls to that function a second, and you're not doing a good job ignoring 'rubbish' data.

Firstly, move as much logic as you can into your compassChangedTimes condition (also, you should reset that count to avoid overflow). Even better, decide whether to update the UI based on a sensor change threshold rather than an arbitrary number of samples (what happens if the sensor reports a constant value, and the onChangeEvent() function is called sporadically?)

Secondly, use a low pass filter to help strip out irrelevant updates. There should be an example of this in the Android API documentation for sensors.

Upvotes: 1

Related Questions