Zigbee: Add dimmable light endpoint class by FaBjE · Pull Request #10727 · espressif/arduino-esp32

FaBjE · 2024-12-13T13:37:27Z

Description of Change

This PR adds a Zigbee endpoint for a normal dimmable light. (So no colors or white-temperature change)
I based it on the "color dimmable light" example.

Unfortunately this endpoint type is not in the ESP Zigbee library by default.
So I had to make my own "esp_zb_dimmable_light_clusters_create" function.
It appears to be functioning good, but please do a proper check if I figured it out correctly.

Tests scenarios

I've tested this with Home Assistant.
The Arduino applications runs on a ESP32-C6 with Arduino-esp32 core on "master" branch.

I'm looking forward to your feedback 😃

CLAassistant · 2024-12-13T13:37:35Z

All committers have signed the CLA.

github-actions · 2024-12-13T13:38:07Z

	Warnings
⚠️	Some issues found for the commit messages in this PR: the commit message `"feat(Zigbee): Add Zigbee Dimmable light endpoint class"`: scope/component should be lowercase without whitespace, allowed special characters are `_` `/` `.` `,` `` `-` `.` the commit message `"feat(Zigbee): Add Zigbee Dimmable light example"`: scope/component* should be lowercase without whitespace, allowed special characters are `_` `/` `.` `,` `` `-` `.` the commit message `"feat(Zigbee): Add Zigbee Dimmable light to CMakeLists.txt"`: scope/component* should be lowercase without whitespace, allowed special characters are `_` `/` `.` `,` `` `-` `.` the commit message `"feat(Zigbee): Add additional zigbee enabled check"`: scope/component* should be lowercase without whitespace, allowed special characters are `_` `/` `.` `,` `` `-` `.` the commit message `"feat(Zigbee): Adjusted example author comment"`: scope/component* should be lowercase without whitespace, allowed special characters are `_` `/` `.` `,` `` `-` `.` the commit message `"feat(Zigbee): Applied formatter + add formatter protection"`: scope/component* should be lowercase without whitespace, allowed special characters are `_` `/` `.` `,` `` `-` `.` the commit message `"feat(Zigbee): Update Zigbee Dimmable light example config/define names"`: scope/component* should be lowercase without whitespace, allowed special characters are `_` `/` `.` `,` `` `-` `.` the commit message `"feat(Zigbee): Update Zigbee Dimmable light example to 3.1.x features"`: scope/component* should be lowercase without whitespace, allowed special characters are `_` `/` `.` `,` `` `-` `.` Please fix these commit messages* - here are some basic tips: follow Conventional Commits style correct format of commit message should be: `<type/action>(<scope/component>): <summary>`, for example `fix(esp32): Fixed startup timeout issue` allowed types are: `change,ci,docs,feat,fix,refactor,remove,revert,test` sufficiently descriptive message summary should be between 20 to 72 characters and start with upper case letter avoid Jira references in commit messages (unavailable/irrelevant for our customers) `TIP:` Install pre-commit hooks and run this check when committing (uses the Conventional Precommit Linter).
⚠️	Please consider squashing your 10 commits (simplifying branch history).
⚠️	The source branch `"feature/zigbeeDimmableLight"` incorrect format: contains uppercase letters. This can cause troubles on case-insensitive file systems (macOS). Please rename your branch.
⚠️	The target branch for this Pull Request must be the default branch of the project (`master`). If you would like to add this feature to a different branch, please state this in the PR description and we will consider it.

👋 Hello FaBjE, we appreciate your contribution to this project!

Click to see more instructions ...

This automated output is generated by the PR linter DangerJS, which checks if your Pull Request meets the project's requirements and helps you fix potential issues.

DangerJS is triggered with each push event to a Pull Request and modify the contents of this comment.

