[QGIS-Developer] Generic use of QgsLocator and friends?

[Nyall Dawson]

Hi Richard,

On 9 October 2017 at 23:10, Richard Duivenvoorde <rdmailings at duif.net> wrote:
>
> (sorry for long email and lot of questions...
> tldr; I think qlocator is UBERCOOL!!!! and want to use if for everything)
> for a screenie, see:
> http://pix.toile-libre.org/upload/original/1507554555.png

Awesome... that's what we like to see!


> # QUESTION: should we create base (geocoder)classes for the
> GeocoderLocator and GeocoderFilter (in cpp or python):
> - I created a baseclass for my 2 geocoders, thinking it would be
> helpfull to have something like 'mapToResult', but after some initial
> tests I think it is maybe only usefull to define some scale presets
> (like in current version of this plugin)
> - these baseclasses could be python or cpp...

I'd say not yet. I think in future we'll need these, but at the moment
I don't believe we know enough about what we require from such a base
class. I'd rather we let things "settle" and then see what the common
functionality is across the various geocoder implementations.

> # QUESTION: IF a service does not have a suggest-service (like
> Nominatim) adding a space ' ' at the end to force a search
> - Nominatim is not supposed to be used as a suggest service (see [0]) in
> contrast to the PDOK geocoder), so I decided to do the actual
> GET-request as soon as the user added a space in the end to let us know
> a searchstring was ended. This is just an idea, if others have better
> idea's just let us know

I'm not sure how to handle this situation. Right now I can't think of
a nicer approach than what you've taken.


> # QUESTION: should we have a (cpp/python?) NetworkAccessManager(factory)
> somewhere?
> - I took Boundless networkaccessmanager.py to do the HTTP requests. The
> advantage of this class is that it mimics http2 lib interface, BUT
> actually uses the Qt/QgsNetworking classings...
> I think we (as in QGIS) should have such a class in core (be it cpp or
> python)

No objections here... but I'll let boundless take the lead here, if
they think it's suitable for inclusion in master/the stable API.

> # QUESTION: having different geocoder locators running:
> synchron/blocking or asynchron/non-blocking networking?
> Because of the fast suggest service of I used the blocking way, but I'm
> not sure what the preferred way is here. Also it is not fully clear to
> me what to do with running requests (if you type very fast....)

The locator should handle this situation fine. What happens is that:

1. as a search string is entered, that string is passed to every
filter's "fetchResults" method, each of which runs in a different
thread.
2. results are added to the list as soon as a filter reports each one
- it doesn't wait until the individual filter is finished, or any
other filters are finished.

So a slow filter will not block the results coming back from a fast filter.

However - when a new string is entered (or a few extra characters
added to the string), the locator won't fire up a new search until all
the filters have finished the last request. So an individual filter
will only EVER have a single request happening at a time. This does
mean that filters which are slow to return can potential delay new
searches from occurring. To avoid this, well behaved filters should
use the passed QgsFeedback and check for cancelation (or attach to
it's canceled signal) and abort a running request as soon as possible.

> # QUESTION: is a normal plugin the right thing to do?
> - in the implementation you actually have to do 2 things:
> 1) register and deregister the LocatorFilter
> 2) create and implement the actual QgsLocatorFilter
> It is easy to do this as a plugin too (as you can see). I'm ok with
> every country implementing its QLocator (do we need a 'term/word' for
> it?) and add it as plugin

Yep, I think that's the best approach. Then users can install whatever
search plugins they find useful for their own particular workflow.

> # QUESTION: should a Filter already have a NetworkAccessManager and an iface
> - if implemented as a plugin, you have the iface to actually get a
> handle to the mapcanvas etc etc. (you need it so check crs or let themap
> zoom/pan) I was wondering if a QgsLocator should not actually be able to
> get to the mapcanvas (directly, or via current QgsProject)
> Same with an actual NetworkAccessManager...

I think your approach is fine here (using iface to get the canvas).
Locator filters are purely a core class, so can't rely on gui widgets
like the canvas.

> # QUESTION: why only 3 chars for prefix? (better make this configurable?)
> - I think it would be best if a user can decide him/herself to which
> locators to use, and thus decide what prefixes to use for them. In my
> case I would do: p (for pdok), f (for features), o (for osm/nomanitim)
> and g (for google). Others are not so interesting to e...
> I think only having the possibility to use 3 character prefixes is not
> very usefull?

Ok - so the rationale here is that we DON'T want plugins to "steal"
all the best key combinations and prevent us from using these for
future core locator uses. E.g. if we allow a plugin to use the "o"
shortcut for OSM search, we'll never be able to take that back and use
"o" for "open file" (or something else). That's why I took the
somewhat harsh approach of restricting plugins to a minimum 3
character shortcut. I think in future (as we see greater use of
locator and have a better idea of how it's being used) we could
potentially grant widely used 3rd party plugins access to the
whitelist to avoid this restriction.

But for now, I'd rather play it safe and keep this restriction in place.

Of course, we could also potentially allow users to configure the
shortcuts (Qt Creator does this) so they can do whatever the want.

> #    would be cool to be able to have a object-type to scale mapping for
> all geocoders, so  you determine for every object type on which z-level
> (exact) you want to come... I tried to do that by doing:
>     ADDRESS = 750
>     STREET = 1500
>     ZIP = 3000
>     PLACE = 30000
>     CITY = 120000
>     ISLAND = 250000
>     COUNTRY = 4000000
> But not sure if this is the best way.

That's a good idea! We could use the feature bounding box for
non-point features, and then fall back to some predefined scales like
this for point results.

> # QUESTION: why short result texts with osm (missing the 'types' of the
> object)
> - the results (of osm) seem to be cut of rather short?

I don't understand... have you got an example?


> # QUESTION: should the BaseGeocoderLocator set a point or label? As Option?
> - Alesandro's geocoder plugin (for 2.x) put's a point/label in the map.
> I was wondering if this should maybe be an (optional) thing in a base
> class too? Maybe adding all properties that you receive from the geocoder

We could also flash the point/line/polygon match geometry... I've no
strong opinions here yet.


> # QUESTION: during loading/reloading of the plugin I had a LOT of crashes:
> ^[QGIS died on signal 11[New LWP 26919]
> [New LWP 26920]
> [New LWP 26921]
> etc
> Not sure if this is my install, or has something to do with this plugin,
> or the combi of network-threads and application threads...
> Please confirm.

Run a debug build, and get a backtrace. Then we'll talk ;)

Nyall