[BUG] dynamic-bridge - handling of 0 as default attribute value is different for static and dynamic clusters

[bndkk]

Reproduction steps

As the title says: If the default value of a nullable numeric cluster attribute is set to 0 in a .zap file it doesn't behave the same for static and dynamic(generated from matter idl) clusters.
Because of this line, if a an attribute default in a .zap file is set to 0 the default doesn't get set in the .matter file and a nullable value gets initialized to NULL, when using dynamic generation.
On the other hand if the same cluster attribute is used statically(from GENERATED_CLUSTERS array), 0 is a legitimate value and isn't considered as NULL.

This has caused me some issues, because Apple Home would always say "Device not responding", when I used it with the dynamically generated WindowCovering cluster, which has the CurrentPositionLiftPercent100ths attribute, which is left empty(0) (NULL when generating dynamically) by default. When using it with fixed clusters(all-clusters-app) it would work fine because the attribute would return 0.

Bug prevalence

Whenever I do this

GitHub hash of the SDK that was being used

7ccd5d4

Platform

core

Platform Version(s)

No response

Anything else?

No response

[bzbarsky-apple]

This seems like a bug in the IDL bits, @andy31415

[andy31415]

Looks like some inconsistency for sure.

@bndkk could you provide me with a concrete example/reproduction? What zap file should I use or how should I change things to see this behavior? Can you give me a concrete nullable numeric attribute that I can try things out with?

[andy31415]

I see CurrentPositionLiftPercent100ths referenced. Taking a look.

What would be your expectation for the IDL to contain?

[andy31415]

Currently I see in examples/window-app/common/window-app.matter:

 server cluster WindowCovering {
    ram      attribute type default = 0x08;
    // ...
    ram      attribute endProductType;
    persist  attribute currentPositionLiftPercent100ths;
    // ...

[andy31415]

In zap I see (for server side):

            {
              "name": "CurrentPositionLiftPercent100ths",
              "code": 14,
              "mfgCode": null,
              "side": "server",
              "type": "Percent100ths",
              "included": 1,
              "storageOption": "NVM",
              "singleton": 0,
              "bounded": 0,
              "defaultValue": "0",
              "reportable": 1,
              "minInterval": 0,
              "maxInterval": 65344,
              "reportableChange": 0
            },

so it does seem like it should want a defaultValue of 0.

[andy31415]

The issue may be with the logic of {{~#if_is_non_zero_default defaultValue~}} within the template. This default is obviously zero, however the value is nullable so being zero is important.

[bzbarsky-apple]

For a nullable attribute where the default is actually set to null, the defaulValue will in fact be null, I believe.

if_is_non_zero_default calls isNonZeroValue which returns false for null, 0, empty string, and in general anything else that will be treated as falsy by JS.

It also returns false for the string "0", strings that look like "0x00", and the string "false".

Which means it's broken for nullable attributes of various types.

Why are we testing for this "non_zero" condition at all? If the default value is 0, then the default value is 0 and should be represented accordingly in the .matter file, no?

[andy31415]

Not sure about the logic directly, from what I remember it may be that zap was setting default value 0 for things that could not be 0 (e.g. nullable strings defaulting to "0").

I will try to see how things behave when I remove that logic.