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

[ESP32]Add ColorControl for ESP32C3-DevKitM #8201

Merged

Conversation

wqx6
Copy link
Contributor

@wqx6 wqx6 commented Jul 8, 2021

Problem

We should use the on-board RGB-LED instead of an external LED for ESP32C3-DevKitM.

Change overview

  1. Use onboard RGB-LED(WS2812) for status-LED in all-clusters-app
  2. Add ColorControl cluster support for ESP32C3_DevkitM

Testing

Tested the On/Off/Toggle, MoveToLevel, MoveToHue/MoveToSaturation commands manually.

Used the following commands to change the hue and saturation.

zcl ColorControl MoveToHue 12345 1 1 hue=100 direction=0 transitionTime=0 optionsMask=0 optionsOverride=0
zcl ColorControl MoveToSaturation 12345 1 1 saturation=200 transitionTime=0 optionsMask=0 optionsOverride=0

@dhrishi
Copy link
Contributor

dhrishi commented Jul 8, 2021

Tested the On/Off/Toggle, MoveToLevel, MoveToHue/MoveToSaturation commands manually.

@wqx6 Can you please add a few actual commands for reference

examples/all-clusters-app/esp32/main/DeviceCallbacks.cpp Outdated Show resolved Hide resolved
Comment on lines 185 to 186
emberAfReadServerAttribute(endpointId, ZCL_COLOR_CONTROL_CLUSTER_ID, ZCL_COLOR_CONTROL_CURRENT_SATURATION_ATTRIBUTE_ID,
reinterpret_cast<uint8_t *>(&saturation), sizeof(uint8_t));
Copy link
Contributor

Choose a reason for hiding this comment

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

Include app/common/gen/attributes/Accessors.h and use:

  app::Clusters::ColorControl::Attributes::GetCurrentSaturation(endpointId, &saturation);

(or using namespace chip::app::Clusters::ColorControl::Attributes and then skip the prefix, of course).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are two issues after including the app/common/gen/attributes/Accessors.h

  1. The app/common/gen/attributes/Accessors.h(or app/common/gen/attributes/Accessors.cpp ) doesn't include files app/util/af.h and app/common/gen/attribute-type.h.
  2. The Attribute LastConfiguredBy of the cluster Scenes has an undefined type ZCL_EUI64_ATTRIBUTE_TYPE.

Copy link
Contributor

Choose a reason for hiding this comment

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

@vivien-apple See above.

@wqx6 alright, let's not worry about that for now. But in that case, can we at least remove the unnecessary reinterpret_cast bits?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

Comment on lines 191 to 192
emberAfReadServerAttribute(endpointId, ZCL_COLOR_CONTROL_CLUSTER_ID, ZCL_COLOR_CONTROL_CURRENT_HUE_ATTRIBUTE_ID,
reinterpret_cast<uint8_t *>(&hue), sizeof(uint8_t));
Copy link
Contributor

Choose a reason for hiding this comment

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

app::Clusters::ColorControl::Attributes::GetCurrentHue(endpointId, &hue);

@wqx6
Copy link
Contributor Author

wqx6 commented Jul 9, 2021

@wqx6 Can you please add a few actual commands for reference

Done

use onboard RGB-LED(WS2812) for status-LED in all-clusters-app
add colorcontrol cluster for ESP32C3_DevkitM
@wqx6 wqx6 force-pushed the add_color_support_for_esp32c3 branch from 9e474ff to 731c790 Compare July 9, 2021 05:37
@github-actions
Copy link

github-actions bot commented Jul 9, 2021

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

File Section File VM
chip-lock-app.elf .flash.text 64 64
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

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

sections,vmsize,filesize
.flash.text,64,64
[Unmapped],0,-64

Comparing ./master_artifact/chip-shell.elf and ./pull_artifact/chip-shell.elf:

sections,vmsize,filesize

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

sections,vmsize,filesize

Comparing ./master_artifact/chip-persistent-storage.elf and ./pull_artifact/chip-persistent-storage.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-temperature-measurement-app.elf and ./pull_artifact/chip-temperature-measurement-app.elf:

sections,vmsize,filesize

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

sections,vmsize,filesize


@bzbarsky-apple bzbarsky-apple merged commit a999ccd into project-chip:master Jul 9, 2021
@wqx6 wqx6 deleted the add_color_support_for_esp32c3 branch July 12, 2021 02:37
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this pull request Sep 23, 2021
use onboard RGB-LED(WS2812) for status-LED in all-clusters-app
add colorcontrol cluster for ESP32C3_DevkitM
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