shill: Keeps track of successful wifi connection frequencies.
Saves the connection frequencies (and counts therefor) in the default
profile. Sends number of frequencies on which successful connections
have been made to an UMA stat.
BUG=chromium:222081
TEST=unittests and manual (look for UMA stat
'Network.Shill.Wifi.FrequenciesConnectedEver', disconnect from
wifi, reconnect, verify that the UMA stat went up by 1).
Change-Id: I1e3c75b82ac387dd01066c4da4ebfce2c4b2ddc0
Reviewed-on: https://gerrit.chromium.org/gerrit/47154
Commit-Queue: Wade Guthrie <wdg@chromium.org>
Reviewed-by: Wade Guthrie <wdg@chromium.org>
Tested-by: Wade Guthrie <wdg@chromium.org>
diff --git a/default_profile.cc b/default_profile.cc
index b9336a8..475441e 100644
--- a/default_profile.cc
+++ b/default_profile.cc
@@ -18,6 +18,7 @@
#include "shill/portal_detector.h"
#include "shill/resolver.h"
#include "shill/store_interface.h"
+#include "shill/wifi_provider.h"
using base::FilePath;
using std::string;
@@ -161,6 +162,10 @@
return device->Save(storage()) && storage()->Flush();
}
+bool DefaultProfile::UpdateWiFiProvider(const WiFiProvider &wifi_provider) {
+ return wifi_provider.Save(storage()) && storage()->Flush();
+}
+
bool DefaultProfile::GetStoragePath(FilePath *path) {
*path = storage_path_.Append(base::StringPrintf("%s.profile",
profile_id_.c_str()));
diff --git a/default_profile.h b/default_profile.h
index 4680f14..387d3d0 100644
--- a/default_profile.h
+++ b/default_profile.h
@@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#ifndef SHILL_DEFAULT_PROFILE_
-#define SHILL_DEFAULT_PROFILE_
+#ifndef SHILL_DEFAULT_PROFILE_H_
+#define SHILL_DEFAULT_PROFILE_H_
#include <string>
#include <vector>
@@ -21,6 +21,7 @@
namespace shill {
class ControlInterface;
+class WiFiProvider;
class DefaultProfile : public Profile {
public:
@@ -50,6 +51,9 @@
// Inherited from Profile.
virtual bool UpdateDevice(const DeviceRefPtr &device);
+ // Inherited from Profile.
+ virtual bool UpdateWiFiProvider(const WiFiProvider &wifi_provider);
+
protected:
// Sets |path| to the persistent store file path for the default, global
// profile. Returns true on success, and false if unable to determine an
@@ -85,4 +89,4 @@
} // namespace shill
-#endif // SHILL_DEFAULT_PROFILE_
+#endif // SHILL_DEFAULT_PROFILE_H_
diff --git a/default_profile_unittest.cc b/default_profile_unittest.cc
index 22c17c6..6ff59ff 100644
--- a/default_profile_unittest.cc
+++ b/default_profile_unittest.cc
@@ -21,9 +21,11 @@
#include "shill/mock_device.h"
#include "shill/mock_service.h"
#include "shill/mock_store.h"
+#include "shill/mock_wifi_provider.h"
#include "shill/portal_detector.h"
#include "shill/property_store_unittest.h"
#include "shill/resolver.h"
+#include "shill/wifi_service.h"
using base::FilePath;
using std::map;
@@ -321,4 +323,32 @@
EXPECT_FALSE(profile_->UpdateDevice(device_));
}
+TEST_F(DefaultProfileTest, UpdateWiFiProvider) {
+ MockWiFiProvider wifi_provider;
+
+ {
+ scoped_ptr<MockStore> storage(new MockStore());
+ EXPECT_CALL(*storage, Flush()).Times(0);
+ EXPECT_CALL(wifi_provider, Save(storage.get())).WillOnce(Return(false));
+ profile_->set_storage(storage.release());
+ EXPECT_FALSE(profile_->UpdateWiFiProvider(wifi_provider));
+ }
+
+ {
+ scoped_ptr<MockStore> storage(new MockStore());
+ EXPECT_CALL(*storage, Flush()).WillOnce(Return(false));
+ EXPECT_CALL(wifi_provider, Save(storage.get())).WillOnce(Return(true));
+ profile_->set_storage(storage.release());
+ EXPECT_FALSE(profile_->UpdateWiFiProvider(wifi_provider));
+ }
+
+ {
+ scoped_ptr<MockStore> storage(new MockStore());
+ EXPECT_CALL(*storage, Flush()).WillOnce(Return(true));
+ EXPECT_CALL(wifi_provider, Save(storage.get())).WillOnce(Return(true));
+ profile_->set_storage(storage.release());
+ EXPECT_TRUE(profile_->UpdateWiFiProvider(wifi_provider));
+ }
+}
+
} // namespace shill
diff --git a/manager.cc b/manager.cc
index f43f62c..55c6e1a 100644
--- a/manager.cc
+++ b/manager.cc
@@ -220,6 +220,8 @@
UpdateDevice(*devices_it);
}
+ UpdateWiFiProvider();
+
// Persist profile, service information to disk.
vector<ProfileRefPtr>::iterator profiles_it;
for (profiles_it = profiles_.begin(); profiles_it != profiles_.end();
@@ -653,7 +655,8 @@
}
void Manager::OnProfileStorageInitialized(StoreInterface *storage) {
- wifi_provider_->FixupServiceEntries(storage, IsDefaultProfile(storage));
+ wifi_provider_->LoadAndFixupServiceEntries(storage,
+ IsDefaultProfile(storage));
}
DeviceRefPtr Manager::GetEnabledDeviceWithTechnology(
@@ -936,9 +939,11 @@
void Manager::UpdateDevice(const DeviceRefPtr &to_update) {
LOG(INFO) << "Device " << to_update->link_name() << " updated: "
<< (to_update->enabled_persistent() ? "enabled" : "disabled");
- // Saves the device to the topmost profile that accepts it. Normally, this
- // would be the only DefaultProfile at the bottom of the stack except in
- // autotests that push a second test-only DefaultProfile.
+ // Saves the device to the topmost profile that accepts it (ordinary
+ // profiles don't update but default profiles do). Normally, the topmost
+ // updating profile would be the DefaultProfile at the bottom of the stack.
+ // Autotests, differ from the normal scenario, however, in that they push a
+ // second test-only DefaultProfile.
for (vector<ProfileRefPtr>::reverse_iterator rit = profiles_.rbegin();
rit != profiles_.rend(); ++rit) {
if ((*rit)->UpdateDevice(to_update)) {
@@ -947,6 +952,20 @@
}
}
+void Manager::UpdateWiFiProvider() {
+ // Saves |wifi_provider_| to the topmost profile that accepts it (ordinary
+ // profiles don't update but default profiles do). Normally, the topmost
+ // updating profile would be the DefaultProfile at the bottom of the stack.
+ // Autotests, differ from the normal scenario, however, in that they push a
+ // second test-only DefaultProfile.
+ for (vector<ProfileRefPtr>::reverse_iterator rit = profiles_.rbegin();
+ rit != profiles_.rend(); ++rit) {
+ if ((*rit)->UpdateWiFiProvider(*wifi_provider_)) {
+ return;
+ }
+ }
+}
+
void Manager::SaveServiceToProfile(const ServiceRefPtr &to_update) {
if (IsServiceEphemeral(to_update)) {
if (profiles_.empty()) {
@@ -1758,7 +1777,7 @@
map<string, GeolocationInfos> networks;
for (vector<DeviceRefPtr>::iterator it = devices_.begin();
it != devices_.end(); ++it) {
- switch((*it)->technology()) {
+ switch ((*it)->technology()) {
// TODO(gauravsh): crosbug.com/35736 Need a strategy for combining
// geolocation objects from multiple devices of the same technolgy.
// Currently, we just pick the geolocation objects from the first found
diff --git a/manager.h b/manager.h
index 05e4fa4..321dc1c 100644
--- a/manager.h
+++ b/manager.h
@@ -114,6 +114,8 @@
// Persists |to_update| into an appropriate profile.
virtual void UpdateDevice(const DeviceRefPtr &to_update);
+ virtual void UpdateWiFiProvider();
+
void FilterByTechnology(Technology::Identifier tech,
std::vector<DeviceRefPtr> *found) const;
diff --git a/manager_unittest.cc b/manager_unittest.cc
index 34a9c99..3b26f1d 100644
--- a/manager_unittest.cc
+++ b/manager_unittest.cc
@@ -2564,6 +2564,7 @@
EXPECT_CALL(*profile.get(),
UpdateDevice(DeviceRefPtr(mock_devices_[0].get())))
.WillOnce(Return(true));
+ EXPECT_CALL(*profile.get(), UpdateWiFiProvider(_)).WillOnce(Return(true));
EXPECT_CALL(*profile.get(), Save()).WillOnce(Return(true));
EXPECT_CALL(*service.get(), Disconnect(_)).Times(1);
manager()->Stop();
diff --git a/metrics.cc b/metrics.cc
index 2a10c73..0edd193 100644
--- a/metrics.cc
+++ b/metrics.cc
@@ -133,6 +133,12 @@
const char Metrics::kMetricPortalResult[] = "Network.Shill.%s.PortalResult";
+const char Metrics::kMetricFrequenciesConnectedEver[] =
+ "Network.Shill.WiFi.FrequenciesConnectedEver";
+const int Metrics::kMetricFrequenciesConnectedMax = 50;
+const int Metrics::kMetricFrequenciesConnectedMin = 1;
+const int Metrics::kMetricFrequenciesConnectedNumBuckets = 50;
+
const char Metrics::kMetricTerminationActionTimeOnTerminate[] =
"Network.Shill.TerminationActionTime.OnTerminate";
const char Metrics::kMetricTerminationActionResultOnTerminate[] =
@@ -690,7 +696,7 @@
} else {
metric_disconnect_reason = kMetricLinkClientDisconnectReason;
metric_disconnect_type = kMetricLinkClientDisconnectType;
- switch(reason) {
+ switch (reason) {
case IEEE_80211::kReasonCodeSenderHasLeft:
case IEEE_80211::kReasonCodeDisassociatedHasLeft:
type = kStatusCodeTypeByUser;
diff --git a/metrics.h b/metrics.h
index d1af070..614cf86 100644
--- a/metrics.h
+++ b/metrics.h
@@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#ifndef SHILL_METRICS_
-#define SHILL_METRICS_
+#ifndef SHILL_METRICS_H_
+#define SHILL_METRICS_H_
#include <list>
@@ -303,6 +303,12 @@
// The result of the portal detection.
static const char kMetricPortalResult[];
+ // Wifi connection frequencies.
+ static const char kMetricFrequenciesConnectedEver[];
+ static const int kMetricFrequenciesConnectedMax;
+ static const int kMetricFrequenciesConnectedMin;
+ static const int kMetricFrequenciesConnectedNumBuckets;
+
static const char kMetricPowerManagerKey[];
// LinkMonitor statistics.
@@ -584,7 +590,7 @@
Service::ConnectState new_state);
void SendServiceFailure(const Service *service);
- DeviceMetrics *GetDeviceMetrics (int interface_index) const;
+ DeviceMetrics *GetDeviceMetrics(int interface_index) const;
void AutoConnectMetricsReset(DeviceMetrics *device_metrics);
void HourlyTimeoutHandler();
@@ -632,4 +638,4 @@
} // namespace shill
-#endif // SHILL_METRICS_
+#endif // SHILL_METRICS_H_
diff --git a/mock_manager.h b/mock_manager.h
index 77d9947..bbafc55 100644
--- a/mock_manager.h
+++ b/mock_manager.h
@@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#ifndef SHILL_MOCK_MANAGER_
-#define SHILL_MOCK_MANAGER_
+#ifndef SHILL_MOCK_MANAGER_H_
+#define SHILL_MOCK_MANAGER_H_
#include <base/basictypes.h>
#include <gmock/gmock.h>
@@ -38,10 +38,11 @@
int(const ServiceCallback &callback));
MOCK_METHOD1(DeregisterDefaultServiceCallback, void(int tag));
MOCK_METHOD1(UpdateDevice, void(const DeviceRefPtr &to_update));
+ MOCK_METHOD0(UpdateWiFiProvider, void());
MOCK_METHOD1(RecheckPortalOnService, void(const ServiceRefPtr &service));
MOCK_METHOD2(HandleProfileEntryDeletion,
- bool (const ProfileRefPtr &profile,
- const std::string &entry_name));
+ bool(const ProfileRefPtr &profile,
+ const std::string &entry_name));
MOCK_CONST_METHOD0(GetDefaultService, ServiceRefPtr());
MOCK_CONST_METHOD0(IsOnline, bool());
MOCK_METHOD0(UpdateEnabledTechnologies, void());
@@ -64,4 +65,4 @@
} // namespace shill
-#endif // SHILL_MOCK_MANAGER_
+#endif // SHILL_MOCK_MANAGER_H_
diff --git a/mock_profile.h b/mock_profile.h
index 2ab3087..64b8f96 100644
--- a/mock_profile.h
+++ b/mock_profile.h
@@ -2,14 +2,15 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#ifndef SHILL_MOCK_PROFILE_
-#define SHILL_MOCK_PROFILE_
+#ifndef SHILL_MOCK_PROFILE_H_
+#define SHILL_MOCK_PROFILE_H_
#include <string>
#include <gmock/gmock.h>
#include "shill/profile.h"
+#include "shill/wifi_provider.h"
namespace shill {
@@ -32,6 +33,7 @@
MOCK_METHOD1(GetStoragePath, bool(base::FilePath *filepath));
MOCK_METHOD1(UpdateService, bool(const ServiceRefPtr &service));
MOCK_METHOD1(UpdateDevice, bool(const DeviceRefPtr &device));
+ MOCK_METHOD1(UpdateWiFiProvider, bool(const WiFiProvider &wifi_provider));
MOCK_METHOD0(Save, bool());
MOCK_CONST_METHOD0(GetConstStorage, const StoreInterface *());
@@ -41,4 +43,4 @@
} // namespace shill
-#endif // SHILL_MOCK_PROFILE_
+#endif // SHILL_MOCK_PROFILE_H_
diff --git a/mock_wifi_provider.h b/mock_wifi_provider.h
index 01edcea..79bd579 100644
--- a/mock_wifi_provider.h
+++ b/mock_wifi_provider.h
@@ -36,8 +36,10 @@
void(const WiFiEndpointConstRefPtr &endpoint));
MOCK_METHOD1(OnServiceUnloaded, bool(const WiFiServiceRefPtr &service));
MOCK_METHOD0(GetHiddenSSIDList, ByteArrays());
- MOCK_METHOD2(FixupServiceEntries, void(StoreInterface *storage,
- bool is_default_profile));
+ MOCK_METHOD2(LoadAndFixupServiceEntries, void(StoreInterface *storage,
+ bool is_default_profile));
+ MOCK_CONST_METHOD1(Save, bool(StoreInterface *storage));
+ MOCK_METHOD1(IncrementConnectCount, void(uint16 frequency));
private:
DISALLOW_COPY_AND_ASSIGN(MockWiFiProvider);
diff --git a/profile.cc b/profile.cc
index 912a17c..6aa0406 100644
--- a/profile.cc
+++ b/profile.cc
@@ -301,6 +301,10 @@
return false;
}
+bool Profile::UpdateWiFiProvider(const WiFiProvider &wifi_provider) {
+ return false;
+}
+
void Profile::HelpRegisterDerivedStrings(
const string &name,
Strings(Profile::*get)(Error *),
diff --git a/profile.h b/profile.h
index a6e5abd..de7e980 100644
--- a/profile.h
+++ b/profile.h
@@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#ifndef SHILL_PROFILE_
-#define SHILL_PROFILE_
+#ifndef SHILL_PROFILE_H_
+#define SHILL_PROFILE_H_
#include <map>
#include <string>
@@ -31,6 +31,7 @@
class Metrics;
class ProfileAdaptorInterface;
class StoreInterface;
+class WiFiProvider;
class Profile : public base::RefCounted<Profile> {
public:
@@ -129,6 +130,12 @@
// DefaultProfile.
virtual bool UpdateDevice(const DeviceRefPtr &device);
+ // Clobbers persisted notion of |wifi_provider| with data from
+ // |wifi_provider|. Returns true if |wifi_provider| was found and updated,
+ // false otherwise. The base implementation always returns false -- currently
+ // wifi_provider is persisted only in DefaultProfile.
+ virtual bool UpdateWiFiProvider(const WiFiProvider &wifi_provider);
+
// Write all in-memory state to disk via |storage_|.
virtual bool Save();
@@ -201,4 +208,4 @@
} // namespace shill
-#endif // SHILL_PROFILE_
+#endif // SHILL_PROFILE_H_
diff --git a/wifi.cc b/wifi.cc
index cc6b5c8..3865605 100644
--- a/wifi.cc
+++ b/wifi.cc
@@ -684,6 +684,7 @@
<< " " << LogSSID(endpoint->ssid_string());
service->NotifyCurrentEndpoint(endpoint);
+ provider_->IncrementConnectCount(endpoint->frequency());
if (pending_service_.get() &&
service.get() != pending_service_.get()) {
diff --git a/wifi_provider.cc b/wifi_provider.cc
index 2557845..00f8d54 100644
--- a/wifi_provider.cc
+++ b/wifi_provider.cc
@@ -4,6 +4,9 @@
#include "shill/wifi_provider.h"
+#include <stdlib.h>
+
+#include <limits>
#include <set>
#include <string>
#include <vector>
@@ -42,6 +45,9 @@
"security mode is unsupported";
const char WiFiProvider::kManagerErrorUnsupportedServiceMode[] =
"service mode is unsupported";
+const char WiFiProvider::kFrequencyDelimiter[] = ":";
+const char WiFiProvider::kStorageId[] = "wifi_provider";
+const char WiFiProvider::kStorageFrequencies[] = "Frequencies";
WiFiProvider::WiFiProvider(ControlInterface *control_interface,
EventDispatcher *dispatcher,
@@ -51,7 +57,8 @@
dispatcher_(dispatcher),
metrics_(metrics),
manager_(manager),
- running_(false) {}
+ running_(false),
+ total_frequency_connections_(-1L) {}
WiFiProvider::~WiFiProvider() {}
@@ -301,7 +308,7 @@
return true;
}
-void WiFiProvider::FixupServiceEntries(
+void WiFiProvider::LoadAndFixupServiceEntries(
StoreInterface *storage, bool is_default_profile) {
if (WiFiService::FixupServiceEntries(storage)) {
storage->Flush();
@@ -315,6 +322,32 @@
profile_type,
Metrics::kMetricServiceFixupMax);
}
+ // TODO(wdg): Determine how this should be structured for, currently
+ // non-existant, autotests. |kStorageFrequencies| should only exist in the
+ // default profile except for autotests where a test_profile is pushed. This
+ // may need to be modified for that case.
+ if (is_default_profile) {
+ vector<string> frequencies;
+ if (storage->GetStringList(kStorageId,
+ kStorageFrequencies,
+ &frequencies)) {
+ StringListToFrequencyMap(frequencies, &connect_count_by_frequency_);
+ total_frequency_connections_ = 0L;
+ WiFiProvider::ConnectFrequencyMap::const_iterator i;
+ for (i = connect_count_by_frequency_.begin();
+ i != connect_count_by_frequency_.end();
+ ++i) {
+ total_frequency_connections_ += i->second;
+ }
+ }
+ }
+}
+
+bool WiFiProvider::Save(StoreInterface *storage) const {
+ vector<string> frequencies;
+ FrequencyMapToStringList(connect_count_by_frequency_, &frequencies);
+ storage->SetStringList(kStorageId, kStorageFrequencies, frequencies);
+ return true;
}
WiFiServiceRefPtr WiFiProvider::AddService(const vector<uint8_t> &ssid,
@@ -430,4 +463,78 @@
return true;
}
+// static
+void WiFiProvider::StringListToFrequencyMap(const vector<string> &strings,
+ ConnectFrequencyMap *numbers) {
+ if (!numbers) {
+ LOG(ERROR) << "Null |numbers| parameter";
+ return;
+ }
+
+ vector<string>::const_iterator i;
+ for (i = strings.begin(); i != strings.end(); ++i) {
+ size_t delimiter = i->find(kFrequencyDelimiter);
+ if (delimiter == i->npos) {
+ LOG(WARNING) << "Found no '" << kFrequencyDelimiter << "' in '"
+ << *i << "'";
+ continue;
+ }
+ uint16 freq = atoi(i->c_str());
+ uint64 connections = atoll(i->c_str() + delimiter + 1);
+ (*numbers)[freq] = connections;
+ }
+}
+
+// static
+void WiFiProvider::FrequencyMapToStringList(const ConnectFrequencyMap &numbers,
+ vector<string> *strings) {
+ if (!strings) {
+ LOG(ERROR) << "Null |strings| parameter";
+ return;
+ }
+
+ ConnectFrequencyMap::const_iterator i;
+ for (i = numbers.begin(); i != numbers.end(); ++i) {
+ // Use base::Int64ToString() instead of using something like "%llu"
+ // (not correct for native 64 bit architectures) or PRId64 (does not
+ // work correctly using cros_workon_make due to include intricacies).
+ string result = StringPrintf("%u%s%s", i->first, kFrequencyDelimiter,
+ base::Int64ToString(i->second).c_str());
+ strings->push_back(result);
+ }
+}
+
+void WiFiProvider::IncrementConnectCount(uint16 frequency_mhz) {
+ // Freeze the accumulation of frequency counts when the total maxes-out.
+ // This ensures that no count wraps and that the relative values are
+ // consistent.
+ // TODO(wdg): In future CL, |total_frequency_connections_| is used to
+ // calculate percentiles for progressive scan. This check needs to be in
+ // place so _that_ value doesn't wrap, either.
+ // TODO(wdg): Replace this, simple, 'forever' collection of connection
+ // statistics with a more clever 'aging' algorithm. crbug.com/227233
+ if (total_frequency_connections_ + 1 == std::numeric_limits<int64_t>::max()) {
+ LOG(ERROR) << "Shill has logged " << total_frequency_connections_
+ << " connections -- must be an error. Resetting connection "
+ << "count.";
+ connect_count_by_frequency_.clear();
+ }
+
+ int64 previous_value = 0;
+ if (ContainsKey(connect_count_by_frequency_, frequency_mhz)) {
+ previous_value = connect_count_by_frequency_[frequency_mhz];
+ }
+
+ connect_count_by_frequency_[frequency_mhz] = ++previous_value;
+ ++total_frequency_connections_;
+ manager_->UpdateWiFiProvider();
+
+ metrics_->SendToUMA(
+ Metrics::kMetricFrequenciesConnectedEver,
+ connect_count_by_frequency_.size(),
+ Metrics::kMetricFrequenciesConnectedMin,
+ Metrics::kMetricFrequenciesConnectedMax,
+ Metrics::kMetricFrequenciesConnectedNumBuckets);
+}
+
} // namespace shill
diff --git a/wifi_provider.h b/wifi_provider.h
index 0c4ffd0..ebe8da2 100644
--- a/wifi_provider.h
+++ b/wifi_provider.h
@@ -2,8 +2,12 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#ifndef SHILL_WIFI_PROVIDER_
-#define SHILL_WIFI_PROVIDER_
+#ifndef SHILL_WIFI_PROVIDER_H_
+#define SHILL_WIFI_PROVIDER_H_
+
+#include <map>
+
+#include <gtest/gtest_prod.h> // for FRIEND_TEST
#include "shill/accessor_interface.h" // for ByteArrays
#include "shill/refptr_types.h"
@@ -25,6 +29,8 @@
// (created due to user or storage configuration) Services.
class WiFiProvider {
public:
+ typedef std::map<uint16, int64> ConnectFrequencyMap;
+
WiFiProvider(ControlInterface *control_interface,
EventDispatcher *dispatcher,
Metrics *metrics,
@@ -81,11 +87,20 @@
// Calls WiFiService::FixupServiceEntries() and adds a UMA metric if
// this causes entries to be updated.
- virtual void FixupServiceEntries(
- StoreInterface *storage, bool is_default_profile);
+ virtual void LoadAndFixupServiceEntries(StoreInterface *storage,
+ bool is_default_profile);
+
+ // Save configuration for wifi_provider to |storage|.
+ virtual bool Save(StoreInterface *storage) const;
+
+ virtual void IncrementConnectCount(uint16 frequency_mhz);
private:
friend class WiFiProviderTest;
+ FRIEND_TEST(WiFiProviderTest, FrequencyMapToStringList);
+ FRIEND_TEST(WiFiProviderTest, LoadAndFixupServiceEntries);
+ FRIEND_TEST(WiFiProviderTest, LoadAndFixupServiceEntriesNothingToDo);
+ FRIEND_TEST(WiFiProviderTest, StringListToFrequencyMap);
typedef std::map<const WiFiEndpoint *, WiFiServiceRefPtr> EndpointServiceMap;
@@ -94,6 +109,9 @@
static const char kManagerErrorSSIDRequired[];
static const char kManagerErrorUnsupportedSecurityMode[];
static const char kManagerErrorUnsupportedServiceMode[];
+ static const char kFrequencyDelimiter[];
+ static const char kStorageId[];
+ static const char kStorageFrequencies[];
// Add a service to the service_ vector and register it with the Manager.
WiFiServiceRefPtr AddService(const std::vector<uint8_t> &ssid,
@@ -123,6 +141,18 @@
bool *hidden_ssid,
Error *error);
+ // Converts frequency profile information from a list of strings of the form
+ // "frequency:connections" to a form consistent with
+ // |connect_count_by_frequency_|
+ static void StringListToFrequencyMap(const std::vector<std::string> &strings,
+ ConnectFrequencyMap *numbers);
+
+ // Converts frequency profile information from a form consistent with
+ // |connect_count_by_frequency_| to a list of strings of the form
+ // "frequency:connections"
+ static void FrequencyMapToStringList(const ConnectFrequencyMap &numbers,
+ std::vector<std::string> *strings);
+
ControlInterface *control_interface_;
EventDispatcher *dispatcher_;
Metrics *metrics_;
@@ -133,9 +163,17 @@
bool running_;
+ // Map of frequencies at which we've connected and the number of times a
+ // successful connection has been made at that frequency. Absent frequencies
+ // have not had a successful connection.
+ ConnectFrequencyMap connect_count_by_frequency_;
+
+ // Count of successful wifi connections we've made.
+ int64_t total_frequency_connections_;
+
DISALLOW_COPY_AND_ASSIGN(WiFiProvider);
};
} // namespace shill
-#endif // SHILL_WIFI_PROVIDER_
+#endif // SHILL_WIFI_PROVIDER_H_
diff --git a/wifi_provider_unittest.cc b/wifi_provider_unittest.cc
index cfad87d..8ff8f26 100644
--- a/wifi_provider_unittest.cc
+++ b/wifi_provider_unittest.cc
@@ -30,6 +30,7 @@
using std::string;
using std::vector;
using ::testing::_;
+using ::testing::ContainerEq;
using ::testing::Mock;
using ::testing::NiceMock;
using ::testing::Return;
@@ -62,8 +63,8 @@
provider_.CreateServicesFromProfile(profile_);
}
- void FixupServiceEntries(bool is_default_profile) {
- provider_.FixupServiceEntries(&storage_, is_default_profile);
+ void LoadAndFixupServiceEntries(bool is_default_profile) {
+ provider_.LoadAndFixupServiceEntries(&storage_, is_default_profile);
}
const vector<WiFiServiceRefPtr> GetServices() {
@@ -195,6 +196,45 @@
const WiFiEndpointConstRefPtr &endpoint) {
provider_.service_by_endpoint_[endpoint] = service;
}
+
+ void BuildFreqCountStrings(vector<string> *strings) {
+ // NOTE: These strings match the frequencies in |BuildFreqCountMap|. They
+ // are also provided, here, in sorted order to match the frequency map
+ // (iterators for which will provide them in frequency-sorted order).
+ static const char *kStrings[] = {
+ "5180:14", "5240:16", "5745:7", "5765:4", "5785:14", "5805:5"
+ };
+ if (!strings) {
+ LOG(ERROR) << "NULL |strings|.";
+ return;
+ }
+ for (size_t i = 0; i < arraysize(kStrings); ++i) {
+ (*strings).push_back(kStrings[i]);
+ }
+ }
+
+ void BuildFreqCountMap(WiFiProvider::ConnectFrequencyMap *frequencies) {
+ // NOTE: These structures match the strings in |BuildFreqCountStrings|.
+ static const struct FreqCount {
+ uint16 freq;
+ int64 count;
+ } kConnectFreq[] = {
+ {5180, 14},
+ {5240, 16},
+ {5745, 7},
+ {5765, 4},
+ {5785, 14},
+ {5805, 5}
+ };
+ if (!frequencies) {
+ LOG(ERROR) << "NULL |frequencies|.";
+ return;
+ }
+ for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kConnectFreq); ++i) {
+ (*frequencies)[kConnectFreq[i].freq] = kConnectFreq[i].count;
+ }
+ }
+
NiceMockControl control_;
MockEventDispatcher dispatcher_;
MockMetrics metrics_;
@@ -957,9 +997,9 @@
Mock::VerifyAndClearExpectations(&manager_);
}
-TEST_F(WiFiProviderTest, FixupServiceEntries) {
- // We test FixupServiceEntries indirectly since it calls a static method
- // in WiFiService.
+TEST_F(WiFiProviderTest, LoadAndFixupServiceEntries) {
+ // We test LoadAndFixupServiceEntries indirectly since it calls a static
+ // method in WiFiService.
EXPECT_CALL(metrics_, SendEnumToUMA(
"Network.Shill.Wifi.ServiceFixupEntries",
Metrics::kMetricServiceFixupDefaultProfile,
@@ -977,7 +1017,10 @@
set<string> groups;
groups.insert(kGroupId);
EXPECT_CALL(storage_, GetGroups()).WillRepeatedly(Return(groups));
- FixupServiceEntries(true);
+ EXPECT_CALL(storage_, GetStringList(WiFiProvider::kStorageId,
+ WiFiProvider::kStorageFrequencies, _))
+ .WillOnce(Return(true));
+ LoadAndFixupServiceEntries(true);
Mock::VerifyAndClearExpectations(&metrics_);
EXPECT_CALL(metrics_, SendEnumToUMA(
@@ -985,10 +1028,11 @@
Metrics::kMetricServiceFixupUserProfile,
Metrics::kMetricServiceFixupMax)).Times(1);
EXPECT_CALL(storage_, Flush()).Times(1);
- FixupServiceEntries(false);
+ EXPECT_CALL(storage_, GetStringList(_, _, _)).Times(0);
+ LoadAndFixupServiceEntries(false);
}
-TEST_F(WiFiProviderTest, FixupServiceEntriesNothingToDo) {
+TEST_F(WiFiProviderTest, LoadAndFixupServiceEntriesNothingToDo) {
EXPECT_CALL(metrics_, SendEnumToUMA(_, _, _)).Times(0);
EXPECT_CALL(storage_, Flush()).Times(0);
const string kGroupId =
@@ -1001,7 +1045,10 @@
set<string> groups;
groups.insert(kGroupId);
EXPECT_CALL(storage_, GetGroups()).WillOnce(Return(groups));
- FixupServiceEntries(true);
+ EXPECT_CALL(storage_, GetStringList(WiFiProvider::kStorageId,
+ WiFiProvider::kStorageFrequencies, _))
+ .WillOnce(Return(true));
+ LoadAndFixupServiceEntries(true);
}
TEST_F(WiFiProviderTest, GetHiddenSSIDList) {
@@ -1054,4 +1101,26 @@
EXPECT_TRUE(ssid_list[1] == ssid4);
}
+TEST_F(WiFiProviderTest, StringListToFrequencyMap) {
+ vector<string> strings;
+ BuildFreqCountStrings(&strings);
+ WiFiProvider::ConnectFrequencyMap frequencies_result;
+ WiFiProvider::StringListToFrequencyMap(strings, &frequencies_result);
+
+ WiFiProvider::ConnectFrequencyMap frequencies_expect;
+ BuildFreqCountMap(&frequencies_expect);
+ EXPECT_THAT(frequencies_result, ContainerEq(frequencies_expect));
+}
+
+TEST_F(WiFiProviderTest, FrequencyMapToStringList) {
+ WiFiProvider::ConnectFrequencyMap frequencies;
+ BuildFreqCountMap(&frequencies);
+ vector<string> strings_result;
+ WiFiProvider::FrequencyMapToStringList(frequencies, &strings_result);
+
+ vector<string> strings_expect;
+ BuildFreqCountStrings(&strings_expect);
+ EXPECT_THAT(strings_result, ContainerEq(strings_expect));
+}
+
} // namespace shill