Spec addition proposal: location preload

@Raghu_Mittal has generously offered to contribute a Collect feature that will read the device location in the background. Read more about the feature at Auto-gps implementation on ODK Collect. This is something he has built and used in a fork. In that fork, the code populates a node in the form with name auto_gps.

Since we try to avoid reserved node names and want to make sure forms can work across implementations, I think it's worth discussing possible alternatives.

I think the approach that would be most consistent with W3C XForms is to introduce a new action that given an event and a node would populate that node with a location when the event happens. For example, the event in this case would generally be form load. That would look something like:

<bind nodeset="/data/current_location" type="geopoint"/>
<orx:getlocation event="xforms-ready" ref="/data/current_location"/>

Conveniently, Dimagi has already had the same idea: https://dimagi.github.io/xform-spec/#actions. For some reason, they called their action orx:pollsensor so we'd have to weigh having a specific, descriptive name against bringing the two specifications into better alignment. Or perhaps we could document something like orx:getlocation and alias orx:pollsensor for compatibility.

A great thing about this strategy is that it would also allow us to request location readings on different events. For example:

<bind nodeset="/data/species" type="string" />
<bind nodeset="/data/species_location" type="geopoint" />
...
<input ref="/data/species">
    <orx:pollsensor event="xforms-value-changed" ref="/data/species_location" />
</input>

This would trigger a request for a location reading when the value of species is changed. The reading would be stored in species_location when acquired.

@martijnr, @Xiphware, @michal_dudzinski and I have been involved in a related conversation about capturing timestamps here (the conversation started before the forum existed!) and it would be nice for these two features to harmonize. What do you all think of this approach?

@Raghu_Mittal the client implementation would be the same and someone can definitely help you connect the pieces together.

9 Likes

@LN I think it's good approach as it will allow far more than just preloading a location at form load (in opposite to jr:preload thing).

But I don't like the way Dimagi has implemented this because there is almost no sign of orx:pollsensor in their engine codebase (https://github.com/dimagi/commcare-j2me/search?utf8=✓&q=pollsensor&type=). Only a single xml file which is just an example and doesn't seem to be used anywhere. They've put everything in their client implementation (https://github.com/dimagi/commcare-android/search?utf8=✓&q=PollSensor&type=).
So I'm not really sure if one can say Dimagi's xform engine follows their XForm specs :wink:

I realize this is a high-level discussion about the specification changes and we shouldn't tackle the implementation yet but I believe this problem should be highlighted now.

Below some high-level proposition:

Since the orx:getlocation (or orx.pollsensor) action must be implemented on the client side,
I think that ideally we should design some clean interfaces or "hooks" on the engine side which can be documented and implemented (or called) on the Collect (or other clients) side.
This approach may help us avoid confusion in the future and will allow to:

  • Unit test parsing of the orx:getlocation element
  • Unit test that engine properly asks for the location at expected events with expected context

Just my two cents :slight_smile:

Correct! Once we get agreement on the specification change each implementation can choose how to go about making it happen.

For now the questions are:

  • Are XForms actions the right approach for this?
  • What should the name of the new action be? getlocation, pollsensor, something else?

I don't see any reason why it they wouldn't be. It's explicit, generic and follows the xform way for doing things.

I honestly don't get why Dimagi chose pollsensor. Seems like something generic but their spec clearly says it's used to set geopoint value of the current location. I opt for the explicit name.

1 Like

I asked Dimagi about their naming choice here and it sounds like they didn't have a strong reason for picking pollsensor.

@LN setlocation is the best name for this I've seen so far :smiley: It's a good point though to maintain compatibility but it seems like Dimagi's spec doesn't support action nested inside the input element, does it?

I mean the following:

<input ref="/data/species">
     <orx:pollsensor event="xforms-value-changed" ref="/data/species_location" />
</input>

If so, then potential compatibility would be only partial, right? Forms following Dimagi's spec would work in with ODK XForm implementations but forms following ODK XForm spec using nested actions wouldn't work with the Dimagi implementation. In such situation I think adding an alias as you suggested may be the best approach.

You're right. So maybe we shouldn't worry too much about compatibility there since we're going for a more complete implementation anyway.

I had another thought. Is introducing a new action really the right way to do this or would it make more sense to introduce a new function? That would be something like here() by analogy to now().

We could then use the existing setvalue action:

<bind nodeset="/data/current_location" type="geopoint" />
<setvalue event="xforms-ready" ref="/data/current_location" value="here()" />

All of the advantages described above are the same and as a bonus we don't have to introduce a custom action.

2 Likes

Very interesting and creative! That actually may make more sense than introducing a custom action which would be doing basically same thing as setvalue but limited to geopoint values. And could be used with conditional logic, mixed with other functions etc.

2 Likes

Sorry, for my not responding to this!

I agree this feature would be a welcome addition. I think the custom XForms action <orx:setlocation> is the way to go instead of an XPath function, because obtaining a location can be assumed to an asynchronous operation, which would be very problematic in an XPath evaluator (it would have to block until a response is received or timeout has passed).

We should also think about the value to return when it is not possible to obtain the location. This should be assumed to be quite likely to happen, due to technical issues, lack of user permission (in a browser, OS), or context (http instead of https in a browser). I haven't given this much thought yet, but "NaN NaN NaN NaN" (yeah, that's ugly) comes to mind following the error response of a Number Function in XPath.

1 Like

Agreed, the here() concept was a bit of wishful thinking on my part! I could go either way on getlocation vs. setlocation. My argument for getlocation would be that this is also useful without a ref to trigger GPS in anticipation of a foreground reading (warming, basically).

That sounds good to me; I can't think of a better option.

Actually. Why not leave the field blank when a location can't be captured? This is what currently happens with geopoint.

Yes, maybe it's not necessary to make a distinction between not-attempted and failed. Hmm.

I can create a PR for documenting <odk:setlocation> with empty failure response, unless there are further comments.

1 Like

This has been revived because a Collect implementation is now almost ready. :tada: Thank you, @martijnr, for merging in the ODK XForms spec at https://github.com/opendatakit/xforms-spec/pull/198.

There is a proposal for bringing this to XLSForm here.

I reviewed the @TAB call notes and saw that there was talk of changing the action name to setgeopoint for consistency with the geopoint type. That would be fine by me. I do like that it might leave space for introducing an alternate geo type with a different format as has previously been discussed.

I didn't listen to the call -- was it a done decision or more of an idea for another round of discussion?

I believe we were only referring to XLSForm syntax (at least I was). I think the XForms syntax is settled.

Oh, I think I misunderstood the TSC discussion. Sorry, I merged the XForms spec addition yesterday. Shall I revert that?

Sounds like it may have been confusing all around. I'm really sorry I can't attend!

What do you think about changing the action name, @martijnr? I don't think anyone feels super strongly one way or the other. The most compelling argument for me is that we may eventually change the canonical "location" format and so setgeopoint keeps things more consistent in that case.

If you're on board, I think you can just issue another spec PR to change the name.

1 Like

I'm on board! Will create PR now.

1 Like

Thanks, @martijnr!

That means that for getting background location on first form load, we've settled on:

<bind nodeset="/data/current_location" type="geopoint"/>
<odk:setgeopoint event="odk-instance-first-load" ref="/data/current_location"/>

Spec documentation is now at https://opendatakit.github.io/xforms-spec/#action:setgeopoint.

1 Like

Presumably javaRosa/Validate will check the target element binding is a geopoint, and throw an error if not? Or are we just going to dump the geopoint string into whatever the ref happens to point to...?