Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix for 'Verhoeff code is not using tables as const causing unnecessa… #11726

Merged
merged 1 commit into from
Nov 29, 2021
Merged

Fix for 'Verhoeff code is not using tables as const causing unnecessa… #11726

merged 1 commit into from
Nov 29, 2021

Conversation

michalbudzon-q
Copy link
Contributor

@michalbudzon-q michalbudzon-q commented Nov 12, 2021 •
edited
Loading

…ry RAM use #9564'

Problem

What is being fixed?

  • Optimize RAM use

Change overview

Const specifier were added to lookup tables to move them to flash section.

Testing

How was this tested? (at least one bullet point required)

  • Compared memory footprint before and after the change
  • Sanity runtime test

@boring-cyborg boring-cyborg bot added the lib label Nov 12, 2021
@CLAassistant
Copy link

CLAassistant commented Nov 12, 2021 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@restyled-io restyled-io bot mentioned this pull request Nov 12, 2021
Closed
@github-actions
Copy link

github-actions bot commented Nov 12, 2021 •
edited
Loading

PR #11726: Size comparison from c7018a7 to 8243cf7

Increases (27 builds for esp32, k32w, linux, mbed, nrfconnect, p6, qpg)
platform target config section c7018a7 8243cf7 change % change
esp32 all-clusters-app c3devkit .flash.rodata 166976 167088 112 0.1
m5stack .flash.rodata 194020 194132 112 0.1
k32w lighting-app k32w061+se05x+release .text 615792 615900 108 0.0
lock-app k32w061+debug .text 516536 516644 108 0.0
shell k32w061+debug .text 571444 571552 108 0.0
linux all-clusters-app debug (read only) 1708081 1708209 128 0.0
.rodata 139829 139957 128 0.1
chip-tool debug (read only) 4580005 4580165 160 0.0
.rodata 258922 259082 160 0.1
lighting-app debug+rpc (read only) 1568857 1568985 128 0.0
.rodata 129457 129585 128 0.1
shell debug (read only) 788905 789033 128 0.0
.rodata 77839 77970 131 0.2
tv-app debug (read only) 1846841 1847001 160 0.0
.rodata 156525 156685 160 0.1
mbed all-clusters-app CY8CPROTO_062_4343W+release (read/write) 2290392 2290504 112 0.0
.heap 851616 851728 112 0.0
.text 1252992 1253104 112 0.0
lighting-app CY8CPROTO_062_4343W+release (read/write) 2273200 2273312 112 0.0
.heap 858312 858424 112 0.0
.text 1235800 1235912 112 0.0
lock-app CY8CPROTO_062_4343W+release (read/write) 2248832 2248944 112 0.0
.heap 859440 859552 112 0.0
.text 1211432 1211544 112 0.0
shell CY8CPROTO_062_4343W+release (read/write) 2048376 2048480 104 0.0
.heap 875568 875680 112 0.0
.text 1010976 1011080 104 0.0
nrfconnect lighting-app nrf52840dk_nrf52840 rodata 97084 97196 112 0.1
nrf52840dk_nrf52840+rpc rodata 88276 88384 108 0.1
nrf5340dk_nrf5340_cpuapp rodata 92344 92452 108 0.1
lock-app nrf52840dk_nrf52840 rodata 93152 93260 108 0.1
nrf5340dk_nrf5340_cpuapp rodata 88456 88568 112 0.1
pump-app nrf52840dk_nrf52840 rodata 94860 94972 112 0.1
pump-controller-app nrf52840dk_nrf52840 rodata 93152 93260 108 0.1
shell nrf52840dk_nrf52840 (read/write) 775483 775491 8 0.0
rodata 72160 72272 112 0.2
nrf5340dk_nrf5340_cpuapp rodata 66804 66916 112 0.2
p6 all-clusters-app default (read/write) 2298576 2298688 112 0.0
.heap 918216 918328 112 0.0
.text 1256840 1256952 112 0.0
lock-app default (read/write) 2212408 2212520 112 0.0
.heap 929648 929760 112 0.0
.text 1170672 1170784 112 0.0
qpg lighting-app qpg6100+debug (read only) 492636 492744 108 0.0
.text 487316 487424 108 0.0
lock-app qpg6100+debug (read only) 467372 467480 108 0.0
.text 462052 462160 108 0.0
Decreases (22 builds for esp32, k32w, linux, mbed, nrfconnect, p6, qpg)
platform target config section c7018a7 8243cf7 change % change
esp32 all-clusters-app c3devkit (read only) 822918 822394 -524 -0.1
.dram0.data 14092 13980 -112 -0.8
.flash.text 822918 822394 -524 -0.1
.iram0.text 61394 61390 -4 -0.0
m5stack .dram0.data 34056 33944 -112 -0.3
k32w lighting-app k32w061+se05x+release (read/write) 701008 701004 -4 -0.0
.data 1908 1796 -112 -5.9
lock-app k32w061+debug (read/write) 592240 592236 -4 -0.0
.data 1876 1764 -112 -6.0
shell k32w061+debug (read/write) 657908 657904 -4 -0.0
.data 1844 1732 -112 -6.1
linux all-clusters-app debug (read/write) 126272 126144 -128 -0.1
.data 1010 880 -130 -12.9
chip-tool debug (read/write) 163880 163752 -128 -0.1
.data 2272 2144 -128 -5.6
lighting-app debug+rpc (read/write) 110128 110000 -128 -0.1
.data 1202 1072 -130 -10.8
shell debug (read/write) 65064 64936 -128 -0.2
.data 242 128 -114 -47.1
tv-app debug (read/write) 407864 407704 -160 -0.0
.data 2736 2592 -144 -5.3
mbed all-clusters-app CY8CPROTO_062_4343W+release .data 5216 5104 -112 -2.1
lighting-app CY8CPROTO_062_4343W+release .data 5576 5464 -112 -2.0
lock-app CY8CPROTO_062_4343W+release .data 5568 5456 -112 -2.0
shell CY8CPROTO_062_4343W+release .data 4968 4856 -112 -2.3
nrfconnect lighting-app nrf52840dk_nrf52840+rpc (read/write) 825891 825887 -4 -0.0
nrf5340dk_nrf5340_cpuapp (read/write) 788558 788554 -4 -0.0
lock-app nrf52840dk_nrf52840 (read/write) 838751 838747 -4 -0.0
pump-controller-app nrf52840dk_nrf52840 (read/write) 838607 838603 -4 -0.0
p6 all-clusters-app default .data 2520 2408 -112 -4.4
lock-app default .data 2400 2288 -112 -4.7
qpg lighting-app qpg6100+debug (read/write) 114144 114140 -4 -0.0
.data 1008 900 -108 -10.7
lock-app qpg6100+debug .data 964 852 -112 -11.6
Full report (38 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
platform target config section c7018a7 8243cf7 change % change
efr32 lighting-app BRD4161A (read only) 747600 747600 0 0.0
(read/write) 115940 115940 0 0.0
.bss 114140 114140 0 0.0
.data 1800 1800 0 0.0
.text 747592 747592 0 0.0
BRD4161A+rpc (read only) 735164 735164 0 0.0
(read/write) 132568 132568 0 0.0
.bss 130644 130644 0 0.0
.data 1924 1924 0 0.0
.text 735156 735156 0 0.0
lock-app BRD4161A (read only) 724440 724440 0 0.0
(read/write) 113724 113724 0 0.0
.bss 111964 111964 0 0.0
.data 1756 1756 0 0.0
.text 724432 724432 0 0.0
window-app BRD4161A (read only) 725352 725352 0 0.0
(read/write) 114044 114044 0 0.0
.bss 112284 112284 0 0.0
.data 1760 1760 0 0.0
.text 725344 725344 0 0.0
esp32 all-clusters-app c3devkit (read only) 822918 822394 -524 -0.1
(read/write) 1222330 1222330 0 0.0
.dram0.bss 56016 56016 0 0.0
.dram0.data 14092 13980 -112 -0.8
.flash.rodata 166976 167088 112 0.1
.flash.text 822918 822394 -524 -0.1
.iram0.text 61394 61390 -4 -0.0
m5stack (read only) 893891 893891 0 0.0
(read/write) 420504 420504 0 0.0
.dram0.bss 61104 61104 0 0.0
.dram0.data 34056 33944 -112 -0.3
.flash.rodata 194020 194132 112 0.1
.flash.text 888507 888507 0 0.0
.iram0.text 122987 122987 0 0.0
k32w lighting-app k32w061+se05x+release (read/write) 701008 701004 -4 -0.0
.bss 77508 77508 0 0.0
.data 1908 1796 -112 -5.9
.text 615792 615900 108 0.0
lock-app k32w061+debug (read/write) 592240 592236 -4 -0.0
.bss 68028 68028 0 0.0
.data 1876 1764 -112 -6.0
.text 516536 516644 108 0.0
shell k32w061+debug (read/write) 657908 657904 -4 -0.0
.bss 78820 78820 0 0.0
.data 1844 1732 -112 -6.1
.text 571444 571552 108 0.0
linux all-clusters-app debug (read only) 1708081 1708209 128 0.0
(read/write) 126272 126144 -128 -0.1
.bss 57680 57680 0 0.0
.data 1010 880 -130 -12.9
.data.rel.ro 62304 62304 0 0.0
.dynamic 592 592 0 0.0
.got 4088 4088 0 0.0
.init 27 27 0 0.0
.init_array 552 552 0 0.0
.rodata 139829 139957 128 0.1
.text 1434498 1434498 0 0.0
bridge-app debug+rpc (read only) 1302149 1302149 0 0.0
(read/write) 77424 77424 0 0.0
.bss 42896 42896 0 0.0
.data 1568 1568 0 0.0
.data.rel.ro 27992 27992 0 0.0
.dynamic 592 592 0 0.0
.got 3952 3952 0 0.0
.init 27 27 0 0.0
.init_array 408 408 0 0.0
.rodata 111508 111508 0 0.0
.text 1094005 1094005 0 0.0
chip-tool debug (read only) 4580005 4580165 160 0.0
(read/write) 163880 163752 -128 -0.1
.bss 41192 41192 0 0.0
.data 2272 2144 -128 -5.6
.data.rel.ro 114976 114976 0 0.0
.dynamic 592 592 0 0.0
.got 4368 4368 0 0.0
.init 27 27 0 0.0
.init_array 472 472 0 0.0
.rodata 258922 259082 160 0.1
.text 4020101 4020101 0 0.0
lighting-app debug+rpc (read only) 1568857 1568985 128 0.0
(read/write) 110128 110000 -128 -0.1
.bss 48080 48080 0 0.0
.data 1202 1072 -130 -10.8
.data.rel.ro 55552 55552 0 0.0
.dynamic 608 608 0 0.0
.got 4112 4112 0 0.0
.init 27 27 0 0.0
.init_array 536 536 0 0.0
.rodata 129457 129585 128 0.1
.text 1304978 1304978 0 0.0
ota-provider-app debug (read only) 1261625 1261625 0 0.0
(read/write) 75176 75176 0 0.0
.bss 44512 44512 0 0.0
.data 752 752 0 0.0
.data.rel.ro 24808 24808 0 0.0
.dynamic 592 592 0 0.0
.got 4016 4016 0 0.0
.init 27 27 0 0.0
.init_array 448 448 0 0.0
.rodata 112999 112999 0 0.0
.text 1051778 1051778 0 0.0
ota-requestor-app debug (read only) 1346409 1346409 0 0.0
(read/write) 78944 78944 0 0.0
.bss 46976 46976 0 0.0
.data 816 816 0 0.0
.data.rel.ro 26056 26056 0 0.0
.dynamic 592 592 0 0.0
.got 3992 3992 0 0.0
.init 27 27 0 0.0
.init_array 472 472 0 0.0
.rodata 123824 123824 0 0.0
.text 1123186 1123186 0 0.0
shell debug (read only) 788905 789033 128 0.0
(read/write) 65064 64936 -128 -0.2
.bss 23400 23400 0 0.0
.data 242 128 -114 -47.1
.data.rel.ro 36928 36928 0 0.0
.dynamic 592 592 0 0.0
.got 3528 3528 0 0.0
.init 27 27 0 0.0
.init_array 344 344 0 0.0
.rodata 77839 77970 131 0.2
.text 609170 609170 0 0.0
tv-app debug (read only) 1846841 1847001 160 0.0
(read/write) 407864 407704 -160 -0.0
.bss 339752 339752 0 0.0
.data 2736 2592 -144 -5.3
.data.rel.ro 59728 59728 0 0.0
.dynamic 592 592 0 0.0
.got 4408 4408 0 0.0
.init 27 27 0 0.0
.init_array 616 616 0 0.0
.rodata 156525 156685 160 0.1
.text 1545506 1545506 0 0.0
mbed all-clusters-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2290392 2290504 112 0.0
.bss 179612 179612 0 0.0
.data 5216 5104 -112 -2.1
.heap 851616 851728 112 0.0
.text 1252992 1253104 112 0.0
lighting-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2273200 2273312 112 0.0
.bss 172556 172556 0 0.0
.data 5576 5464 -112 -2.0
.heap 858312 858424 112 0.0
.text 1235800 1235912 112 0.0
lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2248832 2248944 112 0.0
.bss 171436 171436 0 0.0
.data 5568 5456 -112 -2.0
.heap 859440 859552 112 0.0
.text 1211432 1211544 112 0.0
pigweed-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 1139744 1139744 0 0.0
.bss 11752 11752 0 0.0
.data 4368 4368 0 0.0
.heap 1020328 1020328 0 0.0
.text 103128 103128 0 0.0
shell CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2048376 2048480 104 0.0
.bss 155912 155912 0 0.0
.data 4968 4856 -112 -2.3
.heap 875568 875680 112 0.0
.text 1010976 1011080 104 0.0
nrfconnect lighting-app nrf52840dk_nrf52840 (read/write) 863515 863515 0 0.0
bss 110964 110964 0 0.0
rodata 97084 97196 112 0.1
text 579844 579844 0 0.0
nrf52840dk_nrf52840+rpc (read/write) 825891 825887 -4 -0.0
bss 107316 107316 0 0.0
rodata 88276 88384 108 0.1
text 554016 554016 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 788558 788554 -4 -0.0
bss 112336 112336 0 0.0
rodata 92344 92452 108 0.1
text 509312 509312 0 0.0
lock-app nrf52840dk_nrf52840 (read/write) 838751 838747 -4 -0.0
bss 109988 109988 0 0.0
rodata 93152 93260 108 0.1
text 560148 560148 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 764046 764046 0 0.0
bss 111400 111400 0 0.0
rodata 88456 88568 112 0.1
text 489708 489708 0 0.0
pigweed-app nrf52840dk_nrf52840 (read/write) 497327 497327 0 0.0
bss 51824 51824 0 0.0
rodata 45780 45780 0 0.0
text 339436 339436 0 0.0
pump-app nrf52840dk_nrf52840 (read/write) 844827 844827 0 0.0
bss 110128 110128 0 0.0
rodata 94860 94972 112 0.1
text 564308 564308 0 0.0
pump-controller-app nrf52840dk_nrf52840 (read/write) 838607 838603 -4 -0.0
bss 110024 110024 0 0.0
rodata 93152 93260 108 0.1
text 559884 559884 0 0.0
shell nrf52840dk_nrf52840 (read/write) 775483 775491 8 0.0
bss 108736 108736 0 0.0
rodata 72160 72272 112 0.2
text 520008 520008 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 690538 690538 0 0.0
bss 109720 109720 0 0.0
rodata 66804 66916 112 0.2
text 440616 440616 0 0.0
p6 all-clusters-app default (read/write) 2298576 2298688 112 0.0
.bss 112608 112608 0 0.0
.data 2520 2408 -112 -4.4
.heap 918216 918328 112 0.0
.text 1256840 1256952 112 0.0
lock-app default (read/write) 2212408 2212520 112 0.0
.bss 101296 101296 0 0.0
.data 2400 2288 -112 -4.7
.heap 929648 929760 112 0.0
.text 1170672 1170784 112 0.0
qpg lighting-app qpg6100+debug (read only) 492636 492744 108 0.0
(read/write) 114144 114140 -4 -0.0
.bss 50640 50640 0 0.0
.data 1008 900 -108 -10.7
.text 487316 487424 108 0.0
lock-app qpg6100+debug (read only) 467372 467480 108 0.0
(read/write) 114140 114140 0 0.0
.bss 49576 49576 0 0.0
.data 964 852 -112 -11.6
.text 462052 462160 108 0.0
persistent-storage-app qpg6100+debug (read only) 105408 105408 0 0.0
(read/write) 114142 114142 0 0.0
.bss 8978 8978 0 0.0
.data 272 272 0 0.0
.text 100088 100088 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 665742 665742 0 0.0
bss 69320 69320 0 0.0
noinit 33216 33216 0 0.0
text 460372 460372 0 0.0

@boring-cyborg boring-cyborg bot added config documentation Improvements or additions to documentation github vscode workflows labels Nov 17, 2021
@todo
Copy link

todo bot commented Nov 17, 2021

AbortTransfer() needs to support GeneralStatusCode failures as well as BDX specific errors.

connectedhomeip/examples/ota-provider-app/esp32/main/BdxOtaSender.cpp

Lines 89 to 99 in 370ed3c

// TODO: AbortTransfer() needs to support GeneralStatusCode failures as well as BDX specific errors.
mTransfer.AbortTransfer(StatusCode::kUnknown);
return;
}
if (mOnBlockQueryCallback != nullptr && mOnBlockQueryCallback->mCall != nullptr)
{
if (CHIP_NO_ERROR !=
mOnBlockQueryCallback->mCall(mOnBlockQueryCallback->mContext, blockBuf, blockData.Length, blockData.IsEof,
mNumBytesSent))
{

This comment was generated by todo based on a TODO comment in 370ed3c in #11726. cc @michalbudzon-q.

@todo
Copy link

todo bot commented Nov 17, 2021

(cecille): Fix for the case where BLE and SoftAP are both enabled.`

connectedhomeip/examples/ota-provider-app/esp32/main/CHIPDeviceManager.cpp

Lines 68 to 78 in 370ed3c

// TODO(cecille): Fix for the case where BLE and SoftAP are both enabled.`
ConnectivityMgr().SetBLEAdvertisingEnabled(false);
ConnectivityMgr().SetWiFiAPMode(ConnectivityManager::kWiFiAPMode_Enabled);
}
else
{
// If rendezvous is bypassed, enable SoftAP so that the device can still
// be communicated with via its SoftAP as needed.
ConnectivityMgr().SetWiFiAPMode(ConnectivityManager::kWiFiAPMode_Enabled);
}

This comment was generated by todo based on a TODO comment in 370ed3c in #11726. cc @michalbudzon-q.

@todo
Copy link

todo bot commented Nov 17, 2021

handle this callback in switch statement

connectedhomeip/examples/ota-provider-app/esp32/main/DeviceCallbacks.cpp

Lines 75 to 85 in 370ed3c

// TODO handle this callback in switch statement
ESP_LOGI(TAG, "Unhandled cluster ID: " ChipLogFormatMEI, ChipLogValueMEI(clusterId));
ESP_LOGI(TAG, "Current free heap: %d\n", heap_caps_get_free_size(MALLOC_CAP_8BIT));
}
void DeviceCallbacks::OnInternetConnectivityChange(const ChipDeviceEvent * event)
{
if (event->InternetConnectivityChange.IPv4 == kConnectivity_Established)
{
ESP_LOGI(TAG, "Server ready at: %s:%d", event->InternetConnectivityChange.address, CHIP_PORT);

This comment was generated by todo based on a TODO comment in 370ed3c in #11726. cc @michalbudzon-q.

@todo
Copy link

todo bot commented Nov 17, 2021

this should probably be done dynamically

connectedhomeip/examples/ota-provider-app/esp32/main/main.cpp

Lines 67 to 77 in 370ed3c

// TODO: this should probably be done dynamically
constexpr chip::EndpointId kOtaProviderEndpoint = 0;
constexpr uint32_t kMaxBdxBlockSize = 1024;
constexpr chip::System::Clock::Timeout kBdxTimeout = chip::System::Clock::Seconds16(5 * 60); // Specification mandates >= 5 minutes
constexpr chip::System::Clock::Timeout kBdxPollFreq = chip::System::Clock::Milliseconds32(500);
const char * otaFilename = "hello-world.bin";
const esp_partition_t * otaPartition = nullptr;
uint32_t otaImageLen = 0;
uint32_t otaTransferInProgress = false;
chip::Callback::Callback<OnBdxBlockQuery> onBlockQueryCallback(OnBlockQuery, nullptr);

This comment was generated by todo based on a TODO comment in 370ed3c in #11726. cc @michalbudzon-q.

@todo
Copy link

todo bot commented Nov 17, 2021

Use the OTA image header specified in the specification

connectedhomeip/examples/ota-provider-app/esp32/main/main.cpp

Lines 197 to 207 in 370ed3c

// TODO: Use the OTA image header specified in the specification
// Right now we are using just image length instead of full header
esp_partition_read(otaPartition, 0, &otaImageLen, sizeof(otaImageLen));
if (otaImageLen > otaPartition->size)
{
otaImageLen = 0;
}
ESP_LOGI(TAG, "OTA image length %d bytes", otaImageLen);
}
else
{

This comment was generated by todo based on a TODO comment in 370ed3c in #11726. cc @michalbudzon-q.

@todo
Copy link

todo bot commented Nov 17, 2021

while convenient, we should not do a synchronous block write in our example application - this is bad practice

connectedhomeip/examples/ota-requestor-app/esp32/main/BDXDownloader.cpp

Lines 87 to 97 in 370ed3c

// TODO: while convenient, we should not do a synchronous block write in our example application - this is bad practice
if (mOnBlockReceivedCallback != nullptr && mOnBlockReceivedCallback->mCall != nullptr)
{
mOnBlockReceivedCallback->mCall(mOnBlockReceivedCallback->mContext, event.blockdata);
}
else
{
ChipLogError(BDX, "onBlockReceived Callback not set");
}
if (event.blockdata.IsEof)
{

This comment was generated by todo based on a TODO comment in 370ed3c in #11726. cc @michalbudzon-q.

@todo
Copy link

todo bot commented Nov 17, 2021

(cecille): Fix for the case where BLE and SoftAP are both enabled.`

connectedhomeip/examples/ota-requestor-app/esp32/main/CHIPDeviceManager.cpp

Lines 68 to 78 in 370ed3c

// TODO(cecille): Fix for the case where BLE and SoftAP are both enabled.`
ConnectivityMgr().SetBLEAdvertisingEnabled(false);
ConnectivityMgr().SetWiFiAPMode(ConnectivityManager::kWiFiAPMode_Enabled);
}
else
{
// If rendezvous is bypassed, enable SoftAP so that the device can still
// be communicated with via its SoftAP as needed.
ConnectivityMgr().SetWiFiAPMode(ConnectivityManager::kWiFiAPMode_Enabled);
}

This comment was generated by todo based on a TODO comment in 370ed3c in #11726. cc @michalbudzon-q.

@todo
Copy link

todo bot commented Nov 17, 2021

handle this callback in switch statement

connectedhomeip/examples/ota-requestor-app/esp32/main/DeviceCallbacks.cpp

Lines 75 to 85 in 370ed3c

// TODO handle this callback in switch statement
ESP_LOGI(TAG, "Unhandled cluster ID: " ChipLogFormatMEI, ChipLogValueMEI(clusterId));
ESP_LOGI(TAG, "Current free heap: %d\n", heap_caps_get_free_size(MALLOC_CAP_8BIT));
}
void DeviceCallbacks::OnInternetConnectivityChange(const ChipDeviceEvent * event)
{
if (event->InternetConnectivityChange.IPv4 == kConnectivity_Established)
{
ESP_LOGI(TAG, "Server ready at: %s:%d", event->InternetConnectivityChange.address, CHIP_PORT);

This comment was generated by todo based on a TODO comment in 370ed3c in #11726. cc @michalbudzon-q.

@todo
Copy link

todo bot commented Nov 17, 2021

Encapsulate these globals and the callbacks in some class

connectedhomeip/examples/ota-requestor-app/esp32/main/OTARequesterImpl.cpp

Lines 73 to 83 in 370ed3c

// TODO: Encapsulate these globals and the callbacks in some class
ExchangeContext * exchangeCtx = nullptr;
BdxDownloader bdxDownloader;
enum OTARequestorCommands operationalDeviceContext;
constexpr uint8_t kMaxUpdateTokenLen = 32; // must be between 8 and 32
uint8_t otaUpdateToken[kMaxUpdateTokenLen] = { 0 };
uint8_t otaUpdateTokenLen = 0;
/* Callbacks for operational device proxy connect response */
Callback<OnDeviceConnected> onConnectedCallback(OnConnected, &operationalDeviceContext);

This comment was generated by todo based on a TODO comment in 370ed3c in #11726. cc @michalbudzon-q.

@todo
Copy link

todo bot commented Nov 17, 2021

Handle image URI for protocol other than bdx

connectedhomeip/examples/ota-requestor-app/esp32/main/OTARequesterImpl.cpp

Lines 109 to 119 in 370ed3c

// TODO: Handle image URI for protocol other than bdx
// Ignore the first 23 "bdx://<NodeId>/"
char fileDesignator[128]; // 128 is arbitrary value
memset(fileDesignator, 0, sizeof(fileDesignator));
size_t fileDesignatorLength = response.imageURI.Value().size() - 23 + 1; // + 1 for \0
if ((response.imageURI.Value().size() - 23) > sizeof(fileDesignator))
{
fileDesignatorLength = sizeof(fileDesignator);
}
strlcpy(fileDesignator, response.imageURI.Value().data() + 23, fileDesignatorLength);

This comment was generated by todo based on a TODO comment in 370ed3c in #11726. cc @michalbudzon-q.

@todo
Copy link

todo bot commented Nov 17, 2021

Process/skip the Matter OTA header

connectedhomeip/examples/ota-requestor-app/esp32/main/OTARequesterImpl.cpp

Lines 253 to 263 in 370ed3c

// TODO: Process/skip the Matter OTA header
OTAUpdater::GetInstance().Write(reinterpret_cast<const void *>(blockdata.Data), blockdata.Length);
}
void OnTransferComplete(void * context)
{
ESP_LOGI(TAG, "Transfer complete!");
OTAUpdater::GetInstance().End();
}
void OnTransferFailed(void * context, BdxDownloaderErrorTypes status)

This comment was generated by todo based on a TODO comment in 370ed3c in #11726. cc @michalbudzon-q.

@todo
Copy link

todo bot commented Nov 17, 2021

Determine where this should be instantiated

connectedhomeip/examples/ota-requestor-app/esp32/main/OTARequesterImpl.cpp

Lines 291 to 301 in 370ed3c

// TODO: Determine where this should be instantiated
.imDelegate = chip::Platform::New<chip::Controller::DeviceControllerInteractionModelDelegate>(),
};
PeerId peerID = fabric->GetPeerId();
peerID.SetNodeId(providerNodeId);
operationalDeviceProxy = new chip::OperationalDeviceProxy(initParams, peerID);
server->SetOperationalDeviceProxy(operationalDeviceProxy);
// Explicitly calling UpdateDeviceData() should not be needed once OperationalDeviceProxy can resolve IP address from node
// ID and fabric index

This comment was generated by todo based on a TODO comment in 370ed3c in #11726. cc @michalbudzon-q.

@todo
Copy link

todo bot commented Nov 17, 2021

Apply update after delayed action time

connectedhomeip/examples/ota-requestor-app/esp32/main/OTAUpdater.cpp

Lines 134 to 144 in 370ed3c

// TODO: Apply update after delayed action time
// TODO: Handle applying update after reboot
esp_err_t OTAUpdater::Apply(uint32_t delayedActionTime)
{
if (otaUpdateInProgress == true)
{
return ESP_ERR_INVALID_STATE;
}
esp_err_t err = esp_ota_set_boot_partition(otaUpdatePartition);
if (err != ESP_OK)

This comment was generated by todo based on a TODO comment in 370ed3c in #11726. cc @michalbudzon-q.

@todo
Copy link

todo bot commented Nov 17, 2021

Handle applying update after reboot

connectedhomeip/examples/ota-requestor-app/esp32/main/OTAUpdater.cpp

Lines 135 to 145 in 370ed3c

// TODO: Handle applying update after reboot
esp_err_t OTAUpdater::Apply(uint32_t delayedActionTime)
{
if (otaUpdateInProgress == true)
{
return ESP_ERR_INVALID_STATE;
}
esp_err_t err = esp_ota_set_boot_partition(otaUpdatePartition);
if (err != ESP_OK)
{

This comment was generated by todo based on a TODO comment in 370ed3c in #11726. cc @michalbudzon-q.

@todo
Copy link

todo bot commented Nov 17, 2021

With this approach we might end up adding callback for every bdx event/message.

connectedhomeip/examples/ota-requestor-app/esp32/main/include/BDXDownloader.h

Lines 60 to 70 in 370ed3c

// TODO: With this approach we might end up adding callback for every bdx event/message.
// Can be refactored into a single callback with events
struct BdxDownloaderCallbacks
{
chip::Callback::Callback<OnBdxBlockReceived> * onBlockReceived = nullptr;
chip::Callback::Callback<OnBdxTransferComplete> * onTransferComplete = nullptr;
chip::Callback::Callback<OnBdxTransferFailed> * onTransferFailed = nullptr;
};
class BdxDownloader : public chip::bdx::Initiator
{

This comment was generated by todo based on a TODO comment in 370ed3c in #11726. cc @michalbudzon-q.

@todo
Copy link

todo bot commented Nov 17, 2021

(andy314): IPv6 does Duplicate Address Detection even though

connectedhomeip/scripts/tests/test_suites.sh

Lines 91 to 101 in 370ed3c

# TODO(andy314): IPv6 does Duplicate Address Detection even though
# we know these addresses are isolated. For a while IPv6 addresses
# will be in 'transitional' state and cannot be used.
#
# This sleep waits for the addresses to become 'global'. Ideally
# we should loop/wait here instead.
sleep 2
}
netns_cleanup() {
ip netns del app || true

This comment was generated by todo based on a TODO comment in 370ed3c in #11726. cc @michalbudzon-q.

@todo
Copy link

todo bot commented Nov 17, 2021

check against CATs in subject descriptor

connectedhomeip/src/access/AccessControl.cpp

Lines 120 to 130 in 370ed3c

// TODO: check against CATs in subject descriptor
}
if (!subjectMatched)
{
continue;
}
}
size_t targetCount = 0;
ReturnErrorOnFailure(entry.GetTargetCount(targetCount));
if (targetCount > 0)

This comment was generated by todo based on a TODO comment in 370ed3c in #11726. cc @michalbudzon-q.

@todo
Copy link

todo bot commented Nov 17, 2021

check against target.deviceType (requires lookup)

connectedhomeip/src/access/AccessControl.cpp

Lines 145 to 155 in 370ed3c

// TODO: check against target.deviceType (requires lookup)
targetMatched = true;
break;
}
if (!targetMatched)
{
continue;
}
}
// Entry passed all checks: access is allowed.

This comment was generated by todo based on a TODO comment in 370ed3c in #11726. cc @michalbudzon-q.

Damian-Nordic
Damian-Nordic approved these changes Nov 23, 2021
View reviewed changes
Copy link
Contributor

@Damian-Nordic Damian-Nordic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is anything except Verhoeff10 used at all? If not I suggest removing unused code rather than updating it.

@andy31415
Copy link
Contributor

fast track: change has been up for a while, straight-forward change.

@andy31415 andy31415 merged commit ef9c86c into project-chip:master Nov 29, 2021
@bzbarsky-apple bzbarsky-apple mentioned this pull request Jun 13, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Damian-Nordic Damian-Nordic Damian-Nordic approved these changes

@andy31415 andy31415 andy31415 approved these changes

@anush-apple anush-apple Awaiting requested review from anush-apple

@balducci-apple balducci-apple Awaiting requested review from balducci-apple

@bzbarsky-apple bzbarsky-apple Awaiting requested review from bzbarsky-apple

@carol-apple carol-apple Awaiting requested review from carol-apple

@cecille cecille Awaiting requested review from cecille

@chrisdecenzo chrisdecenzo Awaiting requested review from chrisdecenzo

@chulspro chulspro Awaiting requested review from chulspro

@electrocucaracha electrocucaracha Awaiting requested review from electrocucaracha

@emargolis emargolis Awaiting requested review from emargolis

@erjiaqing erjiaqing Awaiting requested review from erjiaqing

@franck-apple franck-apple Awaiting requested review from franck-apple

@hawk248 hawk248 Awaiting requested review from hawk248

@holbrookt holbrookt Awaiting requested review from holbrookt

@jelderton jelderton Awaiting requested review from jelderton

@jepenven-silabs jepenven-silabs Awaiting requested review from jepenven-silabs

@jmartinez-silabs jmartinez-silabs Awaiting requested review from jmartinez-silabs

@kpschoedel kpschoedel Awaiting requested review from kpschoedel

@LuDuda LuDuda Awaiting requested review from LuDuda

@mrjerryjohns mrjerryjohns Awaiting requested review from mrjerryjohns

@msandstedt msandstedt Awaiting requested review from msandstedt

@mspang mspang Awaiting requested review from mspang

@pan-apple pan-apple Awaiting requested review from pan-apple

@robszewczyk robszewczyk Awaiting requested review from robszewczyk

@sagar-apple sagar-apple Awaiting requested review from sagar-apple

@saurabhst saurabhst Awaiting requested review from saurabhst

@selissia selissia Awaiting requested review from selissia

@tecimovic tecimovic Awaiting requested review from tecimovic

@vivien-apple vivien-apple Awaiting requested review from vivien-apple

@wbschiller wbschiller Awaiting requested review from wbschiller

@woody-apple woody-apple Awaiting requested review from woody-apple

@yufengwangca yufengwangca Awaiting requested review from yufengwangca

@yunhanw-google yunhanw-google Awaiting requested review from yunhanw-google

Assignees
No one assigned
Labels
fast track lib review - pending
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@michalbudzon-q @CLAassistant @andy31415 @Damian-Nordic @tima-q