serial icon indicating copy to clipboard operation
serial copied to clipboard

Published 20 hours ago • verified publisher icon wjwwood

Added android support

Open chzhong opened this issue 9 years ago • 9 comments

  1. Added Android NDK support for serial.
  2. Give a simple sample app for android.

Android library is not strictly tested.

chzhong avatar Jan 21 '16 10:01 chzhong

Wow this is a big contribution, thanks!

A few things though:

  • The CI is failing on OS X
  • You seem to have committed some generated files like generated documentations and .jar files.

Any thing that can be generated should not be committed. I'd argue that this pr should be considerably fewer files. Once you clean that up I'll take a look at the substantive changes to see if they can be merged.

wjwwood avatar Jan 21 '16 17:01 wjwwood

I've corrected the build break and clean up generated files.

Please check again, thanks.

chzhong avatar Jan 23 '16 08:01 chzhong

Thanks I'm going to take a harder look at the changes since it's a big change.

In the mean time, what do you think about putting the sample android project in a different repository (perhaps on your github user account)? My concern is that it adds a lot of files to this repository and it has assets which anyone not using Android will not care about. I could be convinced otherwise, but my instinct is to separate it into a new repository.

wjwwood avatar Jan 25 '16 05:01 wjwwood

It is a nice idea to have android-specific code move to a new repository. I'll move them out, leaving only pure C++ (or native speaking, JNI codes) for android build.

chzhong avatar Jan 25 '16 06:01 chzhong

The key point to make this library available for android is to provide missing header files in NDK toolchain.

I've remove other files, reducing number of changed files to 11. This might a a light change and you could review it faster.

Android-specific files, Java APIs and sample are moved to my own repo: https://github.com/chzhong/serial-android.

chzhong avatar Jan 25 '16 06:01 chzhong

Thanks I'll try to review this sometime today or this week.

wjwwood avatar Jan 31 '16 19:01 wjwwood

This looks fine, but I have one request before merging.

Can you list the files you've imported in the android/README.md and what their respective licenses are and where you got them from? One reason is to consolidate the license information in one place and the other is so that it is relatively easy for someone to go and look for updated versions of those files in the future (if need be). It might be unnecessary, but I'd appreciate it if you'd do that.

After that I think this is ready for merge! Thanks again for the contribution and taking the time to iterate with me.

wjwwood avatar Feb 14 '16 00:02 wjwwood

Got it, I've listed files I imported.

These are standard linux headers and have their licenses in their header.

chzhong avatar Feb 14 '16 10:02 chzhong

I am also looking for Android support. What have been the main blockers to merge this branch?

Nevermind. Checked the code.

stefbo avatar Sep 10 '23 10:09 stefbo