##Constructors and Generics

It is customary to use constructors to set values for instances in Java. This code:

private class SendAndWaitForACK implements Callable<Result> {
    private CameraCommands cmd;

    @Override
    public Result call() throws Exception {
        // does not return on timeout
        result.dataBytebuffer = postCommand(cmd);
        result.isSuccess = true;
        return result;
    }

    /**
     * setCommand must be called prior to use.
     * 
     * @param cmd
     *            : CameraCommands
     */
    public void setCommand(CameraCommands cmd) {
        this.cmd = cmd;
    }
}

Would more typically be written as:

private static class SendAndWaitForACK implements Callable<Result> {
    private final CameraCommands cmd;

    SendAAndWaitForACK(CameraCommands cmd) {
        this.cmd = cmd;
    }

    @Override
    public Result call() throws Exception {
        // does not return on timeout
        result.dataBytebuffer = postCommand(cmd);
        result.isSuccess = true;
        return result;
    }

}

Note that I have replaced the setter with the constructor. I have also made the class static, which saves a tiny amount of memory, but it is good practice because it makes Java GC easier, and has some other benefits.

You will need to change the way you use that class, and your calling code will change from:

Future<?> future = executor.submit(sendAndWaitForACK);

to

Future future = executor.submit(new SendAndWaitForACK(cmd));

Note, the SendAndWaitForACK implements Callable<Result>, so it will produce a Future<Result> when submitted. As a result, I have changed the generic type of the Future to Future<Result>. This also means that you no longer need the explicit cast and it can simply be:

result = future.get(waitMS, TimeUnit.MILLISECONDS);

Network

Note that your class does not do a clean handling of the network. When you submit a Callable you start a thread, and you run the Callable on that thread. Then, in your code, you set a timeout to wait for the Callable to complete. If the callable does not complete within that specified time, you get a timeout. This does not stop the Callable from running in the other thread. All that happens is that the callable keeps running, and eventually it gets something back from the camera... and, what it gets back, may be something belonging to another command sent later.

Fundamentally, your protocol/communication is a little flawed in this regard. As long as things work as expected, you won't notice it, but, if things go screwy, then your code will make the screwy situation even more screwy.

I strongly recommend, in cases like this, to not use threading at all. At least, don't use multiple threads to communicate over a single socket.

One of the benefits of UDP is the easy way to build simple protocols on top of it.

If this were me, I would:

• have a single class that manages the socket, and protocol.
• have a thread on that class that checks periodically to see that the camera is there, and working (perhaps every second or so, a 'heartbeat').
• that class has a ConcurrentBlockingQueue on the input side that takes a SendAndWaitForACK
• (I would probably make the 'heartbeat' one of these SendAndWaitForACK instances)
• When a callable is queued, the class will give that command a unique number, and it will 'transmit' that command every 10 or so milliseconds, until it gets a response, or until the timeout happens
• when it gets the response (or timeout), it will notify the caller.

I would make the camera-side of the system return the unique ID for the command as part of the response. That way you can make sure you are getting the right data back.

Doing things this way means that:

• only one thread accesses the network socket.
• only one command can be 'in transit' at any one time.
• you do not need to worry about threading or concurrency on the socket.
• you always have a 'state' of the camera.

Obviously, that recommendation is very different to what you have at the moment.... but, bottom line, is that almost all UDP-based communication channels will have some form of simple protocol established above the raw network layer.