add ability to enter gps location manually #181

constambeys · 2019-07-16T07:05:45Z

No description provided.

ruleant · 2019-07-28T12:20:59Z

Thanks for the PR implementing #73 , @constambeys
On first glance it looks good. I'll review in more detail and test it soon.

ruleant

Nice contribution. I like how you used method overloading and rewrote the storeLocation method a bit to make it more generic and available for reuse and thus avoid code duplication.
I've added some minor remarks as comments in the code.
As soon as I'm able to test I might add a few more remarks.

res/values/strings.xml

src/java/com/github/ruleant/getback_gps/AbstractGetBackGpsActivity.java

src/java/com/github/ruleant/getback_gps/LocationService.java

constambeys · 2019-08-21T10:35:58Z

is it ok now?

ruleant · 2019-09-04T20:02:19Z

Great, thanks for the changes.

ruleant · 2019-09-04T20:37:42Z

Hi @constambeys
I did a quick test and I was able to enter a location using the form you created, so that's great!

However, input/boundary checking should be included :

A valid range for longitude : +180° to −180°
A valid range for latitude : +90 to -90°

I was unable to enter a negative number, so I guess you have to change the input type for the longitude and latitude input fields.
I was able to enter a number outside the valid range, the coordinate was rejected and as a result the location was not stored, but there should be a message telling the user one of the parameters was invalid.

ruleant · 2019-09-04T20:49:05Z

src/java/com/github/ruleant/getback_gps/AbstractGetBackGpsActivity.java

