Android - Fixes https://github.com/apache/cordova-plugin-network-information#110

[PieterVanPoyer]

Platforms affected

Android

Motivation and Context

Fixes #101
Fixes #110
Fixes #93

Description

The JSONObject of Android does not have an equals implementation. So the equals between previous networkinfo and new networkinfo state always returned false.
I've explicitly tested all properties of the JSONObject for equality.

After a review I noticed that the "extraInfo' value of the JSONObject was not used and needed at all. So I did simplify the plugin to keep the state in a String member. So now the equality is done on a simple String object.

Testing

I did test it on an Android Nexus 5X Emulator and on my Samsung .

Checklist

• I've run the tests to see all new and existing tests pass
• I added automated test coverage as appropriate for this change => Was not necessary
• Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
• If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)
• I've updated the documentation if necessary. => Was not necessary.

I didn't prefix the commit, but I did prefix the PR.

This replaces #111 . The original was started from the master branche. This PR is started from a bugfix-branche.

[breautek]

Comments on the original still applies: #111 (comment)

With the important note of:

This looks good to me. While this is fixing a bug, it is changing behaviour (which is to stop event firing on resume, if there was no change in state), which is a behaviour that some people may be dependent on. Because of this, I think we need to treat this as a breaking change.

[breautek]

Do let me know when it's ready for second review.

[PieterVanPoyer]

Ready for 2nd review.

[breautek]

lgtm 👍

I'd like to see a view from at least one other PMC member before I merge.

[PieterVanPoyer]

Hey @timbru31 or @erisu

I do not like to disturb you guys, but does anyone have time to review this PR?
I am always available to discuss this PR.

Kind regards
Pieter

[timbru31]

Some minor style suggestions and a question. It would also be nice if you could bring all the else/else if statement of getType to the same line as the bracket.

[PieterVanPoyer]

It would also be nice if you could bring all the else/else if statement of getType to the same line as the bracket.

I am not entirely sure about your remark. But I did improve it a little bit.

Or do you mean changing this

} else if (type.startsWith(CDMA) ||
                    type.equals(UMTS) ||
                    type.equals(ONEXRTT) ||
                    type.equals(EHRPD) ||
                    type.equals(HSUPA) ||
                    type.equals(HSDPA) ||
                    type.equals(HSPA) ||
                    type.equals(THREE_G)) {
                return TYPE_3G;
            }

to something like this?

} else if (type.startsWith(CDMA) || type.equals(UMTS) || type.equals(ONEXRTT) || type.equals(EHRPD) || type.equals(HSUPA) || type.equals(HSDPA) || type.equals(HSPA) || type.equals(THREE_G)) {
    return TYPE_3G; 
}

[PieterVanPoyer]

Hey @timbru31 or @erisu

I do not like to disturb you guys, but does anyone have time to review/approve this PR?
I am sometimes available to discuss this PR.

Kind regards
Pieter

[breautek]

It looks like all the requested changes from @timbru31 have been made.

I'm going to say a last call for reviews. If there are no responses by Tuesday, July 7th, I'll merge this in.

[davidpadych]

Hi,
when will the new version be released? merged with this PR .. thank you

[breautek]

Hi,
when will the new version be released? merged with this PR .. thank you

Can't really provide any ETAs on releases.

[breautek]

There was no responses, so I'm merging in.

You can also ignore the TotalPave review, that was me on the wrong account.

Thank you @PieterVanPoyer for your time and effort.

[PieterVanPoyer]

Hey @breautek

I did enjoy contributing a little.
Your enthusiasm is infectious.

I got a few questions

• Can we speed up the release process in some way? If yes, how?

• Working on the plugin was not that smooth for me, I did copy paste the Java code from a test project back to the plugin project. Is there a better workflow, for developing or contributing to plugins?

• Is it possible to open an issue or a milestone, or something to discuss the future requirements of the plugin (v4). Something like a whitepaper. I did made some suggestions in this comment bug: 'onConnect' does not work as expected #110 (comment) . You did already do some suggestions in next comment: Online event fires every time app resumes from background (iOS and Android) #91 (comment)

Kind regards
Pieter

[breautek]

Sorry for the lack of timely response, I think it got lost in the bucket of emails.

Can we speed up the release process in some way? If yes, how?

Cordova is entirely ran by volunteers, so generally speaking we're at the mercy of a volunteer, who happens to have release permissions to spend their time preparing the release, which is in itself a lengthy process.

I personally would like to obtain npm access so that I can prepare releases, but I'm not sure if I'm equipped to do that. Ie. I have no mac hardware, so I'm not sure if that limits my ability to do releases or not. It would definitely limit my ability to conduct local native tests for ios code, for repos that have them.

Working on the plugin was not that smooth for me, I did copy paste the Java code from a test project back to the plugin project. Is there a better workflow, for developing or contributing to plugins?

This is pretty much what I do, but because I don't really delve into native code that much... I believe if you use the platform add or plugin add with the --link flag, it will create symbolic links back to the repo.

e.g: cordova plugin add file:../cordova-plugin-network-information --link

Is it possible to open an issue or a milestone, or something to discuss the future requirements of the plugin (v4). Something like a whitepaper. I did made some suggestions in this comment #110 (comment) . You did already do some suggestions in next comment: #91 (comment)

We generally use github projects, or sometimes a meta ticket like apache/cordova#142 for example.

Other ways to get more involved is by joining our slack, and subscribing to the dev mailing list

And of course, ideas for improving workflows is always welcome.