Add support for setgeopoint

[MartijnR]

after or during #577

Documentation: https://github.com/opendatakit/xforms-spec/pull/198

[eyelidlessness]

I started on this over the last couple days and have some notes on what I've found so far. Some of this may be obvious/not noteworthy but I'm new to Enketo so I'm still taking a lot in.

• Affected projects: odk:setgeopoint is essentially a subset of setvalue where value will populated using the Geolocation API rather than user input. So I looked through the codebase to understand how setvalue is implemented. It turns out that it depends on transformations in enketo-transformer. I confirmed with @MartijnR that there will need to be similar work there to support the implementation in enketo-core.
• Transformations: there are several setvalue edge cases handled in enketo-transformer, some of which may also apply to odk:setgeopoint, I'm still walking through the relevant test code to determine where they overlap.
• Testing:
  • I have an open question about whether there's a preference for parameterizing or repeating test cases where the expected behavior between setvalue and odk:setgeopoint is identical or nearly so. My hunch is that there will be quite a lot of overlap in enketo-transformer which may benefit from parameterization, which may reduce maintenance burden. And in enketo-core my hunch is that they're more likely to diverge.
  • Regardless of preference on that, to the extent possible, I'm planning to use existing setvalue tests and fixtures as a starting point for testing odk:setgeopoint. This should help me familiarize myself with the various edge cases and expectations that a solid implementation should consider.
• Behavior implementation: Once transformation is handled, the behavior implementation in enketo-core appears to largely be handled in src/js/calculate.js. Until transformation is complete, this is pure speculation, but I strongly suspect the work to support odk:setgeopoint will be able to perform the relevant Geolocation API lookup—navigator.geolocation.getCurrentPosition(), if I'm not mistaken—and defer to the existing setValue method from there.
• UX note: there are some UX risks working with the Geolocation API. As you'd likely guess, most if not all browsers and OSes don't permit its usage without explicit user consent. This is actually a bigger barrier to usage than you might think. For example, under many circumstances a browser will silently fail to ask permission on Apple OSes unless the user manually gives consent to the browser first. I also discovered that Chrome (and likely other Chromium-based browsers) will repeatedly revert that OS-level consent if a software update is pending.

I'm sure I'll have more to add and may post questions as I go, but that's my understanding and thinking thus far.

[lognaturel]

there are some UX risks working with the Geolocation API

Understatement! To add to your cases, I ignored requests for location permissions on a test Enketo form from one data server. Now it looks like Chrome doesn't prompt me for that permission for any Enketo form from that data server anymore.

Feels like this is a real can of worms so I'm wondering whether it might make sense to get everything working for the "all permissions have been granted" case first and then to see what options there are for the rest.

Side note -- I'm seeing a console log of [Violation] Only request geolocation information in response to a user gesture. from a form that requests location in the foreground. Seems browsers might start enforcing this sooner than later. Maybe at that point we can do something like on form open, show an overlay that requires user action to get location and open the form or something like that. That might also be a way to handle the cases where permissions haven't been granted -- then we just don't allow the form to be open. I believe that's what we've ended up doing on Collect.

[eyelidlessness]

Understatement!

Isn't it always with the web? 🙃

To add to your cases, I ignored requests for location permissions on a test Enketo form from one data server. Now it looks like Chrome doesn't prompt me for that permission for any Enketo form from that data server anymore.

I did a little quick spike to see what kind of solutions might be available. In the desktop Chrome case it looks like this can be reverted by clicking the lock in the location bar, which opens a dialog allowing the user to override the original choice:

Obviously this is manual and the user would need instructions, presumably a matrix of such for supported browsers/OSes, but this may be necessary. I also found the Permissions API, and navigator.permissions.query( { name: 'geolocation' } ) may help us at least handle the case proactively.

Feels like this is a real can of worms so I'm wondering whether it might make sense to get everything working for the "all permissions have been granted" case first and then to see what options there are for the rest.

This is my instinct as well. Normally I wouldn't defer handling such a common negative UX and focus only on the happy path, but I also don't want to hold this up for something that could be such a heavy lift.

Side note -- I'm seeing a console log of [Violation] Only request geolocation information in response to a user gesture. from a form that requests location in the foreground. Seems browsers might start enforcing this sooner than later. Maybe at that point we can do something like on form open, show an overlay that requires user action to get location and open the form or something like that.

Interesting! I was assuming this would behave similarly to a lot of other APIs which are allowed either immediately (synchronously) on page load but otherwise require user interaction (for instance video autoplay), and I see this on sync calls as well.

I think if user interaction is going to be required eventually anyway, it may be better UX to not support the odk-instance-first-load event, and prefer a required attribute combined with xforms-value-changed instead?

[lognaturel]

Normally I wouldn't defer handling such a common negative UX and focus only on the happy path

One big challenge/opportunity in this ecosystem is that there are wildly different usage patterns. I imagine that getting background location is most desirable in an enumerator-mediated context where there are dedicated data collectors and the project designers want to get location for each record without distracting those data collectors or to validate that the data collectors are doing the right thing. I wouldn't imagine it being useful in a self-report context where a form is blasted to a bunch of people. In an enumerator-mediated context, devices are more likely to be set up proactively by an IT team or are managed by a data collection staff that can be trained and is incentivized to do the right thing (they can be told they don't get paid if their location is off, e.g.). I'm interested to hear what @MartijnR thinks but it would seem ok to me for the feature to only be guaranteed to work as expected when certain conditions are met (and this would need to be documented).

it may be better UX to not support the odk-instance-first-load event

This is really what people want so I think it's worth trying. I'm only speculating about what browsers might do, I have no idea. The warning might also be related to the way location is accessed currently and maybe you're right that there's a way to do it on load without the warning.

[eyelidlessness]

One big challenge/opportunity in this ecosystem is that there are wildly different usage patterns. I imagine that getting background location is most desirable in an enumerator-mediated context where there are dedicated data collectors and the project designers want to get location for each record without distracting those data collectors or to validate that the data collectors are doing the right thing. I wouldn't imagine it being useful in a self-report context where a form is blasted to a bunch of people. In an enumerator-mediated context, devices are more likely to be set up proactively by an IT team or are managed by a data collection staff that can be trained and is incentivized to do the right thing (they can be told they don't get paid if their location is off, e.g.). I'm interested to hear what @MartijnR thinks but it would seem ok to me for the feature to only be guaranteed to work as expected when certain conditions are met (and this would need to be documented).

This is really helpful context, thank you! Also looking forward to what Martijn thinks.

This is really what people want so I think it's worth trying. I'm only speculating about what browsers might do, I have no idea. The warning might also be related to the way location is accessed currently and maybe you're right that there's a way to do it on load without the warning.

This also helps. It's also helped me realize that I started working on this in earnest, but in so doing I skipped some of my normal working process. I went into "task" mode without really gathering more information about user intent. Is there somewhere that actual user stories are tracked, or a similar mapping between a given issue and how it corresponds to user feedback?

[eyelidlessness]

I wanted to check in on my progress before the weekend, which isn’t much to show and probably isn’t even worth pushing up (though I can next time I’m at my desk if that helps). But it does feel like I’ve made some progress getting familiar with the transformer/core projects.

My approach (in enketo-transformer) so far has been:

1. Copy setvalue.xml and modify it to fit the more limited use case for setgeopoint. In this process I have renamed several of the referenced nodes, mostly for my own benefit to understand what they’re expected to test or set up for testing. Should any of this make its way into a PR, I’ll understand if it doesn’t fit with the project preferences but it was easier for me to keep track of explicit names than references to a, b, etc.
2. Copy the setvalue tests and adapt them accordingly. I think I no longer have an open question about parameterizing tests. Even at the transform level, the restriction against providing a value makes many of the tests significantly different.
3. Make relevant changes to the XSL and JS transforms to pass those tests. The good news is... I got one to pass! I’m successfully transforming the hidden input case. I spent some time applying what I learned from that to the other test cases, but I ran out of steam and will have to revisit next week.

---

I’ll also probably want to reconsider some of the XSL approach I discussed with @MartijnR. While there are some cases the overlap with setvalue seem to warrant sharing XSL declarations, doing so generally made many of them harder to read or follow.

This is of course coming from a perspective where I’m using all of the relevant standards for the first time. It’s possible there are ways to abstract these similar cases with XSL semantics which I have yet to encounter without harming readability and maintenance. But absent some direction on that, my instinct is to share the attributes transform and copypasta the much smaller bits, adding comments to indicate where similar changes should be applied. I’m not sure that’s the right instinct! Just the one that feels more likely to be productive approaching several new-to-me APIs.

---

I don’t think I have bandwidth for new material this weekend, but for when I do: I’d welcome any reading material that might help XForms feel less like a huge sprawling standard which looks like it shares semantics with HTML but really doesn’t. W3C has an exceptional documentation structure if you know what you’re looking for. But it can be impenetrable if you don’t.

[lognaturel]

Some background:

• I would generally not advise going to the W3C XForms spec unless you're looking for something very specific. ODK XForms is heavily subsetted. And as we discussed, most users use XLSForm to build their forms which exposes an even narrower subset. For example, only setgeopoint triggered by odk-instance-first-load is exposed.
• Collect documentation - it was good for me to read through and refresh my memory as well because we have a couple of background GPS features. For this one, we don't do any blocking of the form when location is unavailable, the value is just blank.
• Forum features thread - typically forum features threads have a lot of background on how the feature will be used. This one is kind of meandering because it was started by someone who already had an implementation (but did not end up contributing it). If you look at some of the linked threads you'll get more context on usage.

[MartijnR]

I'm interested to hear what @MartijnR thinks but it would seem ok to me for the feature to only be guaranteed to work as expected when certain conditions are met (and this would need to be documented).

Yes, that's fine, I think. The geo-widgets have two features that rely on this permission as well:

1. Centering map on current location
2. Record current location with crosshairs button

Actually, I just now got confused when trying to verify these features on https://enke.to/widgets and thought they were both broken until I realized I had to go into the Mac OS security and privacy settings to enable location services on Chrome (definitely not easy for average user to do, I think). For these 2 and the new setgeopoint we'll need some user permission request feature, but that can wait.

[eyelidlessness]

I opened https://github.com/enketo/enketo-transformer/pull/101 which I believe should handle the transform portion of this. I'm currently depending on it as a local file: but I'd assume it will need to be reviewed and released before the enketo-core work can be fully resolved.

---

Now that I'm working on that, I have a question about the ODK spec. [How] does the geopoint type account for partial values?

Space-separated list of valid latitude (decimal degrees), longitude (decimal degrees), altitude (decimal meters) and accuracy (decimal meters)

In my local environment, the altitude value returns null. I don't see any mention of null/undefined values in ODK XForms. Because the data type is specified as a space separated list, I worry that simply omitting a middle value might be ambiguous and cause mistakes.

[lognaturel]

In my local environment, the altitude value returns null.

Fascinating! What we do with manually-provided points which don't have an accuracy is set the accuracy to 0.0. I think it makes sense to do the same for any other missing values. While it's theoretically possible to get real values of 0.0, it's vanishingly unlikely so it's a pretty good indicator of a missing value. ~We could do a negative (impossible) altitude but I think that would have more downstream side effects.~ (What am I talking about, negative altitude is possible so it's not helpful. We should be consistent and use 0.0)

[eyelidlessness]

I asked @lognaturel on Slack about possibly using -0 as a sentinel here, as it's a valid number but shouldn't occur in a real geolocation lookup. But since there's already a convention to use 0.0 for missing accuracy values, it sounds like that may be a better solution for now.

[MartijnR]

negative altitude is possible

Was about to defend my place of birth at -2m altitude ;)

[eyelidlessness]

I think I'm getting very close on this, but I'm running into an issue that I don't really understand. I have this form (largely adapted from setvalue.xml):

<?xml version="1.0"?>
<h:html xmlns="http://www.w3.org/2002/xforms"
    xmlns:ev="http://www.w3.org/2001/xml-events"
    xmlns:h="http://www.w3.org/1999/xhtml"
    xmlns:jr="http://openrosa.org/javarosa"
    xmlns:odk="http://www.opendatakit.org/xforms"
    xmlns:orx="http://openrosa.org/xforms"
    xmlns:xsd="http://www.w3.org/2001/XMLSchema">
    <h:head>
        <h:title>setgeopoint</h:title>
        <model>
            <instance>
                <data id="setgeopoint">
                    <hidden_first_load/>

                    <visible_first_load/>

                    <repeats>
                        <first_load/>
                        <changes/>
                        <changed_location/>
                    </repeats>

                    <changes/>
                    <changed_location/>

                    <meta>
                        <instanceID/>
                    </meta>
                </data>
            </instance>

            <bind nodeset="/data/hidden_first_load" type="geopoint"/>
            <bind nodeset="/data/visible_first_load" type="geopoint"/>

            <bind nodeset="/data/changes" type="int"/>
            <bind nodeset="/data/changed_location" type="string"/>

            <bind nodeset="/data/repeats/first_load" type="geopoint"/>

            <bind nodeset="/data/repeats/changes" type="string"/>
            <bind nodeset="/data/repeats/changed_location" type="string"/>

            <odk:setgeopoint event="odk-instance-first-load" ref="/data/hidden_first_load"/>
        </model>
    </h:head>
    <h:body>
        <odk:setgeopoint event="odk-instance-first-load" ref="/data/visible_first_load"/>
        <input ref="/data/visible_first_load">
            <label>odk-instance-first-load</label>
        </input>

        <select1 ref="/data/changes" appearance="minimal">
            <label>xforms-value-changed</label>
            <odk:setgeopoint event="xforms-value-changed" ref="/data/changed_location"/>
            <item>
                <label>10</label>
                <value>10</value>
            </item>
            <item>
                <label>11</label>
                <value>11</value>
            </item>
        </select1>
        <input ref="/data/changed_location"/>

        <repeat nodeset="/data/repeats">
            <odk:setgeopoint event="odk-new-repeat odk-instance-first-load" ref="/data/repeats/first_load"/>
            <input ref="/data/repeats/first_load">
                <label>odk-instance-first-load</label>
            </input>

            <trigger ref="/data/repeats/changes">
                <label>xforms-value-changed</label>

                <odk:setgeopoint event="xforms-value-changed" ref="/data/repeats/changed_location"/>
            </trigger>
            <input ref="/data/repeats/changed_location"/>
        </repeat>
    </h:body>
