RPC: Refactor to share common code between examples #9444

rgoliver · 2021-09-02T19:24:57Z

Problem

As more examples get RPC support it is clear that a lot of code is unnecessarily duplicated.

Change overview

Move the common RPC services into a common location and have default virtual implementations which use the ember API.
Applications can customize the behavior when necessary by overriding the methods.
Also cleanup the lighting proto to better match the spec.

Testing

Tested the RPC version of the lighting app on EFR NRF and Linux and ensured Device, Lighting, and Button RPC's worked as expected.
Tested the ESP RPC locking app compiled with RPCs (Don't have the correct board to test functionality)
Tested the ESP RPC all-clusters app and verified Device, Lighting, and Button RPC's worked as expected.

todo · 2021-09-02T19:24:58Z

auto err = DeviceLayer::SoftwareUpdateMgr().CheckNow();

connectedhomeip/examples/common/pigweed/rpc_services/Device.h

Lines 46 to 56 in 0651c3f

    
               // TODO: auto err = DeviceLayer::SoftwareUpdateMgr().CheckNow(); 
        
               return pw::Status::Unimplemented(); 
        
           } 
        
           virtual pw::Status GetDeviceInfo(ServerContext &, const pw_protobuf_Empty & request, chip_rpc_DeviceInfo & response) 
        
           { 
        
               response.vendor_id        = CHIP_DEVICE_CONFIG_DEVICE_VENDOR_ID; 
        
               response.product_id       = CHIP_DEVICE_CONFIG_DEVICE_PRODUCT_ID; 
        
               response.software_version = CHIP_DEVICE_CONFIG_DEVICE_FIRMWARE_REVISION; 
        
               snprintf(response.serial_number, sizeof(response.serial_number), CHIP_DEVICE_CONFIG_TEST_SERIAL_NUMBER); 
        
               return pw::OkStatus();

This comment was generated by todo based on a `TODO` comment in `0651c3f` in #9444. cc @rgoliver.

todo · 2021-09-07T19:57:17Z

auto err = DeviceLayer::SoftwareUpdateMgr().CheckNow();

connectedhomeip/examples/common/pigweed/rpc_services/Device.h

Lines 46 to 56 in d8067bb

    
               // TODO: auto err = DeviceLayer::SoftwareUpdateMgr().CheckNow(); 
        
               return pw::Status::Unimplemented(); 
        
           } 
        
           virtual pw::Status GetDeviceInfo(ServerContext &, const pw_protobuf_Empty & request, chip_rpc_DeviceInfo & response) 
        
           { 
        
               response.vendor_id        = CHIP_DEVICE_CONFIG_DEVICE_VENDOR_ID; 
        
               response.product_id       = CHIP_DEVICE_CONFIG_DEVICE_PRODUCT_ID; 
        
               response.software_version = CHIP_DEVICE_CONFIG_DEVICE_FIRMWARE_REVISION; 
        
               snprintf(response.serial_number, sizeof(response.serial_number), CHIP_DEVICE_CONFIG_TEST_SERIAL_NUMBER); 
        
               return pw::OkStatus();

This comment was generated by todo based on a `TODO` comment in `d8067bb` in #9444. cc @rgoliver.

todo · 2021-09-07T20:26:09Z

auto err = DeviceLayer::SoftwareUpdateMgr().CheckNow();

connectedhomeip/examples/common/pigweed/rpc_services/Device.h

Lines 47 to 57 in 230a62d

    
               // TODO: auto err = DeviceLayer::SoftwareUpdateMgr().CheckNow(); 
        
               return pw::Status::Unimplemented(); 
        
           } 
        
           virtual pw::Status GetDeviceInfo(ServerContext &, const pw_protobuf_Empty & request, chip_rpc_DeviceInfo & response) 
        
           { 
        
               response.vendor_id        = CHIP_DEVICE_CONFIG_DEVICE_VENDOR_ID; 
        
               response.product_id       = CHIP_DEVICE_CONFIG_DEVICE_PRODUCT_ID; 
        
               response.software_version = CHIP_DEVICE_CONFIG_DEVICE_FIRMWARE_REVISION; 
        
               snprintf(response.serial_number, sizeof(response.serial_number), CHIP_DEVICE_CONFIG_TEST_SERIAL_NUMBER); 
        
               return pw::OkStatus();

This comment was generated by todo based on a `TODO` comment in `230a62d` in #9444. cc @rgoliver.

todo · 2021-09-07T20:38:06Z

auto err = DeviceLayer::SoftwareUpdateMgr().CheckNow();

connectedhomeip/examples/common/pigweed/rpc_services/Device.h

Lines 47 to 57 in 914e859

    
               // TODO: auto err = DeviceLayer::SoftwareUpdateMgr().CheckNow(); 
        
               return pw::Status::Unimplemented(); 
        
           } 
        
           virtual pw::Status GetDeviceInfo(ServerContext &, const pw_protobuf_Empty & request, chip_rpc_DeviceInfo & response) 
        
           { 
        
               response.vendor_id        = CHIP_DEVICE_CONFIG_DEVICE_VENDOR_ID; 
        
               response.product_id       = CHIP_DEVICE_CONFIG_DEVICE_PRODUCT_ID; 
        
               response.software_version = CHIP_DEVICE_CONFIG_DEVICE_FIRMWARE_REVISION; 
        
               snprintf(response.serial_number, sizeof(response.serial_number), CHIP_DEVICE_CONFIG_TEST_SERIAL_NUMBER); 
        
               return pw::OkStatus();

This comment was generated by todo based on a `TODO` comment in `914e859` in #9444. cc @rgoliver.

examples/common/pigweed/rpc_services/Lighting.h

examples/common/pigweed/rpc_services/Locking.h

examples/lighting-app/linux/BUILD.gn

todo · 2021-09-13T15:08:42Z

auto err = DeviceLayer::SoftwareUpdateMgr().CheckNow();

connectedhomeip/examples/common/pigweed/rpc_services/Device.h

Lines 47 to 57 in ae62dcc

    
               // TODO: auto err = DeviceLayer::SoftwareUpdateMgr().CheckNow(); 
        
               return pw::Status::Unimplemented(); 
        
           } 
        
           virtual pw::Status GetDeviceInfo(ServerContext &, const pw_protobuf_Empty & request, chip_rpc_DeviceInfo & response) 
        
           { 
        
               response.vendor_id        = CHIP_DEVICE_CONFIG_DEVICE_VENDOR_ID; 
        
               response.product_id       = CHIP_DEVICE_CONFIG_DEVICE_PRODUCT_ID; 
        
               response.software_version = CHIP_DEVICE_CONFIG_DEVICE_FIRMWARE_REVISION; 
        
               snprintf(response.serial_number, sizeof(response.serial_number), CHIP_DEVICE_CONFIG_TEST_SERIAL_NUMBER); 
        
               return pw::OkStatus();

This comment was generated by todo based on a `TODO` comment in `ae62dcc` in #9444. cc @rgoliver.

todo · 2021-09-13T17:42:07Z

auto err = DeviceLayer::SoftwareUpdateMgr().CheckNow();

connectedhomeip/examples/common/pigweed/rpc_services/Device.h

Lines 47 to 57 in c14d2f4

    
               // TODO: auto err = DeviceLayer::SoftwareUpdateMgr().CheckNow(); 
        
               return pw::Status::Unimplemented(); 
        
           } 
        
           virtual pw::Status GetDeviceInfo(ServerContext &, const pw_protobuf_Empty & request, chip_rpc_DeviceInfo & response) 
        
           { 
        
               response.vendor_id        = CHIP_DEVICE_CONFIG_DEVICE_VENDOR_ID; 
        
               response.product_id       = CHIP_DEVICE_CONFIG_DEVICE_PRODUCT_ID; 
        
               response.software_version = CHIP_DEVICE_CONFIG_DEVICE_FIRMWARE_REVISION; 
        
               snprintf(response.serial_number, sizeof(response.serial_number), CHIP_DEVICE_CONFIG_TEST_SERIAL_NUMBER); 
        
               return pw::OkStatus();

This comment was generated by todo based on a `TODO` comment in `c14d2f4` in #9444. cc @rgoliver.

Move the common RPC services into a common location and have default virtual implementations which use the ember API. Applications can customize the behaviour when neccessary by overriding the methods. Also cleanup the lighting proto to better match the spec.

todo · 2021-09-13T17:42:57Z

auto err = DeviceLayer::SoftwareUpdateMgr().CheckNow();

connectedhomeip/examples/common/pigweed/rpc_services/Device.h

Lines 47 to 57 in 8bdddd7

    
               // TODO: auto err = DeviceLayer::SoftwareUpdateMgr().CheckNow(); 
        
               return pw::Status::Unimplemented(); 
        
           } 
        
           virtual pw::Status GetDeviceInfo(ServerContext &, const pw_protobuf_Empty & request, chip_rpc_DeviceInfo & response) 
        
           { 
        
               response.vendor_id        = CHIP_DEVICE_CONFIG_DEVICE_VENDOR_ID; 
        
               response.product_id       = CHIP_DEVICE_CONFIG_DEVICE_PRODUCT_ID; 
        
               response.software_version = CHIP_DEVICE_CONFIG_DEVICE_FIRMWARE_REVISION; 
        
               snprintf(response.serial_number, sizeof(response.serial_number), CHIP_DEVICE_CONFIG_TEST_SERIAL_NUMBER); 
        
               return pw::OkStatus();

This comment was generated by todo based on a `TODO` comment in `8bdddd7` in #9444. cc @rgoliver.

github-actions · 2021-09-13T18:33:36Z

Size increase report for "esp32-example-build" from 92d3a28

File	Section	File	VM
chip-lock-app.elf	.flash.text	-60	-60
chip-temperature-measurement-app.elf	.flash.text	36	36

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-pigweed-app.elf and ./pull_artifact/chip-pigweed-app.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-all-clusters-app.elf and ./pull_artifact/chip-all-clusters-app.elf:

sections,vmsize,filesize
.debug_info,0,1
.debug_loc,0,-1

Comparing ./master_artifact/chip-bridge-app.elf and ./pull_artifact/chip-bridge-app.elf:

sections,vmsize,filesize
.debug_abbrev,0,44
.debug_info,0,3
.debug_loc,0,1

Comparing ./master_artifact/chip-lock-app.elf and ./pull_artifact/chip-lock-app.elf:

sections,vmsize,filesize
[Unmapped],0,60
.xt.prop._ZNK4chip4SpanIhE7SubSpanEjj,0,12
.xt.lit._ZNK4chip4SpanIhE7SubSpanEjj,0,8
.debug_info,0,1
.debug_loc,0,-5
.flash.text,-60,-60

Comparing ./master_artifact/chip-shell.elf and ./pull_artifact/chip-shell.elf:

sections,vmsize,filesize
.debug_abbrev,0,44
.debug_info,0,3
.debug_loc,0,1

Comparing ./master_artifact/chip-temperature-measurement-app.elf and ./pull_artifact/chip-temperature-measurement-app.elf:

sections,vmsize,filesize
.debug_abbrev,0,44
.flash.text,36,36
.debug_info,0,3
.debug_loc,0,1
[Unmapped],0,-36

Comparing ./master_artifact/chip-persistent-storage.elf and ./pull_artifact/chip-persistent-storage.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-ipv6only-app.elf and ./pull_artifact/chip-ipv6only-app.elf:

sections,vmsize,filesize

boring-cyborg bot added esp32 examples platform labels Sep 2, 2021

pullapprove bot requested review from chrisdecenzo, jelderton, jmartinez-silabs, LuDuda, mspang, saurabhst and woody-apple September 2, 2021 19:30

pullapprove bot added the review - pending label Sep 2, 2021

andy31415 added the SDK Approved label Sep 3, 2021

rgoliver force-pushed the chip-rpc-refactor branch from 0651c3f to d8067bb Compare September 7, 2021 19:57

rgoliver force-pushed the chip-rpc-refactor branch from d8067bb to 230a62d Compare September 7, 2021 20:26

restyled-io bot mentioned this pull request Sep 7, 2021

Restyle RPC: Refactor to share common code between examples #9517

Closed

rgoliver force-pushed the chip-rpc-refactor branch from 230a62d to 914e859 Compare September 7, 2021 20:38

jmartinez-silabs approved these changes Sep 9, 2021

View reviewed changes

mspang requested changes Sep 9, 2021

View reviewed changes

rgoliver force-pushed the chip-rpc-refactor branch from 914e859 to ae62dcc Compare September 13, 2021 15:07

rgoliver force-pushed the chip-rpc-refactor branch from ae62dcc to c14d2f4 Compare September 13, 2021 17:42

rgoliver force-pushed the chip-rpc-refactor branch from c14d2f4 to 8bdddd7 Compare September 13, 2021 17:42

woody-apple approved these changes Sep 15, 2021

View reviewed changes

mspang approved these changes Sep 15, 2021

View reviewed changes

pullapprove bot added review - approved and removed review - pending labels Sep 15, 2021

mspang merged commit 8b1cac7 into project-chip:master Sep 15, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RPC: Refactor to share common code between examples #9444

RPC: Refactor to share common code between examples #9444

rgoliver commented Sep 2, 2021

todo bot commented Sep 2, 2021

todo bot commented Sep 7, 2021

todo bot commented Sep 7, 2021

todo bot commented Sep 7, 2021

todo bot commented Sep 13, 2021

todo bot commented Sep 13, 2021

todo bot commented Sep 13, 2021

github-actions bot commented Sep 13, 2021

RPC: Refactor to share common code between examples #9444

RPC: Refactor to share common code between examples #9444

Conversation

rgoliver commented Sep 2, 2021

Problem

Change overview

Testing

todo bot commented Sep 2, 2021

auto err = DeviceLayer::SoftwareUpdateMgr().CheckNow();

This comment was generated by todo based on a TODO comment in 0651c3f in #9444. cc @rgoliver.

todo bot commented Sep 7, 2021

auto err = DeviceLayer::SoftwareUpdateMgr().CheckNow();

This comment was generated by todo based on a TODO comment in d8067bb in #9444. cc @rgoliver.

todo bot commented Sep 7, 2021

auto err = DeviceLayer::SoftwareUpdateMgr().CheckNow();

This comment was generated by todo based on a TODO comment in 230a62d in #9444. cc @rgoliver.

todo bot commented Sep 7, 2021

auto err = DeviceLayer::SoftwareUpdateMgr().CheckNow();

This comment was generated by todo based on a TODO comment in 914e859 in #9444. cc @rgoliver.

todo bot commented Sep 13, 2021

auto err = DeviceLayer::SoftwareUpdateMgr().CheckNow();

This comment was generated by todo based on a TODO comment in ae62dcc in #9444. cc @rgoliver.

todo bot commented Sep 13, 2021

auto err = DeviceLayer::SoftwareUpdateMgr().CheckNow();

This comment was generated by todo based on a TODO comment in c14d2f4 in #9444. cc @rgoliver.

todo bot commented Sep 13, 2021

auto err = DeviceLayer::SoftwareUpdateMgr().CheckNow();

This comment was generated by todo based on a TODO comment in 8bdddd7 in #9444. cc @rgoliver.

github-actions bot commented Sep 13, 2021

This comment was generated by todo based on a `TODO` comment in `0651c3f` in #9444. cc @rgoliver.

This comment was generated by todo based on a `TODO` comment in `d8067bb` in #9444. cc @rgoliver.

This comment was generated by todo based on a `TODO` comment in `230a62d` in #9444. cc @rgoliver.

This comment was generated by todo based on a `TODO` comment in `914e859` in #9444. cc @rgoliver.

This comment was generated by todo based on a `TODO` comment in `ae62dcc` in #9444. cc @rgoliver.

This comment was generated by todo based on a `TODO` comment in `c14d2f4` in #9444. cc @rgoliver.

This comment was generated by todo based on a `TODO` comment in `8bdddd7` in #9444. cc @rgoliver.