kUknownEnumValue of ModeBase derivative clusters clash with CommonCode and CommonTags

[tehampson]

Problem:

In the derived ModeBase cluster (such as RvcRunMode) the enumerated DerivedClusterTags and DerivedClusterCodes contains an kUnknownEnumValue of 0 that overlap with CommonTag,CommonCode from ModeBase.

Severity:

Currently: None to possibly minimal.
Currently it doesn't seems like any derived ModeBase cluster code is using kUnknownEnumValue. It just seems that offering these values in code gen might incorrectly be assumed by some future developer as a valid value enum value. Like perhaps a client assumes that vendor tags/code they don't know will be set to kUnknownEnumValue.

Background:

The ModeTagStruct and ChangeToModeResponse while are of type enum in the xml they don't actually specify a particular enum that contains elements. As a result, any auto-generated code that forms the basis of the data model uses the type uint16_t(here) and uint8_t(here), and is not strongly typed to something like RvcRunMode::ModeTag and RvcRunMode::StatusCode

The code generated enums are not actually used for any DataModel encode/decode checks, but are more so generated to provide better code readability. For example setting tag to ModeBase::ModeTag::kQuiet instead of 0x0001

Possible solutions:

1. Allow for enum in xml to specify that they are for code readability purposes only such that zap generate enum to be used more for readability. This would remove the kUnknownEnumValue so it doesn't get used incorrectly and also remove the decode method from the cluster-enum-check.h.
2. Work towards strong typing of the enum values for the ModeBase derivative clusters. This would likely be done by allowing for xml enums to specify the base enum it wishes to inherit values from. Although in doing this I foresee the issue where ModeTagStruct would need to be replaced by one that contains the strongly typed enum value and have a separate definition for each of the derived clusters instead of a shared common version. This will also likely be a breaking change as application are already using a mixture of the two types of enums, as is even done in our examples

[bzbarsky-apple]

Allow for enum in xml to specify that they are for code readability purposes only

Or do this via our src/app/common/templates/config-data.yaml metadata, which would actually be quite simple. Might be a good idea to just do that for now.

[tehampson]

Are you referring to WeakEnums in src/app/common/templates/config-data.yaml? I think Andrei is trying to burn these down to 0. I would like to avoid adding tech debt there if possible

If you are referring to something else please let me know what you were thinking

[bzbarsky-apple]

Are you referring to WeakEnums in src/app/common/templates/config-data.yaml?

I am referring to adding a different list in that file for the behavior we want, then changing our codegen to have the desired behavior for things in that list.

That file is used for various things, some of which are tech debt and some of which are not.