</h:html>

Each case works except the first instance of <odk:setgeopoint event="odk-new-repeat odk-instance-first-load" ref="/data/repeats/first_load"/> (but adding new repeat rows works). In the first instance, this appears to be because its control isn't rendered in the DOM, it's in a structure like div.or-repeat > .question > input[data-setgeopoint].

What's odd is that <setvalue event="odk-new-repeat odk-instance-first-load" ref="/data/repeats/first_load"/> does not reproduce the problem (failure to set the first repeat's input on load), but does reproduce what seems to be the cause (its control is similarly not rendered in the DOM).

I have to run for the evening, but I'll push my branch up (mess of debugging that it currently is) tomorrow if that helps.

[eyelidlessness]

Coming back to this with a fresh brain, I suspect this is what's happening: setvalue, and now odk:setgeopoint, is operating on a control in the repeat's template DOM fragment, and the difference in behavior is caused by the fact that odk:setgeopoint is asynchronous. So the setvalue assignment blocks cloning the template, and the value is already assigned before the clone is rendered, but the odk:setgeopoint assignment occurs after the clone and essentially goes into the memory hole.

I'm a little stumped how to proceed, and it's not clear to me whether it's intended to operate on the template or just happens to work for setvalue by lucky accident.

[lognaturel]

Feels to me like supporting setgeopoint in repeats could be a separate issue handled at some later time. There's no XLSForm support for it yet so it's not likely to be used (it's very powerful, though, and I do hope we can expose it sooner than later). I can see how it would add a whole set of other concerns.

[eyelidlessness]

I’m open to that if the repeat isn’t an expected part of the scope (which goes back to my earlier question about user stories).

But it does seem like the underlying issue may cause future problems if other data types need to be set asynchronously. I think it would be good to clarify whether the current behavior (controls selected for odk-instance-first-load events within a repeat are in the template rather than the first instance) is expected. If so it would be helpful to document it, if not I could file an issue to resolve that separately.

Debugging this took more time than I’d have liked, and knowing my brain I wouldn’t be surprised if I get tripped up on it again in the future (or if other new contributors would too).

[eyelidlessness]

I was hoping to be able to open a PR today, but ran out of steam. But wanted to update what Hélène and I discussed in a check in meeting:

• Since the primary use case, as I understand it, is odk-instance-first-load, tomorrow I'll clean up my WIP to open a PR specifically covering that.
• Since I have also solved xforms-value-changed, I will open a second PR adding that implementation to keep the PRs small.
• Both of those will be dependent on https://github.com/enketo/enketo-transformer/pull/101, so they'll temporarily reference the enketo-transformer dependency as a file URL to validate the implementation, but will be updated to reference the new release version incorporating those changes when available. Edit: actually I may see if I can switch it to a git URL so that it has a chance of passing CI.
• I'll also review https://github.com/enketo/enketo-transformer/pull/102, which Hélène said would likely go into the same release.

[eyelidlessness]

I've opened two PRs, #778 and #779. The work to support repeat is in progress but partially blocked by the async issues discussed earlier. I'm adding this checklist so that those don't get lost in case they're needed in the future.

• [x] Support odk-instance-first-load in model #778
• [x] Support odk-instance-first-load in h:body #778
• [x] Support xforms-value-changed #779
• [ ] Support odk-instance-first-load in repeat
• [ ] Support odk-new-repeat