ESP32: Add bridge app example to ESP32 #8368

sweetymhaiske · 2021-07-14T07:43:30Z

Problem

Add bridge-app example to ESP32.

Change overview

Created 4 devices (non-chip devices).
Created dynamic endpoints, clusters, and attributes
Non CHIP devices are added to the endpoint 2, 4, 5, 6.

Testing

Build and flash the application on esp32 DevkitC
Performed on-off cluster control using python chip controller on the dynamically created endpoints.

command 1

chip-device-ctrl > zclread Descriptor PartsList 135246 0 0

output 1

[1626447466.983630][446912:446920] CHIP:ZCL: ReadAttributesResponse:
[1626447466.983634][446912:446920] CHIP:ZCL:   ClusterId: 0x001d
[1626447466.983640][446912:446920] CHIP:ZCL:   attributeId: 0x0003
[1626447466.983642][446912:446920] CHIP:ZCL:   status: Success                (0x0000)
[1626447466.983649][446912:446920] CHIP:ZCL:   attribute TLV Type: 0x17
[1626447466.983659][446912:446920] CHIP:ZCL:   attributeValue: List of length 4

command 2

chip-device-ctrl > zcl OnOff Off 135246 4 0

output 2

I (217115) chip[ZCL]: On/Off set value: 4 0
I (217125) chip[DL]: HandleReadOnOffAttribute: attrId=0, maxReadLength=1
I (217125) chip[ZCL]: Toggle on/off from 1 to 0
I (217135) chip[DL]: HandleWriteOnOffAttribute: attrId=0
I (217145) chip[DL]: Device[Light 3]: OFF

todo · 2021-07-14T07:43:31Z

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

connectedhomeip/examples/bridge-app/esp32/main/CHIPDeviceManager.cpp

Lines 67 to 77 in fa94d0d

    
               // 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 `fa94d0d` in #8368. cc @sweetymhaiske.

todo · 2021-07-14T10:37:33Z

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

connectedhomeip/examples/bridge-app/esp32/main/CHIPDeviceManager.cpp

Lines 67 to 77 in 6b33427

    
               // 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 `6b33427` in #8368. cc @sweetymhaiske.

todo · 2021-07-14T11:17:51Z

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

connectedhomeip/examples/bridge-app/esp32/main/CHIPDeviceManager.cpp

Lines 67 to 77 in 24f3bbc

    
               // 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 `24f3bbc` in #8368. cc @sweetymhaiske.

todo · 2021-07-14T11:21:02Z

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

connectedhomeip/examples/bridge-app/esp32/main/CHIPDeviceManager.cpp

Lines 67 to 77 in 239dede

    
               // 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 `239dede` in #8368. cc @sweetymhaiske.

todo · 2021-07-14T11:43:55Z

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

connectedhomeip/examples/bridge-app/esp32/main/CHIPDeviceManager.cpp

Lines 67 to 77 in d6c7bb9

    
               // 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 `d6c7bb9` in #8368. cc @sweetymhaiske.

todo · 2021-07-14T12:07:38Z

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

connectedhomeip/examples/bridge-app/esp32/main/CHIPDeviceManager.cpp

Lines 67 to 77 in f0f071e

    
               // 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 `f0f071e` in #8368. cc @sweetymhaiske.

todo · 2021-07-14T12:10:43Z

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

connectedhomeip/examples/bridge-app/esp32/main/CHIPDeviceManager.cpp

Lines 67 to 77 in e6d60d5

    
               // 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 `e6d60d5` in #8368. cc @sweetymhaiske.

examples/bridge-app/esp32/main/CHIPDeviceManager.cpp

examples/bridge-app/esp32/main/Device.cpp

examples/bridge-app/esp32/main/DeviceCallbacks.cpp

examples/bridge-app/esp32/main/main.cpp

examples/bridge-app/esp32/CMakeLists.txt

examples/bridge-app/esp32/main/CMakeLists.txt

examples/bridge-app/esp32/main/Kconfig.projbuild

examples/bridge-app/esp32/main/main.cpp

dhrishi · 2021-07-14T19:14:54Z

@sweetymhaiske Please add the actual controller commands in the Testing section.

todo · 2021-07-19T06:42:41Z

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

connectedhomeip/examples/bridge-app/esp32/main/CHIPDeviceManager.cpp

Lines 70 to 80 in e020577

    
               // 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 `e020577` in #8368. cc @sweetymhaiske.

sweetymhaiske · 2021-07-19T07:09:01Z

@dhrishi @bzbarsky-apple @andy31415 PTAL.

sweetymhaiske · 2021-07-20T05:01:19Z

@bzbarsky-apple PTAL

bzbarsky-apple

Thank you for addressing most of the issues!

The two problems remaining are:

This is still adding yet another copy/paste of ToZclCharString instead of putting it as a utility somewhere shared. I'd really prefer for this to be fixed before merging this, but can probably live with a commitment to do it immediately after this merges.
No tests. Followup for this, I guess, since testing on-device things is not trivial.

todo · 2021-07-23T08:35:24Z

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

connectedhomeip/examples/bridge-app/esp32/main/CHIPDeviceManager.cpp

Lines 70 to 80 in 4cf8219

    
               // 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 `4cf8219` in #8368. cc @sweetymhaiske.

sweetymhaiske · 2021-07-23T08:39:02Z

Thank you for addressing most of the issues!

The two problems remaining are:

This is still adding yet another copy/paste of ToZclCharString instead of putting it as a utility somewhere shared. I'd really prefer for this to be fixed before merging this, but can probably live with a commitment to do it immediately after this merges.

No tests. Followup for this, I guess, since testing on-device things is not trivial.

I am waiting for this PR [TV app] Store string attributes as ZCL strings #8325 to get merged. Once done I assure you, I will add those changes in this app too.
For now, I have added the local test using python controller in PR description.

todo · 2021-07-23T09:19:02Z

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

connectedhomeip/examples/bridge-app/esp32/main/CHIPDeviceManager.cpp

Lines 70 to 80 in fdca9cc

    
               // 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 `fdca9cc` in #8368. cc @sweetymhaiske.

sweetymhaiske · 2021-07-28T07:58:13Z

@bzbarsky-apple PTAL

bzbarsky-apple · 2021-07-29T22:43:00Z

@andy31415 are your comments here addressed?

sweetymhaiske · 2021-08-03T07:14:01Z

@andy31415 Please help to get this merge.

This reverts commit 1ef0443.

bzbarsky-apple · 2021-08-04T20:57:39Z

@sweetymhaiske Should this new app have a README.md explaining how to build and whatnot?

bzbarsky-apple · 2021-08-04T21:01:49Z

@sweetymhaiske And shouldn't examples/bridge-app/esp32/build/ and examples/bridge-app/esp32/sdkconfig be added to .gitignore?

boring-cyborg bot added the examples label Jul 14, 2021

restyled-io bot mentioned this pull request Jul 14, 2021

Restyle ESP32: Add bridge app example to ESP32 #8369

Closed

sweetymhaiske force-pushed the bridge_app_esp32 branch from fa94d0d to 6b33427 Compare July 14, 2021 10:37

sweetymhaiske force-pushed the bridge_app_esp32 branch from 6b33427 to 24f3bbc Compare July 14, 2021 11:17

sweetymhaiske force-pushed the bridge_app_esp32 branch from 24f3bbc to 239dede Compare July 14, 2021 11:21

sweetymhaiske force-pushed the bridge_app_esp32 branch from 239dede to d6c7bb9 Compare July 14, 2021 11:43

sweetymhaiske force-pushed the bridge_app_esp32 branch from d6c7bb9 to f0f071e Compare July 14, 2021 12:07

sweetymhaiske force-pushed the bridge_app_esp32 branch from f0f071e to e6d60d5 Compare July 14, 2021 12:10

pullapprove bot requested review from andy31415, bzbarsky-apple, chrisdecenzo, Damian-Nordic, hawk248, jepenven-silabs and msandstedt July 14, 2021 12:14

pullapprove bot added the review - pending label Jul 14, 2021

msandstedt approved these changes Jul 14, 2021

View reviewed changes

andy31415 added the Tested manually label Jul 14, 2021

andy31415 reviewed Jul 14, 2021

View reviewed changes

bzbarsky-apple requested changes Jul 14, 2021

View reviewed changes

dhrishi reviewed Jul 14, 2021

View reviewed changes

sweetymhaiske force-pushed the bridge_app_esp32 branch from e6d60d5 to d76720d Compare July 15, 2021 06:23

woody-apple added the SDK Approved label Jul 20, 2021

woody-apple approved these changes Jul 22, 2021

View reviewed changes

bzbarsky-apple reviewed Jul 23, 2021

View reviewed changes

sweetymhaiske force-pushed the bridge_app_esp32 branch from e020577 to 4cf8219 Compare July 23, 2021 08:35

boring-cyborg bot added github workflows labels Jul 23, 2021

ESP32: Add bridge app example to ESP32

fdca9cc

sweetymhaiske force-pushed the bridge_app_esp32 branch from 4cf8219 to fdca9cc Compare July 23, 2021 09:18

dhrishi requested a review from bzbarsky-apple July 27, 2021 15:48

dhrishi approved these changes Jul 27, 2021

View reviewed changes

bzbarsky-apple approved these changes Jul 29, 2021

View reviewed changes

andy31415 approved these changes Aug 4, 2021

View reviewed changes

pullapprove bot added review - approved and removed review - pending labels Aug 4, 2021

andy31415 added the technical review - DONE label Aug 4, 2021

andy31415 merged commit 1ef0443 into project-chip:master Aug 4, 2021

kpschoedel added a commit to kpschoedel/connectedhomeip that referenced this pull request Aug 4, 2021

Revert "ESP32: Add bridge app example to ESP32 (project-chip#8368)"

b2caf1f

This reverts commit 1ef0443.

bzbarsky-apple mentioned this pull request Aug 4, 2021

Fix esp32 bridge-app compile #8804

Merged

nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this pull request Sep 23, 2021

ESP32: Add bridge app example to ESP32 (project-chip#8368)

1b717b7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ESP32: Add bridge app example to ESP32 #8368

ESP32: Add bridge app example to ESP32 #8368

sweetymhaiske commented Jul 14, 2021 •

edited

Loading

todo bot commented Jul 14, 2021

todo bot commented Jul 14, 2021

todo bot commented Jul 14, 2021

todo bot commented Jul 14, 2021

todo bot commented Jul 14, 2021

todo bot commented Jul 14, 2021

todo bot commented Jul 14, 2021

dhrishi commented Jul 14, 2021

todo bot commented Jul 19, 2021

sweetymhaiske commented Jul 19, 2021

sweetymhaiske commented Jul 20, 2021

bzbarsky-apple left a comment

todo bot commented Jul 23, 2021

sweetymhaiske commented Jul 23, 2021

todo bot commented Jul 23, 2021

sweetymhaiske commented Jul 28, 2021

bzbarsky-apple commented Jul 29, 2021

sweetymhaiske commented Aug 3, 2021

bzbarsky-apple commented Aug 4, 2021

bzbarsky-apple commented Aug 4, 2021

ESP32: Add bridge app example to ESP32 #8368

ESP32: Add bridge app example to ESP32 #8368

Conversation

sweetymhaiske commented Jul 14, 2021 • edited Loading

Problem

Change overview

Testing

todo bot commented Jul 14, 2021

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

This comment was generated by todo based on a TODO comment in fa94d0d in #8368. cc @sweetymhaiske.

todo bot commented Jul 14, 2021

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

This comment was generated by todo based on a TODO comment in 6b33427 in #8368. cc @sweetymhaiske.

todo bot commented Jul 14, 2021

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

This comment was generated by todo based on a TODO comment in 24f3bbc in #8368. cc @sweetymhaiske.

todo bot commented Jul 14, 2021

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

This comment was generated by todo based on a TODO comment in 239dede in #8368. cc @sweetymhaiske.

todo bot commented Jul 14, 2021

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

This comment was generated by todo based on a TODO comment in d6c7bb9 in #8368. cc @sweetymhaiske.

todo bot commented Jul 14, 2021

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

This comment was generated by todo based on a TODO comment in f0f071e in #8368. cc @sweetymhaiske.

todo bot commented Jul 14, 2021

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

This comment was generated by todo based on a TODO comment in e6d60d5 in #8368. cc @sweetymhaiske.

dhrishi commented Jul 14, 2021

todo bot commented Jul 19, 2021

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

This comment was generated by todo based on a TODO comment in e020577 in #8368. cc @sweetymhaiske.

sweetymhaiske commented Jul 19, 2021

sweetymhaiske commented Jul 20, 2021

bzbarsky-apple left a comment

Choose a reason for hiding this comment

todo bot commented Jul 23, 2021

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

This comment was generated by todo based on a TODO comment in 4cf8219 in #8368. cc @sweetymhaiske.

sweetymhaiske commented Jul 23, 2021

todo bot commented Jul 23, 2021

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

This comment was generated by todo based on a TODO comment in fdca9cc in #8368. cc @sweetymhaiske.

sweetymhaiske commented Jul 28, 2021

bzbarsky-apple commented Jul 29, 2021

sweetymhaiske commented Aug 3, 2021

bzbarsky-apple commented Aug 4, 2021

bzbarsky-apple commented Aug 4, 2021

sweetymhaiske commented Jul 14, 2021 •

edited

Loading

This comment was generated by todo based on a `TODO` comment in `fa94d0d` in #8368. cc @sweetymhaiske.

This comment was generated by todo based on a `TODO` comment in `6b33427` in #8368. cc @sweetymhaiske.

This comment was generated by todo based on a `TODO` comment in `24f3bbc` in #8368. cc @sweetymhaiske.

This comment was generated by todo based on a `TODO` comment in `239dede` in #8368. cc @sweetymhaiske.

This comment was generated by todo based on a `TODO` comment in `d6c7bb9` in #8368. cc @sweetymhaiske.

This comment was generated by todo based on a `TODO` comment in `f0f071e` in #8368. cc @sweetymhaiske.

This comment was generated by todo based on a `TODO` comment in `e6d60d5` in #8368. cc @sweetymhaiske.

This comment was generated by todo based on a `TODO` comment in `e020577` in #8368. cc @sweetymhaiske.

This comment was generated by todo based on a `TODO` comment in `4cf8219` in #8368. cc @sweetymhaiske.

This comment was generated by todo based on a `TODO` comment in `fdca9cc` in #8368. cc @sweetymhaiske.