From 98094ad5d92c5031853a26dcc7fe9ea5efeb2be1 Mon Sep 17 00:00:00 2001
From: Ricardo Casallas <ricardo.casallas@silabs.com>
Date: Wed, 10 Nov 2021 10:41:06 -0500
Subject: [PATCH] PeerAddress: Code review comments applied.

---
 src/inet/tests/TestInetAddress.cpp | 24 ++++++++++++++++++++++++
 src/transport/raw/PeerAddress.h    |  6 +-----
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/src/inet/tests/TestInetAddress.cpp b/src/inet/tests/TestInetAddress.cpp
index f29ca49e2b5327..89f7ae60779a9b 100644
--- a/src/inet/tests/TestInetAddress.cpp
+++ b/src/inet/tests/TestInetAddress.cpp
@@ -25,6 +25,8 @@
  */
 
 #include <inet/IPAddress.h>
+#include <lib/core/DataModelTypes.h>
+#include <transport/raw/PeerAddress.h>
 
 #include <string.h>
 
@@ -839,6 +841,27 @@ void CheckIsIPv6LLA(nlTestSuite * inSuite, void * inContext)
  *  Test correct identification of IPv6 multicast addresses.
  */
 void CheckIsIPv6Multicast(nlTestSuite * inSuite, void * inContext)
+{
+    constexpr chip::FabricId fabric = 0xa1a2a4a8b1b2b4b8;
+    constexpr chip::GroupId group = 0xe10f;
+    chip::Transport::PeerAddress addr = chip::Transport::PeerAddress::Multicast(fabric, group);
+    NL_TEST_ASSERT(inSuite, chip::Transport::Type::kUdp == addr.GetTransportType());
+
+    const Inet::IPAddress &ip = addr.GetIPAddress();
+    NL_TEST_ASSERT(inSuite, ip.IsMulticast());
+    NL_TEST_ASSERT(inSuite, IPAddressType::kIPv6 == ip.Type());
+
+    constexpr uint8_t expected[NL_INET_IPV6_ADDR_LEN_IN_BYTES] = { 0xff, 0x35, 0x00, 0x40, 0xfd, 0xa1, 0xa2, 0xa4, 0xa8, 0xb1, 0xb2, 0xb4, 0xb8, 0x00, 0xe1, 0x0f };
+    uint8_t result[NL_INET_IPV6_ADDR_LEN_IN_BYTES];
+    uint8_t *p = result;
+    ip.WriteAddress(p);
+    NL_TEST_ASSERT(inSuite, !memcmp(expected, result, NL_INET_IPV6_ADDR_LEN_IN_BYTES ));
+}
+
+/**
+ *  Test correct identification of IPv6 multicast addresses.
+ */
+void CheckBuildIPv6Multicast(nlTestSuite * inSuite, void * inContext)
 {
     const struct TestContext * lContext       = static_cast<const struct TestContext *>(inContext);
     IPAddressExpandedContextIterator lCurrent = lContext->mIPAddressExpandedContextRange.mBegin;
@@ -1734,6 +1757,7 @@ const nlTest sTests[] =
     NL_TEST_DEF("IPv6 ULA Detection",                          CheckIsIPv6ULA),
     NL_TEST_DEF("IPv6 Link Local Detection",                   CheckIsIPv6LLA),
     NL_TEST_DEF("IPv6 Multicast Detection",                    CheckIsIPv6Multicast),
+    NL_TEST_DEF("IPv6 Multicast Build",                        CheckBuildIPv6Multicast),
     NL_TEST_DEF("Multicast Detection",                         CheckIsMulticast),
     NL_TEST_DEF("Equivalence Operator",                        CheckOperatorEqual),
     NL_TEST_DEF("Non-Equivalence Operator",                    CheckOperatorNotEqual),
diff --git a/src/transport/raw/PeerAddress.h b/src/transport/raw/PeerAddress.h
index 94c614818b40d7..ac153c4b5dca1b 100644
--- a/src/transport/raw/PeerAddress.h
+++ b/src/transport/raw/PeerAddress.h
@@ -201,10 +201,6 @@ class PeerAddress
         return TCP(addr).SetPort(port).SetInterface(interface);
     }
 
-    static PeerAddress Multicast(chip::FabricId fabric, chip::NodeId node)
-    {
-        return Multicast(fabric, static_cast<uint16_t>(node & 0xffff));
-    }
     static PeerAddress Multicast(chip::FabricId fabric, chip::GroupId group)
     {
         constexpr uint8_t scope        = 0x05; // Site-Local
@@ -217,7 +213,7 @@ class PeerAddress
         // * The lower 8-bits of the Fabric ID
         // * 0x00
         // * The 16-bits Group Identifier in big-endian order
-        uint32_t groupId = static_cast<uint32_t>((fabric << 24) & 0xff000000) | static_cast<uint32_t>(group & 0x0000ffff);
+        uint32_t groupId = static_cast<uint32_t>((fabric << 24) & 0xff000000) | group;
         return UDP(Inet::IPAddress::MakeIPv6PrefixMulticast(scope, prefixLength, prefix, groupId));
     }