Commit 287747a
[PowerTopology] Add ElectricalCircuit (CIRC) feature and ElectricalCircuitNodes attribute (project-chip#73402)
* [PowerTopology] Add ElectricalCircuit (CIRC) feature + ElectricalCircuitNodes
Add the Matter 1.7 ElectricalCircuit (CIRC) feature to the code-driven
power-topology-server, exposing the ElectricalCircuitNodes attribute
(0x0002): a fabric-scoped, writable, non-volatile List<CircuitNodeStruct>.
- Read is fabric-filtered automatically (fabric-scoped item type).
- Write is fabric-scoped: the accessing fabric is authoritative, ReplaceAll
replaces only that fabric's slice with full staging/validation before any
mutation, AppendItem appends one node.
- Non-volatile persistence via an explicit TLV blob (retaining fabricIndex,
which the generated fabric-scoped write codec omits), restored on Startup.
- Fabric removal purges the removed fabric's nodes via FabricTable::Delegate;
the FabricTable is an optional nullable field on the cluster Config.
Enable CIRC on the EVSE example app and thread the FabricTable through
PtConfig so fabric-removal cleanup is active. The added FabricTable
parameters are defaulted, so the water-heater app (which shares
ElectricalSensorManager) is unaffected.
Extend TC_PWRTL_2_1 to assert ElectricalCircuitNodes present iff CIRC and to
read, write (round-trip), and verify Non-volatile persistence of the
attribute.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* [PowerTopology] Scope TC_PWRTL_2_1 CIRC change to attribute presence
Keep only the AttributeList presence-iff-CIRC assertion for
ElectricalCircuitNodes in TC_PWRTL_2_1 (the attributes test). Functional
read/write coverage of the attribute belongs to the dedicated TC_PWRTL_2_2,
which this PR's CIRC-enabled DUT unblocks; avoid duplicating it here.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* [PowerTopology] Heap-allocate ElectricalCircuitNodes write staging buffer
WriteElectricalCircuitNodes staged the incoming list in a stack array of
kMaxCircuitNodes StoredCircuitNode entries (~8 KB), which can overflow the
constrained Matter thread stack on embedded platforms. Allocate the staging
buffer on the heap via ScopedMemoryBuffer, sized to the incoming count.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* [PowerTopology] Heap-allocate ElectricalCircuitNodes node storage
The ElectricalCircuitNodes storage was an inline StoredCircuitNode array of
kMaxCircuitNodes entries (~8 KB: char label[128] x 50). As a member this
made every stack-allocated PowerTopologyCluster instance ~8 KB, exceeding
the -Wstack-usage=8192 limit on 32-bit embedded builds (Zephyr/ESP32) in
the unit tests, and inflating the cluster's footprint on constrained
platforms generally.
Store the nodes in a ScopedMemoryBuffer allocated on Startup, only when the
CIRC feature is enabled.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* [PowerTopology] Move ElectricalCircuitNodes storage behind an interface
Review feedback: the cluster should not hard-code how ElectricalCircuitNodes is
stored, and holding the entries in a heap buffer rules out platforms with
stricter memory requirements.
Following thread-network-directory-server, which solves the same problem for a
writable persisted list:
- CircuitNodeStorage is an abstract interface: Capacity(), Count(),
CountForFabric(), GetNodeAtIndex(), ReplaceNodesForFabric() for the ReplaceAll
write, AppendNode() for AppendItem, and RemoveNodesForFabric() for fabric
purge.
- DefaultCircuitNodeStorage implements it against the cluster's attribute
storage, carrying over the previous TLV format unchanged, so existing
behaviour (durable nodes, same location) is preserved with no application
work.
- The cluster holds only a CircuitNodeStorage pointer and allocates nothing for
this attribute. StoredCircuitNode, the ScopedMemoryBuffer member, and
SaveCircuitNodes()/LoadCircuitNodes() are gone.
A platform without a heap now supplies a fixed-array implementation and reports
its real limit through Capacity(). The unit tests do exactly that, so the
allocation-free path is exercised rather than merely possible.
CircuitNodeStorage::Init() is a no-op by default and exists so an implementation
that persists through the cluster's attribute storage can obtain it at cluster
Startup, which is the earliest point it exists. Implementations with their own
persistence ignore it.
PowerTopology::Instance gains an overload taking application-provided storage;
the existing constructor keeps using DefaultCircuitNodeStorage, so applications
need no change.
Load() stops at Capacity() rather than erroring, so reducing a platform's
capacity cannot prevent startup on previously persisted data.
Testing: unit tests pass (22 cases, up from 21). The added
DefaultCircuitNodeStoragePersistsAcrossInstances covers the TLV round trip that
previously ran through the cluster, since the cluster-level tests now use the
non-persisting fixed-array storage. all-clusters-app builds and links.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* [PowerTopology] Shrink the fixed-array test storage to fit the Zephyr stack budget
The FixedCircuitNodeStorage test double held Node[50], roughly 8 KB, and the
tests place it on the stack. That tripped -Werror=stack-usage=8192 on the Zephyr
native_posix unit-test build (worst case 12112 bytes), breaking the nRF Connect
SDK and ESP32_QEMU checks.
Make the capacity a template parameter defaulting to 8 (about 1.3 KB), which
brings the worst case to roughly 5.4 KB. No test stores more than four entries,
and the resource-exhaustion test writes far past any capacity so it still
exercises the limit.
Fittingly, the double was failing the very constraint it exists to demonstrate.
Testing: unit tests pass (22 cases).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* [PowerTopology] Require the fabric table for CIRC; bound the ReplaceAll staging write
Two review findings on project-chip#73403, which carries this file until project-chip#73402 merges.
1. The ElectricalCircuit feature needs the fabric table, not "uses it if
present". ElectricalCircuitNodes is fabric-scoped, so a removed fabric's
entries must be purged, and that only happens through the OnFabricRemoved
callback registered via AddFabricDelegate. Startup() previously skipped the
registration silently when Config::fabricTable was null, and PowerTopology
Instance defaults that parameter to nullptr, so an application could enable
CIRC, compile cleanly, and leak a removed fabric's nodes with no diagnostic.
Startup() now fails with CHIP_ERROR_INCORRECT_STATE and a log line, matching
the Config::circuitNodeStorage check three lines above it. Returning an error
is preferred over VerifyOrDie: Startup() already reports this class of
misconfiguration through its return value, and a crash would be a harsher
contract than the surrounding code uses. Shutdown() keeps its null check,
since it can run on a cluster that never started or does not have CIRC.
The unit tests construct CIRC clusters, so they now supply a fabric table via
FabricTestFixture, as ElectricalCircuitNodesPurgedOnFabricRemovalTest already
did. Added ElectricalCircuitWithoutFabricTableTest to pin the new contract.
2. WriteElectricalCircuitNodes staged the decoded ReplaceAll list into a buffer
sized from list.ComputeSize() and indexed it with a counter driven by the
iterator, without checking the two agree. Added a bounds check. Beyond
decoder inconsistency this also covers newCount == 0, where the staging
buffer is never allocated at all yet staging[0] would be written if the
iterator yielded an element.
Testing: TestPowerTopologyCluster 23/23 pass (was 22, plus the new one) and
TestPowerTopologyClusterBackwardsCompatibility 1/1 passes, built natively with
chip_build_tests=true. Confirmed the new guard is actually exercised rather than
merely compiled: its error log appears exactly once across the suite, in the new
test. clang-format clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* [PowerTopology] Discard a partially decoded ElectricalCircuitNodes value
Load() appends entries as it decodes them, so a failure part way through a
stored blob left the decoded prefix in place. Init() logs that error and
returns CHIP_NO_ERROR, since a bad stored value must not brick startup, so the
cluster went on to serve an arbitrary fraction of a corrupt list as the
attribute value. Reset the count in that path so the list reads as empty
instead.
Also document the persisted blob's worst case size against the per-value
limits some platforms impose, and move the anonymous namespace close below the
final test so every fixture-derived test sits inside it.
* TC-PWRTL-2.1: align the script step list with the test plan
The script and the plan had drifted: the script combined the AttributeList
read and its composition check into one step, wrote to only one of the two
read-only attributes, and split the reboot and the persistence read across
two steps. Step n in the script and step n in the plan stopped describing
the same thing from step 5 onwards.
Both are now 11 steps and match one for one. The reboot step also moves to
request_device_reboot() with an app-ready-pattern directive, matching
TC-PWRTL-2.2, instead of the is_pics_sdk_ci_only and wait_for_user_input
dispatch, so it runs in CI rather than being skipped.
* [TC-PWRTL-2.1] Make the reserved-bits check effective
Step 4 is specified to validate the CIRC bit and reserved bits 5..31, but the
reserved-bits half was inert. PowerTopology.Feature is an IntFlag whose members
cover exactly bits 0..4, so `~KNOWN_BITS_MASK` complements only within the class
mask and evaluates to 0. `reserved_bits` was therefore unconditionally 0 and a
DUT setting any reserved bit passed.
Casting both operands to int restores the full-width complement.
* Drop the step-numbering remark from the TC-PWRTL-2.1 docstring
It describes the file rather than the test, and goes stale the moment the
plan renumbers.
* Drop the unsupported-write step from TC-PWRTL-2.1
Per James Harrow: unsupported-write checks come out, since the IDM and
low-level SDK tests already cover write access. Step 11 becomes step 10; the
Status import goes with the last use.
* Align TC-PWRTL-2.1 with its test plan
Per James Harrow: align the script to the plan, and pre-existing lines are not
exempt. The merged TC-PWRTL-2.1 plan has three steps; the script had eleven.
It is now the plan's three, each carrying the plan's expected outcome as an
expectation. Implementing the plan literally also adds an assertion the script
never had: the plan requires both endpoint lists to hold no more than 20
entries, and nothing checked that.
Removed: FeatureMap O.a conformance, the CIRC and reserved-bit checks, and the
AttributeList composition checks, all covered by TC_IDM_10_2 per cecille, which
is why the plan rework (chip-test-plans#6166) and its script alignment (project-chip#73732)
were closed on 2026-08-28. Also removed the Non-Volatile persistence check,
which the plan does not describe.
* Drop the plan URL from the TC-PWRTL-2.1 docstring
Only 16 of 501 scripts on master embed one, and a hardcoded branch-and-anchor
link goes stale on any renumber.
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>1 parent 18745b7 commit 287747a
14 files changed
Lines changed: 1419 additions & 156 deletions
File tree
- examples
- energy-management/electrical-sensor
- include
- src
- evse-app/evse-common/src
- src
- app/clusters/power-topology-server
- tests
- python_testing
Lines changed: 3 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
59 | 59 | | |
60 | 60 | | |
61 | 61 | | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
62 | 65 | | |
63 | 66 | | |
64 | 67 | | |
| |||
Lines changed: 6 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
20 | 20 | | |
21 | 21 | | |
22 | 22 | | |
| 23 | + | |
23 | 24 | | |
24 | 25 | | |
25 | 26 | | |
| |||
39 | 40 | | |
40 | 41 | | |
41 | 42 | | |
42 | | - | |
43 | | - | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
44 | 46 | | |
45 | 47 | | |
46 | 48 | | |
| |||
70 | 72 | | |
71 | 73 | | |
72 | 74 | | |
73 | | - | |
| 75 | + | |
| 76 | + | |
74 | 77 | | |
75 | 78 | | |
76 | 79 | | |
| |||
Lines changed: 2 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
49 | 49 | | |
50 | 50 | | |
51 | 51 | | |
52 | | - | |
| 52 | + | |
| 53 | + | |
53 | 54 | | |
54 | 55 | | |
55 | 56 | | |
| |||
Lines changed: 3 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
51 | 51 | | |
52 | 52 | | |
53 | 53 | | |
54 | | - | |
| 54 | + | |
| 55 | + | |
55 | 56 | | |
56 | 57 | | |
57 | 58 | | |
| |||
68 | 69 | | |
69 | 70 | | |
70 | 71 | | |
71 | | - | |
| 72 | + | |
72 | 73 | | |
73 | 74 | | |
74 | 75 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
252 | 252 | | |
253 | 253 | | |
254 | 254 | | |
255 | | - | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
256 | 258 | | |
257 | 259 | | |
258 | 260 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
16 | 16 | | |
17 | 17 | | |
18 | 18 | | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
19 | 22 | | |
20 | 23 | | |
21 | 24 | | |
| |||
Lines changed: 28 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
17 | 17 | | |
18 | 18 | | |
19 | 19 | | |
| 20 | + | |
20 | 21 | | |
21 | 22 | | |
22 | 23 | | |
| |||
31 | 32 | | |
32 | 33 | | |
33 | 34 | | |
34 | | - | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
35 | 39 | | |
36 | | - | |
37 | | - | |
38 | | - | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
39 | 45 | | |
40 | 46 | | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
41 | 61 | | |
42 | 62 | | |
43 | 63 | | |
44 | 64 | | |
45 | 65 | | |
46 | 66 | | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
47 | 71 | | |
48 | 72 | | |
49 | 73 | | |
| |||
Lines changed: 249 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
0 commit comments