espressif / esp-matter

Espressif's SDK for Matter
Apache License 2.0
694 stars 157 forks source link

Attribute event-list is always empty in Switch cluster (CON-771) #638

Closed jzwyssig closed 1 year ago

jzwyssig commented 1 year ago

The Matter certification requires to set the correct event-list attribute in the Switch cluster. If I read the event-list attribute with chip-tool chip-tool switch read event-list <node> <endpoint> it always returns an empty array.

Therefore, I tried to set the correct value:

uint8_t event_list[] = {1};
attribute_t *attribute = attribute::get(cluster, chip::app::Clusters::Switch::Attributes::EventList::Id);
esp_matter_attr_val_t val = esp_matter_array(event_list, sizeof(event_list), sizeof(event_list)/sizeof(uint8_t));
attribute::set_val(attribute, &val);

Despite of the code above the event-list attribute remains empty. I tried attribute::update(...) as well, but that did not help either. What else can I do? It should return a single list item with value 1.

Environment

jonsmirl commented 1 year ago

Is this really needed for certification? The command lists on every cluster are empty too. I don't think this was ever implemented in CHIP core.

BTW, support for switch is completely non-functional in Google Home. I have bugged them many times to fix it. It does work in IOS.

jzwyssig commented 1 year ago

Appreciate the quick response, @jonsmirl! Indeed, they want me to implement this, and we're currently in the certification process. I'm aware that generic switches are exclusive to iOS under the name "Programmable Switch," as it's referred to in HomeKit.

However, you think Matter just returns an empty list instead of the correct attribute? Attribute checking seams like a general thing to me... Any ideas where to check? Should I reposte this in connectedhomeip?

jonsmirl commented 1 year ago

I have not checked the CHIP code base in a couple of months, but the last time I checked CHIP core was not implementing the AcceptedCommandList, GeneratedCommandList and EventList attributes. The attributes are there, but they are always empty. This applies to every cluster, not just switch.


I looked through some issues reports. There has been work to fix this. I am unsure about how much is implemented currently.

jonsmirl commented 1 year ago

I checked my devices, AcceptedCommandList and GeneratedCommandList look like they are correct now. But I don't any clusters with a filled-in EventList. Mine is empty too.

2023-09-15 10:46:01.500 17931-18039 DMG                     com.lowpan.m2                        D          AttributeReportIB =
2023-09-15 10:46:01.500 17931-18039 DMG                     com.lowpan.m2                        D          {
2023-09-15 10:46:01.500 17931-18039 DMG                     com.lowpan.m2                        D              AttributeDataIB =
2023-09-15 10:46:01.500 17931-18039 DMG                     com.lowpan.m2                        D              {
2023-09-15 10:46:01.500 17931-18039 DMG                     com.lowpan.m2                        D                  DataVersion = 0x75829103,
2023-09-15 10:46:01.500 17931-18039 DMG                     com.lowpan.m2                        D                  AttributePathIB =
2023-09-15 10:46:01.500 17931-18039 DMG                     com.lowpan.m2                        D                  {
2023-09-15 10:46:01.500 17931-18039 DMG                     com.lowpan.m2                        D                      Endpoint = 0x3,
2023-09-15 10:46:01.500 17931-18039 DMG                     com.lowpan.m2                        D                      Cluster = 0x3b,
2023-09-15 10:46:01.500 17931-18039 DMG                     com.lowpan.m2                        D                      Attribute = 0x0000_FFFA,
2023-09-15 10:46:01.500 17931-18039 DMG                     com.lowpan.m2                        D                  }
2023-09-15 10:46:01.500 17931-18039 DMG                     com.lowpan.m2                        D                      
2023-09-15 10:46:01.500 17931-18039 DMG                     com.lowpan.m2                        D                  Data = [
2023-09-15 10:46:01.500 17931-18039 DMG                     com.lowpan.m2                        D                          
2023-09-15 10:46:01.500 17931-18039 DMG                     com.lowpan.m2                        D                  ],
2023-09-15 10:46:01.500 17931-18039 DMG                     com.lowpan.m2                        D              },
2023-09-15 10:46:01.500 17931-18039 DMG                     com.lowpan.m2                        D              
2023-09-15 10:46:01.500 17931-18039 DMG                     com.lowpan.m2                        D          },

Here is one with the correct info for GeneratedCommandList

2023-09-15 10:46:01.702 17931-18039 DMG                     com.lowpan.m2                        D          AttributeReportIB =
2023-09-15 10:46:01.702 17931-18039 DMG                     com.lowpan.m2                        D          {
2023-09-15 10:46:01.702 17931-18039 DMG                     com.lowpan.m2                        D              AttributeDataIB =
2023-09-15 10:46:01.702 17931-18039 DMG                     com.lowpan.m2                        D              {
2023-09-15 10:46:01.702 17931-18039 DMG                     com.lowpan.m2                        D                  DataVersion = 0x778f0fce,
2023-09-15 10:46:01.702 17931-18039 DMG                     com.lowpan.m2                        D                  AttributePathIB =
2023-09-15 10:46:01.702 17931-18039 DMG                     com.lowpan.m2                        D                  {
2023-09-15 10:46:01.702 17931-18039 DMG                     com.lowpan.m2                        D                      Endpoint = 0x4,
2023-09-15 10:46:01.702 17931-18039 DMG                     com.lowpan.m2                        D                      Cluster = 0x5,
2023-09-15 10:46:01.702 17931-18039 DMG                     com.lowpan.m2                        D                      Attribute = 0x0000_FFF8,
2023-09-15 10:46:01.702 17931-18039 DMG                     com.lowpan.m2                        D                  }
2023-09-15 10:46:01.702 17931-18039 DMG                     com.lowpan.m2                        D                      
2023-09-15 10:46:01.702 17931-18039 DMG                     com.lowpan.m2                        D                  Data = [
2023-09-15 10:46:01.702 17931-18039 DMG                     com.lowpan.m2                        D                          0, 1, 2, 3, 4, 6, 
2023-09-15 10:46:01.702 17931-18039 DMG                     com.lowpan.m2                        D                  ],
2023-09-15 10:46:01.702 17931-18039 DMG                     com.lowpan.m2                        D              },
2023-09-15 10:46:01.702 17931-18039 DMG                     com.lowpan.m2                        D              
2023-09-15 10:46:01.702 17931-18039 DMG                     com.lowpan.m2                        D          },
jonsmirl commented 1 year ago

These attributes should be generated by CHIP core. Maybe the Espressif people can get a fix in for the missing event list.

jonsmirl commented 1 year ago

Try turning this on: app/BUILD.gn: "CHIP_CONFIG_ENABLE_EVENTLIST_ATTRIBUTE=${enable_eventlist_attribute}",

jonsmirl commented 1 year ago

CHIP_CONFIG_ENABLE_EVENTLIST_ATTRIBUTE is already turned on.

jonsmirl commented 1 year ago

the source code is here, you'll need to add some debugging to figure out why it isn't working.

https://github.com/project-chip/connectedhomeip/blob/master/src/app/util/ember-compatibility-functions.cpp#L328

jzwyssig commented 1 year ago

Thank you @jonsmirl! Unfortunately, it looks as chip_enable_session_resumption = true is enabled by default.

declare_args() {
  # Enable strict schema checks.
  enable_im_pretty_print =
      is_debug && (current_os == "linux" || current_os == "mac" ||
                   current_os == "ios" || current_os == "android")

  # Logging verbosity control for Access Control implementation
  #
  # If set to > 0, it is desired to get additional logging on all
  # access control checks for better debugging ability.
  #
  # If set to > 1, it is desired to log every single check
  chip_access_control_policy_logging_verbosity = 0
  if (is_debug && (current_os == "linux" || current_os == "mac")) {
    chip_access_control_policy_logging_verbosity = 2
  }

  chip_enable_session_resumption = true

  # By default, the resources used by each fabric is unlimited if they are allocated on heap. This flag is for checking the resource usage even when they are allocated on heap to increase code coverage in integration tests.
  chip_im_force_fabric_quota_check = false

  enable_eventlist_attribute = true
}

buildconfig_header("app_buildconfig") {
  header = "AppBuildConfig.h"
  header_dir = "app"

  defines = [
    "CHIP_CONFIG_IM_PRETTY_PRINT=${enable_im_pretty_print}",
    "CHIP_CONFIG_IM_FORCE_FABRIC_QUOTA_CHECK=${chip_im_force_fabric_quota_check}",
    "CHIP_CONFIG_ENABLE_SESSION_RESUMPTION=${chip_enable_session_resumption}",
    "CHIP_CONFIG_ACCESS_CONTROL_POLICY_LOGGING_VERBOSITY=${chip_access_control_policy_logging_verbosity}",
    "CHIP_CONFIG_PERSIST_SUBSCRIPTIONS=${chip_persist_subscriptions}",
    "CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION=${chip_subscription_timeout_resumption}",
    "CHIP_CONFIG_ENABLE_EVENTLIST_ATTRIBUTE=${enable_eventlist_attribute}",
    "CHIP_CONFIG_ENABLE_ICD_SERVER=${chip_enable_icd_server}",
  ]
}
jonsmirl commented 1 year ago

Cert test should not be requiring this....

image

jonsmirl commented 1 year ago

The Switch cluster is not filling in mCluster->eventCount so the count is zero.

jzwyssig commented 1 year ago

Cert test should not be requiring this....

image

Perfect, thanks - that actually helps, I will tell them!

jzwyssig commented 1 year ago

The Switch cluster is not filling in mCluster->eventCount so the count is zero.

A quick search (please correct me if I am wrong) shows that it is filled in esp_matter_core.cpp. But the Switch cluster remains always empty, very strange. So maybe it is an issue in esp-matter. @dhrishi could you maybe look into this please?

https://github.com/espressif/esp-matter/blob/8b4f19c5236d765345d53a388356acde8040e762/components/esp_matter/esp_matter_core.cpp#L694-L695

jonsmirl commented 1 year ago

The events do work and my device generates them. Nothing calls these which build the attribute:

event_t *create_switch_latched(cluster_t *cluster)
{
    return esp_matter::event::create(cluster, Switch::Events::SwitchLatched::Id);
}

event_t *create_initial_press(cluster_t *cluster)
{
    return esp_matter::event::create(cluster, Switch::Events::InitialPress::Id);
}

event_t *create_long_press(cluster_t *cluster)
{
    return esp_matter::event::create(cluster, Switch::Events::LongPress::Id);
}

event_t *create_short_release(cluster_t *cluster)
{
    return esp_matter::event::create(cluster, Switch::Events::ShortRelease::Id);
}

event_t *create_long_release(cluster_t *cluster)
{
    return esp_matter::event::create(cluster, Switch::Events::LongRelease::Id);
}

event_t *create_multi_press_ongoing(cluster_t *cluster)
{
    return esp_matter::event::create(cluster, Switch::Events::MultiPressOngoing::Id);
}

event_t *create_multi_press_complete(cluster_t *cluster)
{
    return esp_matter::event::create(cluster, Switch::Events::MultiPressComplete::Id);
}
jonsmirl commented 1 year ago

I'm starting to understand. You have to say in your code which events you are producing. I produce all six, so I need this code:

    cluster::switch_cluster::event::create_switch_latched(switch_cluster);
    cluster::switch_cluster::event::create_initial_press(switch_cluster);
    cluster::switch_cluster::event::create_long_press(switch_cluster);
    cluster::switch_cluster::event::create_short_release(switch_cluster);
    cluster::switch_cluster::event::create_long_release(switch_cluster);
    cluster::switch_cluster::event::create_multi_press_ongoing(switch_cluster);
    cluster::switch_cluster::event::create_multi_press_complete(switch_cluster);

And when I add that code, the events appear.

2023-09-15 14:42:10.245 21812-22013 DMG                     com.lowpan.m2                        D          AttributeReportIB =
2023-09-15 14:42:10.245 21812-22013 DMG                     com.lowpan.m2                        D          {
2023-09-15 14:42:10.245 21812-22013 DMG                     com.lowpan.m2                        D              AttributeDataIB =
2023-09-15 14:42:10.245 21812-22013 DMG                     com.lowpan.m2                        D              {
2023-09-15 14:42:10.245 21812-22013 DMG                     com.lowpan.m2                        D                  DataVersion = 0x446d4246,
2023-09-15 14:42:10.245 21812-22013 DMG                     com.lowpan.m2                        D                  AttributePathIB =
2023-09-15 14:42:10.245 21812-22013 DMG                     com.lowpan.m2                        D                  {
2023-09-15 14:42:10.245 21812-22013 DMG                     com.lowpan.m2                        D                      Endpoint = 0x8,
2023-09-15 14:42:10.245 21812-22013 DMG                     com.lowpan.m2                        D                      Cluster = 0x3b,
2023-09-15 14:42:10.245 21812-22013 DMG                     com.lowpan.m2                        D                      Attribute = 0x0000_FFFA,
2023-09-15 14:42:10.245 21812-22013 DMG                     com.lowpan.m2                        D                  }
2023-09-15 14:42:10.245 21812-22013 DMG                     com.lowpan.m2                        D                      
2023-09-15 14:42:10.245 21812-22013 DMG                     com.lowpan.m2                        D                  Data = [
2023-09-15 14:42:10.245 21812-22013 DMG                     com.lowpan.m2                        D                          0, 1, 2, 3, 4, 5, 6, 
2023-09-15 14:42:10.245 21812-22013 DMG                     com.lowpan.m2                        D                  ],
2023-09-15 14:42:10.245 21812-22013 DMG                     com.lowpan.m2                        D              },
2023-09-15 14:42:10.245 21812-22013 DMG                     com.lowpan.m2                        D              
2023-09-15 14:42:10.245 21812-22013 DMG                     com.lowpan.m2                        D          },
jonsmirl commented 1 year ago

The bug here is that this code is missing from the switch example so nobody knew it was needed.

jzwyssig commented 1 year ago

Thank you @jonsmirl! It works perfectly. Actually, it is necessary now to set the correct event list value for Matter certification 1.1. I showed them the paragraph in the spec, but but they insisted. Good it works now!

@dhrishi It would be great to have this in the switch example:

    cluster::switch_cluster::event::create_switch_latched(switch_cluster);
    cluster::switch_cluster::event::create_initial_press(switch_cluster);
    cluster::switch_cluster::event::create_long_press(switch_cluster);
    cluster::switch_cluster::event::create_short_release(switch_cluster);
    cluster::switch_cluster::event::create_long_release(switch_cluster);
    cluster::switch_cluster::event::create_multi_press_ongoing(switch_cluster);
    cluster::switch_cluster::event::create_multi_press_complete(switch_cluster);
PSONALl commented 1 year ago

@jzwyssig @jonsmirl These events were not created as they are not mandatory in the spec, Since the LatchingSwitch and MomentarySwitch have optional and exclusive conformance

jonsmirl commented 1 year ago

That's ok, please add the appropriate events into the switch example so that people will see that they are needed.