Skip to content

Commit

Permalink
Addressed first part of review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
kkasperczyk-no committed Oct 6, 2021
1 parent 154a2ad commit f047caf
Show file tree
Hide file tree
Showing 8 changed files with 64 additions and 17 deletions.
10 changes: 6 additions & 4 deletions src/app/server/Mdns.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -244,13 +244,14 @@ CHIP_ERROR MdnsServer::AdvertiseOperational()
{
if (fabricInfo.IsInitialized())
{
uint8_t mac[8];
uint8_t macBuffer[DeviceLayer::ConfigurationManager::kMACAddressLength];
MutableByteSpan mac(macBuffer);
chip::DeviceLayer::ConfigurationMgr().GetPrimaryMACAddress(mac);

const auto advertiseParameters =
chip::Mdns::OperationalAdvertisingParameters()
.SetPeerId(fabricInfo.GetPeerId())
.SetMac(chip::ByteSpan(mac, 8))
.SetMac(mac)
.SetPort(GetSecuredPort())
.SetMRPRetryIntervals(Optional<uint32_t>(CHIP_CONFIG_MRP_DEFAULT_INITIAL_RETRY_INTERVAL),
Optional<uint32_t>(CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL))
Expand Down Expand Up @@ -282,9 +283,10 @@ CHIP_ERROR MdnsServer::Advertise(bool commissionableNode, chip::Mdns::Commission

char pairingInst[chip::Mdns::kKeyPairingInstructionMaxLength + 1];

uint8_t mac[8];
uint8_t macBuffer[DeviceLayer::ConfigurationManager::kMACAddressLength];
MutableByteSpan mac(macBuffer);
chip::DeviceLayer::ConfigurationMgr().GetPrimaryMACAddress(mac);
advertiseParameters.SetMac(chip::ByteSpan(mac, 8));
advertiseParameters.SetMac(mac);

uint16_t value;
if (DeviceLayer::ConfigurationMgr().GetVendorId(value) != CHIP_NO_ERROR)
Expand Down
10 changes: 8 additions & 2 deletions src/include/platform/ConfigurationManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@ class ConfigurationManager
kMaxPairingCodeLength = 16,
kMaxSerialNumberLength = 32,
kMaxFirmwareRevisionLength = 32,
#if CHIP_DEVICE_CONFIG_ENABLE_THREAD
kMACAddressLength = 8,
#else
kMACAddressLength = 6,
#endif
kMaxMACAddressLength = 8,
};

CHIP_ERROR GetVendorName(char * buf, size_t bufSize);
Expand All @@ -69,7 +75,7 @@ class ConfigurationManager
CHIP_ERROR GetProductRevisionString(char * buf, size_t bufSize);
CHIP_ERROR GetProductRevision(uint16_t & productRev);
CHIP_ERROR GetSerialNumber(char * buf, size_t bufSize, size_t & serialNumLen);
CHIP_ERROR GetPrimaryMACAddress(uint8_t (&buf)[8]);
CHIP_ERROR GetPrimaryMACAddress(MutableByteSpan buf);
CHIP_ERROR GetPrimaryWiFiMACAddress(uint8_t * buf);
CHIP_ERROR GetPrimary802154MACAddress(uint8_t * buf);
CHIP_ERROR GetManufacturingDate(uint16_t & year, uint8_t & month, uint8_t & dayOfMonth);
Expand Down Expand Up @@ -266,7 +272,7 @@ inline CHIP_ERROR ConfigurationManager::GetSerialNumber(char * buf, size_t bufSi
return static_cast<ImplClass *>(this)->_GetSerialNumber(buf, bufSize, serialNumLen);
}

inline CHIP_ERROR ConfigurationManager::GetPrimaryMACAddress(uint8_t (&buf)[8])
inline CHIP_ERROR ConfigurationManager::GetPrimaryMACAddress(MutableByteSpan buf)
{
return static_cast<ImplClass *>(this)->_GetPrimaryMACAddress(buf);
}
Expand Down
20 changes: 13 additions & 7 deletions src/include/platform/internal/GenericConfigurationManagerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -231,24 +231,30 @@ CHIP_ERROR GenericConfigurationManagerImpl<ImplClass>::_StorePrimaryWiFiMACAddre
}

template <class ImplClass>
CHIP_ERROR GenericConfigurationManagerImpl<ImplClass>::_GetPrimaryMACAddress(uint8_t (&buf)[8])
CHIP_ERROR GenericConfigurationManagerImpl<ImplClass>::_GetPrimaryMACAddress(MutableByteSpan buf)
{
memset(buf, 0, 8);
if (buf.size() != ConfigurationManager::kMACAddressLength)
return CHIP_ERROR_INVALID_ARGUMENT;

memset(buf.data(), 0, buf.size());

#if CHIP_DEVICE_CONFIG_ENABLE_THREAD
if (chip::DeviceLayer::ThreadStackMgr().GetPrimary802154MACAddress(buf) == CHIP_NO_ERROR)
if (chip::DeviceLayer::ThreadStackMgr().GetPrimary802154MACAddress(buf.data()) == CHIP_NO_ERROR)
{
ChipLogDetail(DeviceLayer, "Using Thread extended MAC for hostname.");
return CHIP_NO_ERROR;
}
#endif
if (DeviceLayer::ConfigurationMgr().GetPrimaryWiFiMACAddress(buf) == CHIP_NO_ERROR)
#else
if (DeviceLayer::ConfigurationMgr().GetPrimaryWiFiMACAddress(buf.data()) == CHIP_NO_ERROR)
{
ChipLogDetail(DeviceLayer, "Using wifi MAC for hostname");
return CHIP_NO_ERROR;
}
#endif

ChipLogError(DeviceLayer, "MAC is not known, using a default.");
uint8_t temp[6] = { 0xEE, 0xAA, 0xBA, 0xDA, 0xBA, 0xD0 };
memcpy(buf, temp, 6);
uint8_t temp[ConfigurationManager::kMaxMACAddressLength] = { 0xEE, 0xAA, 0xBA, 0xDA, 0xBA, 0xD0, 0xDD, 0xCA };
memcpy(buf.data(), temp, buf.size());
return CHIP_NO_ERROR;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ class GenericConfigurationManagerImpl
uint8_t & second);
CHIP_ERROR _GetSerialNumber(char * buf, size_t bufSize, size_t & serialNumLen);
CHIP_ERROR _StoreSerialNumber(const char * serialNum, size_t serialNumLen);
CHIP_ERROR _GetPrimaryMACAddress(uint8_t (&buf)[8]);
CHIP_ERROR _GetPrimaryMACAddress(MutableByteSpan buf);
CHIP_ERROR _GetPrimaryWiFiMACAddress(uint8_t * buf);
CHIP_ERROR _StorePrimaryWiFiMACAddress(const uint8_t * buf);
CHIP_ERROR _GetPrimary802154MACAddress(uint8_t * buf);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1819,6 +1819,7 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::_ClearSrpHost(co
Impl()->LockThreadStack();

VerifyOrExit(aHostName, error = CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrExit(strlen(aHostName) <= SrpClient::kMaxHostNameSize, error = CHIP_ERROR_INVALID_STRING_LENGTH);
VerifyOrExit(mSrpClient.mInitializedCallback, error = CHIP_ERROR_INCORRECT_STATE);

// Add host and remove it with notifying SRP server to clean old information related to the host.
Expand Down
5 changes: 3 additions & 2 deletions src/platform/OpenThread/MdnsImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,11 @@ CHIP_ERROR ChipMdnsInit(MdnsAsyncReturnCallback initCallback, MdnsAsyncReturnCal
{
ReturnErrorOnFailure(ThreadStackMgr().SetSrpDnsCallbacks(initCallback, errorCallback, context));

uint8_t mac[8];
uint8_t macBuffer[ConfigurationManager::kMACAddressLength];
MutableByteSpan mac(macBuffer);
char hostname[kMdnsHostNameMaxSize + 1] = "";
ReturnErrorOnFailure(DeviceLayer::ConfigurationMgr().GetPrimaryMACAddress(mac));
MakeHostName(hostname, sizeof(hostname), chip::ByteSpan(mac, 8));
MakeHostName(hostname, sizeof(hostname), mac);

return ThreadStackMgr().ClearSrpHost(hostname);
}
Expand Down
2 changes: 1 addition & 1 deletion src/platform/fake/ConfigurationManagerImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class ConfigurationManagerImpl final : public ConfigurationManager
}
CHIP_ERROR _GetSerialNumber(char * buf, size_t bufSize, size_t & serialNumLen) { return CHIP_ERROR_NOT_IMPLEMENTED; }
CHIP_ERROR _StoreSerialNumber(const char * serialNum, size_t serialNumLen) { return CHIP_ERROR_NOT_IMPLEMENTED; }
CHIP_ERROR _GetPrimaryMACAddress(uint8_t (&buf)[8]) { return CHIP_ERROR_NOT_IMPLEMENTED; }
CHIP_ERROR _GetPrimaryMACAddress(MutableByteSpan buf) { return CHIP_ERROR_NOT_IMPLEMENTED; }
CHIP_ERROR _GetPrimaryWiFiMACAddress(uint8_t * buf) { return CHIP_ERROR_NOT_IMPLEMENTED; }
CHIP_ERROR _StorePrimaryWiFiMACAddress(const uint8_t * buf) { return CHIP_ERROR_NOT_IMPLEMENTED; }
CHIP_ERROR _GetPrimary802154MACAddress(uint8_t * buf) { return CHIP_ERROR_NOT_IMPLEMENTED; }
Expand Down
31 changes: 31 additions & 0 deletions src/platform/tests/TestConfigurationMgr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,36 @@ static void TestConfigurationMgr_Breadcrumb(nlTestSuite * inSuite, void * inCont
NL_TEST_ASSERT(inSuite, breadcrumb == 12345);
}

static void TestConfigurationMgr_GetPrimaryMACAddress(nlTestSuite * inSuite, void * inContext)
{
CHIP_ERROR err = CHIP_NO_ERROR;
const uint8_t defaultMacAddress[8] = {0xEE, 0xAA, 0xBA, 0xDA, 0xBA, 0xD0, 0xDD, 0xCA };
uint8_t macBuffer8Bytes[8];
uint8_t macBuffer6Bytes[6];
MutableByteSpan mac8Bytes(macBuffer8Bytes);
MutableByteSpan mac6Bytes(macBuffer6Bytes);

err = ConfigurationMgr().GetPrimaryMACAddress(mac8Bytes);
if (mac8Bytes.size() != ConfigurationManager::kMACAddressLength) {
NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_INVALID_ARGUMENT);
} else {
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);

// Verify default MAC address value
NL_TEST_ASSERT(inSuite, strncmp(reinterpret_cast<char*>(mac8Bytes.data()), reinterpret_cast<const char*>(defaultMacAddress), mac8Bytes.size()) == 0);
}

err = ConfigurationMgr().GetPrimaryMACAddress(mac6Bytes);
if (mac6Bytes.size() != ConfigurationManager::kMACAddressLength) {
NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_INVALID_ARGUMENT);
} else {
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);

// Verify default MAC address value
NL_TEST_ASSERT(inSuite, strncmp(reinterpret_cast<char*>(mac6Bytes.data()), reinterpret_cast<const char*>(defaultMacAddress), mac6Bytes.size()) == 0);
}
}

/**
* Test Suite. It lists all the test functions.
*/
Expand All @@ -467,6 +497,7 @@ static const nlTest sTests[] = {
NL_TEST_DEF("Test ConfigurationMgr::RegulatoryLocation", TestConfigurationMgr_RegulatoryLocation),
NL_TEST_DEF("Test ConfigurationMgr::CountryCode", TestConfigurationMgr_CountryCode),
NL_TEST_DEF("Test ConfigurationMgr::Breadcrumb", TestConfigurationMgr_Breadcrumb),
NL_TEST_DEF("Test ConfigurationMgr::GetPrimaryMACAddress", TestConfigurationMgr_GetPrimaryMACAddress),
NL_TEST_SENTINEL()
};

Expand Down

0 comments on commit f047caf

Please sign in to comment.