shill: WiFi: Add metric for Signal Strength
Add a metric that gives the signal strength at time of
successful association.
BUG=chromium-os:34782
TEST=Run unit tests (with edits for new metric), run on device,
connect to WiFi network, browse to chrome://histograms
Change-Id: Idf0754fd5adf7f4931a0a647940477922335d196
Reviewed-on: https://gerrit.chromium.org/gerrit/34095
Reviewed-by: Thieu Le <thieule@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>
Reviewed-by: mukesh agrawal <quiche@chromium.org>
Commit-Ready: Paul Stewart <pstew@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
diff --git a/metrics.cc b/metrics.cc
index 1eee4b7..578f596 100644
--- a/metrics.cc
+++ b/metrics.cc
@@ -43,6 +43,11 @@
const char Metrics::kMetricNetworkServiceErrors[] =
"Network.Shill.ServiceErrors";
const int Metrics::kMetricNetworkServiceErrorsMax = Service::kFailureMax;
+const char Metrics::kMetricNetworkSignalStrength[] =
+ "Network.Shill.%s.SignalStrength";
+const int Metrics::kMetricNetworkSignalStrengthMax = 200;
+const int Metrics::kMetricNetworkSignalStrengthMin = 0;
+const int Metrics::kMetricNetworkSignalStrengthNumBuckets = 40;
const char Metrics::kMetricTimeOnlineSeconds[] = "Network.Shill.%s.TimeOnline";
const int Metrics::kMetricTimeOnlineSecondsMax = 8 * 60 * 60; // 8 hours
diff --git a/metrics.h b/metrics.h
index 5383403..d0d070c 100644
--- a/metrics.h
+++ b/metrics.h
@@ -157,6 +157,10 @@
static const int kMetricNetworkSecurityMax;
static const char kMetricNetworkServiceErrors[];
static const int kMetricNetworkServiceErrorsMax;
+ static const char kMetricNetworkSignalStrength[];
+ static const int kMetricNetworkSignalStrengthMin;
+ static const int kMetricNetworkSignalStrengthMax;
+ static const int kMetricNetworkSignalStrengthNumBuckets;
static const char kMetricTimeOnlineSeconds[];
static const int kMetricTimeOnlineSecondsMax;
static const int kMetricTimeOnlineSecondsMin;
diff --git a/metrics_unittest.cc b/metrics_unittest.cc
index 76c5c94..864b121 100644
--- a/metrics_unittest.cc
+++ b/metrics_unittest.cc
@@ -57,7 +57,8 @@
protected:
void ExpectCommonPostReady(Metrics::WiFiChannel channel,
Metrics::WiFiNetworkPhyMode mode,
- Metrics::WiFiSecurity security) {
+ Metrics::WiFiSecurity security,
+ int signal_strength) {
EXPECT_CALL(library_, SendEnumToUMA("Network.Shill.Wifi.Channel",
channel,
Metrics::kMetricNetworkChannelMax));
@@ -67,6 +68,12 @@
EXPECT_CALL(library_, SendEnumToUMA("Network.Shill.Wifi.Security",
security,
Metrics::kWiFiSecurityMax));
+ EXPECT_CALL(library_,
+ SendToUMA("Network.Shill.Wifi.SignalStrength",
+ signal_strength,
+ Metrics::kMetricNetworkSignalStrengthMin,
+ Metrics::kMetricNetworkSignalStrengthMax,
+ Metrics::kMetricNetworkSignalStrengthNumBuckets));
}
Metrics metrics_; // This must be destroyed after service_ and wifi_service_
@@ -137,14 +144,17 @@
new chromeos_metrics::TimerMock;
metrics_.set_time_resume_to_ready_timer(mock_time_resume_to_ready_timer);
+ const int kStrength = -42;
ExpectCommonPostReady(Metrics::kWiFiChannel2412,
Metrics::kWiFiNetworkPhyMode11a,
- Metrics::kWiFiSecurityWep);
+ Metrics::kWiFiSecurityWep,
+ -kStrength);
EXPECT_CALL(library_, SendToUMA("Network.Shill.Wifi.TimeResumeToReady",
_, _, _, _)).Times(0);
wifi_service_->frequency_ = 2412;
wifi_service_->physical_mode_ = Metrics::kWiFiNetworkPhyMode11a;
wifi_service_->security_ = flimflam::kSecurityWep;
+ wifi_service_->raw_signal_strength_ = kStrength;
metrics_.RegisterService(wifi_service_);
metrics_.NotifyServiceStateChanged(wifi_service_, Service::kStateConnected);
Mock::VerifyAndClearExpectations(&library_);
@@ -152,7 +162,8 @@
// Simulate a system suspend, resume and an AP reconnect.
ExpectCommonPostReady(Metrics::kWiFiChannel2412,
Metrics::kWiFiNetworkPhyMode11a,
- Metrics::kWiFiSecurityWep);
+ Metrics::kWiFiSecurityWep,
+ -kStrength);
EXPECT_CALL(library_, SendToUMA("Network.Shill.Wifi.TimeResumeToReady",
Ge(0),
Metrics::kTimerHistogramMillisecondsMin,
@@ -169,7 +180,8 @@
// Make sure subsequent connects do not count towards TimeResumeToReady.
ExpectCommonPostReady(Metrics::kWiFiChannel2412,
Metrics::kWiFiNetworkPhyMode11a,
- Metrics::kWiFiSecurityWep);
+ Metrics::kWiFiSecurityWep,
+ -kStrength);
EXPECT_CALL(library_, SendToUMA("Network.Shill.Wifi.TimeResumeToReady",
_, _, _, _)).Times(0);
metrics_.NotifyServiceStateChanged(wifi_service_, Service::kStateConnected);
diff --git a/wifi_service.cc b/wifi_service.cc
index 58a4a61..3735bcb 100644
--- a/wifi_service.cc
+++ b/wifi_service.cc
@@ -63,6 +63,7 @@
hidden_ssid_(hidden_ssid),
frequency_(0),
physical_mode_(0),
+ raw_signal_strength_(0),
wifi_(device),
ssid_(ssid),
nss_(NSS::GetInstance()) {
@@ -334,6 +335,17 @@
security_uma,
Metrics::kMetricNetworkSecurityMax);
+ // We invert the sign of the signal strength value, since UMA histograms
+ // cannot represent negative numbers (it stores them but cannot display
+ // them), and dBm values of interest start at 0 and go negative from there.
+ metrics()->SendToUMA(
+ metrics()->GetFullMetricName(Metrics::kMetricNetworkSignalStrength,
+ technology()),
+ -raw_signal_strength_,
+ Metrics::kMetricNetworkSignalStrengthMin,
+ Metrics::kMetricNetworkSignalStrengthMax,
+ Metrics::kMetricNetworkSignalStrengthNumBuckets);
+
if (time_resume_to_ready_milliseconds > 0) {
metrics()->SendToUMA(
metrics()->GetFullMetricName(
@@ -482,9 +494,12 @@
int16 signal = std::numeric_limits<int16>::min();
string bssid;
Stringmap vendor_information;
+ // Represent "unknown raw signal strength" as 0.
+ raw_signal_strength_ = 0;
if (representative_endpoint) {
frequency = representative_endpoint->frequency();
signal = representative_endpoint->signal_strength();
+ raw_signal_strength_ = signal;
bssid = representative_endpoint->bssid_string();
vendor_information = representative_endpoint->GetVendorInformation();
}
diff --git a/wifi_service.h b/wifi_service.h
index 6b82c65..732f279 100644
--- a/wifi_service.h
+++ b/wifi_service.h
@@ -186,6 +186,8 @@
// TODO(quiche): I noticed this is not hooked up to anything. In fact, it
// was undefined until now. crosbug.com/26490
uint16 physical_mode_;
+ // The raw dBm signal strength from the associated endpoint.
+ int16 raw_signal_strength_;
std::string hex_ssid_;
std::string storage_identifier_;
std::string bssid_;