-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat: Add Microwave Oven Chef device #38628
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
Conversation
PR #38628: Size comparison from c3b6b76 to ad90549 Full report (46 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, psoc6, qpg, stm32, telink, tizen)
|
examples/chef/common/clusters/microwave-oven-mode/chef-microwave-oven-mode.cpp
Outdated
Show resolved
Hide resolved
PR #38628: Size comparison from 621748d to 9003dd9 Full report (86 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
examples/chef/common/clusters/microwave-oven-mode/chef-microwave-oven-mode.cpp
Outdated
Show resolved
Hide resolved
examples/chef/common/clusters/microwave-oven-mode/chef-microwave-oven-mode.cpp
Show resolved
Hide resolved
Can we list out the
|
|
PR #38628: Size comparison from 1e7f673 to 58e32a7 Full report (25 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, qpg, stm32, telink, tizen)
|
PR #38628: Size comparison from 1e7f673 to 6170b63 Full report (82 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces support for a new Microwave Oven Chef device by adding configuration files, updating build systems, and implementing clusters for microwave oven mode and control.
- Added a "Microwave Oven" device type entry in the device types JSON.
- Implemented the MicrowaveOvenMode delegate and its initialization along with the corresponding build file updates.
- Implemented the MicrowaveOvenControl delegate with methods to handle cooking parameters and cook time modifications.
Reviewed Changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
examples/chef/sample_app_util/matter_device_types.json | Added a new device type "Microwave Oven" with identifier 121. |
examples/chef/nrfconnect/CMakeLists.txt | Updated source lists to include microwave oven mode and control cluster implementations. |
examples/chef/linux/BUILD.gn | Added microwave oven mode and control source files to the build. |
examples/chef/esp32/main/CMakeLists.txt | Included new clusters directories for microwave oven mode and control. |
examples/chef/common/stubs.cpp | Added includes and initialization blocks for microwave oven clusters. |
examples/chef/common/clusters/microwave-oven-mode/* | Introduced header and source for the MicrowaveOvenMode cluster delegate. |
examples/chef/common/clusters/microwave-oven-control/* | Introduced header and source for the MicrowaveOvenControl cluster delegate. |
Comments suppressed due to low confidence (1)
examples/chef/common/clusters/microwave-oven-mode/chef-microwave-oven-mode.cpp:93
- [nitpick] Consider renaming the parameter 'NewMode' to 'newMode' for consistency with common C++ naming conventions.
void MicrowaveOvenMode::ChefDelegate::HandleChangeToMode(uint8_t NewMode, ModeBase::Commands::ChangeToModeResponse::Type & response)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces the Microwave Oven Chef device, including configuration files, C++ delegate implementation, and updates to build files and device type definitions. The changes look good overall, with some minor suggestions for documentation and code clarity.
examples/chef/common/clusters/microwave-oven-control/chef-microwave-oven-control.cpp
Outdated
Show resolved
Hide resolved
examples/chef/common/clusters/microwave-oven-control/chef-microwave-oven-control.h
Outdated
Show resolved
Hide resolved
examples/chef/common/clusters/microwave-oven-control/chef-microwave-oven-control.h
Show resolved
Hide resolved
examples/chef/common/clusters/microwave-oven-control/chef-microwave-oven-control.h
Outdated
Show resolved
Hide resolved
examples/chef/common/clusters/microwave-oven-control/chef-microwave-oven-control.cpp
Outdated
Show resolved
Hide resolved
examples/chef/common/clusters/microwave-oven-control/chef-microwave-oven-control.cpp
Outdated
Show resolved
Hide resolved
examples/chef/common/clusters/microwave-oven-control/chef-microwave-oven-control.cpp
Outdated
Show resolved
Hide resolved
Also test heating cycle by either sending start on operationalState cluster or SetCookingParameters with StartAfterSetting=True on Microwave oven control cluster. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolve Gemini suggestions.
Tested with following commands |
PR #38628: Size comparison from 63a015c to c493291 Full report (82 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
PR #38628: Size comparison from 4944739 to 1b1239a Full report (82 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
PR #38628: Size comparison from dda2252 to add6af9 Full report (82 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Carrying @sxb427 review
* add: chef app for microwave oven * change file name * remove old files * add: add required attribute * fix typo * Restyled by whitespace * merge master * add microwave oven control cluster impl * Restyled by whitespace * Restyled by clang-format * remove multiple type inheritance * Restyled by clang-format * ModeTagStructType * update pointer * Restyled by whitespace * Restyled by clang-format * rebuild matter file, enable WattRating attribute * determine endpoint when device start * Restyled by clang-format * chore: set FeatureMap global * chore: Address review comments * fix ptr * Restyled by whitespace * Restyled by clang-format * chore:Build matter file * update time; set cycle * chore: rebuild matter file * create instance for each endpoint * update zap file --------- Co-authored-by: Restyled.io <commits@restyled.io> Co-authored-by: Sting Chang <houlunc@google.com> Co-authored-by: Sting Chang <33673360+stingchang@users.noreply.github.com>
This PR introduces the initial implementation for the Microwave Oven Chef device.
Key changes include:
.zap
and.matter
configuration files for the Microwave Oven device type.MicrowaveOvenMode
cluster.matter_device_types.json
.Testing:
chip-tool
andrpc-console
.TC_DeviceBasicComposition.py
test suite passes.Run with
python src/python_testing/TC_DeviceBasicComposition.py --qr-code MT:Y.K90AFN00KA0648G00 --dut-node-id 0x20 --commissioning-method on-network
TC_DeviceConformance.py
test suite passes.Run with
python src/python_testing/TC_DeviceConformance.py --qr-code MT:Y.K90AFN00KA0648G00 --dut-node-id 0x20 --commissioning-method on-network --bool-arg allow_provisional:True
chip-tool commands
Microwave-Oven-Mode
Microwave-Oven-Control