connectedhomeip icon indicating copy to clipboard operation
connectedhomeip copied to clipboard

[BUG] Software Diagnostics: The static-assert breaks the firmware builds in case of dynamic root node endpoint.

Open shripad621git opened this issue 5 months ago • 2 comments

Reproduction steps

In the current Matter codebase implementation, certain clusters such as OTA Software Update Provider and Software Diagnostics handle their endpoint configurations via a StaticApplicationConfig::kFixedClusterConfig array. Case 1: OTA Provider Cluster In case of OTA Provider for dynamic endpoints, we pass empty configuration in case of static-cluster-config files. Else the code crashes at https://github.com/project-chip/connectedhomeip/blob/0c2115b7efd391b9b1fd5c72aea98be658f71bb6/src/app/util/attribute-storage.cpp#L917 since fixed endpoint count is zero and that function gets called through https://github.com/project-chip/connectedhomeip/blob/0c2115b7efd391b9b1fd5c72aea98[…]e658f71bb6/src/app/clusters/ota-provider/CodegenIntegration.cpp.

namespace chip {
namespace app {
namespace Clusters {
namespace OtaSoftwareUpdateProvider {
namespace StaticApplicationConfig {
using FeatureBitmapType = Clusters::StaticApplicationConfig::NoFeatureFlagsDefined;
inline constexpr std::array<Clusters::StaticApplicationConfig::ClusterConfiguration<FeatureBitmapType>, 0> kFixedClusterConfig = { };
} // namespace StaticApplicationConfig
} // namespace OtaSoftwareUpdateProvider
} // namespace Clusters
} // namespace app
} // namespace chip

Case 2: Software Diagnostics Cluster: The static asserts in CodegenIntegration.cpp of this cluster https://github.com/project-chip/connectedhomeip/blob/0c2115b7efd391b9b1fd5c72aea98[…]app/clusters/software-diagnostics-server/CodegenIntegration.cpp sort of mandate the fixed endpoints and asserts when we pass the empty config as we do in the case of OTAProvider. While the OTA Provider cluster permits dynamic-only endpoint configurations by allowing an empty kFixedClusterConfig array (with appropriate handling in CodegenIntegration.cpp), the Software Diagnostics cluster uses static_asserts in its CodegenIntegration.cpp to enforce the presence of exactly one fixed endpoint (on endpoint 0), which causes a build-time failure if an empty configuration is provided. Would it be possible to consider a similar dynamic endpoint handling approach for the Software Diagnostics cluster CodegenIntegration? This would allow applications to optionally omit fixed endpoints for this cluster and rely on dynamic endpoint registrations.

Bug prevalence

In case of firmwares on which endpoint 0 is also dynamic.

GitHub hash of the SDK that was being used

871b4347b3b068dc161d6a02f867a0335cff13ed

Platform

esp32

Platform Version(s)

No response

Anything else?

No response

shripad621git avatar Jun 16 '25 14:06 shripad621git

@andy31415 I would think that if everything is fully dynamic we should not be building CodegenIntegration bits at all.

But the bigger question of what happens if endpoint 0 is dynamic but other endpoints are not, and CodegenIntegration is therefore required, remains. It looks like SoftwareDiagnostics is forcing a static config for endpoint 0, which is not desirable.....

bzbarsky-apple avatar Jun 16 '25 15:06 bzbarsky-apple

I will look into creating a PR that allows dynamic config on EP0 (so restriction of "only on EP0" remains but we allow it to be dynamic).

Beyond that though, we would like to eventually support fully dynamic without any codegen build, however for that we have to convert all EP0 clusters to start and then start supporting device types. That work is ongoing but it takes a very long time.

andy31415 avatar Jun 16 '25 15:06 andy31415