New IOIOCoreLib

[ThanosFisherman]

Hey Ytai I hope I made this pull request correctly. Let me explain briefly what I did.
I just took all the common IOIO code and moved it to this seperate library called IOIOCoreLib.
Then I made a static Logger.log class inside the core that implements a "strategy-like" pattern so that when it's called from desktop it will print on console using println() and when it's called from android it will print on logcat using log().

Now in order to do that I had to move the static blocks from IOIOAndroidApplicationHelper to its constructor. Otherwise i would get NPE cause static block tries to log before my Logger is instantiated. But then I saw this caused some issues probably related to threading stuff im not sure So I then moved the code back to static block and added a simple system.out.println to IOIOConnectionRegistry. Now my unified logger works fine. But if you definatelly want those log messages inside IOIOConnectionRegistry to be printed as a verbose on logcat, we will have to find another way (I have something in my mind)

[ThanosFisherman]

Hey Ytai thanks for your suggestions they really helped me. Check out my last 4 commits:

• IOIOCoreLib renamed to IOIOLibCore
• getType and its related methods were indeed unnecessary and they have been removed
• I'm not sure where is that log_ field that you want it private, But if you mean the Logger.log one then I suggest we should leave it public so that 3d party applications made by users (i.e HelloIOIO) can use it too and benefit from it
• Logger abstraction works fine now. I instatiate the Logger.log class in static blocks of both Pc and android backends as you suggested using Class.forname("AConcreteClassHere") I just hope I understood what you just explained here. Check out the code and let me know.

[ytai]

Hey Thanos,

Let's start with a single commit refactoring the log. After that, following up with an additional commit for breaking the libraries would be straightforward, and that can be followed by your introduction of the AS projects, which is our ultimate goal. Breaking up this pull-request into those logical pieces would make the review process more efficient and would result in a much cleaner history.

I believe this change can be made with no impact at all on the call sites.
The (now-platform-agnostic) Log class will retain its static i(), d(), e(), ... methods. What these methods will do is simply forward to a private static member ("log_") of type ILogger. This member will be initialized along the lines of (actual code needs some error handling):

private static ILogger log_ = Class<T extends ILogger>.forName("ioio.lib.spi.ConcreteLog").newInstance();

This removes the compile-time dependency of the Log class on ioio.lib.spi.ConcreteLog, and it would be sufficient to provide it in runtime, as you've done.

As a side-note:
Line endings have been messed up on several files. Looks like you may have the core.autocrlf setting wrong on your machine. Make sure it is set to "auto" if you're on Windows, "input" otherwise.

Thanks for bearing with me!

[ThanosFisherman]

Hey,

I've been trying to figure out what exactly you want to achive with this but I am a little confused here. I have no clue where you want me to declare that private log_ thing of the type ILogger. In which class should I make that initialization inside? Also where is that Log class you are refering to?

Can you please take a look at my Logger class of IOIOLibCore under ioio.lib.spi.Logger ?
Inside the Logger class there is the nested ILogger interface, then there is a public log field of type ILogger and finally the addLogBootstrap method.

Take a look at the whole Logger class and especially at addLogBootstrap method and let me know which part you don't like and want to change (I wouldn't change a thing cause everything works as expected now but you know better)

My whole point from the beginning was to use that syntax: Logger.log.i("tag","im logging") everywhere (both IOIOLibAndroid and IOIOLibPC are already using it) And it already works everywhere now!

[ytai]

If you rename your Logger class to ioio.lib.spi.Log and implement all the
static methods as in the existing classes in a way that delegates to a
private instance of type ILogger, then you wouldn't need any changes on the
call sites, e.g. your change will not have to go and change every call to
Log.d(...) to Logger.log.d(...).
The addLogBootstrap is not clean. It requires the app to do this
initialization and it is not clear that it will always be initialized early
enough. On the other hand, if you initialized from a static {} block, we
can be sure that the first time the Log class is used, the correct
implementation will be bound.

On Sat, Feb 7, 2015 at 5:25 PM, Thanos Fisherman notifications@github.com
wrote:

Hey,

I've been trying to figure out what exactly you want to achive with this
but I am a little confused here. I have no clue where you want me to
declare that private log_ thing of the type ILogger. In which class
should I make that initialization inside? Also where is that Log class
you are refering to?

Can you please take a look at my Logger class of IOIOLibCore under
ioio.lib.spi.Logger ?
Inside the Logger class there is the nested ILogger interface, then there
is a public log field of type ILogger and finally the addLogBootstrap
method.

Take a look at the whole Logger class and especially at addLogBootstrap
method and let me know which part you don't like and want to change (I
wouldn't change a thing cause everything works as expected now but you know
better)

My whole point from the beginning was to use that syntax: Logger.log.i("tag","im
logging") everywhere (both IOIOLibAndroid and IOIOLibPC are already using
it) And it already works everywhere now!

—
Reply to this email directly or view it on GitHub
#103 (comment).

[ThanosFisherman]

Ah I see what you mean with "no call site changes". So you want instead of Logger.log.i() to just do Log.i() just like the normal android Log API while keeping this platform agnostic at the same time.

As for the addLogBootstrap it is initialized inside the static blocks of IOIOAndroidApplicationHelper and IOIOPcApplicationHelper. Isn't that ok?

P.S. I think we have 8 hours difference or something so it may take me a while to see your posts

[ytai]

Doing the initialization in a static block is a better interface since it
is more encapsulated and has less requirements from the user (of this
class).
On Feb 7, 2015 8:09 PM, "Thanos Fisherman" notifications@github.com wrote:

Ah I see what you mean with "no call site changes". So you want instead of
Logger.log.i() to just do Log.i() just like the normal android Log API
while keeping this platform agnostic at the same time.

As for the addLogBootstrap it is initialized inside the static blocks of
IOIOAndroidApplicationHelper and IOIOPcApplicationHelper. Isn't that ok?

P.S. I think we have 8 hours difference or something so it may take me a
while to see your posts

—
Reply to this email directly or view it on GitHub
#103 (comment).

[ThanosFisherman]

hey Ytai check out my refactored Log class. I made the static d,e,i....etc methods that forward to the corespondings methods of private log_ interface. Now the call sites have been renamed back to as they used to be (using Log.i instead of Logger.log.i) I believe it's ok now.

I didn't do any change on addLogBootstrap method tho cause I still don't get it. Check this. I initialize the private log_ interface using the following code inside IOIOAndroidApplicationHelper

    static {
         Log.addLogBootstrap("ioio.lib.util.android.ConcreteLog");
        IOIOConnectionRegistry
                .addBootstraps(new String[] {
                        "ioio.lib.impl.SocketIOIOConnectionBootstrap",
                        "ioio.lib.android.accessory.AccessoryConnectionBootstrap",
                        "ioio.lib.android.bluetooth.BluetoothIOIOConnectionBootstrap",
                        "ioio.lib.android.device.DeviceConnectionBootstrap"});
    }

and inside IOIOPcApplicationHelper

static {
        Log.addLogBootstrap("ioio.lib.util.pc.ConcreteLog");
        IOIOConnectionRegistry
                .addBootstraps(new String[] { "ioio.lib.pc.SerialPortIOIOConnectionBootstrap" });
    }

I believe the Log is initialized early enough. Could it be earlier?

[ytai]

You're getting close: if you now simply move the log_ initialization into the Log class as a static block and use a unified class name (e.g. ioio.lib.SPI.ConcreteLog), there is no longer any impact on client code.
Once you're done, please send me only this change for review and I'll merge it. The next step would be changing the library structure (still in Eclipse) and then the next one is AS/gradle project files.

[ytai]

Oh, and please get rid of the log level filtering.

[ThanosFisherman]

So you want me to make a new package ioio.lib.SPI in both IOIOPcApplicationHelper and IOIOAndroidApplicationHelper and move each of their ConcreteLog classes in there?

[ytai]

It is not new. This package (ioio.lib.spi) has always existed in the
platform-specific code area. Previously it had the Log class, now it will
have the ConcreteLog (is LogImpl a better name, since the Log class itself
is also concrete?) class instead.

On Sun, Feb 8, 2015 at 5:21 PM, Thanos Psaridis notifications@github.com
wrote:

So you want me to make a new package ioio.lib.SPI in both
IOIOPcApplicationHelper and IOIOAndroidApplicationHelper and move each of
their ConcreteLog classes in there?

—
Reply to this email directly or view it on GitHub
#103 (comment).

[ThanosFisherman]

Ah you are right it's there! I just deleted it cause I didn't need it anymore :P I Could never thought we would follow that kind of implementation (which may seem very natural in practise but it took me a while to figure it out how to do it)
OK I think I got this I'm commiting the final Log class

[ThanosFisherman]

ok cool I'm renaming the ConcreteLog into LogImpl too

[ytai]

