Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm writing a simple remote PC app (mouse-keyboard). Android is the client and is connected with WiFi to Java PC Server. I'm using TCP but I see a bit of latency compared to other remote apps. I'm sending a String to Server like these: "a","B","2".

Why am I getting latency sometimes? When I press any key, sometimes something doesn't happen for 1 sec, and then coming (pressing key) when I press the key. How can improve it for low latency?

Original question is here.

Client on Android (TCP):

     public class SendFile extends Activity {

Socket sockClient;
PrintWriter out;
 @Override
public void onCreate(Bundle savedInstanceState) {
    super.onCreate(savedInstanceState);
    setContentView(R.layout.test);

   }

  public void commandto(String sip, int port ,String byt) { 
try
{   
        sockClient = new Socket(sip,port);
        //sockClient.connect(new InetSocketAddress(sip,port),9000);     

        out = new PrintWriter(new BufferedWriter(new OutputStreamWriter(sockClient.getOutputStream())),true);
        out.println(byt);
        out.flush();
        Log.w("Client", "Client sent message");
        sockClient.close(); 
}catch (final UnknownHostException e)
{
    Toast.makeText(getBaseContext(),"Hata UnknownHostException"+e.getMessage(), Toast.LENGTH_LONG).show();

}catch (final IOException e)
{
    //
    Toast.makeText(getBaseContext(),"Hata IOException"+e.getMessage(), Toast.LENGTH_LONG).show();
}
}

@Override
 public boolean dispatchKeyEvent(KeyEvent KEvent) 
{

int keyaction = KEvent.getAction();

if(keyaction == KeyEvent.ACTION_DOWN)
{
    int keycode = KEvent.getKeyCode();
    int keyunicode = KEvent.getUnicodeChar(KEvent.getMetaState() );
    char character = (char) keyunicode;
    if(keycode != 82){
        if(keycode!=59){    
         commandto("192.168.2.195",9512,String.valueOf(character));
        }

    }

}
return super.dispatchKeyEvent(KEvent);
 }

 }

TCP Server Codes:

   import java.awt.AWTException;


     public class MainForma extends JFrame {

private ServerSocket ss= null;
private Socket s =null;
private BufferedReader br=null;

public Thread thread;
 Robot  robo;
 String command;
 ObjectInputStream ois;

public static void main(String[] args) {
    EventQueue.invokeLater(new Runnable() {
        public void run() {
            try {

                frame.setVisible(true);
                btnCheckUpdate.doClick();

            } catch (Exception e) {
                e.printStackTrace();
            }
        }
    });
}


     private void cAccept (int portt)  throws AWTException {

    try{
        if(ss==null){
        ss = new ServerSocket(portt);}

    }catch (IOException e){
      lblInf.setText("Could not listen on port :"+portt+" "+e.getMessage());
    }
    btnStop.setEnabled(true);btnStart.setEnabled(false);
    lblInf.setText("Server is started!");

    }

        while(true){
        try{

        s = ss.accept();
            br = new BufferedReader(new InputStreamReader(s.getInputStream()));

            command =br.readLine();
             System.out.println(command+"\n");
             type(command);

            s.close();
            br.close();


        }catch (IOException e){
            try {

                    if(s!=null)
                    s.close();
                    if(br!=null)
                    br.close();

            } catch (IOException es) {
                // TODO Auto-generated catch block
                es.printStackTrace();
            } 
             lblInf.setText("Server is stopped!");
             lblInf2.setText("");

        }
        }
    }


    public void type(String character){
    try {
            robo = new Robot();
        if(character!=""){
        switch (character) {
        case "a": robo.keyPress(KeyEvent.VK_A); break;
        case "b": robo.keyPress(KeyEvent.VK_B); break;
        case "c": robo.keyPress(KeyEvent.VK_C); break;
        case "d": robo.keyPress(KeyEvent.VK_D); break;
        case "A": robo.keyPress(KeyEvent.VK_SHIFT);       robo.keyPress(KeyEvent.VK_A); robo.keyRelease(KeyEvent.VK_SHIFT);break;
        case "B": robo.keyPress(KeyEvent.VK_SHIFT); robo.keyPress(KeyEvent.VK_B); robo.keyRelease(KeyEvent.VK_SHIFT);break;
        case "C": robo.keyPress(KeyEvent.VK_SHIFT); robo.keyPress(KeyEvent.VK_C); robo.keyRelease(KeyEvent.VK_SHIFT);break;
        case "D": robo.keyPress(KeyEvent.VK_SHIFT); robo.keyPress(KeyEvent.VK_D); robo.keyRelease(KeyEvent.VK_SHIFT);break;
        case "E": robo.keyPress(KeyEvent.VK_SHIFT); robo.keyPress(KeyEvent.VK_E); robo.keyRelease(KeyEvent.VK_SHIFT);break;
        case "F": robo.keyPress(KeyEvent.VK_SHIFT); 
        case "0": robo.keyPress(KeyEvent.VK_0); break;
        case "1": robo.keyPress(KeyEvent.VK_1); break;
        case "2": robo.keyPress(KeyEvent.VK_2); break;
        case "3": robo.keyPress(KeyEvent.VK_3); break;

        default:
            System.out.println("fucj"+"/ "+character);
        }
        }
    } catch (AWTException e) {
        // TODO Auto-generated catch block
        e.printStackTrace();
        System.out.println(e.getMessage());
    }

    }


    btnStart = new JButton("Start");
    btnStart.addActionListener(new ActionListener()
    {
        public void actionPerformed(ActionEvent e)
        {
            btnStart.setEnabled(false);
            btnStop.setEnabled(true);

              thread = new Thread() {

                  public void run(){
                      try {
                        cAccept(9512);
                    } catch (AWTException e) {
                        // TODO Auto-generated catch block
                        e.printStackTrace();
                    }    

                  }

                };
             thread.start();


    }    });

