Skip to content

Commit

Permalink
Fix Darwin CI issues
Browse files Browse the repository at this point in the history 
Editorial fixes
  • Loading branch information
@nivi-apple
nivi-apple committed Jul 26, 2024
1 parent 3dad3f3 commit 4be03cb
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 47 deletions.
6 changes: 1 addition & 5 deletions examples/thermostat/linux/thermostat-delegate-impl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ void ThermostatDelegate::InitializePresets()
uint8_t index = 0;
for (PresetScenarioEnum presetScenario : presetScenarioEnumArray)
{
ChipLogDetail(Zcl, "initializing preset for scenario %hhu", presetScenario);
mPresets[index].SetPresetScenario(presetScenario);

// Set the preset handle to the preset scenario value as a unique id.
Expand Down Expand Up @@ -195,10 +194,7 @@ CHIP_ERROR ThermostatDelegate::AppendToPendingPresetList(const PresetStruct::Typ
mNextFreeIndexInPendingPresetsList++;
return CHIP_NO_ERROR;
}
else
{
return CHIP_ERROR_WRITE_FAILED;
}
return CHIP_ERROR_WRITE_FAILED;
}

CHIP_ERROR ThermostatDelegate::GetPendingPresetAtIndex(size_t index, PresetStructWithOwnedMembers & preset)
Expand Down
34 changes: 16 additions & 18 deletions examples/thermostat/linux/thermostat-manager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,6 @@ CHIP_ERROR ThermostatManager::Init()
DeviceLayer::PlatformMgr().AddEventHandler(OnPlatformChipDeviceEvent, reinterpret_cast<intptr_t>(this));
DeviceLayer::PlatformMgr().ScheduleWork(InitBindingManager);

PlatformMgr().LockChipStack();
mLocalTemperature = GetCurrentTemperature();
mSystemMode = GetSystemMode();
mRunningMode = GetRunningMode();
Expand All @@ -179,14 +178,11 @@ CHIP_ERROR ThermostatManager::Init()
// TODO: Gotta expose this properly on attribute
mOccupiedSetback = 5; // 0.5 C

PlatformMgr().UnlockChipStack();

ChipLogError(AppServer,
"Initialized a thermostat with \n "
"mSystemMode: %hhu (%s) \n mRunningMode: %hhu (%s) \n mLocalTemperature: %d \n mOccupiedHeatingSetpoint: %d \n "
"mOccupiedCoolingSetpoint: %d"
"NumberOfPresets: %d",
mSystemMode, SystemModeString(mSystemMode), mRunningMode, RunningModeString(mRunningMode), mLocalTemperature,
"mSystemMode: %u (%s) \n mRunningMode: %u (%s) \n mLocalTemperature: %d \n mOccupiedHeatingSetpoint: %d \n "
"mOccupiedCoolingSetpoint: %d" "NumberOfPresets: %d",
static_cast<uint8_t>(mSystemMode), SystemModeString(mSystemMode), static_cast<uint8_t>(mRunningMode), RunningModeString(mRunningMode), mLocalTemperature,
mOccupiedHeatingSetpoint, mOccupiedCoolingSetpoint, GetNumberOfPresets());

