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

Daylight Saving Time support #1058

Merged
merged 2 commits into from Jul 20, 2016

Conversation

@avtolstoy
Copy link
Member

commented Jul 10, 2016

Implements initial Daylight Saving Time (DST) API proposed in #211.

Added Wiring functions:

  /* Retrieve the current DST offset that is added to the current local time when
   * Time.beginDST() has been called.
   * The default is 1 hour.
   */
  float TimeClass::getDSTOffset();
  /* Set a custom DST offset */
  void TimeClass::setDSTOffset(float offset);
  /* Add the offset from getDSTOffset() to the current time */
  void TimeClass::beginDST();
  /* Do not add the offset from getDSTOffset() to the current time */
  void TimeClass::endDST();
  /* Returns true if DST is in effect (beginDST() was called previously) */
  uint8_t TimeClass::isDST();

Doneness:

  • Contributor has signed CLA
  • Problem and Solution clearly stated
  • Code peer reviewed
  • API tests compiled
  • Run unit/integration/application tests on device
  • Add documentation
  • Add to CHANGELOG.md after merging (add links to docs and issues)

FEATURE

  • added Daylight Saving Time support #1058 per proposed #211

@avtolstoy avtolstoy added this to the 0.6.x milestone Jul 10, 2016

@technobly technobly force-pushed the feature/time-dst branch from b176bca to bbefda2 Jul 20, 2016

@technobly technobly merged commit ddd190d into develop Jul 20, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@ScruffR

This comment has been minimized.

Copy link
Contributor

commented Sep 2, 2016

I'm not sure how this will actually address the issue of switching DST on and off 😕

In #211 there was talk about a way to set the switching pattern

TimeChangeRule usEDT = {"EDT", Second, Sun, Mar, 2, -240}; //UTC - 4 hours
TimeChangeRule usEST = {"EST", First, Sun, Nov, 2, -300}; //UTC - 5 hours
Timezone usEastern(usEDT, usEST);

// calls Time.zone(), Time.enableDST(), Time.setDSTOffset() using a DST setting determined from
// the current UTC time and the data in the Timezone instance.
Time.timezone(usEstern);

But I can't see any way to set an auto-switching pattern in the docs or in this PR, but that would seem to be the only thing that we are actually missing.
If I still have to decide to switch DST on or off in code, I could do that just as much via Time.zone() as usual - so what?

BTW, these values (zone, DSTOffset and the swtiching rules) should be made sticky too, to add value, otherwise we could dump (setDSTOffset(float)) in favour of beginDST(float) - since we'd have to code both lines ourselves anyway, what's the benefit?

Without the ability to set switching rules (actually not switching rules but ranges to automatically check agains to apply DST or not whenever a time is requested) and persisting the settings the whole "enhancement" here is worth little - IMHO.

@pomplesiegel

This comment has been minimized.

Copy link

commented Oct 12, 2016

Oh, i didn't realize that this DST implementation wouldn't automatically switch on and off based on the calendar! Unfortunately I have to agree - an implementation for adding or subtracting amounts of an hour from the current time won't help us very much as product developers.

Right now we're manually mimicking DST by changing the Time.zone() offset as an OTA function call. I don't think this feature in its current state will be be much more elegant in final application implementation.

Is there a gameplan for an automatically adjusting time based on DST calendar rules? Virtually every ioT product in the world needs this.

@technobly technobly deleted the feature/time-dst branch Oct 27, 2016

@ScruffR

This comment has been minimized.

Copy link
Contributor

commented Nov 24, 2016

For keeping some things on the radar, I carried over some bits and bobs to issue #1161

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.