Please consider the following:
- Danger mainly focuses on the PR structure and formatting and can't understand the meaning behind your code or changes.
- Danger is not a substitute for human code reviews; it's still important to request a code review from your colleagues.
- Resolve all warnings (⚠️ ) before requesting a review from human reviewers - they will appreciate it.
- To manually retry these Danger checks, please navigate to the Actions tab and re-run last Danger workflow.

Review and merge process you can expect ...

We do welcome contributions in the form of bug reports, feature requests and pull requests.

1. An internal issue has been created for the PR, we assign it to the relevant engineer.
2. They review the PR and either approve it or ask you for changes or clarifications.
3. Once the GitHub PR is approved we do the final review, collect approvals from core owners and make sure all the automated tests are passing.
- At this point we may do some adjustments to the proposed change, or extend it by adding tests or documentation.
4. If the change is approved and passes the tests it is merged into the default branch.

Generated by 🚫 dangerJS against 4f863d5

me-no-dev · 2024-12-13T13:40:12Z

Please add example that is inline with the other zigbee examples

me-no-dev · 2024-12-13T13:40:41Z

Also please target the release/v3.1.x branch

github-actions · 2024-12-13T13:47:39Z

Test Results

62 files 62 suites 5m 41s ⏱️
21 tests 21 ✅ 0 💤 0 ❌
154 runs 154 ✅ 0 💤 0 ❌

Results for commit 4f863d5.

♻️ This comment has been updated with latest results.

SuGlider · 2024-12-13T14:46:21Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.You have signed the CLA already but the status is still pending? Let us recheck it.

@FaBjE - Please sign the CLA.

P-R-O-C-H-Y · 2024-12-13T16:18:28Z

Hi @FaBjE, I am happy to see a PR from community. Overall it's looking good, but I am missing an implementation of setting the light manually. Can you please check a ZigbeeColorDimmableLight you have as a template on the release/v3.1.x branch? There was more stuff added recently.

arduino-esp32/libraries/Zigbee/src/ep/ZigbeeColorDimmableLight.h

Line 24 in 7dc1c92

void setLightState(bool state);

So you are missing:

  void setLightState(bool state);
  void setLightLevel(uint8_t level);
  void setLight(bool state, uint8_t level);

  bool getLightState() {
    return _current_state;
  }
  uint8_t getLightLevel() {
    return _current_level;
  }

Add a endpoint type class for a dimmable light. Based on a copy of color dimmable light.

Add example for a dimmable light. Based on a copy of color dimmable light example.

libraries/Zigbee/src/ep/ZigbeeDimmableLight.h

FaBjE · 2024-12-13T18:18:02Z

@me-no-dev
I added an example and modified the target branch.

I modified my commit message according the bot message.
I can "rename" the branch, but we will probably loose this PR. Please advise.

@P-R-O-C-H-Y
Thanks for the review. I have:

Added the missing functions (Sorry copied from 3.0.7)
Updated the example to 3.1.x standards
Renamed define/config struct (did change ZIGEEE_ to ZIGBEE_ as I assumed a typo)

P-R-O-C-H-Y · 2024-12-13T19:56:04Z

@FaBjE Please add the source file to the CMakeLists.txt file here:

arduino-esp32/CMakeLists.txt

Line 281 in 7dc1c92

libraries/Zigbee/src/ep/ZigbeeThermostat.cpp

FaBjE · 2024-12-14T10:18:00Z

@FaBjE Please add the source file to the CMakeLists.txt file here:

arduino-esp32/CMakeLists.txt

Line 281 in 7dc1c92

libraries/Zigbee/src/ep/ZigbeeThermostat.cpp

Added. Somehow it just compiled fine without here...

github-actions · 2024-12-14T16:09:31Z

Memory usage test (comparing PR against master branch)

The table below shows the summary of memory usage change (decrease - increase) in bytes and percentage for each target.

Memory	FLASH [bytes]		FLASH [%]		RAM [bytes]		RAM [%]
Target	DEC	INC	DEC	INC	DEC	INC	DEC	INC
ESP32C6	0	‼️ +21K	0.00	‼️ +3.90	💚 -476	⚠️ +604	💚 -1.44	‼️ +2.03
ESP32H2	0	‼️ +16K	0.00	‼️ +2.96	💚 -596	⚠️ +468	💚 -1.84	‼️ +1.61

