Implement geofence-based presence #355

jjhuff · 2017-08-29T18:56:14Z

This implements #343

Added prefs.
If Lat/Lng aren't set, it fetches /services/org.eclipse.smarthome.core.i18nprovider/config to get location. I'd be totally in favor of fetching that another way if there is one. Maybe a new core service to expose for location?
Request permissions dynamically if they weren't already granted
Setup geofences (on boot too!)
Update item state on enter/exit

I don't do tons of Java or Android, so please tell me what can be improved!

mueller-ma · 2017-08-30T10:40:52Z

mobile/src/main/java/org/openhab/habdroid/core/GeofenceIntentService.java

+ NotificationCompat.Builder builder = new NotificationCompat.Builder(this);
+
+ // Define the notification settings.
+ builder.setSmallIcon(R.drawable.ic_launcher)


Usually apps use their icon with a background in notifications

Indeed! This was lifted from elsewhere. Fixed.

mueller-ma · 2017-08-30T10:41:00Z

mobile/src/main/java/org/openhab/habdroid/core/GeofenceIntentService.java

+ // to decode the Bitmap.
+ .setLargeIcon(BitmapFactory.decodeResource(getResources(),
+ R.drawable.ic_launcher))
+ .setColor(Color.RED)


mueller-ma · 2017-08-30T10:41:19Z

mobile/src/main/java/org/openhab/habdroid/core/GeofenceIntentService.java

+ R.drawable.ic_launcher))
+ .setColor(Color.RED)
+ .setContentTitle(notificationDetails)
+ .setContentText("Geofence transition")


Please use strings.xml ressources for this.

mueller-ma · 2017-08-30T10:41:44Z

mobile/src/main/java/org/openhab/habdroid/core/GeofenceIntentService.java

+ case Geofence.GEOFENCE_TRANSITION_DWELL:
+ return "GEOFENCE_TRANSITION_DWELL";
+ default:
+ return "Unknown type"+transitionType;


Is this visible to the user? If so please use strings.xml ressources for this.

mueller-ma · 2017-08-30T10:42:37Z

mobile/src/main/java/org/openhab/habdroid/core/GeofenceRegistrationService.java

+ lat = Float.parseFloat(mSettings.getString(Constants.PREFERENCE_PRESENCE_LAT, "0"));
+ lng = Float.parseFloat(mSettings.getString(Constants.PREFERENCE_PRESENCE_LNG, "0"));
+ } catch(NumberFormatException e) {
+ Log.i(TAG, "Invalid lat/lng");


Does the user get feedback that lat/lng is invalid?

They do now:)

mueller-ma · 2017-08-30T10:45:21Z

mobile/src/main/res/xml/preferences.xml

+ <CheckBoxPreference
+ android:defaultValue="false"
+ android:key="default_openhab_presence_enable"
+ android:summary="Enable location-based presence reporting"


Please use strings.xml ressources for this.

mueller-ma · 2017-08-30T10:45:32Z

mobile/src/main/res/xml/preferences.xml

+ android:inputType="textNoSuggestions"
+ android:key="default_openhab_presence_item"
+ android:summary=""
+ android:title="Presence Item" />


Please use strings.xml ressources for this.

mueller-ma · 2017-08-30T10:45:36Z

mobile/src/main/res/xml/preferences.xml

+ android:inputType="numberDecimal"
+ android:key="default_openhab_presence_lat"
+ android:summary=""
+ android:title="Latitude" />


Please use strings.xml ressources for this.

mueller-ma · 2017-08-30T10:45:41Z

mobile/src/main/res/xml/preferences.xml

+ android:inputType="numberDecimal"
+ android:key="default_openhab_presence_lng"
+ android:summary=""
+ android:title="Longitude" />


Please use strings.xml ressources for this.

mueller-ma · 2017-08-30T10:45:51Z

mobile/src/main/res/xml/preferences.xml

+ android:dependency="default_openhab_presence_enable"
+ android:key="default_openhab_presence_debug"
+ android:summary="Enable debug notifications"
+ android:title="Debugging" />


Please use strings.xml ressources for this.

mueller-ma · 2017-08-30T10:53:07Z

mobile/src/main/res/xml/preferences.xml

+ android:summary=""
+ android:title="Latitude" />
+ <EditTextPreference
+ android:defaultValue=""


You could replace "" with "@string/empty_string"

Not entire clear what you meant, but I just removed the default.

digitaldan · 2017-08-30T15:18:37Z

Hi @jjhuff , thanks for the PR, and thanks @mueller-ma for the initial review. I need to find a large block of time to get through all the android PR's this week.

jjhuff · 2017-08-30T16:46:07Z

Thanks for the review @mueller-ma. I think I fixed everything you flagged.

mueller-ma · 2017-08-30T18:01:29Z

mobile/src/main/res/xml/preferences.xml

@@ -58,6 +58,38 @@
 android:summary="@string/settings_openhab_sslhost_summary"
 android:title="@string/settings_openhab_sslhost" />
 </PreferenceCategory>
+ <PreferenceCategory android:title="Presence">


Hardcoded string

mueller-ma · 2017-08-30T18:03:30Z

mobile/src/main/res/xml/preferences.xml

+ android:dependency="default_openhab_presence_enable"
+ android:inputType="numberDecimal"
+ android:key="default_openhab_presence_lng"
+ android:summary=""


Can you display the selected value as summary?

That's a good idea, done!

digitaldan · 2017-09-05T23:57:06Z

@jjhuff I just had a chance to run this. Couple of suggestions. My thoughts for this functionality would be that you select a location from a map (using google map api) and then select and item to switch on when you enter it, off when you exit it. I was not expecting the user to manually enter a single lat/lon or the actual item name. Would it be possible to let the user select a map for the location, and also fetch a list of items (/rest/items) from the users server to select? Even better, supporting multiple geofences -> items would be awesome.

jjhuff · 2017-09-06T16:37:19Z

@digitaldan I think both of those would be great, but I wanted to start somewhere

Selecting items: This might be a little thorny, but I need to dig into the preferences code some more. I'm not sure we'd always have server info available (especially when initially configuring the app).
As for the map: I think the best option is for the server to provide the location(s) in most (maybe all) cases. I implemented that by pulling location from i18nprovider, but I'd prefer a better option. Failing all of that, I agree 100% that selecting from a map is preferable to entering LatLng.

digitaldan · 2017-09-06T17:43:31Z

Selecting items: This might be a little thorny, but I need to dig into the preferences code some more. I'm not sure we'd always have server info available (especially when initially configuring the app).

What I would like to see is this not in the preferences menu, but in the main menu where we already have "write nfc tag", so something like "Geo Fences". when selecting this it pops up a window showing your saved geofences , or a button/link to create a new one.

For selecting an item, If you are not connected to a server then you would have an empty dropdown menu, but could still type into the field (if we think its an issue) , selecting from the dropdown would populate that item name in the field.

As for the map: I think the best option is for the server to provide the location(s) in most (maybe all) cases. I implemented that by pulling location from i18nprovider, but I'd prefer a better option. Failing all of that, I agree 100% that selecting from a map is preferable to entering LatLng.

So, I think your point of view here is that this is meant for geofencing your home, but that is only one use case. For example, I have a geofence when I drive into my neighborhood and another one for when I get home. I also have a geo fence for work. Tying this to a single location seems a bit limiting, and not what I would expect from a geo fence feature.

Btw, I love this feature! We want to bump our app version to 2.0 soon and this could be a great reason to do so.

jjhuff · 2017-09-06T19:43:58Z

What I would like to see is this not in the preferences menu, but in the main menu where we already have

Gotcha. Yup, that solves a lot of problems.

Tying this to a single location seems a bit limiting, and not what I would expect from a geo fence feature.

100% agree! However, my main point is that I think that the fences should be defined/stored on the server rather than every single client. Perhaps I have multiple devices, a family, or employees.

Proposal 1:
What if we made a service that does nothing but export a list of geofences similar to how the OwnTracks stuff is configured

Proposal 2:
Perhaps better, configure all of it in the item. Then the app just needs to select a list of geofence items to update!

Switch  PresenceBen_PhoneHome   "Ben @ Home"   { geofence="-47.456,122.366,100" }
Switch  PresenceBen_PhoneWork   "Ben @ Work"   { geofence="-47.134,122.655,100" }

I suppose you could also have a config file to assign symbolic names to replace the lat,lng,radius values.

digitaldan · 2017-09-29T17:50:28Z

Apologies for the very late reply, I am catching up on PR's today.

So i had to think about this, I still would prefer that a user selects a location and radius from a map, then selects an item. I think this would be amazingly simple and be used frequently.

I am however trying to also see it from your POV, and so keeping an open mind.

For this project, we always have to keep in mind that it's the official Android app for openHAB. This means we need to keep design focused on complimenting the openHAB server and its built in functionality. So I'm generally opposed to requiring users to have to install a new binding unless it would absolutely be necessary . I also believe we can not overload basic OH concepts like items or sitemaps to meet bespoke functionality.

That being said, we do have the concept of a Location item. If the user were to select a location item and another item to switch on or off I have a couple questions on how this would work.

how would we select and store the geofence radius?
what happens when the user changes this on the server?

100% agree! However, my main point is that I think that the fences should be defined/stored on the server rather than every single client. Perhaps I have multiple devices, a family, or employees.

