|
| 1 | +# Designing Clusters for Testing and Portability |
| 2 | + |
| 3 | +## Unit Testable, Modular Cluster Design |
| 4 | + |
| 5 | +When designing new clusters, consider the following approach: |
| 6 | + |
| 7 | +- Separate the cluster logic from the on-the-wire data model |
| 8 | + - Server vs. ClusterLogic |
| 9 | + - Makes the cluster logic unit-testable without generating TLV. |
| 10 | +- Separate the basic cluster logic from code that is platform- or |
| 11 | + device-specific. |
| 12 | + - ClusterLogic uses a ClusterDriver |
| 13 | + - Makes the cluster logic portable between platforms / manufacturers |
| 14 | + - Removes necessity of overriding global singleton functions like |
| 15 | + PostAttributeChangeCallback. |
| 16 | + |
| 17 | +General approach: |
| 18 | + |
| 19 | + |
| 20 | + |
| 21 | +Class proposal: |
| 22 | + |
| 23 | + |
| 24 | + |
| 25 | +### ClusterServer |
| 26 | + |
| 27 | + |
| 28 | + |
| 29 | +The ClusterServerClass is a **Very** light wrapper over ClusterLogic. It |
| 30 | +translates Interaction Model wire format handling into API calls for cluster |
| 31 | +logic methods. |
| 32 | + |
| 33 | +This class iImplements both the AttributeAccessInterface and the CommandHandler |
| 34 | +interfaces so ClusterLogic properly handles data dependencies between command |
| 35 | +and attributes. |
| 36 | + |
| 37 | +An example code snippet showing the translation of the TLV into API calls to the |
| 38 | +ClusterLogic class: |
| 39 | + |
| 40 | +```c++ |
| 41 | +CHIP_ERROR DiscoBallServer::Read(const ConcreteReadAttributePath & aPath, |
| 42 | + AttributeValueEncoder & aEncoder) |
| 43 | +{ |
| 44 | + DiscoBallClusterLogic * cluster = FindEndpoint(aPath.mEndpointId); |
| 45 | + VerifyOrReturnError(cluster != nullptr, CHIP_IM_GLOBAL_STATUS(UnsupportedEndpoint)); |
| 46 | + |
| 47 | + switch (aPath.mAttributeId) |
| 48 | + { |
| 49 | + case Clusters::DiscoBall::Attributes::Run::Id: |
| 50 | + return aEncoder.Encode(cluster->GetRunAttribute()); |
| 51 | + … |
| 52 | + } |
| 53 | +} |
| 54 | +``` |
| 55 | +
|
| 56 | +### ClusterLogic |
| 57 | +
|
| 58 | + |
| 59 | +
|
| 60 | +The ClusterLogic class is for all the code that is SHARED between platforms. It |
| 61 | +does NOT include any TLV parsing or direct calls to Ember/IM/LogEvent etc. |
| 62 | +
|
| 63 | +The class should include attribute getter/setters and handlers for all commands. |
| 64 | +
|
| 65 | +The class receive “plain data” Matter requests from ClusterServer class, |
| 66 | +performs required common actions, and calls driver class to perform platform- or |
| 67 | +hardware-specific actions. It also receives driver updates (e.g. |
| 68 | +application-driven value changes) from the ClusterDriver class and updates state |
| 69 | +as appropriate. |
| 70 | +
|
| 71 | +The class should handle spec-requirements for: |
| 72 | +
|
| 73 | + - Range checking (CONSTRAINT_ERROR) |
| 74 | + - Attribute and metadata storage (persistent or in-memory) |
| 75 | + - Data dependencies between commands and attributes |
| 76 | + - Event generation / dirty attributes (callback to server) |
| 77 | + - Calling driver when platform or hardware interactions are required |
| 78 | +
|
| 79 | +API recommendation: |
| 80 | +
|
| 81 | + - Maintain all cluster state in a separate data-only class. |
| 82 | + - Provider getters/setters for application logic to use. |
| 83 | + - Implement handlers for all commands, conditional on features. |
| 84 | + - Let the caller provide (inject) dependencies. Avoid explicit memory |
| 85 | + management. |
| 86 | +
|
| 87 | +### ClusterDriver |
| 88 | +
|
| 89 | +Implements hardware or platform-specific actions required on cluster |
| 90 | +interactions or when application wants to report state changes. |
| 91 | +
|
| 92 | + |
| 93 | +
|
| 94 | +### MatterContext |
| 95 | +
|
| 96 | +The ClusterLogic class must not directly use global resource because they cannot |
| 97 | +be isolated for testing. Instead, the MatterContext holds pointers to Matter |
| 98 | +stack objects, which can be be injected / faked for testing. This includes - |
| 99 | +Wrapper over IM Engine interface functions for marking attributes dirty, and |
| 100 | +logging events. - Storage - Anything you would normally access with |
| 101 | +Server::GetInstance() |
| 102 | +
|
| 103 | + |
| 104 | +
|
| 105 | +### ClusterDriver |
| 106 | +
|
| 107 | +The ClusterDriver is called by the ClusterLogic class and is used to translate |
| 108 | +attribute changes and commands into application actions. It also reports |
| 109 | +external changes back to the ClusterLogic class. |
| 110 | +
|
| 111 | +The API design for this class with vary by the cluster, but it si generally |
| 112 | +recommended to use a generic API where possible, so the API ports easily to |
| 113 | +other platforms. For example an attribute changed callback with the changes |
| 114 | +listed. It is important to be careful about the design and revisit this early if |
| 115 | +issues arise. |
| 116 | +
|
| 117 | +## Unit testing with the modular cluster design |
| 118 | +
|
| 119 | +### ClusterLogic class |
| 120 | +
|
| 121 | +The unit test instantiates the ClusterLogic, provides MatterContext and |
| 122 | +ClusterDriver instance with fakes/mocks for testing. |
| 123 | +
|
| 124 | +Unit test against the API, and check the fakes/mocks to ensure they are being |
| 125 | +called as appropriate. |
| 126 | +
|
| 127 | +As with all unit tests, prefer testing for behavior rather than implementation |
| 128 | +details. |
| 129 | +
|
| 130 | +Important tests to consider: |
| 131 | +
|
| 132 | +- Initialization and initial attribute correctness. |
| 133 | +- Errors for out-of-range on all attribute setters and command handlers. |
| 134 | +- All spec-defined error conditions, especially ones that are difficult to |
| 135 | + trigger. |
| 136 | +- Data dependencies between commands and attributes. |
| 137 | +- Incoming actions in different states (stopped, running, etc). |
| 138 | +- Calls out to storage for persistent attributes. |
| 139 | +- Calls out to driver for changes as appropriate. |
| 140 | +- Driver error reporting. |
| 141 | +- Event generation and dirty attribute marking, including attributes that are |
| 142 | + changed from the driver side. |
| 143 | +- Others - very dependent on the cluster. |
| 144 | +
|
| 145 | +# Unit testing ClusterServer |
| 146 | +
|
| 147 | +- Best to have the lightest wrapping possible |
| 148 | + - If the wrapper is light, the code can be covered by integration or unit |
| 149 | + tests, or a combination. |
| 150 | + - Correctness can mostly be validated by inspection if it’s trivial. |
| 151 | +- Important tests |
| 152 | + - Errors when ClusterLogic instances aren’t properly registered. |
| 153 | + - Flow through to ClusterLogic for all reads/writes/command. |
| 154 | +- Can unit test this class by generating the TLV / path for input, parsing the |
| 155 | + TLV output. |
| 156 | +
|
| 157 | +# Unit testing existing clusters |
| 158 | +
|
| 159 | +- Important for clusters where there are multiple configurations that cannot |
| 160 | + easily be represented with example apps |
| 161 | +- Option 1 |
| 162 | + - Refactor the cluster logic to be unit-testable. |
| 163 | +- Option 2 |
| 164 | + - Test at AttributeAccessInterface boundary. |
| 165 | + - Instantiate or access the cluster server instance in the test. |
| 166 | + - Read / Write TLV and use TLV encode/decode functions to verify |
| 167 | + correctness. |
| 168 | + - See TestPowerSourceCluster for an example of how to do this |
| 169 | +- Additional test coverage on clusters, especially for hard to trigger |
| 170 | + conditions, is important. However, **don’t let perfection be the enemy of |
| 171 | + progress** . |
0 commit comments