Resolve comment

project-chip · May 26, 2021 · 551cd5c · 551cd5c
1 parent 5db8f44
commit 551cd5c
Show file tree

Hide file tree

Showing 9 changed files with 65 additions and 91 deletions.
diff --git a/src/app/clusters/ias-zone-client/ias-zone-client.cpp b/src/app/clusters/ias-zone-client/ias-zone-client.cpp
@@ -390,8 +390,7 @@ static void removeServer(uint8_t * ieeeAddress)
 static EmberStatus sendCommand(EmberNodeId destAddress)
 {
     emberAfSetCommandEndpoints(myEndpoint, emberAfIasZoneClientKnownServers[currentIndex].endpoint);
-    MessageSendDestination destination = MessageSendDestination(MessageSendDestination::OutGoingDirect(destAddress));
-    EmberStatus status                 = emberAfSendCommandUnicast(destination);
+    EmberStatus status = emberAfSendCommandUnicast(MessageSendDestination::Direct(destAddress));
     emberAfIasZoneClusterPrintln("Sent IAS Zone Client Command to 0x%2X (%d -> %d) status: 0x%X", destAddress, myEndpoint,
                                  emberAfIasZoneClientKnownServers[currentIndex].endpoint, status);
     if (status != EMBER_SUCCESS)

diff --git a/src/app/clusters/messaging-client/messaging-client.cpp b/src/app/clusters/messaging-client/messaging-client.cpp
@@ -328,7 +328,7 @@ EmberAfStatus emberAfPluginMessagingClientConfirmMessage(EndpointId endpoint)
                                       emberAfGetCurrentTime(), 0x00, "");
             // The source and destination are reversed for the confirmation.
             emberAfSetCommandEndpoints(messageTable[ep].clientEndpoint, esiEntry->endpoint);
-            MessageSendDestination destination = MessageSendDestination(MessageSendDestination::OutGoingDirect(nodeId));
+            const MessageSendDestination destination = MessageSendDestination::Direct(nodeId);
             status =
                 ((emberAfSendCommandUnicast(destination) == EMBER_SUCCESS) ? EMBER_ZCL_STATUS_SUCCESS : EMBER_ZCL_STATUS_FAILURE);
         }

diff --git a/src/app/clusters/messaging-server/messaging-server.cpp b/src/app/clusters/messaging-server/messaging-server.cpp
@@ -226,8 +226,7 @@ void emberAfPluginMessagingServerDisplayMessage(EmberNodeId nodeId, uint8_t srcE
                               message.startTime, message.durationInMinutes, message.message, message.extendedMessageControl);
     emberAfSetCommandEndpoints(srcEndpoint, dstEndpoint);
     emberAfGetCommandApsFrame()->options |= EMBER_APS_OPTION_SOURCE_EUI64;