Awesome. Please just make a pull request containing one commit with only
that.
On Feb 8, 2015 5:33 PM, "Thanos Psaridis" notifications@github.com wrote:

Ah you are right it's there! I just deleted it cause I didn't need it
anymore :P I Could never thought we would follow that kind of
implementation (which may seem very natural in practise but it took me a
while to figure it out how to do it)
OK I think I got this I'm commiting the final Log class

—
Reply to this email directly or view it on GitHub
#103 (comment).

[ThanosFisherman]

A single commit with everything I've done so far? Or just the Log class?

[ytai]

Just the Log. Let's get it merged and then we can follow up with the next
steps.
On Feb 8, 2015 5:52 PM, "Thanos Psaridis" notifications@github.com wrote:

A single commit with everything I've done so far? Or just the Log class?

—
Reply to this email directly or view it on GitHub
#103 (comment).

[ThanosFisherman]

ok here you go ^^^

[ThanosFisherman]

Btw If you eventually merge my last commit you'd better delete those auto generated //TODO comments cause android studio inspection usually complains about that stuff (among other as well)

[ytai]

Thanos, if you look at the current state of this pull request, it has 10 commits and 132 changed files.
Can you change it (or make a new pull request) that I can apply on the current ioio/master branch, which includes only the Log changes. There is some git magic that should make this a simple task (e.g. git rebase). If you prefer, I can do that instead.

[ThanosFisherman]

Well I've never used that rebase thing before. I'm using egit from within
eclipse and I'm afraid I might mess things up if i start experimenting with
it now, can you do this? Just take the Log class from my master branch or
you know do your magic

2015-02-10 4:47 GMT+02:00 Ytai Ben-Tsvi notifications@github.com:

Thanos, if you look at the current state of this pull request, it has 10
commits and 132 changed files.
Can you change it (or make a new pull request) that I can apply on the
current ioio/master branch, which includes only the Log changes. There
is some git magic that should make this a simple task (e.g. git rebase).
If you prefer, I can do that instead.

—
Reply to this email directly or view it on GitHub
#103 (comment).

[ytai]

IOIO Android apps are just normal Android apps, so you can do from them
anything that you can do with Android, such as GUI, internal sensors,
Internet, etc. Of it wasn't for that, there wouldn't be a big difference
between using a IOIO and using an Arduino in terms of what you can do.
For anything that is not IOIO specific, please check
http://developer.android.com
There is definitely some learning curve, especially if you don't have a
strong Java background, but it's well worth it!
On Feb 9, 2015 7:14 PM, "Thanos Psaridis" notifications@github.com wrote:

Well I've never used that rebase thing before. I'm using egit from within
eclipse and I'm afraid I might mess things up if i start experimenting with
it now, can you do this? Just take the Log class from my master branch or
you know do your magic

2015-02-10 4:47 GMT+02:00 Ytai Ben-Tsvi notifications@github.com:

Thanos, if you look at the current state of this pull request, it has 10
commits and 132 changed files.
Can you change it (or make a new pull request) that I can apply on the
current ioio/master branch, which includes only the Log changes. There
is some git magic that should make this a simple task (e.g. git rebase).
If you prefer, I can do that instead.

—
Reply to this email directly or view it on GitHub
#103 (comment).

—
Reply to this email directly or view it on GitHub
#103 (comment).

[ThanosFisherman]

I guess this answer was not intended for me here :P Check out my previous reply tho (I've never used rebase and pull requests before so I don't know what to do now) 👍

[ThanosFisherman]

Neat! Good Idea how you shrunk the ILogger interface into 1 method keeping everything minimally invasive :D

Now if you still want to move all the common code to a separate project (IOIOLibCore) that's up to you. I'd want to have it as a separate project tho cause it is easier to deal with in android studio and maybe make it a .jar file and push it to maven or something. (no need for .aar files in common code)

I still don't know how the maven procedure is done, but I'm willing to help as far as my knowledge allows

[ytai]

I'm still debating myself about the usefulness of breaking the libraries
vs. the cost.
A few arguments for / against the split:

• For: in some environments having a single source directory found in a
  specific place relative to the project directory is easier / more natural.
• For: it would be possible to distribute a single JAR that would apply
  to both environments.