My opinion is that this over complicates the feature, but again its' just my opinion.

Proposal 1:
What if we made a service that does nothing but export a list of geofences similar to how the OwnTracks stuff is configured

The android app should work without any special configuration on the server that is specific to the app, i would like to avoid the user having to install a binding unless there was no alternative.

Proposal 2:
Perhaps better, configure all of it in the item. Then the app just needs to select a list of geofence items to update!

I think we are overloading what an item is, so I don't think we can do that in this app.

I'm hoping you are still interested in developing this feature! I think it would be awesome.

lolodomo · 2017-10-08T17:55:43Z

Just an idea: why not having something similar to the voice recognition, that is the Android app updating a location item with the current phone position ? Then the user could use this position in rules and set switch for example when the phone position is around a particular position.
Of course, the item name has to be a setting in the Android app because we must consider and differenciate several phones on server side.

With which frequence the app will update the position ? With what impact on phone battery ?

digitaldan · 2017-10-08T20:26:40Z

With which frequence the app will update the position ? With what impact on phone battery ?

I think that's the issue, not only would our app now wake up in the background for location changes events, but it also tries to wake up the radio and make REST calls at some frequent interval. I was hoping not to have to deal with possible battery issues with our app.

jjhuff · 2017-11-25T01:41:58Z

I apologize for being MIA -- new job sucked away all of my free time!

How are people feeling about where this sits? I see an approval (I missed that earlier). I'm happy to clean up the merge conflicts if there's still overall interest in the feature.

I agree that a map would be a logical next step, but I'm also not super keen to draw this out.

Thoughts?

mueller-ma

Having the data stored on client isnt very practical, especially since you dont have a map to pick the location.

digitaldan · 2017-11-25T16:24:29Z

Having the data stored on client isnt very practical, especially since you dont have a map to pick the location.

Maybe I don't understand, apps that I use that do geo fencing pop up a google map for me to select a location and radius, so we can have client access to a map, yes?

Also on Android you have to register for entry/exits events to geo-fence locations, which implies the client does need to store the coordinates and some item to trigger locally. In fact, I believe on each reboot of the device, apps need to listen for a boot complete action (or something) and re-register its geofences.

jjhuff · 2017-11-25T16:39:17Z

FWIW, I'm actually a big fan of having the geofences come entirely from the server -- I'm just not sure of the best mechanism to make that happen.

mueller-ma · 2017-11-25T16:54:18Z

Maybe as state of a string item, but thats not the best solution

mueller-ma · 2017-11-26T09:58:22Z

I really like proposal 2 of @jjhuff, but that would need some work on the server, wouldn't it?

mueller-ma · 2017-11-26T09:59:01Z

Triggering travis

kubawolanin · 2018-01-28T16:10:36Z

Looking forward to test this!
Is there anything missing on this PR (apart frorm the conflicts) to have it merged to the master branch?
Best regards

mueller-ma · 2018-01-28T16:35:27Z

Currently you have to enter the location's coordinates. It would be a lot nicer to fetch them from the server.

maniac103 · 2018-01-28T17:32:05Z

mobile/src/main/java/org/openhab/habdroid/core/GeofenceBroadcastReceiver.java

