Added servo component to the registry #419

pedrominatel · 2024-10-31T14:47:36Z

Description

This PR adds the servo to the ESP Component Registry, including an example.

Testing

N/A

Checklist

Before submitting a Pull Request, please ensure the following:

🚨 This PR does not introduce breaking changes.
All CI checks (GH Actions) pass.
Documentation is updated as needed.
Tests are updated or added as necessary.
Code is well-commented, especially in complex areas.
Git history is clean — commits are squashed to the minimum necessary.

CLAassistant · 2024-10-31T14:47:42Z

All committers have signed the CLA.

github-actions · 2024-10-31T14:48:17Z

	Warnings
⚠️	Some issues found for the commit messages in this PR: the commit message `"feat: Added servo component to the registry"`: body must have leading blank line 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).

	Messages
📖	You might consider squashing your 3 commits (simplifying branch history).

👋 Hello pedrominatel, 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 f00128b

leeebo · 2024-11-01T06:15:26Z

Hi @pedrominatel Thanks for your contribution!

YanKE01

Thank you for your contribution. We've made some suggestions in response to your submission to guarantee that the style of the component is consistent with the repository. In addition, we suggest adding the servo component to the CI, you can refer to this link.

components/motor/servo/idf_component.yml

components/motor/servo/README.md

components/motor/servo/examples/servo_control/CMakeLists.txt

pedrominatel · 2024-11-26T16:43:05Z

Thank you very much @YanKE01 for your review and thoughts.
Please take a look at the latest changes.

YanKE01

Hi, @pedrominatel . Thanks for your contribution. Some CI rules and component specifications still need your attention, and we've provided some reference suggestions.

YanKE01 · 2024-11-27T01:43:04Z

components/motor/servo/idf_component.yml

@@ -0,0 +1,9 @@
+version: 1.1.0


As the first version of the servo component and being on the master branch, I suggest using 0.1.0 as the initial version. Additionally, you need to add a CHANGELOG.md file to document the changes for each version. You can refer to other components in the repository, such as:

# ChangeLog ## v0.1.0 - 2024-11-27 ### Enhancements: * Initial version

YanKE01 · 2024-11-27T02:09:19Z

.gitlab/ci/build.yml

@@ -574,6 +574,18 @@ build_example_motor_foc_knob:
  variables:
    EXAMPLE_DIR: examples/motor/foc_knob_example

+build_example_motor_servo:


In addition to adding the servo component in build.yml, it is also necessary to add rules for servo in rules.yml to ensure that changes to servo and projects dependent on servo can trigger the CI. You can refer to any existing component in rules.yml as a reference to complete the rule definition for servo.

I did but I'm not sure if I did correctly. Can you kindly review it?
Thank you, @YanKE01.

YanKE01 · 2024-11-27T15:23:12Z

LGTM! Hi, @leeebo , can you help review this again? Thank you.

leeebo · 2024-11-28T06:39:42Z

@pedrominatel LGTM! Thanks. Let's sync the PR first to run the CI

leeebo · 2024-11-28T06:40:05Z

sha=9eda883f174cda110b084a4cd1726fb336bc4e74

leeebo · 2024-11-28T06:44:57Z

hi @pedrominatel please install pre-commit to the current repo using pre-commit install, there are some files miss the Check copyright notices

components/motor/servo/examples/servo_control/main/servo_control.c

pedrominatel · 2024-11-28T08:51:03Z

Thank you @leeebo.

leeebo · 2024-11-28T09:04:34Z

sha=3be1564f71c950f970c5c17ac9b841e0bfdf39d9

leeebo · 2024-11-28T10:04:59Z

Hi @pedrominatel , the github action Sync approved PRs to internal codebase failed with RuntimeError: PR modifying forbidden files!!!, the internal MR not be created.

Please manually create it.

add internal Gitlab repo to git remote
push to Gitlab then create MR
add close https://github.com/espressif/esp-iot-solution/pull/419 in the commit message to link this PR

pedrominatel · 2024-11-28T14:09:20Z

Hi @pedrominatel , the github action Sync approved PRs to internal codebase failed with RuntimeError: PR modifying forbidden files!!!, the internal MR not be created.

