Updated my branch with that change now. On 3/29/12 11:31 AM, "Filip Maj" <[email protected]> wrote:
>Good catch Becky, thanks. I'll patch that shortly. > >On 3/29/12 11:02 AM, "Becky Gibson" <[email protected]> wrote: > >>I think there is an issue in the LocationManager: didUpdateToLocation: >>fromLocation method. The NSFastEnumeration protocol for NSDictionary >>iterates over the keys and based how watchCallbacks is created I think >>you >>want the object. >> >>Thus, I think this: >> >>for (NSString *callbackId in self.locationData.watchCallbacks) { >> [self returnLocationInfo:callbackId]; >> } >> >>should be: >> >>for (NSString *timerId in self.locationData.watchCallbacks) { >> [self returnLocationInfo:[self.locationData.watchCallbacks >>objectForKey: timerId ]]; >> } >> >>-becky >> >>On Wed, Mar 28, 2012 at 9:37 PM, Shazron <[email protected]> wrote: >> >>> Never mind - saw that NetworkStatus commit was already in the mainline >>>as >>> well >>> >>> 2012/3/28 Shazron <[email protected]>: >>> > Oh - when you merge it in, don't squash your commits in this case >>> > unless you list all the changes in the squash. Good to know about the >>> > "Network Status" change since I needed to know about this in the >>> > updated guides I'm writing. >>> > >>> > 2012/3/28 Filip Maj <[email protected]>: >>> >> Thanks for looking it over Shaz, much appreciated - the link helps >>>too. >>> I >>> >> am slowly getting over my aversion of invoking functions using >>>square >>> >> brackets and accepting objective-c for what it is. >>> >> >>> >> On 3/28/12 4:26 PM, "Shazron" <[email protected]> wrote: >>> >> >>> >>>Looks good Fil - good to get the iOS only cruft out of there. >>> >>> >>> >>>Minor point but there's a way to hide the private vars in the >>> >>>implementation file since users won't need to really see them in the >>> >>>public headers, through class extensions: >>> >>> >>> >>>http://stackoverflow.com/questions/5826345/how-to-declare-instance-varia >>>b >>>l >>> >>>es-and-methods-not-visible-or-usable-outside-of-t >>> >>>Not that prevents access, just visibility, and we are open source >>>after >>> >>>all... >>> >>> >>> >>>2012/3/28 Filip Maj <[email protected]>: >>> >>>> I've sent a pull request to both apache/cordova-js and >>> >>>>apache/cordova-ios >>> >>>> for the geo stuff (side note: github pull requests might be broken >>> again >>> >>>> :/) >>> >>>> >>> >>>> https://github.com/apache/incubator-cordova-ios/pull/8 >>> >>>> >>> >>>> >>> >>>> The above gets the native iOS geolocation plugin lined up to the >>>new >>> >>>> interface imposed by cordova-js. >>> >>>> >>> >>>> The JS file is bundled in the branch so you don't need to rebuild >>> >>>> cordova-js to see these improvements. >>> >>>> >>> >>>> Shaz and anyone else keen on the iOS platform: can you guys take a >>> look? >>> >>>> My tests on my end look good but would love another set of eyes. >>> >>>> >>> >>>> On 3/27/12 5:00 PM, "Filip Maj" <[email protected]> wrote: >>> >>>> >>> >>>>>So we'll keep it around (even though we won't override the >>> >>>>>implementation >>> >>>>>that we get for free in the WebView), and thus we'll need to >>>update >>> the >>> >>>>>native implementation. Gotcha. I will update the JIRA issue as >>>such >>> >>>>>then. >>> >>>>> >>> >>>>>On 3/27/12 4:57 PM, "Joe Bowser" <[email protected]> wrote: >>> >>>>> >>> >>>>>>We should axe it/move it to its own plugins repo. Someone may >>>want >>> it, >>> >>>>>>but >>> >>>>>>I don't want to make promises for it. >>> >>>>>> >>> >>>>>>On Tue, Mar 27, 2012 at 4:55 PM, Filip Maj <[email protected]> wrote: >>> >>>>>> >>> >>>>>>> I am going to assume then that will be merged in and we'll be >>> making >>> >>>>>>>the >>> >>>>>>> necessary native tweaks across the platforms we want to >>>support. >>> >>>>>>> >>> >>>>>>> ANDROID peeps: should we axe native geo code, or should we keep >>>it >>> >>>>>>>around >>> >>>>>>> and thus update the implementation to follow this new approach? >>> >>>>>>> >>> >>>>>>> On 3/27/12 3:33 PM, "Shazron" <[email protected]> wrote: >>> >>>>>>> >>> >>>>>>> >Apple should be following W3C, although I don't know if they >>>fixed >>> >>>>>>> >this bug yet, it's still unresolved for me in Radar: >>> >>>>>>> >http://openradar.appspot.com/radar?id=1160403 but based on >>>what >>> my >>> >>>>>>> >test on 5.1, they've fixed it. >>> >>>>>>> > >>> >>>>>>> >Sure ping me on email/jabber let's set up a time. >>> >>>>>>> > >>> >>>>>>> >On Tue, Mar 27, 2012 at 3:07 PM, Filip Maj <[email protected]> >>>wrote: >>> >>>>>>> >> Assuming that the native WebView implementations across >>>whatever >>> >>>>>>> >>platforms >>> >>>>>>> >> adhere to the W3C Geo spec, then these native changes would >>>line >>> >>>>>>>up >>> >>>>>>>our >>> >>>>>>> >> implementation with what users are expecting in their >>>browser. >>> >>>>>>> >> >>> >>>>>>> >> I can help with tweaking the implementation on iOS, but >>>would >>> love >>> >>>>>>>if >>> >>>>>>> >>you >>> >>>>>>> >> could once-over it, Shaz, and perhaps jump on a quick remote >>> hack >>> >>>>>>>sesh >>> >>>>>>> >> with me for 15-20 mins to make sure we are looking good. >>> >>>>>>> >> >>> >>>>>>> >> On 3/27/12 2:46 PM, "Shazron" <[email protected]> wrote: >>> >>>>>>> >> >>> >>>>>>> >>>Thanks Fil - I'm all for fixing geolocation in iOS. There's >>> >>>>>>>several >>> >>>>>>> >>>jira issues for it, and I've been attempting to fix it as >>>best I >>> >>>>>>>can, >>> >>>>>>> >>>but users are still reporting problems with it since it >>>doesn't >>> >>>>>>>match >>> >>>>>>> >>>the native implementation of UIWebView. >>> >>>>>>> >>> >>> >>>>>>> >>>On Mon, Mar 26, 2012 at 4:00 PM, Filip Maj <[email protected]> >>> wrote: >>> >>>>>>> >>>> Hey all, >>> >>>>>>> >>>> >>> >>>>>>> >>>> The past week or so I've been working on revamping the >>> >>>>>>>geolocation >>> >>>>>>> >>>>tests according to what is laid out by the W3C API [1]. >>>Been >>> >>>>>>>tracking >>> >>>>>>> >>>>progress and whatnot in a JIRA issue [2]. >>> >>>>>>> >>>> >>> >>>>>>> >>>> Good news: I've got the tests implemented plus cordova-js >>> >>>>>>>passing >>> >>>>>>>said >>> >>>>>>> >>>>tests (compare view to see diff available @ [3]). >>> >>>>>>> >>>> >>> >>>>>>> >>>> Bad news: we've been doing it wrong in our native >>> >>>>>>>implementations >>> >>>>>>>forŠ >>> >>>>>>> >>>>well, ever, it seems. >>> >>>>>>> >>>> >>> >>>>>>> >>>> Moving forward would like to hear suggestions from >>>everyone. >>> >>>>>>> >>>> >>> >>>>>>> >>>> Breaking down what we didn't do in the past that the spec >>> >>>>>>>mandates: >>> >>>>>>> >>>> >>> >>>>>>> >>>> * Properly implementing a timeout. It is one of the >>> available >>> >>>>>>> >>>>options that you can pass into getCurrentPosition / >>> >>>>>>>watchPosition. >>> >>>>>>> >>>>However, we've been using it to date as essentially a >>> "frequency" >>> >>>>>>> >>>>parameter for watchPosition, I.e. "give me position updates >>> every >>> >>>>>>> >>>><options.timeout> milliseconds". This is wrong. According >>>to >>> the >>> >>>>>>>spec, >>> >>>>>>> >>>>the timeout option defines how long after invoking a >>> >>>>>>>watch/getCurrent >>> >>>>>>> >>>>the error callback should wait before it fires with a >>>TIMEOUT >>> >>>>>>> >>>>PositionError object. >>> >>>>>>> >>>> * There is no control over how often watchPosition >>>should >>> >>>>>>>fire >>> >>>>>>> >>>>success callbacks. Instead, the spec says: "In step 5.2.2 >>>of >>> the >>> >>>>>>>watch >>> >>>>>>> >>>>process, the successCallback is only invoked when a new >>> position >>> >>>>>>>is >>> >>>>>>> >>>>obtained and this position differs signifficantly from the >>> >>>>>>>previously >>> >>>>>>> >>>>reported position. The definition of what consitutes a >>> >>>>>>>significant >>> >>>>>>> >>>>difference is left to the implementation." >>> >>>>>>> >>>> * I've also added tests + control of comparing the >>> >>>>>>>"maximumAge" >>> >>>>>>> >>>>parameter on the JS side, and keeping a reference to the >>>last >>> >>>>>>> >>>>successful >>> >>>>>>> >>>>position retrieved from the native framework and comparing >>>its >>> >>>>>>> >>>>timestamp >>> >>>>>>> >>>>together with maximumAge. This should implement proper >>>caching >>> of >>> >>>>>>> >>>>positioning on the WebView side and hopefully save some >>>native >>> >>>>>>>round >>> >>>>>>> >>>>trips. >>> >>>>>>> >>>> >>> >>>>>>> >>>> All of this means the the API on the native side for >>> geolocation >>> >>>>>>>will >>> >>>>>>> >>>>change (sorry iOS!). Basically we have three actions that >>>the >>> >>>>>>> >>>>Geolocation plugin should listen for: >>> >>>>>>> >>>> >>> >>>>>>> >>>> * getLocation, which takes as parameters >>>enableHighAccuracy >>> >>>>>>> >>>>(boolean) and maximumAge (int as milliseconds). >>> >>>>>>> >>>> * addWatch, parameter: only the usual callbackID >>>required. >>> >>>>>>> >>>> * clearWatch, parameter: only the usual callbackID >>> required. >>> >>>>>>> >>>> >>> >>>>>>> >>>> getLocation should require very little changing (other >>>than >>> not >>> >>>>>>> >>>>needing >>> >>>>>>> >>>>the timeout parameter anymore, since that is handled on the >>>JS >>> >>>>>>>side >>> >>>>>>>in >>> >>>>>>> >>>>my patch). >>> >>>>>>> >>>> >>> >>>>>>> >>>> addWatch should keep a list of callback Ids, and, as soon >>>as >>> we >>> >>>>>>>have >>> >>>>>>> >>>>one watch started, the native framework should start >>>watching >>> the >>> >>>>>>> >>>>position for a "significant position difference". Once that >>> >>>>>>>happens, it >>> >>>>>>> >>>>should fire the success callback(s) for all stored watch >>> callback >>> >>>>>>>Ids. >>> >>>>>>> >>>>If there is an issue retrieving position, it should fire >>>the >>> >>>>>>>error >>> >>>>>>> >>>>callback(s) for all stored watch callback Ids. >>> >>>>>>> >>>> >>> >>>>>>> >>>> I commented out a bunch of iOS-specific code that already >>>did >>> a >>> >>>>>>> >>>>"distance filter" type of approach to position acquisition, >>>but >>> >>>>>>>was >>> >>>>>>> >>>>only >>> >>>>>>> >>>>available if you provided undocumented parameters in the >>> options >>> >>>>>>> >>>>object. >>> >>>>>>> >>>>Not sure about how feasible a distance filter is in >>>Android, or >>> >>>>>>>Windows >>> >>>>>>> >>>>Phone, or our other supported platforms. >>> >>>>>>> >>>> >>> >>>>>>> >>>> One final point of discussion worth bringing up about this >>> >>>>>>>issue. >>> >>>>>>> >>>>BlackBerry and Android use the default implementation of >>> >>>>>>>geolocation >>> >>>>>>> >>>>abilities in their respective WEbViews. Because of this I >>>would >>> >>>>>>>mandate >>> >>>>>>> >>>>removal of any Geolocation java code from the Android + >>> >>>>>>>BlackBerry >>> >>>>>>> >>>>implementations. Saves some bytes. Originally we had the >>> Android >>> >>>>>>>plugin >>> >>>>>>> >>>>classes in there for support for devices before 2.0. Since >>>we >>> are >>> >>>>>>>only >>> >>>>>>> >>>>supporting 2.0 and above, this is no longer needed. Are >>>there >>> any >>> >>>>>>> >>>>issues >>> >>>>>>> >>>>with this? >>> >>>>>>> >>>> >>> >>>>>>> >>>> Appreciate you guys looking this over. >>> >>>>>>> >>>> >>> >>>>>>> >>>> Cheers, >>> >>>>>>> >>>> Fil >>> >>>>>>> >>>> >>> >>>>>>> >>>> [1] >>> http://dev.w3.org/geo/api/spec-source.html#api_description >>> >>>>>>> >>>> [2] https://issues.apache.org/jira/browse/CB-359 >>> >>>>>>> >>>> [3] >>> >>>>>>> >>>> >>> >>>>>>> >>> >>>>>>> >>> https://github.com/filmaj/incubator-cordova-js/compare/master...geotes >>> >>>>>>>t >>> >>>>>>> >>>>s >>> >>>>>>> >> >>> >>>>>>> >>> >>>>>>> >>> >>>>> >>> >>>> >>> >> >>> >