+        builder.create().show();
+    }
+    public final void enterLocation() {
+        if (mBound) {


should be if (!mBound) {

ruleant · 2019-09-04T20:53:25Z

If you want to test the app, you can find some info on how to build and deploy the app using a vagrant box : https://github.com/ruleant/getback_gps/wiki/Development
Or if you use an IDE that integrates with Android, you can probably test using an emulated android device.
Let me know if you need help.

constambeys · 2019-09-07T09:51:00Z

fixed check now

ruleant · 2019-09-07T19:45:11Z

Great stuff, @constambeys!
Can you just add the error strings to the strings.xml file, so they can be translated?

constambeys · 2019-09-08T10:28:57Z

moved

ruleant

Great stuff, @constambeys
Again apologies for the late reaction. I'm a bit busy at the moment and unfortunately I can't find much time for working on the getback_gps android app, or testing/reviewing contributions. But that doesn't mean I don't appreciate your work and contribution, on the contrary.

I think your contributed 'manual location entry' form is nearly ready to merge with the code base. I've added a few cosmetic remarks regarding strings, that should be easy to fix.

After that, there's a few administrative things :

don't forget to add your name to the Copyright notice at the beginning of each code file you made changes to. Look for the Copyright line where my name is mentioned and add one with your name :
Copyright (C) 2019 Timotheos Constambeys
add an entry to the Changelog file (in the 0.7 release section), something like : issue #73 : add form to manually enter a location, thanks to Timotheos Constambeys
can you rebase your branch to the master branch and squash all your commits into 1?
That way your work on this feature is all in one commit, which makes the commit history cleaner.
use git rebase -i upstream/master (where upstream is the name of my repo that you use in your config)
Tip : For future work, it's better to use a separate branch to do your development (and rebase that branch regularly to upstream master), then you avoid those unnecesary merge commits.

ruleant · 2019-09-29T10:52:13Z

res/values/strings.xml

    <string name="cancel">Cancel</string>
    <string name="store_location">Store Location</string>
+    <string name="enter_location">Enter Location</string>


better : "Enter destination manually"
this way it indicates you can manually enter a destination.

ruleant · 2019-09-29T10:56:09Z

src/java/com/github/ruleant/getback_gps/AbstractGetBackGpsActivity.java

        MenuItem miRenameDest = menu.findItem(R.id.menu_renamedestination);
        if (isBound()) {
            // enable store location button if a location is set
            miStoreLocation.setEnabled(mService.getLocation() != null);
+            miEnterLocation.setEnabled(mService.getLocation() != null);


Manual entry can be enabled any time, it doesn't depend on a current location being present, so this can be miEnterLocation.setEnabled(true);

ruleant · 2019-09-29T11:01:52Z

src/java/com/github/ruleant/getback_gps/AbstractGetBackGpsActivity.java

+                = (EditText) dialogView.findViewById(R.id.location_longtitude);
+        // Set the layout for the dialog
+        builder.setView(dialogView)
+                .setTitle(R.string.store_location)


you should use the 'R.string.enter_location' string for the title

ruleant · 2019-09-29T11:41:07Z

After merging your feature, would you be interested in contributing some more?
You can have a look at new features you like to work on in the issues section.
Or perhaps you can improve the form you added and do some refactoring? I was thinking your form can be merged with the 'rename destination' form, where you can edit not only the name of the current destination, but also the coordinates. But this of course is work for another PR, it doesn't have to be done in this PR.

constambeys · 2019-11-10T09:21:31Z

Thanks for your help and guidance. At the moment I don't have much free time but this is a great project and will keep an eye of it in the future !!

ruleant · 2019-12-12T21:16:25Z

Merged! Thanks for your contribution. It will be part of the upcoming v0.7 release.

ruleant · 2021-01-16T14:50:38Z

@constambeys Version v0.7 was released, including the form you added!

ruleant self-assigned this Jul 28, 2019

ruleant added this to the v0.7 milestone Jul 28, 2019

ruleant added the enhancement label Jul 28, 2019

ruleant reviewed Jul 28, 2019

View reviewed changes

ruleant reviewed Sep 4, 2019

View reviewed changes

ruleant reviewed Sep 29, 2019

View reviewed changes

constambeys added 5 commits November 10, 2019 10:44

add ability to enter gps location manually

566865f

code remarks

0a7b3ea

fix user input and add error checking

d4a818b

move error messages to resource file

7216a72

add copyright notice and some code remarks

5d9e92e

constambeys force-pushed the master branch from 28b8380 to 5d9e92e Compare November 10, 2019 09:18

ruleant merged commit 8de714a into ruleant:master Dec 12, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add ability to enter gps location manually #181

add ability to enter gps location manually #181

constambeys commented Jul 16, 2019

ruleant commented Jul 28, 2019

ruleant left a comment

constambeys commented Aug 21, 2019

ruleant commented Sep 4, 2019

ruleant commented Sep 4, 2019

ruleant Sep 4, 2019

ruleant commented Sep 4, 2019

constambeys commented Sep 7, 2019

ruleant commented Sep 7, 2019

constambeys commented Sep 8, 2019

ruleant left a comment •

edited

Loading

ruleant Sep 29, 2019 •

edited

Loading

ruleant Sep 29, 2019

ruleant Sep 29, 2019

ruleant commented Sep 29, 2019

constambeys commented Nov 10, 2019

ruleant commented Dec 12, 2019

ruleant commented Jan 16, 2021

add ability to enter gps location manually #181

add ability to enter gps location manually #181

Conversation

constambeys commented Jul 16, 2019

ruleant commented Jul 28, 2019

ruleant left a comment

Choose a reason for hiding this comment

constambeys commented Aug 21, 2019

ruleant commented Sep 4, 2019

ruleant commented Sep 4, 2019

ruleant Sep 4, 2019

Choose a reason for hiding this comment

ruleant commented Sep 4, 2019

constambeys commented Sep 7, 2019

ruleant commented Sep 7, 2019

constambeys commented Sep 8, 2019

ruleant left a comment • edited Loading

Choose a reason for hiding this comment

ruleant Sep 29, 2019 • edited Loading

Choose a reason for hiding this comment

ruleant Sep 29, 2019

Choose a reason for hiding this comment

ruleant Sep 29, 2019

Choose a reason for hiding this comment

ruleant commented Sep 29, 2019

constambeys commented Nov 10, 2019

ruleant commented Dec 12, 2019

ruleant commented Jan 16, 2021

ruleant left a comment •

edited

Loading

ruleant Sep 29, 2019 •

edited

Loading