+ } else {
+ ComponentName comp = new ComponentName(context.getPackageName(),
+ GeofenceIntentService.class.getName());
+ startWakefulService(context, (intent.setComponent(comp)));


The if and the else block are 100% equivalent.

lolodomo · 2018-02-10T10:02:54Z

I just proposed to implement a new geofence service in the server explaning how any UI could interact with it.
eclipse-archived/smarthome#5066
I am waiting for your comments.

lolodomo · 2018-02-10T15:09:34Z

My proposal has the advantage to have the geofences defined on server side. Using Paper UI, it will be easy to select the locations insde a map (coming feature in openHAB).
By the way, the Android app could then display all the geofences in a map too, to help the user select the one to use.
The other difference is that the computation whether device is inside geofence or not, is done by the server, the UI just has to call the server with its current position, the selected geofence and an item to update.
WDYT ?

maniac103 · 2018-02-10T18:27:58Z

I see that (computation being done server side) as a problem though. When doing it client side, one can leverage OS features (here: Play Services) when determining geofence status. When doing it server side, there's no other option than regularly polling location, which likely is less efficient.

lolodomo · 2018-02-10T19:02:58Z

No problem, we can have a service mode where the geofence status is determined by the client. In this case, the client only call the geofence service when it detects a change of status; the service will not compute the status again but consider the status provided in parameter.

lolodomo · 2018-02-10T19:14:38Z

In case the app only call the server when it enters or leaves a geo-fence, we have no chance to track the position of the person inside openHAB server. Maybe the service could be called by the Android app every XX minutes if the position has changed in order to set a location item. XX should be configurable.

maniac103 · 2018-02-10T19:33:08Z

A geofence is not the same thing as location tracking. The former only provides an in/out information, the latter provides the actual location. We could probably support both modes, but the latter rather sounds like an extension of Location items.

lolodomo · 2018-02-10T19:55:47Z

Location tracking could already be done by any client UI simply by updating a particular location item. That is true that no additional service is required for that.

I thought about location tracking because if we have a service that receives the position for geofence computation, it is easy to track position at the same time. But that would be just an option of the service.

lolodomo · 2018-02-10T19:58:50Z

At least the service could allow sharing geofences. It could even be used to save on server side new geofences defined by a client.

lolodomo · 2018-02-11T08:48:00Z

@kaikreuzer rejected my proposal explaning that there is no need to introduce a new geofence "concept" in Eclipse Smarthome because we already have Location item and distance between location items can be computed inside rules.
So please forget my proposal and let's continue with a full Android app solution for geofencing.

kaikreuzer · 2018-02-12T11:48:00Z

@kaikreuzer rejected my proposal

... sounds a bit hard, I would say I questioned the suggested way to implement it.

But I think implementing this in the client side only isn't too bad as the precision and size of the fence might anyhow depend on the OS.

mueller-ma · 2018-07-11T16:16:42Z

Superseded by #914

jjhuff added 7 commits August 21, 2017 14:54

Basic geofences working

e6eef77

Send REST commands

720c77b

Make presence debug notifications optional. Add error reporting

6020e19

Disable presence reporting by default

b0702c3

Register geofences on boot

45f5556

Ignore bad pref data

87fb0f1

Load location from server if possible

f53581e

mueller-ma requested changes Aug 30, 2017

View reviewed changes

mueller-ma reviewed Aug 30, 2017

View reviewed changes

Review comments

e643ed9

mueller-ma requested changes Aug 30, 2017

View reviewed changes

jjhuff added 2 commits August 30, 2017 11:35

Remove hard coded string

21d387f

Set item, lat, lng preference summary to value

fc9fe4d

mueller-ma approved these changes Aug 30, 2017

View reviewed changes

digitaldan added the awaiting feedback label Oct 8, 2017

mueller-ma requested changes Nov 25, 2017

View reviewed changes

mueller-ma closed this Nov 26, 2017

mueller-ma reopened this Nov 26, 2017

mueller-ma added the enhancement Indicates new feature requests label Dec 11, 2017

maniac103 reviewed Jan 28, 2018

View reviewed changes

mueller-ma force-pushed the master branch 2 times, most recently from d84840f to d3ee35c Compare March 30, 2018 10:24

kaikreuzer mentioned this pull request Jun 3, 2018

[WIP] Geofences with Google Location API #914

Closed

mueller-ma closed this Jul 11, 2018

Implement geofence-based presence #355

Implement geofence-based presence #355

Conversation

jjhuff commented Aug 29, 2017

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

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

mueller-ma Aug 30, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

digitaldan commented Aug 30, 2017

jjhuff commented Aug 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

digitaldan commented Sep 5, 2017

jjhuff commented Sep 6, 2017

digitaldan commented Sep 6, 2017

jjhuff commented Sep 6, 2017

digitaldan commented Sep 29, 2017

lolodomo commented Oct 8, 2017 • edited Loading

digitaldan commented Oct 8, 2017

jjhuff commented Nov 25, 2017

mueller-ma left a comment

Choose a reason for hiding this comment

digitaldan commented Nov 25, 2017

jjhuff commented Nov 25, 2017

mueller-ma commented Nov 25, 2017

mueller-ma commented Nov 26, 2017

mueller-ma commented Nov 26, 2017

kubawolanin commented Jan 28, 2018

mueller-ma commented Jan 28, 2018

Choose a reason for hiding this comment

lolodomo commented Feb 10, 2018 • edited Loading

lolodomo commented Feb 10, 2018

maniac103 commented Feb 10, 2018

lolodomo commented Feb 10, 2018

lolodomo commented Feb 10, 2018

maniac103 commented Feb 10, 2018

lolodomo commented Feb 10, 2018

lolodomo commented Feb 10, 2018 • edited Loading

lolodomo commented Feb 11, 2018

kaikreuzer commented Feb 12, 2018

mueller-ma commented Jul 11, 2018

mueller-ma Aug 30, 2017 •

edited

Loading

lolodomo commented Oct 8, 2017 •

edited

Loading

lolodomo commented Feb 10, 2018 •

edited

Loading

lolodomo commented Feb 10, 2018 •

edited

Loading