From 8eb600660505b0b109faefae3f09a5ea49d8f0c4 Mon Sep 17 00:00:00 2001 From: Andrew Beltrano Date: Thu, 22 Feb 2024 23:10:36 +0000 Subject: [PATCH 1/9] Add IAccessPointController::SetOperationalState function. --- src/common/service/NetRemoteService.cxx | 22 ++++++++++++++++++- .../net/wifi/AccessPointOperationStatus.hxx | 8 +++++++ .../net/wifi/IAccessPointController.hxx | 9 ++++++++ .../wifi/core/AccessPointControllerLinux.cxx | 7 ++++++ .../net/wifi/AccessPointControllerLinux.hxx | 9 ++++++++ .../helpers/AccessPointControllerTest.cxx | 11 ++++++++++ .../wifi/test/AccessPointControllerTest.hxx | 18 +++++++++++---- .../net/wifi/test/AccessPointTest.hxx | 1 + 8 files changed, 80 insertions(+), 5 deletions(-) diff --git a/src/common/service/NetRemoteService.cxx b/src/common/service/NetRemoteService.cxx index dbf6ac12..d4cb2d76 100644 --- a/src/common/service/NetRemoteService.cxx +++ b/src/common/service/NetRemoteService.cxx @@ -269,8 +269,28 @@ NetRemoteService::WifiAccessPointDisable([[maybe_unused]] grpc::ServerContext* c { const NetRemoteWifiApiTrace traceMe{ request->accesspointid(), result->mutable_status() }; + + // Create an AP controller for the requested AP. + auto accessPointController = detail::TryGetAccessPointController(request, result, m_accessPointManager); + if (accessPointController == nullptr) { + return grpc::Status::OK; + } + + + bool isEnabled{ false }; + try { + isEnabled = accessPointController->GetIsEnabled(); + } catch (const AccessPointControllerException& apce) { + return HandleFailure(request, result, WifiAccessPointOperationStatusCode::WifiAccessPointOperationStatusCodeInternalError, std::format("Failed to get enabled state for access point {} ({})", request->accesspointid(), apce.what())); + } + WifiAccessPointOperationStatus status{}; - // TODO: Disable the access point. + if (isEnabled) { + // TODO: Disable the access point. + } else { + LOGI << std::format("Access point {} is already disabled", request->accesspointid()); + } + status.set_code(WifiAccessPointOperationStatusCode::WifiAccessPointOperationStatusCodeSucceeded); result->set_accesspointid(request->accesspointid()); diff --git a/src/common/wifi/core/include/microsoft/net/wifi/AccessPointOperationStatus.hxx b/src/common/wifi/core/include/microsoft/net/wifi/AccessPointOperationStatus.hxx index ec424b66..92e18221 100644 --- a/src/common/wifi/core/include/microsoft/net/wifi/AccessPointOperationStatus.hxx +++ b/src/common/wifi/core/include/microsoft/net/wifi/AccessPointOperationStatus.hxx @@ -6,6 +6,14 @@ namespace Microsoft::Net::Wifi { +/** + * @brief Operational state of an access point. + */ +enum class AccessPointOperationalState : bool { + Disabled = false, + Enabled = true, +}; + /** * @brief High-level status reported by various access point operations. */ diff --git a/src/common/wifi/core/include/microsoft/net/wifi/IAccessPointController.hxx b/src/common/wifi/core/include/microsoft/net/wifi/IAccessPointController.hxx index 15a7eda4..0e92e3f8 100644 --- a/src/common/wifi/core/include/microsoft/net/wifi/IAccessPointController.hxx +++ b/src/common/wifi/core/include/microsoft/net/wifi/IAccessPointController.hxx @@ -76,6 +76,15 @@ struct IAccessPointController virtual Ieee80211AccessPointCapabilities GetCapabilities() = 0; + /** + * @brief Set the operational state of the access point. + * + * @param operationalState The desired operational state. + * @return AccessPointOperationStatus + */ + virtual AccessPointOperationStatus + SetOperationalState(AccessPointOperationalState operationalState) = 0; + /** * @brief Set the Ieee80211 protocol of the access point. * diff --git a/src/linux/wifi/core/AccessPointControllerLinux.cxx b/src/linux/wifi/core/AccessPointControllerLinux.cxx index 62de7a80..4eecd386 100644 --- a/src/linux/wifi/core/AccessPointControllerLinux.cxx +++ b/src/linux/wifi/core/AccessPointControllerLinux.cxx @@ -190,6 +190,13 @@ AccessPointControllerLinux::GetIsEnabled() return isEnabled; } +AccessPointOperationStatus +AccessPointControllerLinux::SetOperationalState([[maybe_unused]] AccessPointOperationalState operationalState) +{ + // TODO: Implement this method. + return AccessPointOperationStatus(AccessPointOperationStatusCode::OperationNotSupported); +} + bool AccessPointControllerLinux::SetProtocol(Microsoft::Net::Wifi::Ieee80211Protocol ieeeProtocol) { diff --git a/src/linux/wifi/core/include/microsoft/net/wifi/AccessPointControllerLinux.hxx b/src/linux/wifi/core/include/microsoft/net/wifi/AccessPointControllerLinux.hxx index 3ea6ea44..6dddc1ee 100644 --- a/src/linux/wifi/core/include/microsoft/net/wifi/AccessPointControllerLinux.hxx +++ b/src/linux/wifi/core/include/microsoft/net/wifi/AccessPointControllerLinux.hxx @@ -59,6 +59,15 @@ struct AccessPointControllerLinux : Ieee80211AccessPointCapabilities GetCapabilities() override; + /** + * @brief Set the operational state of the access point. + * + * @param operationalState The desired operational state. + * @return AccessPointOperationStatus + */ + AccessPointOperationStatus + SetOperationalState(AccessPointOperationalState operationalState) override; + /** * @brief Set the Ieee80211 protocol. * diff --git a/tests/unit/wifi/helpers/AccessPointControllerTest.cxx b/tests/unit/wifi/helpers/AccessPointControllerTest.cxx index b1594b10..a1f15099 100644 --- a/tests/unit/wifi/helpers/AccessPointControllerTest.cxx +++ b/tests/unit/wifi/helpers/AccessPointControllerTest.cxx @@ -54,6 +54,17 @@ AccessPointControllerTest::GetCapabilities() return AccessPoint->Capabilities; } +AccessPointOperationStatus +AccessPointControllerTest::SetOperationalState(AccessPointOperationalState operationalState) +{ + if (AccessPoint == nullptr) { + throw std::runtime_error("AccessPointControllerTest::SetOperationalState called with null AccessPoint"); + } + + AccessPoint->OperationalState = operationalState; + return AccessPointOperationStatus::MakeSucceeded(); +} + bool AccessPointControllerTest::SetProtocol(Ieee80211Protocol ieeeProtocol) { diff --git a/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointControllerTest.hxx b/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointControllerTest.hxx index b47fa305..5217a37b 100644 --- a/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointControllerTest.hxx +++ b/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointControllerTest.hxx @@ -19,9 +19,10 @@ struct AccessPointTest; * @brief IAccessPointController implementation for testing purposes. * * This implementation takes an AccessPointTest object which it uses to implement the IAccessPointController interface. - * The owner of this class must ensure that the passes AccessPointTest* remains valid for the lifetime of this object. + * The owner of this class must ensure that the passed AccessPointTest* remains valid for the lifetime of this object. */ -struct AccessPointControllerTest final : public Microsoft::Net::Wifi::IAccessPointController +struct AccessPointControllerTest final : + public Microsoft::Net::Wifi::IAccessPointController { AccessPointTest *AccessPoint{ nullptr }; @@ -71,6 +72,15 @@ struct AccessPointControllerTest final : public Microsoft::Net::Wifi::IAccessPoi Ieee80211AccessPointCapabilities GetCapabilities() override; + /** + * @brief Set the operational state of the access point. + * + * @param operationalState The desired operational state. + * @return AccessPointOperationStatus + */ + AccessPointOperationStatus + SetOperationalState(AccessPointOperationalState operationalState) override; + /** * @brief Set the Ieee80211 protocol of the access point. * @@ -93,9 +103,9 @@ struct AccessPointControllerTest final : public Microsoft::Net::Wifi::IAccessPoi /** * @brief Set the SSID of the access point. - * + * * @param ssid The SSID to be set. - * @return AccessPointOperationStatus + * @return AccessPointOperationStatus */ AccessPointOperationStatus SetSssid(std::string_view ssid) override; diff --git a/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointTest.hxx b/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointTest.hxx index ea9e4f06..c9b12369 100644 --- a/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointTest.hxx +++ b/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointTest.hxx @@ -29,6 +29,7 @@ struct AccessPointTest final : Microsoft::Net::Wifi::Ieee80211Protocol Protocol; std::vector FrequencyBands; std::string Ssid; + AccessPointOperationalState OperationalState{ AccessPointOperationalState::Disabled }; /** * @brief Construct a new AccessPointTest object with the given interface name and default capabilities. From 169cb0c718d3405331c540d53c8adf01cd189f7c Mon Sep 17 00:00:00 2001 From: Andrew Beltrano Date: Thu, 22 Feb 2024 23:25:17 +0000 Subject: [PATCH 2/9] Implement WifiAccessPointDisable. --- src/common/service/NetRemoteService.cxx | 35 +++++++++++++++++++++---- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/src/common/service/NetRemoteService.cxx b/src/common/service/NetRemoteService.cxx index d4cb2d76..b5d794e2 100644 --- a/src/common/service/NetRemoteService.cxx +++ b/src/common/service/NetRemoteService.cxx @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -63,6 +64,28 @@ HandleFailure(RequestT& request, ResultT& result, WifiAccessPointOperationStatus return returnValue; } +/** + * @brief Wrapper for HandleFailure that converts a Dot11AccessPointOperationStatusCode to a WifiAccessPointOperationStatusCode. + * + * @tparam RequestT The request type. This must contain an access point id (trait). + * @tparam ResultT The result type. This must contain an access point id and a status (traits). + * @return ReturnT The type of the return value. Defaults to grpc::Status with a value of grpc::OK. + * @param request A reference to the request. + * @param result A reference to the result. + * @param code The error code to set in the result message. + * @param message The error message to set in the result message. + * @param returnValue The value to return from the function. + */ +template < + typename RequestT, + typename ResultT, + typename ReturnT = grpc::Status> +ReturnT +HandleFailure(RequestT& request, ResultT& result, AccessPointOperationStatusCode code, std::string_view message, ReturnT returnValue = {}) +{ + return HandleFailure(request, result, ToDot11AccessPointOperationStatusCode(code), message, returnValue); +} + /** * @brief Attempt to obtain an IAccessPoint instance for the access point in the specified request message. * @@ -193,7 +216,7 @@ IAccessPointWeakToNetRemoteAccessPointResultItem(std::weak_ptr& ac auto accessPoint = accessPointWeak.lock(); if (accessPoint != nullptr) { - item = IAccessPointToNetRemoteAccessPointResultItem(*accessPoint.get()); + item = IAccessPointToNetRemoteAccessPointResultItem(*accessPoint); } else { item = detail::MakeInvalidAccessPointResultItem(); } @@ -269,14 +292,12 @@ NetRemoteService::WifiAccessPointDisable([[maybe_unused]] grpc::ServerContext* c { const NetRemoteWifiApiTrace traceMe{ request->accesspointid(), result->mutable_status() }; - // Create an AP controller for the requested AP. auto accessPointController = detail::TryGetAccessPointController(request, result, m_accessPointManager); if (accessPointController == nullptr) { return grpc::Status::OK; } - bool isEnabled{ false }; try { isEnabled = accessPointController->GetIsEnabled(); @@ -284,13 +305,17 @@ NetRemoteService::WifiAccessPointDisable([[maybe_unused]] grpc::ServerContext* c return HandleFailure(request, result, WifiAccessPointOperationStatusCode::WifiAccessPointOperationStatusCodeInternalError, std::format("Failed to get enabled state for access point {} ({})", request->accesspointid(), apce.what())); } - WifiAccessPointOperationStatus status{}; if (isEnabled) { - // TODO: Disable the access point. + // Disable the access point. + auto statusSetOperationState = accessPointController->SetOperationalState(AccessPointOperationalState::Disabled); + if (statusSetOperationState.Code != AccessPointOperationStatusCode::Succeeded) { + return HandleFailure(request, result, statusSetOperationState.Code, std::format("Failed to disable access point {}", request->accesspointid())); + } } else { LOGI << std::format("Access point {} is already disabled", request->accesspointid()); } + WifiAccessPointOperationStatus status{}; status.set_code(WifiAccessPointOperationStatusCode::WifiAccessPointOperationStatusCodeSucceeded); result->set_accesspointid(request->accesspointid()); From f290e2aa54ecf9fe23d64da9c374cac5607edc26 Mon Sep 17 00:00:00 2001 From: Andrew Beltrano Date: Thu, 22 Feb 2024 23:34:01 +0000 Subject: [PATCH 3/9] Implement AccessPointControllerLinux::SetOperationalState. --- .../wifi/core/AccessPointControllerLinux.cxx | 33 +++++++++++++++++-- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/src/linux/wifi/core/AccessPointControllerLinux.cxx b/src/linux/wifi/core/AccessPointControllerLinux.cxx index 4eecd386..b6d2bd9c 100644 --- a/src/linux/wifi/core/AccessPointControllerLinux.cxx +++ b/src/linux/wifi/core/AccessPointControllerLinux.cxx @@ -191,10 +191,37 @@ AccessPointControllerLinux::GetIsEnabled() } AccessPointOperationStatus -AccessPointControllerLinux::SetOperationalState([[maybe_unused]] AccessPointOperationalState operationalState) +AccessPointControllerLinux::SetOperationalState(AccessPointOperationalState operationalState) { - // TODO: Implement this method. - return AccessPointOperationStatus(AccessPointOperationStatusCode::OperationNotSupported); + AccessPointOperationStatus status{}; + + switch (operationalState) { + case AccessPointOperationalState::Enabled: { + try { + m_hostapd.Enable(); + } catch (const Wpa::HostapdException& ex) { + status.Code = AccessPointOperationStatusCode::InternalError; + status.Message = std::format("Failed to set operational state to 'enabled' for {} (unknown error)", GetInterfaceName()); + } + break; + } + case AccessPointOperationalState::Disabled: { + try { + m_hostapd.Disable(); + } catch (const Wpa::HostapdException& ex) { + status.Code = AccessPointOperationStatusCode::InternalError; + status.Message = std::format("Failed to set operational state to 'disabled' for {} (unknown error)", GetInterfaceName()); + } + break; + } + default: { + status.Code = AccessPointOperationStatusCode::InvalidParameter; + status.Message = std::format("Invalid operational state value '{}' for {}", magic_enum::enum_name(operationalState), GetInterfaceName()); + break; + } + } + + return status; } bool From 81db52a66352838389df3a0896ee616eb7d9b47f Mon Sep 17 00:00:00 2001 From: Andrew Beltrano Date: Thu, 22 Feb 2024 23:37:53 +0000 Subject: [PATCH 4/9] Rename GetIsEnabled to GetOperationalState --- src/common/service/NetRemoteService.cxx | 4 ++-- .../include/microsoft/net/wifi/IAccessPointController.hxx | 2 +- src/linux/wifi/core/AccessPointControllerLinux.cxx | 2 +- .../include/microsoft/net/wifi/AccessPointControllerLinux.hxx | 2 +- tests/unit/wifi/helpers/AccessPointControllerTest.cxx | 4 ++-- .../microsoft/net/wifi/test/AccessPointControllerTest.hxx | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/common/service/NetRemoteService.cxx b/src/common/service/NetRemoteService.cxx index b5d794e2..fb972878 100644 --- a/src/common/service/NetRemoteService.cxx +++ b/src/common/service/NetRemoteService.cxx @@ -187,7 +187,7 @@ IAccessPointToNetRemoteAccessPointResultItem(IAccessPoint& accessPoint) } try { - isEnabled = accessPointController->GetIsEnabled(); + isEnabled = accessPointController->GetOperationalState(); } catch (const AccessPointControllerException& apce) { LOGE << std::format("Failed to get enabled state for access point {} ({})", interfaceName, apce.what()); return MakeInvalidAccessPointResultItem(); @@ -300,7 +300,7 @@ NetRemoteService::WifiAccessPointDisable([[maybe_unused]] grpc::ServerContext* c bool isEnabled{ false }; try { - isEnabled = accessPointController->GetIsEnabled(); + isEnabled = accessPointController->GetOperationalState(); } catch (const AccessPointControllerException& apce) { return HandleFailure(request, result, WifiAccessPointOperationStatusCode::WifiAccessPointOperationStatusCodeInternalError, std::format("Failed to get enabled state for access point {} ({})", request->accesspointid(), apce.what())); } diff --git a/src/common/wifi/core/include/microsoft/net/wifi/IAccessPointController.hxx b/src/common/wifi/core/include/microsoft/net/wifi/IAccessPointController.hxx index 0e92e3f8..fcf8222c 100644 --- a/src/common/wifi/core/include/microsoft/net/wifi/IAccessPointController.hxx +++ b/src/common/wifi/core/include/microsoft/net/wifi/IAccessPointController.hxx @@ -66,7 +66,7 @@ struct IAccessPointController * @return false */ virtual bool - GetIsEnabled() = 0; + GetOperationalState() = 0; /** * @brief Get the capabilities of the access point. diff --git a/src/linux/wifi/core/AccessPointControllerLinux.cxx b/src/linux/wifi/core/AccessPointControllerLinux.cxx index b6d2bd9c..af883c41 100644 --- a/src/linux/wifi/core/AccessPointControllerLinux.cxx +++ b/src/linux/wifi/core/AccessPointControllerLinux.cxx @@ -176,7 +176,7 @@ AccessPointControllerLinux::GetCapabilities() } bool -AccessPointControllerLinux::GetIsEnabled() +AccessPointControllerLinux::GetOperationalState() { bool isEnabled{ false }; diff --git a/src/linux/wifi/core/include/microsoft/net/wifi/AccessPointControllerLinux.hxx b/src/linux/wifi/core/include/microsoft/net/wifi/AccessPointControllerLinux.hxx index 6dddc1ee..8e1f94d6 100644 --- a/src/linux/wifi/core/include/microsoft/net/wifi/AccessPointControllerLinux.hxx +++ b/src/linux/wifi/core/include/microsoft/net/wifi/AccessPointControllerLinux.hxx @@ -49,7 +49,7 @@ struct AccessPointControllerLinux : * @return false */ bool - GetIsEnabled() override; + GetOperationalState() override; /** * @brief Get the Capabilities object diff --git a/tests/unit/wifi/helpers/AccessPointControllerTest.cxx b/tests/unit/wifi/helpers/AccessPointControllerTest.cxx index a1f15099..b731366b 100644 --- a/tests/unit/wifi/helpers/AccessPointControllerTest.cxx +++ b/tests/unit/wifi/helpers/AccessPointControllerTest.cxx @@ -35,10 +35,10 @@ AccessPointControllerTest::GetInterfaceName() const } bool -AccessPointControllerTest::GetIsEnabled() +AccessPointControllerTest::GetOperationalState() { if (AccessPoint == nullptr) { - throw std::runtime_error("AccessPointControllerTest::GetIsEnabled called with null AccessPoint"); + throw std::runtime_error("AccessPointControllerTest::GetOperationalState called with null AccessPoint"); } return AccessPoint->IsEnabled; diff --git a/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointControllerTest.hxx b/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointControllerTest.hxx index 5217a37b..ea4302e4 100644 --- a/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointControllerTest.hxx +++ b/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointControllerTest.hxx @@ -62,7 +62,7 @@ struct AccessPointControllerTest final : * @return false */ bool - GetIsEnabled() override; + GetOperationalState() override; /** * @brief Get the capabilities of the access point. From 8559bece8f632d78da4a41375b7b8c6f353f61a0 Mon Sep 17 00:00:00 2001 From: Andrew Beltrano Date: Fri, 23 Feb 2024 00:01:17 +0000 Subject: [PATCH 5/9] Rename GetIsEnabled to GetOperationalState. --- src/common/service/NetRemoteService.cxx | 38 +++++++++---------- .../net/wifi/IAccessPointController.hxx | 10 ++--- .../wifi/core/AccessPointControllerLinux.cxx | 16 +++++--- .../net/wifi/AccessPointControllerLinux.hxx | 10 ++--- .../helpers/AccessPointControllerTest.cxx | 7 ++-- .../wifi/test/AccessPointControllerTest.hxx | 10 ++--- .../net/wifi/test/AccessPointTest.hxx | 1 - 7 files changed, 48 insertions(+), 44 deletions(-) diff --git a/src/common/service/NetRemoteService.cxx b/src/common/service/NetRemoteService.cxx index fb972878..0163cc51 100644 --- a/src/common/service/NetRemoteService.cxx +++ b/src/common/service/NetRemoteService.cxx @@ -171,12 +171,7 @@ MakeInvalidAccessPointResultItem() WifiEnumerateAccessPointsResultItem IAccessPointToNetRemoteAccessPointResultItem(IAccessPoint& accessPoint) { - WifiEnumerateAccessPointsResultItem item{}; - - bool isEnabled{ false }; std::string id{}; - Dot11AccessPointCapabilities dot11AccessPointCapabilities{}; - auto interfaceName = accessPoint.GetInterfaceName(); id.assign(std::cbegin(interfaceName), std::cend(interfaceName)); @@ -186,13 +181,14 @@ IAccessPointToNetRemoteAccessPointResultItem(IAccessPoint& accessPoint) return MakeInvalidAccessPointResultItem(); } - try { - isEnabled = accessPointController->GetOperationalState(); - } catch (const AccessPointControllerException& apce) { - LOGE << std::format("Failed to get enabled state for access point {} ({})", interfaceName, apce.what()); + AccessPointOperationalState operationalState{}; + auto operationStatus = accessPointController->GetOperationalState(operationalState); + if (!operationStatus) { + LOGE << std::format("Failed to get operational state for access point {} ({})", interfaceName, magic_enum::enum_name(operationStatus.Code)); return MakeInvalidAccessPointResultItem(); } + Dot11AccessPointCapabilities dot11AccessPointCapabilities{}; try { auto ieee80211AccessPointCapabilities = accessPointController->GetCapabilities(); dot11AccessPointCapabilities = ToDot11AccessPointCapabilities(ieee80211AccessPointCapabilities); @@ -201,7 +197,10 @@ IAccessPointToNetRemoteAccessPointResultItem(IAccessPoint& accessPoint) return MakeInvalidAccessPointResultItem(); } + const bool isEnabled{ operationalState == AccessPointOperationalState::Enabled }; + // Populate the result item. + WifiEnumerateAccessPointsResultItem item{}; item.set_accesspointid(std::move(id)); item.set_isenabled(isEnabled); *item.mutable_capabilities() = std::move(dot11AccessPointCapabilities); @@ -298,21 +297,22 @@ NetRemoteService::WifiAccessPointDisable([[maybe_unused]] grpc::ServerContext* c return grpc::Status::OK; } - bool isEnabled{ false }; - try { - isEnabled = accessPointController->GetOperationalState(); - } catch (const AccessPointControllerException& apce) { - return HandleFailure(request, result, WifiAccessPointOperationStatusCode::WifiAccessPointOperationStatusCodeInternalError, std::format("Failed to get enabled state for access point {} ({})", request->accesspointid(), apce.what())); + // Obtain current operational state. + AccessPointOperationalState operationalState{}; + auto operationStatus = accessPointController->GetOperationalState(operationalState); + if (!operationStatus) { + return HandleFailure(request, result, operationStatus.Code, std::format("Failed to get operational state for access point {}", request->accesspointid())); } - if (isEnabled) { + // Disable the access point if it's not already disabled. + if (operationalState != AccessPointOperationalState::Disabled) { // Disable the access point. - auto statusSetOperationState = accessPointController->SetOperationalState(AccessPointOperationalState::Disabled); - if (statusSetOperationState.Code != AccessPointOperationStatusCode::Succeeded) { - return HandleFailure(request, result, statusSetOperationState.Code, std::format("Failed to disable access point {}", request->accesspointid())); + operationStatus = accessPointController->SetOperationalState(AccessPointOperationalState::Disabled); + if (!operationStatus) { + return HandleFailure(request, result, operationStatus.Code, std::format("Failed to set operational state to 'disabled' for access point {}", request->accesspointid())); } } else { - LOGI << std::format("Access point {} is already disabled", request->accesspointid()); + LOGI << std::format("Access point {} is in 'disabled' operational state", request->accesspointid()); } WifiAccessPointOperationStatus status{}; diff --git a/src/common/wifi/core/include/microsoft/net/wifi/IAccessPointController.hxx b/src/common/wifi/core/include/microsoft/net/wifi/IAccessPointController.hxx index fcf8222c..78b853e6 100644 --- a/src/common/wifi/core/include/microsoft/net/wifi/IAccessPointController.hxx +++ b/src/common/wifi/core/include/microsoft/net/wifi/IAccessPointController.hxx @@ -60,13 +60,13 @@ struct IAccessPointController GetInterfaceName() const = 0; /** - * @brief Get whether the access point is enabled. + * @brief Get the access point operational state. * - * @return true - * @return false + * @param operationalState The value to store the operational state. + * @return AccessPointOperationStatus */ - virtual bool - GetOperationalState() = 0; + virtual AccessPointOperationStatus + GetOperationalState(AccessPointOperationalState& operationalState) = 0; /** * @brief Get the capabilities of the access point. diff --git a/src/linux/wifi/core/AccessPointControllerLinux.cxx b/src/linux/wifi/core/AccessPointControllerLinux.cxx index af883c41..1f3cc552 100644 --- a/src/linux/wifi/core/AccessPointControllerLinux.cxx +++ b/src/linux/wifi/core/AccessPointControllerLinux.cxx @@ -175,19 +175,23 @@ AccessPointControllerLinux::GetCapabilities() return capabilities; } -bool -AccessPointControllerLinux::GetOperationalState() +AccessPointOperationStatus +AccessPointControllerLinux::GetOperationalState(AccessPointOperationalState& operationalState) { - bool isEnabled{ false }; + AccessPointOperationStatus status{}; try { auto hostapdStatus = m_hostapd.GetStatus(); - isEnabled = (hostapdStatus.State == Wpa::HostapdInterfaceState::Enabled); + operationalState = (hostapdStatus.State == Wpa::HostapdInterfaceState::Enabled) + ? AccessPointOperationalState::Enabled + : AccessPointOperationalState::Disabled; + status = AccessPointOperationStatus::MakeSucceeded(); } catch (const Wpa::HostapdException& ex) { - throw AccessPointControllerException(std::format("Failed to get status for interface {} ({})", GetInterfaceName(), ex.what())); + status.Code = AccessPointOperationStatusCode::InternalError; + status.Message = std::format("Failed to get operational state for interface {} ({})", GetInterfaceName(), ex.what()); } - return isEnabled; + return status; } AccessPointOperationStatus diff --git a/src/linux/wifi/core/include/microsoft/net/wifi/AccessPointControllerLinux.hxx b/src/linux/wifi/core/include/microsoft/net/wifi/AccessPointControllerLinux.hxx index 8e1f94d6..e312ba9f 100644 --- a/src/linux/wifi/core/include/microsoft/net/wifi/AccessPointControllerLinux.hxx +++ b/src/linux/wifi/core/include/microsoft/net/wifi/AccessPointControllerLinux.hxx @@ -43,13 +43,13 @@ struct AccessPointControllerLinux : operator=(AccessPointControllerLinux&&) = delete; /** - * @brief Get whether the access point is enabled. + * @brief Get the access point operational state. * - * @return true - * @return false + * @param operationalState The value to store the operational state. + * @return AccessPointOperationStatus */ - bool - GetOperationalState() override; + AccessPointOperationStatus + GetOperationalState(AccessPointOperationalState& operationalState) override; /** * @brief Get the Capabilities object diff --git a/tests/unit/wifi/helpers/AccessPointControllerTest.cxx b/tests/unit/wifi/helpers/AccessPointControllerTest.cxx index b731366b..d68f301f 100644 --- a/tests/unit/wifi/helpers/AccessPointControllerTest.cxx +++ b/tests/unit/wifi/helpers/AccessPointControllerTest.cxx @@ -34,14 +34,15 @@ AccessPointControllerTest::GetInterfaceName() const return AccessPoint->InterfaceName; } -bool -AccessPointControllerTest::GetOperationalState() +AccessPointOperationStatus +AccessPointControllerTest::GetOperationalState(AccessPointOperationalState& operationalState) { if (AccessPoint == nullptr) { throw std::runtime_error("AccessPointControllerTest::GetOperationalState called with null AccessPoint"); } - return AccessPoint->IsEnabled; + operationalState = AccessPoint->OperationalState; + return AccessPointOperationStatus::MakeSucceeded(); } Ieee80211AccessPointCapabilities diff --git a/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointControllerTest.hxx b/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointControllerTest.hxx index ea4302e4..3a14f0ce 100644 --- a/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointControllerTest.hxx +++ b/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointControllerTest.hxx @@ -56,13 +56,13 @@ struct AccessPointControllerTest final : GetInterfaceName() const override; /** - * @brief Get whether the access point is enabled. + * @brief Get the access point operational state. * - * @return true - * @return false + * @param operationalState The value to store the operational state. + * @return AccessPointOperationStatus */ - bool - GetOperationalState() override; + AccessPointOperationStatus + GetOperationalState(AccessPointOperationalState& operationalState) override; /** * @brief Get the capabilities of the access point. diff --git a/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointTest.hxx b/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointTest.hxx index c9b12369..5e06f838 100644 --- a/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointTest.hxx +++ b/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointTest.hxx @@ -25,7 +25,6 @@ struct AccessPointTest final : { std::string InterfaceName; Microsoft::Net::Wifi::Ieee80211AccessPointCapabilities Capabilities; - bool IsEnabled{ false }; Microsoft::Net::Wifi::Ieee80211Protocol Protocol; std::vector FrequencyBands; std::string Ssid; From fb50b2a5548bad325f1ac63c0212e87db08d7ad8 Mon Sep 17 00:00:00 2001 From: Andrew Beltrano Date: Fri, 23 Feb 2024 00:10:27 +0000 Subject: [PATCH 6/9] Fix typo. --- src/common/service/NetRemoteService.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/service/NetRemoteService.cxx b/src/common/service/NetRemoteService.cxx index 0163cc51..1b2796d4 100644 --- a/src/common/service/NetRemoteService.cxx +++ b/src/common/service/NetRemoteService.cxx @@ -65,7 +65,7 @@ HandleFailure(RequestT& request, ResultT& result, WifiAccessPointOperationStatus } /** - * @brief Wrapper for HandleFailure that converts a Dot11AccessPointOperationStatusCode to a WifiAccessPointOperationStatusCode. + * @brief Wrapper for HandleFailure that converts a AccessPointOperationStatusCode to a WifiAccessPointOperationStatusCode. * * @tparam RequestT The request type. This must contain an access point id (trait). * @tparam ResultT The result type. This must contain an access point id and a status (traits). From 438c5b6518afba66b98ce8bb5c5277bb80b4709d Mon Sep 17 00:00:00 2001 From: Andrew Beltrano Date: Fri, 23 Feb 2024 16:28:50 +0000 Subject: [PATCH 7/9] Avoid implicit casts to bool. --- tests/unit/linux/wpa-controller/TestHostapd.cxx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/unit/linux/wpa-controller/TestHostapd.cxx b/tests/unit/linux/wpa-controller/TestHostapd.cxx index 5f501e8c..0d8a24d3 100644 --- a/tests/unit/linux/wpa-controller/TestHostapd.cxx +++ b/tests/unit/linux/wpa-controller/TestHostapd.cxx @@ -115,12 +115,12 @@ TEST_CASE("Send command: GetStatus() (root)", "[wpa][hostapd][client][remote]") const auto ieee80211nInitial = hostapd.GetStatus().Ieee80211n; - auto ieee80211nValueExpected = !!ieee80211nInitial; + auto ieee80211nValueExpected = static_cast(ieee80211nInitial); REQUIRE(hostapd.SetProperty(ProtocolHostapd::PropertyNameIeee80211N, GetPropertyEnablementValue(ieee80211nValueExpected))); auto ieee80211nValueUpdated = hostapd.GetStatus().Ieee80211n; REQUIRE(ieee80211nValueUpdated == ieee80211nValueExpected); - ieee80211nValueExpected = !!ieee80211nValueUpdated; + ieee80211nValueExpected = static_cast(ieee80211nValueUpdated); REQUIRE(hostapd.SetProperty(ProtocolHostapd::PropertyNameIeee80211N, GetPropertyEnablementValue(ieee80211nValueExpected))); ieee80211nValueUpdated = hostapd.GetStatus().Ieee80211n; REQUIRE(ieee80211nValueUpdated == ieee80211nValueExpected); @@ -132,12 +132,12 @@ TEST_CASE("Send command: GetStatus() (root)", "[wpa][hostapd][client][remote]") const auto ieee80211acInitial = hostapd.GetStatus().Ieee80211ac; - auto ieee80211acValueExpected = !!ieee80211acInitial; + auto ieee80211acValueExpected = static_cast(ieee80211acInitial); REQUIRE(hostapd.SetProperty(ProtocolHostapd::PropertyNameIeee80211AC, GetPropertyEnablementValue(ieee80211acValueExpected))); auto ieee80211acValueUpdated = hostapd.GetStatus().Ieee80211ac; REQUIRE(ieee80211acValueUpdated == ieee80211acValueExpected); - ieee80211acValueExpected = !!ieee80211acValueUpdated; + ieee80211acValueExpected = static_cast(ieee80211acValueUpdated); REQUIRE(hostapd.SetProperty(ProtocolHostapd::PropertyNameIeee80211AC, GetPropertyEnablementValue(ieee80211acValueExpected))); ieee80211acValueUpdated = hostapd.GetStatus().Ieee80211ac; REQUIRE(ieee80211acValueUpdated == ieee80211acValueExpected); @@ -149,12 +149,12 @@ TEST_CASE("Send command: GetStatus() (root)", "[wpa][hostapd][client][remote]") const auto ieee80211axInitial = hostapd.GetStatus().Ieee80211ax; - auto ieee80211axValueExpected = !!ieee80211axInitial; + auto ieee80211axValueExpected = static_cast(ieee80211axInitial); REQUIRE(hostapd.SetProperty(ProtocolHostapd::PropertyNameIeee80211AX, GetPropertyEnablementValue(ieee80211axValueExpected))); auto ieee80211axValueUpdated = hostapd.GetStatus().Ieee80211ax; REQUIRE(ieee80211axValueUpdated == ieee80211axValueExpected); - ieee80211axValueExpected = !!ieee80211axValueUpdated; + ieee80211axValueExpected = static_cast(ieee80211axValueUpdated); REQUIRE(hostapd.SetProperty(ProtocolHostapd::PropertyNameIeee80211AX, GetPropertyEnablementValue(ieee80211axValueExpected))); ieee80211axValueUpdated = hostapd.GetStatus().Ieee80211ax; REQUIRE(ieee80211axValueUpdated == ieee80211axValueExpected); From 2c509253c747deb0ee8fb7ba1656e6128b9f43e5 Mon Sep 17 00:00:00 2001 From: Andrew Beltrano Date: Fri, 23 Feb 2024 16:49:55 +0000 Subject: [PATCH 8/9] Add tests. --- tests/unit/TestNetRemoteServiceClient.cxx | 101 ++++++++++++++++++++++ 1 file changed, 101 insertions(+) diff --git a/tests/unit/TestNetRemoteServiceClient.cxx b/tests/unit/TestNetRemoteServiceClient.cxx index 34ac2340..35366b0e 100644 --- a/tests/unit/TestNetRemoteServiceClient.cxx +++ b/tests/unit/TestNetRemoteServiceClient.cxx @@ -122,6 +122,107 @@ TEST_CASE("WifiAccessPointEnable API", "[basic][rpc][client][remote]") } } +TEST_CASE("WifiAccessPointDisable API", "[basic][rpc][client][remote]") +{ + using namespace Microsoft::Net::Remote; + using namespace Microsoft::Net::Remote::Service; + using namespace Microsoft::Net::Remote::Test; + using namespace Microsoft::Net::Remote::Wifi; + using namespace Microsoft::Net::Wifi; + using namespace Microsoft::Net::Wifi::Test; + + constexpr auto InterfaceName1{ "TestWifiAccessPointDisable1" }; + constexpr auto InterfaceName2{ "TestWifiAccessPointDisable2" }; + constexpr auto InterfaceNameInvalid{ "TestWifiAccessPointDisableInvalid" }; + + auto apManagerTest = std::make_shared(); + Ieee80211AccessPointCapabilities apCapabilities{ + .Protocols{ std::cbegin(AllProtocols), std::cend(AllProtocols) } + }; + + auto apTest1 = std::make_shared(InterfaceName1, apCapabilities); + auto apTest2 = std::make_shared(InterfaceName2, apCapabilities); + apManagerTest->AddAccessPoint(apTest1); + apManagerTest->AddAccessPoint(apTest2); + + NetRemoteServerConfiguration Configuration{ + .ServerAddress = RemoteServiceAddressHttp, + .AccessPointManager = apManagerTest, + }; + + NetRemoteServer server{ Configuration }; + server.Run(); + + auto channel = grpc::CreateChannel(RemoteServiceAddressHttp, grpc::InsecureChannelCredentials()); + auto client = NetRemote::NewStub(channel); + + SECTION("Can be called") + { + WifiAccessPointDisableRequest request{}; + request.set_accesspointid(InterfaceName1); + + WifiAccessPointDisableResult result{}; + grpc::ClientContext clientContext{}; + + grpc::Status status; + REQUIRE_NOTHROW(status = client->WifiAccessPointDisable(&clientContext, request, &result)); + REQUIRE(status.ok()); + REQUIRE(result.accesspointid() == request.accesspointid()); + } + + SECTION("Fails with invalid access point") + { + WifiAccessPointDisableRequest request{}; + request.set_accesspointid(InterfaceNameInvalid); + + WifiAccessPointDisableResult result{}; + grpc::ClientContext clientContext{}; + + grpc::Status status; + REQUIRE_NOTHROW(status = client->WifiAccessPointDisable(&clientContext, request, &result)); + REQUIRE(status.ok()); + REQUIRE(result.accesspointid() == request.accesspointid()); + REQUIRE(result.has_status()); + REQUIRE(result.status().code() == WifiAccessPointOperationStatusCode::WifiAccessPointOperationStatusCodeAccessPointInvalid); + } + + SECTION("Succeeds with valid access point") + { + WifiAccessPointDisableRequest request{}; + request.set_accesspointid(InterfaceName1); + + WifiAccessPointDisableResult result{}; + grpc::ClientContext clientContext{}; + + grpc::Status status; + REQUIRE_NOTHROW(status = client->WifiAccessPointDisable(&clientContext, request, &result)); + REQUIRE(status.ok()); + REQUIRE(result.accesspointid() == request.accesspointid()); + REQUIRE(result.has_status()); + REQUIRE(result.status().code() == WifiAccessPointOperationStatusCode::WifiAccessPointOperationStatusCodeSucceeded); + REQUIRE(apTest1->OperationalState == AccessPointOperationalState::Disabled); + } + + SECTION("Succeeds with valid access point when already disabled") + { + for (auto i = 0; i < 2; ++i) { + WifiAccessPointDisableRequest request{}; + request.set_accesspointid(InterfaceName1); + + WifiAccessPointDisableResult result{}; + grpc::ClientContext clientContext{}; + + grpc::Status status; + REQUIRE_NOTHROW(status = client->WifiAccessPointDisable(&clientContext, request, &result)); + REQUIRE(status.ok()); + REQUIRE(result.accesspointid() == request.accesspointid()); + REQUIRE(result.has_status()); + REQUIRE(result.status().code() == WifiAccessPointOperationStatusCode::WifiAccessPointOperationStatusCodeSucceeded); + REQUIRE(apTest1->OperationalState == AccessPointOperationalState::Disabled); + } + } +} + TEST_CASE("WifiAccessPointSetPhyType API", "[basic][rpc][client][remote]") { using namespace Microsoft::Net::Remote; From bac0d9c6ae48494ac471577b0a7ea6773688f29d Mon Sep 17 00:00:00 2001 From: Andrew Beltrano Date: Fri, 23 Feb 2024 16:58:51 +0000 Subject: [PATCH 9/9] Add AccessPointOperationStatus helpers to determine success/failed status with unary function. --- .../wifi/core/AccessPointOperationStatus.cxx | 13 +++++++++++++ .../net/wifi/AccessPointOperationStatus.hxx | 18 ++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/src/common/wifi/core/AccessPointOperationStatus.cxx b/src/common/wifi/core/AccessPointOperationStatus.cxx index a0bdc2b9..dc43f427 100644 --- a/src/common/wifi/core/AccessPointOperationStatus.cxx +++ b/src/common/wifi/core/AccessPointOperationStatus.cxx @@ -10,7 +10,20 @@ AccessPointOperationStatus::MakeSucceeded() noexcept return AccessPointOperationStatus{ AccessPointOperationStatusCode::Succeeded }; } +bool +AccessPointOperationStatus::Succeeded() const noexcept +{ + return (Code == AccessPointOperationStatusCode::Succeeded); +} + +bool +AccessPointOperationStatus::Failed() const noexcept +{ + return !Succeeded(); +} + AccessPointOperationStatus::operator bool() const noexcept { + return Succeeded(); return (Code == AccessPointOperationStatusCode::Succeeded); } diff --git a/src/common/wifi/core/include/microsoft/net/wifi/AccessPointOperationStatus.hxx b/src/common/wifi/core/include/microsoft/net/wifi/AccessPointOperationStatus.hxx index 92e18221..e9bb92b5 100644 --- a/src/common/wifi/core/include/microsoft/net/wifi/AccessPointOperationStatus.hxx +++ b/src/common/wifi/core/include/microsoft/net/wifi/AccessPointOperationStatus.hxx @@ -60,6 +60,24 @@ struct AccessPointOperationStatus static AccessPointOperationStatus MakeSucceeded() noexcept; + /** + * @brief Determine whether the operation succeeded. + * + * @return true + * @return false + */ + bool + Succeeded() const noexcept; + + /** + * @brief Determine whether the operation failed. + * + * @return true + * @return false + */ + bool + Failed() const noexcept; + /** * @brief Implicit bool operator allowing AccessPointOperationStatus to be used directly in condition statements * (eg. if (status) // ...).