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

Add ccronexpr components for parse CRON expression (IEC-88) #304

Merged
merged 1 commit into from
May 21, 2024

Conversation

ESP-YJM
Copy link
Contributor

@ESP-YJM ESP-YJM commented Feb 6, 2024

Checklist

  • Component contains License
  • Component contains README.md
  • Component contains idf_component.yml file with url field defined
  • Component was added to upload job
  • Component was added to build job
  • Optional: Component contains unit tests
  • CI passing

Change description

Please describe your change here

@CLAassistant
Copy link

CLAassistant commented Feb 6, 2024

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot changed the title Add ccronexpr components for parse CRON expression Add ccronexpr components for parse CRON expression (IEC-88) Feb 6, 2024
@ESP-YJM ESP-YJM force-pushed the feature/add_ccronexpr_component branch from 3ab21c6 to 3233531 Compare February 6, 2024 11:46
Copy link
Member

@igrr igrr left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @ESP-YJM, I left a few comments.

Please also add at least a simple example project illustrating how to use this library.

ccronexpr/CMakeLists.txt Outdated Show resolved Hide resolved
ccronexpr/Kconfig Outdated Show resolved Hide resolved
ccronexpr/Kconfig Outdated Show resolved Hide resolved
ccronexpr/Kconfig Outdated Show resolved Hide resolved
ccronexpr/Kconfig Outdated Show resolved Hide resolved
ccronexpr/Kconfig Outdated Show resolved Hide resolved
ccronexpr/LICENSE.txt Outdated Show resolved Hide resolved
ccronexpr/idf_component.yml Outdated Show resolved Hide resolved
.github/workflows/upload_component.yml Outdated Show resolved Hide resolved
supertinycron/README.md Outdated Show resolved Hide resolved
@ESP-YJM ESP-YJM force-pushed the feature/add_ccronexpr_component branch from 5f59334 to 8165a09 Compare April 10, 2024 11:34
@ESP-YJM
Copy link
Contributor Author

ESP-YJM commented Apr 10, 2024

@igrr Sorry for taking so long to update. If you have time, please help to review it again.


## API Reference

To learn more about how to use this component, please check API Documentation from header file [ccronexpr.h](https://github.com/espressif/idf-extra-components/blob/master/supertinycron/supertinycron/ccronexpr.h)
Copy link
Member

Choose a reason for hiding this comment

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

This link gives a 404 error (i think because it points inside a submodule, which github doesn't support.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It points to master branch, maybe after merge the PR and can access it.

@igrr
Copy link
Member

igrr commented Apr 19, 2024

@ESP-YJM LGTM aside the few things mentioned.

@ESP-YJM ESP-YJM force-pushed the feature/add_ccronexpr_component branch from f858b9d to a04ca02 Compare April 22, 2024 02:42
@ESP-YJM ESP-YJM force-pushed the feature/add_ccronexpr_component branch from a04ca02 to ff41564 Compare May 15, 2024 01:57
@igrr
Copy link
Member

igrr commented May 15, 2024

@ESP-YJM Thanks for the fixes, just one more thing seems to be remaining:

/__w/idf-extra-components/idf-extra-components/supertinycron/examples/cron_example/main/cron_example_main.c
/__w/idf-extra-components/idf-extra-components/supertinycron/examples/cron_example/main/cron_example_main.c:234:6: error: function declaration isn't a prototype [-Werror=strict-prototypes]
  234 | void check_calc_invalid()
      |      ^~~~~~~~~~~~~~~~~~
/__w/idf-extra-components/idf-extra-components/supertinycron/examples/cron_example/main/cron_example_main.c:282:6: error: function declaration isn't a prototype [-Werror=strict-prototypes]
  282 | void test_expr()
      |      ^~~~~~~~~
/__w/idf-extra-components/idf-extra-components/supertinycron/examples/cron_example/main/cron_example_main.c:634:6: error: function declaration isn't a prototype [-Werror=strict-prototypes]
  634 | void test_parse()
      |      ^~~~~~~~~~
/__w/idf-extra-components/idf-extra-components/supertinycron/examples/cron_example/main/cron_example_main.c:861:6: error: function declaration isn't a prototype [-Werror=strict-prototypes]
  861 | void test_bits()
      |      ^~~~~~~~~
/__w/idf-extra-components/idf-extra-components/supertinycron/examples/cron_example/main/cron_example_main.c:896:6: error: function declaration isn't a prototype [-Werror=strict-prototypes]
  896 | void app_main()
      |      ^~~~~~~~

Please use (void) in function declarations inside the example to avoid strict-prototypes errors.

@ESP-YJM ESP-YJM force-pushed the feature/add_ccronexpr_component branch from ff41564 to 35a448b Compare May 15, 2024 08:29
@igrr igrr merged commit 8e1c091 into espressif:master May 21, 2024
69 of 70 checks passed
@igrr
Copy link
Member

igrr commented May 21, 2024

@ESP-YJM Thank you for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants