Merge pull request #113 from microsoft/apcapsapi

Partially implement WifiEnumerateAccessPoints to expose access point ids

src/common/server/NetRemoteServer.cxx

@@ -9,8 +9,9 @@
 
 using namespace Microsoft::Net::Remote;
 
-NetRemoteServer::NetRemoteServer(std::string_view serverAddress) :
-    m_serverAddress{ serverAddress }
+NetRemoteServer::NetRemoteServer(NetRemoteServerConfiguration configuration) :
+    m_serverAddress(configuration.ServerAddress),
+    m_service(configuration.AccessPointManager)
 {}
 
 NetRemoteServer::~NetRemoteServer()
@@ -24,6 +25,12 @@ NetRemoteServer::GetGrpcServer() noexcept
     return m_server;
 }
 
+Service::NetRemoteService&
+NetRemoteServer::GetService() noexcept
+{
+    return m_service;
+}
+
 void
 NetRemoteServer::Run()
 {
@@ -36,7 +43,7 @@ NetRemoteServer::Run()
     builder.RegisterService(&m_service);
 
     m_server = builder.BuildAndStart();
-    LOG_INFO << std::format("netremote server started listening on {}", m_serverAddress);
+    LOG_INFO << std::format("Netremote server started listening on {}", m_serverAddress);
 }
 
 void

src/common/server/NetRemoteServerConfiguration.cxx

@@ -19,12 +19,12 @@ ConfigureCliAppOptions(CLI::App& app, NetRemoteServerConfiguration& config)
     app.add_option(
         "-a,--address",
         config.ServerAddress,
-        "The address to listen on for incoming connections.");
+        "The address to listen on for incoming connections");
 
-    app.add_option(
+    app.add_flag(
         "-v,--verbosity",
         config.LogVerbosity,
-        "The log verbosity level. Supply multiple times to increase verbosity (0=warnings, errors, and fatal messages, 1=info messages, 2=debug messages, 3=verbose messages).");
+        "The log verbosity level. Supply multiple times to increase verbosity (0=warnings, errors, and fatal messages, 1=info messages, 2=debug messages, 3=verbose messages)");
 
     return app;
 }

src/common/server/include/microsoft/net/remote/NetRemoteServer.hxx

@@ -7,7 +7,9 @@
 #include <string_view>
 
 #include <grpcpp/server.h>
+#include <microsoft/net/remote/NetRemoteServerConfiguration.hxx>
 #include <microsoft/net/remote/NetRemoteService.hxx>
+#include <microsoft/net/wifi/AccessPointManager.hxx>
 
 namespace Microsoft::Net::Remote
 {
@@ -19,11 +21,11 @@ struct NetRemoteServer
     virtual ~NetRemoteServer();
 
     /**
-     * @brief Construct a new NetRemoteServer object.
+     * @brief Construct a new NetRemoteServer object with the specified configuration.
      *
-     * @param serverAddress
+     * @param configuration
      */
-    NetRemoteServer(std::string_view serverAddress);
+    NetRemoteServer(NetRemoteServerConfiguration configuration);
 
     /**
      * @brief Get the GrpcServer object.
@@ -33,6 +35,14 @@ struct NetRemoteServer
     std::unique_ptr<grpc::Server>&
     GetGrpcServer() noexcept;
 
+    /**
+     * @brief Get the NetRemoteService object instance.
+     *
+     * @return Service::NetRemoteService&
+     */
+    Service::NetRemoteService&
+    GetService() noexcept;
+
     /**
      * @brief Start the server if not already started.
      */

src/common/server/include/microsoft/net/remote/NetRemoteServerConfiguration.hxx

@@ -3,11 +3,20 @@
 #define NET_REMOTE_SERVER_CONFIGURATION_HXX
 
 #include <cstdint>
+#include <memory>
 #include <string>
 #include <vector>
 
+namespace Microsoft::Net::Wifi
+{
+class AccessPointManager;
+} // namespace Microsoft::Net::Wifi
+
 namespace Microsoft::Net::Remote
 {
+/**
+ * @brief Collects configuration options for the NetRemoteServer class.
+ */
 struct NetRemoteServerConfiguration
 {
     /**
@@ -54,7 +63,12 @@ struct NetRemoteServerConfiguration
      * show all info messages, and a level of 2 will show all debug messages,
      * and a level of 3 or above will show all verbose messages.
      */
-    uint8_t LogVerbosity{ 0 };
+    uint32_t LogVerbosity{ 0 };
+
+    /**
+     * @brief Access point manager instance.
+     */
+    std::shared_ptr<Microsoft::Net::Wifi::AccessPointManager> AccessPointManager;
 };
 
 } // namespace Microsoft::Net::Remote

src/common/service/NetRemoteService.cxx

@@ -1,6 +1,5 @@
 
 #include <format>
-#include <iostream>
 #include <iterator>
 #include <string>
 #include <vector>
@@ -13,15 +12,25 @@ using namespace Microsoft::Net::Remote::Service;
 using Microsoft::Net::Wifi::AccessPointManager;
 
 NetRemoteService::NetRemoteService() :
-    m_accessPointManager(AccessPointManager::Create())
+    NetRemoteService(AccessPointManager::Create())
 {}
 
+NetRemoteService::NetRemoteService(std::shared_ptr<AccessPointManager> accessPointManager) :
+    m_accessPointManager(std::move(accessPointManager))
+{}
+
+std::shared_ptr<AccessPointManager>
+NetRemoteService::GetAccessPointManager() noexcept
+{
+    return m_accessPointManager;
+}
+
 ::grpc::Status
 NetRemoteService::WifiEnumerateAccessPoints([[maybe_unused]] ::grpc::ServerContext* context, [[maybe_unused]] const ::Microsoft::Net::Remote::Wifi::WifiEnumerateAccessPointsRequest* request, ::Microsoft::Net::Remote::Wifi::WifiEnumerateAccessPointsResult* response)
 {
     using Microsoft::Net::Remote::Wifi::WifiEnumerateAccessPointsResultItem;
 
-    LOG_VERBOSE << std::format("Received WifiEnumerateAccessPoints request\n");
+    LOGD << std::format("Received WifiEnumerateAccessPoints request");
 
     auto accessPoints = m_accessPointManager->GetAllAccessPoints();
     std::vector<WifiEnumerateAccessPointsResultItem> accessPointResultItems(std::size(accessPoints));
@@ -53,7 +62,7 @@ using Microsoft::Net::Wifi::Dot11PhyType;
 ::grpc::Status
 NetRemoteService::WifiAccessPointEnable([[maybe_unused]] ::grpc::ServerContext* context, const ::Microsoft::Net::Remote::Wifi::WifiAccessPointEnableRequest* request, ::Microsoft::Net::Remote::Wifi::WifiAccessPointEnableResult* response)
 {
-    LOG_VERBOSE << std::format("Received WifiAccessPointEnable request for access point id {}\n", request->accesspointid());
+    LOGD << std::format("Received WifiAccessPointEnable request for access point id {}", request->accesspointid());
 
     WifiAccessPointOperationStatus status{};
 
@@ -72,7 +81,7 @@ NetRemoteService::WifiAccessPointEnable([[maybe_unused]] ::grpc::ServerContext*
 ::grpc::Status
 NetRemoteService::WifiAccessPointDisable([[maybe_unused]] ::grpc::ServerContext* context, const ::Microsoft::Net::Remote::Wifi::WifiAccessPointDisableRequest* request, ::Microsoft::Net::Remote::Wifi::WifiAccessPointDisableResult* response)
 {
-    LOG_VERBOSE << std::format("Received WifiAccessPointDisable request for access point id {}\n", request->accesspointid());
+    LOGD << std::format("Received WifiAccessPointDisable request for access point id {}", request->accesspointid());
 
     WifiAccessPointOperationStatus status{};
     // TODO: Disable the access point.

src/common/service/include/microsoft/net/remote/NetRemoteService.hxx

@@ -9,12 +9,33 @@
 
 namespace Microsoft::Net::Remote::Service
 {
+/**
+ * @brief Implementation of the NetRemote::Service gRPC service.
+ */
 class NetRemoteService :
     public NetRemote::Service
 {
 public:
+    /**
+     * @brief Construct a new NetRemoteService object.
+     */
     NetRemoteService();
 
+    /**
+     * @brief Construct a new NetRemoteService object with the specified access point manager.
+     *
+     * @param accessPointManager The access point manager to use.
+     */
+    NetRemoteService(std::shared_ptr<Microsoft::Net::Wifi::AccessPointManager> accessPointManager);
+
+    /**
+     * @brief Get the AccessPointManager object for this service.
+     *
+     * @return std::shared_ptr<Microsoft::Net::Wifi::AccessPointManager>
+     */
+    std::shared_ptr<Microsoft::Net::Wifi::AccessPointManager>
+    GetAccessPointManager() noexcept;
+
 private:
     virtual ::grpc::Status
     WifiEnumerateAccessPoints(::grpc::ServerContext* context, const ::Microsoft::Net::Remote::Wifi::WifiEnumerateAccessPointsRequest* request, ::Microsoft::Net::Remote::Wifi::WifiEnumerateAccessPointsResult* response) override;

src/common/shared/logging/LogUtils.cxx

@@ -22,7 +22,7 @@ logging::GetLogName(std::string_view componentName)
 }
 
 plog::Severity
-logging::LogVerbosityToPlogSeverity(uint8_t verbosity) noexcept
+logging::LogVerbosityToPlogSeverity(uint32_t verbosity) noexcept
 {
     switch (verbosity) {
     case 0:

src/common/shared/logging/include/logging/LogUtils.hxx

@@ -29,7 +29,7 @@ GetLogName(std::string_view componentName);
  * @return plog::Severity
  */
 plog::Severity
-LogVerbosityToPlogSeverity(uint8_t verbosity) noexcept;
+LogVerbosityToPlogSeverity(uint32_t verbosity) noexcept;
 
 } // namespace logging
 

src/common/tools/cli/NetRemoteCliHandlerOperations.cxx

@@ -29,6 +29,12 @@ NetRemoteCliHandlerOperations::WifiEnumerateAccessPoints()
         LOGE << std::format("Failed to enumerate WiFi access points, error={} details={} message={}", magic_enum::enum_name(status.error_code()), status.error_details(), status.error_message());
         return;
     }
+
+    LOGI << std::format("{} access points discovered", result.accesspoints_size());
+
+    for (const auto& accessPoint : result.accesspoints()) {
+        LOGI << std::format(" - [{}]", accessPoint.accesspointid());
+    }
 }
 
 std::unique_ptr<INetRemoteCliHandlerOperations>

src/common/wifi/apmanager/AccessPointDiscoveryAgent.cxx

@@ -2,6 +2,7 @@
 #include <microsoft/net/wifi/AccessPointDiscoveryAgent.hxx>
 #include <microsoft/net/wifi/IAccessPoint.hxx>
 #include <notstd/Memory.hxx>
+#include <plog/Log.h>
 
 using namespace Microsoft::Net::Wifi;
 
@@ -39,6 +40,7 @@ AccessPointDiscoveryAgent::DevicePresenceChanged(AccessPointPresenceEvent presen
 {
     std::shared_lock<std::shared_mutex> onDevicePresenceChangedLock{ m_onDevicePresenceChangedGate };
     if (m_onDevicePresenceChanged) {
+        LOGD << "Access point discovery agent detected a device presence change";
         m_onDevicePresenceChanged(presence, std::move(interfaceName));
     }
 }
@@ -54,6 +56,7 @@ AccessPointDiscoveryAgent::Start()
 {
     bool expected = false;
     if (m_started.compare_exchange_weak(expected, true)) {
+        LOGD << "Access point discovery agent starting";
         m_operations->Start([weakThis = std::weak_ptr<AccessPointDiscoveryAgent>(GetInstance())](auto&& presence, auto&& interfaceName) {
             // Attempt to promote the weak pointer to a shared pointer to ensure this instance is still valid.
             if (auto strongThis = weakThis.lock(); strongThis) {
@@ -68,12 +71,14 @@ AccessPointDiscoveryAgent::Stop()
 {
     bool expected = true;
     if (m_started.compare_exchange_weak(expected, false)) {
+        LOGD << "Access point discovery agent stopping";
         m_operations->Stop();
     }
 }
 
 std::future<std::vector<std::string>>
 AccessPointDiscoveryAgent::ProbeAsync()
 {
+    LOGD << "Access point discovery agent probing for devices";
     return m_operations->ProbeAsync();
 }

src/common/wifi/apmanager/AccessPointManager.cxx

@@ -89,6 +89,8 @@ AccessPointManager::AddDiscoveryAgent(std::shared_ptr<AccessPointDiscoveryAgent>
 {
     using namespace std::chrono_literals;
 
+    // Register a discovery event callback.
+    //
     // Use a weak_ptr below to ensure that the access point manager can
     // be safely destroyed prior to the discovery agent. This allows the
     // callback to be registered indefinitely, safely checking whether this
@@ -100,28 +102,25 @@ AccessPointManager::AddDiscoveryAgent(std::shared_ptr<AccessPointDiscoveryAgent>
         }
     });
 
-    // If this agent has already started, kick off a probe to ensure any access
-    // points already found will be added to this manager.
-    std::future<std::vector<std::string>> existingAccessPointInterfaceNamesProbe;
-    const bool isStarted = discoveryAgent->IsStarted();
-    if (isStarted) {
-        existingAccessPointInterfaceNamesProbe = discoveryAgent->ProbeAsync();
-    } else {
+    // Start the agent if not done already.
+    if (!discoveryAgent->IsStarted()) {
         discoveryAgent->Start();
     }
 
-    // If the agent hasn't yet been started, start it now, then probe for
-    // existing access points in case they've already been discovered.
+    // Kick off a probe to ensure any access points already present will be added to this manager.
+    auto existingAccessPointInterfaceNamesProbe = discoveryAgent->ProbeAsync();
+
+    // Add the agent.
     {
         std::unique_lock<std::shared_mutex> discoveryAgentLock{ m_discoveryAgentsGate };
         m_discoveryAgents.push_back(std::move(discoveryAgent));
     }
 
     if (existingAccessPointInterfaceNamesProbe.valid()) {
-        static constexpr auto probeTimeout = 3s;
+        static constexpr auto ProbeTimeout = 3s;
 
         // Wait for the operation to complete.
-        const auto waitResult = existingAccessPointInterfaceNamesProbe.wait_for(probeTimeout);
+        const auto waitResult = existingAccessPointInterfaceNamesProbe.wait_for(ProbeTimeout);
 
         // If the operation completed, get the results and add those access points.
         if (waitResult == std::future_status::ready) {

src/common/wifi/apmanager/CMakeLists.txt

@@ -21,6 +21,7 @@ target_sources(wifi-apmanager
 target_link_libraries(wifi-apmanager
     PRIVATE
         notstd
+        plog::plog
     PUBLIC
         ${PROJECT_NAME}-protocol
         wifi-core