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

Automatic DST support #3072

Merged
merged 12 commits into from
May 20, 2018
58 changes: 58 additions & 0 deletions .github/stale.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
# Configuration for probot-stale - https://github.com/probot/stale

# Number of days of inactivity before an Issue or Pull Request becomes stale
daysUntilStale: 60

# Number of days of inactivity before a stale Issue or Pull Request is closed.
# Set to false to disable. If disabled, issues still need to be closed manually, but will remain marked as stale.
daysUntilClose: 14

# Issues or Pull Requests with these labels will never be considered stale. Set to `[]` to disable
exemptLabels:
- BUG
- Feature Request
- Pinned

# Set to true to ignore issues in a project (defaults to false)
exemptProjects: false

# Set to true to ignore issues in a milestone (defaults to false)
exemptMilestones: true

# Label to use when marking as stale
staleLabel: Inactive

# Comment to post when marking as stale. Set to `false` to disable
markComment: >
This issue / pull request has been automatically marked as stale because it
has not had any activity in 60 days. The resources of the INAV team are limited,
and so we are asking for your help.

This issue / pull request will be closed if no further activity occurs within two weeks.


# Comment to post when removing the stale label.
# unmarkComment: >
# Your comment here.

# Comment to post when closing a stale Issue or Pull Request.
closeComment: >
Automatically closing as inactive.

# Limit the number of actions per hour, from 1-30. Default is 30
limitPerRun: 30

# Limit to only `issues` or `pulls`
# only: issues

# Optionally, specify configuration settings that are specific to just 'issues' or 'pulls':
# pulls:
# daysUntilStale: 30
# markComment: >
# This pull request has been automatically marked as stale because it has not had
# recent activity. It will be closed if no further activity occurs. Thank you
# for your contributions.

# issues:
# exemptLabels:
# - confirmed
4 changes: 3 additions & 1 deletion docs/Cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ Re-apply any new defaults as desired.
| gps_auto_config | ON | Enable automatic configuration of UBlox GPS receivers. |
| gps_auto_baud | ON | Automatic configuration of GPS baudrate(The specified baudrate in configured in ports will be used) when used with UBLOX GPS. When used with NAZA/DJI it will automatic detect GPS baudrate and change to it, ignoring the selected baudrate set in ports |
| gps_min_sats | 6 | Minimum number of GPS satellites in view to acquire GPS_FIX and consider GPS position valid. Some GPS receivers appeared to be very inaccurate with low satellite count. |
| gps_ublox_use_galileo | OFF | Enable use of Galileo satellites. This is at the expense of other regional constellations, so benefit may also be regional. Requires M8N and Ublox firmware 3.x (or later) [OFF/ON].
| gps_ublox_use_galileo | OFF | Enable use of Galileo satellites. This is at the expense of other regional constellations, so benefit may also be regional. Requires M8N and Ublox firmware 3.x (or later) [OFF/ON].
| inav_auto_mag_decl | ON | Automatic setting of magnetic declination based on GPS position. When used manual magnetic declination is ignored. |
| inav_gravity_cal_tolerance | 5 | Unarmed gravity calibration tolerance level. Won't finish the calibration until estimated gravity error falls below this value. |
| inav_use_gps_velned | ON | Defined if iNav should use velocity data provided by GPS module for doing position and speed estimation. If set to OFF iNav will fallback to calculating velocity from GPS coordinates. Using native velocity data may improve performance on some GPS modules. Some GPS modules introduce significant delay and using native velocity may actually result in much worse performance. |
Expand Down Expand Up @@ -414,6 +414,8 @@ Re-apply any new defaults as desired.
| rssi_adc_channel | - | ADC channel to use for analog RSSI input. Defaults to board RSSI input (if available). 0 = disabled |
| current_adc_channel | - | ADC channel to use for analog current sensor input. Defaults to board CURRENT sensor input (if available). 0 = disabled |
| airspeed_adc_channel | 0 | ADC channel to use for analog pitot tube (airspeed) sensor. If board doesn't have a dedicated connector for analog airspeed sensor will default to 0 |
| tz_offset | 0 | Time zone offset from UTC, in minutes. This is applied to the GPS time for logging and time-stamping of Blackbox logs |
| tz_automatic_dst | OFF | Automatically add Daylight Saving Time to the GPS time when needed or simply ignore it. Includes presets for EU and the USA - if you live outside these areas it is suggested to manage DST manually via `tz_offset`. |

This Markdown table is made by MarkdwonTableMaker addon for google spreadsheet.
Original Spreadsheet used to make this table can be found here https://docs.google.com/spreadsheets/d/1ubjYdMGmZ2aAMUNYkdfe3hhIF7wRfIjcuPOi_ysmp00/edit?usp=sharing
85 changes: 78 additions & 7 deletions src/main/common/time.c
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,11 @@ static const uint16_t days[4][12] =
{1096, 1127, 1155, 1186, 1216, 1247, 1277, 1308, 1339, 1369, 1400, 1430},
};

PG_REGISTER_WITH_RESET_TEMPLATE(timeConfig_t, timeConfig, PG_TIME_CONFIG, 0);
PG_REGISTER_WITH_RESET_TEMPLATE(timeConfig_t, timeConfig, PG_TIME_CONFIG, 1);

Copy link
Member

Choose a reason for hiding this comment

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

You're adding fields to the parameter group. Please also increase version number in the declaration PG_REGISTER_WITH_RESET_TEMPLATE

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rookie mistake, my bad.

Copy link
Member

Choose a reason for hiding this comment

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

No worries, I keep forgetting to do this myself 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there any more work left to be done on this? Sorry to be bothering, it seems that I cannot manually mark this request as solved.

PG_RESET_TEMPLATE(timeConfig_t, timeConfig,
.tz_offset = 0,
.tz_automatic_dst = TZ_AUTO_DST_OFF,
);

static rtcTime_t dateTimeToRtcTime(dateTime_t *dt)
Expand Down Expand Up @@ -120,14 +121,84 @@ static bool rtcIsDateTimeValid(dateTime_t *dateTime)
(dateTime->millis <= 999);
}

static void dateTimeWithOffset(dateTime_t *dateTimeOffset, dateTime_t *dateTimeInitial, int16_t minutes)
#if defined(RTC_AUTOMATIC_DST)
static int lastSundayOfMonth(int currentYear, int wantedMonth) {
Copy link
Member

Choose a reason for hiding this comment

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

Indentation and formatting need some work in all this function. Opening brace for functions should be in the next line. Indent using 4 spaces and add a space before and after , as well as arithmetic operators.

int days[] = {31,29,31,30,31,30,31,31,30,31,30,31};
int m, y = currentYear, w;
Copy link
Member

Choose a reason for hiding this comment

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

This line seems very hairy. m should be declared in the for, since it's initialised and used only there. y is assigned to w (where is that variable coming from?), but it looks like you could just use currentYear a few lines down without any extra variables.

days[1] -= (y % 4) || (!(y % 100) && (y % 400));
w = y * 365 + (y - 1) / 4 - (y - 1) / 100 + (y - 1) / 400 + 6;

for(m = 0; m < 12; m++) {
Copy link
Member

Choose a reason for hiding this comment

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

Space between for and (

w = (w + days[m]) % 7;
if (m == wantedMonth - 1) {
return days[m] + (w < 5 ? -2 : 5) - w;
}
}
return 0;
}

static int nthSundayOfMonth(int lastSunday, int nth) {
while (lastSunday > 7 * nth) {
lastSunday -= 7;
}
return lastSunday;
}

static bool isDST(rtcTime_t t) {
dateTime_t dateTime;
rtcTimeToDateTime(&dateTime, t);
int lastSunday;
switch ((tz_automatic_dst_e) timeConfig()->tz_automatic_dst) {
case TZ_AUTO_DST_USA: // begins at 2:00 a.m. on the second Sunday of March and ends at 2:00 a.m. on the first Sunday of November
if (dateTime.month < 3 || dateTime.month > 11) { return false; }
if (dateTime.month > 3 && dateTime.month < 11) { return true; }
lastSunday = lastSundayOfMonth(dateTime.year, dateTime.month);
if (dateTime.month == 3) {
int secondSunday = nthSundayOfMonth(lastSunday, 2);
if (dateTime.day == secondSunday) {
return dateTime.hours >= 2;
}
return dateTime.day > secondSunday;
}
if (dateTime.month == 11) {
int firstSunday = nthSundayOfMonth(lastSunday, 1);
if (dateTime.day == firstSunday) {
return dateTime.hours < 2;
}
return dateTime.day < firstSunday;
}
break;
case TZ_AUTO_DST_EU: // begins at 1:00 a.m. on the last Sunday of March and ends at 1:00 a.m. on the last Sunday of October
if (dateTime.month < 3 || dateTime.month > 10) { return false; }
if (dateTime.month > 3 && dateTime.month < 10) { return true; }
lastSunday = lastSundayOfMonth(dateTime.year, dateTime.month);
if (dateTime.day < lastSunday) { return !(dateTime.month == 3); }
if (dateTime.day > lastSunday) { return !(dateTime.month == 3); }
if (dateTime.day == lastSunday) {
if (dateTime.month == 3) {
return dateTime.hours >= 1;
}
if (dateTime.month == 10) {
return dateTime.hours < 1;
}
}
break;
}
return false;
}
#endif

static void dateTimeWithOffset(dateTime_t *dateTimeOffset, dateTime_t *dateTimeInitial, int16_t minutes, bool automatic_dst)
{
rtcTime_t initialTime = dateTimeToRtcTime(dateTimeInitial);
rtcTime_t offsetTime = rtcTimeMake(rtcTimeGetSeconds(&initialTime) + minutes * 60, rtcTimeGetMillis(&initialTime));
#if defined(RTC_AUTOMATIC_DST)
if (automatic_dst && isDST(offsetTime)) { offsetTime += 3600; }
Copy link
Member

Choose a reason for hiding this comment

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

Please, align the #if at the start of the line. Don't put the code inside the if in the same line. We use:

if (condition) {
    do_foo();
}

(this applies to more places in the diff, but no reason to repeat the same comment in all of them)

#endif
rtcTimeToDateTime(dateTimeOffset, offsetTime);
}

static bool dateTimeFormat(char *buf, dateTime_t *dateTime, int16_t offset)
static bool dateTimeFormat(char *buf, dateTime_t *dateTime, int16_t offset, bool automatic_dst)
{
dateTime_t local;

Expand All @@ -139,7 +210,7 @@ static bool dateTimeFormat(char *buf, dateTime_t *dateTime, int16_t offset)
if (offset != 0) {
tz_hours = offset / 60;
tz_minutes = ABS(offset % 60);
dateTimeWithOffset(&local, dateTime, offset);
dateTimeWithOffset(&local, dateTime, offset, automatic_dst);
dateTime = &local;
}

Expand Down Expand Up @@ -176,17 +247,17 @@ uint16_t rtcTimeGetMillis(rtcTime_t *t)

bool dateTimeFormatUTC(char *buf, dateTime_t *dt)
{
return dateTimeFormat(buf, dt, 0);
return dateTimeFormat(buf, dt, 0, false);
}

bool dateTimeFormatLocal(char *buf, dateTime_t *dt)
{
return dateTimeFormat(buf, dt, timeConfig()->tz_offset);
return dateTimeFormat(buf, dt, timeConfig()->tz_offset, timeConfig()->tz_automatic_dst);
}

void dateTimeUTCToLocal(dateTime_t *utcDateTime, dateTime_t *localDateTime)
{
dateTimeWithOffset(localDateTime, utcDateTime, timeConfig()->tz_offset);
dateTimeWithOffset(localDateTime, utcDateTime, timeConfig()->tz_offset, timeConfig()->tz_automatic_dst);
}

bool dateTimeSplitFormatted(char *formatted, char **date, char **time)
Expand Down
7 changes: 7 additions & 0 deletions src/main/common/time.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,15 @@ typedef uint32_t timeUs_t;

static inline timeDelta_t cmpTimeUs(timeUs_t a, timeUs_t b) { return (timeDelta_t)(a - b); }

typedef enum {
TZ_AUTO_DST_OFF,
TZ_AUTO_DST_EU,
TZ_AUTO_DST_USA,
} tz_automatic_dst_e;

typedef struct timeConfig_s {
int16_t tz_offset; // Offset from UTC in minutes, might be positive or negative
uint8_t tz_automatic_dst; // Automatically handle DST or ignore it, values come from tz_automatic_dst_e
} timeConfig_t;
Copy link
Member

Choose a reason for hiding this comment

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

Rather than 2 variables, use just one uint8_t named tz_automatic_dst which can take the values OFF (default), US,EU, etc... That way we use a single byte for storage and users just need to change one setting to make it work. Put a comment at the right indicating that the value comes from tz_automatic_dst_e.


PG_DECLARE(timeConfig_t, timeConfig);
Expand Down
5 changes: 5 additions & 0 deletions src/main/fc/settings.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,9 @@ tables:
- name: smartport_fuel_unit
values: ["PERCENT", "MAH", "MWH"]
enum: smartportFuelUnit_e
- name: tz_automatic_dst
values: ["OFF", "EU", "USA"]
enum: tz_automatic_dst_e

groups:
- name: PG_GYRO_CONFIG
Expand Down Expand Up @@ -1665,6 +1668,8 @@ groups:
- name: tz_offset
min: -1440
max: 1440
- name: tz_automatic_dst
type: uint8_t

- name: PG_DISPLAY_CONFIG
type: displayConfig_t
Expand Down
3 changes: 3 additions & 0 deletions src/main/target/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,9 @@
#define VTX_SMARTAUDIO
#define VTX_TRAMP

//Enable DST calculations
#define RTC_AUTOMATIC_DST

#else // FLASH_SIZE < 128
#define CLI_MINIMAL_VERBOSITY
#define SKIP_TASK_STATISTICS
Expand Down