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

Use explicit duration format for state formatting #23017

Merged
merged 1 commit into from
Nov 27, 2024
Merged

Conversation

piitaya
Copy link
Member

@piitaya piitaya commented Nov 27, 2024

Proposed change

Improve duration format for state

Instead of using hh:mm:ss it will now use explicit format based on unit of measurement and precision.
The rule of formatting is this one : We use the unit of measurement as main unit and we display the digits as secondary unit.

Examples :

0 h : 0h
0.5 h: 0h 30min
30 min: 30min
30.75 min: 30min 45s

fixes #21967
fixes #21984
fixes #18151

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (thank you!)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example configuration

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue or discussion:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works.

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

@github-actions github-actions bot added the Build Related to building the code label Nov 27, 2024
@silamon
Copy link
Contributor

silamon commented Nov 27, 2024

That's a pretty good improvement. Over the years we used different formatting of duration (and we do have different classes) that all do the same. Aligning the duration (or for fields) in the automation picker is a thing on my wishlist.


I actually even introduced a similar (if not same) function for the calendar trigger this month, https://github.com/home-assistant/frontend/blob/dev/src/common/datetime/format_duration.ts#L47, which could probably be removed as well with this, but I may open a PR myself after this one is merged.

@piitaya
Copy link
Member Author

piitaya commented Nov 27, 2024

@silamon Yeah I focused the PR for state formatting but we should improve the other duration formats (It's now using a different function with the same name 🙈). I'll review your PR 👍

@silamon
Copy link
Contributor

silamon commented Nov 27, 2024

The durationFormat class even has a "digital" format that we could use:
"long"
E.g., 1 hour and 50 minutes

"short" (default)
E.g., 1 hr, 50 min

"narrow"
E.g., 1h 50m

"digital"
E.g., 1:50:00

And the "long" formatting is the one I introduced in the calendar trigger.

Comment on lines +67 to +68
days,
hours,
Copy link
Member

Choose a reason for hiding this comment

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

For the future I think we should use the precision to determine if we are also gonna add minutes and seconds, for now it is ok I think.

@bramkragten bramkragten merged commit a532b44 into dev Nov 27, 2024
16 checks passed
@bramkragten bramkragten deleted the duration_format branch November 27, 2024 11:49
@piitaya piitaya mentioned this pull request Nov 27, 2024
9 tasks
@WinterPhoenix
Copy link

So, how do we set the precision on a duration class sensor from YAML?

@dwildstr
Copy link

This change (together with #23132) appears to have broken sensible display of uptime sensors from ESPHome devices or any others that provide sensors of the duration device class measured in seconds. Being able to get uptime (or other durations) in a unit appropriate for its size is useful functionality: reading that something has occurred for "8,324 seconds" is simply not as user-friendly as seeing it as "2 hours, 18 minutes, and 44 seconds". The examples given in the description of the PR don't exhibit this counterintuitive behavior: they only exhibit numbers of hours less than 24 and minutes less than 60, but it's my understanding that a duration given as, say, "94.75 min" would be turned into "90m45 s", and not into "1h30m45s". I understand that this is the intended behavior of this PR, but not being able to override it and get back a behavior which uses units up to the largest one actually present in quantities greater than 1, instead of treating the given unit of measurement as the largest to be displayed, seems like a step backwards for readability of reported durations.

@silamon
Copy link
Contributor

silamon commented Dec 14, 2024

If something is unclear or something is incorrect, please open a new issue.

@home-assistant home-assistant locked and limited conversation to collaborators Dec 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Build Related to building the code cla-signed
Projects
None yet
5 participants