[Marble-devel] Review Request: Transform all runners into plugins; let runners also do reverse geocoding and routing

Dennis Nienhüser earthwings at gentoo.org
Sun Jul 18 18:01:13 CEST 2010 

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/4587/
-----------------------------------------------------------

(Updated 2010-07-18 16:01:13.200564)


Review request for marble.


Changes
-------

Extensions to the former patch:
- Extend MarbleAbstractRunner interface to allow runners do reverse geocoding and routing
- MarbleAbstractRunner does not derive from QThread anymore, threads are handled transparently by the MarbleRunnerManager
- Use QThreadPool and QRunnable (new class RunnerTask) to execute runners in threads
- Improve method names in MarbleAbstractRunner and MarbleRunnerManager
- Move route parsing from RoutingModel to plugins, RoutingModel only accepts GeoDataDocument* from now on
- Transform OpenRouteService and Yours RoutingProvider classes into runner plugins
- Remove AbstractRoutingProvider, not needed anymore
- Add new gosmore runner plugin, it does offline routing by calling the gosmore application and parsing its output. Does nothing if gosmore is not installed or ~/.local/share/marble/maps/earth/gosmore/gosmore.pak does not exist (no default map provided, to be improved)
- Routing UI can handle several alternative routes coming in (displayed fifo in a combobox for now, to be improved, see comments in code)

>From my tests, things are working fine. Some todos are noted in the code, but I'd rather continue to work on them in trunk before the patch gets unmaintainable locally.


Summary (updated)
-------

This is the first part of some refactoring of MarbleAbstractRunner and related classes. It moves all individual runners into their own plugins. Later I plan to add reverse geocoding and routing support to the interface and make more such plugins from the existing code (OSR, Yours for routing, nominatim for reverse geocoding, more to come).

The patch looks longish, but mostly just moves things around without adding/changing too much functionality. UI and behavior should be just the same.

