Skip to content

Commit

Permalink
Removed the fabric_list completely from scene table, change scene tab…
Browse files Browse the repository at this point in the history 
…le to now store transition time in ms, added security on array compression and dissociated handler number from cluster number
  • Loading branch information
@lpbeliveau-silabs @pull
lpbeliveau-silabs authored and pull[bot] committed Aug 22, 2023
1 parent 76f6762 commit 2130848
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 313 deletions.
10 changes: 6 additions & 4 deletions src/app/clusters/scenes/ExtensionFieldSetsImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,15 +121,17 @@ CHIP_ERROR ExtensionFieldSetsImpl::GetFieldSetAtPosition(ExtensionFieldSet & fie

CHIP_ERROR ExtensionFieldSetsImpl::RemoveFieldAtPosition(uint8_t position)
{
VerifyOrReturnError(position < kMaxClusterPerScenes, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnValue(!this->IsEmpty() && !mEFS[position].IsEmpty(), CHIP_NO_ERROR);
VerifyOrReturnValue(position < mFieldSetsCount, CHIP_NO_ERROR);

uint8_t nextPos = static_cast<uint8_t>(position + 1);
uint8_t moveNum = static_cast<uint8_t>(kMaxClusterPerScenes - nextPos);

// TODO: Implement general array management methods
// Compress array after removal
memmove(&mEFS[position], &mEFS[nextPos], sizeof(ExtensionFieldSet) * moveNum);
// Compress array after removal, if the removed position is not the last
if (moveNum)
{
memmove(&mEFS[position], &mEFS[nextPos], sizeof(ExtensionFieldSet) * moveNum);
}

mFieldSetsCount--;
// Clear last occupied position
Expand Down
37 changes: 14 additions & 23 deletions src/app/clusters/scenes/SceneTable.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
#include <app/clusters/scenes/ExtensionFieldSets.h>
#include <lib/support/CHIPMemString.h>
#include <lib/support/CommonIterator.h>
#include <lib/support/CommonPersistentData.h>
#include <lib/support/PersistentData.h>
#include <lib/support/Span.h>

namespace chip {
Expand All @@ -30,8 +30,7 @@ namespace scenes {
typedef uint8_t SceneIndex;

typedef uint32_t TransitionTimeMs;
typedef uint16_t SceneTransitionTime;
typedef uint8_t TransitionTime100ms;
typedef uint32_t SceneTransitionTime;

constexpr GroupId kGlobalGroupSceneId = 0x0000;
constexpr SceneIndex kUndefinedSceneIndex = 0xff;
Expand All @@ -42,7 +41,7 @@ static constexpr size_t kSceneNameMax = CHIP_CONFIG_SCENES_CLUSTER_MAXIMUM

// Handler's are meant to retrieve a single cluster's extension field set, therefore the maximum number allowed is the maximal
// number of extension field set.
static constexpr uint8_t kMaxSceneHandlers = CHIP_CONFIG_SCENES_MAX_CLUSTERS_PER_SCENE;
static constexpr uint8_t kMaxSceneHandlers = CHIP_CONFIG_SCENES_MAX_SCENE_HANDLERS;

/// @brief Abstract class allowing different Endpoints interactions with the ExtensionFieldSets added to and retrieved from the
/// scene Table. The Scene Handler's are meant as interface between various clusters and the Scene table. The expected behaviour of
Expand Down Expand Up @@ -156,27 +155,22 @@ class SceneTable
/// mSceneTransitionTimeSeconds in enhanced scene commands
struct SceneData
{
char mName[kSceneNameMax] = { 0 };
size_t mNameLength = 0;
SceneTransitionTime mSceneTransitionTimeSeconds = 0;
char mName[kSceneNameMax] = { 0 };
size_t mNameLength = 0;
SceneTransitionTime mSceneTransitionTimeMs = 0;
EFStype mExtensionFieldSets;
TransitionTime100ms mTransitionTime100ms = 0;

SceneData(const CharSpan & sceneName = CharSpan(), SceneTransitionTime time = 0, TransitionTime100ms time100ms = 0) :
mSceneTransitionTimeSeconds(time), mTransitionTime100ms(time100ms)
SceneData(const CharSpan & sceneName = CharSpan(), SceneTransitionTime time = 0) : mSceneTransitionTimeMs(time)
{
this->SetName(sceneName);
}
SceneData(EFStype fields, const CharSpan & sceneName = CharSpan(), SceneTransitionTime time = 0,
TransitionTime100ms time100ms = 0) :
mSceneTransitionTimeSeconds(time),
mTransitionTime100ms(time100ms)
SceneData(EFStype fields, const CharSpan & sceneName = CharSpan(), SceneTransitionTime time = 0) :
mSceneTransitionTimeMs(time)
{
this->SetName(sceneName);
mExtensionFieldSets = fields;
}
SceneData(const SceneData & other) :
mSceneTransitionTimeSeconds(other.mSceneTransitionTimeSeconds), mTransitionTime100ms(other.mTransitionTime100ms)
SceneData(const SceneData & other) : mSceneTransitionTimeMs(other.mSceneTransitionTimeMs)
{
this->SetName(CharSpan(other.mName, other.mNameLength));
mExtensionFieldSets = other.mExtensionFieldSets;
Expand All @@ -201,25 +195,22 @@ class SceneTable
void Clear()
{
this->SetName(CharSpan());
mSceneTransitionTimeSeconds = 0;
mTransitionTime100ms = 0;
mSceneTransitionTimeMs = 0;
mExtensionFieldSets.Clear();
}

bool operator==(const SceneData & other)
{
return (this->mNameLength == other.mNameLength && !memcmp(this->mName, other.mName, this->mNameLength) &&
(this->mSceneTransitionTimeSeconds == other.mSceneTransitionTimeSeconds) &&
(this->mTransitionTime100ms == other.mTransitionTime100ms) &&
(this->mSceneTransitionTimeMs == other.mSceneTransitionTimeMs) &&
(this->mExtensionFieldSets == other.mExtensionFieldSets));
}

void operator=(const SceneData & other)
{
this->SetName(CharSpan(other.mName, other.mNameLength));
this->mExtensionFieldSets = other.mExtensionFieldSets;
this->mSceneTransitionTimeSeconds = other.mSceneTransitionTimeSeconds;
this->mTransitionTime100ms = other.mTransitionTime100ms;
this->mExtensionFieldSets = other.mExtensionFieldSets;
this->mSceneTransitionTimeMs = other.mSceneTransitionTimeMs;
}
};

Expand Down
129 changes: 10 additions & 119 deletions src/app/clusters/scenes/SceneTableImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,24 +28,12 @@ namespace scenes {
enum class TagSceneImpl : uint8_t
{
kSceneCount = 1,
kNextFabric,
};

using SceneTableEntry = DefaultSceneTableImpl::SceneTableEntry;
using SceneStorageId = DefaultSceneTableImpl::SceneStorageId;
using SceneData = DefaultSceneTableImpl::SceneData;

struct FabricHavingSceneList : public CommonPersistentData::FabricList
{
// This implementation uses the same key as the GroupFabricList from GroupDataProviderImpl to avoid duplicating the list in
// memory. If a different GroupDataProvider implementation is used, it will create the list in flash memory.
CHIP_ERROR UpdateKey(StorageKeyName & key) override
{
key = DefaultStorageKeyAllocator::GroupFabricList();
return CHIP_NO_ERROR;
}
};

// Worst case tested: Add Scene Command with EFS using the default SerializeAdd Method. This yielded a serialized scene of 212bytes
// when using the OnOff, Level Control and Color Control as well as the maximal name length of 16 bytes. Putting 256 gives some
// slack in case different clusters are used. Value obtained by using writer.GetLengthWritten at the end of the SceneTableData
Expand Down Expand Up @@ -93,8 +81,7 @@ struct SceneTableData : public SceneTableEntry, PersistentData<kPersistentSceneB
ReturnErrorOnFailure(writer.PutString(TLV::ContextTag(TagScene::kName), nameSpan));
}

ReturnErrorOnFailure(writer.Put(TLV::ContextTag(TagScene::kTransitionTime), mStorageData.mSceneTransitionTimeSeconds));
ReturnErrorOnFailure(writer.Put(TLV::ContextTag(TagScene::kTransitionTime100), mStorageData.mTransitionTime100ms));
ReturnErrorOnFailure(writer.Put(TLV::ContextTag(TagScene::kTransitionTimeMs), mStorageData.mSceneTransitionTimeMs));
ReturnErrorOnFailure(mStorageData.mExtensionFieldSets.Serialize(writer));

return writer.EndContainer(container);
Expand Down Expand Up @@ -122,20 +109,18 @@ struct SceneTableData : public SceneTableEntry, PersistentData<kPersistentSceneB
// Scene Data
ReturnErrorOnFailure(reader.Next());
TLV::Tag currTag = reader.GetTag();
VerifyOrReturnError(TLV::ContextTag(TagScene::kName) == currTag || TLV::ContextTag(TagScene::kTransitionTime) == currTag,
VerifyOrReturnError(TLV::ContextTag(TagScene::kName) == currTag || TLV::ContextTag(TagScene::kTransitionTimeMs) == currTag,
CHIP_ERROR_WRONG_TLV_TYPE);

// A name may or may not have been stored. Check whether it was.
if (currTag == TLV::ContextTag(TagScene::kName))
{
ReturnErrorOnFailure(reader.Get(nameSpan));
mStorageData.SetName(nameSpan);
ReturnErrorOnFailure(reader.Next(TLV::ContextTag(TagScene::kTransitionTime)));
ReturnErrorOnFailure(reader.Next(TLV::ContextTag(TagScene::kTransitionTimeMs)));
}

ReturnErrorOnFailure(reader.Get(mStorageData.mSceneTransitionTimeSeconds));
ReturnErrorOnFailure(reader.Next(TLV::ContextTag(TagScene::kTransitionTime100)));
ReturnErrorOnFailure(reader.Get(mStorageData.mTransitionTime100ms));
ReturnErrorOnFailure(reader.Get(mStorageData.mSceneTransitionTimeMs));
ReturnErrorOnFailure(mStorageData.mExtensionFieldSets.Deserialize(reader));

return reader.ExitContainer(container);
Expand All @@ -156,7 +141,6 @@ struct FabricSceneData : public PersistentData<kPersistentFabricBufferMax>
FabricIndex fabric_index = kUndefinedFabricIndex;
uint8_t scene_count = 0;
SceneStorageId scene_map[kMaxScenePerFabric];
FabricIndex next = kUndefinedFabricIndex;

FabricSceneData() = default;
FabricSceneData(FabricIndex fabric) : fabric_index(fabric) {}
Expand All @@ -175,7 +159,6 @@ struct FabricSceneData : public PersistentData<kPersistentFabricBufferMax>
{
scene_map[i].Clear();
}
next = kUndefinedFabricIndex;
}

CHIP_ERROR Serialize(TLV::TLVWriter & writer) const override
Expand All @@ -184,7 +167,6 @@ struct FabricSceneData : public PersistentData<kPersistentFabricBufferMax>
ReturnErrorOnFailure(writer.StartContainer(TLV::AnonymousTag(), TLV::kTLVType_Structure, container));

ReturnErrorOnFailure(writer.Put(TLV::ContextTag(TagSceneImpl::kSceneCount), (scene_count)));
ReturnErrorOnFailure(writer.Put(TLV::ContextTag(TagSceneImpl::kNextFabric), (next)));
// Storing the scene map
for (uint8_t i = 0; i < kMaxScenePerFabric; i++)
{
Expand All @@ -205,8 +187,6 @@ struct FabricSceneData : public PersistentData<kPersistentFabricBufferMax>

ReturnErrorOnFailure(reader.Next(TLV::ContextTag(TagSceneImpl::kSceneCount)));
ReturnErrorOnFailure(reader.Get(scene_count));
ReturnErrorOnFailure(reader.Next(TLV::ContextTag(TagSceneImpl::kNextFabric)));
ReturnErrorOnFailure(reader.Get(next));

uint8_t i = 0;
while (reader.Next(TLV::ContextTag(TagScene::kEndpointID)) != CHIP_END_OF_TLV)
Expand All @@ -223,86 +203,6 @@ struct FabricSceneData : public PersistentData<kPersistentFabricBufferMax>
return reader.ExitContainer(container);
}

// Register the fabric in the list of fabrics having scenes
CHIP_ERROR Register(PersistentStorageDelegate * storage)
{
FabricHavingSceneList fabric_list;
CHIP_ERROR err = fabric_list.Load(storage);
if (CHIP_ERROR_NOT_FOUND == err)
{
// New fabric list
fabric_list.first_entry = fabric_index;
fabric_list.entry_count = 1;
return fabric_list.Save(storage);
}
ReturnErrorOnFailure(err);

// Existing fabric list, search for existing entry
FabricSceneData fabric(fabric_list.first_entry);
for (size_t i = 0; i < fabric_list.entry_count; i++)
{
err = fabric.Load(storage);
if (CHIP_NO_ERROR != err)
{
break;
}
if (fabric.fabric_index == this->fabric_index)
{
// Fabric already registered
return CHIP_NO_ERROR;
}
fabric.fabric_index = fabric.next;
}
// Add this fabric to the fabric list
this->next = fabric_list.first_entry;
fabric_list.first_entry = this->fabric_index;
fabric_list.entry_count++;
return fabric_list.Save(storage);
}

// Remove the fabric from the fabrics' linked list
CHIP_ERROR Unregister(PersistentStorageDelegate * storage) const
{
FabricHavingSceneList fabric_list;
CHIP_ERROR err = fabric_list.Load(storage);
VerifyOrReturnValue(CHIP_ERROR_NOT_FOUND != err, CHIP_NO_ERROR);
ReturnErrorOnFailure(err);

// Existing fabric list, search for existing entry
FabricSceneData fabric(fabric_list.first_entry);
FabricSceneData prev;

for (size_t i = 0; i < fabric_list.entry_count; i++)
{
err = fabric.Load(storage);
if (CHIP_NO_ERROR != err)
{
break;
}
if (fabric.fabric_index == this->fabric_index)
{
// Fabric found
if (i == 0)
{
// Remove first fabric
fabric_list.first_entry = this->next;
}
else
{
// Remove intermediate fabric
prev.next = this->next;
ReturnErrorOnFailure(prev.Save(storage));
}
VerifyOrReturnError(fabric_list.entry_count > 0, CHIP_ERROR_INTERNAL);
fabric_list.entry_count--;
return fabric_list.Save(storage);
}
prev = fabric;
fabric.fabric_index = fabric.next;
}
// Fabric not in the list
return CHIP_ERROR_NOT_FOUND;
}
/// @brief Finds the index where to insert current scene by going through the whole table and looking if the scene is already in
/// there. If the target is not in the table, sets idx to the first empty space
/// @param target_scene Storage Id of scene to store
Expand Down Expand Up @@ -336,17 +236,6 @@ struct FabricSceneData : public PersistentData<kPersistentFabricBufferMax>

return CHIP_ERROR_NO_MEMORY;
}
CHIP_ERROR Save(PersistentStorageDelegate * storage) override
{
ReturnErrorOnFailure(Register(storage));
return PersistentData::Save(storage);
}

CHIP_ERROR Delete(PersistentStorageDelegate * storage) override
{
ReturnErrorOnFailure(Unregister(storage));
return PersistentData::Delete(storage);
}

CHIP_ERROR SaveScene(PersistentStorageDelegate * storage, const SceneTableEntry & entry)
{
Expand Down Expand Up @@ -538,7 +427,10 @@ CHIP_ERROR DefaultSceneTableImpl::UnregisterHandler(SceneHandler * handler)

// TODO: Implement general array management methods
// Compress array after removal
memmove(&this->mHandlers[position], &this->mHandlers[nextPos], sizeof(this->mHandlers[position]) * moveNum);
if (moveNum)
{
memmove(&this->mHandlers[position], &this->mHandlers[nextPos], sizeof(this->mHandlers[position]) * moveNum);
}

this->mNumHandlers--;
// Clear last occupied position
Expand Down Expand Up @@ -602,9 +494,8 @@ CHIP_ERROR DefaultSceneTableImpl::SceneApplyEFS(FabricIndex fabric_index, const
for (uint8_t i = 0; i < scene.mStorageData.mExtensionFieldSets.GetFieldSetCount(); i++)
{
scene.mStorageData.mExtensionFieldSets.GetFieldSetAtPosition(EFS, i);
cluster = EFS.mID;
time = scene.mStorageData.mSceneTransitionTimeSeconds * 1000 +
(scene.mStorageData.mTransitionTime100ms ? scene.mStorageData.mTransitionTime100ms * 10 : 0);
cluster = EFS.mID;
time = scene.mStorageData.mSceneTransitionTimeMs;
ByteSpan EFSSpan = MutableByteSpan(EFS.mBytesBuffer, EFS.mUsedBytes);

if (!EFS.IsEmpty())
Expand Down
6 changes: 2 additions & 4 deletions src/app/clusters/scenes/SceneTableImpl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,14 @@ namespace scenes {
/// kGroupID: Tag for GroupID if the Scene is a Group Scene
/// kID: Tag for the scene ID together with the two previous tag, forms the SceneStorageID
/// kName: Tag for the name of the scene
/// kTransitionTime: Tag for the transition time of the scene in seconds
/// kTransitionTime100: Tag for the transition time of the scene in tenth of a second (enhanced scenes)
/// kTransitionTime: Tag for the transition time of the scene in miliseconds
enum class TagScene : uint8_t
{
kEndpointID = 1,
kGroupID,
kID,
kName,
kTransitionTime,
kTransitionTime100,
kTransitionTimeMs,
};

using clusterId = chip::ClusterId;
Expand Down
Loading

0 comments on commit 2130848

Please sign in to comment.