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

HASS bridge emits incorrectly formatted time #1904

Closed
deviantintegral opened this issue Dec 12, 2021 · 7 comments
Closed

HASS bridge emits incorrectly formatted time #1904

deviantintegral opened this issue Dec 12, 2021 · 7 comments
Labels
bug There is a defect in the code

Comments

@deviantintegral
Copy link
Contributor

The latest Home Assistant 2021.12 release has made time parsing stricter, and now the bridge is throwing errors for time fields:

2021-12-11 23:49:43 ERROR (MainThread) [homeassistant.components.sensor] Error adding entities for domain sensor with platform mqtt
Traceback (most recent call last):
File "/usr/src/homeassistant/homeassistant/helpers/entity_platform.py", line 382, in async_add_entities
await asyncio.gather(*tasks)
File "/usr/src/homeassistant/homeassistant/helpers/entity_platform.py", line 613, in _async_add_entity
await entity.add_to_platform_finish()
File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 748, in add_to_platform_finish
self.async_write_ha_state()
File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 505, in async_write_ha_state
self._async_write_ha_state()
File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 538, in _async_write_ha_state
state = self._stringify_state()
File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 511, in _stringify_state
if (state := self.state) is None:
File "/usr/src/homeassistant/homeassistant/components/sensor/__init__.py", line 411, in state
raise ValueError(
ValueError: Invalid datetime: sensor.auriol_hg02832_1_89_utc provides state '2021-12-11 23:49:23', which is missing timezone information

pbkhrv/rtl_433-hass-addons#35

Since the timestamp is generated from the receiver, and not generally as a part of the sensor, I think we should include the timezone based on the system's location.

It would be great to get any fixes in before the next release, but I won't likely have time in the next day or two to work on it myself.

@deviantintegral deviantintegral changed the title HASS bridge emits incorrectly formatted timez HASS bridge emits incorrectly formatted time Dec 12, 2021
@zuckschwerdt
Copy link
Collaborator

We already have an option for that: -M time:tz, also e.g. -M time:tz:utc, -M time:tz:local, -M time:usec:tz.
Does that work?

@deviantintegral
Copy link
Contributor Author

deviantintegral commented Dec 12, 2021

report_meta time:iso:tz:local in the config files looks to fix this. Thanks!

The only thing I wonder is, do we not want the bridge script to work "as out of the box" as possible? As is, it seems like remembering to set -M is prone to error and likely to cause future support requests. If we would rather the timezone formatting be handled via rtl_433 configs, then I wonder if the bridge should log a warning when it detects a timestampe we know HA won't like.

@zuckschwerdt
Copy link
Collaborator

Ah yes, iso adds the T (instead of the space).
A warning from the script would be good. A fixed formatting of usec:iso:tz:utc for json could also be an idea. People really don't like iso in the terminal output and we really don't known what e.g. CSV needs, but json (or rather JS) recommends ISO 8601 (with fractional seconds and TZ) pretty much.
The big change would be to carry the time value independent from the formatting to each output (currently it's a already formatted string).

@gdt gdt added the bug There is a defect in the code label Jul 3, 2022
@gdt gdt added the feedback request for more information; may be closed id 30d if not received label Sep 30, 2023
@gdt
Copy link
Collaborator

gdt commented Sep 30, 2023

What's the status?

It seems obvious to me that rtl_433 with just -F json piped to the scripts should just work, and not need args to ask for it -- which I view as a workaround. I am not sure we are there yet.

@deviantintegral
Copy link
Contributor Author

I agree that setting this when using JSON output makes the most sense.

@gdt
Copy link
Collaborator

gdt commented Oct 28, 2023

So it seems we have consensus that the code should be enhanced as described above, to be always 8601 in json, and we are waiting a PR.

@gdt gdt removed the feedback request for more information; may be closed id 30d if not received label Oct 28, 2023
@gdt
Copy link
Collaborator

gdt commented Jun 5, 2024

However, we are not seeing reports of problems with up-to-date code. So I'm going to assume this is fixed. Please comment if that's wrong and we can re-open.

@gdt gdt closed this as completed Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug There is a defect in the code
Projects
None yet
Development

No branches or pull requests

3 participants