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

Rename DAYS_PER_WEEK constant #115

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tscofield
Copy link

The DAYS_PER_WEEK constant name conflicts with a constant in the default Time.h library

The DAYS_PER_WEEK constant name conflicts with a constant in the default Time.h library
@cyberman54
Copy link

Same here, please merge this PR, thanks.

@richteel
Copy link

I just ran into this issue as well. Same as Issue 163 in TimeLib. See PaulStoffregen/Time#163.

richteel referenced this pull request in richteel/NeoGPS Sep 15, 2021
…ib's time_t is defined as unsigned long with a value of 7. This is compatable with NeoGPS declaration of DAYS_PER_WEEK with type uint8_t if cast to uint8_t. I modified the code to check if DAYS_PER_WEEK is defined and cast DAYS_PER_WEEK as uint8_t when used.

This is an alternate way to resolve the conflict between NeoGPS and TimeLib. User, tscofield, proposed a similar fix by renaming DAYS_PER_WEEK but I believe this may be a slightly better resolution to the issue.

See issues

- [https://github.com/SlashDevin/NeoGPS/pull/115](https://github.com/SlashDevin/NeoGPS/pull/115 "NeoGPS Issue 115")
- [https://github.com/PaulStoffregen/Time/issues/163](https://github.com/PaulStoffregen/Time/issues/163 "TimeLib Issue 163")

BTW: I looked into applying a similar fix to TimeLib but the issue only seemed to be resolved when applied to NeoGPS.
@richteel
Copy link

Will be creating a pull request. tscofield made a similar pull request however I wanted to make it so the same name may be used by both libraries. This only has the disadvantage if the days of the week ever change from 7.

Below is the commit statement from my change.

TimeLib defines DAYS_PER_WEEK as type time_t with a value of 7. TimeLib's time_t is defined as unsigned long with a value of 7. This is compatable with NeoGPS declaration of DAYS_PER_WEEK with type uint8_t if cast to uint8_t. I modified the code to check if DAYS_PER_WEEK is defined and cast DAYS_PER_WEEK as uint8_t when used.

This is an alternate way to resolve the conflict between NeoGPS and TimeLib. User, tscofield, proposed a similar fix by renaming DAYS_PER_WEEK but I believe this may be a slightly better resolution to the issue.

See issues

BTW: I looked into applying a similar fix to TimeLib but the issue only seemed to be resolved when applied to NeoGPS.

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