I'm waiting for any idea for improving latency.

share|improve this question
    
Please do not remove the code that was originally posted. The below answer corresponds to that code, so it should remain. –  Jamal Mar 13 at 23:39
add comment

3 Answers 3

up vote 5 down vote accepted

The performance problem is still related to creating a socket each time you want to send data.

Here is a revised version of your code... note how the socket is kept open all the time (unless there is a failure, at which point it will re-connect....):

Socket sockClient = null;
String remoteHost = null;
int remotePort = 0;
PrintWriter remoteOut;

@Override
public void onCreate(Bundle savedInstanceState) {
    super.onCreate(savedInstanceState);
    setContentView(R.layout.test);

}

private synchronized final PrintWriter checkSocket(String sip, int port) throws IOException {
    if (sockClient != null && (sockClient.isOutputShutdown() || sockClient.isClosed())) {
        sockClient.close();
        sockClient = null;
    }
    if (sockClient != null && !(remoteHost.equals(sip) && remotePort == port)) {
        sockClient.close();
        sockClient = null;
    }
    if (sockClient == null) {
        remoteHost = sip;
        remotePort = port;
        sockClient = new Socket(remoteHost, remotePort);
        remoteOut = new PrintWriter(new BufferedWriter(new OutputStreamWriter(sockClient.getOutputStream())));
    }
    return remoteOut;
}

public void commandto(String sip, int port, String byt) {
    boolean ok = false;
    try {

        PrintWriter out = checkSocket(sip, port);

        out.println(byt);
        out.flush();
        Log.w("Client", "Client sent message");
        ok = true;
    } catch (final UnknownHostException e) {

        Toast.makeText(getBaseContext(),
                "Hata UnknownHostException" + e.getMessage(),
                Toast.LENGTH_LONG).show();

    } catch (final IOException e) {
        //
        Toast.makeText(getBaseContext(),
                "Hata IOException" + e.getMessage(), Toast.LENGTH_LONG)
                .show();
    } finally {
        if (!ok) {
            // there was a problem on the network.....
            // force a re-connect on the next message.
            synchronized(this) {
                sockClient = null;
            }
        }
    }
}

Using this system, the socket STAYS OPEN ALL THE TIME... do not close it!

This is the way that TCP is designed to work.

NOTE, you need to make the corresponding changes on the server side as well....

The server cannot keep closing the socket after every transmission... it needs to read a line, process it, and wait for the next line without closing the socket.

Read up on the Java Socket Tutorial....

share|improve this answer
    
I've get NullPointerException at "checkSocket". SS : i.imgur.com/lAeAwUg.png –  sallamaniaa Mar 13 at 19:55
1  
@sallamaniaa repaired I think. Edited/fixed code. –  rolfl Mar 13 at 19:57
    
I'm tried this. But It's working for a time. So It's just once press and does'nt happing until when I restart activity. And my log is working in "commandto", "Client sent message". –  sallamaniaa Mar 13 at 20:02
    
I've fixed the issue. I added this "remotePort = 0;" in dispatchKeyEvent. :) little wrong. Meanwhile it's working good. I'm learning what you do. Thank you again. If you want anything of me say it. You have a good ideo. –  sallamaniaa Mar 13 at 20:37
    
When I've test it where a bit far of my Wi-Fi modem then I see some latency sometime. I'm testing latency with this. ( like this start-end = System.currentTimeMillis(); ) I see all time normal latency (3-5) but sometimes I see 1000-2000 ms, after It's coming normal-not normal... It's just happining only when I far of my Wi-fi modem. I'm testing other applications they are not same problem. –  sallamaniaa Mar 23 at 11:49
add comment

Another thing you can try, to reduce latency, is to use setTcpNoDelay to disable the Nagle algorithm.

share|improve this answer
add comment

Two quick notes:

  1. You should format the code. Eclipse (and I guess other IDEs) support autoformat, they do a really good job here, use them. They give the code a more professional look and costs you only a keypress. Code is read more than written, so it's worth to optimize for reading. (It would also help reviewers on this site.)

  2. Instead of a big switch-case you could use a Map<Character, List<Integer>> keyMapping (untested code):

    Map<Character, List<Integer>> keyMapping = new HashMap<>();
    keyMapping.put('a', newArrayList(KeyEvent.VK_A));
    ...
    keyMapping.put('A', newArrayList(KeyEvent.VK_SHIFT, KeyEvent.VK_A, KeyEvent.VK_SHIFT));
    

    and use this map:

    List<Integer> keys = keyMapping.get(character);
    for (int keyCode: keys) {
        robot.keyPress(keyCode);
    }
    

    You might also be able to use the fact that value of VK_A is 65 which is the ASCII code of A.

    (A Multimap would be better here. newArrayList is from Google Guava. It has a quite simple implementation which you can copy-paste to your project if you don't want to include a new jar only for that.)

share|improve this answer
1  
1. I tried Source Format (ctrl+shift+f) in Eclipse. Thanks. 2. But I see error "newArrayList(KeyEvent.VK_A)". Should I create a method name is "newArrayList". How can i use it? There is error. –  sallamaniaa Mar 14 at 12:07
    
@sallamaniaa: I've updated the answer, check it please. –  palacsint Mar 14 at 12:36
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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