// TODO: Should this be called later?
Expand Down Expand Up @@ -253,14 +249,14 @@ void ThermostatManager::ThermostatClusterAttributeChangeHandler(AttributeId attr

case SystemMode::Id: {
mSystemMode = static_cast<SystemModeEnum>(*value);
ChipLogError(AppServer, "System mode changed to %hhu (%s)", mSystemMode, SystemModeString(mSystemMode));
ChipLogError(AppServer, "System mode changed to %d (%s)", *value, SystemModeString(mSystemMode));
EvalThermostatState();
}
break;

case ThermostatRunningMode::Id: {
mRunningMode = static_cast<ThermostatRunningModeEnum>(*value);
ChipLogError(AppServer, "Running mode changed to %hhu (%s)", mRunningMode, RunningModeString(mRunningMode));
ChipLogError(AppServer, "Running mode changed to %u (%s)", *value, RunningModeString(mRunningMode));
}
break;

Expand All @@ -276,14 +272,14 @@ SystemModeEnum ThermostatManager::GetSystemMode()
{
SystemModeEnum systemMode;
SystemMode::Get(kThermostatEndpoint, &systemMode);
return static_cast<SystemModeEnum>(systemMode);
return systemMode;
}

ThermostatRunningModeEnum ThermostatManager::GetRunningMode()
{
ThermostatRunningModeEnum runningMode;
ThermostatRunningMode::Get(kThermostatEndpoint, &runningMode);
return static_cast<ThermostatRunningModeEnum>(runningMode);
return runningMode;
}

int16_t ThermostatManager::GetCurrentTemperature()
Expand Down Expand Up @@ -315,13 +311,14 @@ uint8_t ThermostatManager::GetNumberOfPresets()

CHIP_ERROR ThermostatManager::SetSystemMode(SystemModeEnum systemMode)
{
uint8_t systemModeValue = static_cast<uint8_t>(systemMode);
if (mSystemMode == systemMode)
{
ChipLogDetail(AppServer, "Already in system mode: %hhu (%s)", systemMode, SystemModeString(systemMode));
ChipLogDetail(AppServer, "Already in system mode: %u (%s)", systemModeValue, SystemModeString(systemMode));
return CHIP_NO_ERROR;
}

ChipLogError(AppServer, "Setting system mode: %hhu (%s)", systemMode, SystemModeString(systemMode));
ChipLogError(AppServer, "Setting system mode: %u (%s)", systemModeValue, SystemModeString(systemMode));
Protocols::InteractionModel::Status status = SystemMode::Set(kThermostatEndpoint, systemMode);

// TODO: CHIP_ERROR_WRITE_FAILED might not be the best error code to send
Expand All @@ -330,13 +327,14 @@ CHIP_ERROR ThermostatManager::SetSystemMode(SystemModeEnum systemMode)

CHIP_ERROR ThermostatManager::SetRunningMode(ThermostatRunningModeEnum runningMode)
{
uint8_t runningModeValue = static_cast<uint8_t>(runningMode);
if (mRunningMode == runningMode)
{
ChipLogDetail(AppServer, "Already in running mode: %hhu (%s)", runningMode, RunningModeString(runningMode));
ChipLogDetail(AppServer, "Already in running mode: %u (%s)", runningModeValue, RunningModeString(runningMode));
return CHIP_NO_ERROR;
}

ChipLogError(AppServer, "Setting running mode: %hhu (%s)", runningMode, RunningModeString(runningMode));
ChipLogError(AppServer, "Setting running mode: %u (%s)", runningModeValue, RunningModeString(runningMode));
Protocols::InteractionModel::Status status = ThermostatRunningMode::Set(kThermostatEndpoint, runningMode);

// TODO: CHIP_ERROR_WRITE_FAILED might not be the best error code to send
Expand Down Expand Up @@ -365,12 +363,12 @@ void ThermostatManager::EvalThermostatState()
{
ChipLogError(AppServer,
"Eval Thermostat Running Mode \n "
"mSystemMode: %hhu (%s) \n mRunningMode: %hhu (%s) \n mLocalTemperature: %d \n mOccupiedHeatingSetpoint: %d \n "
"mSystemMode: %u (%s) \n mRunningMode: %u (%s) \n mLocalTemperature: %d \n mOccupiedHeatingSetpoint: %d \n "
"mOccupiedCoolingSetpoint: %d",
mSystemMode, SystemModeString(mSystemMode), mRunningMode, RunningModeString(mRunningMode), mLocalTemperature,
static_cast<uint8_t>(mSystemMode), SystemModeString(mSystemMode), static_cast<uint8_t>(mRunningMode), RunningModeString(mRunningMode), mLocalTemperature,
mOccupiedHeatingSetpoint, mOccupiedCoolingSetpoint);

switch (static_cast<SystemModeEnum>(mSystemMode))
switch (mSystemMode)
{
case SystemModeEnum::kOff: {
SetRunningMode(ThermostatRunningModeEnum::kOff);
Expand Down
45 changes: 21 additions & 24 deletions src/app/clusters/thermostat-server/thermostat-server.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -854,11 +854,11 @@ CHIP_ERROR ThermostatAttrAccess::Write(const ConcreteDataAttributePath & aPath,
Delegate * delegate = GetDelegate(endpoint);
VerifyOrReturnError(delegate != nullptr, CHIP_ERROR_INCORRECT_STATE, ChipLogError(Zcl, "Delegate is null"));

// #1 Presets are not editable, return INVALID_IN_STATE.
// Presets are not editable, return INVALID_IN_STATE.
VerifyOrReturnError(gThermostatAttrAccess.GetPresetsEditable(endpoint), CHIP_IM_GLOBAL_STATUS(InvalidInState),
ChipLogError(Zcl, "Presets are not editable"));

// #2 Check if the OriginatorScopedNodeId at the endpoint is the same as the node editing the presets,
// Check if the OriginatorScopedNodeId at the endpoint is the same as the node editing the presets,
// otherwise return BUSY.
const Access::SubjectDescriptor subjectDescriptor = aDecoder.GetSubjectDescriptor();
ScopedNodeId scopedNodeId = ScopedNodeId(subjectDescriptor.subject, subjectDescriptor.fabricIndex);
Expand Down Expand Up @@ -905,10 +905,7 @@ CHIP_ERROR ThermostatAttrAccess::Write(const ConcreteDataAttributePath & aPath,
{
return delegate->AppendToPendingPresetList(preset);
}
else
{
return CHIP_IM_GLOBAL_STATUS(ConstraintError);
}
return CHIP_IM_GLOBAL_STATUS(ConstraintError);
}
}
break;
Expand Down Expand Up @@ -1292,7 +1289,7 @@ bool emberAfThermostatClusterStartPresetsSchedulesEditRequestCallback(

EndpointId endpoint = commandPath.mEndpointId;

// #1 If the presets are editable and the scoped node id of the client sending StartPresetsSchedulesEditRequest command
// If the presets are editable and the scoped node id of the client sending StartPresetsSchedulesEditRequest command
// is not the same as the one that previously originated a StartPresetsSchedulesEditRequest command, return BUSY.
if (gThermostatAttrAccess.GetPresetsEditable(endpoint) &&
(gThermostatAttrAccess.GetOriginatorScopedNodeId(endpoint) != sourceNodeId))
Expand All @@ -1301,7 +1298,7 @@ bool emberAfThermostatClusterStartPresetsSchedulesEditRequestCallback(
return true;
}

// #2 If presets are editable and the scoped node id of the client sending StartPresetsSchedulesEditRequest command
// If presets are editable and the scoped node id of the client sending StartPresetsSchedulesEditRequest command
// is the same as the one that previously originated a StartPresetsSchedulesEditRequest command, extend the timer.
if (gThermostatAttrAccess.GetPresetsEditable(endpoint))
{
Expand All @@ -1325,7 +1322,7 @@ bool emberAfThermostatClusterCancelPresetsSchedulesEditRequestCallback(
{
EndpointId endpoint = commandPath.mEndpointId;

// #1 If presets are not editable, return INVALID_IN_STATE.
// If presets are not editable, return INVALID_IN_STATE.
if (!gThermostatAttrAccess.GetPresetsEditable(endpoint))
{
commandObj->AddStatus(commandPath, imcode::InvalidInState);
Expand All @@ -1334,7 +1331,7 @@ bool emberAfThermostatClusterCancelPresetsSchedulesEditRequestCallback(

ScopedNodeId sourceNodeId = GetSourceScopedNodeId(commandObj);

// #2 If the node id sending the CancelPresetsSchedulesRequest command is not the same as the one which send the
// If the node id sending the CancelPresetsSchedulesRequest command is not the same as the one which send the
// previous StartPresetsSchedulesEditRequest, return UNSUPPORTED_ACCESS.
if (gThermostatAttrAccess.GetOriginatorScopedNodeId(endpoint) != sourceNodeId)
{
Expand Down Expand Up @@ -1367,15 +1364,15 @@ bool emberAfThermostatClusterCommitPresetsSchedulesRequestCallback(
return SendResponseAndCleanUp(delegate, endpoint, commandObj, commandPath, imcode::InvalidInState);
}

// #1. If presets are not editable, return INVALID_IN_STATE.
// If presets are not editable, return INVALID_IN_STATE.
if (!gThermostatAttrAccess.GetPresetsEditable(endpoint))
{
return SendResponseAndCleanUp(delegate, endpoint, commandObj, commandPath, imcode::InvalidInState);
}

ScopedNodeId sourceNodeId = GetSourceScopedNodeId(commandObj);

// #2. If the node id sending the CommitPresetsSchedulesRequest command is not the same as the one which send the
// If the node id sending the CommitPresetsSchedulesRequest command is not the same as the one which send the
// StartPresetsSchedulesEditRequest, return UNSUPPORTED_ACCESS.
if (gThermostatAttrAccess.GetOriginatorScopedNodeId(endpoint) != sourceNodeId)
{
Expand Down Expand Up @@ -1406,15 +1403,15 @@ bool emberAfThermostatClusterCommitPresetsSchedulesRequestCallback(

bool found = MatchingPendingPresetExists(delegate, preset);

// #3. If a built in preset in the Presets attribute list is removed and not found in the pending presets list, return
// If a built in preset in the Presets attribute list is removed and not found in the pending presets list, return
// CONSTRAINT_ERROR.
if (IsBuiltIn(preset) && !found)
{
return SendResponseAndCleanUp(delegate, endpoint, commandObj, commandPath, imcode::ConstraintError);
}
}

// #4. If there is an ActivePresetHandle set, find the preset in the pending presets list that matches the ActivePresetHandle
// If there is an ActivePresetHandle set, find the preset in the pending presets list that matches the ActivePresetHandle
// attribute. If a preset is not found with the same presetHandle, return INVALID_IN_STATE. If there is no ActivePresetHandle
// attribute set, continue with other checks.
uint8_t buffer[kPresetHandleSize];
Expand Down Expand Up @@ -1458,12 +1455,12 @@ bool emberAfThermostatClusterCommitPresetsSchedulesRequestCallback(
bool isPendingPresetWithNullPresetHandle = pendingPreset.GetPresetHandle().IsNull();
if (isPendingPresetWithNullPresetHandle)
{
// #5. If the presetHandle for a preset is null, the device should set a unique presetHandle.
// If the presetHandle for a preset is null, the device should set a unique presetHandle.
const uint8_t handle[] = { static_cast<uint8_t>(pendingPreset.GetPresetScenario()) };
pendingPreset.SetPresetHandle(DataModel::MakeNullable(ByteSpan(handle)));
}

// #6. If the preset handle is null and the built in field is set to true, return CONSTRAINT_ERROR.
// If the preset handle is null and the built in field is set to true, return CONSTRAINT_ERROR.
if (isPendingPresetWithNullPresetHandle && IsBuiltIn(pendingPreset))
{
return SendResponseAndCleanUp(delegate, endpoint, commandObj, commandPath, imcode::ConstraintError);
Expand All @@ -1475,14 +1472,14 @@ bool emberAfThermostatClusterCommitPresetsSchedulesRequestCallback(
{
foundMatchingPresetInPresets = GetMatchingPresetInPresets(delegate, pendingPreset, matchingPreset);

// #7. If the presetHandle for the pending preset is not null and a matching preset is not found in the
// If the presetHandle for the pending preset is not null and a matching preset is not found in the
// presets attribute list, return NOT_FOUND.
if (!foundMatchingPresetInPresets)
{
return SendResponseAndCleanUp(delegate, endpoint, commandObj, commandPath, imcode::NotFound);
}

// #8. Find the number of presets in the pending preset list that match the preset handle. If there are duplicate
// Find the number of presets in the pending preset list that match the preset handle. If there are duplicate
// entries, return CONSTRAINT_ERROR.
uint8_t count = CountPresetsInPendingListWithPresetHandle(delegate, pendingPreset.GetPresetHandle().Value());
if (count > 1)
Expand All @@ -1491,28 +1488,28 @@ bool emberAfThermostatClusterCommitPresetsSchedulesRequestCallback(
}
}

// #9. If the preset is found in the presets attribute list and the preset is builtIn in the pending presets list
// If the preset is found in the presets attribute list and the preset is builtIn in the pending presets list
// but not in the presets attribute list, return UNSUPPORTED_ACCESS.
if (foundMatchingPresetInPresets && (IsBuiltIn(pendingPreset) && !IsBuiltIn(matchingPreset)))
{
return SendResponseAndCleanUp(delegate, endpoint, commandObj, commandPath, imcode::UnsupportedAccess);
}

// #10. If the preset is found in the presets attribute list and the preset is builtIn in the presets attribute
// If the preset is found in the presets attribute list and the preset is builtIn in the presets attribute
// but not in the pending presets list, return UNSUPPORTED_ACCESS.
if (foundMatchingPresetInPresets && (!IsBuiltIn(pendingPreset) && IsBuiltIn(matchingPreset)))
{
return SendResponseAndCleanUp(delegate, endpoint, commandObj, commandPath, imcode::UnsupportedAccess);
}

// #11. If the presetScenario is not found in the preset types, return CONSTRAINT_ERROR.
// If the presetScenario is not found in the preset types, return CONSTRAINT_ERROR.
PresetScenarioEnum presetScenario = pendingPreset.GetPresetScenario();
if (!FindPresetScenarioInPresetTypes(delegate, presetScenario))
{
return SendResponseAndCleanUp(delegate, endpoint, commandObj, commandPath, imcode::ConstraintError);
}

// #12. If the preset type for the preset scenario does not supports names and a name is specified, return CONSTRAINT_ERROR.
// If the preset type for the preset scenario does not supports names and a name is specified, return CONSTRAINT_ERROR.
if (!PresetTypeSupportsNames(delegate, presetScenario) && pendingPreset.GetName().HasValue())
{
return SendResponseAndCleanUp(delegate, endpoint, commandObj, commandPath, imcode::ConstraintError);
Expand Down Expand Up @@ -1542,14 +1539,14 @@ bool emberAfThermostatClusterCommitPresetsSchedulesRequestCallback(
return SendResponseAndCleanUp(delegate, endpoint, commandObj, commandPath, imcode::InvalidInState);
}

// #12 If the expected length of the presets attribute with the applied changes exceeds the total number of presets supported,
// If the expected length of the presets attribute with the applied changes exceeds the total number of presets supported,
// return RESOURCE_EXHAUSTED. Note that the changes are not yet applied.
if (numberOfPresetsSupported > 0 && totalCount > numberOfPresetsSupported)
{
return SendResponseAndCleanUp(delegate, endpoint, commandObj, commandPath, imcode::ResourceExhausted);
}

// #13 TODO: #25 Check if the number of presets for each presetScenario exceeds the max number of presets supported for that
// TODO: Check if the number of presets for each presetScenario exceeds the max number of presets supported for that
// scenario. We plan to support only one preset for each presetScenario for our use cases so defer this for re-evaluation.

// Call the delegate API to apply the pending presets to the presets attribute and update it.
Expand Down

0 comments on commit 4be03cb

Please sign in to comment.