fairecasoimeme / ZiGate

Zigate is an Universal Zigbee Gateway
http://zigate.fr
171 stars 59 forks source link

Endpoint Registration Failure Due to OTA Cluster #310

Closed schrodingersket closed 3 years ago

schrodingersket commented 4 years ago

Commit 3f57b46931b42c06cec5a2a9c4c9d663103fd3bd broke endpoint registration for both the Wiser (0x0b) and ZLO (0x01) endpoints; I receive the following error for all commits after that on startup (which, notably, does not crash the application but does prevent reporting for clusters on endpoints which did not fully register):

eApp_ZLO_RegisterEndpoint 1

The error is printed from here: https://github.com/fairecasoimeme/ZiGate/blob/8d94e691cd1b7f653d3a3992f5a0806d442cde24/Module%20Radio/Firmware/src/ZiGate/Source/ZigbeeNodeControlBridge/app_zcl_event_handler.c#L183

After adding some log output, it looks like the first failure is when registering the OTA Server cluster in the Wiser endpoint.

schrodingersket commented 4 years ago

Just wanted to add some additional context to this. If I don't register all of the endpoints (Wiser, Livolo, Terncy, etc.), the application successfully starts, and the ZLO endpoint is successfully registered.

My initial thought is that this has to do with insufficient RAM when initializing all of the desired endpoints (particularly the OTA cluster); NXP recommends
sharing common cluster instances across endpoints in JN-UG-3115 to decrease RAM utilization. Are we doing this currently? If not, I think it would be a good idea to do so, since adding new endpoints for various manufacturers won't scale if we initialize a new endpoint for each one.

badzz commented 4 years ago

Are you sure the problem is new ?

schrodingersket commented 4 years ago

Yep. v3.1c did not have this issue; commit 3f57b46 fails on a clean build, but the one before (6000f73) works fine. I also tested ~10-12 commits in between 3f57b46 and v3.1d HEAD (8d94e69 at the time of this comment) with the same result.

badzz commented 4 years ago

But do you have a runtime.error or a compile error ?

fairecasoimeme commented 4 years ago

Hello, I already had this kind of issue. I updated the basic cluster file. Could you try to recompile and test ?

schrodingersket commented 4 years ago

I'm still seeing this as of 6ac632d. @badzz It's a runtime error.

Here's the UART output after a RESET:

u8NumberOfRegEndpoints : 0

 Status : 0

u8NumberOfRegEndpoints : 1

u8NumberOfRegEndpoints : 2

 Status : 0

u8NumberOfRegEndpoints : 3

 Status : 0

eApp_ZLO_RegisterEndpoint 1

tsCLD_Groups 4

tsCLD_GroupTableEntry 28Got bdb event 2

Got bdb event 1

Got stack event 2
badzz commented 4 years ago

ok issue is due to the number of timer available and needed . each endpoint with ota (5 currently) uses 1 timer. If I try to increase the number of timer by 1, it crashes on startup ..

schrodingersket commented 4 years ago

Is there a reason we couldn't share a single OTA cluster instance across all endpoints? If I understand correctly, there would then only be a single timer necessary, yeah?

badzz commented 4 years ago

yes , we are looking at this possibility indeed

On Wed, Jul 8, 2020 at 2:29 AM schrodingersket notifications@github.com wrote:

Is there a reason we couldn't share a single OTA cluster instance across all endpoints? If I understand correctly, there would then only be a single timer necessary, yeah?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/fairecasoimeme/ZiGate/issues/310#issuecomment-655209575, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABR5UYMKBRFOKKOKGTVFVBLR2O4X3ANCNFSM4OODV4UQ .

schrodingersket commented 4 years ago

Excellent. Happy to help with that if you like. I'm looking at potentially adding a couple of endpoints for some manufacturer-specific clusters (air quality data from Climax and Develco, in particular), so I'll be in and around those modules anyway.

badzz commented 4 years ago

https://github.com/fairecasoimeme/ZiGate/pull/315

schrodingersket commented 3 years ago

Closing in favor of #315