To get an overview, here are the high level changes:
- Move MarbleAbstractRunner* from src/lib/runners/ to src/lib.
- Create a RunnerPlugin class in src/lib, derived from PluginInterface, which adds a new MarbleAbstractRunner factory method.
- Export RunnerPlugin and MarbleAbstractRunner headers
- Add support for RunnerPlugins in PluginManager
- Move all individual src/lib/runners/*Runner* classes to their own plugins in src/plugins/runner/$name
- Get rid of src/lib/runners/ directory
- Extend MarbleAbstractRunner interface, runners must tell whether they can work offline and on other planets
- Change MarbleRunnerManager to use the plugins

I made two changes compared to previous approaches wrt plugins:
- RunnerPlugin provides default implementations for all the name(), description() etc methods and lets plugins change it (protected). That removes quite a few lines of IMHO useless code when writing new plugins (instead of overwriting all those methods again, call setName("foo"), setDescription("bla bla") etc in the new plugin).
- A RunnerPlugin is not both a MarbleAbstractRunner and a factory for itself, instead it is just a factory for MarbleAbstractRunner instances. I think this is generally mixed up currently in the other plugins without any benefit, but confusing code (look at PluginManager).

I think it's better this way, speak up if there are reasons against it.


Diffs (updated)
-----

  /trunk/KDE/kdeedu/marble/src/lib/CMakeLists.txt 1151118 
  /trunk/KDE/kdeedu/marble/src/lib/MarbleAbstractRunner.h PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/lib/MarbleAbstractRunner.cpp PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/lib/MarbleControlBox.cpp 1151118 
  /trunk/KDE/kdeedu/marble/src/lib/MarbleRunnerManager.h 1151118 
  /trunk/KDE/kdeedu/marble/src/lib/MarbleRunnerManager.cpp 1151118 
  /trunk/KDE/kdeedu/marble/src/lib/PluginManager.h 1151118 
  /trunk/KDE/kdeedu/marble/src/lib/PluginManager.cpp 1151118 
  /trunk/KDE/kdeedu/marble/src/lib/RunnerPlugin.h PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/lib/RunnerPlugin.cpp PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/lib/RunnerTask.h PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/lib/RunnerTask.cpp PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/lib/routing/AbstractRoutingProvider.h 1151118 
  /trunk/KDE/kdeedu/marble/src/lib/routing/AbstractRoutingProvider.cpp 1151118 
  /trunk/KDE/kdeedu/marble/src/lib/routing/OrsRoutingProvider.h 1151118 
  /trunk/KDE/kdeedu/marble/src/lib/routing/OrsRoutingProvider.cpp 1151118 
  /trunk/KDE/kdeedu/marble/src/lib/routing/RoutingInputWidget.h 1151118 
  /trunk/KDE/kdeedu/marble/src/lib/routing/RoutingInputWidget.cpp 1151118 
  /trunk/KDE/kdeedu/marble/src/lib/routing/RoutingManager.h 1151118 
  /trunk/KDE/kdeedu/marble/src/lib/routing/RoutingManager.cpp 1151118 
  /trunk/KDE/kdeedu/marble/src/lib/routing/RoutingModel.h 1151118 
  /trunk/KDE/kdeedu/marble/src/lib/routing/RoutingModel.cpp 1151118 
  /trunk/KDE/kdeedu/marble/src/lib/routing/RoutingWidget.h 1151118 
  /trunk/KDE/kdeedu/marble/src/lib/routing/RoutingWidget.cpp 1151118 
  /trunk/KDE/kdeedu/marble/src/lib/routing/RoutingWidget.ui 1151118 
  /trunk/KDE/kdeedu/marble/src/lib/routing/YoursRoutingProvider.h 1151118 
  /trunk/KDE/kdeedu/marble/src/lib/routing/YoursRoutingProvider.cpp 1151118 
  /trunk/KDE/kdeedu/marble/src/lib/runners/HostipRunner.h 1151118 
  /trunk/KDE/kdeedu/marble/src/lib/runners/HostipRunner.cpp 1151118 
  /trunk/KDE/kdeedu/marble/src/lib/runners/LatLonRunner.h 1151118 
  /trunk/KDE/kdeedu/marble/src/lib/runners/LatLonRunner.cpp 1151118 
  /trunk/KDE/kdeedu/marble/src/lib/runners/LocalDatabaseRunner.h 1151118 
  /trunk/KDE/kdeedu/marble/src/lib/runners/LocalDatabaseRunner.cpp 1151118 
  /trunk/KDE/kdeedu/marble/src/lib/runners/MarbleAbstractRunner.h 1151118 
  /trunk/KDE/kdeedu/marble/src/lib/runners/MarbleAbstractRunner.cpp 1151118 
  /trunk/KDE/kdeedu/marble/src/lib/runners/OnfRunner.h 1151118 
  /trunk/KDE/kdeedu/marble/src/lib/runners/OnfRunner.cpp 1151118 
  /trunk/KDE/kdeedu/marble/src/lib/runners/OsmNominatimRunner.h 1151118 
  /trunk/KDE/kdeedu/marble/src/lib/runners/OsmNominatimRunner.cpp 1151118 
  /trunk/KDE/kdeedu/marble/src/plugins/CMakeLists.txt 1151118 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/CMakeLists.txt PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/gosmore/CMakeLists.txt PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/gosmore/GosmorePlugin.h PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/gosmore/GosmorePlugin.cpp PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/gosmore/GosmoreRunner.h PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/gosmore/GosmoreRunner.cpp PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/hostip/CMakeLists.txt PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/hostip/HostipPlugin.h PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/hostip/HostipPlugin.cpp PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/hostip/HostipRunner.h PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/hostip/HostipRunner.cpp PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/latlon/CMakeLists.txt PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/latlon/LatLonPlugin.h PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/latlon/LatLonPlugin.cpp PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/latlon/LatLonRunner.h PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/latlon/LatLonRunner.cpp PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/localdatabase/CMakeLists.txt PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/localdatabase/LocalDatabasePlugin.h PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/localdatabase/LocalDatabasePlugin.cpp PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/localdatabase/LocalDatabaseRunner.h PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/localdatabase/LocalDatabaseRunner.cpp PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/namefinder/CMakeLists.txt PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/namefinder/NameFinderPlugin.h PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/namefinder/NameFinderPlugin.cpp PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/namefinder/OnfRunner.h PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/namefinder/OnfRunner.cpp PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/nominatim/CMakeLists.txt PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/nominatim/NominatimPlugin.h PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/nominatim/NominatimPlugin.cpp PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/nominatim/OsmNominatimRunner.h PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/nominatim/OsmNominatimRunner.cpp PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/openrouteservice/CMakeLists.txt PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/openrouteservice/OpenRouteServicePlugin.h PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/openrouteservice/OpenRouteServicePlugin.cpp PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/openrouteservice/OpenRouteServiceRunner.h PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/openrouteservice/OpenRouteServiceRunner.cpp PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/yours/CMakeLists.txt PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/yours/YoursPlugin.h PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/yours/YoursPlugin.cpp PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/yours/YoursRunner.h PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/yours/YoursRunner.cpp PRE-CREATION 

Diff: http://reviewboard.kde.org/r/4587/diff


Testing
-------

Various search and routing queries on earth and moon.


Thanks,

Dennis

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/marble-devel/attachments/20100718/5ff698c1/attachment.htm

More information about the Marble-devel mailing list