Skip to content

Conversation

@DerDreschner
Copy link
Contributor

Currently, there is no support for daylight saving time in zigbee-herdsman. Some devices ask the coordinator repeatedly this information if there is no answer or just the default value as answer (looking at you, Bosch!). To avoid that, there was the need for an customReadResponse function, which isn't that great.

This pull request changes that and includes support for daylight saving time. As the whole topic can be pretty complex, I've settled on the basics: daylight saving time is well-established in the timezone. This is important, as countries can leave or introduce daylight saving time (happened a lot over the past 10-20 years). In that case, I leave the default values, which should match the current behavior.

This is a follow-up to #1515.

@Nerivec
Copy link
Collaborator

Nerivec commented Oct 1, 2025

@DerDreschner DerDreschner requested a review from Nerivec October 2, 2025 06:44
@DerDreschner
Copy link
Contributor Author

DerDreschner commented Oct 5, 2025

I've looked into the converters to see how other devices handle genTime requests apart from what is being done in zigbee-herdsman. Here are a few findings:

It might be sensible to expose the timeService to the converters for this cases, as a lot of them just copied the previous code we now replace. Maybe we should implement a utils.time file there or some methods in modernExtend.ts to handle such non-specification-compliant cases (or at least a subset of commonly used cases) in a central place.

@Koenkk
Copy link
Owner

Koenkk commented Oct 5, 2025

Multiple devices try to read the time from the coordinator, but the read request is being ignored for some reason.

It's an ignore converter to suppress the "No converter available.." message, but since it's not logged anyway for these clusters we can remove this. This converter does not prevent zigbee-herdsman from responding, it was purely there to suppress the log message (before ignoreClusters was introduced). I'll cleanup the converters.

@Nerivec
Copy link
Collaborator

Nerivec commented Oct 6, 2025

I'll cleanup the converters.

Mighty nice cleanup!

@DerDreschner
Copy link
Contributor Author

@Nerivec: Are there any open tasks for me to do that block a possible merge?

@Nerivec Nerivec requested review from Koenkk and Copilot October 15, 2025 12:25
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds comprehensive support for daylight saving time (DST) in the genTime cluster, replacing the previous basic time zone handling with a sophisticated system that can handle DST transitions for both Northern and Southern hemispheres. The implementation caches DST calculations for 24-hour periods to improve performance while ensuring accurate time information is provided to Zigbee devices.

Key changes:

  • New timeService module that calculates all genTime cluster attributes including DST start/end times and offsets
  • Updated controller to use the time service instead of inline time calculations
  • Comprehensive test suite covering various timezone scenarios and DST edge cases

Reviewed Changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/utils/timeService.ts New module implementing DST-aware time cluster attribute calculation with caching
src/controller/model/device.ts Refactored to use timeService for genTime read responses instead of inline calculations
src/index.ts Exports the new getTimeClusterAttributes function from timeService
test/timeService.test.ts Comprehensive test suite for the new time service functionality
test/controller.test.ts Updated genTime read test to verify integration with timeService
package.json Added @date-fns/tz dependency for timezone calculations
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

@Koenkk Koenkk merged commit 4aa797c into Koenkk:master Oct 15, 2025
1 check passed
@Koenkk
Copy link
Owner

Koenkk commented Oct 15, 2025

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants