0
\$\begingroup\$

It's been long since I have put any of my codes for review. Today I want to review my Android code written for a BM70_BLE device with which one can change the color and brightness of the LED light. The code works, however, I would like to improve it further any suggestions for improvement or any logical changes are appreciated.

Following is my code for connecting to the BLE device and changing color and brightness in Android (Java):

package com.inevitablesol.www.ble_samplev2;

import android.app.ProgressDialog;
import android.bluetooth.BluetoothAdapter;
import android.bluetooth.BluetoothGattCharacteristic;
import android.bluetooth.BluetoothGattService;
import android.bluetooth.BluetoothSocket;
import android.content.ComponentName;
import android.content.Context;
import android.content.Intent;
import android.content.ServiceConnection;
import android.graphics.Color;
import android.os.Bundle;
import android.os.IBinder;
import android.support.v7.app.AppCompatActivity;
import android.support.v7.widget.AppCompatButton;
import android.util.Log;
import android.view.View;
import android.widget.ExpandableListView;
import android.widget.SeekBar;
import android.widget.TextView;
import android.widget.Toast;

import com.github.mikephil.charting.charts.PieChart;
import com.github.mikephil.charting.data.Entry;
import com.github.mikephil.charting.data.PieData;
import com.github.mikephil.charting.data.PieDataSet;
import com.github.mikephil.charting.formatter.PercentFormatter;
import com.github.mikephil.charting.highlight.Highlight;
import com.github.mikephil.charting.listener.OnChartValueSelectedListener;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;

public class Activity_BTLE_Services extends AppCompatActivity implements ExpandableListView.OnChildClickListener, View.OnClickListener, OnChartValueSelectedListener {
    private final static String TAG = Activity_BTLE_Services.class.getSimpleName();

    public static final String EXTRA_NAME = "com.inevitablesol.www.Activity_BTLE_Services.NAME";
    public static final String EXTRA_ADDRESS = "com.inevitablesol.www.Activity_BTLE_Services.ADDRESS";

    private ListAdapter_BTLE_Services expandableListAdapter;
    private ExpandableListView expandableListView;

    private ArrayList<BluetoothGattService> services_ArrayList;
    private HashMap<String, BluetoothGattCharacteristic> characteristics_HashMap;
    private HashMap<String, ArrayList<BluetoothGattCharacteristic>> characteristics_HashMapList;

    private Intent mBTLE_Service_Intent;
    private Service_BTLE_GATT mBTLE_Service;
    private boolean mBTLE_Service_Bound;
    private BroadcastReceiver_BTLE_GATT mGattUpdateReceiver;

    private String name;
    private String address;


    private Service_BTLE_GATT service;
    private BluetoothGattCharacteristic characteristic;
    private String title;
    AppCompatButton btnOn, btnOff, btnDis;
    SeekBar brightness;
    TextView lumn;
    private ProgressDialog progress;
    BluetoothAdapter myBluetooth = null;
    BluetoothSocket btSocket = null;
    private boolean isBtConnected = false;

    String color = "R99_G99_B99_I";
    int intensity = 50;

    public void setTitle(String title) {
        this.title = title;
    }

    public void setService(Service_BTLE_GATT service) {
        this.service = service;
    }

    public void setCharacteristic(BluetoothGattCharacteristic characteristic) {
        this.characteristic = characteristic;
    }

    private ServiceConnection mBTLE_ServiceConnection = new ServiceConnection() {

        @Override
        public void onServiceConnected(ComponentName className, IBinder service) {

            // We've bound to LocalService, cast the IBinder and get LocalService instance
            Service_BTLE_GATT.BTLeServiceBinder binder = (Service_BTLE_GATT.BTLeServiceBinder) service;
            mBTLE_Service = binder.getService();
            mBTLE_Service_Bound = true;

            if (!mBTLE_Service.initialize()) {
                Log.e(TAG, "Unable to initialize Bluetooth");
                finish();
            }

            mBTLE_Service.connect(address);


        }

        @Override
        public void onServiceDisconnected(ComponentName arg0) {
            mBTLE_Service = null;
            mBTLE_Service_Bound = false;

//            mBluetoothGatt = null;
//            mGattUpdateReceiver.setBluetoothGatt(null);
//            mGattUpdateReceiver.setBTLeService(null);
        }
    };

    public void colorPickerMethod() {
        int groupPosition = 4;
        int childPosition = 1;
        BluetoothGattCharacteristic characteristic = characteristics_HashMapList.get(
                services_ArrayList.get(groupPosition).getUuid().toString())
                .get(childPosition);

        if (Utils.hasWriteNoResponse(characteristic.getProperties()) != 0) {
            String uuid = characteristic.getUuid().toString();

            this.setTitle(uuid);
            this.setService(mBTLE_Service);
            this.setCharacteristic(characteristic);

        } else if (Utils.hasReadProperty(characteristic.getProperties()) != 0) {
            if (mBTLE_Service != null) {
                mBTLE_Service.readCharacteristic(characteristic);
            }
        } else if (Utils.hasNotifyProperty(characteristic.getProperties()) != 0) {
            if (mBTLE_Service != null) {
                mBTLE_Service.setCharacteristicNotification(characteristic, true);
            }
        }
    }

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_btle_services);

        openPieChart();
        final Intent intent = getIntent();
        name = intent.getStringExtra(Activity_BTLE_Services.EXTRA_NAME);
        address = intent.getStringExtra(Activity_BTLE_Services.EXTRA_ADDRESS);

        services_ArrayList = new ArrayList<>();
        characteristics_HashMap = new HashMap<>();
        characteristics_HashMapList = new HashMap<>();

        expandableListAdapter = new ListAdapter_BTLE_Services(
                this, services_ArrayList, characteristics_HashMapList);

        expandableListView = (ExpandableListView) findViewById(R.id.lv_expandable);
        expandableListView.setAdapter(expandableListAdapter);
        expandableListView.setOnChildClickListener(this);

        ((TextView) findViewById(R.id.tv_name)).setText(name + " Services");
        ((TextView) findViewById(R.id.tv_address)).setText(address);
        //LED Controller code starts
        lumn = (TextView) findViewById(R.id.lumn);
        brightness = (SeekBar) findViewById(R.id.seekBar);
        brightness.setProgress(50);
        brightness.setOnSeekBarChangeListener(new SeekBar.OnSeekBarChangeListener() {
            @Override
            public void onProgressChanged(SeekBar seekBar, int progress, boolean b) {
                if (b == true) {
                    lumn.setText(String.valueOf(progress));
                    intensity = progress;
                    setColor_Intensity(color);
                }
            }

            @Override
            public void onStartTrackingTouch(SeekBar seekBar) {

            }

            @Override
            public void onStopTrackingTouch(SeekBar seekBar) {
                if (intensity != 0) {
                    setColor_Intensity(color);
                }

            }
        });

        btnOn = (AppCompatButton) findViewById(R.id.bt_on);
        btnOn.setOnClickListener(this);

        btnOff = (AppCompatButton) findViewById(R.id.bt_off);
        btnOff.setOnClickListener(this);

        btnDis = (AppCompatButton) findViewById(R.id.bt_disconnect);
        btnDis.setOnClickListener(this);

    }

    @Override
    protected void onStart() {
        super.onStart();

        mGattUpdateReceiver = new BroadcastReceiver_BTLE_GATT(this);
        registerReceiver(mGattUpdateReceiver, Utils.makeGattUpdateIntentFilter());

        mBTLE_Service_Intent = new Intent(this, Service_BTLE_GATT.class);
        bindService(mBTLE_Service_Intent, mBTLE_ServiceConnection, Context.BIND_AUTO_CREATE);
        startService(mBTLE_Service_Intent);


    }

    @Override
    protected void onResume() {
        super.onResume();
    }

    @Override
    protected void onPause() {
        super.onPause();
    }

    @Override
    protected void onStop() {
        super.onStop();

        unregisterReceiver(mGattUpdateReceiver);
        unbindService(mBTLE_ServiceConnection);
        mBTLE_Service_Intent = null;
    }

    @Override
    public boolean onChildClick(ExpandableListView parent, View v, int groupPosition, int childPosition, long id) {

        BluetoothGattCharacteristic characteristic = characteristics_HashMapList.get(
                services_ArrayList.get(groupPosition).getUuid().toString())
                .get(childPosition);

        if (Utils.hasWriteNoResponse(characteristic.getProperties()) != 0) {
            String uuid = characteristic.getUuid().toString();

            Toast.makeText(mBTLE_Service, "Group Pos : " + groupPosition, Toast.LENGTH_SHORT).show();//4
            Toast.makeText(mBTLE_Service, "Child Pos : " + childPosition, Toast.LENGTH_SHORT).show();//1
//            Dialog_BTLE_Characteristic dialog_btle_characteristic = new Dialog_BTLE_Characteristic();
//
//            dialog_btle_characteristic.setTitle(uuid);
//            dialog_btle_characteristic.setService(mBTLE_Service);
//            dialog_btle_characteristic.setCharacteristic(characteristic);

//            dialog_btle_characteristic.show(getFragmentManager(), "Dialog_BTLE_Characteristic");


        } else if (Utils.hasReadProperty(characteristic.getProperties()) != 0) {
            if (mBTLE_Service != null) {
                mBTLE_Service.readCharacteristic(characteristic);
            }
        } else if (Utils.hasNotifyProperty(characteristic.getProperties()) != 0) {
            if (mBTLE_Service != null) {
                mBTLE_Service.setCharacteristicNotification(characteristic, true);
            }
        }

        return false;
    }

    public void updateServices() {

        if (mBTLE_Service != null) {

            services_ArrayList.clear();
            characteristics_HashMap.clear();
            characteristics_HashMapList.clear();

            List<BluetoothGattService> servicesList = mBTLE_Service.getSupportedGattServices();

            for (BluetoothGattService service : servicesList) {

                services_ArrayList.add(service);

                List<BluetoothGattCharacteristic> characteristicsList = service.getCharacteristics();
                ArrayList<BluetoothGattCharacteristic> newCharacteristicsList = new ArrayList<>();

                for (BluetoothGattCharacteristic characteristic : characteristicsList) {
                    characteristics_HashMap.put(characteristic.getUuid().toString(), characteristic);
                    newCharacteristicsList.add(characteristic);
                }

                characteristics_HashMapList.put(service.getUuid().toString(), newCharacteristicsList);
            }

            if (servicesList != null && servicesList.size() > 0) {
                expandableListAdapter.notifyDataSetChanged();
                colorPickerMethod();
            }
        }
    }

    public void updateCharacteristic() {
        expandableListAdapter.notifyDataSetChanged();
    }


    private void openPieChart() {
        PieChart pieChart = (PieChart) findViewById(R.id.piechart);
        ArrayList<Entry> yvalues = new ArrayList<Entry>();
        yvalues.add(new Entry(12.5f, 0));
        yvalues.add(new Entry(12.5f, 1));
        yvalues.add(new Entry(12.5f, 2));
        yvalues.add(new Entry(12.5f, 3));
        yvalues.add(new Entry(12.5f, 4));

        yvalues.add(new Entry(12.5f, 5));
        yvalues.add(new Entry(12.5f, 6));
        yvalues.add(new Entry(12.5f, 7));
        yvalues.add(new Entry(12.5f, 8));
        yvalues.add(new Entry(12.5f, 9));

        yvalues.add(new Entry(12.5f, 10));
        yvalues.add(new Entry(12.5f, 11));
        yvalues.add(new Entry(12.5f, 12));
        yvalues.add(new Entry(12.5f, 13));
        yvalues.add(new Entry(12.5f, 14));

        yvalues.add(new Entry(12.5f, 15));

        PieDataSet dataSet = new PieDataSet(yvalues, "Select color");
        ArrayList<String> xVals = new ArrayList<String>();

        xVals.add("Red");
        xVals.add("Pink");
        xVals.add("Purple");
        xVals.add("Deep Purple");
        xVals.add("Indigo");

        xVals.add("Light Blue");
        xVals.add("Cyan");
        xVals.add("Teal");
        xVals.add("Green");
        xVals.add("Light Green");

        xVals.add("Lime");
        xVals.add("Yellow");
        xVals.add("Amber");
        xVals.add("Blue");
        xVals.add("Orange");

        xVals.add("White");

        PieData data = new PieData(xVals, dataSet);
        // In percentage Term
        data.setValueFormatter(new PercentFormatter());
// Default value
//data.setValueFormatter(new DefaultValueFormatter(0));
        pieChart.setData(data);
        // Color of each Pie Chart Slices
        int red = Color.parseColor("#F44336");
        int pink = Color.parseColor("#E91E63");
        int purple = Color.parseColor("#9C27B0");
        int deep_purple = Color.parseColor("#673AB7");
        int indigo = Color.parseColor("#3F51B5");

        int blue = Color.parseColor("#2196F3");
        int light_blue = Color.parseColor("#03A9F4");
        int cyan = Color.parseColor("#00BCD4");
        int teal = Color.parseColor("#009688");
        int green = Color.parseColor("#4CAF50");

        int light_green = Color.parseColor("#8BC34A");
        int lime = Color.parseColor("#CDDC39");
        int yellow = Color.parseColor("#FFEB3B");
        int amber = Color.parseColor("#FFC107");
        int orange = Color.parseColor("#FF9800");

        int white = Color.parseColor("#FFFFFF");

        int[] colors = {red, pink, purple, deep_purple, indigo,
                blue, light_blue, cyan, teal, green,
                light_green, lime, yellow, amber, orange,
                white};

        dataSet.setColors(colors);
        pieChart.setDrawSliceText(false);
        pieChart.setUsePercentValues(false);
        pieChart.getData().setDrawValues(false);
        pieChart.setDrawMarkerViews(false);
        pieChart.setDescription("");
        pieChart.getLegend().setEnabled(false);
        pieChart.setOnChartValueSelectedListener(this);


    }

    @Override
    public void onClick(View view) {
        int viewId = view.getId();
        switch (viewId) {
            case R.id.bt_on:
                turnOnLed();
                break;
            case R.id.bt_off:
                turnOffLed();
                break;
            case R.id.bt_disconnect:
                Disconnect();
                break;

        }
    }

    private void Disconnect() {
        Toast.makeText(mBTLE_Service, "Service Disconnected", Toast.LENGTH_SHORT).show();
        mBTLE_Service.disconnect();
        mBTLE_Service.close();
        finish();
    }

    private void turnOffLed() {
        color = "R00_G00_B00_I000_E";
        if (service != null) {
            characteristic.setValue(color);
            service.writeCharacteristic(characteristic);
        }
    }

    private void turnOnLed() {
        color = "R96_G26_B21_I100_E";
        if (service != null) {
            characteristic.setValue(color);
            service.writeCharacteristic(characteristic);
        }
    }


    @Override
    public void onValueSelected(Entry entry, int i, Highlight highlight) {
        if (entry == null)
            return;
        Log.i("VAL SELECTED",
                "Value: " + entry.getVal() + ", xIndex: " + entry.getXIndex()
                        + ", DataSet index: " + i);

        switch (entry.getXIndex()) {
            case 0:
                color = "R96_G26_B21_I";
                if (intensity == 0) {

                    Toast.makeText(mBTLE_Service, "Please Increase Brightness", Toast.LENGTH_SHORT).show();
                } else {
                    brightness.setProgress(50);
                    intensity =50;
                    setColor_Intensity(color);
                }
                break;
            case 1:
                color = "R91_G12_B39_I";
                if (intensity == 0) {

                    Toast.makeText(mBTLE_Service, "Please Increase Brightness", Toast.LENGTH_SHORT).show();
                } else {
                    brightness.setProgress(50);
                    intensity =50;
                    setColor_Intensity(color);
                }

                break;
            case 2:
                color = "R61_G15_B69_I";
                if (intensity == 0) {
                    Toast.makeText(mBTLE_Service, "Please Increase Brightness", Toast.LENGTH_SHORT).show();
                } else {
                    brightness.setProgress(50);
                    intensity =50;
                    setColor_Intensity(color);
                }
                break;
            case 3:
                color = "R40_G23_B72_I";
                if (intensity == 0) {
                    Toast.makeText(mBTLE_Service, "Please Increase Brightness", Toast.LENGTH_SHORT).show();
                } else {
                    brightness.setProgress(50);
                    intensity =50;
                    setColor_Intensity(color);
                }

                break;
            case 4:
                color = "R25_G32_B71_I";
                if (intensity == 0) {

                    Toast.makeText(mBTLE_Service, "Please Increase Brightness", Toast.LENGTH_SHORT).show();
                } else {
                    brightness.setProgress(50);
                    intensity =50;
                    setColor_Intensity(color);
                }

                break;
            case 5:
                color = "R13_G59_B95_I";
                if (intensity == 0) {

                    Toast.makeText(mBTLE_Service, "Please Increase Brightness", Toast.LENGTH_SHORT).show();
                } else {
                    brightness.setProgress(50);
                    intensity =50;
                    setColor_Intensity(color);
                }

                break;
            case 6:
                color = "R01_G66_B96_I";
                if (intensity == 0) {
                    Toast.makeText(mBTLE_Service, "Please Increase Brightness", Toast.LENGTH_SHORT).show();
                } else {
                    brightness.setProgress(50);
                    intensity =50;
                    setColor_Intensity(color);
                }

                break;
            case 7:
                color = "R00_G74_B83_I";
                if (intensity == 0){
                    Toast.makeText(mBTLE_Service, "Please Increase Brightness", Toast.LENGTH_SHORT).show();
                }else{
                    brightness.setProgress(50);
                    intensity =50;
                    setColor_Intensity(color); //cyan
                }

                break;
            case 8:
                color = "R00_G59_B53_I";
                if (intensity == 0)
                {
                    Toast.makeText(mBTLE_Service, "Please Increase Brightness", Toast.LENGTH_SHORT).show();
                }else{
                    brightness.setProgress(50);
                    intensity =50;
                    setColor_Intensity(color);
                }

                break;
            case 9:
                color = "R30_G69_B31_I";
                if (intensity == 0)
                {
                    Toast.makeText(mBTLE_Service, "Please Increase Brightness", Toast.LENGTH_SHORT).show();
                }else {
                    brightness.setProgress(50);
                    intensity =50;
                    setColor_Intensity(color);//green
                }

                break;

            case 10:
                color = "R55_G76_B29_I";
                if (intensity ==0 )
                {
                    Toast.makeText(mBTLE_Service, "Please Increase Brightness", Toast.LENGTH_SHORT).show();
                }else{
                    brightness.setProgress(50);
                    intensity =50;
                    setColor_Intensity(color);
                }

                break;

            case 11:
                color = "R80_G86_B22_I";
                if (intensity == 0)
                {
                    Toast.makeText(mBTLE_Service, "Please Increase Brightness", Toast.LENGTH_SHORT).show();
                }else{
                    brightness.setProgress(50);
                    intensity =50;
                    setColor_Intensity(color);
                }

                break;

            case 12:
                color = "R99_G92_B23_I";
                if (intensity == 0)
                {
                    Toast.makeText(mBTLE_Service, "Please Increase Brightness", Toast.LENGTH_SHORT).show();
                }else{
                    brightness.setProgress(50);
                    intensity =50;
                    setColor_Intensity(color);
                }

                break;

            case 13:
                color = "R99_G76_B03_I";
                if (intensity == 0)
                {
                    Toast.makeText(mBTLE_Service, "Please Increase Brightness", Toast.LENGTH_SHORT).show();
                }else{
                    brightness.setProgress(50);
                    intensity =50;
                    setColor_Intensity(color);
                }

                break;

            case 14:
                color = "R99_G60_B00_I";
                if (intensity == 0)
                {
                    Toast.makeText(mBTLE_Service, "Please Increase Brightness", Toast.LENGTH_SHORT).show();
                }else{
                    brightness.setProgress(50);
                    intensity =50;
                    setColor_Intensity(color);
                }
                break;

            case 15:
                color = "R99_G99_B99_I";
                if (intensity == 0)
                {
                    Toast.makeText(mBTLE_Service, "Please Incresae Brightness", Toast.LENGTH_SHORT).show();
                }else{
                    brightness.setProgress(50);
                    intensity =50;
                    setColor_Intensity(color);
                    Toast.makeText(mBTLE_Service, "White was clicked", Toast.LENGTH_SHORT).show();
                }
                break;

        }
//        Toast.makeText(this, ""+entry.getXIndex(), Toast.LENGTH_SHORT).show();
    }

    private void setColor_Intensity(String color) {
        String brightness = String.format("%03d", intensity);
        String brt = brightness.concat("_E");
        String color_intensity = color.concat(brt);

        //Toast.makeText(mBTLE_Service, "CI : "+color_intensity, Toast.LENGTH_SHORT).show();
        Log.d("CI : ", color_intensity);
        if (service != null) {
            characteristic.setValue(color_intensity);
            service.writeCharacteristic(characteristic);
        }
    }

    @Override
    public void onNothingSelected() {
        Log.i("PieChart", "nothing selected");
    }
}
\$\endgroup\$
2
\$\begingroup\$

Here a few points, it's more based on basics. And what hurts the most.

  • The class is too long. 650 Lines. That's the sign, that a class does too much, it has too many responsibilities.
  • Activity_BTLE_Services: Java convetion is camel/pascal case: ActivityBTLEServices. Also: Use singular. If't is more than one service, it's more than one class. And what does BTLE even mean? In general, abbreviations should be avoided, except if it's, let's say a business term or a term which you "have to know" if you're working in the project.
  • private ArrayList<BluetoothGattService> services_ArrayList; Always use interfaces. Same for HashMap Also: servicesArraylist. Even better: "specificServiceName"Service. It's more important, that a variable name gives you an idea, what this variable is used for or what it's job is, instead of it's implementation detail.
  • String color = "R99_G99_B99_I"; What color? Of your panties? :P Always use a proper name. Also, it looks like a constant. So it should be declared as constant.
  • // We've bound to LocalService, cast the IBinder and get LocalService instance When did you bound what to the LocalService? The comment is more misleading than helpful. In general: If you have to write comments, so people understand it, it is a sign, that the code is not well written. I rarely use comments, only if it occurs to be one of those moment, where another reader is thinking somethink like "why whould you do it like this?" In that case, it has usually a reason, this reason can be written down.
  • Never leave commented code. Never. If you don't have a source repository, get one. If you need deleted code again, it's still there in your repo. Nobody who reads commented code knows if that's important or not, if this was commented by accident or what not.
  • colorPickerMethod: Don't name methods with 'method' as suffix.
  • brightness.setProgress(50); What's 50? I see the value 50 a lot, it seems important, so use a constant and give it a proper name.
  • public void onProgressChanged(SeekBar seekBar, int progress, boolean b) { I don't think the method declares the boolean as b. if(b == true): I've no idea what b is. Also: if(b) is sufficient.
  • String uuid = characteristic.getUuid().toString(); Looks like uuid is unused
  • Toast.makeText(mBTLE_Service, "Group Pos : " + groupPosition, Toast.LENGTH_SHORT).show();//4 What does 4 mean? Again: Comments.
  • yvalues.add(new Entry(12.5f, 0)); You do that 16 times. There's those things called loop, you know ;D
  • ArrayList<String> xVals = new ArrayList<String>(); This can be a constant, too. (And what xVals represents is a mystery to mee. What values?!?)
  • int red = Color.parseColor("#F44336"); those can be constants, too.
  • switch (entry.getXIndex()) { It looks like every part except the assignment of the color variable is duplicated within this switch lobck.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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