Please manually create it.
1. add internal Gitlab repo to git remote

2. push to Gitlab then create MR

3. add `close https://github.com/espressif/esp-iot-solution/pull/419` in the commit message to link this PR

Done.

fix: Added copyright notice on the servo_control.c file fix: Added servo example to the patterns in the rules.yml fix: Typo fixed in the servo example pattern in the rules.yml fix: Typo fixed in the .rules:build:components_motor_servo_test_apps fix: Changes in the test script to reduce the test scope for multiple targets

YanKE01 reviewed Nov 6, 2024

View reviewed changes

pedrominatel force-pushed the servo-motor-as-component-registry branch from 23af8b7 to e03d757 Compare November 26, 2024 16:41

YanKE01 reviewed Nov 27, 2024

View reviewed changes

leeebo added the PR-Sync-Merge label Nov 28, 2024

leeebo self-requested a review November 28, 2024 06:41

leeebo approved these changes Nov 28, 2024

View reviewed changes

components/motor/servo/examples/servo_control/main/servo_control.c Outdated Show resolved Hide resolved

components/motor/servo/examples/servo_control/main/servo_control.c Outdated Show resolved Hide resolved

leeebo added PR-Sync-Merge and removed PR-Sync-Merge labels Nov 28, 2024

pedrominatel force-pushed the servo-motor-as-component-registry branch from 962b2d8 to 3be1564 Compare November 28, 2024 08:52

leeebo added PR-Sync-Merge and removed PR-Sync-Merge labels Nov 28, 2024

pedrominatel force-pushed the servo-motor-as-component-registry branch from 420cc24 to aa94cc6 Compare November 29, 2024 17:25

pedrominatel added 2 commits December 2, 2024 10:45

fix: changed the test to only 4 channels and LEDC_LOW_SPEED_MODE

8242311

fix: added the ESP-IDF v4.4 to the build CI

f00128b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added servo component to the registry #419

Added servo component to the registry #419

pedrominatel commented Oct 31, 2024 •

edited

Loading

CLAassistant commented Oct 31, 2024 •

edited

Loading

github-actions bot commented Oct 31, 2024 •

edited

Loading

leeebo commented Nov 1, 2024

YanKE01 left a comment

pedrominatel commented Nov 26, 2024

YanKE01 left a comment

YanKE01 Nov 27, 2024

YanKE01 Nov 27, 2024

pedrominatel Nov 27, 2024

YanKE01 commented Nov 27, 2024

leeebo commented Nov 28, 2024

leeebo commented Nov 28, 2024

leeebo commented Nov 28, 2024

pedrominatel commented Nov 28, 2024

leeebo commented Nov 28, 2024

leeebo commented Nov 28, 2024

pedrominatel commented Nov 28, 2024

Added servo component to the registry #419

Are you sure you want to change the base?

Added servo component to the registry #419

Conversation

pedrominatel commented Oct 31, 2024 • edited Loading

Description

Related

Testing

Checklist

CLAassistant commented Oct 31, 2024 • edited Loading

github-actions bot commented Oct 31, 2024 • edited Loading

leeebo commented Nov 1, 2024

YanKE01 left a comment

Choose a reason for hiding this comment

pedrominatel commented Nov 26, 2024

YanKE01 left a comment

Choose a reason for hiding this comment

YanKE01 Nov 27, 2024

Choose a reason for hiding this comment

YanKE01 Nov 27, 2024

Choose a reason for hiding this comment

pedrominatel Nov 27, 2024

Choose a reason for hiding this comment

YanKE01 commented Nov 27, 2024

leeebo commented Nov 28, 2024

leeebo commented Nov 28, 2024

leeebo commented Nov 28, 2024

pedrominatel commented Nov 28, 2024

leeebo commented Nov 28, 2024

leeebo commented Nov 28, 2024

pedrominatel commented Nov 28, 2024

pedrominatel commented Oct 31, 2024 •

edited

Loading

CLAassistant commented Oct 31, 2024 •

edited

Loading

github-actions bot commented Oct 31, 2024 •

edited

Loading