Merge lp:~thomas-voss/location-service/do-not-rely-on-obsolete-satellite-based-positioning-state into lp:location-service/15.04

[Thomas Voß]

Remove explicit option to disable satellite-based positioning services.

The service is clever enough to dynamically enable/disable the provider, taking into account
accuracy requirements.

Remove explicit option to disable satellite-based positioning services.

The service is clever enough to dynamically enable/disable the provider, taking into account
accuracy requirements.

[Timo Jyrinki (timo-jyrinki)]

Two tests failing (in the silo build):

Engine.reads_state_from_settings_on_construction
Engine.stores_state_from_settings_on_destruction

review: Needs Fixing

[lp:~thomas-voss/location-service/do-not-rely-on-obsolete-satellite-based-positioning-state]

217. By Thomas Voß on 2016-03-02

Adjust test case and remove expectations on satellite-based positioning state.

[Timo Jyrinki (timo-jyrinki)]

Confirming fixed from the silo on my Bq OTA-9.1.

review: Approve

[Scott Sweeny (ssweeny)]

This code looks good, and the tests all pass locally for me with this latest change.

review: Approve

[Timo Jyrinki (timo-jyrinki)]

That is, without the silo upgrade, unable to get a real GPS fix as witnessed with client --bus system. With the silo in identical conditions, a trustworthy GPS fix with accurate location.

[Timo Jyrinki (timo-jyrinki)]

Ok the problem and how to test the fix:

/var/lib/ubuntu-location-service/config.ini:
- On my (stable OTA-9.1, untouched) phone the second line about satellites use was "Off"

Real GPS wasn't use, only the cell/wifi based approximation 500m-1km off the goal.

I used "client" from ubuntu-location-service-example package to ./client --bus system. When it was broken, I only got approximate location and not eg velocity data.

After upgrading to the fixed silo, despite the config.ini still having the "Off" in there on the second line, I started getting (when near window or outside) the exact coordinates of myself and also additional data in the ./client --bus system output regarding velocity that was not there before.

[Timo Jyrinki (timo-jyrinki)]

Ie Engine::SatelliteBasedPositioningState=SatelliteBasedPositioningState::off is the setting that should be set for testing the bug and the fact that silo fixes GPS to work even if that line is there.

=== modified file 'src/location_service/com/ubuntu/location/engine.cpp'
--- src/location_service/com/ubuntu/location/engine.cpp	2015-11-19 09:55:37 +0000
+++ src/location_service/com/ubuntu/location/engine.cpp	2016-03-02 15:26:55 +0000
@@ -97,10 +97,7 @@
                 Configuration::Keys::engine_state,
                 Configuration::Defaults::engine_state);
 
-    configuration.satellite_based_positioning_state =
-            settings->get_enum_for_key<SatelliteBasedPositioningState>(
-                Configuration::Keys::satellite_based_positioning_state,
-                Configuration::Defaults::satellite_based_positioning_state);
+    configuration.satellite_based_positioning_state = Configuration::Defaults::satellite_based_positioning_state;
 
     configuration.wifi_and_cell_id_reporting_state =
             settings->get_enum_for_key<WifiAndCellIdReportingState>(
@@ -110,12 +107,7 @@
     configuration.engine_state.changed().connect([this](const Engine::Status& status)
     {
         Engine::settings->set_enum_for_key<Engine::Status>(Configuration::Keys::engine_state, status);
-    });
-
-    configuration.satellite_based_positioning_state.changed().connect([this](SatelliteBasedPositioningState state)
-    {
-        Engine::settings->set_enum_for_key<SatelliteBasedPositioningState>(Configuration::Keys::satellite_based_positioning_state, state);
-    });
+    });    
 
     configuration.wifi_and_cell_id_reporting_state.changed().connect([this](WifiAndCellIdReportingState state)
     {
@@ -129,10 +121,6 @@
         Configuration::Keys::engine_state,
         configuration.engine_state);
 
-    settings->set_enum_for_key<SatelliteBasedPositioningState>(
-        Configuration::Keys::satellite_based_positioning_state,
-        configuration.satellite_based_positioning_state);
-
     settings->set_enum_for_key<WifiAndCellIdReportingState>(
         Configuration::Keys::wifi_and_cell_id_reporting_state,
         configuration.wifi_and_cell_id_reporting_state);

=== modified file 'tests/engine_test.cpp'
--- tests/engine_test.cpp	2015-11-19 09:55:37 +0000
+++ tests/engine_test.cpp	2016-03-02 15:26:55 +0000
@@ -273,13 +273,9 @@
     auto selection_policy = std::make_shared<NiceMock<MockProviderSelectionPolicy>>();
 
     EXPECT_CALL(*settings, has_value_for_key(_))
-            .Times(3)
+            .Times(2)
             .WillRepeatedly(Return(true));
     EXPECT_CALL(*settings, get_string_for_key_or_throw(
-                    location::Engine::Configuration::Keys::satellite_based_positioning_state))
-            .Times(1)
-            .WillRepeatedly(Return(ss_satellite_based_positioning_state.str()));
-    EXPECT_CALL(*settings, get_string_for_key_or_throw(
                     location::Engine::Configuration::Keys::wifi_and_cell_id_reporting_state))
             .Times(1)
             .WillRepeatedly(Return(ss_wifi_and_cell_id_reporting_state.str()));
@@ -299,11 +295,6 @@
     auto selection_policy = std::make_shared<NiceMock<MockProviderSelectionPolicy>>();
 
     EXPECT_CALL(*settings, set_string_for_key(
-                    location::Engine::Configuration::Keys::satellite_based_positioning_state,
-                    _))
-            .Times(1)
-            .WillRepeatedly(Return(true));
-    EXPECT_CALL(*settings, set_string_for_key(
                     location::Engine::Configuration::Keys::wifi_and_cell_id_reporting_state,
                     _))
             .Times(1)