Skip to content

Commit

Permalink
Merge branch 'master' into feature/app-install-flow-public
Browse files Browse the repository at this point in the history
  • Loading branch information
lazarkov authored Jul 15, 2024
2 parents 3f949ef + 05e4c10 commit 6f340c5
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 35 deletions.
27 changes: 16 additions & 11 deletions src/app/DefaultAttributePersistenceProvider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,34 +37,40 @@ CHIP_ERROR DefaultAttributePersistenceProvider::InternalWriteValue(const Storage
return mStorage->SyncSetKeyValue(aKey.KeyName(), aValue.data(), static_cast<uint16_t>(aValue.size()));
}

CHIP_ERROR DefaultAttributePersistenceProvider::InternalReadValue(const StorageKeyName & aKey, EmberAfAttributeType aType,
size_t aSize, MutableByteSpan & aValue)
CHIP_ERROR DefaultAttributePersistenceProvider::InternalReadValue(const StorageKeyName & aKey, MutableByteSpan & aValue)
{
VerifyOrReturnError(mStorage != nullptr, CHIP_ERROR_INCORRECT_STATE);

uint16_t size = static_cast<uint16_t>(min(aValue.size(), static_cast<size_t>(UINT16_MAX)));
ReturnErrorOnFailure(mStorage->SyncGetKeyValue(aKey.KeyName(), aValue.data(), size));
EmberAfAttributeType type = aType;
if (emberAfIsStringAttributeType(type))
aValue.reduce_size(size);
return CHIP_NO_ERROR;
}

CHIP_ERROR DefaultAttributePersistenceProvider::InternalReadValue(const StorageKeyName & aKey, EmberAfAttributeType aType,
size_t aExpectedSize, MutableByteSpan & aValue)
{
ReturnErrorOnFailure(InternalReadValue(aKey, aValue));
size_t size = aValue.size();
if (emberAfIsStringAttributeType(aType))
{
// Ensure that we've read enough bytes that we are not ending up with
// un-initialized memory. Should have read length + 1 (for the length
// byte).
VerifyOrReturnError(size >= emberAfStringLength(aValue.data()) + 1, CHIP_ERROR_INCORRECT_STATE);
VerifyOrReturnError(size >= 1 && size - 1 >= emberAfStringLength(aValue.data()), CHIP_ERROR_INCORRECT_STATE);
}
else if (emberAfIsLongStringAttributeType(type))
else if (emberAfIsLongStringAttributeType(aType))
{
// Ensure that we've read enough bytes that we are not ending up with
// un-initialized memory. Should have read length + 2 (for the length
// bytes).
VerifyOrReturnError(size >= emberAfLongStringLength(aValue.data()) + 2, CHIP_ERROR_INCORRECT_STATE);
VerifyOrReturnError(size >= 2 && size - 2 >= emberAfLongStringLength(aValue.data()), CHIP_ERROR_INCORRECT_STATE);
}
else
{
// Ensure we got the expected number of bytes for all other types.
VerifyOrReturnError(size == aSize, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(size == aExpectedSize, CHIP_ERROR_INVALID_ARGUMENT);
}
aValue.reduce_size(size);
return CHIP_NO_ERROR;
}

Expand All @@ -90,8 +96,7 @@ CHIP_ERROR DefaultAttributePersistenceProvider::SafeWriteValue(const ConcreteAtt
CHIP_ERROR DefaultAttributePersistenceProvider::SafeReadValue(const ConcreteAttributePath & aPath, MutableByteSpan & aValue)
{
return InternalReadValue(
DefaultStorageKeyAllocator::SafeAttributeValue(aPath.mEndpointId, aPath.mClusterId, aPath.mAttributeId), 0x20,
aValue.size(), aValue);
DefaultStorageKeyAllocator::SafeAttributeValue(aPath.mEndpointId, aPath.mClusterId, aPath.mAttributeId), aValue);
}

namespace {
Expand Down
4 changes: 3 additions & 1 deletion src/app/DefaultAttributePersistenceProvider.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,9 @@ class DefaultAttributePersistenceProvider : public AttributePersistenceProvider,

private:
CHIP_ERROR InternalWriteValue(const StorageKeyName & aKey, const ByteSpan & aValue);
CHIP_ERROR InternalReadValue(const StorageKeyName & aKey, EmberAfAttributeType aType, size_t aSize, MutableByteSpan & aValue);
CHIP_ERROR InternalReadValue(const StorageKeyName & aKey, MutableByteSpan & aValue);
CHIP_ERROR InternalReadValue(const StorageKeyName & aKey, EmberAfAttributeType aType, size_t aExpectedSize,
MutableByteSpan & aValue);
};

} // namespace app
Expand Down
40 changes: 18 additions & 22 deletions src/app/SafeAttributePersistenceProvider.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ class SafeAttributePersistenceProvider

// The following API provides helper functions to simplify the access of commonly used types.
// The API may not be complete.
// Currently implemented write and read types are: uint8_t, uint16_t, uint32_t, unit64_t and
// their nullable varieties, and bool.
// Currently implemented write and read types are: bool, uint8_t, uint16_t, uint32_t, unit64_t and
// their nullable varieties, as well as ByteSpans.

