hasadna / open-bus-map-search

open-bus-map-search
https://open-bus-map-search.hasadna.org.il/dashboard
MIT License
56 stars 90 forks source link

fix: מפה לפי קו - לא מוצגות נסיעות לאחר השעה 6 בערב #902

Open witty-code opened 3 weeks ago

witty-code commented 3 weeks ago

יש כרגע בעיה עם הממשק בעמוד של מפה לפי קו, הוא לא מציג נסיעות שמתחילות לאחר השעה 6 בערב.

לאחר בדיקה איתרתי את הבאג: הפונקציה useSingleLineData יוצרת זמן התחלה לתחילת היום וזמן סיום עבור הרגע האחרון של היום:

const { locations, isLoading: locationsAreLoading } = useVehicleLocations({
    from: +new Date(timestamp).setHours(0, 0, 0, 0),
    to: +new Date(timestamp).setHours(23, 59, 59, 999),
    lineRef,
    splitMinutes: 360,
    pause: !lineRef,
  })

הפונקציה useVehicleLocations מחלקת את טווח הזמן לחלקים שווים לפי הערך split באמצעות הפונקציה getMinutesInRange:

function getMinutesInRange(from: Dateable, to: Dateable, gap = 1) {
  const start = moment(from).startOf('minute')
  const end = moment(to).startOf('minute')

  // array of minutes to load
  const minutes = Array.from({ length: end.diff(start, 'minutes') / gap }, (_, i) => ({
    from: start.clone().add(i * gap, 'minutes'),
    to: start.clone().add((i + 1) * gap, 'minutes'),
  }))
  return minutes
}

הבעיה שנוצרת היא שהקוד:

length: end.diff(start, 'minutes') / gap

מחזיר במקרה הזה את הערך 3.9972222222222222, אשר מתורגם כנראה בתור ערך מספרי 3, נסה:

parseInt(end.diff(start, 'minutes') / gap)

זה גורם לכך שנשלחות לשרת בקשות של 3 טווחי זמן של 6 שעות, במקום 4 כפי הנדרש.

יש 2 פתרונות אפשריים, אבל מכיוון שלא חקרתי את כל הקוד אינני יכול להמליץ על אחד מהם, ולכן אני מניח כאן את הדברים במקום לבצע Fork פשוט:

א. לבצע שינוי בפונקציה useSingleLineData כך שערך הסיום יוגדר לשנייה הראשונה של היום הבא, לדוגמא:

to: +new Date(timestamp + 24 * 60 * 60 * 1000).setHours(0, 0, 0, 0);

ב. לשנות את הפונקציה getMinutesInRange ולהחיל round על תוצאות החלוקה ב-gap (נראה פחות מומלץ, יש לבדוק את ההשלכות על כל מקרי השימוש במרבי הממשק):

function getMinutesInRange(from, to, gap = 1) {
  const start = moment(from).startOf('minute')
  const end = moment(to).startOf('minute')

  // array of minutes to load
  const minutes = Array.from({ length: Math.round(end.diff(start, 'minutes') / gap) }, (_, i) => ({
    from: start.clone().add(i * gap, 'minutes'),
    to: start.clone().add((i + 1) * gap, 'minutes'),
  }))
  return minutes
}
NoamGaash commented 1 week ago

מעולה! great catch :clap: ממליץ לקחת את הissue הזה לכל מי שרוצה לצלול לתוך החלקים javascript שלנו - אעשה לו pinned.

TomRytt commented 3 days ago

היי נועם, אני אשמח לטפל בזה :) מתחיל עכשיו

TomRytt commented 3 days ago

היי נועם, פתרתי את הבאג בעזרת הטכניקה שהועלתה כאן עם שינוי קל:

