Skip to content
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

Fixes issues with Droneshare Uploader service #920

Merged

Conversation

m4gr3d
Copy link
Member

@m4gr3d m4gr3d commented Jul 18, 2014

This PR fixes a couple of issues with the Droneshare uploader service:

- fixed issues with uploader service notification handling.
- improved network connectivity receiver/detector based on http://developer.android.com/training/monitoring-device-state/manifest-receivers.html#ToggleReceivers
@m4gr3d
Copy link
Member Author

m4gr3d commented Jul 18, 2014

@geeksville I entered a bogus username and password for droneshare in order to test the uploader service, and it was successfully able to upload the log files. Is that expected, or by design?

@squilter
Copy link
Member

I think that it just created an account with that login info. It should reject if you retry that same bogus username with a different bogus password.

@geeksville
Copy link
Contributor

@ne0fhyk - yes. If you pick a username that is not in use it will create that user.

@geeksville
Copy link
Contributor

@ne0fhyk - thanks for your great fixes! One question: Why did you want preferences keys to be localizable? It seems to me those strings are opaque and therefore best excluded from strings.xml.

@arthurbenemann
Copy link
Member

@kevin: It is because then they stay linked by the AS and eclipse tools,
making easy to navigate between the files.
On Jul 19, 2014 12:59 PM, "Kevin Hester" [email protected] wrote:

@ne0fhyk https://github.com/ne0fhyk - thanks for your great fixes! One
question: Why did you want preferences keys to be localizable? It seems to
me those strings are opaque and therefore best excluded from strings.xml.


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

@m4gr3d
Copy link
Member Author

m4gr3d commented Jul 19, 2014

@geeksville it also allows to have a constant reference to the preference keys strings that is accessible from code and from the preference XML file. This way if any keys need to be updated, only one value has to be changed.
The set of keys is also stored in its own file, preference_keys.xml, instead of the strings.xml file, with a comment forbidding localization of that file.

@m4gr3d
Copy link
Member Author

m4gr3d commented Jul 23, 2014

@arthurbenemann @squilter can one (or both) of you review the PR, and gives me a thumb up (or down). I'd like to merge it, as I'm having a lot of issues with the bugs it fixes when used with android wear.

@arthurbenemann
Copy link
Member

@squilter Can you review it, so we have at least one peer review.

@ne0fhyk I'm sorry for the slow down on review/ merge of pull. It's because I'm on vacation to the end of this week.

@squilter
Copy link
Member

@ne0fhyk Seems to work as expected.

There were three localization FIXMEs.

Are passwords really stored in plaintext?

@m4gr3d
Copy link
Member Author

m4gr3d commented Jul 23, 2014

@geeksville how is authentication handled on droneshare? Would it be possible to send the username/password credentials pair, and receive a unique token from droneshare that can be used for further/future authentication?
As @squilter is pointing out, the credentials should not be stored in plaintext.

@m4gr3d
Copy link
Member Author

m4gr3d commented Jul 23, 2014

@squilter I suggest we create another issue for the localization, and the credentials handling. This fix is only targeting the frequency of uploads, and the notification dispatching from the uploader service.

@m4gr3d
Copy link
Member Author

m4gr3d commented Jul 23, 2014

@arthurbenemann no problem, enjoy the break!

@m4gr3d
Copy link
Member Author

m4gr3d commented Jul 24, 2014

@squilter the localization fixmes were taking care of.
@arthurbenemann since this PR was peer reviewed, I'll go ahead, and merge it. Feel free to revert it if you find any issue.

m4gr3d added a commit that referenced this pull request Jul 24, 2014
…y_detection

Fixes issues with Droneshare Uploader service
@m4gr3d m4gr3d merged commit a81d06d into DroidPlanner:master Jul 24, 2014
@arthurbenemann
Copy link
Member

@ne0fhyk Thanks for the merge.

@m4gr3d m4gr3d deleted the fix_uploader_service_connectivity_detection branch August 9, 2014 02:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants