-
Notifications
You must be signed in to change notification settings - Fork 16
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
mobile: Go based replacement for MKAsyncTask #347
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1. using GOPATH 2. using `go mod vendor` Binary size is 4x the binary size of MK on Android. This leads me to think maybe we can have them side by size initially. MK is ~7 MiB per arch, this is ~28 MiB per arch. If we don't split app by architecture, binary size is probably going to be a problem, I am afraid.
This was referenced Feb 18, 2020
This was referenced Feb 18, 2020
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR introduces a new API in
./mobile/oonimkall
. This API satisfies the requirements described by #339 and, hence, closes #339. It is called oonimkall because it's a OONI reimplementation of the mkall API implemented at https://github.com/measurement-kit/mkall-ios and at https://github.com/measurement-kit/android-libs. This reimplementation just focuses on the AsyncTask class in such MK API, which is the one allowing to run experiments.In the PRs that prepared this PR we have already seen that we've now the possibility of generating a probe-engine library that does not link MK. This allows OONI to use MK as before and to add, on the side, probe-engine, incrementally delegating more functionality to it.
Such new new API is as follows:
Passing this API to
gomobile bind
yields the following:Whereas https://github.com/measurement-kit/android-libs is:
The result is close enough. Perhaps, we should adjust https://github.com/measurement-kit/android-libs to throw explicitly
Exception
rather than throwingRuntimeException
implicitly.The new code is fully tested but the tests do not stress all we care about. There is a bunch of places in which we could improve, by making sure we emit the right events in sequence.
We also extended the
example
experiment to help with writing tests.