GPS Passive recording feature #275

eotin · 2019-03-20T15:19:30Z

No description provided.

Merge Onaio mater into eotin master

merge master into Develop

get master from onaio

Merge onaio master to Develop

Add PassiveRecordObjectActivity Refactor Storage helpers First commit -> Need refactoring now + tests + testing mService bounded to avoid null reference exception

Register smaple string in values/string.xml Move StoreLocation from TrackingStorage

Refactor TrackingService

Merge Onaio Master to local master

Develop

Eo/merge passive record location

From master to update develop

Develop

Fix code regarding Codacy/PR Quality Review

Update .travis.yml file to modifiy the build tools version

Fix code regarding Codacy Bot

Develop

Add Comments in TrackingServiceTest

library/src/main/java/io/ona/kujaku/views/KujakuMapView.java

ekigamba · 2019-04-11T12:45:27Z

@bripatand

Briandp: we believe relying on the "recording" API call to "display" the collected points on the screen is bad design, confusing and encapsulate too much behaviour in one API call (pattern "separation of concerns"). OK to use ArrowLine in the host to show direction thought.

Very true. I also understand separation of concerns and did not mean that literally. We are putting a lot of effort to make sure all features are configurable and not default so the the client application can use whichever part it wants and in whichever way.

Briandp: Sounds good but why only there? We've tried to be consistent. For instance the focusOnMyLocation Icon is not configurable and hardcoded in the library. And there are a few others. So shouldn't we apply the same logic everywhere in Kujaku where icons are used.

I don't refute that and sorry 🙏 if there was a misunderstanding. You guys are part of the small GeoWidget community and we(Ona and I) are very thankful for your contributions to the repository. The aim from the start of the project has been to create a library that can be used by the multiple host applications that are currently using it for various purposes, and in the near future. That is why you were able to convince us on using an actual View. The general aim is to provide the configurability aspect whenever required and makes sense.

The API has mostly been designed to either be used with a UI or without for features that would have a UI. That is why we have IKujakuMapView and IKujakuMapViewLowLevel
For features, configurability would be required due to different designs and requirements. I extended this discussion and the design provided would end up being different for different host applications. That's the reason I brought it up. The icon specifically would end up being different, the position and even whether it should be shown when the recording is started programmatically.
The addPoint is one of the features that offers such configurability in the icon to show with two options due to the nature of the feature and usability on different applications
The arrow line layer and boundary layer also offer such configurability to change such UI elements.

From the discussions on the feature and how important this feature is:

Whether the icon is shown
The position of the icon
The actual icon shown

need to be configurable.

@bripatand I hope this makes sense?

Add Comments in TrackingServiceTest

Update eotin master

update Develop branch

bripatand · 2019-04-11T16:29:22Z

@ekigamba: Ok for making the following configurable:
Whether the icon is shown
The position of the icon
The actual icon shown

Develop

sample/src/main/java/io/ona/kujaku/sample/activities/PassiveRecordObjectActivity.java

Fix TrackingService binding when resuming Add method initializeTrackingService

Make the TrackingService icon configurable

ekigamba · 2019-04-15T07:34:07Z

@bripatand Sorry, I forgot to reply to this

Briandp: from our testing, the Fuse Location API provide inaccurate points and in an unpredictable manner especially if you are not connected to a network. Most of time the resulting line or polygon is highly irregular.

It does provide inaccurate points sometimes and it aggregates all. The use of the tracking feature means that is will be on for long hours as the user performs duties in the field. The team thought that it would be good to provide the option to use a different provider not necessarily completely change to GPS. GPS is still the most accurate but it might have some cons that might not fit the situation as opposed to FusedLocation API which includes all these providers to provide an OK location while not using too much energy.

I also tested the feature while commuting(3 times) and it worked excellent once. One of the cases was a bug which caused the app to crash when I tried to check my route on arrival. For the other case, the only points that were recorded was when I started the journey and when I arrived. My assumption is that it lost GPS connection and therefore only got the points when I was stationery. On the other hand, I used Google Maps for directions twice still commuting in the similar manner and it was pretty accurate. I never lost connection, thus it would be nice to have other options. I believe FusedLocationAPI.

**

The route taken was the same
The area is urban, Kenya's capital city
I also had my internet connection off on all cases

Only the google maps situation is different and circumstances much more harsh
**

@bripatand What are your thoughts?

bripatand · 2019-04-16T06:30:57Z

@ekigamba: as you have seen, the FusedLocationAPI is unpredictable as you cannot garanty to get a point every X meter using this API. You tested it 3 times and you got 3 different behaviour. And if you don't have any connectivity, the FusedLocationAPI is going to use the GPS anyway, just adding an unpredictability... Note we have 2 modes in the current API, one that keep the GPS on all the time which gives the best results in term of accuracy and another one which relies on the GPS to tell us when the phone has moved by X meters but it is much less accurate. If we want the ability to use the FusedLocationAPI, we would need to make significant changes to the code and provide clear instruction on when to use GPS and when to use Fuse. We honestly don't know when we would recommend to use the Fuse if your objective is to collect a "route" you can use. Note as well that Google apps (Map etc...) have root privililege and can do some actions that no other app can, typically bypassing the power saving feature enforced by Android when needed...
Finally, you cannot really lose connection to the GPS unless you are inside or potentially in New York surrounded by tall buildings. There are always at least 3 GPS satellites above your head at any time anywhere on earth (may be not at the poles). It might take several minutes to connect to them if the GPS has been off for a long time but once you get the first fix/point, it means it is connected to at least 3 of them and from that moment getting a point is immediate.
But if you think it is important, you can raise it to the group for adding it in the next release.

ekigamba · 2019-04-16T09:12:47Z

@bripatand

Cool, I understand why you are against it. Would it be possible to just abstract the specific location provider used just as we have done before in the KujakuMapView, so that all one would have to do is implement their own provider and pass it over?

There is a location client interface already https://github.com/onaio/kujaku/blob/master/library/src/main/java/io/ona/kujaku/interfaces/ILocationClient.java and all that would be required is to have this as the client in the service and have a GPS client or rather AndroidLocationClient implementing this and being the default LocationClient unless another one is specified.

Would this be OK by you guys?

library/src/main/res/values/strings.xml

library/src/main/java/io/ona/kujaku/views/KujakuMapView.java

Fix crash when the TrackingService is running and another sample activity opened

Update kujaku_permission_reason string

ekigamba · 2019-04-17T12:29:36Z

library/src/main/java/io/ona/kujaku/services/TrackingService.java

+    private final IBinder binder = new LocalBinder();
+
+    // Listener to register to some Tracking functions
+    private TrackingServiceListener trackingServiceListener = null;


I have been testing for memory leaks and your code is pretty good. Listeners attached or implemented in activities or contexts are going to have a shorter life compared to the event emitter causing memory leaks.

This listener here causes a memory leak of the PassiveRecordObjectActivity because there is no way for the activity to nullify the listener when it exits. The service however still holds a reference to the activity through the listener. Bound services fix this by having the unbind

Provide a good way to handle such memory leaks. I can see that registerTrackingServiceListener can take in nulls.

Provide a way to unregister through both the service and mapview.

Also fix the memory leak in the sample activity

Hi Ephraim, i have fixed this memory leak last week. Could you please tell me how do you test/fin memory leak ? do you use a tool or a library ? Thanks

You can generally test for leaks in code using https://github.com/square/leakcanary

The leak is fixed

ekigamba · 2019-04-17T12:35:57Z

library/src/main/java/io/ona/kujaku/interfaces/IKujakuMapView.java

+    List<Location> stopTrackingService(@NonNull Context context);
+
+    /**
+     * Unbind from the TrackingService instance to be able to stop the service


At this point our service life is not controlled by the binding idea

Add the TrackingService link to the Table of Contents

I would like to know how much you have tested the code in the field using the Kujaku sample app probably moving for more than 1 km. I am having unexpected results

What kind of unexpected results ? i have tested the kujaku sample multiple times during the itinerary between office and home and the passive activity shows me the exact route i have driven.

I walked out of the office to a Cafe and left the app as the foreground application and put it in my pocket. It recorded half my route. The route is recorded halfway there and from that point back.

Device is Android version 8.1.0

What android version did you test with?

Ok,There are some Android settings to modify :

You need to whitelist the sample application from the battery optimization. This is mandatory.

On some Huawei phones on Android 8.1, there is a new setting to modify : Settings->Battery->Launch->Managed Manually: Set the 3 Auto-launch, Secondary Launch, Run in Background

I tested the app with Android 8.1, 8.0, 7.0, 5.1

Thanks, this worked and it recorded for 3 hours without a gap while using other applications.

Can you create that as an issue and we can also document it later for easy reference and troubleshooting? The issue will basically state that this has to be done manually and that documentation for how to do it is to be added using the issue. A different issue will address adding the ability to prompt the user to enable this from the following guidelines
https://developer.android.com/training/monitoring-device-state/doze-standby#support_for_other_use_cases

ok make sense

Add method to unregister the tracking listener

bripatand · 2019-04-23T08:05:38Z

@bripatand

Cool, I understand why you are against it. Would it be possible to just abstract the specific location provider used just as we have done before in the KujakuMapView, so that all one would have to do is implement their own provider and pass it over?

There is a location client interface already https://github.com/onaio/kujaku/blob/master/library/src/main/java/io/ona/kujaku/interfaces/ILocationClient.java and all that would be required is to have this as the client in the service and have a GPS client or rather AndroidLocationClient implementing this and being the default LocationClient unless another one is specified.

Would this be OK by you guys?

We should test it first to see if we can get the FuseAPI to provide a good enough sequence of points. Our experience of this API is that it can be unpredictable. We should also discuss with the geo spatial widget group. But for a future release potentially.

ekigamba · 2019-04-25T06:56:19Z

We should test it first to see if we can get the FuseAPI to provide a good enough sequence of points. Our experience of this API is that it can be unpredictable. We should also discuss with the geo spatial widget group. But for a future release potentially.

That's OK, any notes on the issue can be added here for future reference #283

eotin and others added 23 commits January 28, 2019 17:00

Merge pull request #21 from onaio/master

60462f3

Merge Onaio mater into eotin master

Merge pull request #22 from eotin/master

88c93c2

merge master into Develop

Merge pull request #23 from onaio/master

1be6529

get master from onaio

Merge pull request #24 from eotin/master

3a6444b

Merge onaio master to Develop

Fix issue in the WMTS layout

01ea0ce

Fix spelling Wmts activity

8a588ab

Add TrackingService functionality

6749217

Add PassiveRecordObjectActivity Refactor Storage helpers First commit -> Need refactoring now + tests + testing mService bounded to avoid null reference exception

Add recording images

dcbcd1d

Register smaple string in values/string.xml Move StoreLocation from TrackingStorage

Add Unit Tests

909beb6

Refactor TrackingService

Merge pull request #25 from onaio/master

e441be4

Merge Onaio Master to local master

Merge branch 'eo/merge_passive_record_location' into Develop

d789e38

Merge pull request #26 from eotin/Develop

2b06ba4

Develop

Fix import issue after merging from onaio/master

e8b4a74

Merge pull request #27 from eotin/eo/merge_passive_record_location

8c0e4d5

Eo/merge passive record location

Merge pull request #28 from eotin/master

3875380

From master to update develop

Fix code regarding Codacy/PR Quality Review

17c85d6

Merge pull request #29 from eotin/Develop

bca4fd3

Develop

Fix code regarding Codacy/PR Quality Review

6e098a0

Merge pull request #30 from eotin/Develop

6bf33a3

Fix code regarding Codacy/PR Quality Review

Fix code regarding Codacy/PR Quality Review

2ba6a28

Merge pull request #31 from eotin/Develop

789c88e

Fix code regarding Codacy/PR Quality Review

Fix code regarding Codacy Bot

a42d1a0

Update .travis.yml file to modifiy the build tools version

Merge pull request #32 from eotin/Develop

c8aa557

Fix code regarding Codacy Bot

onaio deleted a comment Mar 21, 2019

onaio deleted a comment from eotin Mar 21, 2019

onaio deleted a comment Mar 21, 2019

onaio deleted a comment from eotin Mar 21, 2019

onaio deleted a comment Mar 21, 2019

onaio deleted a comment from eotin Mar 21, 2019

eotin and others added 4 commits April 10, 2019 18:21

Fix TrackingService code after coding review

adae2d9

Merge pull request #56 from eotin/Develop

7ad91c0

Develop

Add Comments in TrackingServiceTest

13f2285

Merge pull request #57 from eotin/Develop

bc1274c

Add Comments in TrackingServiceTest

ekigamba reviewed Apr 11, 2019

View reviewed changes

library/src/main/java/io/ona/kujaku/views/KujakuMapView.java Outdated Show resolved Hide resolved

eotin and others added 4 commits April 11, 2019 15:54

Add Comments in TrackingServiceTest

dce1b99

Merge pull request #58 from eotin/Develop

e0b890d

Add Comments in TrackingServiceTest

Merge pull request #59 from onaio/master

3c6d41f

Update eotin master

Merge pull request #60 from eotin/master

acb57fb

update Develop branch

eotin and others added 2 commits April 11, 2019 18:32

Make the TrackingService icon configurable

32e841d

Merge pull request #61 from eotin/Develop

532bebd

Develop

ekigamba mentioned this pull request Apr 12, 2019

NPE causing crash #281

Open

ekigamba suggested changes Apr 12, 2019

View reviewed changes

sample/src/main/java/io/ona/kujaku/sample/activities/PassiveRecordObjectActivity.java Show resolved Hide resolved

eotin and others added 2 commits April 12, 2019 15:04

Make the TrackingService icon configurable

5bb6db0

Fix TrackingService binding when resuming Add method initializeTrackingService

Merge pull request #62 from eotin/Develop

3f7a462

Make the TrackingService icon configurable

ekigamba suggested changes Apr 16, 2019

View reviewed changes

library/src/main/res/values/strings.xml Outdated Show resolved Hide resolved

library/src/main/java/io/ona/kujaku/views/KujakuMapView.java Show resolved Hide resolved

eotin and others added 2 commits April 16, 2019 15:57

Update kujaku_permission_reason string

0bc8e75

Fix crash when the TrackingService is running and another sample activity opened

Merge pull request #63 from eotin/Develop

ce15819

Update kujaku_permission_reason string

ekigamba suggested changes Apr 17, 2019

View reviewed changes

eotin and others added 2 commits April 18, 2019 09:57

Add method to unregister the tracking listener

0a2bf67

Merge pull request #64 from eotin/Develop

7ed98eb

Add method to unregister the tracking listener

ekigamba approved these changes May 2, 2019

View reviewed changes

ekigamba merged commit da6c16f into onaio:master May 2, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GPS Passive recording feature #275

GPS Passive recording feature #275

eotin commented Mar 20, 2019

ekigamba commented Apr 11, 2019

bripatand commented Apr 11, 2019

ekigamba commented Apr 15, 2019

bripatand commented Apr 16, 2019

ekigamba commented Apr 16, 2019 •

edited

Loading

ekigamba Apr 17, 2019 •

edited

Loading

eotin Apr 23, 2019

ekigamba Apr 25, 2019

ekigamba Apr 25, 2019

eotin Apr 25, 2019

ekigamba Apr 17, 2019

eotin Apr 17, 2019

ekigamba Apr 17, 2019

eotin Apr 17, 2019 •

edited

Loading

ekigamba Apr 25, 2019 •

edited

Loading

eotin Apr 25, 2019

bripatand commented Apr 23, 2019

ekigamba commented Apr 25, 2019

GPS Passive recording feature #275

GPS Passive recording feature #275

Conversation

eotin commented Mar 20, 2019

ekigamba commented Apr 11, 2019

bripatand commented Apr 11, 2019

ekigamba commented Apr 15, 2019

bripatand commented Apr 16, 2019

ekigamba commented Apr 16, 2019 • edited Loading

ekigamba Apr 17, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eotin Apr 17, 2019 • edited Loading

Choose a reason for hiding this comment

ekigamba Apr 25, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bripatand commented Apr 23, 2019

ekigamba commented Apr 25, 2019

ekigamba commented Apr 16, 2019 •

edited

Loading

ekigamba Apr 17, 2019 •

edited

Loading

eotin Apr 17, 2019 •

edited

Loading

ekigamba Apr 25, 2019 •

edited

Loading