/**
* Write an attribute value of type intX, uintX or bool to non-volatile memory.
Expand All @@ -50,7 +50,7 @@ class SafeAttributePersistenceProvider
* @param [in] aValue the data to write.
*/
template <typename T, std::enable_if_t<std::is_integral<T>::value, bool> = true>
CHIP_ERROR WriteScalarValue(const ConcreteAttributePath & aPath, T & aValue)
CHIP_ERROR WriteScalarValue(const ConcreteAttributePath & aPath, T aValue)
{
uint8_t value[sizeof(T)];
auto w = Encoding::LittleEndian::BufferWriter(value, sizeof(T));
Expand All @@ -71,18 +71,16 @@ class SafeAttributePersistenceProvider
*
* @param [in] aPath the attribute path for the data being persisted.
* @param [in,out] aValue where to place the data.
*
* @retval CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND if no stored value exists for the attribute
*/
template <typename T, std::enable_if_t<std::is_integral<T>::value, bool> = true>
CHIP_ERROR ReadScalarValue(const ConcreteAttributePath & aPath, T & aValue)
{
uint8_t attrData[sizeof(T)];
MutableByteSpan tempVal(attrData);
auto err = SafeReadValue(aPath, tempVal);
if (err != CHIP_NO_ERROR)
{
return err;
}

ReturnErrorOnFailure(SafeReadValue(aPath, tempVal));
VerifyOrReturnError(tempVal.size() == sizeof(T), CHIP_ERROR_INCORRECT_STATE);
Encoding::LittleEndian::Reader r(tempVal.data(), tempVal.size());
r.RawReadLowLevelBeCareful(&aValue);
return r.StatusCode();
Expand All @@ -95,7 +93,7 @@ class SafeAttributePersistenceProvider
* @param [in] aValue the data to write.
*/
template <typename T, std::enable_if_t<std::is_integral<T>::value, bool> = true>
CHIP_ERROR WriteScalarValue(const ConcreteAttributePath & aPath, DataModel::Nullable<T> & aValue)
CHIP_ERROR WriteScalarValue(const ConcreteAttributePath & aPath, const DataModel::Nullable<T> & aValue)
{
typename NumericAttributeTraits<T>::StorageType storageValue;
if (aValue.IsNull())
Expand All @@ -114,16 +112,14 @@ class SafeAttributePersistenceProvider
*
* @param [in] aPath the attribute path for the data being persisted.
* @param [in,out] aValue where to place the data.
*
* @retval CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND if no stored value exists for the attribute
*/
template <typename T, std::enable_if_t<std::is_integral<T>::value, bool> = true>
CHIP_ERROR ReadScalarValue(const ConcreteAttributePath & aPath, DataModel::Nullable<T> & aValue)
{
typename NumericAttributeTraits<T>::StorageType storageValue;
CHIP_ERROR err = ReadScalarValue(aPath, storageValue);
if (err != CHIP_NO_ERROR)
{
return err;
}
ReturnErrorOnFailure(ReadScalarValue(aPath, storageValue));

if (NumericAttributeTraits<T>::IsNullValue(storageValue))
{
Expand All @@ -137,25 +133,25 @@ class SafeAttributePersistenceProvider
return CHIP_NO_ERROR;
}

protected:
/**
* Write an attribute value from the attribute store (i.e. not a struct or
* list) to non-volatile memory.
*
* @param [in] aPath the attribute path for the data being written.
* @param [in] aValue the data to write. The value should be stored as-is.
* @param [in] aValue the data to write. The value will be stored as-is.
*/
virtual CHIP_ERROR SafeWriteValue(const ConcreteAttributePath & aPath, const ByteSpan & aValue) = 0;

/**
* Read an attribute value from non-volatile memory.
* It can be assumed that this method will never be called upon to read
* an attribute of type string or long-string.
* Read an attribute value as a raw sequence of bytes from non-volatile memory.
*
* @param [in] aPath the attribute path for the data being persisted.
* @param [in,out] aValue where to place the data. The size of the buffer
* will be equal to `aValue.size()`. The callee is expected to adjust
* aValue's size to the actual number of bytes read.
* will be equal to `aValue.size()`. On success aValue.size()
* will be the actual number of bytes read.
*
* @retval CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND if no stored value exists for the attribute
* @retval CHIP_ERROR_BUFFER_TOO_SMALL aValue.size() is too small to hold the value.
*/
virtual CHIP_ERROR SafeReadValue(const ConcreteAttributePath & aPath, MutableByteSpan & aValue) = 0;
};
Expand Down
9 changes: 8 additions & 1 deletion src/app/tests/TestAttributePersistenceProvider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,13 @@ TEST_F(TestAttributePersistenceProvider, TestStorageAndRetrivalByteSpans)
MutableByteSpan valueReadBack(getArray);
err = persistenceProvider.SafeReadValue(TestConcretePath, valueReadBack);
EXPECT_EQ(err, CHIP_NO_ERROR);
EXPECT_TRUE(std::equal(valueReadBack.begin(), valueReadBack.end(), value.begin(), value.end()));
EXPECT_TRUE(valueReadBack.data_equal(value));

uint8_t getArrayThatIsLongerThanNeeded[10];
MutableByteSpan valueReadBack2(getArrayThatIsLongerThanNeeded);
err = persistenceProvider.SafeReadValue(TestConcretePath, valueReadBack2);
EXPECT_EQ(err, CHIP_NO_ERROR);
EXPECT_TRUE(valueReadBack2.data_equal(value));

// Finishing
persistenceProvider.Shutdown();
Expand Down Expand Up @@ -320,6 +326,7 @@ TEST_F(TestAttributePersistenceProvider, TestBufferTooSmallErrors)
err = persistenceProvider.SafeReadValue(TestConcretePath, valueReadBackByteSpan8);
EXPECT_EQ(err, CHIP_ERROR_BUFFER_TOO_SMALL);

// TODO: ReadScalarValue() does not take a buffer, so expecting CHIP_ERROR_BUFFER_TOO_SMALL is bad API
// Fail to get value as uint8_t
uint8_t valueReadBack8;
err = persistenceProvider.ReadScalarValue(TestConcretePath, valueReadBack8);
Expand Down

0 comments on commit 6f340c5

Please sign in to comment.