Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle typedef in zapt templates & matter_idl #36124

Open
wants to merge 256 commits into
base: master
Choose a base branch
from

Conversation

gmarcosb
Copy link
Contributor

@gmarcosb gmarcosb commented Oct 17, 2024

Dependent on zap updates at project-chip/zap#1458

Update zap templaets to processtypedef in zap, supporting things like defining a VideoStreamID as a uint16, see the camera spec: https://github.com/CHIP-Specifications/connectedhomeip-spec/pull/10004

This should also eliminate the need for many of the things in chip-types.xml, which can now be simply defined in the XML files as a simple typedef

See also discussion with @bzbarsky-apple: #35773 (comment)

@gmarcosb gmarcosb requested review from a team as code owners October 17, 2024 15:32
Copy link

semanticdiff-com bot commented Oct 17, 2024

Review changes with  SemanticDiff

@@ -24,10 +24,6 @@
"name": "cluster_header",
"path": "../../../app/zap-templates/partials/cluster_header.zapt"
},
{
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is somewhat unrelated & really more just cleanup: this file no longer exists

Copy link

PR #36124: Size comparison from dead378 to 61d1605

Full report (1 build for stm32)
platform target config section dead378 61d1605 change % change
stm32 light STM32WB5MM-DK FLASH 481896 481896 0 0.0
RAM 144844 144844 0 0.0

yufengwangca and others added 5 commits October 17, 2024 19:34
* [Fabric-Admin] Implement FabricSyncGetter APIs

* Update examples/fabric-admin/device_manager/FabricSyncGetter.h

Co-authored-by: Terence Hampson <[email protected]>

---------

Co-authored-by: Terence Hampson <[email protected]>
* Adding accessory for publicKeyData

* Update src/darwin/Framework/CHIP/MTRCertificateInfo.mm

* Restyled by whitespace

* Restyled by clang-format

* braces for multi-line conditional statement

---------

Co-authored-by: Restyled.io <[email protected]>
Co-authored-by: Kiel Oleson <[email protected]>
* Fix up group iterator releasing

* Add missing bracket

* Renames and comments

* Restyled by clang-format

* Update src/app/WriteHandler.cpp

Co-authored-by: Arkadiusz Bokowy <[email protected]>

---------

Co-authored-by: Restyled.io <[email protected]>
Co-authored-by: Arkadiusz Bokowy <[email protected]>
@@ -56,6 +56,10 @@
"name": "enum_decl",
"path": "partials/enum_decl.zapt"
},
{
"name": "typedef_decl",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But where is this partial used?

Copy link
Contributor

@bzbarsky-apple bzbarsky-apple left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs some test coverage to exercise these bits: please add something with typedefs to the UnitTesting cluster.

namespace Clusters {
{{#zcl_typedefs}}
{{#if has_more_than_one_cluster}}
{{> cluster_typedefs_ensure_known_value ns="detail"}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is cluster_typedefs_ensure_known_value defined? What is it supposed to do?


{{#if has_more_than_one_cluster}}

{{> cluster_typedefs_enum ns=""}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is cluster_typedefs_enum defined?

yufengwangca and others added 6 commits October 17, 2024 22:34
…ip#36057)

* Implement CommissionerControl command APIs

* Address review comment

* Add missing change

* Init mCommissionerControl in SetRemoteBridgeNodeId

* Remove redundant optimization

* Fix merge conflict

* Call OnDone on failure

* Fix build error after merge
…moveAllEntries (project-chip#36053)

* [ICD] Add log for storeEntry/removeEntry/removeAllEntires

* Restyled by clang-format

---------

Co-authored-by: Restyled.io <[email protected]>
… self (project-chip#36131)

* [Darwin] Some MTRDevice mtr_strongify sites need to guard against nil self

* Took ksperling-apple's suggestion to use VerifyOrReturn instead for cleaner code
* Remove various un-necessary code from MTRDevice.

The _deviceInternalStateChanged declaration was duplicated in
MTRDevice_Concrete and only needed there.

MTRDeviceClusterData implementation can move into its own file.

setPersistedClusterData: is only used on MTRDevice_Concrete instances, so its
declaration can move to MTRDevice_Concrete and the unused implementation in
MTRDevice can be removed.

setPersistedDeviceData: is only used on MTRDevice_Concrete instances, so its
declaration can move to MTRDevice_Concrete and the unused implementation in
MTRDevice can be removed.

Now _setLastInitialSubscribeLatency and _updateAttributeDependentDescriptionData
on MTRDevice are unused and can be removed.

At this point, _informationalVendorID, _informationalProductID,
_networkFeatures, _vid, _pid, _allNetworkFeatures, and _descriptionLock are all
unused on MTRDevice and can be removed.

Now _informationalNumberAtAttributePath is unused on MTRDevice and can be
removed.

_endpointList is also unused on MTRDevice and can be removed.

deviceCachePrimed is implemented by both MTRDevice_XPC and MTRDevice_Concrete,
so the implementation on MTRDevice can be removed.

unitTestGetClusterDataForPath, unitTestGetPersistedClusters,
unitTestClusterHasBeenPersisted, unitTestResetSubscription,
unitTestAttributesReportedSinceLastCheck, unitTestClearClusterData,
unitTestInjectEventReport, unitTestInjectAttributeReport, and
unitTestAttributeCount are only used on MTRDevice_Concrete, so the duplicate
implementations on MTRDevice can be removed.

At this point, _cachedAttributeValueForPath is unused on MTRDevice and can be
removed.

Now _getCachedDataVersions is unused on MTRDevice and can be removed.

deviceUsesThread is only used by MTRDeviceController_Concrete on
MTRDevice_Concrete instances and so can move to MTRDevice_Concrete entirely.
Then _deviceUsesThread becomes unused on MTRDevice and can be removed.

Then _clusterDataForPath is unused on MTRDevice and can be removed.

Now _reconcilePersistedClustersWithStorage is unused on MTRDevice and can be
removed.

Also, _knownClusters is unused on MTRDevice and can be removed.

setStorageBehaviorConfiguration is only used from MTRDeviceController_Concrete
(which has an MTRDevice_Concrete) and from unit tests (which have an
MTRDevice_Concrete, though they don't know it), so it can be removed from
MTRDevice.  Then _resetStorageBehaviorState also becomes unused and can be
removed.

At this point, _scheduleClusterDataPersistence is not used on MTRDevice and can
be removed.  This makes _persistClusterDataAsNeeded and _deviceCachePrimed
unused, and they can also be removed.

At this point, _persistClusterData, _deviceIsReportingExcessively,
_reportToPersistenceDelayTimeAfterMutiplier,
_reportToPersistenceDelayTimeMaxAfterMutiplier, _mostRecentReportTimes, and
_clusterDataPersistenceFirstScheduledTime are not used on MTRDevice and can be
removed.

Now _dataStoreExists, _clusterDataToPersistSnapshot,
_storageBehaviorConfiguration, _reportToPersistenceDelayCurrentMultiplier,
_deviceReportingExcessivelyStartTime, and _persistedClusters are unused on
MTRDevice and can be removed.

At this point _clusterDataToPersist and _persistedClusterData are unused on
MTRDevice and can be removed.

_systemTimeChangeObserverToken was never actually used on MTRDevice (it's only
used on MTRDevice_Concrete) and can be removed.

The MTRDeviceAttributeReportHandler typedef is unused on MTRDevice and can be
removed.

kSecondsToWaitBeforeMarkingUnreachableAfterSettingUpSubscription and
ENABLE_CONNECTIVITY_MONITORING are unused on MTRDevice and can be removed.

assertChipStackLockedByCurrentThread is not used in MTRDevice, so the
LockTracker.h include can be removed.

MTRAsyncWorkQueue, MTRAttributeIsSpecified, MTRCommandNeedsTimedInvoke,
MTRDeviceConnectivityMonitor, MTRDeviceControllerOverXPC, MTRDecodeEventPayload,
are all unused in MTRDevice, so the imports of the corresponding headers can be
removed.

arrayOfNumbersFromAttributeValue is now unused on MTRDevice and can be removed.

The fabricIndex property is write-only on both MTRDevice and MTRDevice_Concrete
and can be removed on both.

_dataValueWithoutDataVersion is unused on MTRDevice and can be removed.

* Move MTRDeviceClusterData into a separate header/implementation file.
# or integration with wifi/ethernet/bluetootn/etc.)
#
# linux:
# - "/(linux)/i"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is part of a different PR already merged into master.

Something is wrong with the merges in this PR. Could you merge with master so that deltas are smaller and directly relate to the PR description?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.