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.

Sign up for GitHub

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

[SILABS] lock storage refactor #37849

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

[SILABS] lock storage refactor #37849

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
7fb40ab
lock storage refactor
mykrupp Mar 3, 2025
4309778
restyle
mykrupp Mar 3, 2025
d864edb
mykrupp Mar 3, 2025
29a9f25
mykrupp Mar 3, 2025
158d766
segement credentials by credential type
mykrupp Mar 5, 2025
dd3a8e5
add endpoint verification to each function
mykrupp Mar 6, 2025
18ad77e
check for userInStorage.currentCredentialCount <= kMaxCredentialsPerUser
mykrupp Mar 6, 2025
7bd0669
move weekDayScheduleInStorage yearDayScheduleInStorage holidaySchedul…
mykrupp Mar 6, 2025
9c0e2c1
swap out CopyString with memcpy using src buffer size
mykrupp Mar 10, 2025
64e96bc
inject PersistentStorageDelegate * storage into LockManager
mykrupp Mar 11, 2025
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
72 changes: 60 additions & 12 deletions examples/lock-app/silabs/include/LockManager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@
#include <cmsis_os2.h>
#include <lib/core/CHIPError.h>

struct WeekDaysScheduleInfo
#include <lib/support/DefaultStorageKeyAllocator.h>

struct WeekDayScheduleInfo
{
DlScheduleStatus status;
EmberAfPluginDoorLockWeekDaySchedule schedule;
Expand Down Expand Up @@ -112,6 +114,30 @@ class ParamBuilder
using namespace ::chip;
using namespace EFR32DoorLock::ResourceRanges;

struct LockUserInfo
{
char userName[DOOR_LOCK_USER_NAME_BUFFER_SIZE];
size_t userNameSize;
uint32_t userUniqueId;
UserStatusEnum userStatus;
UserTypeEnum userType;
CredentialRuleEnum credentialRule;
chip::EndpointId endpointId;
chip::FabricIndex createdBy;
chip::FabricIndex lastModifiedBy;
uint16_t currentCredentialCount;
};

struct LockCredentialInfo
{
DlCredentialStatus status;
CredentialTypeEnum credentialType;
chip::FabricIndex createdBy;
chip::FabricIndex lastModifiedBy;
uint8_t credentialData[kMaxCredentialSize];
size_t credentialDataSize;
};

class LockManager
{
public:
Expand Down Expand Up @@ -192,8 +218,6 @@ class LockManager
OperationErrorEnum & err);
const char * lockStateToString(DlLockState lockState) const;

bool ReadConfigValues();

void UnlockAfterUnlatch();

private:
Expand Down Expand Up @@ -244,17 +268,41 @@ class LockManager
static void ActuatorMovementTimerEventHandler(AppEvent * aEvent);

osTimerId_t mLockTimer;
EmberAfPluginDoorLockUserInfo mLockUsers[kMaxUsers];
EmberAfPluginDoorLockCredentialInfo mLockCredentials[kNumCredentialTypes][kMaxCredentials];
WeekDaysScheduleInfo mWeekdaySchedule[kMaxUsers][kMaxWeekdaySchedulesPerUser];
YearDayScheduleInfo mYeardaySchedule[kMaxUsers][kMaxYeardaySchedulesPerUser];
HolidayScheduleInfo mHolidaySchedule[kMaxHolidaySchedules];

char mUserNames[MATTER_ARRAY_SIZE(mLockUsers)][DOOR_LOCK_MAX_USER_NAME_SIZE];
uint8_t mCredentialData[kNumCredentialTypes][kMaxCredentials][kMaxCredentialSize];
CredentialStruct mCredentials[kMaxUsers][kMaxCredentials];

EFR32DoorLock::LockInitParams::LockParam LockParams;

static StorageKeyName LockUserEndpoint(uint16_t userIndex, chip::EndpointId endpoint)
{
return StorageKeyName::Formatted("g/lu/%x/e/%x", userIndex, endpoint);
}
static StorageKeyName LockCredentialEndpoint(uint16_t credentialIndex, chip::EndpointId endpoint)
{
return StorageKeyName::Formatted("g/lc/%x/e/%x", credentialIndex, endpoint);
}
static StorageKeyName LockUserCredentialMap(uint16_t userIndex)
{
return StorageKeyName::Formatted("g/lu/%x/lc", userIndex);
} // Stores all the credential indices that belong to a user
static StorageKeyName LockUserWeekDayScheduleEndpoint(uint16_t userIndex, uint16_t scheduleIndex, chip::EndpointId endpoint)
{
return StorageKeyName::Formatted("g/lu/%x/lw/%x/e/%x", userIndex, scheduleIndex, endpoint);
}
static StorageKeyName LockUserYearDayScheduleEndpoint(uint16_t userIndex, uint16_t scheduleIndex, chip::EndpointId endpoint)
{
return StorageKeyName::Formatted("g/lu/%x/ly/%x/e/%x", userIndex, scheduleIndex, endpoint);
}
static StorageKeyName LockHolidayScheduleEndpoint(uint16_t scheduleIndex, chip::EndpointId endpoint)
{
return StorageKeyName::Formatted("g/lh/%x/e/%x", scheduleIndex, endpoint);
}

LockUserInfo userInStorage;
LockCredentialInfo credentialInStorage;
WeekDayScheduleInfo weekDayScheduleInStorage;
YearDayScheduleInfo yearDayScheduleInStorage;
HolidayScheduleInfo holidayScheduleInStorage;
CredentialStruct mCredential;
CredentialStruct mCredentials[kMaxCredentialsPerUser];
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the intended purpose of having these variables as members of the class rather than having them as local variables in the members that uses them?
As it is now, the memory for all of these will always be allocated with the static LockManager, whereas if they would be declared in the members where they are used, the memory would only be allocated on the stack when they are needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will move the schedule storage to local variables in schedule functions, but the user/credential items return spans which need consistent storage somewhere.

This could be revisited if we decide to change EmberAfPluginDoorLockCredentialInfo and EmberAfPluginDoorLockUserInfo to have their own storage

Copy link
Contributor

Choose a reason for hiding this comment

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

We can, but I think this will force us to rely on shared pointers in the meantime to ensure the caller of methods where we pass a pointer to our buffers will be done with the buffers when the next call happens.

This runs the risk of being equally tedious if we want to ensure the memory access is safe.

};

LockManager & LockMgr();
3 changes: 0 additions & 3 deletions examples/lock-app/silabs/src/AppTask.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,9 +266,6 @@ void AppTask::AppTaskMain(void * pvParameter)

SILABS_LOG("App Task started");

// Users and credentials should be checked once from nvm flash on boot
LockMgr().ReadConfigValues();

while (true)
{
osStatus_t eventReceived = osMessageQueueGet(sAppEventQueue, &event, NULL, osWaitForever);
Expand Down
Loading
Loading