Address review comments

- Rename Server -> WiFiNetworkManagementServer and move it up a namespace
- Move internal definitions into an anonymous namespace

Author: ksperling-apple

examples/network-manager-app/linux/main.cpp

@@ -39,8 +39,8 @@ int main(int argc, char * argv[])
         return -1;
     }
 
-    WiFiNetworkManagement::Server::Instance().SetNetworkCredentials(ByteSpanFromCharSpan("MatterAP"_span),
-                                                                    ByteSpanFromCharSpan("Setec Astronomy"_span));
+    WiFiNetworkManagementServer::Instance().SetNetworkCredentials(ByteSpanFromCharSpan("MatterAP"_span),
+                                                                  ByteSpanFromCharSpan("Setec Astronomy"_span));
 
     ChipLinuxAppMainLoop();
     return 0;

src/app/clusters/wifi-network-management-server/wifi-network-management-server.cpp

@@ -31,29 +31,51 @@ using namespace chip;
 using namespace chip::app;
 using namespace chip::app::Clusters;
 using namespace chip::app::Clusters::WiFiNetworkManagement::Attributes;
+using namespace chip::app::Clusters::WiFiNetworkManagement::Commands;
 using namespace std::placeholders;
 
 namespace chip {
 namespace app {
 namespace Clusters {
-namespace WiFiNetworkManagement {
 
-Global<Server> gServerInstance;
+namespace {
 
-Server & Server::Instance()
+// TODO: Move this into lib/support somewhere and also use it network-commissioning.cpp
+bool IsValidWpaPersonalCredential(ByteSpan credential)
+{
+    // As per spec section 11.9.7.3. AddOrUpdateWiFiNetwork Command
+    if (8 <= credential.size() && credential.size() <= 63) // passphrase
+    {
+        return true;
+    }
+    if (credential.size() == 64) // raw hex psk
+    {
+        return std::all_of(credential.begin(), credential.end(), [](auto c) { return std::isxdigit(c); });
+    }
+    return false;
+}
+
+Global<WiFiNetworkManagementServer> gWiFiNetworkManagementServerInstance;
+
+} // namespace
+
+WiFiNetworkManagementServer & WiFiNetworkManagementServer::Instance()
 {
-    return gServerInstance.get();
+    return gWiFiNetworkManagementServerInstance.get();
 }
 
-Server::Server() : AttributeAccessInterface(NullOptional, Id), CommandHandlerInterface(NullOptional, Id) {}
+WiFiNetworkManagementServer::WiFiNetworkManagementServer() :
+    AttributeAccessInterface(NullOptional, WiFiNetworkManagement::Id),
+    CommandHandlerInterface(NullOptional, WiFiNetworkManagement::Id)
+{}
 
-Server::~Server()
+WiFiNetworkManagementServer::~WiFiNetworkManagementServer()
 {
     unregisterAttributeAccessOverride(this);
     InteractionModelEngine::GetInstance()->UnregisterCommandHandler(this);
 }
 
-CHIP_ERROR Server::Init(EndpointId endpoint)
+CHIP_ERROR WiFiNetworkManagementServer::Init(EndpointId endpoint)
 {
     VerifyOrReturnError(endpoint != kInvalidEndpointId, CHIP_ERROR_INVALID_ARGUMENT);
     VerifyOrReturnError(mEndpointId == kInvalidEndpointId, CHIP_ERROR_INCORRECT_STATE);
@@ -64,32 +86,17 @@ CHIP_ERROR Server::Init(EndpointId endpoint)
     return CHIP_NO_ERROR;
 }
 
-CHIP_ERROR Server::ClearNetworkCredentials()
+CHIP_ERROR WiFiNetworkManagementServer::ClearNetworkCredentials()
 {
     VerifyOrReturnError(HaveNetworkCredentials(), CHIP_NO_ERROR);
 
     mSsidLen = 0;
     mPassphrase.SetLength(0);
-    MatterReportingAttributeChangeCallback(mEndpointId, Id, Ssid::Id);
+    MatterReportingAttributeChangeCallback(mEndpointId, WiFiNetworkManagement::Id, Ssid::Id);
     return CHIP_NO_ERROR;
 }
 
-// TODO: Move this into lib/support somewhere and also use it network-commissioning.cpp
-bool IsValidWpaPersonalCredential(ByteSpan credential)
-{
-    // As per spec section 11.9.7.3. AddOrUpdateWiFiNetwork Command
-    if (8 <= credential.size() && credential.size() <= 63) // passphrase
-    {
-        return true;
-    }
-    if (credential.size() == 64) // raw hex psk
-    {
-        return std::all_of(credential.begin(), credential.end(), [](auto c) { return std::isxdigit(c); });
-    }
-    return false;
-}
-
-CHIP_ERROR Server::SetNetworkCredentials(ByteSpan ssid, ByteSpan passphrase)
+CHIP_ERROR WiFiNetworkManagementServer::SetNetworkCredentials(ByteSpan ssid, ByteSpan passphrase)
 {
     VerifyOrReturnError(1 <= ssid.size() && ssid.size() <= sizeof(mSsid), CHIP_ERROR_INVALID_ARGUMENT);
     VerifyOrReturnError(IsValidWpaPersonalCredential(passphrase), CHIP_ERROR_INVALID_ARGUMENT);
@@ -107,12 +114,12 @@ CHIP_ERROR Server::SetNetworkCredentials(ByteSpan ssid, ByteSpan passphrase)
     // Note: The spec currently defines no way to signal a passphrase change
     if (ssidChanged)
     {
-        MatterReportingAttributeChangeCallback(mEndpointId, Id, Ssid::Id);
+        MatterReportingAttributeChangeCallback(mEndpointId, WiFiNetworkManagement::Id, Ssid::Id);
     }
     return CHIP_NO_ERROR;
 }
 
-CHIP_ERROR Server::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder)
+CHIP_ERROR WiFiNetworkManagementServer::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder)
 {
     switch (aPath.mAttributeId)
     {
@@ -122,22 +129,23 @@ CHIP_ERROR Server::Read(const ConcreteReadAttributePath & aPath, AttributeValueE
     return CHIP_NO_ERROR;
 }
 
-void Server::InvokeCommand(HandlerContext & ctx)
+void WiFiNetworkManagementServer::InvokeCommand(HandlerContext & ctx)
 {
     switch (ctx.mRequestPath.mCommandId)
     {
-    case Commands::NetworkPassphraseRequest::Id:
-        HandleCommand<Commands::NetworkPassphraseRequest::DecodableType>(
+    case NetworkPassphraseRequest::Id:
+        HandleCommand<NetworkPassphraseRequest::DecodableType>(
             ctx, [this](HandlerContext & aCtx, const auto & req) { HandleNetworkPassphraseRequest(aCtx, req); });
         return;
     }
 }
 
-void Server::HandleNetworkPassphraseRequest(HandlerContext & ctx, const Commands::NetworkPassphraseRequest::DecodableType & req)
+void WiFiNetworkManagementServer::HandleNetworkPassphraseRequest(HandlerContext & ctx,
+                                                                 const NetworkPassphraseRequest::DecodableType & req)
 {
     if (HaveNetworkCredentials())
     {
-        Commands::NetworkPassphraseResponse::Type response;
+        NetworkPassphraseResponse::Type response;
         response.passphrase = mPassphrase.Span();
         ctx.mCommandHandler.AddResponse(ctx.mRequestPath, response);
     }
@@ -148,7 +156,6 @@ void Server::HandleNetworkPassphraseRequest(HandlerContext & ctx, const Commands
     }
 }
 
-} // namespace WiFiNetworkManagement
 } // namespace Clusters
 } // namespace app
 } // namespace chip
@@ -165,5 +172,5 @@ void emberAfWiFiNetworkManagementClusterServerInitCallback(EndpointId endpoint)
     // We could delay constructing the instance until this point; however it's not
     // clear if this is inconvenient in terms of forcing the application to initialize
     // the network credentials later than it otherwise would.
-    LogErrorOnFailure(chip::app::Clusters::WiFiNetworkManagement::Server::Instance().Init(endpoint));
+    LogErrorOnFailure(chip::app::Clusters::WiFiNetworkManagementServer::Instance().Init(endpoint));
 }

