Merge lp:~thomas-voss/location-service/fix-1462664 into lp:location-service/15.04

[Thomas Voß]

Handle responses of clients to updates asynchronously.
Rely on dummy::ConnectivityManager as harvesting is disabled anyway.

Handle responses of clients to updates asynchronously.
Rely on dummy::ConnectivityManager as harvesting is disabled anyway.

[Alberto Mardegan (mardy)]

All the messages say "Failed to communicate position update to client: ", but the last two could say "heading" and "velocity", respectively. Just in case we want to be able to tell apart the cases in the logs.

Which I don't think is really important, that's why I'm approving this as it is anyway.

review: Approve

[Alberto Mardegan (mardy)]

But, how does this fixes the linked issues? The methods where already invoked asynchronously; this just adds error logging (which is good).

[Thomas Voß (thomas-voss)]

The previous *async* methods returned a std::future<core::dbus::Result<..>>, which tries to synchronize its state on destruction. Please also note that I'm proposing this early to enable testing easily.

[Alberto Mardegan (mardy)]

Looks good!

review: Approve

=== modified file 'src/location_service/com/ubuntu/location/service/daemon.cpp'
--- src/location_service/com/ubuntu/location/service/daemon.cpp	2015-04-16 10:03:29 +0000
+++ src/location_service/com/ubuntu/location/service/daemon.cpp	2015-10-23 13:49:32 +0000
@@ -20,6 +20,8 @@
 #include <com/ubuntu/location/boost_ptree_settings.h>
 #include <com/ubuntu/location/provider_factory.h>
 
+#include <com/ubuntu/location/connectivity/dummy_connectivity_manager.h>
+
 #include <com/ubuntu/location/service/default_configuration.h>
 #include <com/ubuntu/location/service/demultiplexing_reporter.h>
 #include <com/ubuntu/location/service/ichnaea_reporter.h>
@@ -214,29 +216,8 @@
         dc.the_permission_manager(config.outgoing),
         location::service::Harvester::Configuration
         {
-            location::connectivity::platform_default_manager(),
-            // We submit location/wifi/cell measurements to both
-            // Mozilla's location service and to Ubuntu's own instance.
-            std::make_shared<service::DemultiplexingReporter>(
-            std::initializer_list<service::Harvester::Reporter::Ptr>
-            {
-                std::make_shared<service::ichnaea::Reporter>(
-                     service::ichnaea::Reporter::Configuration
-                     {
-                         "https://location.services.mozilla.com",
-                         "UbuntuLocationService",
-                         "UbuntuLocationService"
-                     }
-                ),
-                std::make_shared<service::ichnaea::Reporter>(
-                     service::ichnaea::Reporter::Configuration
-                     {
-                         "https://162.213.35.107",
-                         "UbuntuLocationService",
-                         "UbuntuLocationService"
-                     }
-                )
-            })
+            std::make_shared<dummy::ConnectivityManager>(),
+            std::make_shared<NullReporter>()
         }
     };
 

=== modified file 'src/location_service/com/ubuntu/location/service/session/skeleton.cpp'
--- src/location_service/com/ubuntu/location/service/session/skeleton.cpp	2014-08-14 20:31:09 +0000
+++ src/location_service/com/ubuntu/location/service/session/skeleton.cpp	2015-10-23 13:49:32 +0000
@@ -283,7 +283,13 @@
     VLOG(10) << __PRETTY_FUNCTION__;
     try
     {
-        configuration.remote.object->invoke_method_asynchronously<culs::session::Interface::UpdatePosition, void>(position);
+        configuration.remote.object->invoke_method_asynchronously_with_callback<culs::session::Interface::UpdatePosition, void>([](const core::dbus::Result<void>& result)
+        {
+            if (result.is_error())
+            {
+                LOG(INFO) << "Failed to communicate position update to client: " << result.error().print();    
+            }
+        }, position);
     } catch(const std::exception&)
     {
         // We consider the session to be dead once we hit an exception here.
@@ -300,7 +306,13 @@
     VLOG(10) << __PRETTY_FUNCTION__;
     try
     {
-        configuration.remote.object->invoke_method_asynchronously<culs::session::Interface::UpdateHeading, void>(heading);
+        configuration.remote.object->invoke_method_asynchronously_with_callback<culs::session::Interface::UpdateHeading, void>([](const core::dbus::Result<void>& result)
+        {
+            if (result.is_error())
+            {
+                LOG(INFO) << "Failed to communicate position update to client: " << result.error().print();    
+            }
+        }, heading);
     } catch(const std::exception&)
     {
         // We consider the session to be dead once we hit an exception here.
@@ -317,7 +329,13 @@
     VLOG(10) << __PRETTY_FUNCTION__;
     try
     {
-        configuration.remote.object->invoke_method_asynchronously<culs::session::Interface::UpdateVelocity, void>(velocity);
+        configuration.remote.object->invoke_method_asynchronously_with_callback<culs::session::Interface::UpdateVelocity, void>([](const core::dbus::Result<void>& result)
+        {
+            if (result.is_error())
+            {
+                LOG(INFO) << "Failed to communicate position update to client: " << result.error().print();    
+            }
+        }, velocity);
     } catch(const std::exception&)
     {
         // We consider the session to be dead once we hit an exception here.