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

Dont store transition val #26867

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 21 additions & 9 deletions src/app/clusters/color-control-server/color-control-server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1589,7 +1589,7 @@ bool ColorControlServer::colorLoopCommand(app::CommandHandler * commandObj, cons

uint16_t storedEnhancedHue = 0;
Attributes::ColorLoopStoredEnhancedHue::Get(endpoint, &storedEnhancedHue);
Attributes::EnhancedCurrentHue::Set(endpoint, storedEnhancedHue);
Attributes::EnhancedCurrentHue::Set(endpoint, storedEnhancedHue, false);
}
else
{
Expand Down Expand Up @@ -1627,12 +1627,16 @@ void ColorControlServer::updateHueSatCommand(EndpointId endpoint)

bool isHueTansitionDone = computeNewHueValue(colorHueTransitionState);
bool isSaturationTransitionDone = computeNewColor16uValue(colorSaturationTransitionState);
bool isTemp = true;

SetHSVRemainingTime(endpoint);

if (isHueTansitionDone && isSaturationTransitionDone)
{
stopAllColorTransitions(endpoint);
// we have finished transitioning, save the final value into storage
// don't save to storage if we haven't finish transitioning as it might cause time delay
isTemp = false;
}
else
{
Expand All @@ -1643,8 +1647,8 @@ void ColorControlServer::updateHueSatCommand(EndpointId endpoint)
{
if (previousEnhancedhue != colorHueTransitionState->currentEnhancedHue)
{
Attributes::EnhancedCurrentHue::Set(endpoint, colorHueTransitionState->currentEnhancedHue);
Attributes::CurrentHue::Set(endpoint, static_cast<uint8_t>(colorHueTransitionState->currentEnhancedHue >> 8));
Attributes::EnhancedCurrentHue::Set(endpoint, colorHueTransitionState->currentEnhancedHue, isTemp);
Attributes::CurrentHue::Set(endpoint, static_cast<uint8_t>(colorHueTransitionState->currentEnhancedHue >> 8), isTemp);

emberAfColorControlClusterPrintln("Enhanced Hue %d endpoint %d", colorHueTransitionState->currentEnhancedHue, endpoint);
}
Expand All @@ -1653,15 +1657,15 @@ void ColorControlServer::updateHueSatCommand(EndpointId endpoint)
{
if (previousHue != colorHueTransitionState->currentHue)
{
Attributes::CurrentHue::Set(colorHueTransitionState->endpoint, colorHueTransitionState->currentHue);
Attributes::CurrentHue::Set(colorHueTransitionState->endpoint, colorHueTransitionState->currentHue, isTemp);
emberAfColorControlClusterPrintln("Hue %d endpoint %d", colorHueTransitionState->currentHue, endpoint);
}
}

if (previousSaturation != colorSaturationTransitionState->currentValue)
{
Attributes::CurrentSaturation::Set(colorSaturationTransitionState->endpoint,
(uint8_t) colorSaturationTransitionState->currentValue);
(uint8_t) colorSaturationTransitionState->currentValue, isTemp);
emberAfColorControlClusterPrintln("Saturation %d endpoint %d", colorSaturationTransitionState->currentValue, endpoint);
}

Expand Down Expand Up @@ -1989,6 +1993,7 @@ void ColorControlServer::updateXYCommand(EndpointId endpoint)
Color16uTransitionState * colorXTransitionState = getXTransitionState(endpoint);
Color16uTransitionState * colorYTransitionState = getYTransitionState(endpoint);
bool isXTransitionDone, isYTransitionDone;
bool isTemp = true;

// compute new values for X and Y.
isXTransitionDone = computeNewColor16uValue(colorXTransitionState);
Expand All @@ -1999,15 +2004,18 @@ void ColorControlServer::updateXYCommand(EndpointId endpoint)
if (isXTransitionDone && isYTransitionDone)
{
stopAllColorTransitions(endpoint);
// we have finished transitioning, save the final value into storage
// don't save to storage if we haven't finish transitioning as it might cause time delay
isTemp = false;
}
else
{
scheduleTimerCallbackMs(configureXYEventControl(endpoint), UPDATE_TIME_MS);
}

// update the attributes
Attributes::CurrentX::Set(endpoint, colorXTransitionState->currentValue);
Attributes::CurrentY::Set(endpoint, colorYTransitionState->currentValue);
Attributes::CurrentX::Set(endpoint, colorXTransitionState->currentValue, isTemp);
Attributes::CurrentY::Set(endpoint, colorYTransitionState->currentValue, isTemp);

emberAfColorControlClusterPrintln("Color X %d Color Y %d", colorXTransitionState->currentValue,
colorYTransitionState->currentValue);
Expand Down Expand Up @@ -2168,7 +2176,7 @@ void ColorControlServer::startUpColorTempCommand(EndpointId endpoint)
// existing setting of ColorTemp attribute will be left unchanged (i.e., treated as
// if startup color temp was set to null).
updatedColorTemp = startUpColorTemp.Value();
status = Attributes::ColorTemperatureMireds::Set(endpoint, updatedColorTemp);
status = Attributes::ColorTemperatureMireds::Set(endpoint, updatedColorTemp, false);

if (status == EMBER_ZCL_STATUS_SUCCESS)
{
Expand All @@ -2193,6 +2201,7 @@ void ColorControlServer::updateTempCommand(EndpointId endpoint)
{
Color16uTransitionState * colorTempTransitionState = getTempTransitionState(endpoint);
bool isColorTempTransitionDone;
bool isTemp = true;

isColorTempTransitionDone = computeNewColor16uValue(colorTempTransitionState);

Expand All @@ -2201,13 +2210,16 @@ void ColorControlServer::updateTempCommand(EndpointId endpoint)
if (isColorTempTransitionDone)
{
stopAllColorTransitions(endpoint);
// we have finished transitioning, save the final value into storage
// don't save to storage if we haven't finish transitioning as it might cause time delay
isTemp = false;
}
else
{
scheduleTimerCallbackMs(configureTempEventControl(endpoint), UPDATE_TIME_MS);
}

Attributes::ColorTemperatureMireds::Set(endpoint, colorTempTransitionState->currentValue);
Attributes::ColorTemperatureMireds::Set(endpoint, colorTempTransitionState->currentValue, isTemp);

emberAfColorControlClusterPrintln("Color Temperature %d", colorTempTransitionState->currentValue);

Expand Down
39 changes: 25 additions & 14 deletions src/app/clusters/level-control/level-control.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -268,15 +268,6 @@ void emberAfLevelControlClusterServerTickCallback(EndpointId endpoint)
emberAfLevelControlClusterPrint(" to %d ", currentLevel.Value());
emberAfLevelControlClusterPrintln("(diff %c1)", state->increasing ? '+' : '-');

status = Attributes::CurrentLevel::Set(endpoint, currentLevel);
if (status != EMBER_ZCL_STATUS_SUCCESS)
{
emberAfLevelControlClusterPrintln("ERR: writing current level %x", status);
state->callbackSchedule.runTime = System::Clock::Milliseconds32(0);
writeRemainingTime(endpoint, 0);
return;
}

updateCoupledColorTemp(endpoint);

#ifdef EMBER_AF_PLUGIN_SCENES
Expand All @@ -290,6 +281,16 @@ void emberAfLevelControlClusterServerTickCallback(EndpointId endpoint)
// Are we at the requested level?
if (currentLevel.Value() == state->moveToLevel)
{
// we have finished transitioning, save the final value into storage
status = Attributes::CurrentLevel::Set(endpoint, currentLevel, false);
if (status != EMBER_ZCL_STATUS_SUCCESS)
{
emberAfLevelControlClusterPrintln("ERR: writing current level %x", status);
state->callbackSchedule.runTime = System::Clock::Milliseconds32(0);
writeRemainingTime(endpoint, 0);
return;
}

if (state->commandId == Commands::MoveToLevelWithOnOff::Id || state->commandId == Commands::MoveWithOnOff::Id ||
state->commandId == Commands::StepWithOnOff::Id)
{
Expand All @@ -299,7 +300,7 @@ void emberAfLevelControlClusterServerTickCallback(EndpointId endpoint)
if (state->storedLevel != INVALID_STORED_LEVEL)
{
uint8_t storedLevel8u = (uint8_t) state->storedLevel;
status = Attributes::CurrentLevel::Set(endpoint, storedLevel8u);
status = Attributes::CurrentLevel::Set(endpoint, storedLevel8u, false);
if (status != EMBER_ZCL_STATUS_SUCCESS)
{
emberAfLevelControlClusterPrintln("ERR: writing current level %x", status);
Expand All @@ -315,6 +316,16 @@ void emberAfLevelControlClusterServerTickCallback(EndpointId endpoint)
}
else
{
// we haven't finish transitioning, don't save in storage as it might cause some time delay
status = Attributes::CurrentLevel::Set(endpoint, currentLevel, true);
if (status != EMBER_ZCL_STATUS_SUCCESS)
{
emberAfLevelControlClusterPrintln("ERR: writing temp current level %x", status);
state->callbackSchedule.runTime = System::Clock::Milliseconds32(0);
writeRemainingTime(endpoint, 0);
return;
}

state->callbackSchedule.runTime = System::SystemClock().GetMonotonicTimestamp() - callbackStartTimestamp;
writeRemainingTime(endpoint, static_cast<uint16_t>(state->transitionTimeMs - state->elapsedTimeMs));
scheduleTimerCallbackMs(endpoint, computeCallbackWaitTimeMs(state->callbackSchedule, state->eventDurationMs));
Expand Down Expand Up @@ -1172,7 +1183,7 @@ void emberAfOnOffClusterLevelControlEffectCallback(EndpointId endpoint, bool new
{
// If newValue is OnOff::Commands::On::Id...
// "Set CurrentLevel to minimum level allowed for the device."
status = Attributes::CurrentLevel::Set(endpoint, minimumLevelAllowedForTheDevice);
status = Attributes::CurrentLevel::Set(endpoint, minimumLevelAllowedForTheDevice, false);
if (status != EMBER_ZCL_STATUS_SUCCESS)
{
emberAfLevelControlClusterPrintln("ERR: reading current level %x", status);
Expand Down Expand Up @@ -1288,18 +1299,18 @@ void emberAfLevelControlClusterServerInitCallback(EndpointId endpoint)
}
// Otherwise Set the CurrentLevel attribute to its previous value which was already fetch above

Attributes::CurrentLevel::Set(endpoint, currentLevel);
Attributes::CurrentLevel::Set(endpoint, currentLevel, false);
}
}
#endif // IGNORE_LEVEL_CONTROL_CLUSTER_START_UP_CURRENT_LEVEL
// In any case, we make sure that the respects min/max
if (currentLevel.IsNull() || currentLevel.Value() < state->minLevel)
{
Attributes::CurrentLevel::Set(endpoint, state->minLevel);
Attributes::CurrentLevel::Set(endpoint, state->minLevel, false);
}
else if (currentLevel.Value() > state->maxLevel)
{
Attributes::CurrentLevel::Set(endpoint, state->maxLevel);
Attributes::CurrentLevel::Set(endpoint, state->maxLevel, false);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ static void setEffectiveModes(EndpointId endpoint)
Attributes::EffectiveControlMode::Set(endpoint, ControlModeEnum::kConstantSpeed);
#ifdef EMBER_AF_PLUGIN_LEVEL_CONTROL
LevelControl::Attributes::MaxLevel::Get(endpoint, &maxLevel);
LevelControl::Attributes::CurrentLevel::Set(endpoint, maxLevel);
LevelControl::Attributes::CurrentLevel::Set(endpoint, maxLevel, false);
#endif
if (isPumpStatusAvailable)
{
Expand All @@ -187,7 +187,7 @@ static void setEffectiveModes(EndpointId endpoint)
// Bump the minimum level to 1, since the value of 0 means stop
minLevel = 1;
}
LevelControl::Attributes::CurrentLevel::Set(endpoint, minLevel);
LevelControl::Attributes::CurrentLevel::Set(endpoint, minLevel, false);
#endif
if (isPumpStatusAvailable)
{
Expand Down
13 changes: 7 additions & 6 deletions src/app/clusters/scenes/scenes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,8 @@ EmberAfStatus emberAfScenesClusterRecallSavedSceneCallback(chip::FabricIndex fab
#ifdef ZCL_USING_LEVEL_CONTROL_CLUSTER_SERVER
if (entry.hasCurrentLevelValue)
{
logWriteError(LevelControl::Attributes::CurrentLevel::Set(endpoint, entry.currentLevelValue), "CurrentLevel");
logWriteError(LevelControl::Attributes::CurrentLevel::Set(endpoint, entry.currentLevelValue, false),
"CurrentLevel");
}
#endif
#ifdef ZCL_USING_THERMOSTAT_CLUSTER_SERVER
Expand All @@ -634,20 +635,20 @@ EmberAfStatus emberAfScenesClusterRecallSavedSceneCallback(chip::FabricIndex fab
using namespace ColorControl::Attributes;
if (entry.hasCurrentXValue)
{
logWriteError(CurrentX::Set(endpoint, entry.currentXValue), "CurrentX");
logWriteError(CurrentX::Set(endpoint, entry.currentXValue, false), "CurrentX");
}
if (entry.hasCurrentYValue)
{
logWriteError(CurrentY::Set(endpoint, entry.currentXValue), "CurrentY");
logWriteError(CurrentY::Set(endpoint, entry.currentXValue, false), "CurrentY");
}

if (entry.hasEnhancedCurrentHueValue)
{
logWriteError(EnhancedCurrentHue::Set(endpoint, entry.enhancedCurrentHueValue), "EnhancedCurrentHue");
logWriteError(EnhancedCurrentHue::Set(endpoint, entry.enhancedCurrentHueValue, false), "EnhancedCurrentHue");
}
if (entry.hasCurrentSaturationValue)
{
logWriteError(CurrentSaturation::Set(endpoint, entry.currentSaturationValue), "CurrentSaturation");
logWriteError(CurrentSaturation::Set(endpoint, entry.currentSaturationValue, false), "CurrentSaturation");
}
if (entry.hasColorLoopActiveValue)
{
Expand All @@ -663,7 +664,7 @@ EmberAfStatus emberAfScenesClusterRecallSavedSceneCallback(chip::FabricIndex fab
}
if (entry.hasColorTemperatureMiredsValue)
{
logWriteError(ColorTemperatureMireds::Set(endpoint, entry.colorTemperatureMiredsValue),
logWriteError(ColorTemperatureMireds::Set(endpoint, entry.colorTemperatureMiredsValue, false),
"ColorTemperatureMireds");
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/app/util/af.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ bool emberAfContainsClient(chip::EndpointId endpoint, chip::ClusterId clusterId)
* to perform the given operation.
*/
EmberAfStatus emberAfWriteAttribute(chip::EndpointId endpoint, chip::ClusterId cluster, chip::AttributeId attributeID,
uint8_t * dataPtr, EmberAfAttributeType dataType);
uint8_t * dataPtr, EmberAfAttributeType dataType, bool isTemp);

/**
* @brief Read the attribute value, performing all the checks.
Expand Down
25 changes: 17 additions & 8 deletions src/app/util/attribute-table.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,18 +54,19 @@ EmberAfStatus emberAfWriteAttributeExternal(EndpointId endpoint, ClusterId clust
case EmberAfAttributeWritePermission::AllowWriteNormal:
case EmberAfAttributeWritePermission::AllowWriteOfReadOnly:
return emAfWriteAttribute(endpoint, cluster, attributeID, dataPtr, dataType,
(extWritePermission == EmberAfAttributeWritePermission::AllowWriteOfReadOnly), false);
(extWritePermission == EmberAfAttributeWritePermission::AllowWriteOfReadOnly), false, false);
default:
return (EmberAfStatus) extWritePermission;
}
}

EmberAfStatus emberAfWriteAttribute(EndpointId endpoint, ClusterId cluster, AttributeId attributeID, uint8_t * dataPtr,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest using a new method emberAfWriteAttributeSkipNonVolatile to be much more explicit, which calls the version with isTemp.

This way, almost none of the accessors below need to be changed.

EmberAfAttributeType dataType)
EmberAfAttributeType dataType, bool isTemp)
{
return emAfWriteAttribute(endpoint, cluster, attributeID, dataPtr, dataType,
true, // override read-only?
false); // just test?
true, // override read-only?
false, // just test?
isTemp); // is temporary value (don't store in kvs)?
}

EmberAfStatus emberAfReadAttribute(EndpointId endpoint, ClusterId cluster, AttributeId attributeID, uint8_t * dataPtr,
Expand Down Expand Up @@ -233,8 +234,12 @@ static bool IsNullValue(const uint8_t * data, uint16_t dataLen, bool isAttribute
// the table or the data is too large, returns true and writes to dataPtr
// if the attribute is supported and the readLength specified is less than
// the length of the data.
//
// if true is passed in for isTemp, then the value is a temporary value, don't save it into storage.
// a temporary value is an intermediate value during a transition to the final value.
// we avoid storing such values so as to not delay the transitioning process
EmberAfStatus emAfWriteAttribute(EndpointId endpoint, ClusterId cluster, AttributeId attributeID, uint8_t * data,
EmberAfAttributeType dataType, bool overrideReadOnlyAndDataType, bool justTest)
EmberAfAttributeType dataType, bool overrideReadOnlyAndDataType, bool justTest, bool isTemp)
{
const EmberAfAttributeMetadata * metadata = nullptr;
EmberAfAttributeSearchRecord record;
Expand Down Expand Up @@ -356,9 +361,13 @@ EmberAfStatus emAfWriteAttribute(EndpointId endpoint, ClusterId cluster, Attribu
return status;
}

// Save the attribute to persistent storage if needed
// The callee will weed out attributes that do not need to be stored.
emAfSaveAttributeToStorageIfNeeded(data, endpoint, cluster, metadata);
if (!isTemp)
{
// only save to storage if data is not temporary (intermediate value during transition)
// Save the attribute to persistent storage if needed
// The callee will weed out attributes that do not need to be stored.
emAfSaveAttributeToStorageIfNeeded(data, endpoint, cluster, metadata);
}

MatterReportingAttributeChangeCallback(endpoint, cluster, attributeID);

Expand Down
2 changes: 1 addition & 1 deletion src/app/util/attribute-table.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ EmberAfStatus emberAfWriteAttributeExternal(chip::EndpointId endpoint, chip::Clu
void emberAfPrintAttributeTable(void);

EmberAfStatus emAfWriteAttribute(chip::EndpointId endpoint, chip::ClusterId cluster, chip::AttributeId attributeID, uint8_t * data,
EmberAfAttributeType dataType, bool overrideReadOnlyAndDataType, bool justTest);
EmberAfAttributeType dataType, bool overrideReadOnlyAndDataType, bool justTest, bool isTemp);

EmberAfStatus emAfReadAttribute(chip::EndpointId endpoint, chip::ClusterId cluster, chip::AttributeId attributeID,
uint8_t * dataPtr, uint16_t readLength, EmberAfAttributeType * dataType);
Loading