Add metrics for effectiveness of wifi network selection.
Report number of auto connectable wifi services available when
auto connect is started for wifi.
Report number of BSSes associated with a wifi service when connecting
to that service.
BUG=chromium:230884
TEST=unittest, manual
1. Connect a peppy to GoogleGuest.
2. Browse to "chrome://histograms", and verify histogram for
Network.Shill.wifi.AvailableBSSesWhenConnect existed.
3. Verify GoogleGuest is in Preferred Network
4. Restart shill to start auto connect.
5. Browse to "chrome://histograms", and verify histogram for
Network.Shill.wifi.AutoConnectableServices existed.
Change-Id: I5315210944e8f5a5a2253562f680de8217963024
Reviewed-on: https://chromium-review.googlesource.com/193226
Reviewed-by: Peter Qiu <zqiu@chromium.org>
Tested-by: Peter Qiu <zqiu@chromium.org>
Commit-Queue: Peter Qiu <zqiu@chromium.org>
diff --git a/manager.cc b/manager.cc
index 41d471d..caf6112 100644
--- a/manager.cc
+++ b/manager.cc
@@ -1488,6 +1488,13 @@
}
}
+ // Report the number of auto-connectable wifi services available when wifi is
+ // idle (no active or pending connection), which will trigger auto connect
+ // for wifi services.
+ if (IsWifiIdle()) {
+ wifi_provider_->ReportAutoConnectableServices();
+ }
+
// Perform auto-connect.
for (vector<ServiceRefPtr>::iterator it = services_.begin();
it != services_.end(); ++it) {
@@ -2029,6 +2036,22 @@
SortServices();
}
+bool Manager::IsWifiIdle() {
+ bool ret = false;
+
+ // Since services are sorted by connection state, status of the wifi device
+ // can be determine by examing the connection state of the first wifi service.
+ for (const auto &service : services_) {
+ if (service->technology() == Technology::kWifi) {
+ if (!service->IsConnecting() && !service->IsConnected()) {
+ ret = true;
+ }
+ break;
+ }
+ }
+ return ret;
+}
+
void Manager::UpdateProviderMapping() {
providers_[Technology::kEthernetEap] = ethernet_eap_provider_.get();
providers_[Technology::kVPN] = vpn_provider_.get();
diff --git a/manager.h b/manager.h
index 6bde320..2bc287c 100644
--- a/manager.h
+++ b/manager.h
@@ -416,6 +416,7 @@
FRIEND_TEST(ManagerTest, SortServices);
FRIEND_TEST(ManagerTest, SortServicesWithConnection);
FRIEND_TEST(ManagerTest, StartupPortalList);
+ FRIEND_TEST(ManagerTest, IsWifiIdle);
static const char kErrorNoDevice[];
static const char kErrorTypeRequired[];
@@ -521,6 +522,10 @@
ResultStringCallback cb, const Error &error,
bool success);
+ // Return true if wifi device is enabled with no existing connection (pending
+ // or connected).
+ bool IsWifiIdle();
+
// For unit testing.
void set_metrics(Metrics *metrics) { metrics_ = metrics; }
void UpdateProviderMapping();
diff --git a/manager_unittest.cc b/manager_unittest.cc
index cc950ba..0561c5f 100644
--- a/manager_unittest.cc
+++ b/manager_unittest.cc
@@ -4275,4 +4275,62 @@
EXPECT_TRUE(ContainsKey(location_infos, kGeoCellTowersProperty));
}
+TEST_F(ManagerTest, IsWifiIdle) {
+ // No registered service.
+ EXPECT_FALSE(manager()->IsWifiIdle());
+
+ scoped_refptr<MockService> wifi_service(new MockService(control_interface(),
+ dispatcher(),
+ metrics(),
+ manager()));
+
+ scoped_refptr<MockService> cell_service(new MockService(control_interface(),
+ dispatcher(),
+ metrics(),
+ manager()));
+
+ manager()->RegisterService(wifi_service);
+ manager()->RegisterService(cell_service);
+
+ EXPECT_CALL(*wifi_service.get(), technology())
+ .WillRepeatedly(Return(Technology::kWifi));
+ EXPECT_CALL(*cell_service.get(), technology())
+ .WillRepeatedly(Return(Technology::kCellular));
+
+ // Cellular is connected.
+ EXPECT_CALL(*cell_service.get(), IsConnected())
+ .WillRepeatedly(Return(true));
+ manager()->UpdateService(cell_service);
+
+ // No wifi connection attempt.
+ EXPECT_CALL(*wifi_service.get(), IsConnecting())
+ .WillRepeatedly(Return(false));
+ EXPECT_CALL(*wifi_service.get(), IsConnected())
+ .WillRepeatedly(Return(false));
+ manager()->UpdateService(wifi_service);
+ EXPECT_TRUE(manager()->IsWifiIdle());
+
+ // Attempt wifi connection.
+ Mock::VerifyAndClearExpectations(wifi_service);
+ EXPECT_CALL(*wifi_service.get(), technology())
+ .WillRepeatedly(Return(Technology::kWifi));
+ EXPECT_CALL(*wifi_service.get(), IsConnecting())
+ .WillRepeatedly(Return(true));
+ EXPECT_CALL(*wifi_service.get(), IsConnected())
+ .WillRepeatedly(Return(false));
+ manager()->UpdateService(wifi_service);
+ EXPECT_FALSE(manager()->IsWifiIdle());
+
+ // wifi connected.
+ Mock::VerifyAndClearExpectations(wifi_service);
+ EXPECT_CALL(*wifi_service.get(), technology())
+ .WillRepeatedly(Return(Technology::kWifi));
+ EXPECT_CALL(*wifi_service.get(), IsConnecting())
+ .WillRepeatedly(Return(false));
+ EXPECT_CALL(*wifi_service.get(), IsConnected())
+ .WillRepeatedly(Return(true));
+ manager()->UpdateService(wifi_service);
+ EXPECT_FALSE(manager()->IsWifiIdle());
+}
+
} // namespace shill
diff --git a/metrics.cc b/metrics.cc
index 02609da..bff1ab3 100644
--- a/metrics.cc
+++ b/metrics.cc
@@ -277,6 +277,20 @@
const int Metrics::kMetricExpiredLeaseLengthSecondsNumBuckets =
Metrics::kMetricExpiredLeaseLengthSecondsMax;
+// static
+const char Metrics::kMetricWifiAutoConnectableServices[] =
+ "Network.Shill.wifi.AutoConnectableServices";
+const int Metrics::kMetricWifiAutoConnectableServicesMax = 50;
+const int Metrics::kMetricWifiAutoConnectableServicesMin = 1;
+const int Metrics::kMetricWifiAutoConnectableServicesNumBuckets = 10;
+
+// static
+const char Metrics::kMetricWifiAvailableBSSes[] =
+ "Network.Shill.wifi.AvailableBSSesAtConnect";
+const int Metrics::kMetricWifiAvailableBSSesMax = 50;
+const int Metrics::kMetricWifiAvailableBSSesMin = 1;
+const int Metrics::kMetricWifiAvailableBSSesNumBuckets = 10;
+
Metrics::Metrics(EventDispatcher *dispatcher)
: dispatcher_(dispatcher),
library_(&metrics_library_),
@@ -1037,6 +1051,22 @@
kDHCPOptionFailureMax);
}
+void Metrics::NotifyWifiAutoConnectableServices(int num_services) {
+ SendToUMA(kMetricWifiAutoConnectableServices,
+ num_services,
+ kMetricWifiAutoConnectableServicesMin,
+ kMetricWifiAutoConnectableServicesMax,
+ kMetricWifiAutoConnectableServicesNumBuckets);
+}
+
+void Metrics::NotifyWifiAvailableBSSes(int num_bss) {
+ SendToUMA(kMetricWifiAvailableBSSes,
+ num_bss,
+ kMetricWifiAvailableBSSesMin,
+ kMetricWifiAvailableBSSesMax,
+ kMetricWifiAvailableBSSesNumBuckets);
+}
+
bool Metrics::SendEnumToUMA(const string &name, int sample, int max) {
SLOG(Metrics, 5)
<< "Sending enum " << name << " with value " << sample << ".";
diff --git a/metrics.h b/metrics.h
index 2b0f35f..8a9aceb 100644
--- a/metrics.h
+++ b/metrics.h
@@ -436,6 +436,19 @@
static const int kMetricExpiredLeaseLengthSecondsMin;
static const int kMetricExpiredLeaseLengthSecondsNumBuckets;
+ // Number of wifi services available when auto-connect is initiated.
+ static const char kMetricWifiAutoConnectableServices[];
+ static const int kMetricWifiAutoConnectableServicesMax;
+ static const int kMetricWifiAutoConnectableServicesMin;
+ static const int kMetricWifiAutoConnectableServicesNumBuckets;
+
+ // Number of BSSes available for a wifi service when we attempt to connect
+ // to that service.
+ static const char kMetricWifiAvailableBSSes[];
+ static const int kMetricWifiAvailableBSSesMax;
+ static const int kMetricWifiAvailableBSSesMin;
+ static const int kMetricWifiAvailableBSSesNumBuckets;
+
explicit Metrics(EventDispatcher *dispatcher);
virtual ~Metrics();
@@ -593,6 +606,14 @@
// out-of-credits.
void NotifyCellularOutOfCredits(Metrics::CellularOutOfCreditsReason reason);
+ // Notifies this object about number of wifi services available for auto
+ // connect when auto-connect is initiated.
+ virtual void NotifyWifiAutoConnectableServices(int num_services);
+
+ // Notifies this object about number of BSSes available for a wifi service
+ // when attempt to connect to that service.
+ virtual void NotifyWifiAvailableBSSes(int num_services);
+
// Notifies this object about a corrupted profile.
virtual void NotifyCorruptedProfile();
diff --git a/mock_metrics.h b/mock_metrics.h
index 5103f03..a2359c3 100644
--- a/mock_metrics.h
+++ b/mock_metrics.h
@@ -42,6 +42,8 @@
int max));
MOCK_METHOD5(SendToUMA, bool(const std::string &name, int sample, int min,
int max, int num_buckets));
+ MOCK_METHOD1(NotifyWifiAutoConnectableServices, void(int num_service));
+ MOCK_METHOD1(NotifyWifiAvailableBSSes, void(int num_bss));
private:
DISALLOW_COPY_AND_ASSIGN(MockMetrics);
diff --git a/mock_wifi_service.h b/mock_wifi_service.h
index ba47005..241c19e 100644
--- a/mock_wifi_service.h
+++ b/mock_wifi_service.h
@@ -53,6 +53,7 @@
MOCK_CONST_METHOD0(IsRemembered, bool());
MOCK_METHOD0(ResetWiFi, void());
MOCK_CONST_METHOD0(GetSupplicantConfigurationParameters, DBusPropertiesMap());
+ MOCK_CONST_METHOD1(IsAutoConnectable, bool(const char **reason));
private:
DISALLOW_COPY_AND_ASSIGN(MockWiFiService);
diff --git a/wifi_provider.cc b/wifi_provider.cc
index 3b3b11f..94515df 100644
--- a/wifi_provider.cc
+++ b/wifi_provider.cc
@@ -647,4 +647,22 @@
return freq_connects_list;
}
+void WiFiProvider::ReportAutoConnectableServices() {
+ const char *reason = NULL;
+ int num_services = 0;
+
+ // Determine the number of services available for auto-connect.
+ for (const auto &service : services_) {
+ // Service is available for auto connect if it is configured for auto
+ // connect, and is auto-connectable.
+ if (service->auto_connect() && service->IsAutoConnectable(&reason)) {
+ num_services++;
+ }
+ }
+
+ // Only report stats when there are wifi services available.
+ if (num_services) {
+ metrics_->NotifyWifiAutoConnectableServices(num_services);
+ }
+}
} // namespace shill
diff --git a/wifi_provider.h b/wifi_provider.h
index 0378471..7594f88 100644
--- a/wifi_provider.h
+++ b/wifi_provider.h
@@ -111,6 +111,10 @@
// connected. This data is accumulated across multiple shill runs.
virtual FrequencyCountList GetScanFrequencies() const;
+ // Report the number of auto connectable services available to uma
+ // metrics.
+ void ReportAutoConnectableServices();
+
bool disable_vht() { return disable_vht_; }
void set_disable_vht(bool disable_vht) { disable_vht_ = disable_vht; }
diff --git a/wifi_provider_unittest.cc b/wifi_provider_unittest.cc
index ddd2b00..d5d6b58 100644
--- a/wifi_provider_unittest.cc
+++ b/wifi_provider_unittest.cc
@@ -1483,4 +1483,33 @@
this_week));
}
+TEST_F(WiFiProviderTest, ReportAutoConnectableServices) {
+ MockWiFiServiceRefPtr service0 = AddMockService(vector<uint8_t>(1, '0'),
+ kModeManaged,
+ kSecurityNone,
+ false);
+ MockWiFiServiceRefPtr service1 = AddMockService(vector<uint8_t>(1, '1'),
+ kModeManaged,
+ kSecurityNone,
+ false);
+ service0->EnableAndRetainAutoConnect();
+ service0->SetConnectable(true);
+ service1->EnableAndRetainAutoConnect();
+ service1->SetConnectable(true);
+
+ EXPECT_CALL(*service0, IsAutoConnectable(_))
+ .WillOnce(Return(true))
+ .WillOnce(Return(false));
+ EXPECT_CALL(*service1, IsAutoConnectable(_))
+ .WillRepeatedly(Return(false));
+
+ // With 1 auto connectable service.
+ EXPECT_CALL(metrics_, NotifyWifiAutoConnectableServices(1));
+ provider_.ReportAutoConnectableServices();
+
+ // With no auto connectable service.
+ EXPECT_CALL(metrics_, NotifyWifiAutoConnectableServices(_)).Times(0);
+ provider_.ReportAutoConnectableServices();
+}
+
} // namespace shill
diff --git a/wifi_service.cc b/wifi_service.cc
index 9b5666d..a362548 100644
--- a/wifi_service.cc
+++ b/wifi_service.cc
@@ -536,6 +536,9 @@
return;
}
+ // Report number of BSSes available for this service.
+ metrics()->NotifyWifiAvailableBSSes(endpoints_.size());
+
if (Is8021x()) {
// If EAP key management is not set, set to a default.
if (GetEAPKeyManagement().empty())
diff --git a/wifi_service.h b/wifi_service.h
index 01f6389..6166122 100644
--- a/wifi_service.h
+++ b/wifi_service.h
@@ -150,6 +150,8 @@
// This function maps them all into "psk".
static std::string GetSecurityClass(const std::string &security);
+ virtual bool IsAutoConnectable(const char **reason) const;
+
// Signal level in dBm. If no current endpoint, returns
// std::numeric_limits<int>::min().
int16 SignalLevel() const;
@@ -158,7 +160,6 @@
bool expecting_disconnect() const { return expecting_disconnect_; }
protected:
- virtual bool IsAutoConnectable(const char **reason) const;
virtual void SetEAPKeyManagement(const std::string &key_management);
virtual std::string GetTethering(Error *error) const override;
diff --git a/wifi_service_unittest.cc b/wifi_service_unittest.cc
index 82933b5..d300c67 100644
--- a/wifi_service_unittest.cc
+++ b/wifi_service_unittest.cc
@@ -451,6 +451,19 @@
ContainsKey(arg, WPASupplicant::kNetworkPropertyFrequency);
}
+TEST_F(WiFiServiceTest, ConnectReportBSSes) {
+ WiFiEndpointRefPtr endpoint1 =
+ MakeOpenEndpoint("a", "00:00:00:00:00:01", 0, 0);
+ WiFiEndpointRefPtr endpoint2 =
+ MakeOpenEndpoint("a", "00:00:00:00:00:02", 0, 0);
+ WiFiServiceRefPtr wifi_service = MakeServiceWithWiFi(kSecurityNone);
+ wifi_service->AddEndpoint(endpoint1);
+ wifi_service->AddEndpoint(endpoint2);
+ EXPECT_CALL(*metrics(), NotifyWifiAvailableBSSes(2));
+ EXPECT_CALL(*wifi(), ConnectTo(wifi_service.get()));
+ wifi_service->Connect(NULL, "in test");
+}
+
TEST_F(WiFiServiceTest, ConnectTaskWPA) {
WiFiServiceRefPtr wifi_service = MakeServiceWithWiFi(kSecurityWpa);
EXPECT_CALL(*wifi(), ConnectTo(wifi_service.get()));