src/app/clusters/wifi-network-management-server/wifi-network-management-server.h

@@ -30,31 +30,31 @@ void emberAfWiFiNetworkManagementClusterServerInitCallback(chip::EndpointId);
 namespace chip {
 namespace app {
 namespace Clusters {
-namespace WiFiNetworkManagement {
 
-class Server : private AttributeAccessInterface, private CommandHandlerInterface
+class WiFiNetworkManagementServer : private AttributeAccessInterface, private CommandHandlerInterface
 {
 public:
-    static Server & Instance();
+    static WiFiNetworkManagementServer & Instance();
 
     CHIP_ERROR ClearNetworkCredentials();
     CHIP_ERROR SetNetworkCredentials(ByteSpan ssid, ByteSpan passphrase);
 
 private:
-    friend Global<Server>;
+    friend Global<WiFiNetworkManagementServer>;
     friend void ::emberAfWiFiNetworkManagementClusterServerInitCallback(chip::EndpointId);
 
-    Server();
-    ~Server();
+    WiFiNetworkManagementServer();
+    ~WiFiNetworkManagementServer();
     CHIP_ERROR Init(EndpointId endpoint);
 
-    Server(Server const &)             = delete;
-    Server & operator=(Server const &) = delete;
+    WiFiNetworkManagementServer(WiFiNetworkManagementServer const &)             = delete;
+    WiFiNetworkManagementServer & operator=(WiFiNetworkManagementServer const &) = delete;
 
     CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override;
     void InvokeCommand(HandlerContext & handlerContext) override;
 
-    void HandleNetworkPassphraseRequest(HandlerContext & ctx, const Commands::NetworkPassphraseRequest::DecodableType & req);
+    void HandleNetworkPassphraseRequest(HandlerContext & ctx,
+                                        const WiFiNetworkManagement::Commands::NetworkPassphraseRequest::DecodableType & req);
 
     EndpointId mEndpointId = kInvalidEndpointId;
 
@@ -69,7 +69,6 @@ class Server : private AttributeAccessInterface, private CommandHandlerInterface
     bool HaveNetworkCredentials() { return mSsidLen > 0; }
 };
 
-} // namespace WiFiNetworkManagement
 } // namespace Clusters
 } // namespace app
 } // namespace chip