Skip to content

Conversation

petslane
Copy link
Contributor

getEpochTime() should return epoch time that does not include offsets.

User offset is included in methods returning local times - getDay(), getHours(), getMinutes(), getSeconds(), getFormattedTime().

Fixes #35

@trapper-
Copy link

Can this be merged? the epoch time is wrong as it currently is

@aentinger
Copy link
Contributor

Hi @petslane 👋
As much as I agree with your point, if we do in fact merge it then it would break compatibility with everyone who is relying on getEpochTime returning the unix epoch time + the set time offset. I'd propose a "softer" approach:

  1. Add 2 new methods which are in fact doing what their name advertises
unsigned long getUnixEpochTime() const; /* Returns the true unix epoch time without time offset */
unsigned long getUnixEpochTimeWithOffset() const { return getEpochTime(); }
  1. Mark getEpochTime as deprecated.
unsigned long getEpochTime() const __attribute__ ((deprecated));
  1. Release a new version and wait some time.
  2. Remove getEpochTime().

Nora465 added a commit to Nora465/NTPClient that referenced this pull request Nov 4, 2020
Fix the epoch time by removing the time offset (by user)
Come from PR#43 : arduino-libraries#43
@CLAassistant
Copy link

CLAassistant commented Apr 9, 2021

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Peeter Normak seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@per1234 per1234 added type: imperfection Perceived defect in any part of project status: changes requested Changes to PR are required before merge topic: code Related to content of the project itself labels Sep 5, 2025
@per1234
Copy link
Contributor

per1234 commented Sep 5, 2025

There is an alternative proposal at #227

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: changes requested Changes to PR are required before merge topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Offset inappropriately applied by getEpochTime
5 participants