From 2190336c94f2f40367fc26590f29ec3d0044853a Mon Sep 17 00:00:00 2001 From: Kamil Kasperczyk <66371704+kkasperczyk-no@users.noreply.github.com> Date: Mon, 16 May 2022 15:15:16 +0200 Subject: [PATCH] Refactored OnboardingCodesUtil to decrease memory footprint. (#18318) * Refactored OnboardingCodesUtil to decrease memory footprint. OnboardingCodesUtil uses SetupPayload object that contains a lot unnecessary data and takes a lot of flash. Replaced using SetupPayload with PayloadContents and QRCodeSetupPayloadGenerator with QRCodeBasicSetupPayloadGenerator. It allowed to save about 3.28 k of flash for nrfconnect platform. * Fixed linux builds * Changed char span size for codes buffers and fixed builds * Added documentation for OnboardingCodesUtil methods * Changed char span sizes returned by codes encoding methods to not include null terminator * Aligned examples using GetQRCode and GetManualPairingCode to the new API. * Fixed EFR32 build * Addressed review comments regarding reducing span size. * Fixed unit tests to reset span size after every encoding call. --- .../esp32/main/DeviceWithDisplay.cpp | 10 ++- examples/chef/esp32/main/main.cpp | 9 ++- examples/common/pigweed/rpc_services/Device.h | 7 +- .../light-switch-app/efr32/src/AppTask.cpp | 6 +- examples/lighting-app/efr32/src/AppTask.cpp | 6 +- examples/lock-app/efr32/src/AppTask.cpp | 6 +- .../ota-requestor-app/efr32/src/AppTask.cpp | 6 +- examples/platform/linux/AppMain.cpp | 4 +- examples/platform/linux/Options.h | 2 +- .../window-app/efr32/include/WindowAppImpl.h | 5 +- .../window-app/efr32/src/WindowAppImpl.cpp | 5 +- src/app/server/OnboardingCodesUtil.cpp | 69 ++++++++++--------- src/app/server/OnboardingCodesUtil.h | 44 ++++++++++-- src/controller/CommissioningWindowOpener.cpp | 2 +- src/lib/shell/commands/OnboardingCodes.cpp | 26 ++++--- src/setup_payload/Base38Encode.cpp | 2 + src/setup_payload/Base38Encode.h | 10 ++- .../ManualSetupPayloadGenerator.cpp | 4 ++ .../ManualSetupPayloadGenerator.h | 2 + .../QRCodeSetupPayloadGenerator.h | 2 + src/setup_payload/tests/TestQRCode.cpp | 50 +++++++++----- 21 files changed, 186 insertions(+), 91 deletions(-) diff --git a/examples/all-clusters-app/esp32/main/DeviceWithDisplay.cpp b/examples/all-clusters-app/esp32/main/DeviceWithDisplay.cpp index 55a77f4d3aaef4..987a095134b16d 100644 --- a/examples/all-clusters-app/esp32/main/DeviceWithDisplay.cpp +++ b/examples/all-clusters-app/esp32/main/DeviceWithDisplay.cpp @@ -17,6 +17,7 @@ */ #include "DeviceWithDisplay.h" +#include #if CONFIG_HAVE_DISPLAY using namespace ::chip; @@ -651,8 +652,11 @@ esp_err_t InitM5Stack(std::string qrCodeText) void InitDeviceDisplay() { - std::string qrCodeText; + // Create buffer for QR code that can fit max size and null terminator. + char qrCodeBuffer[chip::QRCodeBasicSetupPayloadGenerator::kMaxQRCodeBase38RepresentationLength + 1]; + chip::MutableCharSpan qrCodeText(qrCodeBuffer); + // Get QR Code and emulate its content using NFC tag GetQRCode(qrCodeText, chip::RendezvousInformationFlags(CONFIG_RENDEZVOUS_MODE)); // Initialize the display device. @@ -681,12 +685,12 @@ void InitDeviceDisplay() #if CONFIG_DEVICE_TYPE_M5STACK - InitM5Stack(qrCodeText); + InitM5Stack(qrCodeText.data()); #elif CONFIG_DEVICE_TYPE_ESP32_WROVER_KIT // Display the QR Code - QRCodeScreen qrCodeScreen(qrCodeText); + QRCodeScreen qrCodeScreen(qrCodeText.data()); qrCodeScreen.Display(); #endif diff --git a/examples/chef/esp32/main/main.cpp b/examples/chef/esp32/main/main.cpp index 03751d641dafd1..5fd864313caa8a 100644 --- a/examples/chef/esp32/main/main.cpp +++ b/examples/chef/esp32/main/main.cpp @@ -45,6 +45,7 @@ #include #include #include +#include #include "Display.h" #include "QRCodeScreen.h" @@ -137,7 +138,9 @@ const char * TAG = "chef-app"; #if CONFIG_HAVE_DISPLAY void printQRCode() { - std::string qrCodeText; + // Create buffer for QR code that can fit max size and null terminator. + char qrCodeBuffer[chip::QRCodeBasicSetupPayloadGenerator::kMaxQRCodeBase38RepresentationLength + 1]; + chip::MutableCharSpan qrCodeText(qrCodeBuffer); GetQRCode(qrCodeText, chip::RendezvousInformationFlags(CONFIG_RENDEZVOUS_MODE)); @@ -153,8 +156,8 @@ void printQRCode() ScreenManager::Init(); ESP_LOGI(TAG, "Opening QR code screen"); - ESP_LOGI(TAG, "QR CODE Text: '%s'", qrCodeText.c_str()); - ScreenManager::PushScreen(chip::Platform::New(qrCodeText)); + ESP_LOGI(TAG, "QR CODE Text: '%s'", qrCodeText.data()); + ScreenManager::PushScreen(chip::Platform::New(qrCodeText.data())); } #endif // CONFIG_HAVE_DISPLAY diff --git a/examples/common/pigweed/rpc_services/Device.h b/examples/common/pigweed/rpc_services/Device.h index e0979365529f3f..57ef6a327557e8 100644 --- a/examples/common/pigweed/rpc_services/Device.h +++ b/examples/common/pigweed/rpc_services/Device.h @@ -28,6 +28,7 @@ #include "platform/ConfigurationManager.h" #include "platform/DiagnosticDataProvider.h" #include "platform/PlatformManager.h" +#include namespace chip { namespace rpc { @@ -146,10 +147,12 @@ class Device : public pw_rpc::nanopb::Device::Service snprintf(response.serial_number, sizeof(response.serial_number), CHIP_DEVICE_CONFIG_TEST_SERIAL_NUMBER); } - std::string qrCodeText; + // Create buffer for QR code that can fit max size and null terminator. + char qrCodeBuffer[chip::QRCodeBasicSetupPayloadGenerator::kMaxQRCodeBase38RepresentationLength + 1]; + chip::MutableCharSpan qrCodeText(qrCodeBuffer); if (GetQRCode(qrCodeText, chip::RendezvousInformationFlags(chip::RendezvousInformationFlag::kBLE)) == CHIP_NO_ERROR) { - snprintf(response.pairing_info.qr_code, sizeof(response.pairing_info.qr_code), "%s", qrCodeText.c_str()); + snprintf(response.pairing_info.qr_code, sizeof(response.pairing_info.qr_code), "%s", qrCodeText.data()); GetQRCodeUrl(response.pairing_info.qr_code_url, sizeof(response.pairing_info.qr_code_url), qrCodeText); response.has_pairing_info = true; } diff --git a/examples/light-switch-app/efr32/src/AppTask.cpp b/examples/light-switch-app/efr32/src/AppTask.cpp index ea6ce7073eb128..0de6074a6eaa3b 100644 --- a/examples/light-switch-app/efr32/src/AppTask.cpp +++ b/examples/light-switch-app/efr32/src/AppTask.cpp @@ -239,11 +239,13 @@ CHIP_ERROR AppTask::Init() // Print setup info on LCD if available #ifdef DISPLAY_ENABLED - std::string QRCode; + // Create buffer for QR code that can fit max size and null terminator. + char qrCodeBuffer[chip::QRCodeBasicSetupPayloadGenerator::kMaxQRCodeBase38RepresentationLength + 1]; + chip::MutableCharSpan QRCode(qrCodeBuffer); if (GetQRCode(QRCode, chip::RendezvousInformationFlags(chip::RendezvousInformationFlag::kBLE)) == CHIP_NO_ERROR) { - LCDWriteQRCode((uint8_t *) QRCode.c_str()); + LCDWriteQRCode((uint8_t *) QRCode.data()); } else { diff --git a/examples/lighting-app/efr32/src/AppTask.cpp b/examples/lighting-app/efr32/src/AppTask.cpp index c61198b6954ce9..d9c5de9bc766b1 100644 --- a/examples/lighting-app/efr32/src/AppTask.cpp +++ b/examples/lighting-app/efr32/src/AppTask.cpp @@ -283,11 +283,13 @@ CHIP_ERROR AppTask::Init() // Print setup info on LCD if available #ifdef DISPLAY_ENABLED - std::string QRCode; + // Create buffer for QR code that can fit max size and null terminator. + char qrCodeBuffer[chip::QRCodeBasicSetupPayloadGenerator::kMaxQRCodeBase38RepresentationLength + 1]; + chip::MutableCharSpan QRCode(qrCodeBuffer); if (GetQRCode(QRCode, chip::RendezvousInformationFlags(chip::RendezvousInformationFlag::kBLE)) == CHIP_NO_ERROR) { - LCDWriteQRCode((uint8_t *) QRCode.c_str()); + LCDWriteQRCode((uint8_t *) QRCode.data()); } else { diff --git a/examples/lock-app/efr32/src/AppTask.cpp b/examples/lock-app/efr32/src/AppTask.cpp index 066d5704111bb1..18da28e917740b 100644 --- a/examples/lock-app/efr32/src/AppTask.cpp +++ b/examples/lock-app/efr32/src/AppTask.cpp @@ -274,11 +274,13 @@ CHIP_ERROR AppTask::Init() // Print setup info on LCD if available #ifdef DISPLAY_ENABLED - std::string QRCode; + // Create buffer for QR code that can fit max size and null terminator. + char qrCodeBuffer[chip::QRCodeBasicSetupPayloadGenerator::kMaxQRCodeBase38RepresentationLength + 1]; + chip::MutableCharSpan QRCode(qrCodeBuffer); if (GetQRCode(QRCode, chip::RendezvousInformationFlags(chip::RendezvousInformationFlag::kBLE)) == CHIP_NO_ERROR) { - LCDWriteQRCode((uint8_t *) QRCode.c_str()); + LCDWriteQRCode((uint8_t *) QRCode.data()); } else { diff --git a/examples/ota-requestor-app/efr32/src/AppTask.cpp b/examples/ota-requestor-app/efr32/src/AppTask.cpp index 558db22f1860b5..e3255f3eace0df 100644 --- a/examples/ota-requestor-app/efr32/src/AppTask.cpp +++ b/examples/ota-requestor-app/efr32/src/AppTask.cpp @@ -170,11 +170,13 @@ CHIP_ERROR AppTask::Init() // Print setup info on LCD if available #ifdef DISPLAY_ENABLED - std::string QRCode; + // Create buffer for QR code that can fit max size and null terminator. + char qrCodeBuffer[chip::QRCodeBasicSetupPayloadGenerator::kMaxQRCodeBase38RepresentationLength + 1]; + chip::MutableCharSpan QRCode(qrCodeBuffer); if (GetQRCode(QRCode, chip::RendezvousInformationFlags(chip::RendezvousInformationFlag::kBLE)) == CHIP_NO_ERROR) { - LCDWriteQRCode((uint8_t *) QRCode.c_str()); + LCDWriteQRCode((uint8_t *) QRCode.data()); } else { diff --git a/examples/platform/linux/AppMain.cpp b/examples/platform/linux/AppMain.cpp index aa5a1622e92ee6..83f867f9cdc86c 100644 --- a/examples/platform/linux/AppMain.cpp +++ b/examples/platform/linux/AppMain.cpp @@ -241,7 +241,7 @@ int ChipLinuxAppInit(int argc, char * const argv[], OptionSet * customOptions) LinuxDeviceOptions::GetInstance()); SuccessOrExit(err); - err = GetSetupPayload(LinuxDeviceOptions::GetInstance().payload, rendezvousFlags); + err = GetPayloadContents(LinuxDeviceOptions::GetInstance().payload, rendezvousFlags); SuccessOrExit(err); ConfigurationMgr().LogDeviceConfig(); @@ -254,7 +254,7 @@ int ChipLinuxAppInit(int argc, char * const argv[], OptionSet * customOptions) { // For testing of manual pairing code with custom commissioning flow ChipLogProgress(NotSpecified, "==== Onboarding payload for Custom Commissioning Flows ===="); - err = GetSetupPayload(LinuxDeviceOptions::GetInstance().payload, rendezvousFlags); + err = GetPayloadContents(LinuxDeviceOptions::GetInstance().payload, rendezvousFlags); SuccessOrExit(err); LinuxDeviceOptions::GetInstance().payload.commissioningFlow = chip::CommissioningFlow::kCustom; diff --git a/examples/platform/linux/Options.h b/examples/platform/linux/Options.h index 9b46d828740520..4f55f6f7f54e7a 100644 --- a/examples/platform/linux/Options.h +++ b/examples/platform/linux/Options.h @@ -38,7 +38,7 @@ struct LinuxDeviceOptions { - chip::SetupPayload payload; + chip::PayloadContents payload; chip::Optional discriminator; chip::Optional> spake2pVerifier; chip::Optional> spake2pSalt; diff --git a/examples/window-app/efr32/include/WindowAppImpl.h b/examples/window-app/efr32/include/WindowAppImpl.h index 7c03131955b524..a052838add1c89 100644 --- a/examples/window-app/efr32/include/WindowAppImpl.h +++ b/examples/window-app/efr32/include/WindowAppImpl.h @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -77,7 +78,9 @@ class WindowAppImpl : public WindowApp QueueHandle_t mQueue = nullptr; LEDWidget mStatusLED; LEDWidget mActionLED; - std::string mQRCode; + + // Get QR Code and emulate its content using NFC tag + char mQRCodeBuffer[chip::QRCodeBasicSetupPayloadGenerator::kMaxQRCodeBase38RepresentationLength + 1]; Timer mIconTimer; LcdIcon mIcon = LcdIcon::None; }; diff --git a/examples/window-app/efr32/src/WindowAppImpl.cpp b/examples/window-app/efr32/src/WindowAppImpl.cpp index 5cfc4b6206c760..2330bade5d2506 100644 --- a/examples/window-app/efr32/src/WindowAppImpl.cpp +++ b/examples/window-app/efr32/src/WindowAppImpl.cpp @@ -433,9 +433,10 @@ void WindowAppImpl::UpdateLCD() #ifdef QR_CODE_ENABLED else { - if (GetQRCode(mQRCode, chip::RendezvousInformationFlags(chip::RendezvousInformationFlag::kBLE)) == CHIP_NO_ERROR) + chip::MutableCharSpan qrCode(mQRCodeBuffer); + if (GetQRCode(qrCode, chip::RendezvousInformationFlags(chip::RendezvousInformationFlag::kBLE)) == CHIP_NO_ERROR) { - LCDWriteQRCode((uint8_t *) mQRCode.c_str()); + LCDWriteQRCode((uint8_t *) qrCode.data()); } } #endif // QR_CODE_ENABLED diff --git a/src/app/server/OnboardingCodesUtil.cpp b/src/app/server/OnboardingCodesUtil.cpp index 9cfd99999f7e62..28451e207dbd36 100644 --- a/src/app/server/OnboardingCodesUtil.cpp +++ b/src/app/server/OnboardingCodesUtil.cpp @@ -37,21 +37,21 @@ using namespace ::chip::DeviceLayer; void PrintOnboardingCodes(chip::RendezvousInformationFlags aRendezvousFlags) { - chip::SetupPayload payload; + chip::PayloadContents payload; - CHIP_ERROR err = GetSetupPayload(payload, aRendezvousFlags); + CHIP_ERROR err = GetPayloadContents(payload, aRendezvousFlags); if (err != CHIP_NO_ERROR) { - ChipLogError(AppServer, "GetSetupPayload() failed: %s", chip::ErrorStr(err)); + ChipLogError(AppServer, "GetPayloadContents() failed: %s", chip::ErrorStr(err)); } PrintOnboardingCodes(payload); } -void PrintOnboardingCodes(const chip::SetupPayload & payload) +void PrintOnboardingCodes(const chip::PayloadContents & payload) { - std::string qrCode; - std::string manualPairingCode; + char payloadBuffer[chip::QRCodeBasicSetupPayloadGenerator::kMaxQRCodeBase38RepresentationLength + 1]; + chip::MutableCharSpan qrCode(payloadBuffer); if (GetQRCode(qrCode, payload) == CHIP_NO_ERROR) { @@ -59,7 +59,7 @@ void PrintOnboardingCodes(const chip::SetupPayload & payload) const size_t qrCodeBufferMaxSize = strlen(kQrCodeBaseUrl) + strlen(kUrlDataAssignmentPhrase) + 3 * qrCode.size() + 1; qrCodeBuffer.Alloc(qrCodeBufferMaxSize); - ChipLogProgress(AppServer, "SetupQRCode: [%s]", qrCode.c_str()); + ChipLogProgress(AppServer, "SetupQRCode: [%s]", qrCode.data()); if (GetQRCodeUrl(qrCodeBuffer.Get(), qrCodeBufferMaxSize, qrCode) == CHIP_NO_ERROR) { ChipLogProgress(AppServer, "Copy/paste the below URL in a browser to see the QR Code:"); @@ -71,9 +71,10 @@ void PrintOnboardingCodes(const chip::SetupPayload & payload) ChipLogError(AppServer, "Getting QR code failed!"); } + chip::MutableCharSpan manualPairingCode(payloadBuffer); if (GetManualPairingCode(manualPairingCode, payload) == CHIP_NO_ERROR) { - ChipLogProgress(AppServer, "Manual pairing code: [%s]", manualPairingCode.c_str()); + ChipLogProgress(AppServer, "Manual pairing code: [%s]", manualPairingCode.data()); } else { @@ -85,47 +86,49 @@ void PrintOnboardingCodes(const chip::SetupPayload & payload) void ShareQRCodeOverNFC(chip::RendezvousInformationFlags aRendezvousFlags) { // Get QR Code and emulate its content using NFC tag - std::string qrCode; + char payloadBuffer[chip::QRCodeBasicSetupPayloadGenerator::kMaxQRCodeBase38RepresentationLength + 1]; + chip::MutableCharSpan qrCode(payloadBuffer); + ReturnOnFailure(GetQRCode(qrCode, chip::RendezvousInformationFlags(chip::RendezvousInformationFlag::kBLE))); - ReturnOnFailure(NFCMgr().StartTagEmulation(qrCode.c_str(), qrCode.size())); + ReturnOnFailure(NFCMgr().StartTagEmulation(qrCode.data(), qrCode.size())); } #endif -CHIP_ERROR GetSetupPayload(chip::SetupPayload & aSetupPayload, chip::RendezvousInformationFlags aRendezvousFlags) +CHIP_ERROR GetPayloadContents(chip::PayloadContents & aPayload, chip::RendezvousInformationFlags aRendezvousFlags) { - CHIP_ERROR err = CHIP_NO_ERROR; - aSetupPayload.version = 0; - aSetupPayload.rendezvousInformation = aRendezvousFlags; + CHIP_ERROR err = CHIP_NO_ERROR; + aPayload.version = 0; + aPayload.rendezvousInformation = aRendezvousFlags; - err = GetCommissionableDataProvider()->GetSetupPasscode(aSetupPayload.setUpPINCode); + err = GetCommissionableDataProvider()->GetSetupPasscode(aPayload.setUpPINCode); if (err != CHIP_NO_ERROR) { ChipLogError(AppServer, "GetCommissionableDataProvider()->GetSetupPasscode() failed: %s", chip::ErrorStr(err)); #if defined(CHIP_DEVICE_CONFIG_USE_TEST_SETUP_PIN_CODE) && CHIP_DEVICE_CONFIG_USE_TEST_SETUP_PIN_CODE ChipLogProgress(AppServer, "*** Using default EXAMPLE passcode %u ***", static_cast(CHIP_DEVICE_CONFIG_USE_TEST_SETUP_PIN_CODE)); - aSetupPayload.setUpPINCode = CHIP_DEVICE_CONFIG_USE_TEST_SETUP_PIN_CODE; + aPayload.setUpPINCode = CHIP_DEVICE_CONFIG_USE_TEST_SETUP_PIN_CODE; #else return err; #endif } - err = GetCommissionableDataProvider()->GetSetupDiscriminator(aSetupPayload.discriminator); + err = GetCommissionableDataProvider()->GetSetupDiscriminator(aPayload.discriminator); if (err != CHIP_NO_ERROR) { ChipLogError(AppServer, "GetCommissionableDataProvider()->GetSetupDiscriminator() failed: %s", chip::ErrorStr(err)); return err; } - err = ConfigurationMgr().GetVendorId(aSetupPayload.vendorID); + err = ConfigurationMgr().GetVendorId(aPayload.vendorID); if (err != CHIP_NO_ERROR) { ChipLogError(AppServer, "ConfigurationMgr().GetVendorId() failed: %s", chip::ErrorStr(err)); return err; } - err = ConfigurationMgr().GetProductId(aSetupPayload.productID); + err = ConfigurationMgr().GetProductId(aPayload.productID); if (err != CHIP_NO_ERROR) { ChipLogError(AppServer, "ConfigurationMgr().GetProductId() failed: %s", chip::ErrorStr(err)); @@ -135,25 +138,23 @@ CHIP_ERROR GetSetupPayload(chip::SetupPayload & aSetupPayload, chip::RendezvousI return err; } -CHIP_ERROR GetQRCode(std::string & aQRCode, chip::RendezvousInformationFlags aRendezvousFlags) +CHIP_ERROR GetQRCode(chip::MutableCharSpan & aQRCode, chip::RendezvousInformationFlags aRendezvousFlags) { - chip::SetupPayload payload; + chip::PayloadContents payload; - CHIP_ERROR err = GetSetupPayload(payload, aRendezvousFlags); + CHIP_ERROR err = GetPayloadContents(payload, aRendezvousFlags); if (err != CHIP_NO_ERROR) { - ChipLogError(AppServer, "GetSetupPayload() failed: %s", chip::ErrorStr(err)); + ChipLogError(AppServer, "GetPayloadContents() failed: %s", chip::ErrorStr(err)); return err; } return GetQRCode(aQRCode, payload); } -CHIP_ERROR GetQRCode(std::string & aQRCode, const chip::SetupPayload & payload) +CHIP_ERROR GetQRCode(chip::MutableCharSpan & aQRCode, const chip::PayloadContents & payload) { - // TODO: Usage of STL will significantly increase the image size, this should be changed to more efficient method for - // generating payload - CHIP_ERROR err = chip::QRCodeSetupPayloadGenerator(payload).payloadBase38Representation(aQRCode); + CHIP_ERROR err = chip::QRCodeBasicSetupPayloadGenerator(payload).payloadBase38Representation(aQRCode); if (err != CHIP_NO_ERROR) { ChipLogError(AppServer, "Generating QR Code failed: %s", chip::ErrorStr(err)); @@ -163,7 +164,7 @@ CHIP_ERROR GetQRCode(std::string & aQRCode, const chip::SetupPayload & payload) return CHIP_NO_ERROR; } -CHIP_ERROR GetQRCodeUrl(char * aQRCodeUrl, size_t aUrlMaxSize, const std::string & aQRCode) +CHIP_ERROR GetQRCodeUrl(char * aQRCodeUrl, size_t aUrlMaxSize, const chip::CharSpan & aQRCode) { VerifyOrReturnError(aQRCodeUrl, CHIP_ERROR_INVALID_ARGUMENT); VerifyOrReturnError(aUrlMaxSize >= (strlen(kQrCodeBaseUrl) + strlen(kUrlDataAssignmentPhrase) + aQRCode.size() + 1), @@ -173,24 +174,24 @@ CHIP_ERROR GetQRCodeUrl(char * aQRCodeUrl, size_t aUrlMaxSize, const std::string VerifyOrReturnError((writtenDataSize > 0) && (static_cast(writtenDataSize) < aUrlMaxSize), CHIP_ERROR_INVALID_STRING_LENGTH); - return EncodeQRCodeToUrl(aQRCode.c_str(), aQRCode.size(), aQRCodeUrl + writtenDataSize, + return EncodeQRCodeToUrl(aQRCode.data(), aQRCode.size(), aQRCodeUrl + writtenDataSize, aUrlMaxSize - static_cast(writtenDataSize)); } -CHIP_ERROR GetManualPairingCode(std::string & aManualPairingCode, chip::RendezvousInformationFlags aRendezvousFlags) +CHIP_ERROR GetManualPairingCode(chip::MutableCharSpan & aManualPairingCode, chip::RendezvousInformationFlags aRendezvousFlags) { - chip::SetupPayload payload; + chip::PayloadContents payload; - CHIP_ERROR err = GetSetupPayload(payload, aRendezvousFlags); + CHIP_ERROR err = GetPayloadContents(payload, aRendezvousFlags); if (err != CHIP_NO_ERROR) { - ChipLogError(AppServer, "GetSetupPayload() failed: %s", chip::ErrorStr(err)); + ChipLogError(AppServer, "GetPayloadContents() failed: %s", chip::ErrorStr(err)); return err; } return GetManualPairingCode(aManualPairingCode, payload); } -CHIP_ERROR GetManualPairingCode(std::string & aManualPairingCode, const chip::SetupPayload & payload) +CHIP_ERROR GetManualPairingCode(chip::MutableCharSpan & aManualPairingCode, const chip::PayloadContents & payload) { CHIP_ERROR err = chip::ManualSetupPayloadGenerator(payload).payloadDecimalStringRepresentation(aManualPairingCode); if (err != CHIP_NO_ERROR) diff --git a/src/app/server/OnboardingCodesUtil.h b/src/app/server/OnboardingCodesUtil.h index e586b852c132eb..78d170c0840435 100644 --- a/src/app/server/OnboardingCodesUtil.h +++ b/src/app/server/OnboardingCodesUtil.h @@ -17,17 +17,47 @@ #pragma once +#include #include void PrintOnboardingCodes(chip::RendezvousInformationFlags aRendezvousFlags); -void PrintOnboardingCodes(const chip::SetupPayload & payload); +void PrintOnboardingCodes(const chip::PayloadContents & payload); void ShareQRCodeOverNFC(chip::RendezvousInformationFlags aRendezvousFlags); -CHIP_ERROR GetQRCode(std::string & aQRCode, chip::RendezvousInformationFlags aRendezvousFlags); -CHIP_ERROR GetQRCode(std::string & aQRCode, const chip::SetupPayload & payload); -CHIP_ERROR GetQRCodeUrl(char * aQRCodeUrl, size_t aUrlMaxSize, const std::string & aQRCode); -CHIP_ERROR GetManualPairingCode(std::string & aManualPairingCode, chip::RendezvousInformationFlags aRendezvousFlags); -CHIP_ERROR GetManualPairingCode(std::string & aManualPairingCode, const chip::SetupPayload & payload); -CHIP_ERROR GetSetupPayload(chip::SetupPayload & aSetupPayload, chip::RendezvousInformationFlags aRendezvousFlags); + +/** + * Creates a null-terminated QR code from the payload created based on rendezvous flag information. + * + * The resulting size of the QR code span will be the size of data written and not including the null terminator. + */ +CHIP_ERROR GetQRCode(chip::MutableCharSpan & aQRCode, chip::RendezvousInformationFlags aRendezvousFlags); + +/** + * Creates a null-terminated QR code based on the provided payload. + * + * The resulting size of the QR code span will be the size of data written and not including the null terminator. + */ +CHIP_ERROR GetQRCode(chip::MutableCharSpan & aQRCode, const chip::PayloadContents & payload); + +/** + * Creates a null-terminated QR code url. + */ +CHIP_ERROR GetQRCodeUrl(char * aQRCodeUrl, size_t aUrlMaxSize, const chip::CharSpan & aQRCode); + +/** + * Creates a null-terminated manual pairing code from the payload created based on rendezvous flag information. + * + * The resulting size of the manual pairing code span will be the size of data written and not including the null terminator. + */ +CHIP_ERROR GetManualPairingCode(chip::MutableCharSpan & aManualPairingCode, chip::RendezvousInformationFlags aRendezvousFlags); + +/** + * Creates a null-terminated manual pairing code based on the provided payload. + * + * The resulting size of the manual pairing code span will be the size of data written and not including the null terminator. + */ +CHIP_ERROR GetManualPairingCode(chip::MutableCharSpan & aManualPairingCode, const chip::PayloadContents & payload); + +CHIP_ERROR GetPayloadContents(chip::PayloadContents & aPayload, chip::RendezvousInformationFlags aRendezvousFlags); /** * Initialize DataModelHandler and start CHIP datamodel server, the server diff --git a/src/controller/CommissioningWindowOpener.cpp b/src/controller/CommissioningWindowOpener.cpp index 9ff1c486ee6a8f..a6127d3a29848c 100644 --- a/src/controller/CommissioningWindowOpener.cpp +++ b/src/controller/CommissioningWindowOpener.cpp @@ -144,7 +144,7 @@ CHIP_ERROR CommissioningWindowOpener::OpenCommissioningWindowInternal(Operationa ReturnErrorOnFailure(cluster.InvokeCommand(request, this, OnOpenCommissioningWindowSuccess, OnOpenCommissioningWindowFailure, MakeOptional(kTimedInvokeTimeoutMs))); - char payloadBuffer[QRCodeBasicSetupPayloadGenerator::kMaxQRCodeBase38RepresentationLength]; + char payloadBuffer[QRCodeBasicSetupPayloadGenerator::kMaxQRCodeBase38RepresentationLength + 1]; MutableCharSpan manualCode(payloadBuffer); ReturnErrorOnFailure(ManualSetupPayloadGenerator(mSetupPayload).payloadDecimalStringRepresentation(manualCode)); diff --git a/src/lib/shell/commands/OnboardingCodes.cpp b/src/lib/shell/commands/OnboardingCodes.cpp index 3acb3a4e6501f7..1ecb791904280b 100644 --- a/src/lib/shell/commands/OnboardingCodes.cpp +++ b/src/lib/shell/commands/OnboardingCodes.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #define CHIP_SHELL_MAX_BUFFER_SIZE 128 @@ -36,21 +37,27 @@ namespace Shell { static CHIP_ERROR GetOnboardingQRCode(bool printHeader, chip::RendezvousInformationFlags aRendezvousFlags) { streamer_t * sout = streamer_get(); - std::string QRCode; + + // Create buffer for QR code that can fit max size and null terminator. + char qrCodeBuffer[chip::QRCodeBasicSetupPayloadGenerator::kMaxQRCodeBase38RepresentationLength + 1]; + chip::MutableCharSpan QRCode(qrCodeBuffer); if (printHeader) { streamer_printf(sout, "QRCode: "); } ReturnErrorOnFailure(GetQRCode(QRCode, aRendezvousFlags)); - streamer_printf(sout, "%s\r\n", QRCode.c_str()); + streamer_printf(sout, "%s\r\n", QRCode.data()); return CHIP_NO_ERROR; } static CHIP_ERROR GetOnboardingQRCodeUrl(bool printHeader, chip::RendezvousInformationFlags aRendezvousFlags) { streamer_t * sout = streamer_get(); - std::string QRCode; + + // Create buffer for QR code that can fit max size and null terminator. + char qrCodeBuffer[chip::QRCodeBasicSetupPayloadGenerator::kMaxQRCodeBase38RepresentationLength + 1]; + chip::MutableCharSpan QRCode(qrCodeBuffer); if (printHeader) { @@ -58,24 +65,27 @@ static CHIP_ERROR GetOnboardingQRCodeUrl(bool printHeader, chip::RendezvousInfor } ReturnErrorOnFailure(GetQRCode(QRCode, aRendezvousFlags)); - char qrCodeBuffer[CHIP_SHELL_MAX_BUFFER_SIZE]; + char qrCodeUrlBuffer[CHIP_SHELL_MAX_BUFFER_SIZE]; - ReturnErrorOnFailure(GetQRCodeUrl(qrCodeBuffer, sizeof(qrCodeBuffer), QRCode)); - streamer_printf(sout, "%s\r\n", qrCodeBuffer); + ReturnErrorOnFailure(GetQRCodeUrl(qrCodeUrlBuffer, sizeof(qrCodeUrlBuffer), QRCode)); + streamer_printf(sout, "%s\r\n", qrCodeUrlBuffer); return CHIP_NO_ERROR; } static CHIP_ERROR GetOnboardingManualPairingCode(bool printHeader, chip::RendezvousInformationFlags aRendezvousFlags) { streamer_t * sout = streamer_get(); - std::string manualPairingCode; + + // Create buffer for manual pariting code that can fit max size + check digit + null terminator. + char manualPairingCodeBuffer[chip::kManualSetupLongCodeCharLength + 1]; + chip::MutableCharSpan manualPairingCode(manualPairingCodeBuffer); if (printHeader) { streamer_printf(sout, "ManualPairingCode: "); } ReturnErrorOnFailure(GetManualPairingCode(manualPairingCode, aRendezvousFlags)); - streamer_printf(sout, "%s\r\n", manualPairingCode.c_str()); + streamer_printf(sout, "%s\r\n", manualPairingCode.data()); return CHIP_NO_ERROR; } diff --git a/src/setup_payload/Base38Encode.cpp b/src/setup_payload/Base38Encode.cpp index f00f4b54602eff..5e663d23bec5ed 100644 --- a/src/setup_payload/Base38Encode.cpp +++ b/src/setup_payload/Base38Encode.cpp @@ -81,6 +81,8 @@ CHIP_ERROR base38Encode(ByteSpan in_buf, MutableCharSpan & out_buf) if (out_idx < out_buf.size()) { out_buf.data()[out_idx] = '\0'; + // Reduce output span size to be the size of written data and to not include null-terminator. + out_buf.reduce_size(out_idx); } else { diff --git a/src/setup_payload/Base38Encode.h b/src/setup_payload/Base38Encode.h index 05d4f901e9f0e5..a6df4592cb612c 100644 --- a/src/setup_payload/Base38Encode.h +++ b/src/setup_payload/Base38Encode.h @@ -25,10 +25,16 @@ namespace chip { -// out_buf is null-terminated on success +/* + * The out_buf is null-terminated on success. + * + * The resulting size of the out_buf span will be the size of data written and not including the null terminator. + */ CHIP_ERROR base38Encode(ByteSpan in_buf, MutableCharSpan & out_buf); -// returns size needed to store encoded string given number of input bytes +/* + * Returns size needed to store encoded string given number of input bytes including null terminator. + */ size_t base38EncodedLength(size_t num_bytes); } // namespace chip diff --git a/src/setup_payload/ManualSetupPayloadGenerator.cpp b/src/setup_payload/ManualSetupPayloadGenerator.cpp index a1e26dbcb93073..25ffa738e99048 100644 --- a/src/setup_payload/ManualSetupPayloadGenerator.cpp +++ b/src/setup_payload/ManualSetupPayloadGenerator.cpp @@ -151,6 +151,10 @@ CHIP_ERROR ManualSetupPayloadGenerator::payloadDecimalStringRepresentation(Mutab int checkDigit = Verhoeff10::CharToVal(Verhoeff10::ComputeCheckChar(outBuffer.data())); ReturnErrorOnFailure(decimalStringWithPadding(outBuffer.SubSpan(offset, 2), static_cast(checkDigit))); + offset += 1; + + // Reduce outBuffer span size to be the size of written data and to not include null-terminator. + outBuffer.reduce_size(offset); return CHIP_NO_ERROR; } diff --git a/src/setup_payload/ManualSetupPayloadGenerator.h b/src/setup_payload/ManualSetupPayloadGenerator.h index bb04735f1b3d1c..7a73e0c3a29c26 100644 --- a/src/setup_payload/ManualSetupPayloadGenerator.h +++ b/src/setup_payload/ManualSetupPayloadGenerator.h @@ -58,6 +58,8 @@ class ManualSetupPayloadGenerator * This function is called to encode the binary data of a payload to a * decimal null-terminated string. * + * The resulting size of the outBuffer will be the size of data written and not including the null terminator. + * * @param[out] outBuffer * Output buffer to write the decimal string. * diff --git a/src/setup_payload/QRCodeSetupPayloadGenerator.h b/src/setup_payload/QRCodeSetupPayloadGenerator.h index c82aaa6ac71f77..7d79e67cede8fc 100644 --- a/src/setup_payload/QRCodeSetupPayloadGenerator.h +++ b/src/setup_payload/QRCodeSetupPayloadGenerator.h @@ -112,6 +112,8 @@ class QRCodeBasicSetupPayloadGenerator * This function is called to encode the binary data of a payload to a * base38 null-terminated string. * + * The resulting size of the out_buf span will be the size of data written and not including the null terminator. + * * @param[out] outBuffer * The buffer to copy the base38 to. * diff --git a/src/setup_payload/tests/TestQRCode.cpp b/src/setup_payload/tests/TestQRCode.cpp index c39dbd773a91be..1920f8f9022cab 100644 --- a/src/setup_payload/tests/TestQRCode.cpp +++ b/src/setup_payload/tests/TestQRCode.cpp @@ -119,21 +119,25 @@ void TestPayloadBase38Rep(nlTestSuite * inSuite, void * inContext) void TestBase38(nlTestSuite * inSuite, void * inContext) { uint8_t input[3] = { 10, 10, 10 }; - char encodedBuf[32]; + char encodedBuf[64]; MutableByteSpan inputSpan(input); MutableCharSpan encodedSpan(encodedBuf); // basic stuff base38Encode(inputSpan.SubSpan(0, 0), encodedSpan); NL_TEST_ASSERT(inSuite, strlen(encodedBuf) == 0); + encodedSpan = MutableCharSpan(encodedBuf); base38Encode(inputSpan.SubSpan(0, 1), encodedSpan); NL_TEST_ASSERT(inSuite, strcmp(encodedBuf, "A0") == 0); + encodedSpan = MutableCharSpan(encodedBuf); base38Encode(inputSpan.SubSpan(0, 2), encodedSpan); NL_TEST_ASSERT(inSuite, strcmp(encodedBuf, "OT10") == 0); + encodedSpan = MutableCharSpan(encodedBuf); base38Encode(inputSpan, encodedSpan); NL_TEST_ASSERT(inSuite, strcmp(encodedBuf, "-N.B0") == 0); // test null termination of output buffer + encodedSpan = MutableCharSpan(encodedBuf); MutableCharSpan subSpan = encodedSpan.SubSpan(0, 2); NL_TEST_ASSERT(inSuite, base38Encode(inputSpan.SubSpan(0, 1), subSpan) == CHIP_ERROR_BUFFER_TOO_SMALL); // Force no nulls in output buffer @@ -146,60 +150,72 @@ void TestBase38(nlTestSuite * inSuite, void * inContext) // passing empty parameters MutableCharSpan emptySpan; + encodedSpan = MutableCharSpan(encodedBuf); NL_TEST_ASSERT(inSuite, base38Encode(inputSpan, emptySpan) == CHIP_ERROR_BUFFER_TOO_SMALL); base38Encode(MutableByteSpan(), encodedSpan); NL_TEST_ASSERT(inSuite, strcmp(encodedBuf, "") == 0); NL_TEST_ASSERT(inSuite, base38Encode(MutableByteSpan(), emptySpan) == CHIP_ERROR_BUFFER_TOO_SMALL); // test single odd byte corner conditions - input[2] = 0; + encodedSpan = MutableCharSpan(encodedBuf); + input[2] = 0; base38Encode(inputSpan, encodedSpan); NL_TEST_ASSERT(inSuite, strcmp(encodedBuf, "OT100") == 0); - input[2] = 40; + input[2] = 40; + encodedSpan = MutableCharSpan(encodedBuf); base38Encode(inputSpan, encodedSpan); NL_TEST_ASSERT(inSuite, strcmp(encodedBuf, "Y6V91") == 0); - input[2] = 41; + input[2] = 41; + encodedSpan = MutableCharSpan(encodedBuf); base38Encode(inputSpan, encodedSpan); NL_TEST_ASSERT(inSuite, strcmp(encodedBuf, "KL0B1") == 0); - input[2] = 255; + input[2] = 255; + encodedSpan = MutableCharSpan(encodedBuf); base38Encode(inputSpan, encodedSpan); NL_TEST_ASSERT(inSuite, strcmp(encodedBuf, "Q-M08") == 0); // verify chunks of 1,2 and 3 bytes result in fixed-length strings padded with '0' // for 1 byte we need always 2 characters - input[0] = 35; + input[0] = 35; + encodedSpan = MutableCharSpan(encodedBuf); base38Encode(inputSpan.SubSpan(0, 1), encodedSpan); NL_TEST_ASSERT(inSuite, strcmp(encodedBuf, "Z0") == 0); // for 2 bytes we need always 4 characters - input[0] = 255; - input[1] = 0; + input[0] = 255; + input[1] = 0; + encodedSpan = MutableCharSpan(encodedBuf); base38Encode(inputSpan.SubSpan(0, 2), encodedSpan); NL_TEST_ASSERT(inSuite, strcmp(encodedBuf, "R600") == 0); // for 3 bytes we need always 5 characters - input[0] = 46; - input[1] = 0; - input[2] = 0; + input[0] = 46; + input[1] = 0; + input[2] = 0; + encodedSpan = MutableCharSpan(encodedBuf); base38Encode(inputSpan, encodedSpan); NL_TEST_ASSERT(inSuite, strcmp(encodedBuf, "81000") == 0); // verify maximum available values for each chunk size to check selecting proper characters number // for 1 byte we need 2 characters - input[0] = 255; + input[0] = 255; + encodedSpan = MutableCharSpan(encodedBuf); base38Encode(inputSpan.SubSpan(0, 1), encodedSpan); NL_TEST_ASSERT(inSuite, strcmp(encodedBuf, "R6") == 0); // for 2 bytes we need 4 characters - input[0] = 255; - input[1] = 255; + input[0] = 255; + input[1] = 255; + encodedSpan = MutableCharSpan(encodedBuf); base38Encode(inputSpan.SubSpan(0, 2), encodedSpan); NL_TEST_ASSERT(inSuite, strcmp(encodedBuf, "NE71") == 0); // for 3 bytes we need 5 characters - input[0] = 255; - input[1] = 255; - input[2] = 255; + input[0] = 255; + input[1] = 255; + input[2] = 255; + encodedSpan = MutableCharSpan(encodedBuf); base38Encode(inputSpan, encodedSpan); NL_TEST_ASSERT(inSuite, strcmp(encodedBuf, "PLS18") == 0); // fun with strings + encodedSpan = MutableCharSpan(encodedBuf); base38Encode(ByteSpan((uint8_t *) "Hello World!", sizeof("Hello World!") - 1), encodedSpan); NL_TEST_ASSERT(inSuite, strcmp(encodedBuf, "KKHF3W2S013OPM3EJX11") == 0);