Click to expand the detailed deltas report [usage change in BYTES]

Target	ESP32C6		ESP32H2
Example	FLASH	RAM	FLASH	RAM
Zigbee/examples/Zigbee_CarbonDioxide_Sensor	-	-	-	-
Zigbee/examples/Zigbee_Color_Dimmable_Light	‼️ +21K	⚠️ +588	‼️ +16K	⚠️ +444
Zigbee/examples/Zigbee_Color_Dimmer_Switch	‼️ +13K	💚 -148	‼️ +9K	💚 -276
Zigbee/examples/Zigbee_Dimmable_Light	-	-	-	-
Zigbee/examples/Zigbee_Occupancy_Sensor	-	-	-	-
Zigbee/examples/Zigbee_On_Off_Light	‼️ +20K	⚠️ +604	‼️ +16K	⚠️ +468
Zigbee/examples/Zigbee_On_Off_Switch	‼️ +13K	💚 -172	‼️ +9K	💚 -276
Zigbee/examples/Zigbee_Pressure_Flow_Sensor	-	-	-	-
Zigbee/examples/Zigbee_Scan_Networks	‼️ +17K	⚠️ +176	‼️ +12K	0
Zigbee/examples/Zigbee_Temp_Hum_Sensor_Sleepy	‼️ +18K	💚 -104	‼️ +13K	💚 -296
Zigbee/examples/Zigbee_Temperature_Sensor	‼️ +15K	💚 -132	‼️ +11K	💚 -292
Zigbee/examples/Zigbee_Thermostat	‼️ +15K	💚 -476	‼️ +10K	💚 -596

lucasssvaz · 2024-12-14T16:37:31Z

@FaBjE Please add the source file to the CMakeLists.txt file here:

arduino-esp32/CMakeLists.txt

Line 281 in 7dc1c92

libraries/Zigbee/src/ep/ZigbeeThermostat.cpp

Added. Somehow it just compiled fine without here...

It is only used when compiling as an IDF component.

libraries/Zigbee/examples/Zigbee_Dimmable_Light/Zigbee_Dimmable_Light.ino

libraries/Zigbee/src/ep/ZigbeeDimmableLight.h

As suggested by code review

P-R-O-C-H-Y · 2024-12-16T07:22:53Z

Thank you @FaBjE for adding another endpoint class :)

P-R-O-C-H-Y · 2024-12-16T09:26:06Z

PR is now ready to be merged. @FaBjE hope you don't mind that I committed small change to avoid possible conflicts for future esp-zigbee-sdk releases :)

FaBjE · 2024-12-16T11:50:57Z

PR is now ready to be merged. @FaBjE hope you don't mind that I committed small change to avoid possible conflicts for future esp-zigbee-sdk releases :)

Not at all. Makes complete sense, we just missed it during the rename of the others.
I kept the functions prefix the same as in the zigbee stack, but now that is outside of that scope it shouldn't be.

I'm glad it is merged, and hope it will benefit others.
I think the (only) one still missing is the white-temperature tuneable light.

FaBjE force-pushed the feature/zigbeeDimmableLight branch from 0d3656d to e473560 Compare December 13, 2024 16:14

FaBjE changed the base branch from master to release/v3.1.x December 13, 2024 16:14

FaBjE added 2 commits December 13, 2024 17:23

feat(Zigbee): Add Zigbee Dimmable light endpoint class

a44e457

Add a endpoint type class for a dimmable light. Based on a copy of color dimmable light.

feat(Zigbee): Add Zigbee Dimmable light example

aa0c3da

Add example for a dimmable light. Based on a copy of color dimmable light example.

P-R-O-C-H-Y reviewed Dec 13, 2024

View reviewed changes

