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

Temperature time sensors are not timestamp data type #275

Closed
7 tasks done
jazzyisj opened this issue Aug 5, 2024 · 14 comments
Closed
7 tasks done

Temperature time sensors are not timestamp data type #275

jazzyisj opened this issue Aug 5, 2024 · 14 comments
Labels
bug Something isn't working

Comments

@jazzyisj
Copy link

jazzyisj commented Aug 5, 2024

Describe the bug

The recently added time sensors for high and low temperatures are not timestamp data type.

Expected behavior

A properly instantiated datetime entity can be used to display the date or time in a human friendly format in the UI like this using the entity format parameter.

  - entity: sensor.outdoor_high_temperature_time
    name: "High Temperature Time"
    format: time
image

Actual behavior

The Pirateweather time state sensors are not proper timestamps and do not work with the entity format parameter in the UI.

      - entity: sensor.pirateweather_daytime_high_apparent_temperature_time_0d
        name: "High Temperature Time" 
        format: time
image

Home Assistant version

2024.8.0b2

Integration version

1.5.3

Other details

https://developers.home-assistant.io/docs/core/entity/datetime/

Troubleshooting steps

  • I have updated my Home Assistant installation to the latest version.
  • I have updated the Pirate Weather Integration to the latest version.
  • I have gone through the documentation before opening this issue.
  • I have searched this repository and API Repository to see if the issue has already been reported.
  • I have restarted my Home Assistant installation.
  • I have queried the API in my browser to confirm the issue is not with the API.
  • I have written an informative title.
@jazzyisj jazzyisj added bug Something isn't working Needs Review labels Aug 5, 2024
@jazzyisj jazzyisj changed the title Temperature time sensors are not datetime data type Temperature time sensors are not timestamp data type Aug 5, 2024
@cloneofghosts
Copy link
Collaborator

Thanks for opening this issue. What's happening here is those sensors in the API are in epoch time and not a timestamp. You'll need to convert them to a timestamp.

I'm not sure if its possible to convert them into a timestamp in the integration itself so I'll ping @alexander0042 to see if its possible

@alexander0042
Copy link
Collaborator

Good catch here, and sorry for the slow reply- I was working on fixing some stability issues with the primary API, so that was the focus. The good news here is that this was an easy change, and will be out in 1.5.5 today!

alexander0042 added a commit that referenced this issue Aug 15, 2024
- Corrects a small bug about platforms re: Issue #273
- Changes sensor reported times to datetimes re: Issue #275
- Allows for updating locations via hass re: Issue #241
- Allows changing the scan_interval re: Issue #230
@cloneofghosts
Copy link
Collaborator

V1.5.5 has just been released which contains this fix.

@jazzyisj
Copy link
Author

jazzyisj commented Aug 18, 2024

Hate to be a pain, but the device_class of the pirate weather time sensors is still not a timestamp.

This is a properly formatted timestamp sensor from the Environment Canada integration.

image

This is a newly formatted Pirateweather time sensor.

image

The daily low temperature and forecast time sensors were completely missed.

image image

@alexander0042
Copy link
Collaborator

Thanks for mentioning the Environment Canada integration! I'll take a look at it to see what they do and go from there!

alexander0042 added a commit that referenced this issue Aug 20, 2024
Also corrects the datetime low temperature time to read the correct variable (low instead of min). The entity name is kept the same for compatibility
@cloneofghosts
Copy link
Collaborator

@alexander0042 Was testing the update in a devcontainer before creating a new release and am running into this error with the daily low sensor:

Logger: homeassistant.components.sensor
Source: helpers/entity_platform.py:598
integration: Sensor (documentation, issues)
First occurred: 10:28:22 AM (3 occurrences)
Last logged: 10:32:12 AM

Error adding entity sensor.pirateweather_daily_low_temperature_time_0d for domain sensor with platform pirateweather
Traceback (most recent call last):
  File "/home/vscode/.local/lib/python3.12/site-packages/homeassistant/components/sensor/__init__.py", line 594, in state
    if value.tzinfo is None:
       ^^^^^^^^^^^^
AttributeError: 'int' object has no attribute 'tzinfo'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/vscode/.local/lib/python3.12/site-packages/homeassistant/helpers/entity_platform.py", line 598, in _async_add_entities
    await coro
  File "/home/vscode/.local/lib/python3.12/site-packages/homeassistant/helpers/entity_platform.py", line 912, in _async_add_entity
    await entity.add_to_platform_finish()
  File "/home/vscode/.local/lib/python3.12/site-packages/homeassistant/helpers/entity.py", line 1366, in add_to_platform_finish
    self.async_write_ha_state()
  File "/home/vscode/.local/lib/python3.12/site-packages/homeassistant/helpers/entity.py", line 1005, in async_write_ha_state
    self._async_write_ha_state()
  File "/home/vscode/.local/lib/python3.12/site-packages/homeassistant/helpers/entity.py", line 1130, in _async_write_ha_state
    self.__async_calculate_state()
  File "/home/vscode/.local/lib/python3.12/site-packages/homeassistant/helpers/entity.py", line 1067, in __async_calculate_state
    state = self._stringify_state(available)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vscode/.local/lib/python3.12/site-packages/homeassistant/helpers/entity.py", line 1011, in _stringify_state
    if (state := self.state) is None:
                 ^^^^^^^^^^
  File "/home/vscode/.local/lib/python3.12/site-packages/homeassistant/components/sensor/__init__.py", line 605, in state
    raise ValueError(
ValueError: Invalid datetime: sensor.pirateweather_daily_low_temperature_time_0d has timestamp device class but provides state 1724230800:<class 'int'> resulting in ''int' object has no attribute 'tzinfo''

@alexander0042
Copy link
Collaborator

Thanks for testing this! I missed that variable since it has a different name than the others, but just pushed a commit that should fix it

@cloneofghosts
Copy link
Collaborator

Yup, that fixed the issue. One thing I noticed during testing was the Overnight Low Time sensor was showing the data from temperatureMinTime instead of temperatureLowTime so it shows the low as being 6h ago instead of being in 18h.

@cloneofghosts
Copy link
Collaborator

Other than the issue I pointed out above (maybe it's working as intended?) are we good to release a new version? Been holding off releasing a new version to make sure its stable. I've tested locally and everything works so I think we're good to release the changes?

@alexander0042
Copy link
Collaborator

Yea, I was a little confused by the high/low min/max for that one as well. I initially changed it to match to description in the text, so it's pulling the high/low (Day/Night) times for both apparent and temperature. However, in the entity description it's coded as min, and I think to keep things consistent it's better not to change it, so I just pushed a change to set it back to min

@cloneofghosts
Copy link
Collaborator

Tested the changes again and everything is looking good. Is this good to release or do you want to look at #241 first?

@cloneofghosts cloneofghosts moved this from Todo to In Progress in Pirate Weather Home Assistant Aug 22, 2024
@alexander0042
Copy link
Collaborator

Let's leave that one open until we hear back if those sensors would be useful to have and push a new release in the meantime!

@cloneofghosts
Copy link
Collaborator

Alright will leave that one open. @jazzyisj I've released v1.5.8 which fixes the issues with the datetime sesnors.

@jazzyisj
Copy link
Author

You guys are fast! Installed and tested - thanks! I'll close the issue.

@github-project-automation github-project-automation bot moved this from In Progress to Done in Pirate Weather Home Assistant Aug 23, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Oct 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
Development

No branches or pull requests

3 participants