• Against: while the Android / PC variants of the library share the
  exact same source code, they shouldn't necessarily share the same
  project/build settings. Eclipse won't let you define configurations for
  Java projects. It may not be a real problem in reality, but coming from a
  C++ background I'm very accustomed to the possibility of building the same
  sources in different ways. One setting that comes to mind is the Java 1.6
  vs. 1.7 compatibility mode: older Android targets require 1.6 whereas for
  newer Android / PC you would typically want 1.7.
• Against: yet another library that you'd need to load and reference,
  increasing the amount of boilerplate one need to do in order to get a
  simple IOIO project to build.

The last two points might be somewhat mitigated by AS/Maven, but I'm not
sure how it would impact users that still prefer using Eclipse (which, at
least for PC is a legitimate choice).

On Fri, Feb 13, 2015 at 8:37 AM, Thanos Psaridis notifications@github.com
wrote:

Neat! Good Idea how you shrunk the ILogger interface into 1 method keeping
everything minimally invasive :D

Now if you still want to move all the common code to a separate project
(IOIOLibCore) that's up to you. I'd want to have it as a separate project
tho cause it is easier to deal with in android studio and maybe make it a
.jar file and push it to maven or something. (no need for .aar files in
common code)

I still don't know how the maven procedure is done, but I'm willing to
help as far as my knowledge allows

—
Reply to this email directly or view it on GitHub
#103 (comment).

[ThanosFisherman]

Good points.

• Yeah that what I was thinking when I first came up with the idea of
  breaking the common lib. Cause It took me a while to figure out what was
  that the nested "target" directory thing
• Maybe you should distribute the common lib as a jar. It will benefit the
  users who don't wanna mess with the library sources
• As for the compatibility issues I believe there shouldn't be any problem
  at all cause your code seems to be written using java 1.6 principles and
  backwards compatibility of newer java versions really works. Plus even if
  you are targeting an old android version, there won't be any problem. Java
  1.7 was introduced with android build tools 19 and I don't see any reason
  why one should use build tools prior to 19 and even if that's the case he
  could just code in java 1.6 on his own projects. As for the project/build
  settings most of them are eclipse specific and they are just added to the
  already existing settings of your android/pc projects respectively. I can't
  tell if there will be a problem with that. It seems ok to me cause again
  backwards compatibility works. So I don't know, you decide
• Yeah extra boilerplate libs is something I don't like too. Can't argue
  with that.

Hopefully maven-gradle intergration will work with eclipse and ADT too in
the future. Eclipse is great tool both for android and pc And btw Eclipse
team promised to keep updating ADT by themselves via "andmore" project. Its
still in the works tho

2015-02-14 1:40 GMT+02:00 Ytai Ben-Tsvi notifications@github.com:

I'm still debating myself about the usefulness of breaking the libraries
vs. the cost.
A few arguments for / against the split:

• For: in some environments having a single source directory found in a
  specific place relative to the project directory is easier / more natural.
• For: it would be possible to distribute a single JAR that would apply
  to both environments.
• Against: while the Android / PC variants of the library share the
  exact same source code, they shouldn't necessarily share the same
  project/build settings. Eclipse won't let you define configurations for
  Java projects. It may not be a real problem in reality, but coming from a
  C++ background I'm very accustomed to the possibility of building the same
  sources in different ways. One setting that comes to mind is the Java 1.6
  vs. 1.7 compatibility mode: older Android targets require 1.6 whereas for
  newer Android / PC you would typically want 1.7.
• Against: yet another library that you'd need to load and reference,
  increasing the amount of boilerplate one need to do in order to get a
  simple IOIO project to build.

The last two points might be somewhat mitigated by AS/Maven, but I'm not
sure how it would impact users that still prefer using Eclipse (which, at
least for PC is a legitimate choice).

On Fri, Feb 13, 2015 at 8:37 AM, Thanos Psaridis <notifications@github.com

wrote:

Neat! Good Idea how you shrunk the ILogger interface into 1 method
keeping
everything minimally invasive :D

Now if you still want to move all the common code to a separate project
(IOIOLibCore) that's up to you. I'd want to have it as a separate project
tho cause it is easier to deal with in android studio and maybe make it a
.jar file and push it to maven or something. (no need for .aar files in
common code)

I still don't know how the maven procedure is done, but I'm willing to
help as far as my knowledge allows

—
Reply to this email directly or view it on GitHub
#103 (comment).

—
Reply to this email directly or view it on GitHub
#103 (comment).

[hannesa2]

Sorry, it's hopeless outdated. Please feel free to reopen it again

[ThanosFisherman]

Actually it was about time to close this. The content of this pull request has been already implemented so no need to do anything here. :)