Implement discovering in sending mode and advertising in receiving mode #26
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
- Receive button click shows a dialog notifying the user that we are waiting for a device to connect - Enable cancel button to cancel the dialog - Send and receive buttons are disabled when the dialog is shown
- Add Nearby Connections API - Add Timber dependency - Add P2PLibrary which will enable adding implementation of interfaces and requirements for the library - Instantiate the P2pModeSelectInteractor in the P2pModeSelectPresenter
1. Enforce username when initialising username 2. Make the presenter testable by allowing the interactor as a param 3. Enforce initialising the P2PLibrary by introducing a RuntimeException if it is accessed without being intialised. This means that it will fail fast during developmental testing since getInstance has to be called whenever interacting with the library
1. Add Nearby Connections API permissions check & request 2. Check if location is enabled & request for enabling it if not 3. Add onResume and onActivityResult handlers for the activity 4. Add Permissions util class
Also added P2pModeSelectInteractor tests
Code cleanup
Fixes #2 1. Show discovering progress dialog 2. Start discovery when the send button is clicked 3. Check for permissions when send button is clicked & ensure location is enabled before starting discovering 4. Stop discovering when the cancel button is clicked 5. Fixed bug where advertising is not stopped since the connection client used to stop advertising is not the same as the one that started advertising 6. Add onActivityRequestionPermissionHandler interface to enable easily switching of different handlers for the same permissions request 7. Rename handlers package to handler for more consistency on package naming
|
BLOCKED: This branch and PR depends on #23 |
Added the line enabling robolectric to use the normal module resources as per http://robolectric.org/migrating/\#project-configuration
- Added a TestApplication class to set the theme for the activities
1. Removing references to context and other context carrying objects to remove memory leaks 2. Stop advertising, discovering and any active endpoints 3. Re-initialise context carrying objects on activity start
|
Closed #23 and included in this PR |
githengi
reviewed
Mar 13, 2019
| initializePresenter(); | ||
|
|
||
| sendButton.setOnClickListener(null); | ||
| receiveButton.setOnClickListener(null); |
There was a problem hiding this comment.
Is this needed
sendButton.setOnClickListener(null);
receiveButton.setOnClickListener(null);
There was a problem hiding this comment.
The activity is/was behaving kinda weird whereby the onCreate method is called twice everytime it starts. This also weirdly resulted in the presenter.onSendButtonClicked() being called twice (😞 which shouldn't be remotely possible) I think that this should remedy the situation. I'll retest it and get back to you
There was a problem hiding this comment.
After testing on my device onCreate and onStart is only invoked only once.
Shouldn't the click handlers be setup in onCreate?
Also if you set them up on onStart then you should unregister then on onStop
githengi
reviewed
Mar 13, 2019
p2p-sync/src/main/java/org/smartregister/p2p/activity/P2pModeSelectActivity.java
Show resolved
Hide resolved
githengi
reviewed
Mar 13, 2019
| * Created by Ephraim Kigamba - ekigamba@ona.io on 11/03/2019 | ||
| */ | ||
|
|
||
| public abstract class Constants { |
There was a problem hiding this comment.
Consider using an interface instead on abstract class for configs
githengi
reviewed
Mar 13, 2019
|
|
||
| public static final Strategy STRATEGY = Strategy.P2P_STAR; | ||
|
|
||
| public abstract static class RQ_CODE { |
There was a problem hiding this comment.
Consider using an interface instead on abstract class for configs
- Change Constants class to interface instead of abstract class
githengi
approved these changes
Mar 14, 2019
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.
Fixes #2
Fixes #1
When installed on two devices, the sender should be able to discover the receiver. This will be shown with a toast message giving the name John Doe and packageName