Address code review comments

- Avoid multiple calls to Server::GetInstance()
- Update parts of README
- Add TODO for workarounds to be removed later
- Remove usage of __FUNCTION__

examples/ota-requestor-app/linux/README.md

@@ -65,10 +65,10 @@ In terminal 2:
 
 ### Limitations
 
--   Needs chip-tool to pair to the OTA Provider device first because the Node ID
-    and IP Address of the OTA Provider can must be supplied to this reference
+-   Needs chip-tool to commission the OTA Provider device first because the Node
+    ID and IP Address of the OTA Provider must be supplied to this reference
     application
 -   Does not verify QueryImageResponse message contents
--   Stores the downloaded file at a hardcoded filepath
--   Does not close the BDX ExchangeContext when the exchange is over
+-   Stores the downloaded file in the directory this reference app is launched
+    from
 -   Does not support AnnounceOTAProvider command or OTA Requestor attributes

examples/ota-requestor-app/linux/main.cpp

@@ -80,6 +80,7 @@ OptionDef cmdLineOptionsDef[] = {
     { "providerFabricIndex", chip::ArgParser::kArgumentRequired, kOptionProviderFabricIndex },
     { "udpPort", chip::ArgParser::kArgumentRequired, kOptionUdpPort },
     { "discriminator", chip::ArgParser::kArgumentRequired, kOptionDiscriminator },
+    // TODO: This can be removed once OperationalDeviceProxy can resolve the IP Address from Node ID
     { "ipaddress", chip::ArgParser::kArgumentRequired, kOptionIPAddress },
     { "delayQuery", chip::ArgParser::kArgumentRequired, kOptionDelayQuery },
     {},
@@ -252,17 +253,22 @@ void SendQueryImageCommand()
     PeerAddress addr = PeerAddress::UDP(ipAddr, CHIP_PORT);
     gOperationalDeviceProxy.UpdateAddress(addr);
 
+    Server * server                             = &(Server::GetInstance());
     OperationalDeviceProxyInitParams initParams = {
-        .sessionManager = &(Server::GetInstance().GetSecureSessionManager()),
-        .exchangeMgr    = &(Server::GetInstance().GetExchangeManager()),
-        .idAllocator    = &(Server::GetInstance().GetSessionIDAllocator()),
-        .fabricsTable   = &(Server::GetInstance().GetFabricTable()),
+        .sessionManager = &(server->GetSecureSessionManager()),
+        .exchangeMgr    = &(server->GetExchangeManager()),
+        .idAllocator    = &(server->GetSessionIDAllocator()),
+        .fabricsTable   = &(server->GetFabricTable()),
     };
 
     CHIP_ERROR err              = CHIP_NO_ERROR;
     FabricIndex peerFabricIndex = providerFabricIndex;
     gOperationalDeviceProxy.Init(providerNodeId, peerFabricIndex, initParams);
     err = gOperationalDeviceProxy.Connect(&mOnConnectedCallback, &mOnConnectionFailureCallback);
+    if (err != CHIP_NO_ERROR)
+    {
+        ChipLogError(SoftwareUpdate, "Cannot establish connection to peer device: %" CHIP_ERROR_FORMAT, err.Format());
+    }
 }
 
 void OnStartDelayTimerHandler(Layer * systemLayer, void * appState)
@@ -299,7 +305,7 @@ int main(int argc, char * argv[])
     err = chip::DeviceLayer::ConfigurationMgr().StoreSetupDiscriminator(setupDiscriminator);
     if (err != CHIP_NO_ERROR)
     {
-        ChipLogError(SoftwareUpdate, "Setup discriminator setting failed with code: %" CHIP_ERROR_FORMAT "\n", err.Format());
+        ChipLogError(SoftwareUpdate, "Setup discriminator setting failed with code: %" CHIP_ERROR_FORMAT, err.Format());
         return 1;
     }
 

src/app/device/OperationalDeviceProxy.cpp

@@ -137,7 +137,7 @@ void OperationalDeviceProxy::OnConnectionExpired(SessionHandle session)
 
 void OperationalDeviceProxy::OnDeviceConnectedFn(void * context, Controller::Device * device)
 {
-    VerifyOrReturn(context != nullptr, ChipLogError(OperationalDeviceProxy, "%s: invalid context", __FUNCTION__));
+    VerifyOrReturn(context != nullptr, ChipLogError(OperationalDeviceProxy, "Device connected: invalid context"));
     OperationalDeviceProxy * operationalDevice = reinterpret_cast<OperationalDeviceProxy *>(context);
 
     VerifyOrReturn(device != nullptr, ChipLogError(OperationalDeviceProxy, "%s: invalid device", __FUNCTION__));
@@ -149,7 +149,7 @@ void OperationalDeviceProxy::OnDeviceConnectedFn(void * context, Controller::Dev
 
 void OperationalDeviceProxy::OnDeviceConnectionFailureFn(void * context, NodeId deviceId, CHIP_ERROR error)
 {
-    VerifyOrReturn(context != nullptr, ChipLogError(OperationalDeviceProxy, "%s: invalid context", __FUNCTION__));
+    VerifyOrReturn(context != nullptr, ChipLogError(OperationalDeviceProxy, "Device connection failure: invalid context"));
     OperationalDeviceProxy * operationalDevice = reinterpret_cast<OperationalDeviceProxy *>(context);
 
     operationalDevice->GetDevice().UpdateSession(false);