libraries/Zigbee/src/ep/ZigbeeDimmableLight.h Outdated Show resolved Hide resolved

P-R-O-C-H-Y reviewed Dec 13, 2024

View reviewed changes

libraries/Zigbee/src/ep/ZigbeeDimmableLight.h Outdated Show resolved Hide resolved

P-R-O-C-H-Y added Status: In Progress ⚠️ Issue is in progress Area: Libraries Issue is related to Library support. labels Dec 13, 2024

FaBjE added 2 commits December 13, 2024 19:09

feat(Zigbee): Update Zigbee Dimmable light example to 3.1.x features

538c57a

feat(Zigbee): Update Zigbee Dimmable light example config/define names

a0d57bb

FaBjE force-pushed the feature/zigbeeDimmableLight branch from e473560 to a0d57bb Compare December 13, 2024 18:16

P-R-O-C-H-Y added this to the 3.1.0 milestone Dec 13, 2024

feat(Zigbee): Add Zigbee Dimmable light to CMakeLists.txt

af541a5

feat(Zigbee): Add additional zigbee enabled check

42f9cfc

lucasssvaz reviewed Dec 14, 2024

View reviewed changes

libraries/Zigbee/examples/Zigbee_Dimmable_Light/Zigbee_Dimmable_Light.ino Outdated Show resolved Hide resolved

lucasssvaz reviewed Dec 14, 2024

View reviewed changes

libraries/Zigbee/src/ep/ZigbeeDimmableLight.h Show resolved Hide resolved

FaBjE added 2 commits December 14, 2024 17:57

feat(Zigbee): Applied formatter + add formatter protection

ff7e504

As suggested by code review

feat(Zigbee): Adjusted example author comment

ae274ee

lucasssvaz approved these changes Dec 14, 2024

View reviewed changes

P-R-O-C-H-Y approved these changes Dec 16, 2024

View reviewed changes

P-R-O-C-H-Y added Status: Pending Merge Pull Request is ready to be merged and removed Status: In Progress ⚠️ Issue is in progress labels Dec 16, 2024

ci(pre-commit): Apply automatic fixes

ddf8c42

P-R-O-C-H-Y removed the Status: Pending Merge Pull Request is ready to be merged label Dec 16, 2024

fix(zigbee): Rename method to avoid future conflict

4f863d5

P-R-O-C-H-Y added the Status: Pending Merge Pull Request is ready to be merged label Dec 16, 2024

me-no-dev merged commit 926c043 into espressif:release/v3.1.x Dec 16, 2024

Conversation

FaBjE commented Dec 13, 2024

Description of Change

Tests scenarios

Uh oh!

CLAassistant commented Dec 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Dec 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

me-no-dev commented Dec 13, 2024

Uh oh!

me-no-dev commented Dec 13, 2024

Uh oh!

github-actions bot commented Dec 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

SuGlider commented Dec 13, 2024

Uh oh!

P-R-O-C-H-Y commented Dec 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

FaBjE commented Dec 13, 2024

Uh oh!

P-R-O-C-H-Y commented Dec 13, 2024

Uh oh!

FaBjE commented Dec 14, 2024

Uh oh!

github-actions bot commented Dec 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Memory usage test (comparing PR against master branch)

Uh oh!

lucasssvaz commented Dec 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

P-R-O-C-H-Y commented Dec 16, 2024

Uh oh!

P-R-O-C-H-Y commented Dec 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

FaBjE commented Dec 16, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

CLAassistant commented Dec 13, 2024 •

edited

Loading

github-actions bot commented Dec 13, 2024 •

edited

Loading

github-actions bot commented Dec 13, 2024 •

edited

Loading

P-R-O-C-H-Y commented Dec 13, 2024 •

edited

Loading

github-actions bot commented Dec 14, 2024 •

edited

Loading

lucasssvaz commented Dec 14, 2024 •

edited

Loading

P-R-O-C-H-Y commented Dec 16, 2024 •

edited

Loading