8000 Add new time.has_time condition by ashmckenzie · Pull Request #1255 · esphome/esphome · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add new time.has_time condition #1255

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 4, 2020
Merged

Add new time.has_time condition #1255

merged 1 commit into from
Oct 4, 2020

Conversation

ashmckenzie
Copy link
Contributor
@ashmckenzie ashmckenzie commented Aug 18, 2020 •
edited by glmnet
Loading

Description:

This PR adds the time.has_time condition which can be used as part of a wait_until action, e.g.

script:
  - id: on_boot
    then:
      - logger.log:
          level: INFO
          format: "Starting on_boot.."
      - script.execute: prevent_sleep
      - logger.log:
          level: INFO
          format: "Waiting for WIFI, MQTT and NTP.."
      - wait_until:
          - time.has_time:
          - mqtt.connected:
      - logger.log:
          level: INFO
          format: "WIFI, MQTT and NTP up!"
      - script.execute: mqtt_ping
      - script.execute: do_work

Related issue (if applicable): fixes N/A

Pull request in esphome-docs with documentation (if applicable): esphome/esphome-docs#782

Checklist:

  • The code change is tested and works locally.
  • Tests have been added to verify that the new code works (under tests/ folder).

If user exposed functionality or configuration variables are added/changed:

< 8000 /include-fragment>
@OttoWinter
Copy link
Member

Please change this to time.has_time (because this is not just for sntp, but all time platforms). So move the condition in C++ and Python code to the time module and rename it.

@ashmckenzie
Copy link
Contributor Author

Please change this to time.has_time (because this is not just for sntp, but all time platforms). So move the condition in C++ and Python code to the time module and rename it.

OK, thanks @OttoWinter 👍

@ashmckenzie ashmckenzie changed the title WIP: Add new sntp.synchronized condition WIP: Add new time.has_time condition Sep 16, 2020
@ashmckenzie ashmckenzie changed the title WIP: Add new time.has_time condition Add new time.has_time condition Sep 16, 2020
@ashmckenzie
Copy link
Contributor Author

@OttoWinter, as requested I've updated this PR to now be time.has_time. May you please take a look when convenient? 🙏

OttoWinter
OttoWinter requested changes Sep 16, 2020
View reviewed changes
Copy link
Member
@OttoWinter OttoWinter left a comment

Choose a reason for hiding this comment

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

Ah sorry I think I wasn't clear enough.

There's already a method to check if time is valid/has been set in the ESPTime struct: is_valid() (before a time has been set, the ESP will default to 1 jan 1970).

I've added a comment on what I think would be best. This way, the time components don't need to know that the has_time_ variable has been set.

@ashmckenzie
Copy link
Contributor Author

@OttoWinter, thanks for the feedback, I believe this is ready for another look 🙂 🙏

@glmnet
Copy link
Member
glmnet commented Sep 27, 2020

GTM when docs added

@ashmckenzie ashmckenzie mentioned this pull request Sep 28, 2020
Merged
2 tasks
@ashmckenzie
Copy link
Contributor Author

GTM when docs added

Thanks @glmnet, I've created esphome/esphome-docs#782 🙂

@OttoWinter OttoWinter merged commit 5c86f33 into esphome:dev Oct 4, 2020
sashao pushed a commit to sashao/esphome that referenced this pull request Oct 23, 2020
@ashmckenzie @sashao
Add new time.has_time condition (esphome#1255)
9a61dba
This was referenced Jan 7, 2021
Merged
Merged
Merged
Merged
This was referenced Feb 3, 2021< 8000 /relative-time>
Merged
Merged
@ashmckenzie ashmckenzie deleted the ashmckenzie/add-sntp-synchronized-condition branch February 4, 2021 01:20
@github-actions github-actions bot locked and limited conversation to collaborators Sep 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@glmnet glmnet glmnet approved these changes

@OttoWinter OttoWinter OttoWinter approved these changes

@motwok motwok motwok approved these changes

Assignees

@OttoWinter OttoWinter

Labels
has-tests integration: time small-pr
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ashmckenzie @OttoWinter @glmnet @motwok
0