Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tag.java wrapper questions and a possible bug #189

Closed
GitHubDragonFly opened this issue Aug 27, 2020 · 32 comments
Closed

Tag.java wrapper questions and a possible bug #189

GitHubDragonFly opened this issue Aug 27, 2020 · 32 comments
Labels

Comments

@GitHubDragonFly
Copy link
Contributor

This would be related to my using Android Studio in attempt to make libplctag working on Android.

Can somebody explain the imports stated in Tag.java:

import dev.java.net.jna.Library;
import dev.java.net.jna.Native;
import dev.java.net.jna.NativeLibrary;

Is that from here: https://github.com/java-native-access , and if so, should these be com.sun.jna instead of dev.java.net.jna?

JNA seems to also require libclang_rt.asan-i686-android.so library when accessing libplctag.so, but when that library is manually included in the project then there is an error stating "The SecurityManager implementation on this platform is broken; you must explicitly provide the class to register". Where would one add a code to register this ASan library?

As for the possible bug(s), Android didn't like these:

private static native int plc_tag_get_lib_version(void); <-- void in particular

public static boolean checkLibraryVersion(int req_major, int req_minor, int req_patch) {
if(Tag.plc_tag_check_lib_version(req_major, req_minor, req_patch) == Tag.PLCTAG_STATUS_OK) {
return true;
} else {
return false;
}
}

which doesn't match:

private static native int plc_tag_check_lib_version(int encoded_version);

@kyle-github
Copy link
Member

Ooh. Yeah, the Java wrapper is really out of date isn't it :-(

I will need to fix it. It looks like it is about half-changed for some of the more recent API additions.

@kyle-github
Copy link
Member

On most of your questions, I am not sure I can help much. I have not built anything under Android.

I will need to start a completely new Java project in order to make sure that this all works. I will work on that later this week and through the weekend. I will probably try to reimplement tag_rw as that uses most of the API.

@GitHubDragonFly
Copy link
Contributor Author

I am not much of either Android or a java programmer, but gave it a try with the latest Android Studio and so far it did compile your library.

In order to bypass all the compilation errors, I had to remove all the entries related to "examples" and "tests" within the CMakeLists.txt file.
Another thing I did was to remove "L" from LPTHREAD entries within 3 files (platform.h, platform.c and logging.cpp) upon some suggestions on the Internet and a compiler error pointing to -lpthread, so I am currently not sure if this will have any adverse effect.

Added to the project jna.aar module, modified your Tag.java wrapper to match what I suggested in my initial post (for imports and methods), created a small Android app to just try and read 3 values but it crashes due to the error I mentioned about ASan library, while trying to extract the libplctag library.

Hopefully somebody might shine a bit more light on java so I can try it.

@kyle-github
Copy link
Member

I am going to move the Java wrapper over to the other libplctag-org repository for Java: libplctag.java. I have created a new Gradle project and am slowly (re)learning how to set up Java projects. So far so good. I will implement a few examples too. That will hopefully allow me to get this all worked out cleanly. Ideally I will be able to push the resulting JAR to the Maven repo.

@GitHubDragonFly
Copy link
Contributor Author

According to Android Developers, there is a "simple" addition that can be included to the C library to expose all its methods to Java Native Interface. They have provided an example in C++ and I am not sure of how it translates to C. You can read about it here and if you can figure out how to include it in your library then it might make things simpler:

https://developer.android.com/training/articles/perf-jni

The example should require inclusion of the jni.h header file.

So far in my dealing with libplctag in Android Studio, I have hit the point at which the java Native.Register function breaks while locating suitable method for plc_tag_abort, which by the way, intentionally or not, doesn't have LIB_EXPORT associated with it in the lib.c (as well as plc_tag_status method).

@kyle-github
Copy link
Member

Thanks for the pointer to the JNI information. I used JNA rather than JNI way back (almost ten years ago) because we were doing Java at the time for our HMIs and JNA did not require any changes to the C DLL.

Thanks for catching the lack of LIB_EXPORT on plc_tag_abort()! I wonder if that is a new regression or if it has been that way for a while :-( I will patch this up in the prerelease branch today.

Would you be interested in collaborating on the new repo in libplctag-org/libplctag.java? You seem to be much more up to date with Java than I am (I started using Java when it was called Oak, but mostly stopped using it around Java 1.4).

@GitHubDragonFly
Copy link
Contributor Author

Exporting methods to JNI, as Android Developers suggested, appears to be a universal solution since that additional code shouldn't affect the library in any way and would be used by JNI only.

Here is the C version of that C++ example code they have on their website (as translated by me, possibly incorrect and I don't really know if it's functional):

JNIEXPORT jint JNI_OnLoad(JavaVM* vm, void* reserved) {
JavaVM jvm = *vm;
JNIEnv env;

if (jvm->GetEnv(vm, (void **)&env, JNI_VERSION_1_6) != JNI_OK) {
    return JNI_ERR;
} else{
    // Find your class. JNI_OnLoad is called from the correct class loader context for this to work.
    jclass clazz = env->FindClass(&env, "com/abmodbusmaster/Tag");
    if (!clazz)
        return JNI_ERR;

    // Register your class' native methods.
    static const JNINativeMethod methods[] = {
            {"Java_plc_tag_abort", "(Ljava/lang/I)I", (int*)plc_tag_abort},
            {"Java_plc_tag_create", "(Ljava/lang/String;I)I", (int*)plc_tag_create},
    };

    int rc = env->RegisterNatives(&env, clazz, methods, sizeof methods / sizeof methods[0]);
    if (rc != JNI_OK)
        return rc;

    return JNI_VERSION_1_6;
}

}

I will pass on your invitation to collaborate in any other way than what I am currently doing.

As for your library on Android, I did manage to get the library respond with error code -25 to my attempt of creating a tag.
So there is some sort of communication but it might take a seasoned Android programmer to figure this out (which I am not).

@kyle-github
Copy link
Member

Your bug report was a well needed kick in the pants. I finally took the time to split out the Java wrapper into a different repo. I have a preliminary version here. This still uses JNA. The JAR includes all the required dependencies including the C DLLs for various platforms. The callback functions are all commented out.

There is still a lot of work to do but at least there are some rudimentary tests that run on build now. Over time I want to convert this all over to JNI and the automatic method registration that you pointed out. That sounds like the right way to go in the long run.

It sounds like you are making huge progress on Android! The error -25 is a NULL pointer. If you turn on debugging (either in the tag string or via the API), and capture it, I can probably tell you how far it got.

@GitHubDragonFly
Copy link
Contributor Author

Your library works on Android.

I just tested it with Android Emulator on Windows 10, Android 4.4.2 old phone and Android 9.0 newer phone.
It worked for communicating with both AB MicroLogix 1100 and Modbus.

The app is simple, with 1 button for AB and 1 button for Modbus, with hardcoded strings, requesting values of 3 consecutive registers (N7:0 and hr0) and with 3 TextView controls to display those values.

So, the way I look at this, your library is versatile and might change the industry.

As for Android, you should probably create a separate branch and remove "examples" and "tests" folders and all related entries in the CMakeLists.txt file. This way only the library will be compiled and bunch of compilation errors bypassed.

I could upload the Android Studio project to the dropbox, but beware that it is using some deprecated methods and all together is not refined.

@kyle-github
Copy link
Member

Wow! That's great work!

I think your idea of a separate branch might work, or perhaps just a separate repo since JNI/JNA and other things would need to be included and would be more specific to Android. Or it might be easier to split the work into two parts: one set of modifications to the CMake file to directly support building on Android for the C part, and another for the Java part.

Please let me know if you can put it in Dropbox or into a PR. Either way I would love to see this!

@GitHubDragonFly
Copy link
Contributor Author

GitHubDragonFly commented Aug 31, 2020

It is designed as a quick and dirty testing app without any refinements.
People can deal with your Android repository/branch once you create it.

The way it works is that once you press a button it will create an AsyncTask, try to create a tag and fetch those 3 values.
It will sort of time before cancelling the task, holding the UI thread for short time since the cancellation kind of needs to be done on this thread.

@kyle-github
Copy link
Member

I was able to download it. Now I need to figure out how to use Android Studio! Thanks for this work!

@GitHubDragonFly
Copy link
Contributor Author

GitHubDragonFly commented Sep 1, 2020

Here is how it works in Windows:

  • once the project is opened it is sufficient to just click on the menu Build/Rebuild Project to get the resulting apk file which is used to install on Android devices
  • the path on Windows looks like this: C:\Users\User\AndroidStudioProjects\ABModbusMaster\app\build\outputs\apk\debug
  • before rebuilding the project, since you will need to hardcode your IP addresses, you should edit AsyncTaskAB.java and AsyncTaskModbus.java files, change the string, save them (File/Save All), sync them with gradle (File/Sync Project With Gradle Files) and then Build/Rebuild Project.

If you want I could upload the resulting apk file to the dropbox (which supports APIs 16 to 30).

@GitHubDragonFly
Copy link
Contributor Author

Here is a picture of Android Studio on my computer

Android Studio

@kyle-github
Copy link
Member

Thanks for the explanations. I finally got Android Studio set up on my laptop (Linux) and working. I was able to build the apk!

I was going through the CMake config file and I think I can merge the main one and the one you made by selectively guarding the examples and test parts by checking environmental flags. The CMake variable ANDROID is defined when building with Android Studio's environment.

Did you make any changes to the C code? I have tried to keep the core library fully compatible with C99, so most compilers back to at least 2010 should work. MS Visual Studio C/C++ is a different story but I have a work around for that that seemed to work for Windows XP.

The Java wrapper that I am working on is now able to use either the system installed core libplctag or one that is included in the JAR itself if there is not one installed in the system.

So the order of business here is going to be figuring out:

  1. How to converge the CMake configs to get one configuration.
  2. How to converge any changes to the C files to get one unified core library.
  3. How to build the libplctag4j JAR separately from the Android app.
  4. How to build the core library for Android (all the main versions and CPU types) and get all those artifacts.

Then I can include all the pre-build Android versions into the single JAR for libplctag4j and hopefully just include it as an external dependency in the app. That gives me something to ship.

We'll see how far I get. Thanks again for all this work. I certainly had no idea where to even start!

@GitHubDragonFly
Copy link
Contributor Author

From what I recall, the only changes to the C library should be those 2 LIB_EXPORT entries, one of which might not even be necessary.

The way I look at this, if you create an Android branch with all the things required to use on Android only then you will save yourself time in trying to converge CMake or changes in the C files. This might just create bugs that could take even more time to fix. And, opposite of what I just stated, you should consider making all those changes in a long run.

The whole point of this shebang was to make sure that your library works on Android. It does take an experienced Android developer to create an app, since my test project does not use callbacks and it definitely shouldn't be checking the state of the AsyncTask in a loop since that never lets the AsyncTask complete properly (even though it does fetch the values requested).

@GitHubDragonFly
Copy link
Contributor Author

For CMake and specific Android ABIs you should check this: https://developer.android.com/ndk/guides/abis#cmake

Another thing you might pay attention to is the app's build.gradle file (located in the "app" folder and listed under "Gradle Scripts" in the Android Studio as "build.gradle (Module:app)").

It has added procedure of creating ("$rootDir/app/src/main/jniLibs/" folder with subfolders "arm64-v8a" / "armeabi-v7a" / "x86" / "x86_64" and fetching and copying ASan libraries from ndk.
This procedure will also create "$rootDir/app/src/main/res/lib/" folder with the same subfolder structure as above, and place scripts inside.

The significance of this ("$rootDir/app/src/main/jniLibs/" folder appears to be such that Android Studio will pack all its subfolders and libraries into the apk as well as compile/add the corresponding version of libplctag.so file.

I am not sure if this will work for "mips" / "mips64" / "armeabi" as well, but will eventually try it.

@GitHubDragonFly
Copy link
Contributor Author

If you might be interested, I have refined the app to the point that it can do simultaneous parallel connection to MicroLogix and Modbus with automatic reading. It is still only for hardcoded strings for 3 consecutive values (N7:0 and hr0).

If you can still access the dropbox link I posted previously, it is still there (if not I will post it again).

@GitHubDragonFly
Copy link
Contributor Author

Another change to the library was to remove "L" from LPTHREAD entries within 3 files (platform.h, platform.c and logging.cpp).

I forgot about this but did mention it in my previous posts.

@kyle-github
Copy link
Member

Sorry for the radio silence. Was buried with work lately.

I was able to download the project again. I think what I will do is just drop in a directory alongside my existing project and then get a diff of the two trees. That should show all the core library source differences.

I'll see if I can catch up tonight and make some progress on this.

@GitHubDragonFly
Copy link
Contributor Author

GitHubDragonFly commented Sep 3, 2020

Once you do all that, then for whatever reason download the project again since I updated it again.

It now includes all the changed files which I copied from your prerelease (and modified what needed to be modified).

It also has a routine in both AB and Modbus tasks, which should update UI only when either value has changed.
This allows tasks to communicate as fast as possible on separate threads but there is a commented out code in each task that can put a thread to sleep in order to slow down the communication if necessary.

@GitHubDragonFly
Copy link
Contributor Author

The app works on Android tablet as well, which is no wonder.

Since the tablet display offers more real estate, it allows for more stuff to be added and eventually offer to manually enter the string connection parameters (like IP address, path, name, ... etc).

@GitHubDragonFly
Copy link
Contributor Author

Removing "L" from LPTHREAD entries within 3 files (windows platform.h, windows platform.c and cpp wrapper logging.cpp - which was commented out) appears to be irrelevant. So, these entries should remain as LPTHREAD.

It is only that compiler throws an error pointing to "-lpthread" when compiling examples or tests.

All together, the only change that makes the difference when compiling the library was when those entries were removed from CMakeLists.txt file.

@kyle-github
Copy link
Member

I am finally back to this. I wanted to finish up the separation of Java from the main project and into its own project in the libplctag GitHub organization. That is now basically complete.

I think even the main library can use just "-pthread" rather than "-lpthread". The latter matches other libraries, "-lfoo" for linking with libfoo.so. Apparently pthread was treated specially because it is more than just linking with an external library.

I think what I will do is simply take the whole project as is and drop it into an experimental repo in the libplctag org. That will make sure that I do not lose any changes I make.

@kyle-github
Copy link
Member

I will continue the Android part in in issue #26.

For Java itself, I believe that the current new project is vastly better than what there was before!

@GitHubDragonFly
Copy link
Contributor Author

As for your libplctag4j, you have a line in the build.gradle file: compile 'net.java.dev.jna:jna:5.6.0'
which creates com/sun/jna/subfolders folder structure.

A post on the Internet suggests slightly different for Android: compile 'net.java.dev.jna:jna:5.6.0@aar'
which creates jni/subfolders folder structure.

https://github.com/java-native-access/jna/blob/master/www/FrequentlyAskedQuestions.md#jna-on-android

After creating jar files with both options, Android is still looking at com/sun/jna/subfolders for correct files (just ignoring this jni folder).

So, your scripts would have to be adjusted to properly instruct Android of where to look for files.

Another thing, related to your Tag.java, is that Android doesn't seem to like this line:

NativeLibrary.getInstance(Tag.JNA_LIBRARY_NAME)

It seems to be causing this error: "The SecurityManager implementation on this platform is broken; you must explicitly provide the class to register". I thought it was related to ASan libraries but it looks like I was wrong.

@GitHubDragonFly
Copy link
Contributor Author

JNA website shows that Android archives are separate:

https://github.com/java-native-access/jna/tree/master/dist

and they are not included by default into com/sun/jna.

@kyle-github
Copy link
Member

Thanks for the report. I have fixed it in a PR on the libplctag4j repo. I had found this and fixed it earlier, but I never merged it into the release branch :-(

I opened issue #3 on the libplctag4j repo and closed it with the fix.

I am still trying to figure out how to share Tag.java between the Android repo and the Java repo. Right now the Android one has a copy. I have opened bug #1 in the Android repo to track fixing that.

@GitHubDragonFly
Copy link
Contributor Author

Your library in the release branch is currently missing version.h file. Not sure if you moved the code elsewhere but thought you should know.

Apart from that and since I learn as I go, the code in the Android repo can use some updating, where the interfaces can just be reduced to being interfaces since the rest of the code is never used. For example, the ABTaskCallbackInterface.java can have this code only:

package com.abmodbusmaster;

public interface ABTaskCallbackInterface
{
void UpdateABUI(String val1, String val2, String val3);
}

and then all the references to ABTaskCallbackInterface.ABTaskCallback in the MainActivity and AsyncTaskAB, can be reduced to ABTaskCallbackInterface.

I did make a somewhat simple procedure of adding Android compiled libraries, modified Tag.java and jna.aar files to any new Android project. All these libraries can be extracted from the APK once the original project is rebuilt in Android Studio.

Since you are already adding precompiled libraries to your libplctag4j project then you will eventually need to consider adding Android ones as well.

@kyle-github
Copy link
Member

Hi, the version.h file is generated by CMake from version.h.in. It is not a source file. I deleted all my copies of libplctag in the Android project and rebuilt and it is working fine. Now, it is possible that somehow I still have the file somewhere in the include path so things work. Are you using the Android repo to build it? I wonder if I still have commits in the wrong branch...

Thanks for the heads up about the callbacks.

I think the ASAN libraries are not needed. I went through the CMake config file when I was making the core library work as in under Android Studio and I noticed that for some reason I have ASAN flags set if I compile with CLang. I am going to remove those this weekend to see if they are needed at all. If they are not, then I can probably remove all the code in the Gradle build file that copies them.

I think I may generate two JAR files for the libplctag4j project. One will be a "fat" JAR as is built now. The other will be a "skinny" JAR that only has the Tag.class file in it. That way I can pick up the generated artifact directly for Android and not have to copy around the Tag.java file. Once I figure out how to push all this to jcentral.com or mavencentral.com then it could be included as a simple dependency. At least that is the theory!

@GitHubDragonFly
Copy link
Contributor Author

Now it makes things more clearer.

I don't think that version.h file came from any of your distributions then. It is listed as a library source in CMakeLists.txt so I assumed it should be present and probably saw one in the lib folder after the library was compiled.

I will agree with you about ASAN libraries not being needed, I just never experimented with removing those flags.

@GitHubDragonFly
Copy link
Contributor Author

I just tried removing those -fsanitize flags, as well as the code from the build.gradle file, and everything seems to be working properly without the need for ASAN libraries.

Compiled libplctag libraries are now far less in size, approximately 1/3 of what it used to be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants