Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Part 2: feature/relative humidity measurement cluster #7028

Merged
merged 18 commits into from May 28, 2021
Merged

Part 2: feature/relative humidity measurement cluster #7028

merged 18 commits into from May 28, 2021

Conversation

ghost
Copy link

@ghost ghost commented May 21, 2021

Problem

Add support for the relative humidity measurement cluster.

Change overview

  • added emberAfRelativeHumidityMeasurementClusterServerInitCallback support
  • added relative-humidity-measurement-server.cpp impl.
  • added relative humidity measurement cluster support to chip-device-ctl

Testing

Tested on custom accessory with relative humidity sensor. Test setup was similar to TE2.

HW Platform

Nordic nRF52840

Notes

Related PRs

src/app/zap_cluster_list.py Outdated Show resolved Hide resolved
src/app/zap_cluster_list.py Outdated Show resolved Hide resolved
src/app/zap_cluster_list.py Outdated Show resolved Hide resolved
src/app/zap_cluster_list.py Outdated Show resolved Hide resolved
@@ -5647,7 +5759,7 @@
],
"attributes": [
{
"name": "acceptsHeaderList",
"name": "accepts header list",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change sounds unrelated to the current PR. Maybe rebasing this PR on top of master will remove it ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a change created by running the scripts/tools/zap_regen_all.py

Copy link
Contributor

@vivien-apple vivien-apple left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving for TE3.

It seems like the result of this PR is mostly to expose a set of accessors to facilitate the manipulation of the relative humidity measurement cluster attributes.
It makes perfect sense to facilitate it imho, but I would rather prefer to autogenerate helpers code such as what is proposed in #7183, instead of having a lot of new clusters introduced into src/app/clusters.

Dimitri Mizenko and others added 9 commits May 28, 2021 16:55
var endpointClusterWithInit =
[ 'Basic', 'Identify', 'Groups', 'Scenes', 'Occupancy Sensing', 'On/off', 'Level Control', 'Color Control', 'IAS Zone' ];
var endpointClusterWithInit = [
'Basic', 'Relative Humidity Measurement', 'Identify', 'Groups', 'Scenes', 'Occupancy Sensing', 'On/off', 'Level Control',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to add this here if we don't actually have an init method.

@@ -4994,6 +4994,118 @@
}
]
},
{
"name": "Relative Humidity Measurement",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be on endpoint 1, not endpoint 0.

@github-actions
Copy link

Size increase report for "esp32-example-build" from b4d7048

File Section File VM
chip-all-clusters-app.elf .flash.rodata 200 200
chip-all-clusters-app.elf .dram0.bss 0 192
chip-all-clusters-app.elf .flash.text 8 8
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-pigweed-app.elf and ./pull_artifact/chip-pigweed-app.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-all-clusters-app.elf and ./pull_artifact/chip-all-clusters-app.elf:

sections,vmsize,filesize
.flash.rodata,200,200
.dram0.bss,192,0
.debug_str,0,113
.strtab,0,59
.debug_info,0,33
.debug_loc,0,31
.debug_frame,0,24
.symtab,0,16
.flash.text,8,8
.shstrtab,0,1
.xt.prop._ZTVN4chip11DeviceLayer37DeviceNetworkProvisioningDelegateImplE,0,-1
.debug_line,0,-28
[Unmapped],0,-200


@bzbarsky-apple bzbarsky-apple merged commit e08afe7 into project-chip:master May 28, 2021
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this pull request Sep 23, 2021
)

* format endpointClusterWithInit for easier pull requests

* add support for emberAfRelativeHumidityMeasurementClusterServerInitCallback

* add server side relative humidity measurement cluster impl.

* add relative humidity measurement cluster to controller-clusters.zap

* Add new clusters to zap_cluster_list.py

* Restyled by clang-format

* apply fixes

- replace emberAfReadAttribute with emberAfReadServerAttribute
- replace emberAfWriteAttribute with emberAfWriteServerAttribute
- add emberAfRelativeHumidityMeasurementClusterPrintln
- let the caller do the logging

* remove emberAfRelativeHumidityMeasurementClusterServerInitCallback impl.

The default values are populated by the attribute storage.

* fix year in copyright

* remove unrelated clusters in zap_cluster_list.py

* add relative humidity measurement to all-cluster-app.zap

* fix relative humidity measurement includes

* temptive fix for esp32/all-cluster-app

* another try to fix all-cluster-app/esp32

* regen all

* Remove unnecessary init callback from Relative Humidity Measurement cluster

* Add Relative Humidity Measurement cluster on endpoint 1 also in all-clusters-app

* Regenerate generated files

Co-authored-by: Vivien Nicolas <vnicolas@apple.com>
Co-authored-by: Restyled.io <commits@restyled.io>
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants