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

Unify error code for Persistent Storage Delegate #15671

Closed
jepenven-silabs opened this issue Mar 1, 2022 · 1 comment · Fixed by #15675
Closed

Unify error code for Persistent Storage Delegate #15671

jepenven-silabs opened this issue Mar 1, 2022 · 1 comment · Fixed by #15675

Comments

@jepenven-silabs
Copy link
Contributor

Problem

When a value is not found two possible error code can be returned depending on implementation type :
CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND and CHIP_ERROR_KEY_NOT_FOUND

Proposed Solution

Unify the API so that the same error code is return regardless of implementation.

@bzbarsky-apple
Copy link
Contributor

So in particular, src/include/platform/KeyValueStoreManager.h documents that CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND is used to represent "no value for this key" (for Get and Delete).

So does src/include/platform/PersistedStorage.h (for Read).

But src/lib/core/CHIPPersistentStorageDelegate.h does not document what error codes should be used for SyncGetKeyValue or SyncDeleteKeyValue.

Looking at the implementations, I see the following behavior for SyncGetKeyValue:

  • TestCASESessionPersistentStorageDelegate: returns CHIP_ERROR_INTERNAL
  • TestPersistentStorageDelegate: returns CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND
  • PersistentStorage (in examples/chip-tool/config/PersistentStorage.h): returns CHIP_ERROR_KEY_NOT_FOUND
  • MyServerStorageDelegate (in examples/platform/linux/AppMain.cpp): calls through to KeyValueStoreMgr. Does not handle size inout param correctly...
  • DeviceStorageDelegate: calls through to KeyValueStoreMgr
  • AndroidDeviceControllerWrapper: Calls through to KeyValueStoreMgr
  • PythonPersistentStorageDelegate: returns CHIP_ERROR_KEY_NOT_FOUND
  • StorageAdapter (in python code): returns CHIP_ERROR_KEY_NOT_FOUND
  • ServerStorageDelegate (in python code): calls through to KeyValueStoreMgr. Does not handle size inout param correctly...
  • CHIPPersistentStorageDelegateBridge: returns CHIP_ERROR_KEY_NOT_FOUND

The implementations are also not consistent in terms of what they return when the buffer is too small.

We should align all of these with KeyValueStoreManager so calling through to that is sane.

bzbarsky-apple added a commit to bzbarsky-apple/connectedhomeip that referenced this issue Mar 1, 2022
bzbarsky-apple added a commit to bzbarsky-apple/connectedhomeip that referenced this issue Mar 1, 2022
woody-apple pushed a commit to bzbarsky-apple/connectedhomeip that referenced this issue Mar 1, 2022
bzbarsky-apple added a commit to bzbarsky-apple/connectedhomeip that referenced this issue Mar 1, 2022
woody-apple pushed a commit to bzbarsky-apple/connectedhomeip that referenced this issue Mar 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants