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

Added ability to make default time on the watch be PC clock's during … #414

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

voloved
Copy link
Contributor

@voloved voloved commented Jul 7, 2024

When running make, you can include DATE=, which will set the default time on the watch to the PC's.
Below are example outputs of the setting.

user@DESKTOP-UCQND0L:~/sensorWatch/Sensor-Watch/movement/make$ make COLOR=RED
git submodule update --init
size:
   text    data     bss     dec     hex filename
 109436    2676   11604  123716   1e344 build/watch.elf
 109436    2676   11604  123716   1e344 (TOTALS)
 
user@DESKTOP-UCQND0L:~/sensorWatch/Sensor-Watch/movement/make$ make COLOR=RED DATE=YEAR
Default year and timezone are set to 2024 EDT
git submodule update --init
size:
   text    data     bss     dec     hex filename
 109436    2676   11604  123716   1e344 build/watch.elf
 109436    2676   11604  123716   1e344 (TOTALS)
 
user@DESKTOP-UCQND0L:~/sensorWatch/Sensor-Watch/movement/make$ make COLOR=RED DATE=DAY
Default date set to Jul 07 2024 EDT
git submodule update --init
size:
   text    data     bss     dec     hex filename
 109436    2676   11604  123716   1e344 build/watch.elf
 109436    2676   11604  123716   1e344 (TOTALS)
 
user@DESKTOP-UCQND0L:~/sensorWatch/Sensor-Watch/movement/make$ make COLOR=RED DATE=MIN
Default time set to 12:45 on Jul 07 2024 EDT
git submodule update --init
size:
   text    data     bss     dec     hex filename
 109436    2676   11604  123716   1e344 build/watch.elf
 109436    2676   11604  123716   1e344 (TOTALS)
 
user@DESKTOP-UCQND0L:~/sensorWatch/Sensor-Watch/movement/make$ make COLOR=RED DATE=foo
../../make.mk:276: *** DATE must be YEAR, DAY, or MIN if used..  Stop.

Context in Discord

Comment on lines 388 to 444
for (int i = 0, count = sizeof(movement_timezone_offsets) / sizeof(movement_timezone_offsets[0]); i < count; i++) {
if (movement_timezone_offsets[i] == MAKEFILE_TIMEZONE) {
movement_state.settings.bit.time_zone = i;
break;
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel like there should be a way to set this statically, without looping over the time zones at runtime. We need to figure out the index of the timezone. Perhaps the makefile can figure out or call a script that figures it out, then it can pass the index via preprocessor definition.

Comment on lines 72 to 88
#ifdef MAKEFILE_CURR_YEAR
date_time.unit.year = MAKEFILE_CURR_YEAR;
#else
date_time.unit.year = 3;
#endif
#ifdef MAKEFILE_CURR_MONTH
date_time.unit.month = MAKEFILE_CURR_MONTH;
#endif
#ifdef MAKEFILE_CURR_DAY
date_time.unit.day = MAKEFILE_CURR_DAY;
#endif
#ifdef MAKEFILE_CURR_HOUR
date_time.unit.hour = MAKEFILE_CURR_HOUR;
#endif
#ifdef MAKEFILE_CURR_MINUTE
date_time.unit.minute = MAKEFILE_CURR_MINUTE;
#endif
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would suggest defining static functions that return the initial time components and putting the #ifdefs in them. This avoids adding complexity to the initialization function. They'll just return constants so the compiler is likely to eliminate the functions when they are compiled.

static uint16_t get_initial_year(void)
{
#ifdef MAKEFILE_CURR_YEAR
    return MAKEFILE_CURR_YEAR;
#else
    return 3;
#endif
}

// ...

date_time.unit.year = get_initial_year();

make.mk Outdated
Comment on lines 268 to 273
CFLAGS += -DMAKEFILE_TIMEZONE=$(TIMEZONE)
CFLAGS += -DMAKEFILE_CURR_YEAR=$(CURRENT_YEAR)
CFLAGS += -DMAKEFILE_CURR_MONTH=$(CURRENT_MONTH)
CFLAGS += -DMAKEFILE_CURR_DAY=$(CURRENT_DAY)
CFLAGS += -DMAKEFILE_CURR_HOUR=$(CURRENT_HOUR)
CFLAGS += -DMAKEFILE_CURR_MINUTE=$(CURRENT_MINUTE)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suggest not mentioning MAKEFILE in these variables. Users might add them to a header file and include it instead. The makefile therefore wouldn't be the only source of these values. See also PR #295 for example.

I suggest INITIAL_TIMEZONE, INITIAL_YEAR, and so on.

Comment on lines +253 to +255
CURRENT_MONTH := $(shell date +"%-m")
CURRENT_DAY := $(shell date +"%-d")
CURRENT_HOUR := $(shell date +"%-H")
CURRENT_MINUTE := $(shell date +"%-M")
Copy link
Collaborator

Choose a reason for hiding this comment

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

This adds a direct dependency on GNU coreutils for building the firmware. I consider that to be perfectly OK but then again I've been running Linux for over a decade. I have no idea how this impacts Windows users. Maintainers might want to double check this.

date_time.unit.month = get_initial_month();
date_time.unit.day = get_inital_day();
date_time.unit.hour = get_inital_hour();
date_time.unit.minute = get_inital_minute();
Copy link
Contributor

@mcguirepr89 mcguirepr89 Aug 21, 2024

Choose a reason for hiding this comment

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

Not sure if this is worth much, but it's been helpful for me. For Linux users who only add these 4 lines and don't change any other files, sed can do all the work except setting the timezone offset (though it's kind of hacky).

From movement/make:

sed -Ei "s/(unit.year = ).*(;)/\1$(( $(date +%-g) - 20 ))\2/;s/(unit.month = ).*(;)/\1$(date +%-m)\2/;s/(unit.day = ).*(;)/\1$(date +%-d)\2/;s/(unit.hour = ).*(;)/\1$(date +%-H)\2/;s/(unit.minute = ).*(;)/\1$(( $(date +%-M) + 1))\2/" ../../watch-library/hardware/main.c && make COLOR=YOURCOLOR && make install COLOR=YOURCOLOR

I know it's not pretty, but it is handy. (It also adds 1 minute to the time to allow you to put the watch back together.)

Copy link
Collaborator

@matheusmoreira matheusmoreira left a comment

Choose a reason for hiding this comment

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

Looks great!!

@matheusmoreira matheusmoreira self-assigned this Aug 25, 2024
@matheusmoreira matheusmoreira added the 2.0-wait-list This feature or pull requests has been deferred until the movement 2.0 refactor is complete label Sep 7, 2024
@matheusmoreira
Copy link
Collaborator

This is a good PR but since movement 2.0 will bring about a new build system it seems pointless to merge this in right now. Let's wait and revisit this after the refactoring is done!

@matheusmoreira matheusmoreira added enhancement New feature or request build-system Related to the project's make files and build automation labels Sep 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.0-wait-list This feature or pull requests has been deferred until the movement 2.0 refactor is complete build-system Related to the project's make files and build automation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants