Merge pull request #122 from microsoft/apcapimplapi

Accept linux-specific factory for AccessPointDiscoveryAgentOperationsNetlink
microsoft · Jan 23, 2024 · 44eb34d · 44eb34d
2 parents 1772ee6 + 1fe4fbf
commit 44eb34d
Show file tree

Hide file tree

Showing 16 changed files with 176 additions and 57 deletions.
diff --git a/src/linux/libnl-helpers/Netlink80211Interface.cxx b/src/linux/libnl-helpers/Netlink80211Interface.cxx
@@ -158,5 +158,5 @@ Nl80211Interface::Enumerate()
 std::optional<Nl80211Wiphy>
 Nl80211Interface::GetWiphy() const
 {
-    return Nl80211Wiphy::FromIndex(WiphyIndex); 
+    return Nl80211Wiphy::FromIndex(WiphyIndex);
 }
diff --git a/src/linux/libnl-helpers/include/microsoft/net/netlink/nl80211/Netlink80211Interface.hxx b/src/linux/libnl-helpers/include/microsoft/net/netlink/nl80211/Netlink80211Interface.hxx
@@ -26,6 +26,18 @@ struct Nl80211Interface
     uint32_t Index;
     uint32_t WiphyIndex;
 
+    Nl80211Interface() = default;
+
+    /**
+     * @brief Construct a new Nl80211Interface object with the specified attributes.
+     *
+     * @param name The name of the interface.
+     * @param type The nl80211_iftype of the interface.
+     * @param index The interface index in the kernel.
+     * @param wiphyIndex The phy interface index in the kernel.
+     */
+    Nl80211Interface(std::string_view name, nl80211_iftype type, uint32_t index, uint32_t wiphyIndex) noexcept;
+
     /**
      * @brief Parse a netlink message into an Nl80211Interface. The netlink message must contain a response to the
      * NL80211_CMD_GET_INTERFACE command, which is encoded as a NL80211_CMD_NEW_INTERFACE.
@@ -48,8 +60,8 @@ struct Nl80211Interface
 
     /**
      * @brief Get the Wiphy (PHY) object associated with this interface.
-     * 
-     * @return std::optional<Nl80211Wiphy> 
+     *
+     * @return std::optional<Nl80211Wiphy>
      */
     std::optional<Nl80211Wiphy>
     GetWiphy() const;
@@ -61,17 +73,6 @@ struct Nl80211Interface
      */
     std::string
     ToString() const;
-
-private:
-    /**
-     * @brief Construct a new Nl80211Interface object with the specified attributes.
-     *
-     * @param name The name of the interface.
-     * @param type The nl80211_iftype of the interface.
-     * @param index The interface index in the kernel.
-     * @param wiphyIndex The phy interface index in the kernel.
-     */
-    Nl80211Interface(std::string_view name, nl80211_iftype type, uint32_t index, uint32_t wiphyIndex) noexcept;
 };
 
 } // namespace Microsoft::Net::Netlink::Nl80211

diff --git a/src/linux/wifi/apmanager/AccessPointDiscoveryAgentOperationsNetlink.cxx b/src/linux/wifi/apmanager/AccessPointDiscoveryAgentOperationsNetlink.cxx
@@ -30,7 +30,7 @@ using Microsoft::Net::Netlink::NetlinkMessage;
 using Microsoft::Net::Netlink::NetlinkSocket;
 using Microsoft::Net::Netlink::Nl80211::Nl80211ProtocolState;
 
-AccessPointDiscoveryAgentOperationsNetlink::AccessPointDiscoveryAgentOperationsNetlink(std::shared_ptr<IAccessPointFactory> accessPointFactory) :
+AccessPointDiscoveryAgentOperationsNetlink::AccessPointDiscoveryAgentOperationsNetlink(std::shared_ptr<AccessPointFactoryLinux> accessPointFactory) :
     m_accessPointFactory(std::move(accessPointFactory)),
     m_cookie(CookieValid),
     m_netlink80211ProtocolState(Nl80211ProtocolState::Instance())
@@ -141,9 +141,10 @@ IsNl80211InterfaceTypeAp(const Nl80211Interface &nl80211Interface)
  * @return std::shared_ptr<IAccessPoint>
  */
 std::shared_ptr<IAccessPoint>
-MakeAccessPoint(std::shared_ptr<IAccessPointFactory> accessPointFactory, const Nl80211Interface &nl80211Interface)
+MakeAccessPoint(std::shared_ptr<AccessPointFactoryLinux> accessPointFactory, const Nl80211Interface &nl80211Interface)
 {
-    return accessPointFactory->Create(nl80211Interface.Name);
+    auto createArgs = std::make_unique<AccessPointCreateArgsLinux>(std::move(nl80211Interface));
+    return accessPointFactory->Create(nl80211Interface.Name, std::move(createArgs));
 }
 } // namespace detail
 

diff --git a/src/linux/wifi/apmanager/CMakeLists.txt b/src/linux/wifi/apmanager/CMakeLists.txt
@@ -25,11 +25,5 @@ target_link_libraries(wifi-apmanager-linux
         libnl-helpers
         wifi-apmanager
         wifi-core
-)
-
-install(
-    TARGETS wifi-apmanager-linux
-    EXPORT ${PROJECT_NAME}
-    FILE_SET HEADERS
-    PUBLIC_HEADER DESTINATION "${NETREMOTE_DIR_INSTALL_PUBLIC_HEADER_BASE}/${WIFI_APMANAGER_LINUX_PUBLIC_INCLUDE_SUFFIX}"
+        wifi-core-linux
 )
diff --git a/.../wifi/apmanager/include/microsoft/net/wifi/AccessPointDiscoveryAgentOperationsNetlink.hxx b/.../wifi/apmanager/include/microsoft/net/wifi/AccessPointDiscoveryAgentOperationsNetlink.hxx
@@ -14,6 +14,7 @@
 #include <microsoft/net/netlink/NetlinkSocket.hxx>
 #include <microsoft/net/netlink/nl80211/Netlink80211Interface.hxx>
 #include <microsoft/net/netlink/nl80211/Netlink80211ProtocolState.hxx>
+#include <microsoft/net/wifi/AccessPointLinux.hxx>
 #include <microsoft/net/wifi/IAccessPoint.hxx>
 #include <microsoft/net/wifi/IAccessPointDiscoveryAgentOperations.hxx>
 #include <netlink/netlink.h>
@@ -29,7 +30,12 @@ namespace Microsoft::Net::Wifi
 struct AccessPointDiscoveryAgentOperationsNetlink :
     public IAccessPointDiscoveryAgentOperations
 {
-    AccessPointDiscoveryAgentOperationsNetlink(std::shared_ptr<IAccessPointFactory> accessPointFactory);
+    /**
+     * @brief Construct a new AccessPointDiscoveryAgentOperationsNetlink object with the specified access point factory.
+     *
+     * @param accessPointFactory The access point factory to use for creating access points.
+     */
+    AccessPointDiscoveryAgentOperationsNetlink(std::shared_ptr<AccessPointFactoryLinux> accessPointFactory);
 
     virtual ~AccessPointDiscoveryAgentOperationsNetlink();
 
@@ -101,7 +107,7 @@ private:
     ProcessNetlinkMessage(struct nl_msg *netlinkMessage, AccessPointPresenceEventCallback &accessPointPresenceEventCallback);
 
 private:
-    std::shared_ptr<IAccessPointFactory> m_accessPointFactory;
+    std::shared_ptr<AccessPointFactoryLinux> m_accessPointFactory;
 
     // Cookie used to validate that the callback context is valid.
     static constexpr uint32_t CookieValid{ 0x8BADF00Du };

diff --git a/src/linux/wifi/core/AccessPointLinux.cxx b/src/linux/wifi/core/AccessPointLinux.cxx
@@ -1,9 +1,20 @@
 
+#include <stdexcept>
+
 #include <microsoft/net/wifi/AccessPointControllerLinux.hxx>
 #include <microsoft/net/wifi/AccessPointLinux.hxx>
+#include <plog/Log.h>
+
+using Microsoft::Net::Netlink::Nl80211::Nl80211Interface;
 
 using namespace Microsoft::Net::Wifi;
 
+AccessPointLinux::AccessPointLinux(std::string_view interfaceName, std::shared_ptr<IAccessPointControllerFactory> accessPointControllerFactory, Microsoft::Net::Netlink::Nl80211::Nl80211Interface nl80211Interface) :
+    AccessPoint(interfaceName, std::move(accessPointControllerFactory)),
+    m_nl80211Interface{ std::move(nl80211Interface) }
+{
+}
+
 std::unique_ptr<IAccessPointController>
 AccessPointLinux::CreateController()
 {
@@ -17,7 +28,17 @@ AccessPointFactoryLinux::Create(std::string_view interfaceName)
 }
 
 std::shared_ptr<IAccessPoint>
-AccessPointFactoryLinux::Create(std::string_view interfaceName, [[maybe_unused]] std::unique_ptr<IAccessPointCreateArgs> createArgs)
+AccessPointFactoryLinux::Create(std::string_view interfaceName, std::unique_ptr<IAccessPointCreateArgs> createArgs)
+{
+    auto createArgsLinux = dynamic_cast<AccessPointCreateArgsLinux *>(createArgs.get());
+    if (createArgsLinux == nullptr) {
+        throw std::runtime_error("invalid arguments passed to AccessPointFactoryLinux::Create; this is a bug!");
+    }
+
+    return std::make_shared<AccessPointLinux>(interfaceName, GetControllerFactory(), std::move(createArgsLinux->Interface));
+}
+
+AccessPointCreateArgsLinux::AccessPointCreateArgsLinux(Nl80211Interface nl80211Interface) :
+    Interface{ std::move(nl80211Interface) }
 {
-    return std::make_shared<AccessPointLinux>(interfaceName, GetControllerFactory());
 }
diff --git a/src/linux/wifi/core/CMakeLists.txt b/src/linux/wifi/core/CMakeLists.txt
@@ -21,6 +21,7 @@ target_link_libraries(wifi-core-linux
     PRIVATE
         plog::plog
     PUBLIC
+        libnl-helpers
         wifi-core
         wpa-controller
 )
diff --git a/src/linux/wifi/core/include/microsoft/net/wifi/AccessPointLinux.hxx b/src/linux/wifi/core/include/microsoft/net/wifi/AccessPointLinux.hxx
@@ -6,6 +6,7 @@
 #include <string_view>
 
 #include <microsoft/net/wifi/AccessPoint.hxx>
+#include <microsoft/net/netlink/nl80211/Netlink80211Interface.hxx>
 
 namespace Microsoft::Net::Wifi
 {
@@ -16,12 +17,25 @@ struct AccessPointLinux :
     public AccessPoint
 {
     /**
-     * @brief Inherit the constructors from the base class.
+     * @brief Construct a new AccessPointLinux object with the specified interface name, access point controller
+     * factory, and nl80211 interface.
+     * 
+     * @param interfaceName The name of the interface.
+     * @param accessPointControllerFactory The access point controller factory to use for creating access point.
+     * @param nl80211Interface The nl80211 interface object.
      */
-    using AccessPoint::AccessPoint;
+    AccessPointLinux(std::string_view interfaceName, std::shared_ptr<IAccessPointControllerFactory> accessPointControllerFactory, Microsoft::Net::Netlink::Nl80211::Nl80211Interface nl80211Interface);
 
+    /**
+     * @brief Create a IAccessPointController object of type AccessPointControllerLinux.
+     * 
+     * @return std::unique_ptr<Microsoft::Net::Wifi::IAccessPointController> 
+     */
     virtual std::unique_ptr<Microsoft::Net::Wifi::IAccessPointController>
     CreateController() override;
+
+private:
+    Microsoft::Net::Netlink::Nl80211::Nl80211Interface m_nl80211Interface;
 };
 
 /**
@@ -51,6 +65,22 @@ struct AccessPointFactoryLinux :
     virtual std::shared_ptr<IAccessPoint>
     Create(std::string_view interfaceName, std::unique_ptr<IAccessPointCreateArgs> createArgs) override;
 };
+
+/**
+ * @brief Arguments to be passed to the Linux access point during creation.
+ */
+struct AccessPointCreateArgsLinux :
+    public IAccessPointCreateArgs
+{
+    /**
+     * @brief Construct a new AccessPointCreateArgsLinux object with the specified nl80211 interface.
+     * 
+     * @param nl80211Interface The nl80211 interface object.
+     */
+    AccessPointCreateArgsLinux(Microsoft::Net::Netlink::Nl80211::Nl80211Interface nl80211Interface);
+
+    Microsoft::Net::Netlink::Nl80211::Nl80211Interface Interface;
+};
 } // namespace Microsoft::Net::Wifi
 
 #endif // ACCESS_POINT_LINUX_HXX
diff --git a/tests/unit/linux/wifi/CMakeLists.txt b/tests/unit/linux/wifi/CMakeLists.txt
@@ -1,3 +1,4 @@
 
 add_subdirectory(apmanager)
 add_subdirectory(core)
+add_subdirectory(helpers)
diff --git a/tests/unit/linux/wifi/apmanager/CMakeLists.txt b/tests/unit/linux/wifi/apmanager/CMakeLists.txt
@@ -19,7 +19,7 @@ target_link_libraries(wifi-apmanager-linux-test-unit
         plog::plog
         strings
         wifi-apmanager-linux
-        wifi-test-helpers
+        wifi-test-helpers-linux
 )
 
 catch_discover_tests(wifi-apmanager-linux-test-unit)
diff --git a/tests/unit/linux/wifi/apmanager/TestAccessPointDiscoveryAgentOperationsNetlink.cxx b/tests/unit/linux/wifi/apmanager/TestAccessPointDiscoveryAgentOperationsNetlink.cxx
@@ -6,6 +6,7 @@
 
 #include <microsoft/net/wifi/AccessPointDiscoveryAgentOperationsNetlink.hxx>
 #include <microsoft/net/wifi/test/AccessPointTest.hxx>
+#include <microsoft/net/wifi/test/AccessPointFactoryLinuxTest.hxx>
 
 TEST_CASE("Create AccessPointDiscoveryAgentOperationsNetlink", "[wifi][core][apmanager]")
 {
@@ -14,7 +15,7 @@ TEST_CASE("Create AccessPointDiscoveryAgentOperationsNetlink", "[wifi][core][apm
 
     SECTION("Create doesn't cause a crash")
     {
-        REQUIRE_NOTHROW(AccessPointDiscoveryAgentOperationsNetlink{ std::make_shared<AccessPointFactoryTest>() });
+        REQUIRE_NOTHROW(AccessPointDiscoveryAgentOperationsNetlink{ std::make_shared<AccessPointFactoryLinuxTest>() });
     }
 }
 
@@ -25,7 +26,7 @@ TEST_CASE("Destroy AccessPointDiscoveryAgentOperationsNetlink", "[wifi][core][ap
 
     SECTION("Destroy doesn't cause a crash")
     {
-        std::optional<AccessPointDiscoveryAgentOperationsNetlink> accessPointDiscoveryAgent(std::make_shared<AccessPointFactoryTest>());
+        std::optional<AccessPointDiscoveryAgentOperationsNetlink> accessPointDiscoveryAgent(std::make_shared<AccessPointFactoryLinuxTest>());
         REQUIRE_NOTHROW(accessPointDiscoveryAgent.reset());
     }
 }
@@ -37,20 +38,20 @@ TEST_CASE("AccessPointDiscoveryAgentOperationsNetlink::Start", "[wifi][core][apm
 
     SECTION("Start doesn't cause a crash")
     {
-        AccessPointDiscoveryAgentOperationsNetlink accessPointDiscoveryAgent{ std::make_shared<AccessPointFactoryTest>() };
+        AccessPointDiscoveryAgentOperationsNetlink accessPointDiscoveryAgent{ std::make_shared<AccessPointFactoryLinuxTest>() };
         REQUIRE_NOTHROW(accessPointDiscoveryAgent.Start([](auto &&, auto &&) {}));
     }
 
     SECTION("Start doesn't cause a crash when called twice")
     {
-        AccessPointDiscoveryAgentOperationsNetlink accessPointDiscoveryAgent{ std::make_shared<AccessPointFactoryTest>() };
+        AccessPointDiscoveryAgentOperationsNetlink accessPointDiscoveryAgent{ std::make_shared<AccessPointFactoryLinuxTest>() };
         accessPointDiscoveryAgent.Start([](auto &&, auto &&) {});
         REQUIRE_NOTHROW(accessPointDiscoveryAgent.Start([](auto &&, auto &&) {}));
     }
 
     SECTION("Start doesn't cause a crash when called with a null callback")
     {
-        AccessPointDiscoveryAgentOperationsNetlink accessPointDiscoveryAgent{ std::make_shared<AccessPointFactoryTest>() };
+        AccessPointDiscoveryAgentOperationsNetlink accessPointDiscoveryAgent{ std::make_shared<AccessPointFactoryLinuxTest>() };
         REQUIRE_NOTHROW(accessPointDiscoveryAgent.Start(nullptr));
     }
 }
@@ -62,28 +63,28 @@ TEST_CASE("AccessPointDiscoveryAgentOperationsNetlink::Stop", "[wifi][core][apma
 
     SECTION("Stop doesn't cause a crash when Start hasn't been called")
     {
-        AccessPointDiscoveryAgentOperationsNetlink accessPointDiscoveryAgent{ std::make_shared<AccessPointFactoryTest>() };
+        AccessPointDiscoveryAgentOperationsNetlink accessPointDiscoveryAgent{ std::make_shared<AccessPointFactoryLinuxTest>() };
         REQUIRE_NOTHROW(accessPointDiscoveryAgent.Stop());
     }
 
     SECTION("Stop doesn't cause a crash when Start has been called")
     {
-        AccessPointDiscoveryAgentOperationsNetlink accessPointDiscoveryAgent{ std::make_shared<AccessPointFactoryTest>() };
+        AccessPointDiscoveryAgentOperationsNetlink accessPointDiscoveryAgent{ std::make_shared<AccessPointFactoryLinuxTest>() };
         accessPointDiscoveryAgent.Start([](auto &&, auto &&) {});
         REQUIRE_NOTHROW(accessPointDiscoveryAgent.Stop());
     }
 
     SECTION("Stop doesn't cause a crash when called twice")
     {
-        AccessPointDiscoveryAgentOperationsNetlink accessPointDiscoveryAgent{ std::make_shared<AccessPointFactoryTest>() };
+        AccessPointDiscoveryAgentOperationsNetlink accessPointDiscoveryAgent{ std::make_shared<AccessPointFactoryLinuxTest>() };
         accessPointDiscoveryAgent.Start([](auto &&, auto &&) {});
         accessPointDiscoveryAgent.Stop();
         REQUIRE_NOTHROW(accessPointDiscoveryAgent.Stop());
     }
 
     SECTION("Stop doesn't cause a crash when called with a null callback")
     {
-        AccessPointDiscoveryAgentOperationsNetlink accessPointDiscoveryAgent{ std::make_shared<AccessPointFactoryTest>() };
+        AccessPointDiscoveryAgentOperationsNetlink accessPointDiscoveryAgent{ std::make_shared<AccessPointFactoryLinuxTest>() };
         accessPointDiscoveryAgent.Start(nullptr);
         REQUIRE_NOTHROW(accessPointDiscoveryAgent.Stop());
     }
@@ -96,56 +97,56 @@ TEST_CASE("AccessPointDiscoveryAgentOperationsNetlink::ProbeAsync", "[wifi][core
 
     SECTION("ProbeAsync doesn't cause a crash when Start hasn't been called")
     {
-        AccessPointDiscoveryAgentOperationsNetlink accessPointDiscoveryAgent{ std::make_shared<AccessPointFactoryTest>() };
+        AccessPointDiscoveryAgentOperationsNetlink accessPointDiscoveryAgent{ std::make_shared<AccessPointFactoryLinuxTest>() };
         REQUIRE_NOTHROW(accessPointDiscoveryAgent.ProbeAsync());
     }
 
     SECTION("ProbeAsync doesn't cause a crash when Start has been called")
     {
-        AccessPointDiscoveryAgentOperationsNetlink accessPointDiscoveryAgent{ std::make_shared<AccessPointFactoryTest>() };
+        AccessPointDiscoveryAgentOperationsNetlink accessPointDiscoveryAgent{ std::make_shared<AccessPointFactoryLinuxTest>() };
         accessPointDiscoveryAgent.Start([](auto &&, auto &&) {});
         REQUIRE_NOTHROW(accessPointDiscoveryAgent.ProbeAsync());
     }
 
     SECTION("ProbeAsync doesn't cause a crash when Stop has been called")
     {
-        AccessPointDiscoveryAgentOperationsNetlink accessPointDiscoveryAgent{ std::make_shared<AccessPointFactoryTest>() };
+        AccessPointDiscoveryAgentOperationsNetlink accessPointDiscoveryAgent{ std::make_shared<AccessPointFactoryLinuxTest>() };
         accessPointDiscoveryAgent.Stop();
         REQUIRE_NOTHROW(accessPointDiscoveryAgent.ProbeAsync());
     }
 
     SECTION("ProbeAsync doesn't cause a crash when called twice")
     {
-        AccessPointDiscoveryAgentOperationsNetlink accessPointDiscoveryAgent{ std::make_shared<AccessPointFactoryTest>() };
+        AccessPointDiscoveryAgentOperationsNetlink accessPointDiscoveryAgent{ std::make_shared<AccessPointFactoryLinuxTest>() };
         accessPointDiscoveryAgent.Start([](auto &&, auto &&) {});
         accessPointDiscoveryAgent.ProbeAsync();
         REQUIRE_NOTHROW(accessPointDiscoveryAgent.ProbeAsync());
     }
 
     SECTION("ProbeAsync doesn't cause a crash when called after a Start/Stop sequence")
     {
-        AccessPointDiscoveryAgentOperationsNetlink accessPointDiscoveryAgent{ std::make_shared<AccessPointFactoryTest>() };
+        AccessPointDiscoveryAgentOperationsNetlink accessPointDiscoveryAgent{ std::make_shared<AccessPointFactoryLinuxTest>() };
         accessPointDiscoveryAgent.Start(nullptr);
         accessPointDiscoveryAgent.Stop();
         REQUIRE_NOTHROW(accessPointDiscoveryAgent.ProbeAsync());
     }
 
     SECTION("ProbeAsync returns a valid future")
     {
-        AccessPointDiscoveryAgentOperationsNetlink accessPointDiscoveryAgent{ std::make_shared<AccessPointFactoryTest>() };
+        AccessPointDiscoveryAgentOperationsNetlink accessPointDiscoveryAgent{ std::make_shared<AccessPointFactoryLinuxTest>() };
         accessPointDiscoveryAgent.Start(nullptr);
         REQUIRE(accessPointDiscoveryAgent.ProbeAsync().valid());
     }
 
     SECTION("ProbeAsync result can be obtained without causing a crash")
     {
-        AccessPointDiscoveryAgentOperationsNetlink accessPointDiscoveryAgent{ std::make_shared<AccessPointFactoryTest>() };
+        AccessPointDiscoveryAgentOperationsNetlink accessPointDiscoveryAgent{ std::make_shared<AccessPointFactoryLinuxTest>() };
         REQUIRE_NOTHROW(accessPointDiscoveryAgent.ProbeAsync().get());
     }
 
     SECTION("ProbeAsync result is valid")
     {
-        AccessPointDiscoveryAgentOperationsNetlink accessPointDiscoveryAgent{ std::make_shared<AccessPointFactoryTest>() };
+        AccessPointDiscoveryAgentOperationsNetlink accessPointDiscoveryAgent{ std::make_shared<AccessPointFactoryLinuxTest>() };
         REQUIRE_NOTHROW(accessPointDiscoveryAgent.ProbeAsync().get().clear());
     }
 }