# Cluster standardization

## Goals

- Consistent API/layout to allow for easy maintenance. Requirement to entertain a Long Term Release that allows for cluster updates
- Allow platform certification to include clusters
- Cluster integration would not be application-dependent anymore.
- Allow cluster code to be unit tested for functionality
- We currently have almost no unit testing for clusters. Integration testing generally only covers happy-paths.
- Overcome existing scaling issues with separate per-application configuration
- We currently do not even have CI resources to compile-test all examples and the list of example applications keeps growing
- We want both compile and actual test (conformance at a minmum) for all examples
- Improve speed and consistency of adding new clusters
- Adding new clusters and device types should be much faster and decoupled

## References

- Cluster design [for testability and portability](https://project-chip.github.io/connectedhomeip-doc/cluster_and_device_type_dev/unit_testing_clusters.html)
- Work in progress on a complex [Valve cluster redesign - PR 37207](https://github.com/project-chip/connectedhomeip/pull/37207)
- Example of [ConcentrationMeasurementServer](https://github.com/project-chip/connectedhomeip/blob/master/src/app/clusters/concentration-measurement-server/concentration-measurement-server.h#L98) for using `constexpr` template arguments to control flash usage

## Proposed layout for clusters

Here we will assume the cluster `DiscoBall` as a placeholder of any other cluster (such as AccessControl, AccountLogin, ..., WindowCovering). There are currently 90 clusters in [src/app/clusters](https://github.com/project-chip/connectedhomeip/tree/master/src/app/clusters) and we expect to modify all of them.

- `src/app/clusters/disco-ball/`
- `BUILD.gn`
- `DiscoBallCluster.cpp`
- `DiscoBallCluster.h`
- `DiscoBallLogic.cpp`
- `DiscoBallLogic.h`
- `DiscoBallDriver.h`
- `DiscoBallEmberCompatibility.cpp`
- `tests/`
- `BUILD.gn`
- `TestDiscoBall.cpp`
- `legacy/`
- `<existing files/pure ember coupling> as transitional step`

More extra files may exist for complex structures and to make things less coupled, however the above basic structure would be the expected public API for every cluster.

### Layout details

- `src/app/clusters/disco-ball`
- **`BUILD.gn`** - will contain actual targets, referencing all _except_ the ember compatibility layer. Clusters will declare and enforce their dependencies
- **`DiscoBallCluster.{h,cpp}`** contains the translation layer between data model structures (AttributeEncoders/Decoders, paths) into typed data
- A class instance exists for every **instance** of a cluster (i.e. a endpoint/cluster pair). It does _not_ support wildcard endpoints (as current AttributeAccessInterface does)
- Contains a `DiscoBallLogic` instance for actual operation
- Is initialized with a `Context` pointer to function.
- The generic _everything_ would be a DataModel::Provider as well as an AttributePersistenceProvider. Clusters generally want:
- in-processing support (e.g. attribute persistence)
- async support (e.g. event triggering)
- Contexts are received as _pointers_ to objects whose lifetime exceeds the lifetime of the cluster itself (so that we pay minimal RAM overhead for the context)
:::info
A subset of possible contexts is typical, so that applications only need to construct some objects of this type.
:::
- **`DiscoBallLogic.{h,cpp}`** contains the typed cluster logic, like get/set attributes, reading values
- A class instance exists for every **instance** of a cluster (i.e. a endpoint/cluster pair).
- Contains `State` information that store attribute values (i.e. does NOT use Accessors.h/cpp as that is ember-specific)
- Contains any `driver` pointer
- Contains any `context` pointer
- **`DiscoBallDriver.h`** contains the virtual interface for exposing functionality to applications (overrides to notify clusters of actions). Currently called `Delegate` in many places, however `Delegate` is also often overloaded in meaning hence the rename to `Driver`
- **`DiscoBallEmberCompatibility.cpp`** - this is the only file _NOT_ part of `BUILD.gn` and is instead included as part of `data_model.gni/cmake` based on `.zap` file selection
- Has access to `endpoint_config.h` constants and can initialize clusters based on that (e.g. using endpoint_count)
- implements ember coupling (e.g. startup of clusters during `Init` calls)

## Defining a `Cluster`

Existing code defines a [AttributeAccessInterface](https://github.com/project-chip/connectedhomeip/blob/master/src/app/AttributeAccessInterface.h) for attribute handling and a [CommandHandlerInterface](https://github.com/project-chip/connectedhomeip/blob/master/src/app/CommandHandlerInterface.h) for command handling. They are handled as intrusive lists and managed through _Registries_ of [AttributeAccessInterfaceRegistry](https://github.com/project-chip/connectedhomeip/blob/master/src/app/AttributeAccessInterfaceRegistry.h) and [CommandHandlerInterfaceRegistry](https://github.com/project-chip/connectedhomeip/blob/master/src/app/CommandHandlerInterfaceRegistry.h) respectively.

While some clusters are indeed only attributes or only commands, this separation seems to have been historically motivated by convenience only (attributes used to be handled in ember while commands had a InteractionModelEngine bypass). Every general proposal for cluster implementations usually want a single **`Cluster`** type.

:::warning
This proposes an **API CHANGE** to the existing interfaces and will require a transition from AAI/CHI to a Cluster.
:::

A cluster structure should combine the abilities of AAI and CHI into a single class. More over, it should also provide metadata (currently only incomplete metadata is available for CHI only). Specifically we should provide:
- Read/Write Attributes
- List supported Attributes (metadata for attributes)
- Invoke commands
- List accepted/generated commands (metadata for commands)
- Cluster metadata
- In particular data version is handled by the cluster

The interfaces are heavily modeled after [Provider metadata](https://github.com/project-chip/connectedhomeip/blob/master/src/app/data-model-provider/ProviderMetadataTree.h#L63C1-L65C124) for metadata listing and [Provider insterfaces](https://github.com/project-chip/connectedhomeip/blob/master/src/app/data-model-provider/Provider.h#L69) for read/write/invoke. This also assumes list write notification (begin/end) will be plumbed through (see [Issue 37307](https://github.com/project-chip/connectedhomeip/issues/37307) in the SDK)

/// NOTE the full request/path usage here, to allow cluster interfaces
/// to customize their reply (e.g. based on endpoint ids or on paths)
class ServerClusterInterface {
public:
    ///////////////////// CLUSTER SUPPORT ///////////////////// 
    
    // Every cluster must have a data version. Base class implementation
    // to avoid code duplication
    DataVersion GetDataVersion() const;
    void IncreaseDataVersion();
    
    // Cluster metadata support. Virtual as we expect this to be 
    // stored in flash as code rather than take up RAM as part of 
    // the instance
    virtual ClusterId GetClusterId() const = 0;
    virtual BitFlags<ClusterQualityFlags> GetClusterFlags() const = 0;
    
    ///////////////////// ATTRIBUTE SUPPORT ///////////////////// 
    
    virtual ActionReturnStatus ReadAttribute(const ReadAttributeRequest &request, AttributeValueEncoder &encoder) { return Status::UnsupportedAttribute; }
    virtual ActionReturnStatus WriteAttribute(const WriteAttributeRequest &request, AttributeValueDecoder &decoder) { return Status::UnsupportedAttribute; }
    virtual void ListAttributeNotification(const ListAttributeNotificationRequest &) {}
    virtual CHIP_ERROR Attributes(const ConcreteClusterPath & path, ListBuilder<AttributeEntry> & builder) { return CHIP_NO_ERROR; }
    
    ///////////////////// COMMAND SUPPORT ///////////////////// 
    virtual std::optional<ActionReturnStatus> Invoke(const InvokeRequest & request, chip::TLV::TLVReader & input_arguments,
                                                     CommandHandler * handler) { return Status::UnsupportedCommand; }
    virtual CHIP_ERROR GeneratedCommands(const ConcreteClusterPath & path, ListBuilder<CommandId> & builder) { return CHIP_NO_ERROR; }
    virtual CHIP_ERROR AcceptedCommands(const ConcreteClusterPath & path, ListBuilder<AcceptedCommandEntry> & builder) { return CHIP_NO_ERROR; }
    
    // additional non-public-interface members may exist here,
    // like current AAI supporting intrusive lists, however those
    // will not be considered stable API.
};

For transition from previous interfaces, we will:
- provide the ability to create a `ServerClusterInterface` based on AAI/CHI
- The created `ServerClusterInterface` will use ember for its metadata
- AAI/CHI Registries _will be removed_ and applications will be expected to replace their usage of AAI/CHI registration with a ClusterInterface registration

`ServerClusterInterfaceRegistry` will be available to register cluster interfaces. Actual layout of this will NOT be a public interface. We expect it to contain caching and some form of intrusive container (linear list as today or some form of [intrusive map](https://pigweed.dev/pw_containers/#pw-intrusivemap) for performance reasons). The only public interface for the registry will be:
- `Register(const ConcreteClusterPath &path, ServerClusterInterface *)`
- `Unregister(const ConcreteClusterPath &path)`

## Implementation

### Optimizing for flash usage

The [ConcentrationMeasurementServer](https://github.com/project-chip/connectedhomeip/blob/master/src/app/clusters/concentration-measurement-server/concentration-measurement-server.h#L98) concept will be followed. This will be done **if necessary** only as there is some complexity overhead. Guiding principle is to determine if some class variants are likely to need significantly smaller footprint for small devices (e.g. utility/root endpoint clusters or clusters specifically needing small storage). Approach is:

- `ServerClusterInterface` will be a `template` class that takes as a constexp argument the corresponding `FeatureBits` (**NOTE**: this is different than the sequence of booleans currently used by `ConcentrationMeasurement`)
- Implementations can use:
- `std::conditional_t` inheritance to control what members are used (i.e. RAM utilization for attribute storage or other functionality)
- `if constexpr (...)` to control call paths, generally within a `switch (attributeid)` and `switch (commandid)` equivalent when handling attribute read/writes and command invokes

As a result, we expect to be able to optimize both flash and RAM utilization at the expense of some code complexity.

## Existing ember functionality

### Persistence

Ember automatically handles attribute persistence. AAI (and `ServerClusterInterface` as a result) must handle its own persistence. We have to provide wrappers/methods that ensure backwards compatibility for data (in a tested way)

### Defaults

Ember has `GENERATED_DEFAULTS` inside its `endpoint_config.h`. Transition to `ServerClusterInterface` will likely not need such metadata anymore. We assume cluster interfaces will have correct defaults. `EmberCompatibility` is expected to initialize interfaces with correct defaults, either loaded from ember or with correct values to begin with (e.g. revision is not taken from zap anymore)

### Min/Max or other validation settings

Cluster implementations are expected to validate values during `AttributeWrite` operations.
In particular the following validation needs to be carried over from EMBER (see `emAfWriteAttribute` in `attribute-table.cpp`):
- enforce read-only: this should be done through metadata already in the IteractionModelEngine
- min/max validation if available (see usage of `ATTRIBUTE_MASK_MIN_MAX`). Return ConstraintError on out of range.
- Accept/Reject null values (oddly enough ember validates this only if min/max mask exists)

###

## Existing ember callbacks

### `Matter<Cluster>ClusterServerInitCallback`

Provides a single global initialization for an endpoint. Ember compatibility will use this to set up `ServerClusterRegistry` registration (it is currently used to register AAI/CHI)

### `emberAf<CLUSTER>ClusterInitCallback` / `Matter<CLUSTER>ServerShutdownCallback`

These are _per endpoint_ startup/shutdown functions. Code may chose to use these to control `ServerClusterRegistry` register/unregister.

### `MatterPreAttributeChangeCallback`

Used for a GLOBAL pre-processing. Used for logging (pump/silabs and pump/cc13x4_26x4). Since this does not work today with AAI at all, we expect this to just exist for historical purposes.

:::warning
This callback usage/support will be removed and it will never be called
:::

### `Matter<CLUSTER>ClusterServerAttributeChangedCallback`

This is called by ember after its internal data store has been updated.

Design choices:
- **Use drivers**: Every driver to expose a `OnAttribuesChanged` callback and the ember compatibility layers registers one such driver to call `Matter<CLUSTER>ClusterServerAttributeChangedCallback` or we update applications to register a relevant driver
- **ServerClusterInstance**: The ember compatibility driver will register a subclass of ServerClusterInstance which automatically calls `Matter<CLUSTER>ClusterServerAttributeChangedCallback` after any `AttributeWrite` call that updates DataVersion.

### `MatterReportingAttributeChangeCallback`

This is used A LOT throughout our codebase and likey by any AAI/CHI integration. This does:
- automatic dataVersion increase on corresponding clusters
- triggering of subscription handling/updates

In practice, this was used attribute ownership and data changes were
independent, especially since AAI does not own the versioning.

In terms of path forward we expect:

- This callback **will keep working** and will update DataModelProvider (and as a result `ServerClusterInterface::IncreaseVersion` + a change trigger)
- As clusters own their own data verstion, we expect handling of this to be without this method:
- `ServerClusterInterface` maintains its own cluster version
- `ServerClusterInterface` context should have access to a [ProviderChangeListener::MarkDirty](https://github.com/project-chip/connectedhomeip/blob/master/src/app/data-model-provider/ProviderChangeListener.h#L42) and call that directly (e.g. get this from `DataModel::Provider::Context::dataModelChangeListener`)

Usages of `MatterReportingAttributeChangeCallback` should decrease over time as changing attributes outside cluster implementations should not be possible anymore.

The only eventual use of some global method (TBD) will be structural changes within endpoints.