-    MessageSendDestination destination = MessageSendDestination(MessageSendDestination::OutGoingDirect(nodeId));
-    status                             = emberAfSendCommandUnicast(destination);
+    status = emberAfSendCommandUnicast(MessageSendDestination::Direct(nodeId));
     if (status != EMBER_SUCCESS)
     {
         emberAfMessagingClusterPrintln("Error in display %x", status);
@@ -249,8 +248,7 @@ void emberAfPluginMessagingServerCancelMessage(EmberNodeId nodeId, uint8_t srcEn
                               ZCL_CANCEL_MESSAGE_COMMAND_ID, "wu", message.messageId, message.messageControl);
     emberAfSetCommandEndpoints(srcEndpoint, dstEndpoint);
     emberAfGetCommandApsFrame()->options |= EMBER_APS_OPTION_SOURCE_EUI64;
-    MessageSendDestination destination = MessageSendDestination(MessageSendDestination::OutGoingDirect(nodeId));
-    status                             = emberAfSendCommandUnicast(destination);
+    status = emberAfSendCommandUnicast(MessageSendDestination::Direct(nodeId));
     if (status != EMBER_SUCCESS)
     {
         emberAfMessagingClusterPrintln("Error in cancel %x", status);

diff --git a/src/app/reporting/reporting.cpp b/src/app/reporting/reporting.cpp
@@ -73,8 +73,8 @@ static void removeConfigurationAndScheduleTick(uint8_t index);
 static EmberAfStatus configureReceivedAttribute(const EmberAfClusterCommand * cmd, AttributeId attributeId, uint8_t mask,
                                                 uint16_t timeout);
 static void putReportableChangeInResp(const EmberAfPluginReportingEntry * entry, EmberAfAttributeType dataType);
-static void retrySendReport(MessageSendDestination & destination, EmberApsFrame * apsFrame, uint16_t msgLen, uint8_t * message,
-                            EmberStatus status);
+static void retrySendReport(const MessageSendDestination & destination, EmberApsFrame * apsFrame, uint16_t msgLen,
+                            uint8_t * message, EmberStatus status);
 static uint32_t computeStringHash(uint8_t * data, uint8_t length);
 
 EmberEventControl emberAfPluginReportingTickEventControl;
@@ -99,8 +99,8 @@ EmberAfStatus emberAfPluginReportingConfiguredCallback(const EmberAfPluginReport
     return EMBER_ZCL_STATUS_SUCCESS;
 }
 
-static void retrySendReport(MessageSendDestination & destination, EmberApsFrame * apsFrame, uint16_t msgLen, uint8_t * message,
-                            EmberStatus status)
+static void retrySendReport(const MessageSendDestination & destination, EmberApsFrame * apsFrame, uint16_t msgLen,
+                            uint8_t * message, EmberStatus status)
 {
     // Retry once, and do so by unicasting without a pointer to this callback
     if (status != EMBER_SUCCESS)
@@ -364,7 +364,7 @@ static void conditionallySendReport(EndpointId endpoint, ClusterId clusterId)
     EmberStatus status;
     if (emberAfIsDeviceEnabled(endpoint) || clusterId == ZCL_IDENTIFY_CLUSTER_ID)
     {
-        status = emberAfSendCommandUnicastToBindingsWithCallback((EmberAfMessageSentFunction)(&retrySendReport));
+        status = emberAfSendCommandUnicastToBindingsWithCallback(&retrySendReport);
 
         // If the callback table is full, attempt to send the message with no
         // callback.  Note that this could lead to a message failing to transmit

diff --git a/src/app/util/af-main-common.cpp b/src/app/util/af-main-common.cpp
@@ -331,7 +331,7 @@ EmberStatus emberAfSendMulticastWithAliasWithCallback(GroupId multicastId, Ember
                                                       EmberAfMessageSentFunction callback)
 {
     apsFrame->groupId                        = multicastId;
-    const MessageSendDestination destination = MessageSendDestination::Multicast(multicastId);
+    const MessageSendDestination destination = MessageSendDestination::MulticastWithAlias(multicastId);
     return send(destination, apsFrame, messageLength, message,
                 true, // broadcast
                 alias, sequence, callback);
@@ -436,7 +436,7 @@ EmberStatus emberAfSendUnicastWithCallback(const MessageSendDestination & destin
         EmberBindingTableEntry binding;
         // TODO: This cast should go away once
         // https://github.com/project-chip/connectedhomeip/issues/3584 is fixed.
-        EmberStatus status = emberGetBinding(destination.Get<MessageSendDestination::VariantViaBinding>().mBindingIndex, &binding);
+        EmberStatus status = emberGetBinding(destination.GetBindingIndex(), &binding);
         if (status != EMBER_SUCCESS)
         {
             return status;
@@ -630,54 +630,31 @@ EmberStatus emAfSend(const MessageSendDestination & destination, EmberApsFrame *
     // tracks this.
     *messageTag        = INVALID_MESSAGE_TAG;
     EmberStatus status = EMBER_SUCCESS;
-    switch (destination.GetType())
+    if (destination.IsViaBinding())
     {
-    case MessageSendDestination::VariantViaBinding::VariantId: {
         EmberBindingTableEntry binding;
-        status = emberGetBinding(destination.Get<MessageSendDestination::VariantViaBinding>().mBindingIndex, &binding);
+        status = emberGetBinding(destination.GetBindingIndex(), &binding);
         if (status != EMBER_SUCCESS)
         {
-            break;
+            return status;
         }
         if (binding.type != EMBER_UNICAST_BINDING)
         {
-            status = EMBER_INVALID_BINDING_INDEX;
-            break;
+            return EMBER_INVALID_BINDING_INDEX;
         }
-        status = chipSendUnicast(binding.nodeId, apsFrame, messageLength, message);
-        break;
+        return chipSendUnicast(binding.nodeId, apsFrame, messageLength, message);
     }
-    case MessageSendDestination::VariantViaAddressTable::VariantId:
-        // No implementation yet.
-        status = EMBER_ERR_FATAL;
-        break;
-    case MessageSendDestination::VariantDirect::VariantId:
-        status =
-            chipSendUnicast(destination.Get<MessageSendDestination::VariantDirect>().mNodeId, apsFrame, messageLength, message);
-        break;
-    case MessageSendDestination::VariantMulticast::VariantId:
-        // No implementation yet.
-        status = EMBER_ERR_FATAL;
-        break;
-    case MessageSendDestination::VariantMulticastWithAlias::VariantId:
-        // No implementation yet.
-        status = EMBER_ERR_FATAL;
-        break;
-    case MessageSendDestination::VariantBroadcast::VariantId:
-        // No implementation yet.
-        status = EMBER_ERR_FATAL;
-        break;
-    case MessageSendDestination::VariantBroadcastWithAlias::VariantId:
+    else if (destination.IsDirect())
+    {
+        return chipSendUnicast(destination.GetDirectNodeId(), apsFrame, messageLength, message);
+    }
+    else if (destination.IsViaExchange())
+    {
+        return chipSendUnicast(destination.GetExchangeContext(), apsFrame, messageLength, message);
+    }
+    else
+    {
         // No implementation yet.
-        status = EMBER_ERR_FATAL;
-        break;
-    case MessageSendDestination::VariantViaExchange::VariantId:
-        status = chipSendUnicast(destination.Get<MessageSendDestination::VariantViaExchange>().mExchangeContext, apsFrame,
-                                 messageLength, message);
-        break;
-    default:
-        status = EMBER_BAD_ARGUMENT;
-        break;
+        return EMBER_ERR_FATAL;
     }
-    return status;
 }
diff --git a/src/app/util/af-main.h b/src/app/util/af-main.h
@@ -123,7 +123,7 @@ void emAfIncomingMessageHandler(EmberIncomingMessageType type, EmberApsFrame * a
                                 uint16_t messageLength, uint8_t * messageContents);
 EmberStatus emAfSend(const chip::MessageSendDestination & destination, EmberApsFrame * apsFrame, uint8_t messageLength,
                      uint8_t * message, uint8_t * messageTag, EmberNodeId alias, uint8_t sequence);
-void emAfMessageSentHandler(chip::MessageSendDestination & destination, EmberApsFrame * apsFrame, EmberStatus status,
+void emAfMessageSentHandler(const chip::MessageSendDestination & destination, EmberApsFrame * apsFrame, EmberStatus status,
                             uint16_t messageLength, uint8_t * messageContents, uint8_t messageTag);
 
 void emAfStackStatusHandler(EmberStatus status);

diff --git a/src/app/util/af-types.h b/src/app/util/af-types.h
@@ -1259,6 +1259,40 @@ namespace chip {
 class MessageSendDestination
 {
 public:
+    MessageSendDestination(MessageSendDestination & that)       = default;
+    MessageSendDestination(const MessageSendDestination & that) = default;
+    MessageSendDestination(MessageSendDestination && that)      = default;
+
+    static MessageSendDestination ViaBinding(uint8_t bindingIndex)
+    {
+        return MessageSendDestination(VariantViaBinding(bindingIndex));
+    }
+
+    static MessageSendDestination Direct(NodeId nodeId) { return MessageSendDestination(VariantDirect(nodeId)); }
+
+    static MessageSendDestination Multicast(GroupId groupId) { return MessageSendDestination(VariantMulticast(groupId)); }
+
+    static MessageSendDestination MulticastWithAlias(GroupId groupId)
+    {
+        return MessageSendDestination(VariantMulticastWithAlias(groupId));
+    }
+
+    static MessageSendDestination ViaExchange(Messaging::ExchangeContext * exchangeContext)
+    {
+        return MessageSendDestination(VariantViaExchange(exchangeContext));
+    }
+
+    std::size_t GetType() const { return mDestination.GetType(); }
+
+    bool IsViaBinding() const { return mDestination.Is<VariantViaBinding>(); }
+    bool IsDirect() const { return mDestination.Is<VariantDirect>(); }
+    bool IsViaExchange() const { return mDestination.Is<VariantViaExchange>(); }
+
+    uint8_t GetBindingIndex() const { return mDestination.Get<VariantViaBinding>().mBindingIndex; }
+    NodeId GetDirectNodeId() const { return mDestination.Get<VariantDirect>().mNodeId; }
+    Messaging::ExchangeContext * GetExchangeContext() const { return mDestination.Get<VariantViaExchange>().mExchangeContext; }
+
+private:
     struct VariantViaBinding
     {
         static constexpr const std::size_t VariantId = 1;
@@ -1309,40 +1343,6 @@ class MessageSendDestination
         Messaging::ExchangeContext * mExchangeContext;
     };
 
-    MessageSendDestination(MessageSendDestination & that)       = default;
-    MessageSendDestination(const MessageSendDestination & that) = default;
-    MessageSendDestination(MessageSendDestination && that)      = default;
-
-    static MessageSendDestination ViaBinding(uint8_t bindingIndex)
-    {
-        return MessageSendDestination(VariantViaBinding(bindingIndex));
-    }
-
-    static MessageSendDestination Direct(NodeId nodeId) { return MessageSendDestination(VariantDirect(nodeId)); }
-
-    static MessageSendDestination Multicast(GroupId groupId) { return MessageSendDestination(VariantMulticast(groupId)); }
-
-    static MessageSendDestination MulticastWithAlias(GroupId groupId)
-    {
-        return MessageSendDestination(VariantMulticastWithAlias(groupId));
-    }
-
-    static MessageSendDestination ViaExchange(Messaging::ExchangeContext * exchangeContext)
-    {
-        return MessageSendDestination(VariantViaExchange(exchangeContext));
-    }
-
-    std::size_t GetType() const { return mDestination.GetType(); }
-
-    bool IsViaBinding() const { return mDestination.Is<VariantViaBinding>(); }
-
-    template <typename Destination>
-    const Destination & Get() const
-    {
-        return mDestination.Get<Destination>();
-    }
-
-private:
     template <typename Destination>
     MessageSendDestination(Destination && destination)
     {

diff --git a/src/app/util/attribute-storage.cpp b/src/app/util/attribute-storage.cpp
@@ -240,7 +240,7 @@ void emberAfClusterMessageSentWithMfgCodeCallback(const MessageSendDestination &
 // This function is used to call the per-cluster message sent callback, and
 // wraps the emberAfClusterMessageSentWithMfgCodeCallback with a
 // EMBER_AF_NULL_MANUFACTURER_CODE.
-void emberAfClusterMessageSentCallback(MessageSendDestination & destination, EmberApsFrame * apsFrame, uint16_t msgLen,
+void emberAfClusterMessageSentCallback(const MessageSendDestination & destination, EmberApsFrame * apsFrame, uint16_t msgLen,
                                        uint8_t * message, EmberStatus status)
 {
     emberAfClusterMessageSentWithMfgCodeCallback(destination, apsFrame, msgLen, message, status, EMBER_AF_NULL_MANUFACTURER_CODE);

diff --git a/src/app/util/client-api.cpp b/src/app/util/client-api.cpp
@@ -330,13 +330,13 @@ EmberStatus emberAfSendCommandMulticastToBindings(void)
     return emberAfSendMulticastToBindings(emAfCommandApsFrame, *emAfResponseLengthPtr, emAfZclBuffer);
 }
 
-// EmberStatus emberAfSendCommandUnicastWithCallback(chip::MessageSendDestination & destination, EmberAfMessageSentFunction
+// EmberStatus emberAfSendCommandUnicastWithCallback(const chip::MessageSendDestination & destination, EmberAfMessageSentFunction
 // callback)
 // {
 //     return emberAfSendUnicastWithCallback(destination, emAfCommandApsFrame, *emAfResponseLengthPtr, emAfZclBuffer, callback);
 // }
 
-// EmberStatus emberAfSendCommandUnicast(chip::MessageSendDestination & destination)
+// EmberStatus emberAfSendCommandUnicast(const chip::MessageSendDestination & destination)
 // {
 //     return emberAfSendCommandUnicastWithCallback(destination, NULL);
 // }