Skip to content

Standardize times in time sensors Jewish calendar#26940

Merged
MartinHjelmare merged 10 commits into
home-assistant:devfrom
tsvi:std_time_jcal
Oct 10, 2019
Merged

Standardize times in time sensors Jewish calendar#26940
MartinHjelmare merged 10 commits into
home-assistant:devfrom
tsvi:std_time_jcal

Conversation

@tsvi
Copy link
Copy Markdown
Contributor

@tsvi tsvi commented Sep 26, 2019

Breaking Change:

The output of the timestamp sensors have been streamlined so they're easier to use in automations. All the timestamp sensors will return UTC time in ISO 8601 format.
Attributes have been added to get a UNIX timestamp.

Please note if you have any automations relying on the timestamp, you'll need to update them to compare to the UTC timestamp.

Description:

This standardizes the times in the timestamp sensors for the Jewish calendar, times like candle lighting, havdalah etc. are all defined as timestamp sensors. As such, their state will show ISO8601 UTC timestamps.
Also, timestamp attributes have been added to the sensor. This should allow easier automation triggers and conditions when comparing timestamps.

Related issue (if applicable): fixes #

Pull request with documentation for home-assistant.io (if applicable): home-assistant/home-assistant.io#10463

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • Documentation added/updated in home-assistant.io
  • Tests have been added to verify that the new code works.

Comment thread homeassistant/components/jewish_calendar/sensor.py Outdated
@mikeage
Copy link
Copy Markdown
Contributor

mikeage commented Oct 3, 2019

Nice idea. We make "early shabbat" during DST (well, mostly; we stopped a few weeks ago), so I created my own issur_melacha sensor based on plag hamincha on Friday, and the conversion to compare the current time with plag as a timestamp was a bit of an exercise in Jinja and figuring out what's what. This will make that sort of thing much easier :-)

Comment thread homeassistant/components/jewish_calendar/sensor.py Outdated
@tsvi
Copy link
Copy Markdown
Contributor Author

tsvi commented Oct 7, 2019

I'm closing this pull request for now. I'm trying to envision what I would like ultimately to be possible and I think it will need some other PR's first.

Ultimately, I would like to reach a point where I could write the following automation:

automation:
  trigger:
    platform: time
    at: "{{ states('sensor.timestamp_based_sensor') }}"
    offset:
      minutes: -15

@balloob would you consider such a PR? Or am I in the wrong direction, I suppose this would mean opening up an architecture issue?

@tsvi tsvi closed this Oct 7, 2019
@tsvi tsvi reopened this Oct 7, 2019
@balloob
Copy link
Copy Markdown
Member

balloob commented Oct 8, 2019

Yeah I would consider such a PR . It might be better to just have time trigger be aware of timestamp device classes sensors. That way the UI could offer a dropdown.

tsvi added 9 commits October 10, 2019 10:31
Timestamp device class requires ISO 8601 format
As this is part of the UI decision, it should be decided by lovelace.

A nice addition for a future PR, might be the option to hint to lovelace the preferred way to display some data.
Although I don't understand it, I give up :)
Comment thread homeassistant/components/jewish_calendar/sensor.py Outdated
I don't really see the value in these attributes, if there are any they should be implemented in
the sensor component for the timestamp device class
@MartinHjelmare
Copy link
Copy Markdown
Member

Thanks!

@MartinHjelmare MartinHjelmare merged commit 9e30051 into home-assistant:dev Oct 10, 2019
@tsvi tsvi deleted the std_time_jcal branch October 11, 2019 08:27
@tsvi tsvi restored the std_time_jcal branch October 11, 2019 15:02
@lock lock Bot locked and limited conversation to collaborators Oct 12, 2019
@tsvi tsvi deleted the std_time_jcal branch May 15, 2024 19:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants