For CFFI, remove variadic arguments #31159
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pullapprove
bot
requested review from
andy31415,
andyg-apple,
anush-apple,
arkq,
bzbarsky-apple,
carol-apple,
cecille,
chrisdecenzo,
chshu,
chulspro,
cliffamzn,
Damian-Nordic,
dhrishi,
harsha-rajendran,
hawk248,
hicklin,
jepenven-silabs,
jmartinez-silabs,
jmeg-sfy,
joonhaengHeo,
jtung-apple,
kkasperczyk-no,
kpschoedel,
ksperling-apple and
lazarkov
pullapprove
bot
requested review from
vivien-apple,
wiba-nordic,
woody-apple,
younghak-hwang,
yufengwangca and
yunhanw-google
|
PR #31159: Size comparison from 3e8aeeb to a1a02a9 Decreases (1 build for efr32)
Full report (72 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
tcarmelveilleux
approved these changes
Jan 9, 2024
tehampson
commented
Jan 9, 2024
53 tasks
|
PR #31159: Size comparison from eb7a27c to 2f4cd0f Decreases (1 build for efr32)
Full report (72 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
andy31415
reviewed
Jan 12, 2024
andy31415
reviewed
Jan 12, 2024
andy31415
approved these changes
Jan 12, 2024
jamesharrow
pushed a commit
to jamesharrow/connectedhomeip
that referenced
this pull request
Jan 15, 2024
mergify bot
pushed a commit
that referenced
this pull request
Jan 18, 2024
… removed 2 attributes (#31406) * Fix #30665 (EVSE) - Changed to use amperage_mA, energy_mWh - removed max on epoch_s - removed access for operate - removed side for events * Fix #30665 updates to try to get further with ZAP and autogen, but still fails with some parts of regen_all * Added ember-compatibility-functions.cpp which was missing. * Made all types all lowercase to resolve regen_all issues. * Fixed lint issue (trailing whitespace). * Added Device Energy Management server.cpp, added to all-clusters-app.zap and regen_all * Restyled by whitespace * Restyled by clang-format * Fixes based on similar conversations on EVSE review. Made Forecast and PowerAdjustmentCapability Nullable attributes. Added feature support. * Added delegate class to all-clusters-app * Changed ChipLogProgress to Error. Better handling of EnumerateCommands * Aligned EVSE XML to same state as PR#30857 (includes SessionID being Nullable etc and in Fault Event). * Updated Device Energy Management XML to use power_mw, energy_mwh per spec definition. * Updated controller-clusters.zap * regen_all.py * Restyled by whitespace * Regen_all after merging in changes for XML * Fixed types to be signed=true * Fixed 31032 - revert removal of side="server". * regen_all.py * Added Device Energy Management cluster back into all-clusters.zap and regen_all after merging from master. * Compiles but doesn't work. Needs stub to instantiate class * Fixed EnumerateCommands to allow optional ModifyForecastRequest and RequestConstraintBasedForecast commands * Fixed InvokeCommand issues to allow optional commands. * Improved HandleModifyForecastRequest and HandleRequestConstraintBasedForecast in sdk * Updates to add DeviceEnergyManagement to all-clusters. * Compiles and links * Sync file in energy-management-app * Updated DEM to support SetPowerAdjustmentCapability and SetForecast methods. * Updated based on review comments. Changed Epoch to be Matter 2000 based epochs. * Turned on commands in DEM. Added DEM cluster server to energy-management-app. * Added more comments to Delegate to describe expected behaviour. * Updated BUILD.gn to remove duplicated files from all-clusters-common and energy-management-common * Aim to fix compile issues on other platforms due to logging of %d * Restyled by gn * Updated build files to remove duplicate copies from all-clusters-common to energy-management-common * Fixed ESP32 include path * Added DEM into energy-management-app - refactored main.cpp. * Added DEM into EVSEManufacturerImpl.h * Fix - avoid using global namespace in header file * Restyled by gn * Updated CMakeLists.txt to remove duplicate energy-management files. * Added device-energy-management-server to ESP32 all-clusters CMakeLists.txt * Removed return at end of void function. * Added include to all-clusters-minimal in ESP32 * Removed __FUNCTION__ from logs * Removed stray %s * Added FeatureMap handling in sdk (not in ember) * Removed extra chip:: * Used CHIP_ERROR_FORMAT, err.Format() mechanism * Refactored StartTimeAdjust based on review comments. * Removed unnecessary Write Attributes function * Beginnings of Session handling * Added beginnings of EVConnected,EVNotDetected,EnergyTransferStarted,EnergyTransferStopped handling. State machine is not finished. Callback to read Energy Meter added * Added framework for EVSE Test Event triggers * Added EnergyEvseTestEventTrigger delegates * Restyled by whitespace * Restyled by gn * Added :energy-evse-test-event-trigger to public_deps to see if it resolves build errors * Restyled by gn * Fixed Darwin compile error - do not use else after return * Refactored code so that the EvseManufacturer instance could be retrieved for Test Event triggers * Started adding TC_EEVSE_2_2.py * Updated TC_EEVSE_2_2.py to support test events. Still needs to handle reading of Logged Events and verifying they are correct. * Refactored Handling of TestEvents to allow clear, and better error handling. * Refactored state handling by decomposing into state machine events where similar functions are performed based on state transition. Fixed TC chargingEnabledUntil cast to int. Note gets to step 6e * Fixed step 6e caused by not setting the cable limit / maxHardwareCurrentLimit in test events * Added comment to clarify purpose and definition of test eventtrigger field values. * Fixed several bugs in test script * Made SetChargingEnabledUntil take a nullable type. * Removed Reference to step 5c, and moved reading of SessionID to step 4b. More TC_EEVSE_2_2 bug fixes. Added event checking. Still fails at step 14. Does not have enable timeout timer implemented * Fixed issue with not detecting 2nd plug in event, and session ID not incrementing. Now test case passes all the way. * Restyled by isort * Made some attributes persisted per spec. * Added attributes to zcl.json to mark them as implemented in attributeAccessInterfaceAttributes * Ran regen_all.py after changing zcl.json * Fixed incorrect type - not picked up by all compilers. * Re-ran bootstrap, and then regen-all - to pick up more zap generated fixes * Added provisional handling for Faults * Added new test event triggers to help test Fault and Diagnostics * Added TC_EEVSE_2_4 * Fix lint issue - unused datetime modules. * Committed suggested change to comment * Added TC_EEVSE_2_5.py to support DiagnosticsCommand testing. Also changed the SupplyState reverting to Disabled once diagnostics is complete to match the spec. * Created a helper EEVSE base class to avoid repetition in the different test cases. * Restyled by isort * Fixed Lint issues * Revamped TC_EEVSE_2_5 to match spec behaviour (cannot start diagnostics unless Disabled). Also removed hard-coded endpoint ids in Utils * Implemented timer to disable the EVSE automatically. * Added documentation to cover concern about long-lived bytespan in enableKey * Fixed Lint and build issues on other platforms * Restyled by isort * Implemented some of the feedback on PR * Refactored HwSetState to use nested switch statements to be clear that all enums are caught. * Fixed error messages * Test scripts: Removed hardcoded endpoint 1 (use --endpoint 1 in args), allowed the enableKey to be passed in using --hex-arg enableKey:000102030405060708090a0b0c0d0e0f * Made enum class for callbacks and improved documentation comments based on feedback. * Fixed another python lint issue. * Updated README.md with help on how to build for test event triggers, using chip-repl and python testing. * Tweaks to README.md to avoid Myst syntax highlighting issues. * Improved error logging around GetEpochTS() * Made main use std::unique_ptr instead of using new/delete per PR comments. Also moved GetEVSEManufacturer declaration to header file. * Fixing MISSPELL issues in README.md * Small change missed in main.cpp missed in unique_ptr change. * Changed all-clusters app stubs to use unique_ptr style instead of new/delete. * Removed unhelpful comment * Restyled by whitespace * Fixes #31061 Updated DEVICE_TYPE to 0x050C now this has been allocated * Fixes #31061 Updated DEVICE_TYPE to 0x050C now this has been allocated * Updated energy-evse-cluster.xml to use latest EnergyEVSE.adoc which removes numberOfWeeklyTargets, numberOfDailyTargets, and adds new ChargingTargetScheduleStruct in Get/Set/Clear Target commands * Removed min/max for Randomisation window due to ZAP bug that doesn't allow more than 2 bytes. * Added missing new Java files * Changed DayOfWeekforSequence -> DayOfWeekForSequence in XML. Regen-all * Python testing: Add helper functions for marking steps as skipped in the TH (#31373) * Add two new helper functions for marking steps skipped * python testing: Add helper functions for skipped steps * Do not use gen_config.h directly. (#31401) * Do not use gen_config.h directly. Apparently the right file is config.h, which includes the gen file. * Restyled by clang-format --------- Co-authored-by: Restyled.io <[email protected]> * Bump third_party/ot-br-posix/repo from `657e775` to `58822dc` (#31420) Bumps [third_party/ot-br-posix/repo](https://github.com/openthread/ot-br-posix) from `657e775` to `58822dc`. - [Release notes](https://github.com/openthread/ot-br-posix/releases) - [Commits](openthread/ot-br-posix@657e775...58822dc) --- updated-dependencies: - dependency-name: third_party/ot-br-posix/repo dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump third_party/imgui/repo from `240ab58` to `6228c2e` (#31418) Bumps [third_party/imgui/repo](https://github.com/ocornut/imgui) from `240ab58` to `6228c2e`. - [Release notes](https://github.com/ocornut/imgui/releases) - [Commits](ocornut/imgui@240ab58...6228c2e) --- updated-dependencies: - dependency-name: third_party/imgui/repo dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump third_party/pigweed/repo from `cbbc73d` to `9640cde` (#31414) Bumps [third_party/pigweed/repo](https://github.com/google/pigweed) from `cbbc73d` to `9640cde`. - [Commits](google/pigweed@cbbc73d...9640cde) --- updated-dependencies: - dependency-name: third_party/pigweed/repo dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * [Chef] Fix variable may be used without initialization (#31413) * Fix variable may be used without initialization * Restyled by clang-format --------- Co-authored-by: Restyled.io <[email protected]> * Bump third_party/nanopb/repo from `cf26d28` to `423c03b` (#31421) Bumps [third_party/nanopb/repo](https://github.com/nanopb/nanopb) from `cf26d28` to `423c03b`. - [Commits](nanopb/nanopb@cf26d28...423c03b) --- updated-dependencies: - dependency-name: third_party/nanopb/repo dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Test_TC_DGWIFI_2_1 WiFiVersion enum8 maxValue should be 6 (#31364) * WiFiVersion enum8 maxValue should be 6 Matter Spec 1.2 Section 11.14.5.2. WiFiVersionEnum Type ranges from 0 (802.11a) to 6 (802.11ah) * Update command.h darwin zap-genersated WiFiVersion * Handle energy type naming in data model xml handlers (#31344) * Correct case names for enerty types in data model handlers. * Add unit test * restyle * [nrfconnect] Switch unit tests to PSA crypto (#31408) * [nrfconnect] Switch unit tests to PSA crypto Switch nRF Connect unit tests to PSA crypto backend since legacy mbedTLS is already tested on many platforms. Additionally, clean the configuration of nRF Connect test runner by removing some redundant configurations and better grouping the items. * Restyled by gn --------- Co-authored-by: Restyled.io <[email protected]> * Bump third_party/libwebsockets/repo from `f18fc23` to `49bfef2` (#31417) Bumps [third_party/libwebsockets/repo](https://github.com/warmcat/libwebsockets) from `f18fc23` to `49bfef2`. - [Commits](warmcat/libwebsockets@f18fc23...49bfef2) --- updated-dependencies: - dependency-name: third_party/libwebsockets/repo dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Add initial valve cluster implementation (#30562) * added first version of valve configuration and control cluster * added first version of boolean sensor configuration cluster XML and updated event descriptions for valve config and control cluster * fixed wrong attribute name Co-authored-by: Boris Zbarsky <[email protected]> * fixed typo Co-authored-by: Boris Zbarsky <[email protected]> * removed StartUpLevel attribute and added optional OpenDuration command args * use allocated cluster ID * added code generating bits * added new device types * restyle * added BooleanSensorConfiguration and ValveConfigurationAndControl clusters to all-clusters-app * - fixed wrong attribute name caused by copy paste - min/max is not allowed for attributes with size larger than 2 bytes * regenerate all * added initial implementation for the new clusters * apply provisional property to new clusters * fixed typo Co-authored-by: Boris Zbarsky <[email protected]> * removed empty implementation, removed clusters from all-clusters-app * added a skeleton implementation * fixed typo Co-authored-by: Boris Zbarsky <[email protected]> * fixed typos and regen all * fixes based on PR comments * added descriptor cluster to valve device type * updated zap and idl files to fixed typo in command name * close to final implementation, needs careful going through * activate alarms if enabled, added API for sensor fault event, clean-up * WIP * Implemented open command * finalize initial implementation * added all-clusters-app example for valve device * added null checking * added attribute changed callback and fixed some bugs after testing * did a regen and fixed rebase issue * restyle * added missed generated code by previous commits * attempt to fix failing CI * restyle * add a missed zap gen output * removed redundant return * update valve configuration and control cluster according to latest spec * remove boolean sensor config cluster implementation and into a separate PR * DefaultOpenDuration is writable and updated device type name to Water Valve * updated code to latest spec * changed RemainingDuration to be handled by AttributeAccessInterface * WIP RemainingDuration * moved domain to be the first element * update to the latest spec changes, removed attribute changed callback, handle remainingduration in AAI * added LevelStep attribute in all-clusters-app * removed unused code * set default null values in ZAP * Added transition handling in Delegate * Reworked AutoTimeClose handling in SetValveLevel * Changed include * added mechanism to signal UTCTime change in timesync cluster --------- Co-authored-by: Boris Zbarsky <[email protected]> Co-authored-by: fessehat <[email protected]> Co-authored-by: René Josefsen <[email protected]> Co-authored-by: René Josefsen <[email protected]> * Bump third_party/mbedtls/repo from `ffb18d2` to `56fd26c` (#31422) Bumps [third_party/mbedtls/repo](https://github.com/ARMmbed/mbedtls) from `ffb18d2` to `56fd26c`. - [Release notes](https://github.com/ARMmbed/mbedtls/releases) - [Commits](Mbed-TLS/mbedtls@ffb18d2...56fd26c) --- updated-dependencies: - dependency-name: third_party/mbedtls/repo dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * [K32W0] SDK 2.6.13 additional changes (#31382) * [K32W] Fix a corner case when switching the TLV processors After the current block is processed, if the block size is 0, it means that the processed data was a multiple of received BDX block size (e.g. 8 blocks of 1024 bytes were transferred). After state for selecting next processor is reset, a request for fetching next data must be sent. Signed-off-by: marius-alex-tache <[email protected]> * [K32W] Make Validate public Change access modifier of Validate to public, to be used by K32W0 for factory data validation after factory data OTA. Signed-off-by: marius-alex-tache <[email protected]> * [K32W0] Add CRC validation during factory data OTA After factory data is copied into internal flash, call Validate to ensure the CRC of factory data matches the CRC in the header. If the check fails, then OTA is aborted and factory data is restored to previous version. Signed-off-by: marius-alex-tache <[email protected]> * [K32W0] Add additional check for provider pointer Signed-off-by: marius-alex-tache <[email protected]> * [K32W0] Change root node revision to 2 Signed-off-by: marius-alex-tache <[email protected]> * [K32W] Expose SearchForId in factory data provider public API Removed additional checks on tags with the assumption that factory data is strictly checked at manufacturing time. Applications can now use the public API to search the factory data section for an id, e.g. in the case of custom factory data provider, where the app has additional factory data information besides the default one. Signed-off-by: marius-alex-tache <[email protected]> * [K32W0] Refactor custom factory data provider The example now leverages the actual factory data provider API to search the factory data section for some custom ids. Signed-off-by: marius-alex-tache <[email protected]> * [K32W] Add platform support for Product Appearance Implement GetProductFinish and GetProductPrimaryColor defined in DeviceInstanceInfoProvider. Signed-off-by: marius-alex-tache <[email protected]> * [K32W] Remove maxLengths array This array was used to store the maximum length of factory data fields. It's no longer used, with the assumption that the factory data is strictly checked in manufacturing. Signed-off-by: marius-alex-tache <[email protected]> * [K32W0] Remove usage of maxLength array Signed-off-by: marius-alex-tache <[email protected]> * [K32W0] remove unused code in LowPowerHook.cpp file * [K32W0] use Encoding::HexToBytes to parse the ota encryption key * [K32W0] remove extra PWR_DisallowDeviceToSleep() which will cause unsleep after commissioning * [K32W] Revert removal of disallow to sleep and enclose it in specific tag Only call disallow to sleep when the platform is K32W1. Signed-off-by: marius-alex-tache <[email protected]> * [K32W0] Update reference app readme files This is a minor improvement to the building instructions. It aims to clarify the usage of west SDK or package SDK. Signed-off-by: marius-alex-tache <[email protected]> * [K32W0] Fix ICD parameters Name of the ICD parameters were updated according with the latest stack updates. Signed-off-by: Andrei Menzopol <[email protected]> * Restyled by clang-format * Restyled by gn * Restyled by prettier-markdown * [K32W0] Fix gn check error Signed-off-by: marius-alex-tache <[email protected]> * Restyled by gn * [K32W1] Fix gn check errors Signed-off-by: marius-alex-tache <[email protected]> * Restyled by gn * [K32W1] Fix another gn error Signed-off-by: marius-alex-tache <[email protected]> * Restyled by gn * [K32W] Send a report before resetting the device during OTA State-transition event from Downloading to Applying was not successfully sent to a subscriber during OTA because the device would reset before actually sending the ReportData message. Added an explicit call to handle server shutting down, which will sync send all events. Signed-off-by: marius-alex-tache <[email protected]> * Restyled by clang-format * [K32W0] Remove deprecated dependency Signed-off-by: marius-alex-tache <[email protected]> * [K32W1] Remove deprecated dependency Signed-off-by: marius-alex-tache <[email protected]> * Restyled by gn --------- Signed-off-by: marius-alex-tache <[email protected]> Signed-off-by: Andrei Menzopol <[email protected]> Co-authored-by: Damien Vagner <[email protected]> Co-authored-by: tanyue518 <[email protected]> Co-authored-by: Ethan Tan <[email protected]> Co-authored-by: Andrei Menzopol <[email protected]> Co-authored-by: Restyled.io <[email protected]> * For CFFI in chip-repl, remove variadic arguments (#31159) * Updated energy-evse-cluster.xml to use latest EnergyEVSE.adoc which removes numberOfWeeklyTargets, numberOfDailyTargets, and adds new ChargingTargetScheduleStruct in Get/Set/Clear Target commands * Removed optional=true in xml for DayOfWeekForSequence & ChargingTargets in ChargingTargetScheduleStruct * Small correction to description in test case. * Updated energy-evse-cluster.xml to use latest EnergyEVSE.adoc which removes numberOfWeeklyTargets, numberOfDailyTargets, and adds new ChargingTargetScheduleStruct in Get/Set/Clear Target commands * Removed min/max for Randomisation window due to ZAP bug that doesn't allow more than 2 bytes. * Added missing new Java files * Changed DayOfWeekforSequence -> DayOfWeekForSequence in XML. Regen-all * Updated energy-evse-cluster.xml to use latest EnergyEVSE.adoc which removes numberOfWeeklyTargets, numberOfDailyTargets, and adds new ChargingTargetScheduleStruct in Get/Set/Clear Target commands * Removed optional=true in xml for DayOfWeekForSequence & ChargingTargets in ChargingTargetScheduleStruct * Update examples/energy-management-app/energy-management-common/include/EnergyEvseDelegateImpl.h Co-authored-by: Boris Zbarsky <[email protected]> * Touched file to retrigger restyled job * Removed whitespace which was added to trigger restyled to rerun * Removed potentially unsafe code before merging into PR #30957 * Renamed variable and replaced auto with type so it is clearer to reader. * Restyled by clang-format --------- Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: marius-alex-tache <[email protected]> Signed-off-by: Andrei Menzopol <[email protected]> Co-authored-by: Restyled.io <[email protected]> Co-authored-by: C Freeman <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Erwin Pan <[email protected]> Co-authored-by: simonhmorris1 <[email protected]> Co-authored-by: Andrei Litvin <[email protected]> Co-authored-by: Damian Królik <[email protected]> Co-authored-by: fesseha-eve <[email protected]> Co-authored-by: Boris Zbarsky <[email protected]> Co-authored-by: fessehat <[email protected]> Co-authored-by: René Josefsen <[email protected]> Co-authored-by: René Josefsen <[email protected]> Co-authored-by: Marius Tache <[email protected]> Co-authored-by: Damien Vagner <[email protected]> Co-authored-by: tanyue518 <[email protected]> Co-authored-by: Ethan Tan <[email protected]> Co-authored-by: Andrei Menzopol <[email protected]> Co-authored-by: Terence Hampson <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes: #30986
Removing variadic arguments will make creating
pychip_CommandSender_TestOnlySendBatchCommandsthat will share common implementation withpychip_CommandSender_SendBatchCommandseasier to implement and maintain.Additionally this seems like a more clean way to pass these arguments to c++
Arguable we could follow precedent with passing structures where python side uses
constructand we have a packed structure on the c++ side. But I recently discovered that we can share a common struct over CFFI usingctypes.Structure. This seems easier and is a more sane way to build and pass the structure through.Eventually we should go back and fix all structures to use this, but for now I want this PR to focus on removal of variadic arguments when going from Python to C