firebase / geofire-objc

GeoFire for Objective-C - Realtime location queries with Firebase
MIT License
435 stars 176 forks source link

queriesForRegion does not wrap regions #27

Open alvivi opened 8 years ago

alvivi commented 8 years ago

I created a simple project with an MKMapView, a GeoFire query and an MKMapViewDelegate, which updates GeoFire query region every time UIMapView region changes. But that crash with an unhandled Not a valid geo location exception, but only valid regions where used.

The problem is that queriesForRegion:region is not wrapping locations when computing locations from regions. As a workaround I have to limit zoom on to MKMapView.

jwngr commented 8 years ago

@jdimond - Can you take a look at this please?

austincarrig commented 8 years ago

I've also been experiencing this problem for quite a while. The solution I use is to only update Geofire when the region is valid. The problem I come across is that when I am zoomed out and scroll to the north or south pole, there is an exception thrown in GFGeoHash.m on line 38.

The call throwing this exception is in GFGeoHashQuery +queriesForRegion (line 178), when passing latitudeNorth (line 201) or latitudeSouth (line 205). It's always because these values are > 90 (north) or < -90 (south), which doesn't exist on a map. My problem is that when I call setRegion: on my Geofire query, the region comes directly from the mapView without any alterations, and Geofire should work for any region it's passed.

Hope this helps you track down the problem.

AJMiller commented 8 years ago

I arrived at the same conclusion as @austincarrig. I've written a gist which adds this region validation check as an extension to MKCoordinateRegion so you can just wrap your GeoFire queriesForRegion call in a if mapView.region.IsValid { } check. https://gist.github.com/AJMiller/0def0fd492a09ca22fee095c4526cf68

ahaverty commented 7 years ago

Is there any workaround for returning all keys by distance from the current location, for locations existing globally? I've hit the same issue as @zgosalvez in #64 , and also tried to use MKCoordinateRegion zoomRegion = MKCoordinateRegionForMapRect(MKMapRectWorld); spanning the whole world, but I'm getting the same geo location error @alvivi is hitting here. We're creating a simple list using the user current location, so I don't think the isValid extension would help us either?

It seems geofire-java supports a large radius? https://github.com/firebase/geofire-java/issues/46

AJMiller commented 7 years ago

@ahaverty so I know this probably isn't official, but why not just attach a .childAdded event to the /user_locations endpoint in the database that GeoFire is working in? You're just read-only I assume so you shouldn't affect GeoFire's data integrity at all. That will get you all of the locations, which you can then sort by distance using a CLLocationDistance. You could even attach a .childChanged event to monitor changes in the list ordering.

`import MapKit

let location1 = CLLocation(latitude: 10, longitude: 100) let location2 = CLLocation(latitude: 20, longitude: 120) let distance : CLLocationDistance = location1.distanceFromLocation(location2) print("distance = (distance) m")`

ahaverty commented 7 years ago

@AJMiller Thanks for the suggestion. I've found an alternative workaround that shows my users an 'no nearby locations' screen, but would prefer to handle this using geofire like I have with the android version of geofire, as I can't guarantee geofire will work consistently across the world.

Is this not an important bug, especially for anyone living near the north/south poles?

Ekhoo commented 7 years ago

I also have @austincarrig issue. Did someone find something ? If I use the @AJMiller extension, I lose some points on the map.

G-Steve-E commented 4 years ago

I opened similar issue (#129) as GeoFire fails with a big radius. I have also tried using a region Query but it doesn't return all markers for my map when I zoom out to show the whole world. I'm assuming the same issues exist for Firestore? This issue makes GeoFire unusable for my application. It's a shame as it would be a great solution if it worked.