const today = new Date(timestamp)
  const tomorrow = new Date(today)
  tomorrow.setDate(tomorrow.getDate() + 1)
  const { locations, isLoading: locationsAreLoading } = useVehicleLocations({
    from: +today.setHours(0, 0, 0, 0),
    to: +new Date(tomorrow).setHours(0, 0, 0, 0),
    lineRef,
    splitMinutes: 60,
    pause: !lineRef,
  })

אחרי שסידרתי את זה שמתי לב שיש באג שהשעה 11:00 PM ו-11:30 PM מופיעות ראשונות וזה חירפן אותי. שברתי על זה את הראש ועל הדרך עשיתי אופטימיזציות לפילטור והסידור של השעות כ-options בתוך ה-select:

פונקציה יוטילית להמרה לבסיס 24:

function convertTo24HourAndToNumber(time: string): number {
    const match = time.match(/(\d+):(\d+):(\d+)\s(AM|PM)/)
    if (!match) return 0

    const [, hour, minute, , modifier] = match
    let newHour = parseInt(hour, 10)
    if (modifier === 'AM' && newHour === 12) newHour = 0
    if (modifier === 'PM' && newHour !== 12) newHour += 12

    return newHour * 60 + parseInt(minute, 10)
  }

הקוד החדש שמייצר את ה-options:

const options = useMemo(() => {
    const filteredPositions = positions.filter(
      (position) =>
        !!position.recorded_at_time && position.recorded_at_time > +today.setHours(0, 0, 0, 0),
    )

    const uniqueTimes = Array.from(
      new Set(
        filteredPositions
          .map((position) => position.point?.siri_ride__scheduled_start_time)
          .filter((time): time is string => !!time)
          .map((time) => time.trim()),
      ),
    )
      .map((time) => new Date(time).toLocaleTimeString()) // Convert to 24-hour time string
      .map((time) => ({
        value: time,
        label: time,
      }))

    const sortedOptions = uniqueTimes.sort(
      (a, b) => convertTo24HourAndToNumber(a.value) - convertTo24HourAndToNumber(b.value),
    )

    return sortedOptions
  }, [positions])

יש בעיה שה-loader עובד בלי סוף. אני לא בטוח אם זה באג ישן או חדש... זה שווה בדיקה. אם תגיד לי שמה שפה לדעתך נותן מענה ונראה סבבה אני אפתח pull request מסודר (יכול גם קודם לפתוח, מה שתעדיף)

witty-code commented 3 days ago

הסיבה שבגינה השעות 11:00 PM ו-11:30 PM מופיעות ראשונות היא משום שהשאילתה מחזירה תוצאות של שידורי GPS החל מהשעה 00:00 בתחילת היום שנבחר ועד השעה 23:59 (או 00:00 לאחר התיקון המוצע) בסוף היום. היות והשאילתה מתבצעת לפי שעת שידור המיקום ולא לפי שעת היציאה המתוכננת, נכללות בתוצאת השאילתה גם נסיעות מיום קודם שנמשכו לאחר חצות (00:00), נסיעות אלו מופיעות בתוצאות החיפוש באופן חלקי - מופיעים רק רישומי ה-GPS שנקלטו לאחר השעה 00:00. במקביל - נסיעות שבוצעו ביום של תאריך החיפוש אך נמשכו לאחר השעה 00:00 מופיעות במקוטע - מופיעים רק רישומי ה-GPS של נסיעות אלו אשר נקלטו עד השעה 00:00. הסיבה כאמור - שאילתה למסד הנתונים מתבצעת לפי שעת השידור של המיקום ולא לפי שעת היציאה.

הבלגן מתחיל כאשר הפונקציה ממיינת את מיקומי ה-GPS לפי שעת היציאה של הנסיעה, ולכן נסיעות של יום קודם מופיעות יחד עם נסיעות של היום הנוכחי, במידה והן התחילו בשתי הימים העוקבים באותה השעה. לדוגמא: קו 92 יצא מתחנת המוצא בשעה 23:30 בתאריכים העוקבים: 11/11/2024 ו-12/11/2024 , בחיפוש אחר נסיעות של קו זה עבור תאריך 12/11/2024 ה-dropdown יציג פעמיים את שעת היציאה 23:30, כאשר בחירה בכל אחד מהם תציג נסיעה בודדת ורציפה כביכול:

  1. היא תכלול את רישומי ה-GPS של הנסיעה שהחלה ב-11/11/2024 בשעה 23:30, עבור נסיעה זו יופיעו מיקומים שנקלטו לאחר השעה 00:00. רכב 9265601.
  2. היא תכלול את רישומי ה-GPS של הנסיעה שהחלה ב-12/11/2024 בשעה 23:30, עבור נסיעה זו יופיעו רק מיקומים שנקלטו עד השעה 00:00 בסוף היום. רכב 9234801.

אני סבור ששאילתת החיפוש צריכה להתחיל ולסיים בשעה 04:00 לפנות בוקר, אשר היא השעה הרלוונטית להתחלפות 'יום תחבורה' לפי משרד התחבורה.

TomRytt commented 3 days ago

בוקר טוב,

גם חשבתי שזה מה שקורה, כן שמתי לב שכששמתי מהשעה 1 בלילה עד 1 בלילה הנסיעות האלו באמת לא הופיעו, אבל בגלל שזה מה שהיה לפני ולא היה חלק מהבאג הנוכחי שנפתח, לא רציתי לטפל בזה בצורה ישירה, כנראה שהייתי צריך כבר לסדר את זה גם כן על הדרך. זה שינוי קטן ממש, אני אעשה אותו היום בערב ואוודא שזה אכן סידר את הבעיה והנתונים נראים הגיוניים ואסדר אותם לפי זה. אישית אני גם חושב שסידור השעות על פי חלוקה ל-24 ולא ל-AM/PM תהיה יותר נוחה.

On Wed, Nov 20, 2024 at 3:51 AM witty-code @.***> wrote:

הסיבה שבגינה השעות 11:00 PM ו-11:30 PM מופיעות ראשונות היא משום שהשאילתה מחזירה תוצאות של שידורי GPS החל מהשעה 00:00 בתחילת היום שנבחר ועד השעה 23:59 (או 00:00 לאחר התיקון המוצע) בסוף היום. היות והשאילתה מתבצעת לפי שעת שידור המיקום ולא לפי שעת היציאה המתוכננת, נכללות בתוצאת השאילתה גם נסיעות מיום קודם שנמשכו לאחר חצות (00:00), נסיעות אלו מופיעות בתוצאות החיפוש באופן חלקי - מופיעים רק רישומי ה-GPS שנקלטו לאחר השעה 00:00. במקביל - נסיעות שבוצעו ביום של תאריך החיפוש אך נמשכו לאחר השעה 00:00 מופיעות במקוטע - מופיעים רק רישומי ה-GPS של נסיעות אלו אשר נקלטו עד השעה 00:00. הסיבה כאמור - שאילתה למסד הנתונים מתבצעת לפי שעת השידור של המיקום ולא לפי שעת היציאה.

הבלגן מתחיל כאשר הפונקציה ממיינת את מיקומי ה-GPS לפי שעת היציאה של הנסיעה, ולכן נסיעות של יום קודם מופיעות יחד עם נסיעות של היום הנוכחי, במידה והן התחילו בשתי הימים העוקבים באותה השעה. לדוגמא: קו 92 יצא מתחנת המוצא בשעה 23:30 בתאריכים העוקבים: 11/11/2024 ו-12/11/2024 , בחיפוש אחר נסיעות של קו זה עבור תאריך 12/11/2024 ה-dropdown יציג פעמיים את שעת היציאה 23:30, כאשר בחירה בכל אחד מהם תציג נסיעה בודדת ורציפה כביכול:

  1. היא תכלול את רישומי ה-GPS של הנסיעה שהחלה ב-11/11/2024 בשעה 23:30, עבור נסיעה זו יופיעו מיקומים שנקלטו לאחר השעה 00:00. רכב 9265601.
  2. היא תכלול את רישומי ה-GPS של הנסיעה שהחלה ב-12/11/2024 בשעה 23:30, עבור נסיעה זו יופיעו רק מיקומים שנקלטו עד השעה 00:00 בסוף היום. רכב 9234801.

אני סבור ששאילתת החיפוש צריכה להתחיל ולסיים בשעה 04:00 לפנות בוקר, אשר היא השעה הרלוונטית להתחלפות 'יום תחבורה' לפי משרד התחבורה.

— Reply to this email directly, view it on GitHub https://github.com/hasadna/open-bus-map-search/issues/902#issuecomment-2487149827, or unsubscribe https://github.com/notifications/unsubscribe-auth/AUXFB5JHE7DR7M2LWNZ44WL2BPTJPAVCNFSM6AAAAABQ6YT6NKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIOBXGE2DSOBSG4 . You are receiving this because you were assigned.Message ID: @.***>

NoamGaash commented 3 days ago

:exploding_head: מדהים, תודה רבה על החקר הזה. ההבחנה בין זמן יציאה מתוכננת לזמן שידור המיקום היא חשובה ובאמת התפספסה לאורך הדרך. לא ידעתי שיש שעת החלפת יום רשמית של משרד התחבורה - שמעתי שהם מוסיפים שעה 25 ליממה במקרים מסויימים. יש לך מקור לזה? זה ממש מגניב אם זה משהו שאפשר להסתמך עליו. תודה רבה לשניכם :1st_place_medal:

witty-code commented 2 days ago

המקור שלי הוא מהתיעוד הרשמי של משרד התחבורה בקישור הזה בסעיף 2.8.2, וכן בסעיף 9.2 :

Question: There are cases in which the field arrival_time at ‘stop_times’ file are above 24 hours, for example: 25:31:00

Answer: This is not an error. The logic of the field arrival_time is dictated by the GTFS protocol and is carried out as follows: A trip that begins before midnight and continues after midnight - all of its station's timing will appear on the same calendar day it started, and the hours are 24, 25, 26, etc ... A trip that begins after midnight, passes to the next calendar day. And the hours will be 1 2, 3 ... etc. In terms of the key, after understanding the above logic, you can define in the app itself that every trip that appears between 00:00 and 04:00 will be displayed as the night line. It should be noted that regular public transportation starts at 04:00 in the morning, except in rare cases.

אפשר לחלוק על הפרשנות הזו, היות שבפועל נסיעות שמתחילות לאחר חצות מתוארכות ליום הקלנדרי שבו החלו ולא ליום הקודם.

בנוסף, בצו פיקוח על מחירי מצרכים ושירותים (דמי נסיעה ברכבת וברכבל), התשפ״ב–2022, סעיף 1(א) :

”יום“ – לעניין חיוב תשלום באמצעי תשלום מתקדם – פרק הזמן שמשעה 04:00:00 עד שעה 03:59:59 ביום העוקב;

TomRytt commented 2 days ago

בהמשך לשיחה שלנו, החלפתי את recorded_at_time בפונקציה עם position.point?.siri_ride__scheduled_start_time וגם שיפרתי את הפילטר ויצאתי מהפונקציה במידה והפילטור החזיר מערך ריק (לפני שבוחרים קו)

מהבדיקה שערכתי זה גם עבד מצוין וגם הרבה יותר מהיר מבעבר (הרבה פחות קווים עוברים את הפילטור הראשוני)

זה השינוי המרכזי:

    const filteredPositions = positions.filter((position) => {
      const startTime = position.point?.siri_ride__scheduled_start_time
      return !!startTime && +new Date(startTime) > +today.setHours(0, 0, 0, 0)
    })

    if (filteredPositions.length === 0) return []