feat: Improve POSIX time zone string generation#4087
Merged
Conversation
This commit refactors the `toPosixString()` extension function for `ZoneId` to more accurately generate POSIX-compliant time zone strings, especially for time zones with daylight saving time transitions. Key changes include: - Rewriting the logic to use `zone.rules.transitionRules` for determining standard and daylight saving time transitions, instead of relying on `nextTransition`. - Improving the calculation of transition rule strings (start/end of DST). - Correcting the POSIX offset formatting. - Updating and correcting test cases for various time zones to reflect the improved accuracy. Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the toPosixString() extension function for ZoneId to improve accuracy and correctness when generating POSIX-compliant time zone strings, particularly for zones with daylight saving time transitions.
Key changes:
- Replaced the previous
nextTransition-based approach withtransitionRules-based logic for more reliable DST detection - Improved transition rule calculation to correctly handle occurrence weeks and wall time conversions
- Enhanced offset formatting with proper handling of negative fractional hour offsets
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| core/ui/src/main/kotlin/org/meshtastic/core/ui/timezone/ZoneIdExtensions.kt | Complete rewrite of toPosixString() logic using transitionRules instead of nextTransition; added new helper functions for formatting abbreviations, offsets, and transition rules; improved wall time calculation for different time definitions |
| core/ui/src/test/kotlin/org/meshtastic/core/ui/timezone/ZoneIdExtensionsTest.kt | Updated expected POSIX strings to reflect corrected calculations: week 4 to week 5 for several European zones, GMT abbreviation for Sao Paulo, and corrected occurrence for Africa/Cairo and Pacific/Auckland |
This commit introduces several enhancements to the POSIX timezone string generation: - Updates the POSIX string for `America/Sao_Paulo` from the generic "GMT3" to the more specific "BRT3". - Increases visibility of internal helper functions to `internal` for better testability. - Adds comprehensive unit tests for `formatAbbreviation` and `formatPosixOffset` to ensure correctness of timezone string components. Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
Refine the `toPosixString` extension function for `ZoneId` by addressing an issue where `firstOrNull` could select an incorrect, historical transition rule. By changing `firstOrNull` to `lastOrNull`, the function now correctly selects the most recent "spring" and "fall" daylight saving transition rules. This ensures a more accurate POSIX timezone string generation for zones with multiple historical rule changes. Additionally, the logic is simplified to check for `isEmpty()` on `transitionRules` instead of `< 2`. Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4087 +/- ##
=====================================
Coverage 0.00% 0.00%
=====================================
Files 2 2
Lines 19 19
Branches 7 7
=====================================
Misses 19 19 ☔ View full report in Codecov by Sentry. |
Replaces `Year.now()` with a constant `REFERENCE_YEAR` (2025) when creating a `ZoneOffsetTransitionRule`. This ensures deterministic generation of the POSIX timezone string, independent of the current date. Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
The explicit `java.time.LocalTime` type declaration for `wallTime` is removed, as it can be inferred by the compiler. Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This refactors the
toPosixString()extension function forZoneIdto more accurately generate POSIX-compliant time zone strings, especially for time zones with daylight saving time transitions.Key changes include:
zone.rules.transitionRulesfor determining standard and daylight saving time transitions, instead of relying onnextTransition.uses detailed examples from and resolves #3705