Skip to content

attribute::update() silently fails for some DataModelProvider based clusters (OccupancySensing cluster) #1738

@alexisbalbachan

Description

@alexisbalbachan

Description

Hi, I'm reporting this issue that had me going around in circles for a few days, I hope this will help others in the future.

  • Many sources point to attribute::update() as the primary esp-matter API for application code to push new sensor values to the Matter stack, official examples use it almost exclusively, its description states that:

    • This API updates the attribute value.
    • After this API is called, the application gets the attribute update callback with PRE_UPDATE, then the
    • attribute is updated in the database, then the application get the callback with POST_UPDATE.
  • Furthermore, the development guide at docs.espressif.com never mentions ANY alternative API for updating sensor values.

  • And in my case even the official sensors example (incorrectly) uses it for the Occupancy attribute (of OccupancySensing cluster) here:

    attribute::update(endpoint_id, OccupancySensing::Id, OccupancySensing::Attributes::Occupancy::Id, &val);
  • For DataModelProvider based clusters like OccupancySensing, this function silently succeeds. It returns ESP_OK and logs the value correctly, but the updated value is never visible to Matter clients (Tested in Home Assistant and Alexa). Clients always end up reading the C++ object's default (mOccupancy = 0).

  • There is no error, no warning log, and no documentation indicating that certain clusters require a different API.

Code Analysis

  1. Update/Report will end up calling attribute::set_val(attribute_t*, val, call_callbacks)
  2. In turn set_val(attribute_t*, val, call_callbacks) will call attribute::set_val(endpoint_id, cluster_id, attribute_id, val, call_callbacks):
    uint16_t flags = get_flags(attr);
    
    if (!(flags & ATTRIBUTE_FLAG_MANAGED_INTERNALLY)) {
        // this updates the value of attribute in the esp-matter storage
        return attribute::set_val_internal(attr, val, call_callbacks);
    }
    
    // we can use DataModelProvider::WriteAttribute API for writable attributes
    if (flags & ATTRIBUTE_FLAG_WRITABLE) {
        return set_val_via_write_attribute(endpoint_id, cluster_id, attribute_id, val);
    }
    
    // TODO: If not writable, we could use the cluster-specific setter API to update the value
    //       with the code-driven effort, we can get the cluster object and call the setter API
    return ESP_ERR_NOT_SUPPORTED;
  3. Because Occupancy attribute is created with ATTRIBUTE_FLAG_NONE (SOURCE), the code goes into the first branch and calls set_val_internal(), it returns ESP_OK
  4. Now, when reporting the value, chip engine calls DataModel::Provider->ReadAttribute()
  5. ReadAttribute calls mRegistry.Get(), if it is not nullptr then calls ReadAttribute on the retrieved object (SOURCE)
    5.1. When creating the Occupancy sensor endpoint (at ::create) it ends up registering an OccupancySensingCluster object as its internal representation.
    5.2. So the return value for mRegistry.Get() is non null -> ReadAttribute returns whatever value is set at the internal class which starts with a default value of 0.

In short: attribute::update() writes the value to esp-matter's own attribute store, but for DataModelProvider based clusters like OccupancySensing all reads, whether from Matter clients (subscriptions, reads) or local attribute::get_val(), are served from the cluster object registered in the ServerClusterInterfaceRegistry. These two stores are completely independent, so the written value is never seen by anyone reading it.

Workaround

  • Instead of using the intended attribute::update/report API, I'm relying on non standard methods, which depend on the internal class of the cluster:
#include <clusters/occupancy_sensing/integration.h>

auto *cluster = chip::app::Clusters::OccupancySensing::FindClusterOnEndpoint(endpoint_id);
if (cluster) {
    cluster->SetOccupancy(occupied);
}

Extra Notes

There's actually a TODO note at attribute::set_val and a way for it to actually return an error:

    // TODO: If not writable, we could use the cluster-specific setter API to update the value
    //       with the code-driven effort, we can get the cluster object and call the setter API
    return ESP_ERR_NOT_SUPPORTED;

I can't say anything about the proposed fix mentioned in the TODO as i don't know the project well enough, I think the Occupancy attribute should reach the ESP_ERR_NOT_SUPPORTED return, but it doesn't because it lacks ATTRIBUTE_FLAG_MANAGED_INTERNALLY, so set_val() takes the set_val_internal() branch and writes to the esp-matter RAM store, returning ESP_OK even though that store is never consulted on reads.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions