potentially wrong handling of GTFS Time values

[derhuerst]

GTFS Time is not defined relative to midnight, but relative to noon - 12h. While that makes "writing" GTFS feeds easier, it makes processing a lot harder.

Expected functionality

As explained in my note about GTFS Time values, with the Europe/Berlin time zone (+1h standard time to +2 DST shift occurs at 2021-03-28T02:00+01:00), I expect

• the departure_time of 00:30 of a trip running on 2021-03-28 to happen at 1616884200/2021-03-28T00:30+02:00, not at 1616887800/2021-03-28T00:30+01:00;
• the departure_time of 06:30 of a trip running on 2021-03-28 to happen at 1616905800/2021-03-28T06:30+02:00, not at 1616909400/2021-03-28T06:30+01:00.

Describe the bug

I'm very inexperienced with Java and not familiar with this code base, but it seems that TheTransitClock is affected by this problem on those days that the DST <-> standard time switch occurs on.

I'm not sure how that actually manifests in TheTransitClock's output, but I assume that wrong delays will be calculated, or that realtime data can't be matched against static data.

I tried to identify all affected places in the code base:

• transitime/transitclock/src/main/java/org/transitclock/utils/Time.java

  Lines 178 to 182 in 5347c07

  	// Get seconds into day
  	calendar.setTimeInMillis(epochTime);
  	return calendar.get(Calendar.HOUR_OF_DAY) * 60 * 60 +
  	calendar.get(Calendar.MINUTE) * 60 +
  	calendar.get(Calendar.SECOND);

• transitime/transitclock/src/main/java/org/transitclock/utils/Time.java

  Lines 219 to 224 in 5347c07

  	// Get seconds into day
  	calendar.setTime(epochTime);
  	return calendar.get(Calendar.HOUR_OF_DAY) * 60 * 60 * 1000 +
  	calendar.get(Calendar.MINUTE) * 60 * 1000 +
  	calendar.get(Calendar.SECOND) * 1000 +
  	calendar.get(Calendar.MILLISECOND);

• transitime/transitclock/src/main/java/org/transitclock/utils/Time.java

  Lines 239 to 249 in 5347c07

  	Calendar calendar = new GregorianCalendar(tz);
  	calendar.setTime(date);
  	calendar.set(Calendar.MILLISECOND, 0);
  	calendar.set(Calendar.SECOND, 0);
  	calendar.set(Calendar.MINUTE, 0);
  	calendar.set(Calendar.HOUR_OF_DAY, 0);
  	// Get the epoch time
  	long epochTime = calendar.getTimeInMillis();
  	return epochTime;

• transitime/transitclock/src/main/java/org/transitclock/utils/Time.java

  Lines 262 to 272 in 5347c07

  	Calendar calendar = new GregorianCalendar();
  	calendar.setTime(date);
  	calendar.set(Calendar.MILLISECOND, 0);
  	calendar.set(Calendar.SECOND, 0);
  	calendar.set(Calendar.MINUTE, 0);
  	calendar.set(Calendar.HOUR_OF_DAY, 0);
  	// Get the epoch time
  	long epochTime = calendar.getTimeInMillis();
  	return epochTime;

• transitime/transitclock/src/main/java/org/transitclock/utils/Time.java

  Lines 288 to 331 in 5347c07

  	// Determine seconds, minutes, and hours
  	int seconds = secondsIntoDay % 60;
  	int minutesIntoDay = secondsIntoDay / 60;
  	int minutes = minutesIntoDay % 60;
  	int hoursIntoDay = minutesIntoDay / 60;
  	int hours = hoursIntoDay % 24;
  	// Set the calendar to use the reference time so that get the
  	// proper date.
  	calendar.setTime(referenceDate);
  	// Set the seconds, minutes, and hours so that the calendar has
  	// the proper time. Need to also set milliseconds because otherwise
  	// would use milliseconds from the referenceDate.
  	calendar.set(Calendar.MILLISECOND, 0);
  	calendar.set(Calendar.SECOND, seconds);
  	calendar.set(Calendar.MINUTE, minutes);
  	calendar.set(Calendar.HOUR_OF_DAY, hours);
  	// Get the epoch time
  	long epochTime = calendar.getTimeInMillis();
  	// Need to make sure that didn't have a problem around midnight.
  	// For example, a vehicle is supposed to depart a layover at
  	// 00:05:00 right after midnight but the AVL time might be for
  	// 23:57:13, which is actually for the previous day. If would
  	// simply set the hours, minutes and seconds then would wrongly
  	// get an epoch time for the previous day. Could have the same
  	// problem if the AVL time is right after midnight but the
  	// secondsIntoDay is just before midnight. Therefore if the
  	// resulting epoch time is too far away then adjust the epoch
  	// time by plus or minus day. Note: originally used 12 hours
  	// instead of 20 hours but that caused problems when trying to
  	// determine if a block is active because it might have started
  	// more than 12 hours ago. By using 20 hours we are much more likely
  	// to get the correct day because will only correct if really far
  	// off.
  	if (epochTime > referenceDate.getTime() + 20 * MS_PER_HOUR) {
  	// subtract a day
  	epochTime -= MS_PER_DAY;
  	} else if (epochTime < referenceDate.getTime() - 20 * MS_PER_HOUR) {
  	// add a day
  	epochTime += MS_PER_DAY;
  	}

• transitime/transitclock/src/main/java/org/transitclock/utils/Time.java

  Lines 365 to 376 in 5347c07

  	int timeIntoDay = getMsecsIntoDay(epochTime);
  	long delta = Math.abs(referenceTimeIntoDayMsecs - timeIntoDay);
  	long deltaForBeforeMidnight =
  	Math.abs(referenceTimeIntoDayMsecs - (timeIntoDay - Time.MS_PER_DAY));
  	if (deltaForBeforeMidnight < delta)
  	return timeIntoDay - (int)Time.MS_PER_DAY;
  	long deltaForAfterMidnight =
  	Math.abs(referenceTimeIntoDayMsecs - (timeIntoDay + Time.MS_PER_DAY));
  	if (deltaForAfterMidnight < delta)
  	return timeIntoDay + (int)Time.MS_PER_DAY;

Technically also these are wrong, because they convert a GTFS Time into a human-readable time string:

• transitime/transitclock/src/main/java/org/transitclock/utils/Time.java

  Lines 598 to 632 in 5347c07

  	String timeStr = "";
  	if (secInDay < 0) {
  	timeStr="-";
  	secInDay = -secInDay;
  	}
  	// Handle if time is into next day
  	if (secInDay > 24*60*60)
  	secInDay -= 24*60*60;
  	// Handle if PM instead of AM
  	boolean pm = false;
  	if (secInDay > 12*60*60) {
  	pm = true;
  	secInDay -= 12*60*60;
  	}
  	long hours = secInDay / (60*60);
  	long minutes = (secInDay % (60*60)) / 60;
  	// Use StringBuilder instead of just concatenating strings since it
  	// indeed is faster. Actually measured it and when writing out
  	// GTFS stop_times file it was about 10% faster when using
  	// StringBuilder.
  	StringBuilder b = new StringBuilder(8);
  	b.append(timeStr);
  	if (hours<10) b.append("0");
  	b.append(hours).append(":");
  	if (minutes < 10) b.append("0");
  	b.append(minutes);
  	if (pm)
  	b.append("PM");
  	else
  	b.append("AM");
  	return b.toString();

Version:

Just poked around in the develop source code.

---

related: google/transit#15