summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSohani Rao2017-07-27 12:52:29 -0500
committerSohani Rao2017-08-11 19:39:55 -0500
commit8043a56883bfb32333b4f007ff52bb880a75cd70 (patch)
tree997750d6278d88e460819624a4917370c465e35c
parent69f61f7fd15d44dd32a107d850f05f0eb933ab2c (diff)
downloadsystem-connectivity-wificond-8043a56883bfb32333b4f007ff52bb880a75cd70.tar.gz
system-connectivity-wificond-8043a56883bfb32333b4f007ff52bb880a75cd70.tar.xz
system-connectivity-wificond-8043a56883bfb32333b4f007ff52bb880a75cd70.zip
Wificond: Address bugs in handling Offload scans
This CL addresses the following issues in handling of Offload HAL scan results from the Offload HAL service - Make cached scan results member variable a pointer so that the memory for the vector can be allocated on the heap - When an Async Error occurs in the Offload HAL service, switch to performing PNO scans over netlink instead of letting it decide again. - OffloadScanUtils convertToNativeScanResults() now needs to take in a pointer to the vector that will store the scan results for retrieval. - Populate tsf field fo the scanResult so that it doesn't get filtered out by the framework - Add logging to scan stats Bug: 63148974 Test: Unit tests, on-device testing for ensuring we connect to an available access point from screen off disconnected mode. Change-Id: Ida507d857faa8ea6dbee362cf0116f8ca858963f
-rw-r--r--scanning/offload/offload_scan_manager.cpp14
-rw-r--r--scanning/offload/offload_scan_manager.h2
-rw-r--r--scanning/offload/offload_scan_utils.cpp15
-rw-r--r--scanning/offload/offload_scan_utils.h6
-rw-r--r--scanning/offload/scan_stats.cpp15
-rw-r--r--scanning/offload/scan_stats.h1
-rw-r--r--scanning/scanner_impl.cpp17
-rw-r--r--tests/offload_scan_utils_test.cpp6
8 files changed, 50 insertions, 26 deletions
diff --git a/scanning/offload/offload_scan_manager.cpp b/scanning/offload/offload_scan_manager.cpp
index 420c2fa..2592aee 100644
--- a/scanning/offload/offload_scan_manager.cpp
+++ b/scanning/offload/offload_scan_manager.cpp
@@ -76,6 +76,7 @@ OffloadScanManager::OffloadScanManager(
76 wifi_offload_callback_(nullptr), 76 wifi_offload_callback_(nullptr),
77 death_recipient_(nullptr), 77 death_recipient_(nullptr),
78 offload_status_(OffloadScanManager::kError), 78 offload_status_(OffloadScanManager::kError),
79 cached_scan_results_(new std::vector<NativeScanResult>()),
79 service_available_(false), 80 service_available_(false),
80 offload_service_utils_(utils), 81 offload_service_utils_(utils),
81 offload_callback_handlers_(new OffloadCallbackHandlersImpl(this)), 82 offload_callback_handlers_(new OffloadCallbackHandlersImpl(this)),
@@ -205,8 +206,6 @@ bool OffloadScanManager::startScan(
205 } 206 }
206 207
207 *reason_code = OffloadScanManager::kNone; 208 *reason_code = OffloadScanManager::kNone;
208 /* Clear up the scan cache every time a new scan is requested */
209 cached_scan_results_.clear();
210 return true; 209 return true;
211} 210}
212 211
@@ -240,7 +239,7 @@ OffloadScanManager::StatusCode OffloadScanManager::getOffloadStatus() const {
240 239
241bool OffloadScanManager::getScanResults( 240bool OffloadScanManager::getScanResults(
242 std::vector<NativeScanResult>* out_scan_results) { 241 std::vector<NativeScanResult>* out_scan_results) {
243 for (auto scan_result : cached_scan_results_) { 242 for (auto scan_result : *cached_scan_results_) {
244 out_scan_results->push_back(scan_result); 243 out_scan_results->push_back(scan_result);
245 } 244 }
246 return true; 245 return true;
@@ -262,12 +261,17 @@ OffloadScanManager::~OffloadScanManager() {
262 if (wifi_offload_hal_ != nullptr) { 261 if (wifi_offload_hal_ != nullptr) {
263 wifi_offload_hal_->unlinkToDeath(death_recipient_); 262 wifi_offload_hal_->unlinkToDeath(death_recipient_);
264 } 263 }
264 delete cached_scan_results_;
265} 265}
266 266
267void OffloadScanManager::ReportScanResults( 267void OffloadScanManager::ReportScanResults(
268 const vector<ScanResult>& scanResult) { 268 const vector<ScanResult>& scanResult) {
269 cached_scan_results_ = 269 cached_scan_results_->clear();
270 OffloadScanUtils::convertToNativeScanResults(scanResult); 270 if (!OffloadScanUtils::convertToNativeScanResults(scanResult,
271 cached_scan_results_)) {
272 LOG(WARNING) << "Unable to convert scan results to native format";
273 return;
274 }
271 if (event_callback_ != nullptr) { 275 if (event_callback_ != nullptr) {
272 event_callback_->OnOffloadScanResult(); 276 event_callback_->OnOffloadScanResult();
273 } else { 277 } else {
diff --git a/scanning/offload/offload_scan_manager.h b/scanning/offload/offload_scan_manager.h
index 46f1bad..7b23d8e 100644
--- a/scanning/offload/offload_scan_manager.h
+++ b/scanning/offload/offload_scan_manager.h
@@ -144,7 +144,7 @@ class OffloadScanManager {
144 android::sp<OffloadCallback> wifi_offload_callback_; 144 android::sp<OffloadCallback> wifi_offload_callback_;
145 android::sp<OffloadDeathRecipient> death_recipient_; 145 android::sp<OffloadDeathRecipient> death_recipient_;
146 StatusCode offload_status_; 146 StatusCode offload_status_;
147 std::vector<::com::android::server::wifi::wificond::NativeScanResult> 147 std::vector<::com::android::server::wifi::wificond::NativeScanResult>*
148 cached_scan_results_; 148 cached_scan_results_;
149 bool service_available_; 149 bool service_available_;
150 150
diff --git a/scanning/offload/offload_scan_utils.cpp b/scanning/offload/offload_scan_utils.cpp
index 598fd8e..1446f76 100644
--- a/scanning/offload/offload_scan_utils.cpp
+++ b/scanning/offload/offload_scan_utils.cpp
@@ -16,6 +16,7 @@
16#include "wificond/scanning/offload/offload_scan_utils.h" 16#include "wificond/scanning/offload/offload_scan_utils.h"
17 17
18#include <android-base/logging.h> 18#include <android-base/logging.h>
19#include <utils/Timers.h>
19 20
20#include "wificond/scanning/offload/scan_stats.h" 21#include "wificond/scanning/offload/scan_stats.h"
21#include "wificond/scanning/scan_result.h" 22#include "wificond/scanning/scan_result.h"
@@ -33,10 +34,10 @@ using std::vector;
33namespace android { 34namespace android {
34namespace wificond { 35namespace wificond {
35 36
36vector<NativeScanResult> OffloadScanUtils::convertToNativeScanResults( 37bool OffloadScanUtils::convertToNativeScanResults(
37 const vector<ScanResult>& scan_result) { 38 const vector<ScanResult>& scan_result,
38 vector<NativeScanResult> native_scan_result; 39 vector<NativeScanResult>* native_scan_result) {
39 native_scan_result.reserve(scan_result.size()); 40 if (native_scan_result == nullptr) return false;
40 for (size_t i = 0; i < scan_result.size(); i++) { 41 for (size_t i = 0; i < scan_result.size(); i++) {
41 NativeScanResult single_scan_result; 42 NativeScanResult single_scan_result;
42 single_scan_result.ssid.assign(scan_result[i].networkInfo.ssid.begin(), 43 single_scan_result.ssid.assign(scan_result[i].networkInfo.ssid.begin(),
@@ -46,12 +47,12 @@ vector<NativeScanResult> OffloadScanUtils::convertToNativeScanResults(
46 } 47 }
47 single_scan_result.frequency = scan_result[i].frequency; 48 single_scan_result.frequency = scan_result[i].frequency;
48 single_scan_result.signal_mbm = scan_result[i].rssi; 49 single_scan_result.signal_mbm = scan_result[i].rssi;
49 single_scan_result.tsf = scan_result[i].tsf; 50 single_scan_result.tsf = systemTime(SYSTEM_TIME_MONOTONIC) / 1000;
50 single_scan_result.capability = scan_result[i].capability; 51 single_scan_result.capability = scan_result[i].capability;
51 single_scan_result.associated = false; 52 single_scan_result.associated = false;
52 native_scan_result.emplace_back(single_scan_result); 53 native_scan_result->push_back(std::move(single_scan_result));
53 } 54 }
54 return native_scan_result; 55 return true;
55} 56}
56 57
57ScanParam OffloadScanUtils::createScanParam( 58ScanParam OffloadScanUtils::createScanParam(
diff --git a/scanning/offload/offload_scan_utils.h b/scanning/offload/offload_scan_utils.h
index ce4afa7..22ba8ea 100644
--- a/scanning/offload/offload_scan_utils.h
+++ b/scanning/offload/offload_scan_utils.h
@@ -42,9 +42,9 @@ namespace wificond {
42// Provides utility methods for Offload Scan Manager 42// Provides utility methods for Offload Scan Manager
43class OffloadScanUtils { 43class OffloadScanUtils {
44 public: 44 public:
45 static std::vector<::com::android::server::wifi::wificond::NativeScanResult> 45 static bool convertToNativeScanResults(
46 convertToNativeScanResults( 46 const std::vector<android::hardware::wifi::offload::V1_0::ScanResult>&,
47 const std::vector<android::hardware::wifi::offload::V1_0::ScanResult>&); 47 std::vector<::com::android::server::wifi::wificond::NativeScanResult>*);
48 static android::hardware::wifi::offload::V1_0::ScanParam createScanParam( 48 static android::hardware::wifi::offload::V1_0::ScanParam createScanParam(
49 const std::vector<std::vector<uint8_t>>& ssid_list, 49 const std::vector<std::vector<uint8_t>>& ssid_list,
50 const std::vector<uint32_t>& frequency_list, uint32_t scan_interval_ms); 50 const std::vector<uint32_t>& frequency_list, uint32_t scan_interval_ms);
diff --git a/scanning/offload/scan_stats.cpp b/scanning/offload/scan_stats.cpp
index 4a963cb..bd5c793 100644
--- a/scanning/offload/scan_stats.cpp
+++ b/scanning/offload/scan_stats.cpp
@@ -46,6 +46,7 @@ NativeScanStats::NativeScanStats()
46 : num_scans_requested_by_wifi_(0), 46 : num_scans_requested_by_wifi_(0),
47 num_scans_serviced_by_wifi_(0), 47 num_scans_serviced_by_wifi_(0),
48 subscription_duration_ms_(0), 48 subscription_duration_ms_(0),
49 scan_duration_ms_(0),
49 num_channels_scanned_(0), 50 num_channels_scanned_(0),
50 time_stamp_(0) {} 51 time_stamp_(0) {}
51 52
@@ -87,6 +88,20 @@ status_t NativeScanStats::readFromParcel(const ::android::Parcel* parcel) {
87 return ::android::OK; 88 return ::android::OK;
88} 89}
89 90
91void NativeScanStats::DebugLog() {
92 LOG(INFO) << "num_scans_requested_by_wifi=" << num_scans_requested_by_wifi_;
93 LOG(INFO) << "num_scans_serviced_by_wifi=" << num_scans_serviced_by_wifi_;
94 LOG(INFO) << "subscription_duration=" << subscription_duration_ms_;
95 LOG(INFO) << "scan_duration_ms_=" << scan_duration_ms_;
96 LOG(INFO) << "num_channels_scanned=" << num_channels_scanned_;
97 for (size_t i = 0; i < histogram_channels_.size(); i++) {
98 if (histogram_channels_[i] > 0) {
99 LOG(INFO) << "Channel=" << i << " ScannedTimes="
100 << static_cast<uint32_t>(histogram_channels_[i]);
101 }
102 }
103}
104
90} // namespace wificond 105} // namespace wificond
91} // namespace wifi 106} // namespace wifi
92} // namespace server 107} // namespace server
diff --git a/scanning/offload/scan_stats.h b/scanning/offload/scan_stats.h
index eb1d16a..05220b9 100644
--- a/scanning/offload/scan_stats.h
+++ b/scanning/offload/scan_stats.h
@@ -40,6 +40,7 @@ class NativeScanStats : public ::android::Parcelable {
40 bool operator==(const NativeScanStats&) const; 40 bool operator==(const NativeScanStats&) const;
41 ::android::status_t writeToParcel(::android::Parcel* parcel) const override; 41 ::android::status_t writeToParcel(::android::Parcel* parcel) const override;
42 ::android::status_t readFromParcel(const ::android::Parcel* parcel) override; 42 ::android::status_t readFromParcel(const ::android::Parcel* parcel) override;
43 void DebugLog();
43 44
44 uint32_t num_scans_requested_by_wifi_; 45 uint32_t num_scans_requested_by_wifi_;
45 uint32_t num_scans_serviced_by_wifi_; 46 uint32_t num_scans_serviced_by_wifi_;
diff --git a/scanning/scanner_impl.cpp b/scanning/scanner_impl.cpp
index 5fa7093..b14d788 100644
--- a/scanning/scanner_impl.cpp
+++ b/scanning/scanner_impl.cpp
@@ -514,7 +514,6 @@ void ScannerImpl::OnOffloadScanResult() {
514 514
515void ScannerImpl::OnOffloadError( 515void ScannerImpl::OnOffloadError(
516 OffloadScanCallbackInterface::AsyncErrorReason error_code) { 516 OffloadScanCallbackInterface::AsyncErrorReason error_code) {
517 bool success;
518 if (!pno_scan_running_over_offload_) { 517 if (!pno_scan_running_over_offload_) {
519 // Ignore irrelevant error notifications 518 // Ignore irrelevant error notifications
520 LOG(WARNING) << "Offload HAL Async Error occured but Offload HAL is not " 519 LOG(WARNING) << "Offload HAL Async Error occured but Offload HAL is not "
@@ -522,19 +521,16 @@ void ScannerImpl::OnOffloadError(
522 return; 521 return;
523 } 522 }
524 LOG(ERROR) << "Offload Service Async Failure error_code=" << error_code; 523 LOG(ERROR) << "Offload Service Async Failure error_code=" << error_code;
525 // Stop scans over Offload HAL and request them over netlink
526 stopPnoScan(&success);
527 if (success) {
528 LOG(INFO) << "Pno scans stopped";
529 }
530 switch (error_code) { 524 switch (error_code) {
531 case OffloadScanCallbackInterface::AsyncErrorReason::BINDER_DEATH: 525 case OffloadScanCallbackInterface::AsyncErrorReason::BINDER_DEATH:
526 LOG(ERROR) << "Binder death";
532 if (pno_scan_event_handler_ != nullptr) { 527 if (pno_scan_event_handler_ != nullptr) {
533 pno_scan_event_handler_->OnPnoScanOverOffloadFailed( 528 pno_scan_event_handler_->OnPnoScanOverOffloadFailed(
534 net::wifi::IPnoScanEvent::PNO_SCAN_OVER_OFFLOAD_BINDER_FAILURE); 529 net::wifi::IPnoScanEvent::PNO_SCAN_OVER_OFFLOAD_BINDER_FAILURE);
535 } 530 }
536 break; 531 break;
537 case OffloadScanCallbackInterface::AsyncErrorReason::REMOTE_FAILURE: 532 case OffloadScanCallbackInterface::AsyncErrorReason::REMOTE_FAILURE:
533 LOG(ERROR) << "Remote failure";
538 if (pno_scan_event_handler_ != nullptr) { 534 if (pno_scan_event_handler_ != nullptr) {
539 pno_scan_event_handler_->OnPnoScanOverOffloadFailed( 535 pno_scan_event_handler_->OnPnoScanOverOffloadFailed(
540 net::wifi::IPnoScanEvent::PNO_SCAN_OVER_OFFLOAD_REMOTE_FAILURE); 536 net::wifi::IPnoScanEvent::PNO_SCAN_OVER_OFFLOAD_REMOTE_FAILURE);
@@ -544,7 +540,14 @@ void ScannerImpl::OnOffloadError(
544 LOG(WARNING) << "Invalid Error code"; 540 LOG(WARNING) << "Invalid Error code";
545 break; 541 break;
546 } 542 }
547 startPnoScan(pno_settings_, &success); 543 bool success;
544 // Stop scans over Offload HAL and request them over netlink
545 stopPnoScan(&success);
546 if (success) {
547 LOG(INFO) << "Pno scans stopped";
548 }
549 // Restart PNO scans over netlink interface
550 success = StartPnoScanDefault(pno_settings_);
548 if (success) { 551 if (success) {
549 LOG(INFO) << "Pno scans restarted"; 552 LOG(INFO) << "Pno scans restarted";
550 } 553 }
diff --git a/tests/offload_scan_utils_test.cpp b/tests/offload_scan_utils_test.cpp
index d75143a..15c7e86 100644
--- a/tests/offload_scan_utils_test.cpp
+++ b/tests/offload_scan_utils_test.cpp
@@ -50,13 +50,13 @@ class OffloadScanUtilsTest : public ::testing::Test {
50}; 50};
51 51
52TEST_F(OffloadScanUtilsTest, verifyConversion) { 52TEST_F(OffloadScanUtilsTest, verifyConversion) {
53 vector<NativeScanResult> native_scan_results = 53 vector<NativeScanResult> native_scan_results;
54 OffloadScanUtils::convertToNativeScanResults(dummy_scan_results_); 54 EXPECT_TRUE(OffloadScanUtils::convertToNativeScanResults(
55 dummy_scan_results_, &native_scan_results));
55 EXPECT_EQ(native_scan_results.size(), dummy_scan_results_.size()); 56 EXPECT_EQ(native_scan_results.size(), dummy_scan_results_.size());
56 for (size_t i = 0; i < native_scan_results.size(); i++) { 57 for (size_t i = 0; i < native_scan_results.size(); i++) {
57 EXPECT_EQ(native_scan_results[i].frequency, 58 EXPECT_EQ(native_scan_results[i].frequency,
58 dummy_scan_results_[i].frequency); 59 dummy_scan_results_[i].frequency);
59 EXPECT_EQ(native_scan_results[i].tsf, dummy_scan_results_[i].tsf);
60 EXPECT_EQ(native_scan_results[i].signal_mbm, dummy_scan_results_[i].rssi); 60 EXPECT_EQ(native_scan_results[i].signal_mbm, dummy_scan_results_[i].rssi);
61 EXPECT_EQ(native_scan_results[i].ssid.size(), 61 EXPECT_EQ(native_scan_results[i].ssid.size(),
62 dummy_scan_results_[i].networkInfo.ssid.size()); 62 dummy_scan_results_[i].networkInfo.ssid.size());