-
Notifications
You must be signed in to change notification settings - Fork 31
[SL-ONLY] Oven app commands implementation #693
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
base: main
Are you sure you want to change the base?
Changes from 36 commits
f24e3f0
90e0388
65a5c48
1a8dd8d
8c86d55
204d3a1
ec3943e
2b9505f
03aa29e
ebf324e
6d73e62
9f63524
d5f86e9
00952f3
df13ff4
3f5eaec
dde3035
c36c1ca
efdc6e3
f1aacf6
caad0fb
c9545d8
b1e9be0
0e0ee4e
3643d79
49a6cbb
17e72b9
5826f8f
232fe4f
50bfb9c
efceeac
d3cb6fa
ba72490
f68c4e2
fbc02c4
52cb237
1baad61
6d426f9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -17,14 +17,26 @@ | |||
| */ | ||||
|
|
||||
| #include "CookSurfaceEndpoint.h" | ||||
| #include <lib/core/CHIPError.h> | ||||
| #include <app/clusters/on-off-server/on-off-server.h> | ||||
| #include <protocols/interaction_model/StatusCode.h> | ||||
|
|
||||
| using namespace chip; | ||||
| using namespace chip::app::Clusters; | ||||
| using namespace chip::app::Clusters::CookSurface; | ||||
|
|
||||
| CHIP_ERROR CookSurfaceEndpoint::Init() | ||||
| { | ||||
| // Placeholder for user Init | ||||
| return CHIP_NO_ERROR; | ||||
arun-silabs marked this conversation as resolved.
Show resolved
Hide resolved
|
||||
| } | ||||
|
|
||||
| void CookSurfaceEndpoint::HandleOffCommand() {} | ||||
| chip::Protocols::InteractionModel::Status CookSurfaceEndpoint::GetOnOffState(bool & state) | ||||
| { | ||||
| return OnOffServer::Instance().getOnOffValue(mEndpointId, &state); | ||||
| } | ||||
|
|
||||
| chip::Protocols::InteractionModel::Status CookSurfaceEndpoint::SetOnOffState(bool state) | ||||
| { | ||||
| CommandId commandId = state ? OnOff::Commands::On::Id : OnOff::Commands::Off::Id; | ||||
| return OnOffServer::Instance().setOnOffValue(mEndpointId, commandId, false); | ||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this be encased in a chipTaskLock ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not necessarily if it is called from ChipTask. I have added the function usage in header file (
ScheduleWork or ChipTaskLock should only be used if this function is called from AppTask |
||||
| } | ||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,7 +24,12 @@ using namespace chip::app::Clusters::CookTop; | |
|
|
||
| CHIP_ERROR CookTopEndpoint::Init() | ||
| { | ||
| // Placeholder for user Init | ||
| return CHIP_NO_ERROR; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again please add a comment as why this function is needed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a placeholder function similar to one in CookSurface, added a comment. |
||
| } | ||
|
|
||
| void CookTopEndpoint::HandleOffCommand() {} | ||
| chip::Protocols::InteractionModel::Status CookTopEndpoint::SetOnOffState(bool state) | ||
| { | ||
| CommandId commandId = state ? OnOff::Commands::On::Id : OnOff::Commands::Off::Id; | ||
| return OnOffServer::Instance().setOnOffValue(mEndpointId, commandId, false); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,7 +17,10 @@ | |
| */ | ||
|
|
||
| #include "OvenEndpoint.h" | ||
| #include <app-common/zap-generated/attributes/Accessors.h> | ||
| #include <app-common/zap-generated/cluster-objects.h> | ||
|
|
||
| #include "OvenManager.h" | ||
| #include <app/clusters/mode-base-server/mode-base-cluster-objects.h> | ||
| #include <lib/core/CHIPError.h> | ||
| #include <lib/support/CodeUtils.h> | ||
|
|
@@ -29,59 +32,55 @@ using namespace chip::app::Clusters::Oven; | |
| using namespace chip::app::Clusters::OvenMode; | ||
| using namespace chip::app::Clusters::TemperatureControlledCabinet; | ||
| using chip::Protocols::InteractionModel::Status; | ||
| using detail::Structs::ModeTagStruct::Type; | ||
|
|
||
| // Static member definitions | ||
| const detail::Structs::ModeTagStruct::Type OvenModeDelegate::sModeTagsBake[1] = { { .value = to_underlying(ModeTag::kBake) } }; | ||
| const Type OvenModeDelegate::sModeTagsBake[1] = { { .value = to_underlying(ModeTag::kBake) } }; | ||
|
|
||
| const detail::Structs::ModeTagStruct::Type OvenModeDelegate::sModeTagsConvection[1] = { { .value = to_underlying( | ||
| ModeTag::kConvection) } }; | ||
| const Type OvenModeDelegate::sModeTagsConvection[1] = { { .value = to_underlying(ModeTag::kConvection) } }; | ||
|
|
||
| const detail::Structs::ModeTagStruct::Type OvenModeDelegate::sModeTagsGrill[1] = { { .value = to_underlying(ModeTag::kGrill) } }; | ||
| const Type OvenModeDelegate::sModeTagsGrill[1] = { { .value = to_underlying(ModeTag::kGrill) } }; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do we need an array of 1 ? What is this logic and why is this needed ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This logic is for adding a unique tag to all the oven-modes. |
||
|
|
||
| const detail::Structs::ModeTagStruct::Type OvenModeDelegate::sModeTagsRoast[1] = { { .value = to_underlying(ModeTag::kRoast) } }; | ||
| const Type OvenModeDelegate::sModeTagsRoast[1] = { { .value = to_underlying(ModeTag::kRoast) } }; | ||
|
|
||
| const detail::Structs::ModeTagStruct::Type OvenModeDelegate::sModeTagsClean[1] = { { .value = to_underlying(ModeTag::kClean) } }; | ||
| const Type OvenModeDelegate::sModeTagsClean[1] = { { .value = to_underlying(ModeTag::kClean) } }; | ||
|
|
||
| const detail::Structs::ModeTagStruct::Type OvenModeDelegate::sModeTagsConvectionBake[1] = { { .value = to_underlying( | ||
| ModeTag::kConvectionBake) } }; | ||
| const Type OvenModeDelegate::sModeTagsConvectionBake[1] = { { .value = to_underlying(ModeTag::kConvectionBake) } }; | ||
|
|
||
| const detail::Structs::ModeTagStruct::Type OvenModeDelegate::sModeTagsConvectionRoast[1] = { { .value = to_underlying( | ||
| ModeTag::kConvectionRoast) } }; | ||
| const Type OvenModeDelegate::sModeTagsConvectionRoast[1] = { { .value = to_underlying(ModeTag::kConvectionRoast) } }; | ||
|
|
||
| const detail::Structs::ModeTagStruct::Type OvenModeDelegate::sModeTagsWarming[1] = { { .value = | ||
| to_underlying(ModeTag::kWarming) } }; | ||
| const Type OvenModeDelegate::sModeTagsWarming[1] = { { .value = to_underlying(ModeTag::kWarming) } }; | ||
|
|
||
| const detail::Structs::ModeTagStruct::Type OvenModeDelegate::sModeTagsProofing[1] = { { .value = | ||
| to_underlying(ModeTag::kProofing) } }; | ||
| const Type OvenModeDelegate::sModeTagsProofing[1] = { { .value = to_underlying(ModeTag::kProofing) } }; | ||
arun-silabs marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| const detail::Structs::ModeOptionStruct::Type OvenModeDelegate::skModeOptions[to_underlying(OvenModes::kModeCount)] = { | ||
| { .label = CharSpan::fromCharString("Bake"), | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. again this is unreadable 😢 , there must be a cleaner and more concise way to achieve the desired result. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. simplified the declarations by adding this : |
||
| .mode = to_underlying(OvenModes::kModeBake), | ||
| .modeTags = DataModel::List<const detail::Structs::ModeTagStruct::Type>(sModeTagsBake) }, | ||
| .modeTags = DataModel::List<const Type>(sModeTagsBake) }, | ||
| { .label = CharSpan::fromCharString("Convection"), | ||
| .mode = to_underlying(OvenModes::kModeConvection), | ||
| .modeTags = DataModel::List<const detail::Structs::ModeTagStruct::Type>(sModeTagsConvection) }, | ||
| .modeTags = DataModel::List<const Type>(sModeTagsConvection) }, | ||
| { .label = CharSpan::fromCharString("Grill"), | ||
| .mode = to_underlying(OvenModes::kModeGrill), | ||
| .modeTags = DataModel::List<const detail::Structs::ModeTagStruct::Type>(sModeTagsGrill) }, | ||
| .modeTags = DataModel::List<const Type>(sModeTagsGrill) }, | ||
| { .label = CharSpan::fromCharString("Roast"), | ||
| .mode = to_underlying(OvenModes::kModeRoast), | ||
| .modeTags = DataModel::List<const detail::Structs::ModeTagStruct::Type>(sModeTagsRoast) }, | ||
| .modeTags = DataModel::List<const Type>(sModeTagsRoast) }, | ||
| { .label = CharSpan::fromCharString("Clean"), | ||
| .mode = to_underlying(OvenModes::kModeClean), | ||
| .modeTags = DataModel::List<const detail::Structs::ModeTagStruct::Type>(sModeTagsClean) }, | ||
| .modeTags = DataModel::List<const Type>(sModeTagsClean) }, | ||
| { .label = CharSpan::fromCharString("Convection Bake"), | ||
| .mode = to_underlying(OvenModes::kModeConvectionBake), | ||
| .modeTags = DataModel::List<const detail::Structs::ModeTagStruct::Type>(sModeTagsConvectionBake) }, | ||
| .modeTags = DataModel::List<const Type>(sModeTagsConvectionBake) }, | ||
| { .label = CharSpan::fromCharString("Convection Roast"), | ||
| .mode = to_underlying(OvenModes::kModeConvectionRoast), | ||
| .modeTags = DataModel::List<const detail::Structs::ModeTagStruct::Type>(sModeTagsConvectionRoast) }, | ||
| .modeTags = DataModel::List<const Type>(sModeTagsConvectionRoast) }, | ||
| { .label = CharSpan::fromCharString("Warming"), | ||
| .mode = to_underlying(OvenModes::kModeWarming), | ||
| .modeTags = DataModel::List<const detail::Structs::ModeTagStruct::Type>(sModeTagsWarming) }, | ||
| .modeTags = DataModel::List<const Type>(sModeTagsWarming) }, | ||
| { .label = CharSpan::fromCharString("Proofing"), | ||
| .mode = to_underlying(OvenModes::kModeProofing), | ||
| .modeTags = DataModel::List<const detail::Structs::ModeTagStruct::Type>(sModeTagsProofing) } | ||
| .modeTags = DataModel::List<const Type>(sModeTagsProofing) } | ||
| }; | ||
|
|
||
| CHIP_ERROR OvenModeDelegate::Init() | ||
|
|
@@ -92,8 +91,50 @@ CHIP_ERROR OvenModeDelegate::Init() | |
|
|
||
| void OvenModeDelegate::HandleChangeToMode(uint8_t NewMode, ModeBase::Commands::ChangeToModeResponse::Type & response) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Didn't we have a mode enum somewhere in another header file ? is so we should use it instead of the generic There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. HandleChangeToMode is a function given by the delegate. In the delegate, it has uint8_t as the parameter type. |
||
| { | ||
| ChipLogProgress(Zcl, "OvenModeDelegate::HandleChangeToMode: NewMode=%d", NewMode); | ||
| // TODO: Implement logic to change the oven mode. | ||
| ChipLogProgress(Zcl, "OvenModeDelegate forwarding mode change to OvenManager (ep=%u newMode=%u)", mEndpointId, NewMode); | ||
| // Verify newMode is among supported modes | ||
| bool supported = IsSupportedMode(NewMode); | ||
| if (!supported) | ||
| { | ||
| response.status = to_underlying(ModeBase::StatusCode::kUnsupportedMode); | ||
| return; | ||
| } | ||
|
|
||
| // Read Current Oven Mode | ||
| uint8_t currentMode; | ||
| Status attrStatus = OvenMode::Attributes::CurrentMode::Get(mEndpointId, ¤tMode); | ||
| if (attrStatus != Status::Success) | ||
| { | ||
| ChipLogError(AppServer, "OvenManager: Failed to read CurrentMode"); | ||
| response.status = to_underlying(ModeBase::StatusCode::kGenericFailure); | ||
| response.statusText.SetValue(CharSpan::fromCharString("Read CurrentMode failed")); | ||
| return; | ||
| } | ||
|
|
||
| // No action needed if current mode is the same as new mode | ||
| if (currentMode == NewMode) | ||
| { | ||
| response.status = to_underlying(ModeBase::StatusCode::kSuccess); | ||
| return; | ||
| } | ||
|
|
||
| if (OvenManager::GetInstance().IsTransitionBlocked(currentMode, NewMode)) | ||
| { | ||
| ChipLogProgress(AppServer, "OvenManager: Blocked transition %u -> %u", currentMode, NewMode); | ||
| response.status = to_underlying(ModeBase::StatusCode::kGenericFailure); | ||
| response.statusText.SetValue(CharSpan::fromCharString("Transition blocked")); | ||
| return; | ||
| } | ||
|
|
||
| // Write new mode | ||
| Status writeStatus = OvenMode::Attributes::CurrentMode::Set(mEndpointId, NewMode); | ||
| if (writeStatus != Status::Success) | ||
| { | ||
| ChipLogError(AppServer, "OvenManager: Failed to write CurrentMode"); | ||
| response.status = to_underlying(ModeBase::StatusCode::kGenericFailure); | ||
| response.statusText.SetValue(CharSpan::fromCharString("Write CurrentMode failed")); | ||
| return; | ||
| } | ||
| response.status = to_underlying(ModeBase::StatusCode::kSuccess); | ||
| } | ||
|
|
||
|
|
@@ -135,3 +176,15 @@ CHIP_ERROR OvenEndpoint::Init() | |
| { | ||
| return CHIP_NO_ERROR; | ||
| } | ||
|
|
||
| bool OvenModeDelegate::IsSupportedMode(uint8_t mode) | ||
| { | ||
| for (auto const & opt : skModeOptions) | ||
| { | ||
| if (opt.mode == mode) | ||
| { | ||
| return true; | ||
| } | ||
| } | ||
| return false; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,6 +28,7 @@ | |
|
|
||
| #include "AppEvent.h" | ||
| #include "BaseApplication.h" | ||
| #include "OvenManager.h" | ||
|
||
|
|
||
| #include "FreeRTOS.h" | ||
| #include "timers.h" // provides FreeRTOS timer support | ||
|
|
@@ -87,13 +88,4 @@ class AppTask : public BaseApplication | |
| * @return CHIP_ERROR | ||
| */ | ||
| CHIP_ERROR AppInit() override; | ||
|
|
||
| /** | ||
| * @brief PB0 Button event processing function | ||
| * Press and hold will trigger a factory reset timer start | ||
| * Press and release will restart BLEAdvertising if not commissioned | ||
| * | ||
| * @param aEvent button event being processed | ||
| */ | ||
| static void ButtonHandler(AppEvent * aEvent); | ||
| }; | ||
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.
[nitpick] Extra blank line between documentation and function declaration. Remove the blank line on line 55 to maintain consistency with standard documentation formatting practices.