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

[ZAP] Ordering of EmberAfAttributeMetadata can save 4 bytes padding per attribute #23720

Closed
tima-q opened this issue Nov 22, 2022 · 0 comments · Fixed by #24336
Closed

[ZAP] Ordering of EmberAfAttributeMetadata can save 4 bytes padding per attribute #23720

tima-q opened this issue Nov 22, 2022 · 0 comments · Fixed by #24336
Labels

Comments

@tima-q
Copy link
Contributor

tima-q commented Nov 22, 2022

Reproduction steps / Feature

  1. Build any example app
  2. Re-order EmberAfAttributeMetadata in src/app/util/attribute-metadata.h
  3. Build again
  4. Witness gain of 4 bytes per attribute defined in endpoint_config.h
    -- done for qpg - ARM GCC compiler

Re-ordering to avoid padding in a typical best-effort way - largest members first, fitting into native boundaries - 32-bit in this case.
As this can be different on other native typed platforms, the current situation was always bloated:
Ordering of 1 bytes - 2 byte - 1 byte introduces padding for virtually any platform.

As the content is generated from ZAP out, a header update is not enough.
The template in src/app/zap-templates/templates/app/endpoint_config.zapt takes the content straight from a variable, not populated in the connectedhomeip repo - presumeably from ZAP ?

#define GENERATED_ATTRIBUTES {{ endpoint_attribute_list }}

Re-ordering:
Current:

struct EmberAfAttributeMetadata
{
    /**
     * Attribute ID, according to ZCL specs.
     */
    chip::AttributeId attributeId;
    /**
     * Attribute type, according to ZCL specs.
     */
    EmberAfAttributeType attributeType;
    /**
     * Size of this attribute in bytes.
     */
    uint16_t size;
    /**
     * Attribute mask, tagging attribute with specific
     * functionality.
     */
    EmberAfAttributeMask mask;
    /**
     * Pointer to the default value union. Actual value stored
     * depends on the mask.
     */
    EmberAfDefaultOrMinMaxAttributeValue defaultValue;

Re-ordered:
struct EmberAfAttributeMetadata_II
{
    /**
     * Pointer to the default value union. Actual value stored
     * depends on the mask.
     */
    EmberAfDefaultOrMinMaxAttributeValue defaultValue;

    /**
     * Attribute ID, according to ZCL specs.
     */
    chip::AttributeId attributeId;
    
    /**
     * Size of this attribute in bytes.
     */
    uint16_t size;

    /**
     * Attribute type, according to ZCL specs.
     */
    EmberAfAttributeType attributeType;

    /**
     * Attribute mask, tagging attribute with specific
     * functionality.
     */
    EmberAfAttributeMask mask;

Platform

other

Platform Version(s)

all

Type

Manually tested with SDK

(Optional) If manually tested please explain why this is only manually tested

Build time checked

Anything else?

No response

@tima-q tima-q added the zap label Nov 28, 2022
bzbarsky-apple pushed a commit to bzbarsky-apple/connectedhomeip that referenced this issue Jan 10, 2023
bzbarsky-apple pushed a commit to bzbarsky-apple/connectedhomeip that referenced this issue Jan 10, 2023
andy31415 pushed a commit that referenced this issue Jan 10, 2023
)

Fixes #23720

Co-authored-by: Timothy Maes <timothy.maes@qorvo.com>
kkasperczyk-no pushed a commit to kkasperczyk-no/sdk-connectedhomeip that referenced this issue Mar 15, 2023
kkasperczyk-no pushed a commit to kkasperczyk-no/sdk-connectedhomeip that referenced this issue Mar 15, 2023
lecndav pushed a commit to lecndav/connectedhomeip that referenced this issue Mar 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging a pull request may close this issue.

1 participant