From b4505ac2a724d057791c38821b560180bc21a8ef Mon Sep 17 00:00:00 2001 From: Justin Wood Date: Wed, 7 Sep 2022 13:29:32 -0700 Subject: [PATCH 01/20] Require issues for all PRs (#22457) * First commit * Updating text * Adding workflow issue type * Updating spaces * Fixing spaces, adding feature * Updating types --- .github/ISSUE_TEMPLATE/1-bug-report.yaml | 144 ++++++------- .../100-documentation-issue.yaml | 106 ++++----- .github/ISSUE_TEMPLATE/2-1.0-issue.yaml | 182 ++++++++-------- .github/ISSUE_TEMPLATE/3-sve-1.0-issue.yaml | 203 ++++++++++-------- .github/ISSUE_TEMPLATE/50-tooling-fix.yaml | 85 ++++++++ .github/ISSUE_TEMPLATE/60-platform-fix.yaml | 85 ++++++++ .github/ISSUE_TEMPLATE/70-trivial-fix.yaml | 98 +++++++++ .../ISSUE_TEMPLATE/80-feature-request.yaml | 62 ++++++ .../99-github-workflow-issue.yaml | 59 +++++ .github/workflows/require-issue.yaml | 32 +++ .pullapprove.yml | 7 + 11 files changed, 756 insertions(+), 307 deletions(-) create mode 100644 .github/ISSUE_TEMPLATE/50-tooling-fix.yaml create mode 100644 .github/ISSUE_TEMPLATE/60-platform-fix.yaml create mode 100644 .github/ISSUE_TEMPLATE/70-trivial-fix.yaml create mode 100644 .github/ISSUE_TEMPLATE/80-feature-request.yaml create mode 100644 .github/ISSUE_TEMPLATE/99-github-workflow-issue.yaml create mode 100644 .github/workflows/require-issue.yaml diff --git a/.github/ISSUE_TEMPLATE/1-bug-report.yaml b/.github/ISSUE_TEMPLATE/1-bug-report.yaml index ec13ee3ee5bb44..3266f2bb9ba5e4 100644 --- a/.github/ISSUE_TEMPLATE/1-bug-report.yaml +++ b/.github/ISSUE_TEMPLATE/1-bug-report.yaml @@ -2,76 +2,76 @@ name: "\U0001F41B Bug report" description: Create a report to help Matter title: "[BUG] " body: - - type: markdown - attributes: - value: | - Thanks for reporting an issue against the Matter SDK! We need information about the bug report to follow up, so please help us out by filling out this information. - - type: textarea - id: repro - attributes: - label: Reproduction steps - description: "How do you trigger this bug? Please walk us through it step by step." - value: | - 1. - 2. - 3. - ... - render: bash - validations: - required: true - - type: input - id: prevalence - attributes: - label: Bug prevalence - description: "How often do you or others encounter this bug?" - placeholder: "Example: Whenever I do this, 1-2 times a week, day, hour, etc" - validations: - required: true - - type: input - attributes: - label: GitHub hash of the SDK that was being used - description: Hash of the GitHub SDK used - validations: - required: true - - type: dropdown - attributes: - label: Platform - description: What platforms are affected? - multiple: true - options: - - ameba - - android - - cc13x2_cc26x2 - - darwin - - efr32 - - esp32 - - freeRTOS - - IMX8 - - k32w - - nrf - - python - - raspi - - vscode - - windows - - other - - core - validations: - required: true - - type: input - id: platform-versions - attributes: - label: Platform Version(s) - description: "What platform version(s) are affected [optional]" - placeholder: "eg: 1.0.1, N/A" - - type: textarea - attributes: - label: Anything else? - description: | - Links? References? Anything that will give us more context about the issue you are encountering! + - type: markdown + attributes: + value: | + Thanks for reporting an issue against the Matter SDK! We need information about the bug report to follow up, so please help us out by filling out this information. + - type: textarea + id: repro + attributes: + label: Reproduction steps + description: "How do you trigger this bug? Please walk us through it step by step." + value: | + 1. + 2. + 3. + ... + render: bash + validations: + required: true + - type: input + id: prevalence + attributes: + label: Bug prevalence + description: "How often do you or others encounter this bug?" + placeholder: "Example: Whenever I do this, 1-2 times a week, day, hour, etc" + validations: + required: true + - type: input + attributes: + label: GitHub hash of the SDK that was being used + description: Hash of the GitHub SDK used + validations: + required: true + - type: dropdown + attributes: + label: Platform + description: What platforms are affected? + multiple: true + options: + - ameba + - android + - cc13x2_cc26x2 + - darwin + - efr32 + - esp32 + - freeRTOS + - IMX8 + - k32w + - nrf + - python + - raspi + - vscode + - windows + - other + - core + validations: + required: true + - type: input + id: platform-versions + attributes: + label: Platform Version(s) + description: "What platform version(s) are affected [optional]" + placeholder: "eg: 1.0.1, N/A" + - type: textarea + attributes: + label: Anything else? + description: | + Links? References? Anything that will give us more context about the issue you are encountering! - Tip: You can attach images or log files by clicking this area to highlight it and then dragging files in. - validations: - required: false - - type: markdown - attributes: - value: "Thanks for submitting a bug!" + Tip: You can attach images or log files by clicking this area to highlight it and then dragging files in. + validations: + required: false + - type: markdown + attributes: + value: "Thanks for submitting a bug!" diff --git a/.github/ISSUE_TEMPLATE/100-documentation-issue.yaml b/.github/ISSUE_TEMPLATE/100-documentation-issue.yaml index ced8423ab260ee..5bc63949be7039 100644 --- a/.github/ISSUE_TEMPLATE/100-documentation-issue.yaml +++ b/.github/ISSUE_TEMPLATE/100-documentation-issue.yaml @@ -2,57 +2,57 @@ name: "\U0001F5BA Documentation Issue" description: Create an issue to improve documentation title: "[Documentation] " body: - - type: markdown - attributes: - value: | - Thanks for reporting an issue against the Matter SDK! We need information about the bug report to follow up, so please help us out by filling out this information. - - type: textarea - id: repro - attributes: - label: Documentation issues - description: "What updates to documentation are needed?" - value: | - 1. - 2. - 3. - ... - render: bash - validations: - required: true - - type: dropdown - attributes: - label: Platform - description: What platforms are affected? - multiple: true - options: - - ameba - - android - - cc13x2_cc26x2 - - darwin - - efr32 - - esp32 - - freeRTOS - - IMX8 - - k32w - - nrf connect - - nrf - - python - - raspi - - vscode - - windows - - other - - core (please add to version below) - validations: - required: false - - type: textarea - attributes: - label: Anything else? - description: | - Links? References? Anything that will give us more context about the issue you are encountering! + - type: markdown + attributes: + value: | + Thanks for reporting an issue against the Matter SDK! We need information about the bug report to follow up, so please help us out by filling out this information. + - type: textarea + id: repro + attributes: + label: Documentation issues + description: "What updates to documentation are needed?" + value: | + 1. + 2. + 3. + ... + render: bash + validations: + required: true + - type: dropdown + attributes: + label: Platform + description: What platforms are affected? + multiple: true + options: + - ameba + - android + - cc13x2_cc26x2 + - darwin + - efr32 + - esp32 + - freeRTOS + - IMX8 + - k32w + - nrf connect + - nrf + - python + - raspi + - vscode + - windows + - other + - core (please add to version below) + validations: + required: false + - type: textarea + attributes: + label: Anything else? + description: | + Links? References? Anything that will give us more context about the issue you are encountering! - Tip: You can attach images or log files by clicking this area to highlight it and then dragging files in. - validations: - required: false - - type: markdown - attributes: - value: "Thanks for submitting a documentation issue!" + Tip: You can attach images or log files by clicking this area to highlight it and then dragging files in. + validations: + required: false + - type: markdown + attributes: + value: "Thanks for helping improve our documentation." diff --git a/.github/ISSUE_TEMPLATE/2-1.0-issue.yaml b/.github/ISSUE_TEMPLATE/2-1.0-issue.yaml index 2344a139f74624..15c8a8a64e2937 100644 --- a/.github/ISSUE_TEMPLATE/2-1.0-issue.yaml +++ b/.github/ISSUE_TEMPLATE/2-1.0-issue.yaml @@ -1,95 +1,95 @@ -name: "\U0001F41B 1.0 Issue" +name: "\U0001F680 1.0 Issue" description: Create an issue that is required for Matter 1.0 release title: "[1.0] " body: - - type: markdown - attributes: - value: | - Thanks for reporting an issue against the Matter SDK! We need information about the bug report to follow up, so please help us out by filling out this information. - - type: textarea - id: repro - attributes: - label: Reproduction steps - description: "How do you trigger this bug? Please walk us through it step by step." - value: | - 1. - 2. - 3. - ... - render: bash - validations: - required: true - - type: input - id: prevalence - attributes: - label: Bug prevalence - description: "How often do you or others encounter this bug?" - placeholder: "Example: Whenever I do this, 1-2 times a week, day, hour, etc" - validations: - required: true - - type: input - attributes: - label: GitHub hash of the SDK that was being used - description: Hash of the GitHub SDK used - validations: - required: true - - type: dropdown - attributes: - label: Platform - description: What platforms are affected? - multiple: true - options: - - ameba - - android - - cc13x2_cc26x2 - - darwin - - efr32 - - esp32 - - freeRTOS - - IMX8 - - k32w - - nrf connect - - nrf - - python - - raspi - - vscode - - windows - - other - - core - validations: - required: true - - type: input - id: platform-versions - attributes: - label: Platform Version(s) - description: "What platform version(s) are affected [optional]" - placeholder: "eg: 1.0.1, N/A" - - type: dropdown - attributes: - label: Type - description: What type of issue is this? - multiple: true - options: - - Test Improvement - - Common Cluster Logic - - Spec Compliance Issue - - Security Issue - - Platform Issue - - Core SDK Memory Issue - - Core SDK Crash - - Core SDK Performance Improvement - - Core SDK Interopability Issue - validations: - required: true - - type: textarea - attributes: - label: Anything else? - description: | - Links? References? Anything that will give us more context about the issue you are encountering! + - type: markdown + attributes: + value: | + Thanks for reporting an issue against the Matter SDK! We need information about the bug report to follow up, so please help us out by filling out this information. + - type: textarea + id: repro + attributes: + label: Reproduction steps + description: "How do you trigger this bug? Please walk us through it step by step." + value: | + 1. + 2. + 3. + ... + render: bash + validations: + required: true + - type: input + id: prevalence + attributes: + label: Bug prevalence + description: "How often do you or others encounter this bug?" + placeholder: "Example: Whenever I do this, 1-2 times a week, day, hour, etc" + validations: + required: true + - type: input + attributes: + label: GitHub hash of the SDK that was being used + description: Hash of the GitHub SDK used + validations: + required: true + - type: dropdown + attributes: + label: Platform + description: What platforms are affected? + multiple: true + options: + - ameba + - android + - cc13x2_cc26x2 + - darwin + - efr32 + - esp32 + - freeRTOS + - IMX8 + - k32w + - nrf connect + - nrf + - python + - raspi + - vscode + - windows + - other + - core + validations: + required: true + - type: input + id: platform-versions + attributes: + label: Platform Version(s) + description: "What platform version(s) are affected [optional]" + placeholder: "eg: 1.0.1, N/A" + - type: dropdown + attributes: + label: Type + description: What type of issue is this? + multiple: true + options: + - Test Improvement + - Common Cluster Logic + - Spec Compliance Issue + - Security Issue + - Platform Issue + - Core SDK Memory Issue + - Core SDK Crash + - Core SDK Performance Improvement + - Core SDK Interopability Issue + validations: + required: true + - type: textarea + attributes: + label: Anything else? + description: | + Links? References? Anything that will give us more context about the issue you are encountering! - Tip: You can attach images or log files by clicking this area to highlight it and then dragging files in. - validations: - required: false - - type: markdown - attributes: - value: "Thanks for submitting a bug!" + Tip: You can attach images or log files by clicking this area to highlight it and then dragging files in. + validations: + required: false + - type: markdown + attributes: + value: "Thanks for helping us get to 1.0!" diff --git a/.github/ISSUE_TEMPLATE/3-sve-1.0-issue.yaml b/.github/ISSUE_TEMPLATE/3-sve-1.0-issue.yaml index df8fc198fe33d6..90de36176c2e3f 100644 --- a/.github/ISSUE_TEMPLATE/3-sve-1.0-issue.yaml +++ b/.github/ISSUE_TEMPLATE/3-sve-1.0-issue.yaml @@ -1,95 +1,116 @@ -name: "\U0001F41B SVE Issue" +name: "\U0001F9EA SVE Issue" description: Create an issue that is required for SVE title: "[SVE] " body: - - type: markdown - attributes: - value: | - Thanks for reporting an issue against the Matter SDK! We need information about the bug report to follow up, so please help us out by filling out this information. - - type: textarea - id: repro - attributes: - label: Reproduction steps - description: "How do you trigger this bug? Please walk us through it step by step." - value: | - 1. - 2. - 3. - ... - render: bash - validations: - required: true - - type: input - id: prevalence - attributes: - label: Bug prevalence - description: "How often do you or others encounter this bug?" - placeholder: "Example: Whenever I do this, 1-2 times a week, day, hour, etc" - validations: - required: true - - type: input - attributes: - label: GitHub hash of the SDK that was being used - description: Hash of the GitHub SDK used - validations: - required: true - - type: dropdown - attributes: - label: Platform - description: What platforms are affected? - multiple: true - options: - - ameba - - android - - cc13x2_cc26x2 - - darwin - - efr32 - - esp32 - - freeRTOS - - IMX8 - - k32w - - nrf connect - - nrf - - python - - raspi - - vscode - - windows - - other - - core (please add to version below) - validations: - required: true - - type: input - id: platform-versions - attributes: - label: Platform Version(s) - description: "What platform version(s) are affected [optional]" - placeholder: "eg: 1.0.1, N/A" - - type: dropdown - attributes: - label: Type - description: What type of issue is this? - multiple: true - options: - - Test Improvement - - Common Cluster Logic - - Spec Compliance Issue - - Security Issue - - Platform Issue - - Core SDK Memory Issue - - Core SDK Crash - - Core SDK Performance Improvement - - Core SDK Interopability Issue - validations: - required: true - - type: textarea - attributes: - label: Anything else? - description: | - Links? References? Anything that will give us more context about the issue you are encountering! + - type: markdown + attributes: + value: | + Thanks for reporting an issue against the Matter SDK! We need information about the bug report to follow up, so please help us out by filling out this information. + - type: textarea + id: repro + attributes: + label: Reproduction steps + description: "How do you trigger this bug? Please walk us through it step by step." + value: | + 1. + 2. + 3. + ... + render: bash + validations: + required: true + - type: input + id: prevalence + attributes: + label: Bug prevalence + description: "How often do you or others encounter this bug?" + placeholder: "Example: Whenever I do this, 1-2 times a week, day, hour, etc" + validations: + required: true + - type: input + attributes: + label: GitHub hash of the SDK that was being used + description: Hash of the GitHub SDK used + validations: + required: true + - type: dropdown + attributes: + label: Platform + description: What platforms are affected? + multiple: true + options: + - ameba + - android + - cc13x2_cc26x2 + - darwin + - efr32 + - esp32 + - freeRTOS + - IMX8 + - k32w + - nrf connect + - nrf + - python + - raspi + - vscode + - windows + - other + - core (please add to version below) + validations: + required: true + - type: input + id: platform-versions + attributes: + label: Platform Version(s) + description: "What platform version(s) are affected [optional]" + placeholder: "eg: 1.0.1, N/A" + - type: dropdown + attributes: + label: Type + description: What type of issue is this? + multiple: true + options: + - Test Improvement + - Common Cluster Logic + - Spec Compliance Issue + - Security Issue + - Platform Issue + - Core SDK Memory Issue + - Core SDK Crash + - Core SDK Performance Improvement + - Core SDK Interopability Issue + validations: + required: true + - type: dropdown + attributes: + label: Type + description: How was this tested? + multiple: true + options: + - Unit tested + - YAML tested + - Manually tested with SDK + - CI tested + - Hardware validated + - Platform validated + validations: + required: true + - type: textarea + attributes: + label: (Optional) If manually tested please explain why this is only manually tested + description: | + Please explain how you tested it + validations: + required: false + - type: textarea + attributes: + label: Anything else? + description: | + Links? References? Anything that will give us more context about the issue you are encountering! - Tip: You can attach images or log files by clicking this area to highlight it and then dragging files in. - validations: - required: false - - type: markdown - attributes: - value: "Thanks for submitting a bug!" + Tip: You can attach images or log files by clicking this area to highlight it and then dragging files in. + validations: + required: false + - type: markdown + attributes: + value: "Thanks for helping our SVE!" diff --git a/.github/ISSUE_TEMPLATE/50-tooling-fix.yaml b/.github/ISSUE_TEMPLATE/50-tooling-fix.yaml new file mode 100644 index 00000000000000..ed314f9cb925a0 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/50-tooling-fix.yaml @@ -0,0 +1,85 @@ +name: "\U0001F3C3 Tooling Fix/Feature" +description: Create an issue for a tooling specific fix/feature +title: "[Platform] " +body: + - type: markdown + attributes: + value: | + Thanks for taking the time to submit a an issue for our tooling, can you please please help us understand this change in this quick form? + - type: textarea + id: repro + attributes: + label: Reproduction steps / Feature + description: "How do you trigger this issue and/or can you please explain this new feature? Please walk us through it step by step." + value: | + 1. + 2. + 3. + ... + render: bash + validations: + required: true + - type: dropdown + attributes: + label: Platform + description: What platforms are affected? + multiple: true + options: + - ameba + - android + - cc13x2_cc26x2 + - darwin + - efr32 + - esp32 + - freeRTOS + - IMX8 + - k32w + - nrf connect + - nrf + - python + - raspi + - vscode + - windows + - other + - core (please add to version below) + validations: + required: true + - type: input + id: platform-versions + attributes: + label: Platform Version(s) + description: "What platform version(s) are affected [optional]" + placeholder: "eg: 1.0.1, N/A" + - type: dropdown + attributes: + label: Type + description: How was this tested? + multiple: true + options: + - Unit tested + - YAML tested + - Manually tested with SDK + - CI tested + - Hardware validated + - Platform validated + validations: + required: true + - type: textarea + attributes: + label: (Optional) If manually tested please explain why this is only manually tested + description: | + Please explain how you tested it + validations: + required: false + - type: textarea + attributes: + label: Anything else? + description: | + Links? References? Anything that will give us more context about this! + + Tip: You can attach images or log files by clicking this area to highlight it and then dragging files in. + validations: + required: false + - type: markdown + attributes: + value: "Thanks for help our tooling!" diff --git a/.github/ISSUE_TEMPLATE/60-platform-fix.yaml b/.github/ISSUE_TEMPLATE/60-platform-fix.yaml new file mode 100644 index 00000000000000..9593847a228290 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/60-platform-fix.yaml @@ -0,0 +1,85 @@ +name: "\U0001F9F0 Platform Fix" +description: Create an issue for a platform specific fix +title: "[Platform] " +body: + - type: markdown + attributes: + value: | + Thanks for taking the time to submit a an issue for a platform, can you please please help us understand this change in this quick form? + - type: textarea + id: repro + attributes: + label: Reproduction steps + description: "How do you trigger this issue? Please walk us through it step by step." + value: | + 1. + 2. + 3. + ... + render: bash + validations: + required: true + - type: dropdown + attributes: + label: Platform + description: What platforms are affected? + multiple: true + options: + - ameba + - android + - cc13x2_cc26x2 + - darwin + - efr32 + - esp32 + - freeRTOS + - IMX8 + - k32w + - nrf connect + - nrf + - python + - raspi + - vscode + - windows + - other + - core (please add to version below) + validations: + required: true + - type: input + id: platform-versions + attributes: + label: Platform Version(s) + description: "What platform version(s) are affected [optional]" + placeholder: "eg: 1.0.1, N/A" + - type: dropdown + attributes: + label: Type + description: How was this tested? + multiple: true + options: + - Unit tested + - YAML tested + - Manually tested with SDK + - CI tested + - Hardware validated + - Platform validated + validations: + required: true + - type: textarea + attributes: + label: (Optional) If manually tested please explain why this is only manually tested + description: | + Please explain how you tested it + validations: + required: false + - type: textarea + attributes: + label: Anything else? + description: | + Links? References? Anything that will give us more context about this! + + Tip: You can attach images or log files by clicking this area to highlight it and then dragging files in. + validations: + required: false + - type: markdown + attributes: + value: "Thanks for submitting a platform issue!" diff --git a/.github/ISSUE_TEMPLATE/70-trivial-fix.yaml b/.github/ISSUE_TEMPLATE/70-trivial-fix.yaml new file mode 100644 index 00000000000000..d9438ab611f83b --- /dev/null +++ b/.github/ISSUE_TEMPLATE/70-trivial-fix.yaml @@ -0,0 +1,98 @@ +name: "\U0001F3C3 Trivial Fix" +description: Create an issue for a trivial fix +title: "[Trivial] " +body: + - type: markdown + attributes: + value: | + Thanks for taking the time to submit a trivial fix, can you please please help us understand this change in this quick form? + - type: dropdown + attributes: + label: Type + description: What type of trivial fix is this? + multiple: true + options: + - Comment fix + - Typo fix + - Rename + - Other + validations: + required: true + - type: textarea + id: repro + attributes: + label: Explanation + description: "(Optional) If other, why do you think this is trivial?" + value: | + 1. + 2. + 3. + ... + render: bash + validations: + required: false + - type: dropdown + attributes: + label: Platform + description: What platforms are affected? + multiple: true + options: + - all + - ameba + - android + - cc13x2_cc26x2 + - darwin + - efr32 + - esp32 + - freeRTOS + - IMX8 + - k32w + - nrf connect + - nrf + - python + - raspi + - vscode + - windows + - other + - core (please add to version below) + validations: + required: true + - type: input + id: platform-versions + attributes: + label: Platform Version(s) + description: "What platform version(s) are affected [optional]" + placeholder: "eg: 1.0.1, N/A" + - type: dropdown + attributes: + label: Type + description: How was this tested? + multiple: true + options: + - Unit tested + - YAML tested + - Manually tested with SDK + - CI tested + - Hardware validated + - Platform validated + validations: + required: true + - type: textarea + attributes: + label: (Optional) If manually tested please explain why this is only manually tested + description: | + Please explain how you tested it + validations: + required: false + - type: textarea + attributes: + label: Anything else? + description: | + Links? References? Anything that will give us more context about this! + + Tip: You can attach images or log files by clicking this area to highlight it and then dragging files in. + validations: + required: false + - type: markdown + attributes: + value: "Thanks for submitting a trivial issue!" diff --git a/.github/ISSUE_TEMPLATE/80-feature-request.yaml b/.github/ISSUE_TEMPLATE/80-feature-request.yaml new file mode 100644 index 00000000000000..1526dc23c54755 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/80-feature-request.yaml @@ -0,0 +1,62 @@ +name: "\U0001F4A1 Feature Request" +description: Create an feature to be considered future release +title: "[Feature] " +body: + - type: markdown + attributes: + value: | + Thanks for submitting a feature request to the Matter SDK! We need information about the feature to follow up, so please help us out by filling out this information. + - type: textarea + id: repro + attributes: + label: Feature description + description: "What feature are you looking to add? Please walk us through it!" + value: | + ... + render: bash + validations: + required: true + - type: dropdown + attributes: + label: Platform + description: What platforms are affected? + multiple: true + options: + - all + - ameba + - android + - cc13x2_cc26x2 + - darwin + - efr32 + - esp32 + - freeRTOS + - IMX8 + - k32w + - nrf connect + - nrf + - python + - raspi + - vscode + - windows + - other + - core + validations: + required: true + - type: input + id: platform-versions + attributes: + label: Platform Version(s) + description: "What platform version(s) are affected [optional]" + placeholder: "eg: 1.0.1, N/A" + - type: textarea + attributes: + label: Anything else? + description: | + Links? References? Anything that will give us more context about the issue you are encountering! + + Tip: You can attach images or log files by clicking this area to highlight it and then dragging files in. + validations: + required: false + - type: markdown + attributes: + value: "Thanks for submitting a new feature request." diff --git a/.github/ISSUE_TEMPLATE/99-github-workflow-issue.yaml b/.github/ISSUE_TEMPLATE/99-github-workflow-issue.yaml new file mode 100644 index 00000000000000..2f1570e4b860a0 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/99-github-workflow-issue.yaml @@ -0,0 +1,59 @@ +name: "\U0001F477 GitHub / CI / Workflow" +description: Create an issue to improve CI / Workflows +title: "[Workflow] " +body: + - type: markdown + attributes: + value: | + Thanks for reporting an issue against the Matter SDK's workflows! We need information about the bug report to follow up, so please help us out by filling out this information. + - type: textarea + id: repro + attributes: + label: Workflow issues + description: "What updates to the workflows are needed?" + value: | + 1. + 2. + 3. + ... + render: bash + validations: + required: true + - type: dropdown + attributes: + label: Platform + description: What platforms are affected? + multiple: true + options: + - ameba + - android + - cc13x2_cc26x2 + - darwin + - efr32 + - esp32 + - freeRTOS + - IMX8 + - k32w + - nrf connect + - nrf + - python + - raspi + - vscode + - windows + - other + - core (please add to version below) + - all + validations: + required: false + - type: textarea + attributes: + label: Anything else? + description: | + Links? References? Anything that will give us more context about the issue you are encountering! + + Tip: You can attach images or log files by clicking this area to highlight it and then dragging files in. + validations: + required: false + - type: markdown + attributes: + value: "Thanks for helping improve our development workflows" diff --git a/.github/workflows/require-issue.yaml b/.github/workflows/require-issue.yaml new file mode 100644 index 00000000000000..3915821ae618b7 --- /dev/null +++ b/.github/workflows/require-issue.yaml @@ -0,0 +1,32 @@ +# Copyright (c) 2020 Project CHIP Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +name: Require Issues for PRs + +on: + pull_request: + types: [edited, synchronize, opened, reopened] + +jobs: + verify_issue_exists: + name: Verify Linked Issue + runs-on: ubuntu-latest + + steps: + - name: Verify Linked Issue + uses: hattan/verify-linked-issue-action@v1.1.5 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + with: + message: "All PRs require an issue to be accepted, please link an issue or mention it in the body using #" diff --git a/.pullapprove.yml b/.pullapprove.yml index f9da0a4be5ab3b..90ac75dcec2472 100644 --- a/.pullapprove.yml +++ b/.pullapprove.yml @@ -43,6 +43,13 @@ overrides: status: failure explanation: "Style must be inline before reviewing can be complete" + ############################################################ + # Require Issues + ############################################################ + - if: "'*issue*' not in statuses.successful" + status: failure + explanation: "An issue is required for all PRs" + ############################################################ # Fast tracking ############################################################ From e1966f57428dcd0e0fd1aba5aa753867c86e069a Mon Sep 17 00:00:00 2001 From: Justin Wood Date: Wed, 7 Sep 2022 19:44:12 -0700 Subject: [PATCH 02/20] Update PULL_REQUEST_TEMPLATE.md --- .github/PULL_REQUEST_TEMPLATE.md | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index f556194cf68383..f610e3bfecf7e1 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -1,17 +1,5 @@ -#### Problem -What is being fixed? Examples: -* Fix crash on startup -* Fixes #12345 Frobnozzle is leaky (exactly like that, so GitHub will auto-close the issue). +#### Issue Being Resolved +* Fixes #12345 (exactly like this, so this PR is associated with an issue) #### Change overview What's in this PR - -#### Testing -How was this tested? (at least one bullet point required) -* If unit tests were added, how do they cover this issue? -* If unit tests existed, how were they fixed/modified to prevent this in future? -* If new unit tests are not added, why not? -* If integration tests were added, how do they verify this change? -* If new integration tests are not added, why not? -* If manually tested, what platforms controller and device platforms were manually tested, and how? -* If no testing is required, why not? From 71bf8e632523e87437b9c1d47bf859c5543681f7 Mon Sep 17 00:00:00 2001 From: Justin Wood Date: Wed, 7 Sep 2022 22:21:12 -0700 Subject: [PATCH 03/20] Update .pullapprove.yml --- .pullapprove.yml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.pullapprove.yml b/.pullapprove.yml index 90ac75dcec2472..a2837bd27dc0a1 100644 --- a/.pullapprove.yml +++ b/.pullapprove.yml @@ -46,9 +46,10 @@ overrides: ############################################################ # Require Issues ############################################################ - - if: "'*issue*' not in statuses.successful" - status: failure - explanation: "An issue is required for all PRs" +# disabling until we have PRs up to date +# - if: "'*issue*' not in statuses.successful" +# status: failure +# explanation: "An issue is required for all PRs" ############################################################ # Fast tracking From ac30d99d79ebb909e2285feab1611229c10eacdc Mon Sep 17 00:00:00 2001 From: Tennessee Carmel-Veilleux Date: Thu, 8 Sep 2022 01:23:08 -0400 Subject: [PATCH 04/20] Allow null ICAC in `AndroidDeviceControllerWrapper::AllocateNew` (#22458) * Allow null ICAC in `AndroidDeviceControllerWrapper::AllocateNew` - Passing null for existing ICAC, when a NOC and RCAC were provided and desired to be used casued confusing behavior of NOC/RCAC being ignored and a completely different cert chain being generated. Fixes #22455 Changes done: - Add code to just use empty internal ICAC when null is provided in Java - Add log to tell users when an ephemeral NOC chain is generated. Testing done: - CI still passes - Tested in unit/integration with our internal infrastructure that uses this API both with/without ICAC * Restyled --- .../java/AndroidDeviceControllerWrapper.cpp | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/controller/java/AndroidDeviceControllerWrapper.cpp b/src/controller/java/AndroidDeviceControllerWrapper.cpp index dc223d989859df..f737e9bae81a34 100644 --- a/src/controller/java/AndroidDeviceControllerWrapper.cpp +++ b/src/controller/java/AndroidDeviceControllerWrapper.cpp @@ -209,8 +209,7 @@ AndroidDeviceControllerWrapper * AndroidDeviceControllerWrapper::AllocateNew( // The lifetime of the ephemeralKey variable must be kept until SetupParams is saved. Crypto::P256Keypair ephemeralKey; - if (rootCertificate != nullptr && intermediateCertificate != nullptr && nodeOperationalCertificate != nullptr && - keypairDelegate != nullptr) + if (rootCertificate != nullptr && nodeOperationalCertificate != nullptr && keypairDelegate != nullptr) { CHIPP256KeypairBridge * nativeKeypairBridge = wrapper->GetP256KeypairBridge(); nativeKeypairBridge->SetDelegate(keypairDelegate); @@ -224,21 +223,20 @@ AndroidDeviceControllerWrapper * AndroidDeviceControllerWrapper::AllocateNew( setupParams.hasExternallyOwnedOperationalKeypair = true; JniByteArray jniRcac(env, rootCertificate); - JniByteArray jniIcac(env, intermediateCertificate); JniByteArray jniNoc(env, nodeOperationalCertificate); // Make copies of the cert that outlive the scope so that future factor init does not // cause loss of scope from the JNI refs going away. Also, this keeps the certs // handy for debugging commissioner init. wrapper->mRcacCertificate = std::vector(jniRcac.byteSpan().begin(), jniRcac.byteSpan().end()); - if (!jniIcac.byteSpan().empty()) + + // Intermediate cert could be missing. Let's only copy it if present + wrapper->mIcacCertificate.clear(); + if (intermediateCertificate != nullptr) { + JniByteArray jniIcac(env, intermediateCertificate); wrapper->mIcacCertificate = std::vector(jniIcac.byteSpan().begin(), jniIcac.byteSpan().end()); } - else - { - wrapper->mIcacCertificate.clear(); - } wrapper->mNocCertificate = std::vector(jniNoc.byteSpan().begin(), jniNoc.byteSpan().end()); @@ -248,6 +246,9 @@ AndroidDeviceControllerWrapper * AndroidDeviceControllerWrapper::AllocateNew( } else { + ChipLogProgress(Controller, + "No existing credentials provided: generating ephemeral local NOC chain with OperationalCredentialsIssuer"); + *errInfoOnFailure = ephemeralKey.Initialize(); if (*errInfoOnFailure != CHIP_NO_ERROR) { @@ -289,7 +290,7 @@ AndroidDeviceControllerWrapper * AndroidDeviceControllerWrapper::AllocateNew( { return nullptr; } - ChipLogProgress(Support, "Setting up group data for Fabric Index %u with Compressed Fabric ID:", + ChipLogProgress(Controller, "Setting up group data for Fabric Index %u with Compressed Fabric ID:", static_cast(wrapper->Controller()->GetFabricIndex())); ChipLogByteSpan(Support, compressedFabricIdSpan); From 0969e615db8e8e5bd12b36fadc8f03d94d624c60 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Thu, 8 Sep 2022 10:24:08 -0400 Subject: [PATCH 05/20] Allow and ignore '-' characters in manual pairing codes. (#22238) * Allow and ignore '-' characters in manual pairing codes. The spec does not mention these, but the brand guidelines apparently talk about using them. While ideally all SDK consumers would do the right thing and strip them out on input, we should probably also try for maximal interop and do the same stripping in the SDK itself. * Add test per review comment. --- src/setup_payload/ManualSetupPayloadParser.h | 11 +++++++- src/setup_payload/tests/TestManualCode.cpp | 29 ++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/src/setup_payload/ManualSetupPayloadParser.h b/src/setup_payload/ManualSetupPayloadParser.h index 94b9401b1812e7..1f5f3e24661da0 100644 --- a/src/setup_payload/ManualSetupPayloadParser.h +++ b/src/setup_payload/ManualSetupPayloadParser.h @@ -25,6 +25,7 @@ #include "SetupPayload.h" +#include #include #include #include @@ -41,7 +42,15 @@ class ManualSetupPayloadParser std::string mDecimalStringRepresentation; public: - ManualSetupPayloadParser(std::string decimalRepresentation) : mDecimalStringRepresentation(std::move(decimalRepresentation)) {} + ManualSetupPayloadParser(std::string decimalRepresentation) : mDecimalStringRepresentation(std::move(decimalRepresentation)) + { + // '-' might be being used in the decimal code as a digit group + // separator, to aid in readability. It's not actually part of the + // decomal code, so strip it out. + mDecimalStringRepresentation.erase( + std::remove(mDecimalStringRepresentation.begin(), mDecimalStringRepresentation.end(), '-'), + mDecimalStringRepresentation.end()); + } CHIP_ERROR populatePayload(SetupPayload & outPayload); static CHIP_ERROR CheckDecimalStringValidity(std::string decimalString, std::string & decimalStringWithoutCheckDigit); diff --git a/src/setup_payload/tests/TestManualCode.cpp b/src/setup_payload/tests/TestManualCode.cpp index 89a98780f9c5b8..ea22bbdaf208ad 100644 --- a/src/setup_payload/tests/TestManualCode.cpp +++ b/src/setup_payload/tests/TestManualCode.cpp @@ -33,7 +33,9 @@ #include #include +#include #include +#include using namespace chip; @@ -202,6 +204,14 @@ void TestGenerateAndParser_ManualSetupCodeWithLongDiscriminator(nlTestSuite * in } } +char ComputeCheckChar(const std::string & str) +{ + // Strip out dashes, if any, from the string before computing the checksum. + std::string copy(str); + copy.erase(std::remove(copy.begin(), copy.end(), '-'), copy.end()); + return Verhoeff10::ComputeCheckChar(copy.c_str()); +} + void TestPayloadParser_FullPayload(nlTestSuite * inSuite, void * inContext) { SetupPayload payload; @@ -216,6 +226,15 @@ void TestPayloadParser_FullPayload(nlTestSuite * inSuite, void * inContext) discriminator.SetShortValue(0xa); assertPayloadValues(inSuite, err, CHIP_NO_ERROR, payload, 123456780, discriminator, 45367, 14526); + // The same thing, but with dashes separating digit groups. + decimalString = "6361-0875-3545-3671-4526"; + decimalString += ComputeCheckChar(decimalString.c_str()); + err = ManualSetupPayloadParser(decimalString).populatePayload(payload); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + + discriminator.SetShortValue(0xa); + assertPayloadValues(inSuite, err, CHIP_NO_ERROR, payload, 123456780, discriminator, 45367, 14526); + decimalString = "52927623630456200032"; decimalString += Verhoeff10::ComputeCheckChar(decimalString.c_str()); err = ManualSetupPayloadParser(decimalString).populatePayload(payload); @@ -288,6 +307,16 @@ void TestPayloadParser_PartialPayload(nlTestSuite * inSuite, void * inContext) discriminator.SetShortValue(0xa); assertPayloadValues(inSuite, err, CHIP_NO_ERROR, payload, 123456780, discriminator, 0, 0); + // The same thing, but with dashes separating digit groups. + decimalString = "236-108753-5"; + decimalString += ComputeCheckChar(decimalString.c_str()); + NL_TEST_ASSERT(inSuite, decimalString.length() == 13); + err = ManualSetupPayloadParser(decimalString).populatePayload(payload); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + + discriminator.SetShortValue(0xa); + assertPayloadValues(inSuite, err, CHIP_NO_ERROR, payload, 123456780, discriminator, 0, 0); + decimalString = "0000010000"; decimalString += Verhoeff10::ComputeCheckChar(decimalString.c_str()); NL_TEST_ASSERT(inSuite, decimalString.length() == 11); From a3c05f7205187cae015a007b13e5f072acbf312b Mon Sep 17 00:00:00 2001 From: Tennessee Carmel-Veilleux Date: Thu, 8 Sep 2022 10:32:14 -0400 Subject: [PATCH 06/20] Add `LogBufferAsHex` utility (#22472) - During recent testing, it was found many participants were doing ad-hoc logging code for buffers, due to the lack of a hex dump capability. This is especially important to debug certificate issues and cryptographic algorithms. Fixes #22136 This PR: - Adds a `LogBufferAsHex` utility that can dump large binary buffer to log in a compact and recoverable format, using hexadecimal representation. - Memory usage is only stack, and very minimal. - Method bypasses limits imposed by single long log lines Testing done: - Added a full unit test suite for the behavior, validating the log format expected. --- src/lib/support/BytesToHex.cpp | 31 +++++++ src/lib/support/BytesToHex.h | 28 +++++++ src/lib/support/tests/TestBytesToHex.cpp | 100 +++++++++++++++++++++++ 3 files changed, 159 insertions(+) diff --git a/src/lib/support/BytesToHex.cpp b/src/lib/support/BytesToHex.cpp index c73ea199e9b9c6..ee5777f60aa754 100644 --- a/src/lib/support/BytesToHex.cpp +++ b/src/lib/support/BytesToHex.cpp @@ -201,5 +201,36 @@ size_t UppercaseHexToUint16(const char * src_hex, const size_t src_size, uint16_ return decoded_size; } +void LogBufferAsHex(const char * label, const ByteSpan & span) +{ + constexpr size_t kBytesPerLine = 32u; + + size_t remaining = span.size(); + if (remaining == 0) + { + ChipLogProgress(Support, "%s>>>", ((label != nullptr) ? label : "")); + return; + } + + const uint8_t * cursor = span.data(); + while (remaining > 0u) + { + size_t chunk_size = (remaining < kBytesPerLine) ? remaining : kBytesPerLine; + char hex_buf[(kBytesPerLine * 2) + 1]; + + CHIP_ERROR err = BytesToUppercaseHexString(cursor, chunk_size, &hex_buf[0], sizeof(hex_buf)); + if (err != CHIP_NO_ERROR) + { + ChipLogProgress(Support, "Failed to dump hex %" CHIP_ERROR_FORMAT, err.Format()); + return; + } + + ChipLogProgress(Support, "%s>>>%s", ((label != nullptr) ? label : ""), hex_buf); + + cursor += chunk_size; + remaining -= chunk_size; + } +} + } // namespace Encoding } // namespace chip diff --git a/src/lib/support/BytesToHex.h b/src/lib/support/BytesToHex.h index 8ade696f17db05..c41791f32307ab 100644 --- a/src/lib/support/BytesToHex.h +++ b/src/lib/support/BytesToHex.h @@ -19,6 +19,7 @@ #include #include +#include #include #include @@ -135,6 +136,33 @@ inline CHIP_ERROR BytesToLowercaseHexString(const uint8_t * src_bytes, size_t sr return BytesToHex(src_bytes, src_size, dest_hex_str, dest_size_max, HexFlags::kNullTerminate); } +/** + * @brief Dumps a binary buffer to log as hexadecimal + * + * Output is 32 bytes per line of uppercase hex, prepended with a given label. + * + * This function is useful to dump binary buffers such as certificates + * which may need to be extracted from logs during debugging. + * + * Format is organized to allow easy extraction by searching the regex + * `LABEL>>>[0-9A-F]+$` where LABEL is the `label` argument passed. + * + * Format looks like: + * + * ``` + * label>>>A54A39294B28886E8BFC15B44105A3FD22745225983A753E6BB82DA7C62493BF + * label>>>02C3ED03D41B6F7874E7E887321DE7B4872CEB9F080B6ECE14A8ABFA260573A3 + * label>>>8D759C + * ``` + * + * If buffer is empty, at least one line with the `LABEL>>>` will be logged, + * with no data. + * + * @param label - label to prepend. If nullptr, no label will be prepended + * @param span - Span over buffer that needs to be dumped. + */ +void LogBufferAsHex(const char * label, const ByteSpan & span); + /** * Convert a buffer of hexadecimal characters to bytes. Supports both lowercase * and uppercase (or a mix of cases) hexadecimal characters. Supported input is diff --git a/src/lib/support/tests/TestBytesToHex.cpp b/src/lib/support/tests/TestBytesToHex.cpp index bb1cdcd1cd39c4..ddd92772e16412 100644 --- a/src/lib/support/tests/TestBytesToHex.cpp +++ b/src/lib/support/tests/TestBytesToHex.cpp @@ -17,16 +17,26 @@ #include #include +#include +#include #include +#include +#include +#include #include +#include #include namespace { +using namespace chip; using namespace chip::Encoding; +// To accumulate redirected logs for some tests +std::vector gRedirectedLogLines; + void TestBytesToHexNotNullTerminated(nlTestSuite * inSuite, void * inContext) { // Uppercase @@ -328,12 +338,102 @@ void TestHexToBytesAndUint(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, test16Out == test16OutExpected); } +ENFORCE_FORMAT(3, 0) void AccumulateLogLineCallback(const char * module, uint8_t category, const char * msg, va_list args) +{ + (void) module; + (void) category; + + char line[256]; + memset(&line[0], 0, sizeof(line)); + vsnprintf(line, sizeof(line), msg, args); + gRedirectedLogLines.push_back(std::string(line)); +} + +void ValidateTextMatches(nlTestSuite * inSuite, const char ** expected, size_t numLines, const std::vector & candidate) +{ + NL_TEST_ASSERT(inSuite, candidate.size() == numLines); + if (candidate.size() != numLines) + { + return; + } + for (size_t idx = 0; idx < numLines; idx++) + { + printf("Checking '%s' against '%s'\n", candidate.at(idx).c_str(), expected[idx]); + NL_TEST_ASSERT(inSuite, candidate.at(idx) == expected[idx]); + if (candidate.at(idx) != expected[idx]) + { + return; + } + } +} + +void TestLogBufferAsHex(nlTestSuite * inSuite, void * inContext) +{ + const char * kExpectedText1[] = { + ">>>A54A39294B28886E8BFC15B44105A3FD22745225983A753E6BB82DA7C62493BF", + ">>>02C3ED03D41B6F7874E7E887321DE7B4872CEB9F080B6ECE14A8ABFA260573A3", + ">>>8D759C", + }; + + const char * kExpectedText2[] = { + "label>>>A54A39294B28886E8BFC15B44105A3FD22745225983A753E6BB82DA7C62493BF", + "label>>>02C3ED03D41B6F7874E7E887321DE7B4872CEB9F080B6ECE14A8ABFA260573A3", + "label>>>8D759C", + }; + + const char * kExpectedText3[] = { + "label>>>", + }; + + const char * kExpectedText4[] = { + "label>>>A54A39", + }; + + const uint8_t buffer[67] = { 0xa5, 0x4a, 0x39, 0x29, 0x4b, 0x28, 0x88, 0x6e, 0x8b, 0xfc, 0x15, 0xb4, 0x41, 0x05, + 0xa3, 0xfd, 0x22, 0x74, 0x52, 0x25, 0x98, 0x3a, 0x75, 0x3e, 0x6b, 0xb8, 0x2d, 0xa7, + 0xc6, 0x24, 0x93, 0xbf, 0x02, 0xc3, 0xed, 0x03, 0xd4, 0x1b, 0x6f, 0x78, 0x74, 0xe7, + 0xe8, 0x87, 0x32, 0x1d, 0xe7, 0xb4, 0x87, 0x2c, 0xeb, 0x9f, 0x08, 0x0b, 0x6e, 0xce, + 0x14, 0xa8, 0xab, 0xfa, 0x26, 0x05, 0x73, 0xa3, 0x8d, 0x75, 0x9c }; + + struct TestCase + { + const char * label; + chip::ByteSpan buffer; + const char ** expectedText; + size_t numLines; + }; + + const TestCase kTestCases[] = { + // Basic cases + { "", ByteSpan(buffer), kExpectedText1, ArraySize(kExpectedText1) }, + { nullptr, ByteSpan(buffer), kExpectedText1, ArraySize(kExpectedText1) }, + { "label", ByteSpan(buffer), kExpectedText2, ArraySize(kExpectedText2) }, + + // Empty buffer leads to a single label + { "label", ByteSpan(), kExpectedText3, ArraySize(kExpectedText3) }, + // Less than full single line works + { "label", ByteSpan(&buffer[0], 3), kExpectedText4, ArraySize(kExpectedText4) }, + }; + + for (auto testCase : kTestCases) + { + chip::Logging::SetLogRedirectCallback(&AccumulateLogLineCallback); + gRedirectedLogLines.clear(); + { + LogBufferAsHex(testCase.label, testCase.buffer); + } + chip::Logging::SetLogRedirectCallback(nullptr); + ValidateTextMatches(inSuite, testCase.expectedText, testCase.numLines, gRedirectedLogLines); + } +} + const nlTest sTests[] = { NL_TEST_DEF("TestBytesToHexNotNullTerminated", TestBytesToHexNotNullTerminated), // NL_TEST_DEF("TestBytesToHexNullTerminated", TestBytesToHexNullTerminated), // NL_TEST_DEF("TestBytesToHexErrors", TestBytesToHexErrors), // NL_TEST_DEF("TestBytesToHexUint64", TestBytesToHexUint64), // NL_TEST_DEF("TestHexToBytesAndUint", TestHexToBytesAndUint), // + NL_TEST_DEF("TestLogBufferAsHex", TestLogBufferAsHex), // NL_TEST_SENTINEL() // }; From 25827e8f944eaba1beba3f28d5432503fa0860d9 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 8 Sep 2022 10:48:19 -0400 Subject: [PATCH 07/20] Fix Android Tests in CI (#22464) * Enable android CI on push as well * Revert "Enable android CI on push as well" This reverts commit 3bcca58482b07fefedb86aa4becc518c0130f26a. * Add submodule checkout to full android: needed to set secure.directory * Distinct name for full android builds * Place the submodule checkout to the right location * Undo the wandalen retry removal Co-authored-by: Andrei Litvin --- .github/workflows/full-android.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/full-android.yaml b/.github/workflows/full-android.yaml index 704ad4145e47be..8a327d99066275 100644 --- a/.github/workflows/full-android.yaml +++ b/.github/workflows/full-android.yaml @@ -23,7 +23,7 @@ concurrency: cancel-in-progress: true jobs: - android: + full_android: name: Run timeout-minutes: 75 @@ -48,6 +48,8 @@ jobs: token: ${{ github.token }} attempt_limit: 3 attempt_delay: 2000 + - name: Checkout submodules + run: scripts/checkout_submodules.py --shallow --platform android - name: Bootstrap timeout-minutes: 10 run: scripts/build/gn_bootstrap.sh From 660786b28eab3ab3bdb2bca7dbea51078579ace7 Mon Sep 17 00:00:00 2001 From: Martin Turon Date: Thu, 8 Sep 2022 07:57:09 -0700 Subject: [PATCH 08/20] [build] Fix #21255 - allow circular initialization of SimpleStateMachine test. (#22461) * [build] Fix #21255 - allow circular initialization of SimpleStateMachine test. * [build] Add comment per review feedback. --- src/lib/support/tests/BUILD.gn | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/lib/support/tests/BUILD.gn b/src/lib/support/tests/BUILD.gn index 99cc2801355119..866fd6391feabd 100644 --- a/src/lib/support/tests/BUILD.gn +++ b/src/lib/support/tests/BUILD.gn @@ -60,7 +60,12 @@ chip_test_suite("tests") { test_sources += [ "TestCHIPArgParser.cpp" ] } - cflags = [ "-Wconversion" ] + cflags = [ + "-Wconversion", + + # TODO(#21255): work-around for SimpleStateMachine constructor issue. + "-Wno-error=uninitialized", + ] public_deps = [ "${chip_root}/src/credentials", From 0e3bbaea217be0e9c0b875a31a0a92ccb7ed6ae7 Mon Sep 17 00:00:00 2001 From: ShubhamMalasane <108782058+ShubhamMalasane@users.noreply.github.com> Date: Thu, 8 Sep 2022 21:12:08 +0530 Subject: [PATCH 09/20] [EFR32] window app implementation for wifi (#22451) * [EFR32] window app implementation for wifi * removed log message from common file * [EFR32] added comment --- examples/window-app/efr32/BUILD.gn | 4 +++ .../efr32/include/CHIPProjectConfig.h | 4 ++- .../window-app/efr32/src/WindowAppImpl.cpp | 33 ++++++++++++------- 3 files changed, 28 insertions(+), 13 deletions(-) diff --git a/examples/window-app/efr32/BUILD.gn b/examples/window-app/efr32/BUILD.gn index 08dabc3593696e..45250e7a46341f 100644 --- a/examples/window-app/efr32/BUILD.gn +++ b/examples/window-app/efr32/BUILD.gn @@ -35,6 +35,9 @@ declare_args() { # PIN code for PASE session establishment. setupPinCode = 20202021 + #default Discriminator value + setupDiscriminator = 3840 + # Monitor & log memory usage at runtime. enable_heap_monitoring = false @@ -128,6 +131,7 @@ efr32_sdk("sdk") { defines = [ "BOARD_ID=${efr32_board}", "CHIP_DEVICE_CONFIG_USE_TEST_SETUP_PIN_CODE=${setupPinCode}", + "CHIP_DEVICE_CONFIG_USE_TEST_SETUP_DISCRIMINATOR=${setupDiscriminator}", "OTA_PERIODIC_TIMEOUT=${OTA_periodic_query_timeout}", ] diff --git a/examples/window-app/efr32/include/CHIPProjectConfig.h b/examples/window-app/efr32/include/CHIPProjectConfig.h index 4f69721d3d971e..4621081b6d9d3d 100644 --- a/examples/window-app/efr32/include/CHIPProjectConfig.h +++ b/examples/window-app/efr32/include/CHIPProjectConfig.h @@ -32,8 +32,10 @@ #ifndef CHIP_DEVICE_CONFIG_USE_TEST_SETUP_PIN_CODE #define CHIP_DEVICE_CONFIG_USE_TEST_SETUP_PIN_CODE 20202021 #endif -#define CHIP_DEVICE_CONFIG_USE_TEST_SETUP_DISCRIMINATOR 0xF00 +#ifndef CHIP_DEVICE_CONFIG_USE_TEST_SETUP_DISCRIMINATOR +#define CHIP_DEVICE_CONFIG_USE_TEST_SETUP_DISCRIMINATOR 0xF00 +#endif // For convenience, Chip Security Test Mode can be enabled and the // requirement for authentication in various protocols can be disabled. // diff --git a/examples/window-app/efr32/src/WindowAppImpl.cpp b/examples/window-app/efr32/src/WindowAppImpl.cpp index 3f2cad957c068a..c4a1d3fd6762b8 100644 --- a/examples/window-app/efr32/src/WindowAppImpl.cpp +++ b/examples/window-app/efr32/src/WindowAppImpl.cpp @@ -33,6 +33,8 @@ #ifdef SL_WIFI #include "wfx_host_events.h" +#include +#include #endif #ifdef DISPLAY_ENABLED @@ -51,6 +53,10 @@ using namespace chip::app::Clusters::WindowCovering; #define APP_STATE_LED &sl_led_led0 #define APP_ACTION_LED &sl_led_led1 +#ifdef SL_WIFI +chip::app::Clusters::NetworkCommissioning::Instance + sWiFiNetworkCommissioningInstance(0 /* Endpoint Id */, &(chip::DeviceLayer::NetworkCommissioning::SlWiFiDriver::GetInstance())); +#endif //------------------------------------------------------------------------------ // Timers //------------------------------------------------------------------------------ @@ -145,6 +151,21 @@ WindowAppImpl::WindowAppImpl() {} void WindowAppImpl::OnTaskCallback(void * parameter) { +#ifdef SL_WIFI + /* + * Wait for the WiFi to be initialized + */ + EFR32_LOG("APP: Wait WiFi Init"); + while (!wfx_hw_ready()) + { + vTaskDelay(10); + } + EFR32_LOG("APP: Done WiFi Init"); + /* We will init server when we get IP */ + sWiFiNetworkCommissioningInstance.Init(); + /* added for commisioning with wifi */ +#endif + sInstance.Run(); } @@ -158,18 +179,6 @@ void WindowAppImpl::OnIconTimeout(WindowApp::Timer & timer) CHIP_ERROR WindowAppImpl::Init() { -#ifdef SL_WIFI - /* - * Wait for the WiFi to be initialized - */ - EFR32_LOG("APP: Wait WiFi Init"); - while (!wfx_hw_ready()) - { - vTaskDelay(10); - } - EFR32_LOG("APP: Done WiFi Init"); - /* We will init server when we get IP */ -#endif WindowApp::Init(); // Initialize App Task From 03ca443ecdbd5edf5be3e8e05d25ffc48dd616b0 Mon Sep 17 00:00:00 2001 From: srningap <107042150+srningap@users.noreply.github.com> Date: Fri, 9 Sep 2022 00:17:15 +0530 Subject: [PATCH 10/20] [EFR32] multi-admin fix for rs911x (#22480) --- examples/chef/efr32/BUILD.gn | 2 +- examples/light-switch-app/efr32/BUILD.gn | 2 +- examples/lighting-app/efr32/BUILD.gn | 2 +- examples/lock-app/efr32/BUILD.gn | 2 +- examples/shell/efr32/BUILD.gn | 2 +- examples/thermostat/efr32/BUILD.gn | 2 +- examples/window-app/efr32/BUILD.gn | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/examples/chef/efr32/BUILD.gn b/examples/chef/efr32/BUILD.gn index 55abfcac3d219e..82eec623267b03 100644 --- a/examples/chef/efr32/BUILD.gn +++ b/examples/chef/efr32/BUILD.gn @@ -194,7 +194,7 @@ efr32_executable("chef_app") { ] if (chip_enable_pw_rpc || chip_build_libshell || enable_openthread_cli || - use_wf200) { + use_wf200 || use_rs911x) { sources += [ "${examples_plat_dir}/uart.cpp" ] } diff --git a/examples/light-switch-app/efr32/BUILD.gn b/examples/light-switch-app/efr32/BUILD.gn index 07bcfe6c204e7f..2833eefc47c8a6 100644 --- a/examples/light-switch-app/efr32/BUILD.gn +++ b/examples/light-switch-app/efr32/BUILD.gn @@ -187,7 +187,7 @@ efr32_executable("light_switch_app") { ] if (chip_enable_pw_rpc || chip_build_libshell || enable_openthread_cli || - use_wf200) { + use_wf200 || use_rs911x) { sources += [ "${examples_plat_dir}/uart.cpp" ] } diff --git a/examples/lighting-app/efr32/BUILD.gn b/examples/lighting-app/efr32/BUILD.gn index 4db2f6a8f1afeb..ad9c9f0feeaa0d 100644 --- a/examples/lighting-app/efr32/BUILD.gn +++ b/examples/lighting-app/efr32/BUILD.gn @@ -192,7 +192,7 @@ efr32_executable("lighting_app") { ] if (chip_enable_pw_rpc || chip_build_libshell || enable_openthread_cli || - use_wf200) { + use_wf200 || use_rs911x) { sources += [ "${examples_plat_dir}/uart.cpp" ] } diff --git a/examples/lock-app/efr32/BUILD.gn b/examples/lock-app/efr32/BUILD.gn index 52862d739841f1..d633e177a36b17 100644 --- a/examples/lock-app/efr32/BUILD.gn +++ b/examples/lock-app/efr32/BUILD.gn @@ -186,7 +186,7 @@ efr32_executable("lock_app") { ] if (chip_enable_pw_rpc || chip_build_libshell || enable_openthread_cli || - use_wf200) { + use_wf200 || use_rs911x) { sources += [ "${examples_plat_dir}/uart.cpp" ] } diff --git a/examples/shell/efr32/BUILD.gn b/examples/shell/efr32/BUILD.gn index 842504ae7dd0b0..33bbfac5e4573d 100644 --- a/examples/shell/efr32/BUILD.gn +++ b/examples/shell/efr32/BUILD.gn @@ -62,7 +62,7 @@ efr32_executable("shell_app") { ] if (chip_enable_pw_rpc || chip_build_libshell || enable_openthread_cli || - use_wf200) { + use_wf200 || use_rs911x) { sources += [ "${examples_plat_dir}/uart.cpp" ] } diff --git a/examples/thermostat/efr32/BUILD.gn b/examples/thermostat/efr32/BUILD.gn index 2146ef417a0831..e787b107e26d3c 100644 --- a/examples/thermostat/efr32/BUILD.gn +++ b/examples/thermostat/efr32/BUILD.gn @@ -183,7 +183,7 @@ efr32_executable("thermostat_app") { ] if (chip_enable_pw_rpc || chip_build_libshell || enable_openthread_cli || - use_wf200) { + use_wf200 || use_rs911x) { sources += [ "${examples_plat_dir}/uart.cpp" ] } diff --git a/examples/window-app/efr32/BUILD.gn b/examples/window-app/efr32/BUILD.gn index 45250e7a46341f..a993091d59da1b 100644 --- a/examples/window-app/efr32/BUILD.gn +++ b/examples/window-app/efr32/BUILD.gn @@ -183,7 +183,7 @@ efr32_executable("window_app") { "src/main.cpp", ] - if (chip_build_libshell || enable_openthread_cli || use_wf200) { + if (chip_build_libshell || enable_openthread_cli || use_wf200 || use_rs911x) { sources += [ "${examples_plat_dir}/uart.cpp" ] } From e74bb9488f7ebbf2356d9be290578bb56bc23c7e Mon Sep 17 00:00:00 2001 From: yunhanw-google Date: Thu, 8 Sep 2022 12:24:57 -0700 Subject: [PATCH 11/20] Prevent Reads/Subscription to attributes with no access privilege (#22349) --- src/app/InteractionModelEngine.cpp | 142 +++++++-- src/app/InteractionModelEngine.h | 22 ++ src/app/MessageDef/AttributePathIB.cpp | 57 ++++ src/app/MessageDef/AttributePathIB.h | 8 + src/app/MessageDef/EventPathIB.cpp | 46 +++ src/app/MessageDef/EventPathIB.h | 8 + src/app/ReadHandler.cpp | 114 +------ src/app/ReadHandler.h | 2 +- src/app/tests/BUILD.gn | 2 + src/app/tests/TestAclAttribute.cpp | 283 ++++++++++++++++++ src/app/tests/TestAclEvent.cpp | 8 +- src/app/tests/TestReadInteraction.cpp | 47 +++ .../tests/integration/chip_im_initiator.cpp | 5 + .../tests/integration/chip_im_responder.cpp | 5 + .../util/ember-compatibility-functions.cpp | 12 + src/controller/tests/TestReadChunking.cpp | 29 +- src/controller/tests/data_model/TestRead.cpp | 56 +++- src/darwin/Framework/CHIP/MTRBaseDevice.mm | 8 +- src/darwin/Framework/CHIP/MTRIMDispatch.mm | 5 + .../Framework/CHIPTests/MTRDeviceTests.m | 10 +- 20 files changed, 706 insertions(+), 163 deletions(-) create mode 100644 src/app/tests/TestAclAttribute.cpp diff --git a/src/app/InteractionModelEngine.cpp b/src/app/InteractionModelEngine.cpp index 3f88c40f5f9910..61af4c1b19b77b 100644 --- a/src/app/InteractionModelEngine.cpp +++ b/src/app/InteractionModelEngine.cpp @@ -27,6 +27,9 @@ #include +#include "access/RequestPath.h" +#include "access/SubjectDescriptor.h" +#include #include #include @@ -323,6 +326,79 @@ Status InteractionModelEngine::OnInvokeCommandRequest(Messaging::ExchangeContext return Status::Success; } +CHIP_ERROR InteractionModelEngine::ParseAttributePaths(const Access::SubjectDescriptor & aSubjectDescriptor, + AttributePathIBs::Parser & aAttributePathListParser, + bool & aHasValidAttributePath, size_t & aRequestedAttributePathCount) +{ + TLV::TLVReader pathReader; + aAttributePathListParser.GetReader(&pathReader); + CHIP_ERROR err = CHIP_NO_ERROR; + + aHasValidAttributePath = false; + aRequestedAttributePathCount = 0; + + while (CHIP_NO_ERROR == (err = pathReader.Next())) + { + VerifyOrReturnError(TLV::AnonymousTag() == pathReader.GetTag(), CHIP_ERROR_INVALID_TLV_TAG); + + AttributePathIB::Parser path; + // + // We create an iterator to point to a single item object list that tracks the path we just parsed. + // This avoids the 'parse all paths' approach that is employed in ReadHandler since we want to + // avoid allocating out of the path store during this minimal initial processing stage. + // + ObjectList paramsList; + + ReturnErrorOnFailure(path.Init(pathReader)); + ReturnErrorOnFailure(path.ParsePath(paramsList.mValue)); + + if (paramsList.mValue.IsWildcardPath()) + { + AttributePathExpandIterator pathIterator(¶msList); + ConcreteAttributePath readPath; + + // The definition of "valid path" is "path exists and ACL allows access". The "path exists" part is handled by + // AttributePathExpandIterator. So we just need to check the ACL bits. + for (; pathIterator.Get(readPath); pathIterator.Next()) + { + Access::RequestPath requestPath{ .cluster = readPath.mClusterId, .endpoint = readPath.mEndpointId }; + err = Access::GetAccessControl().Check(aSubjectDescriptor, requestPath, + RequiredPrivilege::ForReadAttribute(readPath)); + if (err == CHIP_NO_ERROR) + { + aHasValidAttributePath = true; + break; + } + } + } + else + { + ConcreteAttributePath concretePath(paramsList.mValue.mEndpointId, paramsList.mValue.mClusterId, + paramsList.mValue.mAttributeId); + if (ConcreteAttributePathExists(concretePath)) + { + Access::RequestPath requestPath{ .cluster = concretePath.mClusterId, .endpoint = concretePath.mEndpointId }; + + err = Access::GetAccessControl().Check(aSubjectDescriptor, requestPath, + RequiredPrivilege::ForReadAttribute(concretePath)); + if (err == CHIP_NO_ERROR) + { + aHasValidAttributePath = true; + } + } + } + + aRequestedAttributePathCount++; + } + + if (err == CHIP_ERROR_END_OF_TLV) + { + err = CHIP_NO_ERROR; + } + + return err; +} + Protocols::InteractionModel::Status InteractionModelEngine::OnReadInitialRequest(Messaging::ExchangeContext * apExchangeContext, const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload, @@ -349,28 +425,25 @@ Protocols::InteractionModel::Status InteractionModelEngine::OnReadInitialRequest reader.Init(aPayload.Retain()); SubscribeRequestMessage::Parser subscribeRequestParser; - CHIP_ERROR err = subscribeRequestParser.Init(reader); - if (err != CHIP_NO_ERROR) - { - return Status::InvalidAction; - } - + VerifyOrReturnError(subscribeRequestParser.Init(reader) == CHIP_NO_ERROR, Status::InvalidAction); { size_t requestedAttributePathCount = 0; size_t requestedEventPathCount = 0; AttributePathIBs::Parser attributePathListParser; - err = subscribeRequestParser.GetAttributeRequests(&attributePathListParser); + bool hasValidAttributePath = false; + + CHIP_ERROR err = subscribeRequestParser.GetAttributeRequests(&attributePathListParser); if (err == CHIP_NO_ERROR) { - TLV::TLVReader pathReader; - attributePathListParser.GetReader(&pathReader); - err = TLV::Utilities::Count(pathReader, requestedAttributePathCount, false); - } - else if (err == CHIP_ERROR_END_OF_TLV) - { - err = CHIP_NO_ERROR; + auto subjectDescriptor = apExchangeContext->GetSessionHandle()->AsSecureSession()->GetSubjectDescriptor(); + err = ParseAttributePaths(subjectDescriptor, attributePathListParser, hasValidAttributePath, + requestedAttributePathCount); + if (err != CHIP_NO_ERROR) + { + return Status::InvalidAction; + } } - if (err != CHIP_NO_ERROR) + else if (err != CHIP_ERROR_END_OF_TLV) { return Status::InvalidAction; } @@ -381,14 +454,34 @@ Protocols::InteractionModel::Status InteractionModelEngine::OnReadInitialRequest { TLV::TLVReader pathReader; eventpathListParser.GetReader(&pathReader); - err = TLV::Utilities::Count(pathReader, requestedEventPathCount, false); + ReturnErrorCodeIf(TLV::Utilities::Count(pathReader, requestedEventPathCount, false) != CHIP_NO_ERROR, + Status::InvalidAction); } - else if (err == CHIP_ERROR_END_OF_TLV) + else if (err != CHIP_ERROR_END_OF_TLV) { - err = CHIP_NO_ERROR; + return Status::InvalidAction; } - if (err != CHIP_NO_ERROR) + + if (requestedAttributePathCount == 0 && requestedEventPathCount == 0) { + ChipLogError(InteractionModel, + "Subscription from [%u:" ChipLogFormatX64 "] has no attribute or event paths. Rejecting request.", + apExchangeContext->GetSessionHandle()->GetFabricIndex(), + ChipLogValueX64(apExchangeContext->GetSessionHandle()->AsSecureSession()->GetPeerNodeId())); + return Status::InvalidAction; + } + + // + // TODO: We don't have an easy way to do a similar 'path expansion' for events to deduce + // access so for now, assume that the presence of any path in the event list means they + // might have some access to those events. + // + if (!hasValidAttributePath && requestedEventPathCount == 0) + { + ChipLogError(InteractionModel, + "Subscription from [%u:" ChipLogFormatX64 "] has no access at all. Rejecting request.", + apExchangeContext->GetSessionHandle()->GetFabricIndex(), + ChipLogValueX64(apExchangeContext->GetSessionHandle()->AsSecureSession()->GetPeerNodeId())); return Status::InvalidAction; } @@ -400,12 +493,8 @@ Protocols::InteractionModel::Status InteractionModelEngine::OnReadInitialRequest } } - err = subscribeRequestParser.GetKeepSubscriptions(&keepExistingSubscriptions); - if (err != CHIP_NO_ERROR) - { - return Status::InvalidAction; - } - + VerifyOrReturnError(subscribeRequestParser.GetKeepSubscriptions(&keepExistingSubscriptions) == CHIP_NO_ERROR, + Status::InvalidAction); if (!keepExistingSubscriptions) { // @@ -426,8 +515,7 @@ Protocols::InteractionModel::Status InteractionModelEngine::OnReadInitialRequest }); } } - - if (aInteractionType == ReadHandler::InteractionType::Read) + else { System::PacketBufferTLVReader reader; reader.Init(aPayload.Retain()); diff --git a/src/app/InteractionModelEngine.h b/src/app/InteractionModelEngine.h index 0bd2c6738a3818..6475f8bceda547 100644 --- a/src/app/InteractionModelEngine.h +++ b/src/app/InteractionModelEngine.h @@ -378,6 +378,21 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler, System::PacketBufferHandle && aPayload) override; void OnResponseTimeout(Messaging::ExchangeContext * ec) override; + /** + * This parses the attribute path list to ensure it is well formed. If so, for each path in the list, it will expand to a list + * of concrete paths and walk each path to check if it has privileges to read that attribute. + * + * If there is AT LEAST one "existent path" (as the spec calls it) that has sufficient privilege, aHasValidAttributePath + * will be set to true. Otherwise, it will be set to false. + * + * aRequestedAttributePathCount will be updated to reflect the number of attribute paths in the request. + * + * + */ + static CHIP_ERROR ParseAttributePaths(const Access::SubjectDescriptor & aSubjectDescriptor, + AttributePathIBs::Parser & aAttributePathListParser, bool & aHasValidAttributePath, + size_t & aRequestedAttributePathCount); + /** * Called when Interaction Model receives a Read Request message. Errors processing * the Read Request are handled entirely within this function. If the @@ -617,6 +632,13 @@ CHIP_ERROR ReadSingleClusterData(const Access::SubjectDescriptor & aSubjectDescr const ConcreteReadAttributePath & aPath, AttributeReportIBs::Builder & aAttributeReports, AttributeValueEncoder::AttributeEncodeState * apEncoderState); +/** + * Check whether concrete attribute path is an "existent attribute path" in spec terms. + * @param[in] aPath The concrete path of the data being read. + * @retval boolean + */ +bool ConcreteAttributePathExists(const ConcreteAttributePath & aPath); + /** * Get the registered attribute access override. nullptr when attribute access override is not found. * diff --git a/src/app/MessageDef/AttributePathIB.cpp b/src/app/MessageDef/AttributePathIB.cpp index c8d0fa51d29f4c..97d7f60568af0e 100644 --- a/src/app/MessageDef/AttributePathIB.cpp +++ b/src/app/MessageDef/AttributePathIB.cpp @@ -227,6 +227,63 @@ CHIP_ERROR AttributePathIB::Parser::GetListIndex(ConcreteDataAttributePath & aAt return err; } +CHIP_ERROR AttributePathIB::Parser::ParsePath(AttributePathParams & aAttribute) const +{ + CHIP_ERROR err = CHIP_NO_ERROR; + + err = GetEndpoint(&(aAttribute.mEndpointId)); + if (err == CHIP_NO_ERROR) + { + VerifyOrReturnError(!aAttribute.HasWildcardEndpointId(), CHIP_IM_GLOBAL_STATUS(InvalidAction)); + } + else if (err == CHIP_END_OF_TLV) + { + err = CHIP_NO_ERROR; + } + VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_IM_GLOBAL_STATUS(InvalidAction)); + + err = GetCluster(&aAttribute.mClusterId); + if (err == CHIP_NO_ERROR) + { + VerifyOrReturnError(IsValidClusterId(aAttribute.mClusterId), CHIP_IM_GLOBAL_STATUS(InvalidAction)); + } + else if (err == CHIP_END_OF_TLV) + { + err = CHIP_NO_ERROR; + } + VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_IM_GLOBAL_STATUS(InvalidAction)); + + err = GetAttribute(&aAttribute.mAttributeId); + if (err == CHIP_NO_ERROR) + { + VerifyOrReturnError(IsValidAttributeId(aAttribute.mAttributeId), CHIP_IM_GLOBAL_STATUS(InvalidAction)); + } + else if (err == CHIP_END_OF_TLV) + { + err = CHIP_NO_ERROR; + } + VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_IM_GLOBAL_STATUS(InvalidAction)); + + // A wildcard cluster requires that the attribute path either be + // wildcard or a global attribute. + VerifyOrReturnError(!aAttribute.HasWildcardClusterId() || aAttribute.HasWildcardAttributeId() || + IsGlobalAttribute(aAttribute.mAttributeId), + CHIP_IM_GLOBAL_STATUS(InvalidAction)); + + err = GetListIndex(&aAttribute.mListIndex); + if (err == CHIP_NO_ERROR) + { + VerifyOrReturnError(!aAttribute.HasWildcardAttributeId() && !aAttribute.HasWildcardListIndex(), + CHIP_IM_GLOBAL_STATUS(InvalidAction)); + } + else if (err == CHIP_END_OF_TLV) + { + err = CHIP_NO_ERROR; + } + VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_IM_GLOBAL_STATUS(InvalidAction)); + return CHIP_NO_ERROR; +} + AttributePathIB::Builder & AttributePathIB::Builder::EnableTagCompression(const bool aEnableTagCompression) { // skip if error has already been set diff --git a/src/app/MessageDef/AttributePathIB.h b/src/app/MessageDef/AttributePathIB.h index e5192ccb119739..81b1a33e58a904 100644 --- a/src/app/MessageDef/AttributePathIB.h +++ b/src/app/MessageDef/AttributePathIB.h @@ -153,6 +153,14 @@ class Parser : public ListParser CHIP_ERROR GetListIndex(ConcreteDataAttributePath & aAttributePath) const; // TODO(#14934) Add a function to get ConcreteDataAttributePath from AttributePathIB::Parser directly. + + /** + * @brief Parse the attribute path into an AttributePathParams object. As part of parsing, + * validity checks for each path item will be done as well. + * + * If any errors are encountered, an IM error of 'InvalidAction' will be returned. + */ + CHIP_ERROR ParsePath(AttributePathParams & attribute) const; }; class Builder : public ListBuilder diff --git a/src/app/MessageDef/EventPathIB.cpp b/src/app/MessageDef/EventPathIB.cpp index 35c5b098b94c55..84c9f6680bf260 100644 --- a/src/app/MessageDef/EventPathIB.cpp +++ b/src/app/MessageDef/EventPathIB.cpp @@ -26,6 +26,8 @@ #include +#include + namespace chip { namespace app { #if CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK @@ -171,6 +173,50 @@ CHIP_ERROR EventPathIB::Parser::GetIsUrgent(bool * const apIsUrgent) const return GetSimpleValue(to_underlying(Tag::kIsUrgent), TLV::kTLVType_Boolean, apIsUrgent); } +CHIP_ERROR EventPathIB::Parser::ParsePath(EventPathParams & aEvent) const +{ + CHIP_ERROR err = GetEndpoint(&(aEvent.mEndpointId)); + if (err == CHIP_NO_ERROR) + { + VerifyOrReturnError(!aEvent.HasWildcardEndpointId(), CHIP_IM_GLOBAL_STATUS(InvalidAction)); + } + else if (err == CHIP_END_OF_TLV) + { + err = CHIP_NO_ERROR; + } + VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_IM_GLOBAL_STATUS(InvalidAction)); + + err = GetCluster(&(aEvent.mClusterId)); + if (err == CHIP_NO_ERROR) + { + VerifyOrReturnError(!aEvent.HasWildcardClusterId(), CHIP_IM_GLOBAL_STATUS(InvalidAction)); + } + else if (err == CHIP_END_OF_TLV) + { + err = CHIP_NO_ERROR; + } + VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_IM_GLOBAL_STATUS(InvalidAction)); + + err = GetEvent(&(aEvent.mEventId)); + if (CHIP_END_OF_TLV == err) + { + err = CHIP_NO_ERROR; + } + else if (err == CHIP_NO_ERROR) + { + VerifyOrReturnError(!aEvent.HasWildcardEventId(), CHIP_IM_GLOBAL_STATUS(InvalidAction)); + } + VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_IM_GLOBAL_STATUS(InvalidAction)); + + err = GetIsUrgent(&(aEvent.mIsUrgentEvent)); + if (CHIP_END_OF_TLV == err) + { + err = CHIP_NO_ERROR; + } + VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_IM_GLOBAL_STATUS(InvalidAction)); + return CHIP_NO_ERROR; +} + EventPathIB::Builder & EventPathIB::Builder::Node(const NodeId aNode) { // skip if error has already been set diff --git a/src/app/MessageDef/EventPathIB.h b/src/app/MessageDef/EventPathIB.h index 32b7948dfc6d54..0ad2000938ac86 100644 --- a/src/app/MessageDef/EventPathIB.h +++ b/src/app/MessageDef/EventPathIB.h @@ -127,6 +127,14 @@ class Parser : public ListParser * #CHIP_ERROR_IM_MALFORMED_EVENT_PATH_IB if the path from the reader is not a valid concrere event path. */ CHIP_ERROR GetEventPath(ConcreteEventPath * const apPath) const; + + /** + * @brief Parse the event path into an EventPathParams object. As part of parsing, + * validity checks for each path item will be done as well. + * + * If any errors are encountered, an IM error of 'InvalidAction' will be returned. + */ + CHIP_ERROR ParsePath(EventPathParams & aEvent) const; }; class Builder : public ListBuilder diff --git a/src/app/ReadHandler.cpp b/src/app/ReadHandler.cpp index 913dcce726ca10..64bcbb1a7e516b 100644 --- a/src/app/ReadHandler.cpp +++ b/src/app/ReadHandler.cpp @@ -315,7 +315,7 @@ CHIP_ERROR ReadHandler::ProcessReadRequest(System::PacketBufferHandle && aPayloa } else if (err == CHIP_NO_ERROR) { - ReturnErrorOnFailure(ProcessAttributePathList(attributePathListParser)); + ReturnErrorOnFailure(ProcessAttributePaths(attributePathListParser)); DataVersionFilterIBs::Parser dataVersionFilterListParser; err = readRequestParser.GetDataVersionFilters(&dataVersionFilterListParser); if (err == CHIP_END_OF_TLV) @@ -363,75 +363,19 @@ CHIP_ERROR ReadHandler::ProcessReadRequest(System::PacketBufferHandle && aPayloa return CHIP_NO_ERROR; } -CHIP_ERROR ReadHandler::ProcessAttributePathList(AttributePathIBs::Parser & aAttributePathListParser) +CHIP_ERROR ReadHandler::ProcessAttributePaths(AttributePathIBs::Parser & aAttributePathListParser) { CHIP_ERROR err = CHIP_NO_ERROR; TLV::TLVReader reader; aAttributePathListParser.GetReader(&reader); while (CHIP_NO_ERROR == (err = reader.Next())) { - VerifyOrExit(TLV::AnonymousTag() == reader.GetTag(), err = CHIP_ERROR_INVALID_TLV_TAG); + VerifyOrReturnError(TLV::AnonymousTag() == reader.GetTag(), CHIP_ERROR_INVALID_TLV_TAG); AttributePathParams attribute; AttributePathIB::Parser path; - err = path.Init(reader); - SuccessOrExit(err); - - err = path.GetEndpoint(&(attribute.mEndpointId)); - if (err == CHIP_NO_ERROR) - { - VerifyOrExit(!attribute.HasWildcardEndpointId(), err = CHIP_IM_GLOBAL_STATUS(InvalidAction)); - } - else if (err == CHIP_END_OF_TLV) - { - err = CHIP_NO_ERROR; - } - SuccessOrExit(err); - - ClusterId clusterId = kInvalidClusterId; - err = path.GetCluster(&clusterId); - if (err == CHIP_NO_ERROR) - { - VerifyOrExit(IsValidClusterId(clusterId), err = CHIP_IM_GLOBAL_STATUS(InvalidAction)); - attribute.mClusterId = clusterId; - } - else if (err == CHIP_END_OF_TLV) - { - err = CHIP_NO_ERROR; - } - SuccessOrExit(err); - - AttributeId attributeId = kInvalidAttributeId; - err = path.GetAttribute(&attributeId); - if (CHIP_END_OF_TLV == err) - { - err = CHIP_NO_ERROR; - } - else if (err == CHIP_NO_ERROR) - { - VerifyOrExit(IsValidAttributeId(attributeId), err = CHIP_IM_GLOBAL_STATUS(InvalidAction)); - attribute.mAttributeId = attributeId; - } - SuccessOrExit(err); - - // A wildcard cluster requires that the attribute path either be - // wildcard or a global attribute. - VerifyOrExit(!attribute.HasWildcardClusterId() || attribute.HasWildcardAttributeId() || - IsGlobalAttribute(attribute.mAttributeId), - err = CHIP_IM_GLOBAL_STATUS(InvalidAction)); - - err = path.GetListIndex(&(attribute.mListIndex)); - if (CHIP_NO_ERROR == err) - { - VerifyOrExit(!attribute.HasWildcardAttributeId() && !attribute.HasWildcardListIndex(), - err = CHIP_IM_GLOBAL_STATUS(InvalidAction)); - } - else if (CHIP_END_OF_TLV == err) - { - err = CHIP_NO_ERROR; - } - SuccessOrExit(err); - err = InteractionModelEngine::GetInstance()->PushFrontAttributePathList(mpAttributePathList, attribute); - SuccessOrExit(err); + ReturnErrorOnFailure(path.Init(reader)); + ReturnErrorOnFailure(path.ParsePath(attribute)); + ReturnErrorOnFailure(InteractionModelEngine::GetInstance()->PushFrontAttributePathList(mpAttributePathList, attribute)); } // if we have exhausted this container if (CHIP_END_OF_TLV == err) @@ -440,8 +384,6 @@ CHIP_ERROR ReadHandler::ProcessAttributePathList(AttributePathIBs::Parser & aAtt mAttributePathExpandIterator = AttributePathExpandIterator(mpAttributePathList); err = CHIP_NO_ERROR; } - -exit: return err; } @@ -487,47 +429,7 @@ CHIP_ERROR ReadHandler::ProcessEventPaths(EventPathIBs::Parser & aEventPathsPars EventPathParams event; EventPathIB::Parser path; ReturnErrorOnFailure(path.Init(reader)); - - err = path.GetEndpoint(&(event.mEndpointId)); - if (err == CHIP_NO_ERROR) - { - VerifyOrReturnError(!event.HasWildcardEndpointId(), err = CHIP_ERROR_IM_MALFORMED_EVENT_PATH_IB); - } - else if (err == CHIP_END_OF_TLV) - { - err = CHIP_NO_ERROR; - } - ReturnErrorOnFailure(err); - - err = path.GetCluster(&(event.mClusterId)); - if (err == CHIP_NO_ERROR) - { - VerifyOrReturnError(!event.HasWildcardClusterId(), err = CHIP_ERROR_IM_MALFORMED_EVENT_PATH_IB); - } - else if (err == CHIP_END_OF_TLV) - { - err = CHIP_NO_ERROR; - } - ReturnErrorOnFailure(err); - - err = path.GetEvent(&(event.mEventId)); - if (CHIP_END_OF_TLV == err) - { - err = CHIP_NO_ERROR; - } - else if (err == CHIP_NO_ERROR) - { - VerifyOrReturnError(!event.HasWildcardEventId(), err = CHIP_ERROR_IM_MALFORMED_EVENT_PATH_IB); - } - ReturnErrorOnFailure(err); - - err = path.GetIsUrgent(&(event.mIsUrgentEvent)); - if (CHIP_END_OF_TLV == err) - { - err = CHIP_NO_ERROR; - } - ReturnErrorOnFailure(err); - + ReturnErrorOnFailure(path.ParsePath(event)); ReturnErrorOnFailure(InteractionModelEngine::GetInstance()->PushFrontEventPathParamsList(mpEventPathList, event)); } @@ -667,7 +569,7 @@ CHIP_ERROR ReadHandler::ProcessSubscribeRequest(System::PacketBufferHandle && aP } else if (err == CHIP_NO_ERROR) { - ReturnErrorOnFailure(ProcessAttributePathList(attributePathListParser)); + ReturnErrorOnFailure(ProcessAttributePaths(attributePathListParser)); DataVersionFilterIBs::Parser dataVersionFilterListParser; err = subscribeRequestParser.GetDataVersionFilters(&dataVersionFilterListParser); if (err == CHIP_END_OF_TLV) diff --git a/src/app/ReadHandler.h b/src/app/ReadHandler.h index b07ef01b2c956a..7e32fa920a2636 100644 --- a/src/app/ReadHandler.h +++ b/src/app/ReadHandler.h @@ -362,7 +362,7 @@ class ReadHandler : public Messaging::ExchangeDelegate CHIP_ERROR SendSubscribeResponse(); CHIP_ERROR ProcessSubscribeRequest(System::PacketBufferHandle && aPayload); CHIP_ERROR ProcessReadRequest(System::PacketBufferHandle && aPayload); - CHIP_ERROR ProcessAttributePathList(AttributePathIBs::Parser & aAttributePathListParser); + CHIP_ERROR ProcessAttributePaths(AttributePathIBs::Parser & aAttributePathListParser); CHIP_ERROR ProcessEventPaths(EventPathIBs::Parser & aEventPathsParser); CHIP_ERROR ProcessEventFilters(EventFilterIBs::Parser & aEventFiltersParser); CHIP_ERROR OnStatusResponse(Messaging::ExchangeContext * apExchangeContext, System::PacketBufferHandle && aPayload, diff --git a/src/app/tests/BUILD.gn b/src/app/tests/BUILD.gn index 7d57e72ebb8667..d11f29a66c72c2 100644 --- a/src/app/tests/BUILD.gn +++ b/src/app/tests/BUILD.gn @@ -102,6 +102,8 @@ chip_test_suite("tests") { test_sources += [ "TestFailSafeContext.cpp" ] } + test_sources += [ "TestAclAttribute.cpp" ] + # # On NRF platforms, the allocation of a large number of pbufs in this test # to exercise chunking causes it to run out of memory. For now, disable it there. diff --git a/src/app/tests/TestAclAttribute.cpp b/src/app/tests/TestAclAttribute.cpp new file mode 100644 index 00000000000000..ca51e96401ebdc --- /dev/null +++ b/src/app/tests/TestAclAttribute.cpp @@ -0,0 +1,283 @@ +/* + * + * Copyright (c) 2021 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "lib/support/CHIPMem.h" +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +namespace { +using namespace chip; +using namespace chip::Access; + +chip::ClusterId kTestClusterId = 1; +chip::ClusterId kTestDeniedClusterId1 = 1000; +chip::ClusterId kTestDeniedClusterId2 = 3; +chip::EndpointId kTestEndpointId = 4; + +class TestAccessControlDelegate : public AccessControl::Delegate +{ +public: + CHIP_ERROR Check(const SubjectDescriptor & subjectDescriptor, const chip::Access::RequestPath & requestPath, + Privilege requestPrivilege) override + { + if (requestPath.cluster == kTestDeniedClusterId2) + { + return CHIP_ERROR_ACCESS_DENIED; + } + return CHIP_NO_ERROR; + } +}; + +AccessControl::Delegate * GetTestAccessControlDelegate() +{ + static TestAccessControlDelegate accessControlDelegate; + return &accessControlDelegate; +} + +class TestDeviceTypeResolver : public AccessControl::DeviceTypeResolver +{ +public: + bool IsDeviceTypeOnEndpoint(DeviceTypeId deviceType, EndpointId endpoint) override { return false; } +} gDeviceTypeResolver; + +class TestAccessContext : public chip::Test::AppContext +{ +public: + static int Initialize(void * context) + { + if (AppContext::Initialize(context) != SUCCESS) + return FAILURE; + Access::GetAccessControl().Finish(); + Access::GetAccessControl().Init(GetTestAccessControlDelegate(), gDeviceTypeResolver); + return SUCCESS; + } + + static int Finalize(void * context) + { + if (AppContext::Finalize(context) != SUCCESS) + return FAILURE; + return SUCCESS; + } + +private: + chip::MonotonicallyIncreasingCounter mEventCounter; +}; + +class MockInteractionModelApp : public chip::app::ReadClient::Callback +{ +public: + void OnAttributeData(const chip::app::ConcreteDataAttributePath & aPath, chip::TLV::TLVReader * apData, + const chip::app::StatusIB & status) override + { + mGotReport = true; + mLastStatusReceived = status; + } + + void OnError(CHIP_ERROR aError) override { mError = aError; } + + void OnDone(chip::app::ReadClient *) override {} + + void OnDeallocatePaths(chip::app::ReadPrepareParams && aReadPrepareParams) override + { + if (aReadPrepareParams.mpAttributePathParamsList != nullptr) + { + delete[] aReadPrepareParams.mpAttributePathParamsList; + } + + if (aReadPrepareParams.mpDataVersionFilterList != nullptr) + { + delete[] aReadPrepareParams.mpDataVersionFilterList; + } + } + + bool mGotReport = false; + chip::app::StatusIB mLastStatusReceived; + CHIP_ERROR mError = CHIP_NO_ERROR; +}; +} // namespace + +namespace chip { +namespace app { + +bool ConcreteAttributePathExists(const ConcreteAttributePath & aPath) +{ + return aPath.mClusterId != kTestDeniedClusterId1; +} + +class TestAclAttribute +{ +public: + static void TestACLDeniedAttribute(nlTestSuite * apSuite, void * apContext); +}; + +// Read Client sends a malformed subscribe request, interaction model engine fails to parse the request and generates a status +// report to client, and client is closed. +void TestAclAttribute::TestACLDeniedAttribute(nlTestSuite * apSuite, void * apContext) +{ + TestAccessContext & ctx = *static_cast(apContext); + CHIP_ERROR err = CHIP_NO_ERROR; + + Messaging::ReliableMessageMgr * rm = ctx.GetExchangeManager().GetReliableMessageMgr(); + NL_TEST_ASSERT(apSuite, rm->TestGetCountRetransTable() == 0); + + MockInteractionModelApp delegate; + auto * engine = chip::app::InteractionModelEngine::GetInstance(); + err = engine->Init(&ctx.GetExchangeManager(), &ctx.GetFabricTable()); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + { + app::ReadClient readClient(chip::app::InteractionModelEngine::GetInstance(), &ctx.GetExchangeManager(), delegate, + chip::app::ReadClient::InteractionType::Subscribe); + + chip::app::AttributePathParams attributePathParams[2]; + attributePathParams[0].mEndpointId = kTestEndpointId; + attributePathParams[0].mClusterId = kTestDeniedClusterId1; + attributePathParams[0].mAttributeId = 1; + + attributePathParams[1].mEndpointId = kTestEndpointId; + attributePathParams[1].mClusterId = kTestDeniedClusterId1; + attributePathParams[1].mAttributeId = 2; + + ReadPrepareParams readPrepareParams(ctx.GetSessionBobToAlice()); + readPrepareParams.mpAttributePathParamsList = attributePathParams; + readPrepareParams.mAttributePathParamsListSize = 2; + + err = readClient.SendRequest(readPrepareParams); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + ctx.DrainAndServiceIO(); + NL_TEST_ASSERT(apSuite, delegate.mError == CHIP_IM_GLOBAL_STATUS(InvalidAction)); + NL_TEST_ASSERT(apSuite, !delegate.mGotReport); + delegate.mError = CHIP_NO_ERROR; + delegate.mGotReport = false; + } + + { + app::ReadClient readClient(chip::app::InteractionModelEngine::GetInstance(), &ctx.GetExchangeManager(), delegate, + chip::app::ReadClient::InteractionType::Subscribe); + + chip::app::AttributePathParams attributePathParams[2]; + + attributePathParams[0].mClusterId = kTestDeniedClusterId2; + attributePathParams[0].mAttributeId = 1; + + attributePathParams[1].mClusterId = kTestDeniedClusterId2; + attributePathParams[1].mAttributeId = 2; + + ReadPrepareParams readPrepareParams(ctx.GetSessionBobToAlice()); + readPrepareParams.mpAttributePathParamsList = attributePathParams; + readPrepareParams.mAttributePathParamsListSize = 2; + + err = readClient.SendRequest(readPrepareParams); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + ctx.DrainAndServiceIO(); + NL_TEST_ASSERT(apSuite, delegate.mError == CHIP_IM_GLOBAL_STATUS(InvalidAction)); + NL_TEST_ASSERT(apSuite, !delegate.mGotReport); + delegate.mError = CHIP_NO_ERROR; + delegate.mGotReport = false; + } + + { + app::ReadClient readClient(chip::app::InteractionModelEngine::GetInstance(), &ctx.GetExchangeManager(), delegate, + chip::app::ReadClient::InteractionType::Subscribe); + + chip::app::AttributePathParams attributePathParams[2]; + attributePathParams[0].mEndpointId = kTestEndpointId; + attributePathParams[0].mClusterId = kTestDeniedClusterId1; + attributePathParams[0].mAttributeId = 1; + + attributePathParams[1].mEndpointId = kTestEndpointId; + attributePathParams[1].mClusterId = kTestClusterId; + attributePathParams[1].mAttributeId = 2; + + ReadPrepareParams readPrepareParams(ctx.GetSessionBobToAlice()); + readPrepareParams.mpAttributePathParamsList = attributePathParams; + readPrepareParams.mAttributePathParamsListSize = 2; + + err = readClient.SendRequest(readPrepareParams); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + ctx.DrainAndServiceIO(); + NL_TEST_ASSERT(apSuite, delegate.mError == CHIP_NO_ERROR); + NL_TEST_ASSERT(apSuite, delegate.mGotReport); + NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadHandlers(ReadHandler::InteractionType::Subscribe) == 1); + delegate.mError = CHIP_NO_ERROR; + delegate.mGotReport = false; + } + + NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadClients() == 0); + engine->Shutdown(); + NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); +} +} // namespace app +} // namespace chip + +namespace { + +/** + * Test Suite. It lists all the test functions. + */ + +// clang-format off +const nlTest sTests[] = +{ + NL_TEST_DEF("TestACLDeniedAttribute", chip::app::TestAclAttribute::TestACLDeniedAttribute), + NL_TEST_SENTINEL() +}; +// clang-format on + +// clang-format off +nlTestSuite sSuite = +{ + "TestAclAttribute", + &sTests[0], + TestAccessContext::Initialize, + TestAccessContext::Finalize +}; +// clang-format on + +} // namespace + +int TestAclAttribute() +{ + return chip::ExecuteTestsWithContext(&sSuite); +} + +CHIP_REGISTER_TEST_SUITE(TestAclAttribute) diff --git a/src/app/tests/TestAclEvent.cpp b/src/app/tests/TestAclEvent.cpp index b67ca621c7e44f..1238e7c7142ee3 100644 --- a/src/app/tests/TestAclEvent.cpp +++ b/src/app/tests/TestAclEvent.cpp @@ -16,12 +16,6 @@ * limitations under the License. */ -/** - * @file - * This file implements unit tests for CHIP Interaction Model Read Interaction - * - */ - #include "lib/support/CHIPMem.h" #include #include @@ -55,7 +49,7 @@ uint8_t gDebugEventBuffer[128]; uint8_t gInfoEventBuffer[128]; uint8_t gCritEventBuffer[128]; chip::app::CircularEventBuffer gCircularEventBuffer[3]; -chip::ClusterId kTestClusterId1 = 6; +chip::ClusterId kTestClusterId1 = 100; chip::ClusterId kTestClusterId2 = 7; chip::EndpointId kTestEndpointId = 1; chip::EventId kTestEventIdDebug = 1; diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp index e0527b9e4c66ff..44ba49c400a4b9 100644 --- a/src/app/tests/TestReadInteraction.cpp +++ b/src/app/tests/TestReadInteraction.cpp @@ -338,6 +338,7 @@ class TestReadInteraction static void TestReadHandlerInvalidSubscribeRequest(nlTestSuite * apSuite, void * apContext); static void TestSubscribeInvalidateFabric(nlTestSuite * apSuite, void * apContext); static void TestShutdownSubscription(nlTestSuite * apSuite, void * apContext); + static void TestReadHandlerMalformedSubscribeRequest(nlTestSuite * apSuite, void * apContext); private: static void GenerateReportData(nlTestSuite * apSuite, void * apContext, System::PacketBufferHandle & aPayload, @@ -3521,6 +3522,51 @@ void TestReadInteraction::TestReadChunkingInvalidSubscriptionId(nlTestSuite * ap ctx.CreateSessionBobToAlice(); } +// Read Client sends a malformed subscribe request, interaction model engine fails to parse the request and generates a status +// report to client, and client is closed. +void TestReadInteraction::TestReadHandlerMalformedSubscribeRequest(nlTestSuite * apSuite, void * apContext) +{ + TestContext & ctx = *static_cast(apContext); + CHIP_ERROR err = CHIP_NO_ERROR; + + Messaging::ReliableMessageMgr * rm = ctx.GetExchangeManager().GetReliableMessageMgr(); + // Shouldn't have anything in the retransmit table when starting the test. + NL_TEST_ASSERT(apSuite, rm->TestGetCountRetransTable() == 0); + + MockInteractionModelApp delegate; + auto * engine = chip::app::InteractionModelEngine::GetInstance(); + err = engine->Init(&ctx.GetExchangeManager(), &ctx.GetFabricTable()); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + ReadPrepareParams readPrepareParams(ctx.GetSessionBobToAlice()); + + { + app::ReadClient readClient(chip::app::InteractionModelEngine::GetInstance(), &ctx.GetExchangeManager(), delegate, + chip::app::ReadClient::InteractionType::Subscribe); + System::PacketBufferHandle msgBuf; + ReadRequestMessage::Builder request; + System::PacketBufferTLVWriter writer; + + chip::app::InitWriterWithSpaceReserved(writer, 0); + err = request.Init(&writer); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + err = writer.Finalize(&msgBuf); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + auto exchange = readClient.mpExchangeMgr->NewContext(readPrepareParams.mSessionHolder.Get().Value(), &readClient); + NL_TEST_ASSERT(apSuite, exchange != nullptr); + readClient.mExchange.Grab(exchange); + readClient.MoveToState(app::ReadClient::ClientState::AwaitingInitialReport); + err = readClient.mExchange->SendMessage(Protocols::InteractionModel::MsgType::ReadRequest, std::move(msgBuf), + Messaging::SendFlags(Messaging::SendMessageFlags::kExpectResponse)); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + ctx.DrainAndServiceIO(); + NL_TEST_ASSERT(apSuite, delegate.mError == CHIP_IM_GLOBAL_STATUS(InvalidAction)); + } + NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadClients() == 0); + engine->Shutdown(); + NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); +} + // Read Client sends a malformed read request, interaction model engine fails to parse the request and generates a status report to // client, and client is closed. void TestReadInteraction::TestReadHandlerMalformedReadRequest1(nlTestSuite * apSuite, void * apContext) @@ -3953,6 +3999,7 @@ const nlTest sTests[] = NL_TEST_DEF("TestReadChunkingInvalidSubscriptionId", chip::app::TestReadInteraction::TestReadChunkingInvalidSubscriptionId), NL_TEST_DEF("TestReadHandlerMalformedReadRequest1", chip::app::TestReadInteraction::TestReadHandlerMalformedReadRequest1), NL_TEST_DEF("TestReadHandlerMalformedReadRequest2", chip::app::TestReadInteraction::TestReadHandlerMalformedReadRequest2), + NL_TEST_DEF("TestReadHandlerMalformedSubscribeRequest", chip::app::TestReadInteraction::TestReadHandlerMalformedSubscribeRequest), NL_TEST_DEF("TestSubscribeSendUnknownMessage", chip::app::TestReadInteraction::TestSubscribeSendUnknownMessage), NL_TEST_DEF("TestSubscribeSendInvalidStatusReport", chip::app::TestReadInteraction::TestSubscribeSendInvalidStatusReport), NL_TEST_DEF("TestReadHandlerInvalidSubscribeRequest", chip::app::TestReadInteraction::TestReadHandlerInvalidSubscribeRequest), diff --git a/src/app/tests/integration/chip_im_initiator.cpp b/src/app/tests/integration/chip_im_initiator.cpp index 4636e14bc3fc0f..5457bbb3fd79ef 100644 --- a/src/app/tests/integration/chip_im_initiator.cpp +++ b/src/app/tests/integration/chip_im_initiator.cpp @@ -654,6 +654,11 @@ const EmberAfAttributeMetadata * GetAttributeMetadata(const ConcreteAttributePat return &stub; } +bool ConcreteAttributePathExists(const ConcreteAttributePath & aPath) +{ + return true; +} + CHIP_ERROR WriteSingleClusterData(const Access::SubjectDescriptor & aSubjectDescriptor, const ConcreteDataAttributePath & aPath, TLV::TLVReader & aReader, WriteHandler *) { diff --git a/src/app/tests/integration/chip_im_responder.cpp b/src/app/tests/integration/chip_im_responder.cpp index 9952256c4d479e..34978611e4f3bf 100644 --- a/src/app/tests/integration/chip_im_responder.cpp +++ b/src/app/tests/integration/chip_im_responder.cpp @@ -122,6 +122,11 @@ CHIP_ERROR ReadSingleClusterData(const Access::SubjectDescriptor & aSubjectDescr return CHIP_NO_ERROR; } +bool ConcreteAttributePathExists(const ConcreteAttributePath & aPath) +{ + return true; +} + const EmberAfAttributeMetadata * GetAttributeMetadata(const ConcreteAttributePath & aConcreteClusterPath) { // Note: This test does not make use of the real attribute metadata. diff --git a/src/app/util/ember-compatibility-functions.cpp b/src/app/util/ember-compatibility-functions.cpp index d90c97a50b6f8e..3ac3b3258c41bc 100644 --- a/src/app/util/ember-compatibility-functions.cpp +++ b/src/app/util/ember-compatibility-functions.cpp @@ -516,6 +516,18 @@ Protocols::InteractionModel::Status UnsupportedAttributeStatus(const ConcreteAtt } // anonymous namespace +bool ConcreteAttributePathExists(const ConcreteAttributePath & aPath) +{ + for (auto & attr : GlobalAttributesNotInMetadata) + { + if (attr == aPath.mAttributeId) + { + return (emberAfFindCluster(aPath.mEndpointId, aPath.mClusterId, CLUSTER_MASK_SERVER) != nullptr); + } + } + return (emberAfLocateAttributeMetadata(aPath.mEndpointId, aPath.mClusterId, aPath.mAttributeId) != nullptr); +} + CHIP_ERROR ReadSingleClusterData(const SubjectDescriptor & aSubjectDescriptor, bool aIsFabricFiltered, const ConcreteReadAttributePath & aPath, AttributeReportIBs::Builder & aAttributeReports, AttributeValueEncoder::AttributeEncodeState * apEncoderState) diff --git a/src/controller/tests/TestReadChunking.cpp b/src/controller/tests/TestReadChunking.cpp index 5dc6006edc09ba..1e1f0c41fd6e7d 100644 --- a/src/controller/tests/TestReadChunking.cpp +++ b/src/controller/tests/TestReadChunking.cpp @@ -68,10 +68,10 @@ constexpr AttributeId kTestListAttribute = 6; constexpr AttributeId kTestBadAttribute = 7; // Reading this attribute will return CHIP_ERROR_NO_MEMORY but nothing is actually encoded. -class TestCommandInteraction +class TestReadChunking { public: - TestCommandInteraction() {} + TestReadChunking() {} static void TestChunking(nlTestSuite * apSuite, void * apContext); static void TestListChunking(nlTestSuite * apSuite, void * apContext); static void TestBadChunking(nlTestSuite * apSuite, void * apContext); @@ -359,7 +359,7 @@ void TestMutableReadCallback::OnAttributeData(const app::ConcreteDataAttributePa * as we can possibly cover. * */ -void TestCommandInteraction::TestChunking(nlTestSuite * apSuite, void * apContext) +void TestReadChunking::TestChunking(nlTestSuite * apSuite, void * apContext) { TestContext & ctx = *static_cast(apContext); auto sessionHandle = ctx.GetSessionBobToAlice(); @@ -424,7 +424,7 @@ void TestCommandInteraction::TestChunking(nlTestSuite * apSuite, void * apContex } // Similar to the test above, but for the list chunking feature. -void TestCommandInteraction::TestListChunking(nlTestSuite * apSuite, void * apContext) +void TestReadChunking::TestListChunking(nlTestSuite * apSuite, void * apContext) { TestContext & ctx = *static_cast(apContext); auto sessionHandle = ctx.GetSessionBobToAlice(); @@ -488,7 +488,7 @@ void TestCommandInteraction::TestListChunking(nlTestSuite * apSuite, void * apCo } // Read an attribute that can never fit into the buffer. Result in an empty report, server should shutdown the transaction. -void TestCommandInteraction::TestBadChunking(nlTestSuite * apSuite, void * apContext) +void TestReadChunking::TestBadChunking(nlTestSuite * apSuite, void * apContext) { TestContext & ctx = *static_cast(apContext); auto sessionHandle = ctx.GetSessionBobToAlice(); @@ -540,7 +540,7 @@ void TestCommandInteraction::TestBadChunking(nlTestSuite * apSuite, void * apCon /* * This test contains two parts, one is to enable a new endpoint on the fly, another is to disable it and re-enable it. */ -void TestCommandInteraction::TestDynamicEndpoint(nlTestSuite * apSuite, void * apContext) +void TestReadChunking::TestDynamicEndpoint(nlTestSuite * apSuite, void * apContext) { TestContext & ctx = *static_cast(apContext); auto sessionHandle = ctx.GetSessionBobToAlice(); @@ -565,16 +565,16 @@ void TestCommandInteraction::TestDynamicEndpoint(nlTestSuite * apSuite, void * a app::ReadClient readClient(engine, &ctx.GetExchangeManager(), readCallback.mBufferedCallback, app::ReadClient::InteractionType::Subscribe); + // Enable the new endpoint + emberAfSetDynamicEndpoint(0, kTestEndpointId, &testEndpoint, Span(dataVersionStorage)); NL_TEST_ASSERT(apSuite, readClient.SendRequest(readParams) == CHIP_NO_ERROR); ctx.DrainAndServiceIO(); - // We should not receive any reports in initial reports, so check mOnSubscriptionEstablished instead. NL_TEST_ASSERT(apSuite, readCallback.mOnSubscriptionEstablished); readCallback.mAttributeCount = 0; - // Enable the new endpoint emberAfSetDynamicEndpoint(0, kTestEndpointId4, &testEndpoint4, Span(dataVersionStorage)); ctx.DrainAndServiceIO(); @@ -623,7 +623,6 @@ void TestCommandInteraction::TestDynamicEndpoint(nlTestSuite * apSuite, void * a emberAfClearDynamicEndpoint(0); } - /* * The tests below are for testing deatiled bwhavior when the attributes are modified between two chunks. In this test, we only care * above whether we will receive correct attribute values in reasonable messages with reduced reporting traffic. @@ -737,7 +736,7 @@ void DoTest(TestMutableReadCallback * callback, Instruction instruction) }; // namespace TestSetDirtyBetweenChunksUtil -void TestCommandInteraction::TestSetDirtyBetweenChunks(nlTestSuite * apSuite, void * apContext) +void TestReadChunking::TestSetDirtyBetweenChunks(nlTestSuite * apSuite, void * apContext) { using namespace TestSetDirtyBetweenChunksUtil; TestContext & ctx = *static_cast(apContext); @@ -895,11 +894,11 @@ void TestCommandInteraction::TestSetDirtyBetweenChunks(nlTestSuite * apSuite, vo // clang-format off const nlTest sTests[] = { - NL_TEST_DEF("TestChunking", TestCommandInteraction::TestChunking), - NL_TEST_DEF("TestListChunking", TestCommandInteraction::TestListChunking), - NL_TEST_DEF("TestBadChunking", TestCommandInteraction::TestBadChunking), - NL_TEST_DEF("TestDynamicEndpoint", TestCommandInteraction::TestDynamicEndpoint), - NL_TEST_DEF("TestSetDirtyBetweenChunks", TestCommandInteraction::TestSetDirtyBetweenChunks), + NL_TEST_DEF("TestChunking", TestReadChunking::TestChunking), + NL_TEST_DEF("TestListChunking", TestReadChunking::TestListChunking), + NL_TEST_DEF("TestBadChunking", TestReadChunking::TestBadChunking), + NL_TEST_DEF("TestDynamicEndpoint", TestReadChunking::TestDynamicEndpoint), + NL_TEST_DEF("TestSetDirtyBetweenChunks", TestReadChunking::TestSetDirtyBetweenChunks), NL_TEST_SENTINEL() }; diff --git a/src/controller/tests/data_model/TestRead.cpp b/src/controller/tests/data_model/TestRead.cpp index 15b2ee6c1f78bf..ff4359659f1f47 100644 --- a/src/controller/tests/data_model/TestRead.cpp +++ b/src/controller/tests/data_model/TestRead.cpp @@ -232,6 +232,12 @@ bool IsDeviceTypeOnEndpoint(DeviceTypeId deviceType, EndpointId endpoint) { return false; } + +bool ConcreteAttributePathExists(const ConcreteAttributePath & aPath) +{ + return true; +} + } // namespace app } // namespace chip @@ -275,6 +281,7 @@ class TestReadInteraction : public app::ReadHandler::ApplicationCallback static void TestReadAttribute_ManyDataValues(nlTestSuite * apSuite, void * apContext); static void TestReadAttribute_ManyDataValuesWrongPath(nlTestSuite * apSuite, void * apContext); static void TestReadAttribute_ManyErrors(nlTestSuite * apSuite, void * apContext); + static void TestSubscribeAttributeDeniedNotExistPath(nlTestSuite * apSuite, void * apContext); private: static uint16_t mMaxInterval; @@ -1550,9 +1557,9 @@ void TestReadInteraction::TestResubscribeAttributeTimeout(nlTestSuite * apSuite, app::AttributePathParams attributePathParams[1]; readPrepareParams.mpAttributePathParamsList = attributePathParams; readPrepareParams.mAttributePathParamsListSize = ArraySize(attributePathParams); - - attributePathParams[0].mClusterId = app::Clusters::TestCluster::Id; - attributePathParams[0].mAttributeId = app::Clusters::TestCluster::Attributes::Boolean::Id; + attributePathParams[0].mEndpointId = kTestEndpointId; + attributePathParams[0].mClusterId = app::Clusters::TestCluster::Id; + attributePathParams[0].mAttributeId = app::Clusters::TestCluster::Attributes::Boolean::Id; readPrepareParams.mMaxIntervalCeilingSeconds = 1; @@ -1626,6 +1633,7 @@ void TestReadInteraction::TestSubscribeAttributeTimeout(nlTestSuite * apSuite, v app::AttributePathParams attributePathParams[1]; readPrepareParams.mpAttributePathParamsList = attributePathParams; readPrepareParams.mAttributePathParamsListSize = ArraySize(attributePathParams); + attributePathParams[0].mEndpointId = kTestEndpointId; attributePathParams[0].mClusterId = app::Clusters::TestCluster::Id; attributePathParams[0].mAttributeId = app::Clusters::TestCluster::Attributes::Boolean::Id; @@ -2885,6 +2893,47 @@ void EstablishReadOrSubscriptions(nlTestSuite * apSuite, const SessionHandle & s } // namespace SubscriptionPathQuotaHelpers +void TestReadInteraction::TestSubscribeAttributeDeniedNotExistPath(nlTestSuite * apSuite, void * apContext) +{ + TestContext & ctx = *static_cast(apContext); + auto sessionHandle = ctx.GetSessionBobToAlice(); + + ctx.SetMRPMode(Test::MessagingContext::MRPMode::kResponsive); + + { + SubscriptionPathQuotaHelpers::TestReadCallback callback; + app::ReadClient readClient(app::InteractionModelEngine::GetInstance(), &ctx.GetExchangeManager(), callback, + app::ReadClient::InteractionType::Subscribe); + + app::ReadPrepareParams readPrepareParams(ctx.GetSessionBobToAlice()); + + app::AttributePathParams attributePathParams[1]; + readPrepareParams.mpAttributePathParamsList = attributePathParams; + readPrepareParams.mAttributePathParamsListSize = ArraySize(attributePathParams); + attributePathParams[0].mClusterId = app::Clusters::TestCluster::Id; + attributePathParams[0].mAttributeId = app::Clusters::TestCluster::Attributes::ListStructOctetString::Id; + + // + // Request a max interval that's very small to reduce time to discovering a liveness failure. + // + readPrepareParams.mMaxIntervalCeilingSeconds = 1; + + auto err = readClient.SendRequest(readPrepareParams); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + ctx.DrainAndServiceIO(); + + NL_TEST_ASSERT(apSuite, callback.mOnError == 1); + NL_TEST_ASSERT(apSuite, callback.mLastError == CHIP_IM_GLOBAL_STATUS(InvalidAction)); + NL_TEST_ASSERT(apSuite, callback.mOnDone == 1); + } + + ctx.SetMRPMode(Test::MessagingContext::MRPMode::kDefault); + + app::InteractionModelEngine::GetInstance()->ShutdownActiveReads(); + NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); +} + void TestReadInteraction::TestReadHandler_KillOverQuotaSubscriptions(nlTestSuite * apSuite, void * apContext) { // Note: We cannot use ctx.DrainAndServiceIO() since the perpetual read will make DrainAndServiceIO never return. @@ -4436,6 +4485,7 @@ const nlTest sTests[] = NL_TEST_DEF("TestReadAttribute_ManyDataValues", TestReadInteraction::TestReadAttribute_ManyDataValues), NL_TEST_DEF("TestReadAttribute_ManyDataValuesWrongPath", TestReadInteraction::TestReadAttribute_ManyDataValuesWrongPath), NL_TEST_DEF("TestReadAttribute_ManyErrors", TestReadInteraction::TestReadAttribute_ManyErrors), + NL_TEST_DEF("TestSubscribeAttributeDeniedNotExistPath", TestReadInteraction::TestSubscribeAttributeDeniedNotExistPath), NL_TEST_DEF("TestResubscribeAttributeTimeout", TestReadInteraction::TestResubscribeAttributeTimeout), NL_TEST_DEF("TestSubscribeAttributeTimeout", TestReadInteraction::TestSubscribeAttributeTimeout), NL_TEST_SENTINEL() diff --git a/src/darwin/Framework/CHIP/MTRBaseDevice.mm b/src/darwin/Framework/CHIP/MTRBaseDevice.mm index 631291b9963fec..d5ffc41f529127 100644 --- a/src/darwin/Framework/CHIP/MTRBaseDevice.mm +++ b/src/darwin/Framework/CHIP/MTRBaseDevice.mm @@ -1218,7 +1218,11 @@ - (void)subscribeAttributeWithEndpointId:(NSNumber * _Nullable)endpointId auto readClient = Platform::New( engine, exchangeManager, callback->GetBufferedCallback(), chip::app::ReadClient::InteractionType::Subscribe); - err = readClient->SendAutoResubscribeRequest(std::move(readParams)); + if (params != nil && params.autoResubscribe != nil && ![params.autoResubscribe boolValue]) { + err = readClient->SendRequest(readParams); + } else { + err = readClient->SendAutoResubscribeRequest(std::move(readParams)); + } if (err != CHIP_NO_ERROR) { if (reportHandler) { @@ -1227,6 +1231,8 @@ - (void)subscribeAttributeWithEndpointId:(NSNumber * _Nullable)endpointId }); } Platform::Delete(readClient); + Platform::Delete(container.pathParams); + container.pathParams = nullptr; return; } diff --git a/src/darwin/Framework/CHIP/MTRIMDispatch.mm b/src/darwin/Framework/CHIP/MTRIMDispatch.mm index c8b1df9f980a21..7979c34a832329 100644 --- a/src/darwin/Framework/CHIP/MTRIMDispatch.mm +++ b/src/darwin/Framework/CHIP/MTRIMDispatch.mm @@ -121,6 +121,11 @@ CHIP_ERROR ReadSingleClusterData(const SubjectDescriptor & aSubjectDescriptor, b return aAttributeReports.EncodeAttributeStatus(aPath, StatusIB(status)); } + bool ConcreteAttributePathExists(const ConcreteAttributePath & aPath) + { + return DetermineAttributeStatus(aPath, /* aIsWrite = */ false) == Status::UnsupportedAccess; + } + Status ServerClusterCommandExists(const ConcreteCommandPath & aPath) { // TODO: Consider making this configurable for applications that are not diff --git a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m index 1e95514ecc1e34..bb6f81a6e12be8 100644 --- a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m +++ b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m @@ -643,10 +643,12 @@ - (void)test009_SubscribeFailure __block void (^reportHandler)(id _Nullable values, NSError * _Nullable error) = nil; // Set up expectation for report - XCTestExpectation * errorReportExpectation = [self expectationWithDescription:@"receive OnOff attribute report"]; + XCTestExpectation * errorReportExpectation = [self expectationWithDescription:@"receive subscription error"]; reportHandler = ^(id _Nullable value, NSError * _Nullable error) { + // Because our subscription has no existent paths, it gets an + // InvalidAction response, which is EMBER_ZCL_STATUS_MALFORMED_COMMAND. XCTAssertNil(value); - XCTAssertEqual([MTRErrorTestUtils errorToZCLErrorCode:error], EMBER_ZCL_STATUS_UNSUPPORTED_ENDPOINT); + XCTAssertEqual([MTRErrorTestUtils errorToZCLErrorCode:error], EMBER_ZCL_STATUS_MALFORMED_COMMAND); [errorReportExpectation fulfill]; }; @@ -662,12 +664,14 @@ - (void)test009_SubscribeFailure }]; [self waitForExpectations:@[ cleanSubscriptionExpectation ] timeout:kTimeoutInSeconds]; + __auto_type * params = [[MTRSubscribeParams alloc] init]; + params.autoResubscribe = @(NO); [device subscribeAttributeWithEndpointId:@10000 clusterId:@6 attributeId:@0 minInterval:@2 maxInterval:@10 - params:nil + params:params clientQueue:queue reportHandler:^(id _Nullable values, NSError * _Nullable error) { NSLog(@"report attribute: OnOff values: %@, error: %@", values, error); From 7475f005bcc30be75e49c3c5d387539f3f2c71f4 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 8 Sep 2022 16:05:22 -0400 Subject: [PATCH 12/20] Fix android full build in CI (#22486) * Split out android builds to not use as much space during building * Ensure we can test android builds with act * Since we use checkout_submodules, submodules are not needed on the other checkout * Add some documentation on using act to test * Fix ENOBUFS being undefined when compiling tv-casting-app on arm --- .github/workflows/full-android.yaml | 41 ++++++++++++++++++++++++---- src/messaging/ReliableMessageMgr.cpp | 1 + 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/.github/workflows/full-android.yaml b/.github/workflows/full-android.yaml index 8a327d99066275..fcf4fc0a14619a 100644 --- a/.github/workflows/full-android.yaml +++ b/.github/workflows/full-android.yaml @@ -40,14 +40,23 @@ jobs: steps: - uses: Wandalen/wretry.action@v1.0.15 + if: ${{ !env.ACT }} name: Checkout with: action: actions/checkout@v3 with: | - submodules: true token: ${{ github.token }} attempt_limit: 3 attempt_delay: 2000 + # To use act like: + # act -j full_android + # + # Note you likely still need to have non submodules setup for the + # local machine, like: + # git submodule deinit --all + - uses: actions/checkout@v3 + if: ${{ env.ACT }} + name: Checkout (ACT for local build) - name: Checkout submodules run: scripts/checkout_submodules.py --shallow --platform android - name: Bootstrap @@ -61,16 +70,38 @@ jobs: path: | .environment/gn_out/.ninja_log .environment/pigweed-venv/*.log - - name: Build Android CHIPTool and CHIPTest (ARM) + - name: Build Android arm-chip-tool run: | ./scripts/run_in_build_env.sh \ - "./scripts/build/build_examples.py --no-log-timestamps --target-glob 'android-arm-*' build" + "./scripts/build/build_examples.py --no-log-timestamps --target android-arm-chip-tool build" - name: Clean out build output run: rm -rf ./out - - name: Build Android CHIPTool and CHIPTest (ARM64) + - name: Build Android arm-tv-casting-app run: | ./scripts/run_in_build_env.sh \ - "./scripts/build/build_examples.py --no-log-timestamps --target-glob 'android-arm64-*' build" + "./scripts/build/build_examples.py --no-log-timestamps --target android-arm-tv-casting-app build" + - name: Clean out build output + run: rm -rf ./out + - name: Build Android arm-tv-server + run: | + ./scripts/run_in_build_env.sh \ + "./scripts/build/build_examples.py --no-log-timestamps --target android-arm-tv-server build" + - name: Clean out build output + run: rm -rf ./out + - name: Build Android arm64-tv-casting-app + run: | + ./scripts/run_in_build_env.sh \ + "./scripts/build/build_examples.py --no-log-timestamps --target android-arm64-tv-casting-app build" + - name: Clean out build output + run: rm -rf ./out + - name: Build Android arm64-tv-server + run: | + ./scripts/run_in_build_env.sh \ + "./scripts/build/build_examples.py --no-log-timestamps --target android-arm64-tv-server build" + - name: Build Android arm64-chip-tool + run: | + ./scripts/run_in_build_env.sh \ + "./scripts/build/build_examples.py --no-log-timestamps --target android-arm64-chip-tool build" - name: Run Android build rule tests run: | ./scripts/run_in_build_env.sh \ diff --git a/src/messaging/ReliableMessageMgr.cpp b/src/messaging/ReliableMessageMgr.cpp index b870e6becc2287..e745e54d46665e 100644 --- a/src/messaging/ReliableMessageMgr.cpp +++ b/src/messaging/ReliableMessageMgr.cpp @@ -21,6 +21,7 @@ * */ +#include #include #include From 9cb4c70eb14880eac4810e3c1be2ec3fde4d1ef8 Mon Sep 17 00:00:00 2001 From: Erwin Pan Date: Fri, 9 Sep 2022 04:51:04 +0800 Subject: [PATCH 13/20] Support Commands for rootnode_extendedcolorlight (#22477) * Support Commands for rootnode_extendedcolorlight The current rootnode_extendedcolorlight_8lcaaYJVAa doesn't support many commands such as MoveToHue, MoveToHueAndSaturation ... etc. We add this patch to support them. * Remove unnecessary Attributes not for ColorControl Some Attributes referenced from examples/lighting-app are not for ColorControl Cluster and not required by us. And these also be complained by CI sanity check --- ...tnode_extendedcolorlight_8lcaaYJVAa.matter | 141 +++++++++++++++++ ...rootnode_extendedcolorlight_8lcaaYJVAa.zap | 130 +++++++++++++++- .../zap-generated/IMClusterCommandHandler.cpp | 145 ++++++++++++++++++ .../zap-generated/endpoint_config.h | 16 ++ 4 files changed, 431 insertions(+), 1 deletion(-) diff --git a/examples/chef/devices/rootnode_extendedcolorlight_8lcaaYJVAa.matter b/examples/chef/devices/rootnode_extendedcolorlight_8lcaaYJVAa.matter index 5f880b59f6a5a7..141aefc3debb50 100644 --- a/examples/chef/devices/rootnode_extendedcolorlight_8lcaaYJVAa.matter +++ b/examples/chef/devices/rootnode_extendedcolorlight_8lcaaYJVAa.matter @@ -1621,6 +1621,59 @@ server cluster ColorControl = 768 { readonly attribute bitmap32 featureMap = 65532; readonly attribute int16u clusterRevision = 65533; + request struct MoveToHueRequest { + INT8U hue = 0; + HueDirection direction = 1; + INT16U transitionTime = 2; + BITMAP8 optionsMask = 3; + BITMAP8 optionsOverride = 4; + } + + request struct MoveHueRequest { + HueMoveMode moveMode = 0; + INT8U rate = 1; + BITMAP8 optionsMask = 2; + BITMAP8 optionsOverride = 3; + } + + request struct StepHueRequest { + HueStepMode stepMode = 0; + INT8U stepSize = 1; + INT8U transitionTime = 2; + BITMAP8 optionsMask = 3; + BITMAP8 optionsOverride = 4; + } + + request struct MoveToSaturationRequest { + INT8U saturation = 0; + INT16U transitionTime = 1; + BITMAP8 optionsMask = 2; + BITMAP8 optionsOverride = 3; + } + + request struct MoveSaturationRequest { + SaturationMoveMode moveMode = 0; + INT8U rate = 1; + BITMAP8 optionsMask = 2; + BITMAP8 optionsOverride = 3; + } + + request struct StepSaturationRequest { + SaturationStepMode stepMode = 0; + INT8U stepSize = 1; + INT8U transitionTime = 2; + BITMAP8 optionsMask = 3; + BITMAP8 optionsOverride = 4; + } + + request struct MoveToHueAndSaturationRequest { + INT8U hue = 0; + INT8U saturation = 1; + INT16U transitionTime = 2; + BITMAP8 optionsMask = 3; + BITMAP8 optionsOverride = 4; + } + request struct MoveToColorRequest { INT16U colorX = 0; INT16U colorY = 1; @@ -1644,9 +1697,97 @@ server cluster ColorControl = 768 { BITMAP8 optionsOverride = 4; } + request struct MoveToColorTemperatureRequest { + INT16U colorTemperature = 0; + INT16U transitionTime = 1; + BITMAP8 optionsMask = 2; + BITMAP8 optionsOverride = 3; + } + + request struct EnhancedMoveToHueRequest { + INT16U enhancedHue = 0; + HueDirection direction = 1; + INT16U transitionTime = 2; + BITMAP8 optionsMask = 3; + BITMAP8 optionsOverride = 4; + } + + request struct EnhancedMoveHueRequest { + HueMoveMode moveMode = 0; + INT16U rate = 1; + BITMAP8 optionsMask = 2; + BITMAP8 optionsOverride = 3; + } + + request struct EnhancedStepHueRequest { + HueStepMode stepMode = 0; + INT16U stepSize = 1; + INT16U transitionTime = 2; + BITMAP8 optionsMask = 3; + BITMAP8 optionsOverride = 4; + } + + request struct EnhancedMoveToHueAndSaturationRequest { + INT16U enhancedHue = 0; + INT8U saturation = 1; + INT16U transitionTime = 2; + BITMAP8 optionsMask = 3; + BITMAP8 optionsOverride = 4; + } + + request struct ColorLoopSetRequest { + ColorLoopUpdateFlags updateFlags = 0; + ColorLoopAction action = 1; + ColorLoopDirection direction = 2; + INT16U time = 3; + INT16U startHue = 4; + BITMAP8 optionsMask = 5; + BITMAP8 optionsOverride = 6; + } + + request struct StopMoveStepRequest { + BITMAP8 optionsMask = 0; + BITMAP8 optionsOverride = 1; + } + + request struct MoveColorTemperatureRequest { + HueMoveMode moveMode = 0; + INT16U rate = 1; + INT16U colorTemperatureMinimumMireds = 2; + INT16U colorTemperatureMaximumMireds = 3; + BITMAP8 optionsMask = 4; + BITMAP8 optionsOverride = 5; + } + + request struct StepColorTemperatureRequest { + HueStepMode stepMode = 0; + INT16U stepSize = 1; + INT16U transitionTime = 2; + INT16U colorTemperatureMinimumMireds = 3; + INT16U colorTemperatureMaximumMireds = 4; + BITMAP8 optionsMask = 5; + BITMAP8 optionsOverride = 6; + } + + command MoveToHue(MoveToHueRequest): DefaultSuccess = 0; + command MoveHue(MoveHueRequest): DefaultSuccess = 1; + command StepHue(StepHueRequest): DefaultSuccess = 2; + command MoveToSaturation(MoveToSaturationRequest): DefaultSuccess = 3; + command MoveSaturation(MoveSaturationRequest): DefaultSuccess = 4; + command StepSaturation(StepSaturationRequest): DefaultSuccess = 5; + command MoveToHueAndSaturation(MoveToHueAndSaturationRequest): DefaultSuccess = 6; command MoveToColor(MoveToColorRequest): DefaultSuccess = 7; command MoveColor(MoveColorRequest): DefaultSuccess = 8; command StepColor(StepColorRequest): DefaultSuccess = 9; + command MoveToColorTemperature(MoveToColorTemperatureRequest): DefaultSuccess = 10; + command EnhancedMoveToHue(EnhancedMoveToHueRequest): DefaultSuccess = 64; + command EnhancedMoveHue(EnhancedMoveHueRequest): DefaultSuccess = 65; + command EnhancedStepHue(EnhancedStepHueRequest): DefaultSuccess = 66; + command EnhancedMoveToHueAndSaturation(EnhancedMoveToHueAndSaturationRequest): DefaultSuccess = 67; + command ColorLoopSet(ColorLoopSetRequest): DefaultSuccess = 68; + command StopMoveStep(StopMoveStepRequest): DefaultSuccess = 71; + command MoveColorTemperature(MoveColorTemperatureRequest): DefaultSuccess = 75; + command StepColorTemperature(StepColorTemperatureRequest): DefaultSuccess = 76; } endpoint 0 { diff --git a/examples/chef/devices/rootnode_extendedcolorlight_8lcaaYJVAa.zap b/examples/chef/devices/rootnode_extendedcolorlight_8lcaaYJVAa.zap index f7366f0da7ec0b..6cab53b2cecf8d 100644 --- a/examples/chef/devices/rootnode_extendedcolorlight_8lcaaYJVAa.zap +++ b/examples/chef/devices/rootnode_extendedcolorlight_8lcaaYJVAa.zap @@ -6954,6 +6954,62 @@ "side": "client", "enabled": 0, "commands": [ + { + "name": "MoveToHue", + "code": 0, + "mfgCode": null, + "source": "client", + "incoming": 1, + "outgoing": 0 + }, + { + "name": "MoveHue", + "code": 1, + "mfgCode": null, + "source": "client", + "incoming": 1, + "outgoing": 0 + }, + { + "name": "StepHue", + "code": 2, + "mfgCode": null, + "source": "client", + "incoming": 1, + "outgoing": 0 + }, + { + "name": "MoveToSaturation", + "code": 3, + "mfgCode": null, + "source": "client", + "incoming": 1, + "outgoing": 0 + }, + { + "name": "MoveSaturation", + "code": 4, + "mfgCode": null, + "source": "client", + "incoming": 1, + "outgoing": 0 + }, + { + "name": "StepSaturation", + "code": 5, + "mfgCode": null, + "source": "client", + "incoming": 1, + "outgoing": 0 + }, + { + "name": "MoveToHueAndSaturation", + "code": 6, + "mfgCode": null, + "source": "client", + "incoming": 1, + "outgoing": 0 + }, { "name": "MoveToColor", "code": 7, @@ -6977,6 +7033,78 @@ "source": "client", "incoming": 1, "outgoing": 0 + }, + { + "name": "MoveToColorTemperature", + "code": 10, + "mfgCode": null, + "source": "client", + "incoming": 1, + "outgoing": 0 + }, + { + "name": "EnhancedMoveToHue", + "code": 64, + "mfgCode": null, + "source": "client", + "incoming": 1, + "outgoing": 0 + }, + { + "name": "EnhancedMoveHue", + "code": 65, + "mfgCode": null, + "source": "client", + "incoming": 1, + "outgoing": 0 + }, + { + "name": "EnhancedStepHue", + "code": 66, + "mfgCode": null, + "source": "client", + "incoming": 1, + "outgoing": 0 + }, + { + "name": "EnhancedMoveToHueAndSaturation", + "code": 67, + "mfgCode": null, + "source": "client", + "incoming": 1, + "outgoing": 0 + }, + { + "name": "ColorLoopSet", + "code": 68, + "mfgCode": null, + "source": "client", + "incoming": 1, + "outgoing": 0 + }, + { + "name": "StopMoveStep", + "code": 71, + "mfgCode": null, + "source": "client", + "incoming": 1, + "outgoing": 0 + }, + { + "name": "MoveColorTemperature", + "code": 75, + "mfgCode": null, + "source": "client", + "incoming": 1, + "outgoing": 0 + }, + { + "name": "StepColorTemperature", + "code": 76, + "mfgCode": null, + "source": "client", + "incoming": 1, + "outgoing": 0 } ], "attributes": [ @@ -8283,4 +8411,4 @@ "deviceIdentifier": 269 } ] -} \ No newline at end of file +} diff --git a/zzz_generated/chef-rootnode_extendedcolorlight_8lcaaYJVAa/zap-generated/IMClusterCommandHandler.cpp b/zzz_generated/chef-rootnode_extendedcolorlight_8lcaaYJVAa/zap-generated/IMClusterCommandHandler.cpp index cf3cadf90eb63c..e2dd0a65337ef0 100644 --- a/zzz_generated/chef-rootnode_extendedcolorlight_8lcaaYJVAa/zap-generated/IMClusterCommandHandler.cpp +++ b/zzz_generated/chef-rootnode_extendedcolorlight_8lcaaYJVAa/zap-generated/IMClusterCommandHandler.cpp @@ -108,6 +108,69 @@ void DispatchServerCommand(CommandHandler * apCommandObj, const ConcreteCommandP { switch (aCommandPath.mCommandId) { + case Commands::MoveToHue::Id: { + Commands::MoveToHue::DecodableType commandData; + TLVError = DataModel::Decode(aDataTlv, commandData); + if (TLVError == CHIP_NO_ERROR) + { + wasHandled = emberAfColorControlClusterMoveToHueCallback(apCommandObj, aCommandPath, commandData); + } + break; + } + case Commands::MoveHue::Id: { + Commands::MoveHue::DecodableType commandData; + TLVError = DataModel::Decode(aDataTlv, commandData); + if (TLVError == CHIP_NO_ERROR) + { + wasHandled = emberAfColorControlClusterMoveHueCallback(apCommandObj, aCommandPath, commandData); + } + break; + } + case Commands::StepHue::Id: { + Commands::StepHue::DecodableType commandData; + TLVError = DataModel::Decode(aDataTlv, commandData); + if (TLVError == CHIP_NO_ERROR) + { + wasHandled = emberAfColorControlClusterStepHueCallback(apCommandObj, aCommandPath, commandData); + } + break; + } + case Commands::MoveToSaturation::Id: { + Commands::MoveToSaturation::DecodableType commandData; + TLVError = DataModel::Decode(aDataTlv, commandData); + if (TLVError == CHIP_NO_ERROR) + { + wasHandled = emberAfColorControlClusterMoveToSaturationCallback(apCommandObj, aCommandPath, commandData); + } + break; + } + case Commands::MoveSaturation::Id: { + Commands::MoveSaturation::DecodableType commandData; + TLVError = DataModel::Decode(aDataTlv, commandData); + if (TLVError == CHIP_NO_ERROR) + { + wasHandled = emberAfColorControlClusterMoveSaturationCallback(apCommandObj, aCommandPath, commandData); + } + break; + } + case Commands::StepSaturation::Id: { + Commands::StepSaturation::DecodableType commandData; + TLVError = DataModel::Decode(aDataTlv, commandData); + if (TLVError == CHIP_NO_ERROR) + { + wasHandled = emberAfColorControlClusterStepSaturationCallback(apCommandObj, aCommandPath, commandData); + } + break; + } + case Commands::MoveToHueAndSaturation::Id: { + Commands::MoveToHueAndSaturation::DecodableType commandData; + TLVError = DataModel::Decode(aDataTlv, commandData); + if (TLVError == CHIP_NO_ERROR) + { + wasHandled = emberAfColorControlClusterMoveToHueAndSaturationCallback(apCommandObj, aCommandPath, commandData); + } + break; + } case Commands::MoveToColor::Id: { Commands::MoveToColor::DecodableType commandData; TLVError = DataModel::Decode(aDataTlv, commandData); @@ -135,6 +198,88 @@ void DispatchServerCommand(CommandHandler * apCommandObj, const ConcreteCommandP } break; } + case Commands::MoveToColorTemperature::Id: { + Commands::MoveToColorTemperature::DecodableType commandData; + TLVError = DataModel::Decode(aDataTlv, commandData); + if (TLVError == CHIP_NO_ERROR) + { + wasHandled = emberAfColorControlClusterMoveToColorTemperatureCallback(apCommandObj, aCommandPath, commandData); + } + break; + } + case Commands::EnhancedMoveToHue::Id: { + Commands::EnhancedMoveToHue::DecodableType commandData; + TLVError = DataModel::Decode(aDataTlv, commandData); + if (TLVError == CHIP_NO_ERROR) + { + wasHandled = emberAfColorControlClusterEnhancedMoveToHueCallback(apCommandObj, aCommandPath, commandData); + } + break; + } + case Commands::EnhancedMoveHue::Id: { + Commands::EnhancedMoveHue::DecodableType commandData; + TLVError = DataModel::Decode(aDataTlv, commandData); + if (TLVError == CHIP_NO_ERROR) + { + wasHandled = emberAfColorControlClusterEnhancedMoveHueCallback(apCommandObj, aCommandPath, commandData); + } + break; + } + case Commands::EnhancedStepHue::Id: { + Commands::EnhancedStepHue::DecodableType commandData; + TLVError = DataModel::Decode(aDataTlv, commandData); + if (TLVError == CHIP_NO_ERROR) + { + wasHandled = emberAfColorControlClusterEnhancedStepHueCallback(apCommandObj, aCommandPath, commandData); + } + break; + } + case Commands::EnhancedMoveToHueAndSaturation::Id: { + Commands::EnhancedMoveToHueAndSaturation::DecodableType commandData; + TLVError = DataModel::Decode(aDataTlv, commandData); + if (TLVError == CHIP_NO_ERROR) + { + wasHandled = + emberAfColorControlClusterEnhancedMoveToHueAndSaturationCallback(apCommandObj, aCommandPath, commandData); + } + break; + } + case Commands::ColorLoopSet::Id: { + Commands::ColorLoopSet::DecodableType commandData; + TLVError = DataModel::Decode(aDataTlv, commandData); + if (TLVError == CHIP_NO_ERROR) + { + wasHandled = emberAfColorControlClusterColorLoopSetCallback(apCommandObj, aCommandPath, commandData); + } + break; + } + case Commands::StopMoveStep::Id: { + Commands::StopMoveStep::DecodableType commandData; + TLVError = DataModel::Decode(aDataTlv, commandData); + if (TLVError == CHIP_NO_ERROR) + { + wasHandled = emberAfColorControlClusterStopMoveStepCallback(apCommandObj, aCommandPath, commandData); + } + break; + } + case Commands::MoveColorTemperature::Id: { + Commands::MoveColorTemperature::DecodableType commandData; + TLVError = DataModel::Decode(aDataTlv, commandData); + if (TLVError == CHIP_NO_ERROR) + { + wasHandled = emberAfColorControlClusterMoveColorTemperatureCallback(apCommandObj, aCommandPath, commandData); + } + break; + } + case Commands::StepColorTemperature::Id: { + Commands::StepColorTemperature::DecodableType commandData; + TLVError = DataModel::Decode(aDataTlv, commandData); + if (TLVError == CHIP_NO_ERROR) + { + wasHandled = emberAfColorControlClusterStepColorTemperatureCallback(apCommandObj, aCommandPath, commandData); + } + break; + } default: { // Unrecognized command ID, error status will apply. apCommandObj->AddStatus(aCommandPath, Protocols::InteractionModel::Status::UnsupportedCommand); diff --git a/zzz_generated/chef-rootnode_extendedcolorlight_8lcaaYJVAa/zap-generated/endpoint_config.h b/zzz_generated/chef-rootnode_extendedcolorlight_8lcaaYJVAa/zap-generated/endpoint_config.h index 08ca5981f0b424..57ae77bc7a2a17 100644 --- a/zzz_generated/chef-rootnode_extendedcolorlight_8lcaaYJVAa/zap-generated/endpoint_config.h +++ b/zzz_generated/chef-rootnode_extendedcolorlight_8lcaaYJVAa/zap-generated/endpoint_config.h @@ -713,9 +713,25 @@ chip::kInvalidCommandId /* end of list */, \ /* Endpoint: 1, Cluster: Color Control (server) */\ /* AcceptedCommandList (index=115) */ \ + 0x00000000 /* MoveToHue */, \ + 0x00000001 /* MoveHue */, \ + 0x00000002 /* StepHue */, \ + 0x00000003 /* MoveToSaturation */, \ + 0x00000004 /* MoveSaturation */, \ + 0x00000005 /* StepSaturation */, \ + 0x00000006 /* MoveToHueAndSaturation */, \ 0x00000007 /* MoveToColor */, \ 0x00000008 /* MoveColor */, \ 0x00000009 /* StepColor */, \ + 0x0000000A /* MoveToColorTemperature */, \ + 0x00000040 /* EnhancedMoveToHue */, \ + 0x00000041 /* EnhancedMoveHue */, \ + 0x00000042 /* EnhancedStepHue */, \ + 0x00000043 /* EnhancedMoveToHueAndSaturation */, \ + 0x00000044 /* ColorLoopSet */, \ + 0x00000047 /* StopMoveStep */, \ + 0x0000004B /* MoveColorTemperature */, \ + 0x0000004C /* StepColorTemperature */, \ chip::kInvalidCommandId /* end of list */, \ } From d947d87ca6fbaef4f6ad5b5890aecf1d53010aad Mon Sep 17 00:00:00 2001 From: Vivien Nicolas Date: Thu, 8 Sep 2022 23:28:31 +0200 Subject: [PATCH 14/20] [all-clusters-app][esp32] Add LockManager/LockEndpoint and the door lock plugin handlers methods to the example application (#22102) * Increase the partition size to account for the LockManager and LockEndpoint additions * [all-clusters-app][esp32] Add LockManager/LockEndpoint and the door lock plugin handlers methods to the example application --- examples/all-clusters-app/esp32/main/CMakeLists.txt | 7 +++++++ examples/all-clusters-app/esp32/partitions.csv | 4 ++-- examples/lock-app/linux/main.cpp | 11 +++++++++++ examples/lock-app/linux/src/ZCLDoorLockCallbacks.cpp | 10 ---------- 4 files changed, 20 insertions(+), 12 deletions(-) diff --git a/examples/all-clusters-app/esp32/main/CMakeLists.txt b/examples/all-clusters-app/esp32/main/CMakeLists.txt index f8ea390ec05a99..f25a2b351094d5 100644 --- a/examples/all-clusters-app/esp32/main/CMakeLists.txt +++ b/examples/all-clusters-app/esp32/main/CMakeLists.txt @@ -21,6 +21,7 @@ set(PRIV_INCLUDE_DIRS_LIST "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/zzz_generated/all-clusters-app" "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/examples/all-clusters-app/all-clusters-common/include" "${CMAKE_CURRENT_LIST_DIR}/include" + "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/examples/lock-app/linux/include" "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/examples/providers" "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/examples/platform/esp32" "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/third_party/nlfaultinjection/repo/include" @@ -30,6 +31,7 @@ set(SRC_DIRS_LIST "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/zzz_generated/all-clusters-app/zap-generated" "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/zzz_generated/app-common/app-common/zap-generated/attributes" "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/zzz_generated/app-common/app-common/zap-generated" + "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/examples/lock-app/linux/src" "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/examples/providers" "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/examples/platform/esp32/route_hook" "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/examples/platform/esp32/ota" @@ -93,6 +95,10 @@ set(SRC_DIRS_LIST "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/examples/all-clusters-app/all-clusters-common/src" ) +set(EXCLUDE_SRCS_LIST + "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/examples/lock-app/linux/src/LockAppCommandDelegate.cpp" +) + if (CONFIG_ENABLE_PW_RPC) # Append additional directories for RPC build set(PRIV_INCLUDE_DIRS_LIST "${PRIV_INCLUDE_DIRS_LIST}" @@ -128,6 +134,7 @@ endif() idf_component_register(PRIV_INCLUDE_DIRS ${PRIV_INCLUDE_DIRS_LIST} SRC_DIRS ${SRC_DIRS_LIST} + EXCLUDE_SRCS ${EXCLUDE_SRCS_LIST} PRIV_REQUIRES ${PRIV_REQUIRES_LIST}) set_property(TARGET ${COMPONENT_LIB} PROPERTY CXX_STANDARD 17) diff --git a/examples/all-clusters-app/esp32/partitions.csv b/examples/all-clusters-app/esp32/partitions.csv index 43acef25d0245d..efe7f6e9caf0e4 100644 --- a/examples/all-clusters-app/esp32/partitions.csv +++ b/examples/all-clusters-app/esp32/partitions.csv @@ -3,6 +3,6 @@ nvs, data, nvs, , 0x6000, otadata, data, ota, , 0x2000, phy_init, data, phy, , 0x1000, -ota_0, app, ota_0, , 1500K, -ota_1, app, ota_1, , 1500K, +ota_0, app, ota_0, , 1700K, +ota_1, app, ota_1, , 1700K, ot_storage, data, 0x3a, , 0x2000, diff --git a/examples/lock-app/linux/main.cpp b/examples/lock-app/linux/main.cpp index 661aaaa53497a2..bf6cb027107406 100644 --- a/examples/lock-app/linux/main.cpp +++ b/examples/lock-app/linux/main.cpp @@ -17,6 +17,7 @@ */ #include "AppMain.h" +#include #include #include @@ -79,3 +80,13 @@ int main(int argc, char * argv[]) ChipLinuxAppMainLoop(); return 0; } + +void MatterPostAttributeChangeCallback(const chip::app::ConcreteAttributePath & attributePath, uint8_t type, uint16_t size, + uint8_t * value) +{ + // TODO: Watch for LockState, DoorState, Mode, etc changes and trigger appropriate action + if (attributePath.mClusterId == Clusters::DoorLock::Id) + { + emberAfDoorLockClusterPrintln("Door Lock attribute changed"); + } +} diff --git a/examples/lock-app/linux/src/ZCLDoorLockCallbacks.cpp b/examples/lock-app/linux/src/ZCLDoorLockCallbacks.cpp index b5ef3a760d64dc..fa7cc2dff164cf 100644 --- a/examples/lock-app/linux/src/ZCLDoorLockCallbacks.cpp +++ b/examples/lock-app/linux/src/ZCLDoorLockCallbacks.cpp @@ -88,16 +88,6 @@ DlStatus emberAfPluginDoorLockSetSchedule(chip::EndpointId endpointId, uint8_t h return LockManager::Instance().SetSchedule(endpointId, holidayIndex, status, localStartTime, localEndTime, operatingMode); } -void MatterPostAttributeChangeCallback(const chip::app::ConcreteAttributePath & attributePath, uint8_t type, uint16_t size, - uint8_t * value) -{ - // TODO: Watch for LockState, DoorState, Mode, etc changes and trigger appropriate action - if (attributePath.mClusterId == DoorLock::Id) - { - emberAfDoorLockClusterPrintln("Door Lock attribute changed"); - } -} - void emberAfDoorLockClusterInitCallback(EndpointId endpoint) { DoorLockServer::Instance().InitServer(endpoint); From d7469025e0ddf0b8fa9cf22d4aee543f4c91360c Mon Sep 17 00:00:00 2001 From: Martin Turon Date: Thu, 8 Sep 2022 16:41:13 -0700 Subject: [PATCH 15/20] [linux] Fix missing include pthread.h. (#22460) --- examples/platform/linux/NamedPipeCommands.h | 1 + 1 file changed, 1 insertion(+) diff --git a/examples/platform/linux/NamedPipeCommands.h b/examples/platform/linux/NamedPipeCommands.h index 4f908de28748d0..ceacec36ca5593 100644 --- a/examples/platform/linux/NamedPipeCommands.h +++ b/examples/platform/linux/NamedPipeCommands.h @@ -19,6 +19,7 @@ #pragma once #include +#include #include class NamedPipeCommandDelegate From 27f9fb49ac45cb3933d96ddf7b25aed6ec9b665d Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Thu, 8 Sep 2022 20:54:12 -0400 Subject: [PATCH 16/20] Fix ESP32 platform manager shutdown. (#22417) * Fix ESP32 platform manager shutdown. The QEMU ESP32 tests start and stop the platform manager several times, and the lack of a shutdown implementation was putting things in a bad state. Also adjusts the test runner script to trigger a test failure when we do end up in that bad state: we were passing even though we crashed quite early in the test run and most of the tests did not run. * Address review comments. * Fix unlocking in FreeRTOS _RunEventLoop. Ensure we always start with an empty event queue after FreeRTOS _InitChipStack. --- .restyled.yaml | 2 + .../GenericPlatformManagerImpl_FreeRTOS.h | 6 +- .../GenericPlatformManagerImpl_FreeRTOS.ipp | 67 +++++++++++++------ src/platform/ESP32/PlatformManagerImpl.cpp | 2 + src/system/tests/BUILD.gn | 2 +- src/test_driver/esp32/run_qemu_image.py | 27 +++++++- 6 files changed, 79 insertions(+), 27 deletions(-) diff --git a/.restyled.yaml b/.restyled.yaml index c45fe98588b7ec..d4ca39f3efa448 100644 --- a/.restyled.yaml +++ b/.restyled.yaml @@ -98,6 +98,7 @@ restylers: - "**/*.c" - "**/*.cc" - "**/*.cpp" + - "**/*.ipp" - "**/*.cxx" - "**/*.c++" - "**/*.C" @@ -137,6 +138,7 @@ restylers: - "**/*.c" - "**/*.cc" - "**/*.cpp" + - "**/*.ipp" - "**/*.cxx" - "**/*.c++" - "**/*.C" diff --git a/src/include/platform/internal/GenericPlatformManagerImpl_FreeRTOS.h b/src/include/platform/internal/GenericPlatformManagerImpl_FreeRTOS.h index 617e5caa39fa33..f1a400fc884efd 100644 --- a/src/include/platform/internal/GenericPlatformManagerImpl_FreeRTOS.h +++ b/src/include/platform/internal/GenericPlatformManagerImpl_FreeRTOS.h @@ -60,9 +60,9 @@ class GenericPlatformManagerImpl_FreeRTOS : public GenericPlatformManagerImpl::_InitChipStack(void) vTaskSetTimeOutState(&mNextTimerBaseTime); mNextTimerDurationTicks = 0; - mEventLoopTask = NULL; - mChipTimerActive = false; + // TODO: This nulling out of mEventLoopTask should happen when we shut down + // the task, not here! + mEventLoopTask = NULL; + mChipTimerActive = false; + // We support calling Shutdown followed by InitChipStack, because some tests + // do that. To keep things simple for existing consumers, we keep not + // destroying our lock and queue in shutdown, but rather check whether they + // already exist here before trying to create them. + + if (mChipStackLock == NULL) + { #if defined(CHIP_CONFIG_FREERTOS_USE_STATIC_SEMAPHORE) && CHIP_CONFIG_FREERTOS_USE_STATIC_SEMAPHORE - mChipStackLock = xSemaphoreCreateMutexStatic(&mChipStackLockMutex); + mChipStackLock = xSemaphoreCreateMutexStatic(&mChipStackLockMutex); #else - mChipStackLock = xSemaphoreCreateMutex(); + mChipStackLock = xSemaphoreCreateMutex(); #endif // CHIP_CONFIG_FREERTOS_USE_STATIC_SEMAPHORE - if (mChipStackLock == NULL) - { - ChipLogError(DeviceLayer, "Failed to create CHIP stack lock"); - ExitNow(err = CHIP_ERROR_NO_MEMORY); + if (mChipStackLock == NULL) + { + ChipLogError(DeviceLayer, "Failed to create CHIP stack lock"); + ExitNow(err = CHIP_ERROR_NO_MEMORY); + } } + if (mChipEventQueue == NULL) + { #if defined(CHIP_CONFIG_FREERTOS_USE_STATIC_QUEUE) && CHIP_CONFIG_FREERTOS_USE_STATIC_QUEUE - mChipEventQueue = - xQueueCreateStatic(CHIP_DEVICE_CONFIG_MAX_EVENT_QUEUE_SIZE, sizeof(ChipDeviceEvent), mEventQueueBuffer, &mEventQueueStruct); + mChipEventQueue = xQueueCreateStatic(CHIP_DEVICE_CONFIG_MAX_EVENT_QUEUE_SIZE, sizeof(ChipDeviceEvent), mEventQueueBuffer, + &mEventQueueStruct); #else - mChipEventQueue = xQueueCreate(CHIP_DEVICE_CONFIG_MAX_EVENT_QUEUE_SIZE, sizeof(ChipDeviceEvent)); + mChipEventQueue = xQueueCreate(CHIP_DEVICE_CONFIG_MAX_EVENT_QUEUE_SIZE, sizeof(ChipDeviceEvent)); #endif - if (mChipEventQueue == NULL) + if (mChipEventQueue == NULL) + { + ChipLogError(DeviceLayer, "Failed to allocate CHIP event queue"); + ExitNow(err = CHIP_ERROR_NO_MEMORY); + } + } + else { - ChipLogError(DeviceLayer, "Failed to allocate CHIP event queue"); - ExitNow(err = CHIP_ERROR_NO_MEMORY); + // Clear out any events that might be stuck in the queue, so we start + // with a clean slate, as if we had just re-created the queue. + xQueueReset(mChipEventQueue); } mShouldRunEventLoop.store(false); @@ -145,7 +164,7 @@ void GenericPlatformManagerImpl_FreeRTOS::_RunEventLoop(void) ChipDeviceEvent event; // Lock the CHIP stack. - Impl()->LockChipStack(); + StackLock lock; bool oldShouldRunEventLoop = false; if (!mShouldRunEventLoop.compare_exchange_strong(oldShouldRunEventLoop /* expected */, true /* desired */)) @@ -198,13 +217,13 @@ void GenericPlatformManagerImpl_FreeRTOS::_RunEventLoop(void) waitTime = portMAX_DELAY; } - // Unlock the CHIP stack, allowing other threads to enter CHIP while the event loop thread is sleeping. - Impl()->UnlockChipStack(); - - BaseType_t eventReceived = xQueueReceive(mChipEventQueue, &event, waitTime); - - // Lock the CHIP stack. - Impl()->LockChipStack(); + BaseType_t eventReceived = pdFALSE; + { + // Unlock the CHIP stack, allowing other threads to enter CHIP while + // the event loop thread is sleeping. + StackUnlock unlock; + eventReceived = xQueueReceive(mChipEventQueue, &event, waitTime); + } // If an event was received, dispatch it. Continue receiving events from the queue and // dispatching them until the queue is empty. @@ -236,6 +255,9 @@ void GenericPlatformManagerImpl_FreeRTOS::EventLoopTaskMain(void * ar { ChipLogDetail(DeviceLayer, "CHIP task running"); static_cast *>(arg)->Impl()->RunEventLoop(); + // TODO: At this point, should we not + // vTaskDelete(static_cast *>(arg)->mEventLoopTask)? + // Or somehow get our caller to do it once this thread is joined? } template @@ -275,6 +297,7 @@ void GenericPlatformManagerImpl_FreeRTOS::PostEventFromISR(const Chip template void GenericPlatformManagerImpl_FreeRTOS::_Shutdown(void) { + GenericPlatformManagerImpl::_Shutdown(); } template diff --git a/src/platform/ESP32/PlatformManagerImpl.cpp b/src/platform/ESP32/PlatformManagerImpl.cpp index d2254692924cca..10f9554477bf13 100644 --- a/src/platform/ESP32/PlatformManagerImpl.cpp +++ b/src/platform/ESP32/PlatformManagerImpl.cpp @@ -150,6 +150,8 @@ void PlatformManagerImpl::_Shutdown() } Internal::GenericPlatformManagerImpl_FreeRTOS::_Shutdown(); + + esp_event_loop_delete_default(); } void PlatformManagerImpl::HandleESPSystemEvent(void * arg, esp_event_base_t eventBase, int32_t eventId, void * eventData) diff --git a/src/system/tests/BUILD.gn b/src/system/tests/BUILD.gn index 51f321ef8dfe94..9d4e1c2b7958f3 100644 --- a/src/system/tests/BUILD.gn +++ b/src/system/tests/BUILD.gn @@ -31,7 +31,7 @@ chip_test_suite("tests") { "TestTimeSource.cpp", ] - if (chip_device_platform != "esp32" && chip_device_platform != "fake") { + if (chip_device_platform != "fake") { test_sources += [ "TestSystemScheduleWork.cpp" ] } diff --git a/src/test_driver/esp32/run_qemu_image.py b/src/test_driver/esp32/run_qemu_image.py index d871083a0c1c77..698649fc64c5d7 100755 --- a/src/test_driver/esp32/run_qemu_image.py +++ b/src/test_driver/esp32/run_qemu_image.py @@ -17,6 +17,7 @@ import click import logging import os +import re import sys import subprocess @@ -104,19 +105,41 @@ def main(log_level, no_log_timestamps, image, file_image_list, qemu, verbose): (path, status.returncode)) # Parse output of the unit test. Generally expect things like: + # I (3034) CHIP-tests: Starting CHIP tests! + # ... + # Various test lines, none ending with "] : FAILED" + # ... # Failed Tests: 0 / 5 # Failed Asserts: 0 / 77 # I (3034) CHIP-tests: CHIP test status: 0 + in_test = False for line in output.split('\n'): + if line.endswith('CHIP-tests: Starting CHIP tests!'): + in_test = True + if line.startswith('Failed Tests:') and not line.startswith('Failed Tests: 0 /'): raise Exception("Tests seem failed: %s" % line) if line.startswith('Failed Asserts:') and not line.startswith('Failed Asserts: 0 /'): raise Exception("Asserts seem failed: %s" % line) - if 'CHIP test status: ' in line and 'CHIP test status: 0' not in line: + if 'CHIP-tests: CHIP test status: 0' in line: + in_test = False + elif 'CHIP-tests: CHIP test status: ' in line: raise Exception("CHIP test status is NOT 0: %s" % line) + # Ignore FAILED messages not in the middle of a test, to reduce + # the chance of false posisitves from other logging. + if in_test and re.match('.*] : FAILED$', line): + raise Exception("Step failed: %s" % line) + + # TODO: Figure out why exit(0) in man_app.cpp's tester_task is aborting and fix that. + if in_test and line.startswith('abort() was called at PC'): + raise Exception("Unexpected crash: %s" % line) + + if in_test: + raise Exception('Not expected to be in the middle of a test when the log ends') + if verbose: print("========== TEST OUTPUT BEGIN ============") print(output) @@ -125,7 +148,9 @@ def main(log_level, no_log_timestamps, image, file_image_list, qemu, verbose): logging.info("Image %s PASSED", path) except: # make sure output is visible in stdout + print("========== TEST OUTPUT BEGIN ============") print(output) + print("========== TEST OUTPUT END ============") raise From 1157652e292e098688b314cda0fed47e1d9a8c19 Mon Sep 17 00:00:00 2001 From: yunhanw-google Date: Fri, 9 Sep 2022 06:57:26 -0700 Subject: [PATCH 17/20] [Debugging]fix cirque test via releasing extra storage (#22500) * update build binary instruction * adjust thread border router log expectation --- .github/workflows/cirque.yaml | 10 +----- scripts/build/gn_gen_cirque.sh | 36 +++++++++++++++++++ .../linux-cirque/helper/CHIPTestBase.py | 2 +- 3 files changed, 38 insertions(+), 10 deletions(-) create mode 100755 scripts/build/gn_gen_cirque.sh diff --git a/.github/workflows/cirque.yaml b/.github/workflows/cirque.yaml index b09770909bd74d..e76022d79f24e0 100644 --- a/.github/workflows/cirque.yaml +++ b/.github/workflows/cirque.yaml @@ -111,15 +111,7 @@ jobs: --volume /tmp:/tmp \ -- sh -c '\ git config --global --add safe.directory "*" \ - && ./gn_build.sh \ - chip_build_tests=false \ - chip_enable_wifi=false \ - chip_im_force_fabric_quota_check=true \ - enable_default_builds=false \ - enable_host_gcc_build=true \ - enable_standalone_chip_tool_build=true \ - enable_linux_all_clusters_app_build=true \ - enable_linux_lighting_app_build=true \ + && scripts/build/gn_gen_cirque.sh ' - name: Run Tests timeout-minutes: 25 diff --git a/scripts/build/gn_gen_cirque.sh b/scripts/build/gn_gen_cirque.sh new file mode 100755 index 00000000000000..f65525bd6dfb7b --- /dev/null +++ b/scripts/build/gn_gen_cirque.sh @@ -0,0 +1,36 @@ +#!/usr/bin/env bash + +# +# Copyright (c) 2022 Project CHIP Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +set -e + +CHIP_ROOT="$(dirname "$0")/../.." +if [[ -z "${CHIP_ROOT_PATH}" ]]; then + CHIP_ROOT_PATH="" +fi + +set -x + +env + +echo "Build: GN configure" + +gn --root="$CHIP_ROOT" gen --check --fail-on-unused-args "$CHIP_ROOT/out/debug" --args='target_os="all"'"chip_build_tests=false chip_enable_wifi=false chip_im_force_fabric_quota_check=true enable_default_builds=false enable_host_gcc_build=true enable_standalone_chip_tool_build=true enable_linux_all_clusters_app_build=true enable_linux_lighting_app_build=true" + +echo "Build: Ninja build" + +time ninja -C "$CHIP_ROOT/out/debug" all check diff --git a/src/test_driver/linux-cirque/helper/CHIPTestBase.py b/src/test_driver/linux-cirque/helper/CHIPTestBase.py index 9ada727aee90b5..415d12acda36fa 100644 --- a/src/test_driver/linux-cirque/helper/CHIPTestBase.py +++ b/src/test_driver/linux-cirque/helper/CHIPTestBase.py @@ -140,7 +140,7 @@ def reset_thread_devices(self, devices: Union[List[str], str]): for device_id in devices: # Wait for otbr-agent and CHIP server start self.assertTrue(self.wait_for_device_output( - device_id, "Border router agent started.", 10)) + device_id, "Thread Border Router started on AIL", 10)) self.assertTrue(self.wait_for_device_output( device_id, "CHIP:SVR: Server Listening...", 15)) # Clear default Thread network commissioning data From 7a45a4bb63e107856b2a07ae3682c95f4963993b Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Fri, 9 Sep 2022 09:58:33 -0400 Subject: [PATCH 18/20] Mark MTRDeviceController as not running before shutting down the DeviceCommissioner. (#22490) Fixes https://github.com/project-chip/connectedhomeip/issues/22488 --- src/darwin/Framework/CHIP/MTRDeviceController.mm | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm index e811579fcbd32f..d5365145da9821 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController.mm @@ -148,9 +148,13 @@ - (void)cleanupAfterStartup - (void)shutDownCppController { if (_cppCommissioner) { - _cppCommissioner->Shutdown(); - delete _cppCommissioner; + auto * commissionerToShutDown = _cppCommissioner; + // Flag ourselves as not running before we start shutting down + // _cppCommissioner, so we're not in a state where we claim to be + // running but are actually partially shut down. _cppCommissioner = nullptr; + commissionerToShutDown->Shutdown(); + delete commissionerToShutDown; if (_operationalCredentialsDelegate != nil) { _operationalCredentialsDelegate->SetDeviceCommissioner(nullptr); } From e220c23d0a745f0b068b7358d5e6d410a4831ad2 Mon Sep 17 00:00:00 2001 From: Martin Turon Date: Fri, 9 Sep 2022 07:10:08 -0700 Subject: [PATCH 19/20] [docs] Improve instructions for building Android. (#22462) * [docs] Improve instructions for building Android. * [docs] [android] Clarify where to find Sync Project with Gradle. --- docs/guides/android_building.md | 55 +++++++++++++++++++++++++++------ 1 file changed, 45 insertions(+), 10 deletions(-) diff --git a/docs/guides/android_building.md b/docs/guides/android_building.md index 6ca2d91dd54a1a..c7c6983a2e0c6e 100644 --- a/docs/guides/android_building.md +++ b/docs/guides/android_building.md @@ -15,14 +15,17 @@ There are following Apps on Android
-- [Source files](#source) -- [Requirements for building](#requirements) - - [ABIs and TARGET_CPU](#abi) - - [Gradle & JDK Version](#jdk) -- [Preparing for build](#preparing) -- [Building Android CHIPTool from scripts](#building-scripts) -- [Building Android CHIPTool from Android Studio](#building-studio) -- [Building Android CHIPTest from scripts](#building-chiptest-scripts) +- [Building Android](#building-android) + - [Source files](#source-files) + - [Requirements for building](#requirements-for-building) + - [Linux](#linux) + - [MacOS](#macos) + - [ABIs and TARGET_CPU](#abis-and-target_cpu) + - [Gradle & JDK Version](#gradle--jdk-version) + - [Preparing for build](#preparing-for-build) + - [Building Android CHIPTool from scripts](#building-android-chiptool-from-scripts) + - [Building Android CHIPTool from Android Studio](#building-android-chiptool-from-android-studio) + - [Building Android CHIPTest from scripts](#building-android-chiptest-from-scripts)
@@ -39,11 +42,43 @@ directory. ## Requirements for building -You need Android SDK 21 & NDK downloaded to your machine. Set the +You need Android SDK 21 & NDK 21.4.7075529 downloaded to your machine. Set the `$ANDROID_HOME` environment variable to where the SDK is downloaded and the `$ANDROID_NDK_HOME` environment variable to point to where the NDK package is downloaded. +1. Install [Android Studio](https://developer.android.com/studio) +2. Install NDK: + 1. Tools -> SDK Manager -> SDK Tools Tab + 2. Click [x] Show Package Details + 3. Select NDK (Side by Side) -> 21.4.7075529 + 4. Apply +3. Install Command Line Tools: + 1. Tools -> SDK Manager -> SDK Tools Tab -> Android SDK Command Line Tools + (latest) + 2. Apply +4. Install SDK 21: + 1. Tools -> SDK Manager -> SDK Platforms Tab -> Android 5.0 (Lollipop) SDK + Level 21 + 2. Apply +5. Install Emulator: + 1. Tools -> Device Manager -> Create device -> Pixel 5 -> Android S API 31 + -> Download + +### Linux + +``` +export ANDROID_HOME=~/Android/Sdk +export ANDROID_NDK_HOME=~/Android/Sdk/ndk/21.4.7075529 +``` + +### MacOS + +``` +export ANDROID_HOME=~/Library/Android/sdk +export ANDROID_NDK_HOME=~/Library/Android/sdk/ndk/21.4.7075529 +``` + ### ABIs and TARGET_CPU @@ -136,7 +171,7 @@ which allows us to directly edit core Matter code in-IDE. and `matterSourceBuildAbiFilters` to the desired ABIs in [src/android/CHIPTool/gradle.properties](https://github.com/project-chip/connectedhomeip/blob/master/src/android/CHIPTool/gradle.properties) -3) Open the project in Android Studio and run **Sync Project with Gradle +3) Open the project in Android Studio and run **File -> Sync Project with Gradle Files**. 4) Use one of the following options to build an Android package: From bf12fce630c7f0c04b2be0ddd59a3f8a86ecf6d6 Mon Sep 17 00:00:00 2001 From: Erwin Pan Date: Fri, 9 Sep 2022 22:11:01 +0800 Subject: [PATCH 20/20] Disable testing code which force custom commission (#22409) * Disable testing code which force custom commission * Description Regular commission should go through Standard Commision Flow. However the Custom Commission flow (for testing only) is enabled, and override the Standard Commission flown * Solution Disable the Custom Commission flow for testing only * Remove testing code which force custom commission * Description Regular commission should go through Standard Commision Flow. However the Custom Commission flow (for testing only) is enabled, and override the Standard Commission flown * Solution Remove the Custom Commission flow for testing only --- examples/platform/linux/AppMain.cpp | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/examples/platform/linux/AppMain.cpp b/examples/platform/linux/AppMain.cpp index 35f85dc3f016ca..9e59d2e63605be 100644 --- a/examples/platform/linux/AppMain.cpp +++ b/examples/platform/linux/AppMain.cpp @@ -201,17 +201,6 @@ int ChipLinuxAppInit(int argc, char * const argv[], OptionSet * customOptions) PrintOnboardingCodes(LinuxDeviceOptions::GetInstance().payload); } - { - // For testing of manual pairing code with custom commissioning flow - ChipLogProgress(NotSpecified, "==== Onboarding payload for Custom Commissioning Flows ===="); - err = GetPayloadContents(LinuxDeviceOptions::GetInstance().payload, rendezvousFlags); - SuccessOrExit(err); - - LinuxDeviceOptions::GetInstance().payload.commissioningFlow = chip::CommissioningFlow::kCustom; - - PrintOnboardingCodes(LinuxDeviceOptions::GetInstance().payload); - } - #if defined(PW_RPC_ENABLED) rpc::Init(); ChipLogProgress(NotSpecified, "PW_RPC initialized.");