Skip to content

Commit

Permalink
Fix Accessors to handle nullable attributes.
Browse files Browse the repository at this point in the history
For setters, the new behavior is:

* For non-string attributes when setting an integer or boolean value,
  do a CanRepresentValue check and fail if that fails.  For string
  attributes, ensure that strings with 0xFF or 0xFFFF as length cannot
  be passed in in practice.

* For nullable attributes add a SetNull method.

* For nullable attributes add a Set() method taking a Nullable<T> and
  calling either SetNull or the setter that expects a non-null value.

For getters, the new behavior is:

* When reading a nullable integer or boolean, NaU/NaS is converted to
  a null value stored in the Nullable.

* When reading a non-nullable integer or boolean, return error if
  CanRepresentValue returns false on the value from the attr store.

* When reading a nullable string or octet string, length 0xFFFF/0xFF
  is converted to a null value stored in the Nullable.

* When reading a non-nullable string or octet string, length
  0xFFFF/0xFF leads to an error.
  • Loading branch information
bzbarsky-apple committed Nov 11, 2021
1 parent bea4de1 commit 07564e8
Show file tree
Hide file tree
Showing 5 changed files with 12,330 additions and 1,107 deletions.
38 changes: 34 additions & 4 deletions src/app/zap-templates/common/attributes/Accessors.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@
* limitations under the License.
*/

const zapPath = '../../../../../third_party/zap/repo/dist/src-electron/';
const ListHelper = require('../../common/ListHelper.js');
const StringHelper = require('../../common/StringHelper.js');
const StructHelper = require('../../common/StructHelper.js');
const zclHelper = require(zapPath + 'generator/helper-zcl.js')

// Issue #8202
// The specification allow non-standard signed and unsigned integer with a width of 24, 40, 48 or 56, but those types does not have
Expand All @@ -29,24 +31,52 @@ function isUnsupportedType(type)
return unsupportedTypes.includes(type.toUpperCase());
}

function canHaveSimpleAccessors(type)
function canHaveSimpleAccessors(attr)
{
if (ListHelper.isList(type)) {
if (attr.isArray || attr.isList) {
return false;
}

if (StructHelper.isStruct(type)) {
if (ListHelper.isList(attr.type)) {
return false;
}

if (isUnsupportedType(type)) {
if (StructHelper.isStruct(attr.type)) {
return false;
}

if (isUnsupportedType(attr.type)) {
return false;
}

return true;
}

async function accessorGetterType(attr)
{
let type;
let mayNeedPointer = false;
if (StringHelper.isCharString(attr.type)) {
type = "chip::MutableCharSpan";
} else if (StringHelper.isOctetString(attr.type)) {
type = "chip::MutableByteSpan";
} else {
mayNeedPointer = true;
const options = { 'hash' : {} };
type = await zclHelper.asUnderlyingZclType.call(this, attr.type, options);
}

if (attr.isNullable) {
type = `DataModel::Nullable<${type}> &`;
} else if (mayNeedPointer) {
type = `${type} *`;
}

return type;
}

//
// Module exports
//
exports.canHaveSimpleAccessors = canHaveSimpleAccessors;
exports.accessorGetterType = accessorGetterType;
82 changes: 73 additions & 9 deletions src/app/zap-templates/templates/app/attributes/Accessors-src.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <app-common/zap-generated/ids/Attributes.h>
#include <app-common/zap-generated/ids/Clusters.h>
#include <app/util/af.h>
#include <app/util/attribute-storage-helpers.h>

namespace chip {
namespace app {
Expand All @@ -25,39 +26,102 @@ namespace Attributes {

{{/first}}
{{#if clusterRef}}
{{#if (canHaveSimpleAccessors type)}}
{{#if (canHaveSimpleAccessors this)}}
namespace {{asUpperCamelCase label}} {

{{#*inline "clusterId"}}Clusters::{{asUpperCamelCase parent.label}}::Id{{/inline}}
{{#*inline "sizingBytes"}}{{#if (isShortString type)}}1{{else}}2{{/if}}{{/inline}}

EmberAfStatus Get(chip::EndpointId endpoint, {{#if (isCharString type)}}chip::MutableCharSpan{{else if (isOctetString type)}}chip::MutableByteSpan{{else}}{{asUnderlyingZclType type}} *{{/if}} value)
EmberAfStatus Get(chip::EndpointId endpoint, {{accessorGetterType this}} value)
{
{{#if (isString type)}}
VerifyOrReturnError(value.size() == {{maxLength}}, EMBER_ZCL_STATUS_INVALID_ARGUMENT);
{{~#if (isString type)}}
{{~#*inline "lengthType"}}uint{{#if (isShortString type)}}8{{else}}16{{/if}}_t{{/inline}}
uint8_t zclString[{{maxLength}} + {{>sizingBytes}}];
EmberAfStatus status = emberAfReadServerAttribute(endpoint, {{>clusterId}}, Id, zclString, sizeof(zclString));
VerifyOrReturnError(EMBER_ZCL_STATUS_SUCCESS == status, status);
memcpy(value.data(), &zclString[{{>sizingBytes}}], {{maxLength}});
value.reduce_size(emberAf{{#if (isLongString type)}}Long{{/if}}StringLength(zclString));
size_t length = emberAf{{#if (isLongString type)}}Long{{/if}}StringLength(zclString);
if (length == NumericAttributeTraits<{{>lengthType}}>::kNullValue)
{
{{#if isNullable}}
value.SetNull();
return EMBER_ZCL_STATUS_SUCCESS;
{{else}}
return EMBER_ZCL_STATUS_INVALID_VALUE;
{{/if}}
}
{{#if isNullable}}
auto & span = value.SetNonNull();
{{/if}}
{{~#*inline "value"}}{{#if isNullable}}span{{else}}value{{/if}}{{/inline}}
VerifyOrReturnError({{>value}}.size() == {{maxLength}}, EMBER_ZCL_STATUS_INVALID_ARGUMENT);
memcpy({{>value}}.data(), &zclString[{{>sizingBytes}}], {{maxLength}});
{{>value}}.reduce_size(length);
return status;
{{else}}
return emberAfReadServerAttribute(endpoint, {{>clusterId}}, Id, (uint8_t *) value, sizeof(*value));
NumericAttributeTraits<{{asUnderlyingZclType type}}>::StorageType temp;
EmberAfStatus status = emberAfReadServerAttribute(endpoint, {{>clusterId}}, Id, reinterpret_cast<uint8_t *>(&temp), sizeof(temp));
VerifyOrReturnError(EMBER_ZCL_STATUS_SUCCESS == status, status);
{{#if isNullable}}
if (NumericAttributeTraits<{{asUnderlyingZclType type}}>::IsNullValue(temp))
{
value.SetNull();
}
else
{
value.SetNonNull() = temp;
}
{{else}}
if (!NumericAttributeTraits<{{asUnderlyingZclType type}}>::CanRepresentValue(/* isNullable = */ {{isNullable}}, temp))
{
return EMBER_ZCL_STATUS_INVALID_VALUE;
}
*value = temp;
{{/if}}
return status;
{{/if}}
}
EmberAfStatus Set(chip::EndpointId endpoint, {{asUnderlyingZclType type}} value)
{
{{#if (isString type)}}
{{~#if (isString type)}}
{{~#*inline "lengthType"}}uint{{#if (isShortString type)}}8{{else}}16{{/if}}_t{{/inline}}
static_assert({{maxLength}} != NumericAttributeTraits<{{>lengthType}}>::kNullValue,
"value.size() might be too big");
VerifyOrReturnError(value.size() <= {{maxLength}}, EMBER_ZCL_STATUS_INVALID_ARGUMENT);
uint8_t zclString[{{maxLength}} + {{>sizingBytes}}];
emberAfCopyInt{{#if (isShortString type)}}8{{else}}16{{/if}}u(zclString, 0, static_cast<uint{{#if (isShortString type)}}8{{else}}16{{/if}}_t>(value.size()));
emberAfCopyInt{{#if (isShortString type)}}8{{else}}16{{/if}}u(zclString, 0, static_cast<{{>lengthType}}>(value.size()));
memcpy(&zclString[{{>sizingBytes}}], value.data(), value.size());
return emberAfWriteServerAttribute(endpoint, {{>clusterId}}, Id, zclString, ZCL_{{asDelimitedMacro type}}_ATTRIBUTE_TYPE);
{{else}}
if (!NumericAttributeTraits<{{asUnderlyingZclType type}}>::CanRepresentValue(/* isNullable = */ {{isNullable}}, value))
{
return EMBER_ZCL_STATUS_INVALID_ARGUMENT;
}
return emberAfWriteServerAttribute(endpoint, {{>clusterId}}, Id, (uint8_t *) &value, ZCL_{{asDelimitedMacro type}}_ATTRIBUTE_TYPE);
{{/if}}
}

{{#if isNullable}}
EmberAfStatus SetNull(chip::EndpointId endpoint)
{
{{#if (isString type)}}
uint8_t zclString[{{>sizingBytes}}] = { {{#if (isShortString type)}}0xFF{{else}}0xFF, 0xFF{{/if}} };
return emberAfWriteServerAttribute(endpoint, {{>clusterId}}, Id, zclString, ZCL_{{asDelimitedMacro type}}_ATTRIBUTE_TYPE);
{{else}}
auto value = NumericAttributeTraits<{{asUnderlyingZclType type}}>::kNullValue;
return emberAfWriteServerAttribute(endpoint, {{>clusterId}}, Id, reinterpret_cast<uint8_t *>(&value), ZCL_{{asDelimitedMacro type}}_ATTRIBUTE_TYPE);
{{/if}}
}

EmberAfStatus Set(chip::EndpointId endpoint, const DataModel::Nullable<{{asUnderlyingZclType type}}> & value)
{
if (value.IsNull()) {
return SetNull(endpoint);
}

return Set(endpoint, value.Value());
}
{{/if}}

} // namespace {{asUpperCamelCase label}}

{{/if}}
Expand Down
9 changes: 7 additions & 2 deletions src/app/zap-templates/templates/app/attributes/Accessors.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#pragma once

#include <app/data-model/Nullable.h>
#include <app/util/af-types.h>
#include <lib/support/Span.h>

Expand All @@ -23,10 +24,14 @@ namespace Attributes {

{{/first}}
{{#if clusterRef}}
{{#if (canHaveSimpleAccessors type)}}
{{#if (canHaveSimpleAccessors this)}}
namespace {{asUpperCamelCase label}} {
EmberAfStatus Get(chip::EndpointId endpoint, {{#if (isCharString type)}}chip::MutableCharSpan{{else if (isOctetString type)}}chip::MutableByteSpan{{else}}{{asUnderlyingZclType type}} *{{/if}} value); // {{type}} {{isArray}}
EmberAfStatus Get(chip::EndpointId endpoint, {{accessorGetterType this}} value); // {{type}} {{isArray}}
EmberAfStatus Set(chip::EndpointId endpoint, {{asUnderlyingZclType type}} value);
{{#if isNullable}}
EmberAfStatus SetNull(chip::EndpointId endpoint);
EmberAfStatus Set(chip::EndpointId endpoint, const DataModel::Nullable<{{asUnderlyingZclType type}}> & value);
{{/if}}
} // namespace {{asUpperCamelCase label}}

{{/if}}
Expand Down
Loading

0 comments on commit 07564e8

Please sign in to comment.