import of JNI file when using a service

[Nikolay ILYIN]

My native library has some transport protocols implementation which I plan to use as an Android service running in separate process (aidl interface).

what's the best recommended approach for organizing communication layers for such app:

should I import jni*.java file inside a newly created Service class? and when create a one more Helper class, which will be used in Activity classes to communicate with this Service (via Binder + Broadcasts)?

or jni file is already can be considered a Helper class itself and can be directly imported into other application classes to access native protocols? And the Service class in this case can be just used to control some tasks with native protocols and will communicate to jni Helper? Or new java Service class in overkill here? Another words, can native library can be considered as a separate process' service itself with jni*.java wrapper package acting is a java Helper class, so I don't need to build any new android level Services to interact with native code?

Please explain and recommend how better to structure the components of my app.

[Alex Cohn]

Please explain what your jni*.java file(s) do. 

If you have C++ code that implements outbound transport, and want to run it in a separate process, then the consumers (e.g. activities) will probably communicate with this service in Javaeeze, and you only use JNI in your service.

If, on the contrary, the service will deliver/receive raw packets which need C++ to parse and/or produce, then JNI belongs to the consumers, and the service does not need JNI part at all.

Or maybe, you don't need a separate process of a Service, and the activities can, through JNI, communicate with the external world directly.

In all these cases, you don't need a separate service that only wraps JNI.

BR,

Alex 

[Nikolay ILYIN]

Alex, hi: thanks for answering me. It looks like I've created too many unnecessary layers (for the sake of true background my native peerprotocols operation) and the better way will be to get rid of the service and its helper class at all. To clarify: my native code is doing some UPNP and Bluetooth logic such as as device discovery and sending text and (whole) files.

here is how my jni file looks, could you please check it out and confirm that no dedicated Service is actually needed:

package myapp.jnipeerprotocol;
import android.content.Context;
import android.os.Environment;
import android.util.Log;
import net.myapp.one.utils.Consts;

public class PeerProtocol {


/*

I had to do this to send callbacks to the specifically created peerprotocol java service, which in turn processes the data received and then LocalBroadcasts that to PeerProtocolHelper class imported to Activity/Fragments

*/

public interface IPeerProtocolCallbackListener {
void deviceScanCallback(String deviceName, String ipAddress, int scanType);

void bTFileTransferCallback(String strDeviceName, String strAddress, int transferType, String strFile, int param);

}


private static IPeerProtocolCallbackListener mListener;

public void setOnPeerProtocolCallbackListener(IPeerProtocolCallbackListener listener) {
mListener = listener;
}

private static final String TAG = "PeerProtocolClass";
private static PeerProtocol instance = null;
private Context mContext;
private static int transfer_result = 0;

public PeerProtocol(Context c) {
mContext = c;
}


public static PeerProtocol getInstance(Context context) {
if (instance == null) {
instance = new PeerProtocol(context.getApplicationContext());
}
return instance;
}


// below a list of C/C++ native functions

public static native int initializePeerProtocol();

public static native int terminatePeerProtocol();

public static native int scanNetwork();

public static native int transferFile(String deviceAddress, String pathFileName);

public static native int setDeviceDiscoveryCallbackFunc(String func);

public static native int setFileTransferCallbackFunc(String func);

public static native int setDownloadFolder(String path);

public static native int hasRouterServices();

public static native String getExternalIPAddress();

public static native int addPortMapping(String remoteHost, int externalPort, String protocol, int internalPort, String internalClient, String description);

public static native int deletePortMapping(String remoteHost, int externalPort, String protocol);


public static void scan() {
PeerProtocol.scanNetwork();
}

public static int init() {
int ret = PeerProtocol.initializePeerProtocol();

if (ret != -1) { // ret = 0 if ok
PeerProtocol.setDeviceDiscoveryCallbackFunc("PeerProtocolDeviceProc");
PeerProtocol.setFileTransferCallbackFunc("PeerProtocolTransferProc");
PeerProtocol.setDownloadFolder(Environment.getExternalStorageDirectory().getAbsolutePath() + Consts.DEFAULT_FOLDER + "/");
}
return ret;
}


public static int PeerProtocolDeviceProc(String strDeviceName, String strIpAddress, int scanType) {

if (strDeviceName == null && strIpAddress == null)
return -1;

final String devname = strDeviceName;
final String ipa = strIpAddress;
final int scantype = scanType;
mListener.deviceScanCallback(devname, ipa, scantype);
return 0;
}


private static boolean callbackReceived = false;

public static int PeerProtocolTransferProc(String strDeviceName, String strAddress, int transferType, String strFile, int param) {
mListener.bTFileTransferCallback(strDeviceName, strAddress, transferType, strFile, param);
waitForResult ();
return transfer_result;
}


public synchronized void setFileTransferCallbackProcResults (int result) {
transfer_result = result;
callbackReceived = true;
this.notify();
}

private static synchronized void waitForResult () {
callbackReceived = false;
while (!callbackReceived) {

try {
PeerProtocol.class.wait(10000);

} catch (InterruptedException ignore) {

}
}
}

static {
System.loadLibrary("jniPeerProtocol");
}
}

[Alex Cohn]

Hi Nikolay,

there are quite a few issues with the code you published.

The major concern is that it could perform network operations and even waits on UI thread. If you design a wrapper, make sure that it enforces correct thread affinity. Additionally, Android has special annotations that can help diagnose such problems at compile time.

Another design advice is to keep all native methods private. It's OK to provide thin public wrappers when necessary, but in this case I think you don't need them.

I don't think it's a good design decision to make the whole class aware that it is a singleton. The good practice (for unit testing and for future extensions) is to isolate the singleton pattern from the rest of implementation.

I believe that named callbacks from native to Java that you set in init() are an overkill, and they break compile-time robustness. Why not hardcode PeerProtocolDeviceProc and PeerProtocolTransferProc in C++? If you extend the PeerProtocol class in the future, you will have an opportunity to override these callback methods, and JNI will handle this for you.

Cheers,

Alex

PS The remarks above are not intended to be a complete code review. There may be other important improvements that I did not think about. On the other hand, my suggestions should be taken with a grain of doubt. Brief overview cannot substitute deep analysis, and some of my conclusions may be false.