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

feat(zigbee): Add Zigbee library #10265

Open
wants to merge 33 commits into
base: master
Choose a base branch
from

Conversation

P-R-O-C-H-Y
Copy link
Member

@P-R-O-C-H-Y P-R-O-C-H-Y commented Aug 30, 2024

Description of Change

Introducing Zigbee Arduino library (wrapper for esp-zigbee-sdk).

Tests scenarios

Locally with all included examples.

Related links

Relates to issue #10135
ZB_STRUCT warnings fix espressif/esp-zigbee-sdk#416

Fixed ep_thermostat to compile successfully
Removed cb from all EP, as it have been removed, virtual methods will be used instead.
Moved zigbee handlers out of Zigbee_core to Zigbee_handlers for better readability.
Fixed zigbeeInit to be bool and return status of initialization for begin function.
Updated examples with edited roles and custom method for on_off light
Implemented basic function calls of switch commands to on/off light:
  lightToggle, lightOn, lightOff, ...
Implemented virtual methods for on/off light that have to be override in user code:
  setOnOff, sceneControl, setOnOffTime, setOffWaitTime
APIs can be changed, still early development.
Implemented Factory reset of Zigbee device, in order to connect to new network without reflashing/erasing flash
Implemented optional setting for Manufacturer and Model names
Added option to allow endpoint to have multiple endpoint connected -> switch - 2 lights (tested)
Minor sketches update
Implemented easy transfer from device it to Device type (0x0000 = ESP_ZB_HA_ON_OFF_SWITCH_DEVICE_ID  -> "General On/Off switch".
Implemeted color dimmable light and color dimmer switch HA devices + examples.
Removed unnecessary stored attribute cluster
Renamed on/off light and switch examples
Implemented Zigbee network scanning (async) to mostly match WiFi scan APIs.
Added Zigbee_Scan_Networks example
Implemeted thermostat and temperature sensor HA devices + examples.
Implemented configure report handler.
Updated READMEs and description of examples.
Minor code updates
Simplified bounded device print as the structure is common for any EP type
Allowed setting custom app version for EP, default is 0
Small fixes and code updates
@P-R-O-C-H-Y P-R-O-C-H-Y added the Area: Libraries Issue is related to Library support. label Aug 30, 2024
@P-R-O-C-H-Y P-R-O-C-H-Y self-assigned this Aug 30, 2024
Copy link
Contributor

github-actions bot commented Aug 30, 2024

Warnings
⚠️

Some issues found for the commit messages in this PR:

  • the commit message "Add Thermostat + fix enum":
    • summary looks empty
    • type/action looks empty
  • the commit message "Add check for SOC_IEEE802154_SUPPORTED":
    • summary looks empty
    • type/action looks empty
  • the commit message "Change virtual methods to callbacks (TODO)":
    • summary looks empty
    • type/action looks empty
  • the commit message "Dev Update: Color DImmable light + switch implemented":
    • summary looks empty
    • type/action looks empty
  • the commit message "Dev Update: Thermostat and Temperature sensor EP":
    • summary looks empty
    • type/action looks empty
  • the commit message "Dev Update: Version setting, Thermostat fix, ...":
    • summary should not end with a period (full stop)
    • summary looks empty
    • type/action looks empty
  • the commit message "Dev update: Device ID to string":
    • body's lines must not be longer than 100 characters
    • summary looks empty
    • type/action looks empty
  • the commit message "Dev update: Factory reset, names, multiple EPs":
    • body's lines must not be longer than 100 characters
    • summary looks empty
    • type/action looks empty
  • the commit message "Dev update: implement on/off light and switch methods":
    • summary looks empty
    • type/action looks empty
  • the commit message "Dev update: roles, cb removal, handlers":
    • summary looks empty
    • type/action looks empty
  • the commit message "Fill keyworkds.txt, remove unnecessary defines":
    • summary looks empty
    • type/action looks empty
  • the commit message "Fix compilation errors in examples":
    • summary looks empty
    • type/action looks empty
  • the commit message "Ignore false positive unused variable/function":
    • summary looks empty
    • type/action looks empty
  • the commit message "Implement Network Scanning":
    • summary looks empty
    • type/action looks empty
  • the commit message "Implement cmd default response handler":
    • summary looks empty
    • type/action looks empty
  • the commit message "Initial commit - light bulb + switch working":
    • summary looks empty
    • type/action looks empty
  • the commit message "Remove EP template + add lib to CMakeLists":
    • summary looks empty
    • type/action looks empty
  • the commit message "Remove outdated comments":
    • summary looks empty
    • type/action looks empty
  • the commit message "Remove ported IDF examples":
    • summary looks empty
    • type/action looks empty
  • the commit message "Remove unnecesary defaults defines":
    • summary looks empty
    • type/action looks empty
  • the commit message "Remove unused _identify_cluster":
    • summary looks empty
    • type/action looks empty
  • the commit message "Rename classes to have proper naming":
    • summary looks empty
    • type/action looks empty
  • the commit message "Rename methods, variables + make private/protected":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update Scan networks Readme":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update Zigbee examples with new APIs":
    • summary looks empty
    • type/action looks empty

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 33 commits (simplifying branch history).
Messages
📖 This PR seems to be quite large (total lines of code: 5254), you might consider splitting it into smaller PRs

👋 Hello P-R-O-C-H-Y, 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.
- Addressing info messages (📖) is strongly recommended; they're less critical but valuable.
- 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 9a354ae

Copy link
Contributor

github-actions bot commented Aug 30, 2024

Test Results

 56 files   -  83   56 suites   - 83   4m 40s ⏱️ - 1h 37m 48s
 21 tests  -   9   21 ✅  -   9  0 💤 ±0  0 ❌ ±0 
135 runs   - 168  135 ✅  - 168  0 💤 ±0  0 ❌ ±0 

Results for commit 9a354ae. ± Comparison against base commit cbe0f2f.

This pull request removes 9 tests.
performance.coremark.test_coremark ‑ test_coremark
performance.fibonacci.test_fibonacci ‑ test_fibonacci
performance.psramspeed.test_psramspeed ‑ test_psramspeed
performance.ramspeed.test_ramspeed ‑ test_ramspeed
performance.superpi.test_superpi ‑ test_superpi
test_touch_errors
test_touch_interrtupt
test_touch_read
validation.periman.test_periman ‑ test_periman

♻️ This comment has been updated with latest results.

@lucasssvaz lucasssvaz changed the title feat(): Zigbee library feat(zigbee): Add Zigbee library Aug 30, 2024
@Jason2866
Copy link
Collaborator

First like the great work you are doing!
But adding "everything" possible to the Arduino core? It is a library. It is not a lib every esp32 MCU can use. Imho it should be considered not to add to the core. Arduino has the possibility to use librarys. Maybe an option to use the Espressif registry?

@P-R-O-C-H-Y P-R-O-C-H-Y added Status: Review needed Issue or PR is awaiting review and removed Status: In Progress Issue is in progress labels Sep 10, 2024
@P-R-O-C-H-Y P-R-O-C-H-Y mentioned this pull request Sep 12, 2024
17 tasks
if((millis() - startTime) > 3000) {
// If key pressed for more than 3secs, factory reset Zigbee and reboot
Serial.printf("Reseting Zigbee to factory settings, reboot.\n");
Zigbee.factoryReset();
Copy link
Member

Choose a reason for hiding this comment

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

maybe break here?

Copy link
Member Author

Choose a reason for hiding this comment

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

No need, the Zigbee.factoryReset(); will reboot the device.

if((millis() - startTime) > 3000) {
// If key pressed for more than 3secs, factory reset Zigbee and reboot
Serial.printf("Reseting Zigbee to factory settings, reboot.\n");
Zigbee.factoryReset();
Copy link
Member

Choose a reason for hiding this comment

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

break here

Copy link
Member

Choose a reason for hiding this comment

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

or wait for the button to be released

Copy link
Member Author

Choose a reason for hiding this comment

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

No need, the Zigbee.factoryReset(); will reboot the device.

//Do what you want with the model string
Serial.printf("Model: %s\n", model);
}
};
Copy link
Member

Choose a reason for hiding this comment

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

is this class required here, if it's only to read manufacturer and model?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not required there, I just wanted to have it implemented in some example so users can see how to use it if they want to. I can remove it if you prefer, as its not necessary for the example.

@me-no-dev
Copy link
Member

Overall I like it, but take action to format the examples properly as I noted on one of them.

@P-R-O-C-H-Y P-R-O-C-H-Y added Status: In Progress Issue is in progress and removed Status: Review needed Issue or PR is awaiting review labels Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Libraries Issue is related to Library support. Status: In Progress Issue is in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants