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

Stop Grouping with Distance Range and (General) Directional filter #193

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions src/components/bookmarked-stop/StopRouteList.tsx
Original file line number Diff line number Diff line change
@@ -1,15 +1,24 @@
import { Box, CircularProgress, List, SxProps, Theme } from "@mui/material";
import SuccinctTimeReport from "../home/SuccinctTimeReport";
import { getStopGroup } from "../../stopGroup";
import { useStopEtas } from "../../hooks/useStopEtas";
import { Company } from "hk-bus-eta";
import DbContext from "../../context/DbContext";
import { useContext, useMemo } from "react";

interface StopRouteListProps {
stops: Array<[Company, string]>; // [[co, stopId]]
routeId : string | undefined;
isFocus: boolean;
}

const StopRouteList = ({ stops, isFocus }: StopRouteListProps) => {
const stopEtas = useStopEtas({ stopKeys: stops, disabled: !isFocus });
const StopRouteList = ({ stops, routeId = undefined, isFocus }: StopRouteListProps) => {
const { db: { routeList, stopList } } = useContext(DbContext);
const stopGroup = useMemo(
() => getStopGroup({ routeList, stopList, stopKeys: stops, routeId }),
[routeList, stopList, stops, routeId]
);
const stopEtas = useStopEtas({ stopKeys: stopGroup, disabled: !isFocus });

if (stopEtas.length === 0) {
return (
Expand Down
1 change: 1 addition & 0 deletions src/components/bookmarked-stop/SwipeableStopList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ const SwipeableStopList = React.forwardRef<
<StopRouteList
key={`savedStops-${idx}`}
stops={stops}
routeId={undefined}
isFocus={getViewIdx() === idx}
/>
))}
Expand Down
5 changes: 3 additions & 2 deletions src/components/route-eta/StopDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,12 @@ import CollectionContext from "../../CollectionContext";

interface StopDialogProps {
open: boolean;
routeId : string;
stops: Array<[Company, string]>;
onClose: () => void;
}

const StopDialog = ({ open, stops, onClose }: StopDialogProps) => {
const StopDialog = ({ open, routeId, stops, onClose }: StopDialogProps) => {
const {
db: { stopList },
} = useContext(DbContext);
Expand Down Expand Up @@ -83,7 +84,7 @@ const StopDialog = ({ open, stops, onClose }: StopDialogProps) => {
</Box>
</DialogTitle>
<DialogContent>
<StopRouteList stops={stops} isFocus={true} />
<StopRouteList routeId={routeId} stops={stops} isFocus={true} />
</DialogContent>
</Dialog>
);
Expand Down
1 change: 1 addition & 0 deletions src/pages/RouteEta.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,7 @@ const RouteEta = () => {
/>
<StopDialog
open={isDialogOpen}
routeId={routeId}
stops={dialogStop}
onClose={handleCloseDialog}
/>
Expand Down
175 changes: 175 additions & 0 deletions src/stopGroup.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
import { Company } from "hk-bus-eta";
import type { RouteList, StopList, StopListEntry } from "hk-bus-eta";
import { getDistance, getBearing } from "./utils";

interface useStopGroupProps {
routeList : RouteList;
stopList : StopList;
stopKeys: Array<[Company, string]>;
routeId : string | undefined;
}
interface StopListEntryExtended extends StopListEntry {
id : string;
distance : number;
routeStops : RouteStopCoSeq[];
}
interface RouteStopCoSeq {
routeKey : string;
co : Company;
seq : number;
}

// stopKey in format "<co>|<stopId>", e.g., "lightRail|LR140"
export const getStopGroup = ({
routeList, stopList, stopKeys, routeId
}: useStopGroupProps) => {

// TODO: put these constants in AppContext user preference
const DISTANCE_THRESHOLD = 50; // in metres, will recursively find stops within this number of metres, so keep it as small as possible. Never choose larger than 300m.
const BEARING_THRESHOLD = 45; // in degrees (°), acceptable deviation to the left or right of current bearing
const STOP_LIST_LIMIT = 50; // max number of stops in a group, if more than that, the ETA list will be too long and meaningless

const getDistanceStops = (a : StopListEntry, b : StopListEntry) => {
return getDistance(a.location, b.location);
};
const getBearingStops = (a : StopListEntry, b : StopListEntry) => {
return getBearing(a.location, b.location);
};

const getAllRouteStopsBearings = (routeStops : RouteStopCoSeq[]) => {
// routeStop example: {"routeKey":"101+1+KENNEDY TOWN+KWUN TONG (YUE MAN SQUARE)","co":"ctb","seq":12}
return routeStops.map(routeStop => {
const { routeKey, co, seq } = routeStop;
const stopLength = routeList[routeKey].stops[co].length;
if(seq == stopLength - 1) { // last stop
// no next stop, hence no forward bearing, just use -1 as dummy value then discard it later
return -1;
}
const stopA = stopList[routeList[routeKey].stops[co][seq]];
const stopB = stopList[routeList[routeKey].stops[co][seq + 1]];
return getBearingStops(stopA, stopB);
}).filter(brng => brng !== -1);
};

const routeStops : RouteStopCoSeq[] = [];
if(routeId !== undefined) {
// StopDialog
let targetRouteStops = routeList[routeId].stops;
stopKeys.forEach(([co, stopId]) => {
let seq = targetRouteStops[co]?.indexOf(stopId) ?? -1;
if(seq != -1) {
routeStops.push({
routeKey : routeId,
co : co,
seq : seq
});
}
});
} else {
// SwipableStopList (saved stop list)
stopKeys.forEach(([co, stopId]) => {
Object.keys(routeList).forEach(routeKey => {
let seq = routeList[routeKey]?.stops[co]?.indexOf(stopId) ?? -1;
if(seq != -1) {
routeStops.push({
routeKey : routeKey,
co : co,
seq : seq
});
}
})
});
}

const bearingTargets = getAllRouteStopsBearings(routeStops);
const isBearingInRange = (bearing : number) => {
if(BEARING_THRESHOLD >= 180 || bearingTargets.length == 0) {
return true;
}
for(let i = 0; i < bearingTargets.length; ++i) {
let bearingMin = bearingTargets[i] - BEARING_THRESHOLD;
let bearingMax = bearingTargets[i] + BEARING_THRESHOLD;
if(bearingMin < 0)
bearingMin += 360;
if(bearingMax > 360)
bearingMax -= 360;
if((bearingMin <= bearingMax && bearingMin <= bearing && bearing <= bearingMax)
|| (bearingMin > bearingMax && (bearingMin <= bearing || bearing <= bearingMax))) // crossing 0/360° mark, eg min=340°,max=020°
return true;
}
return false;
};

const searchNearbyStops = (targetStopId : string, excludedStopIdList : string[]) => {
const targetStop = stopList[targetStopId];

return Object.keys(stopList).filter((stopId) => {
// find stops that are within DISTANCE_THRESHOLD metres of target stop and along similar direction
return getDistanceStops(targetStop, stopList[stopId]) <= DISTANCE_THRESHOLD &&
// filter out stops that have not been added into excludeList before
!excludedStopIdList.includes(stopId);
})
.reduce( (acc, stopId) => {
// get all the routes that has stop with this stopId
const _routeStops = Object.entries(routeList).map(([routeKey, routeListEntry]) => {
const stops = routeListEntry.stops ?? {};
const companies = Object.keys(stops) as Company[];
for(let co of companies) {
let stopPos = stops[co]?.indexOf(stopId) ?? -1;
if(stopPos > -1)
return { routeKey : routeKey, co : co, seq : stopPos } as RouteStopCoSeq;
}
return { routeKey : routeKey, co : 'ctb', seq : -1 } as RouteStopCoSeq; // use ctb as dummy value and seq = -1, will be discarded in next filter
})
.filter((_rs) => _rs.seq != -1);
// if any of the routes passing this stop is facing same direction (+/- BEARING_THRESHOLD), add the stop to the list
// Note: once the stop is added, other routes not facing same direction but passing this stop will also be shown in ETA (most commonly seen in railway lines)
const bearings = getAllRouteStopsBearings(_routeStops);
if(bearings.find(b => isBearingInRange(b)) !== undefined) {
const thisStop : StopListEntryExtended = {
...stopList[stopId],
id : stopId,
routeStops : _routeStops, // _routeStops.length must be > 0 here, as bearings.length must be > 0 to reach into this if-condition
distance : 0 // dummy value
};
acc.push(thisStop);
}
return acc
}, [] as Array<StopListEntryExtended>);
};

const stopGroup : Array<[Company, string]> = [];

let stopListEntries : StopListEntryExtended[] = [];

// recursively search for nearby stops within thresholds (distance and bearing)
// stop searching when no new stops are found within range, or when stop list is getting too large
stopKeys.forEach((stopKey) => {
const [, stopId] = stopKey; // [co, stopId]
stopListEntries = stopListEntries.concat(searchNearbyStops(stopId, stopListEntries.map((stop) => stop.id)));
for(let i = 0; i < stopListEntries.length; ++i) { // use traditional for-loop as the length keeps expanding
stopListEntries = stopListEntries.concat(searchNearbyStops(stopListEntries[i].id, stopListEntries.map((stop) => stop.id)));
if(stopListEntries.length >= STOP_LIST_LIMIT)
break;
}
});

// sort by distance from first stop in stopMap (stopKeys[0])
if(stopKeys.length > 0) {
let [, stopId] = stopKeys[0]; // [co, stopId] but don't use this co
stopListEntries = stopListEntries.map(stop => {
return {
... stop,
distance : getDistanceStops(stopList[stopId], stop)
};
}).sort((stopA, stopB) => {
return stopA.distance - stopB.distance
});
stopListEntries.forEach((stopListEntry) => {
if(stopListEntry.routeStops.length > 0)
stopGroup.push([stopListEntry.routeStops[0].co, stopListEntry.id]);
});
}

return stopGroup;
};
13 changes: 13 additions & 0 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,19 @@ export const getDistanceWithUnit = (distanceInMetre: number) => {
};
};

export const getBearing = (a: GeoLocation, b: GeoLocation) => {
// Reference: https://www.movable-type.co.uk/scripts/latlong.html
const φ1 = a.lat * Math.PI / 180; // φ, λ = lat, lon in radians
const φ2 = b.lat * Math.PI / 180;
const λ1 = a.lng * Math.PI / 180; // φ, λ = lat, lon in radians
const λ2 = b.lng * Math.PI / 180;
const y = Math.sin(λ2 - λ1) * Math.cos(φ2);
const x = Math.cos(φ1) * Math.sin(φ2) - Math.sin(φ1) * Math.cos(φ2) * Math.cos(λ2-λ1);
const θ = Math.atan2(y, x);
const brng = (θ * 180 / Math.PI + 360) % 360; // in degrees
return brng;
};
Comment on lines +37 to +48
Copy link

Choose a reason for hiding this comment

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

Optimize variable declarations and add comments for clarity.

The function is correctly implemented but can be optimized and improved for readability by combining the variable declarations and adding comments to explain the steps.

export const getBearing = (a: GeoLocation, b: GeoLocation) => {
  // Reference: https://www.movable-type.co.uk/scripts/latlong.html
- const φ1 = a.lat * Math.PI / 180; // φ, λ = lat, lon in radians
- const φ2 = b.lat * Math.PI / 180;
- const λ1 = a.lng * Math.PI / 180; // φ, λ = lat, lon in radians
- const λ2 = b.lng * Math.PI / 180;
- const y = Math.sin(λ2 - λ1) * Math.cos(φ2);
- const x = Math.cos(φ1) * Math.sin(φ2) - Math.sin(φ1) * Math.cos(φ2) * Math.cos(λ2-λ1);
- const θ = Math.atan2(y, x);
- const brng = (θ * 180 / Math.PI + 360) % 360; // in degrees
- return brng;
+ const φ1 = a.lat * Math.PI / 180, φ2 = b.lat * Math.PI / 180;
+ const λ1 = a.lng * Math.PI / 180, λ2 = b.lng * Math.PI / 180;
+ 
+ // Calculate the differences
+ const Δλ = λ2 - λ1;
+ const y = Math.sin(Δλ) * Math.cos(φ2);
+ const x = Math.cos(φ1) * Math.sin(φ2) - Math.sin(φ1) * Math.cos(φ2) * Math.cos(Δλ);
+ 
+ // Calculate the bearing
+ const θ = Math.atan2(y, x);
+ return (θ * 180 / Math.PI + 360) % 360; // in degrees
};
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const getBearing = (a: GeoLocation, b: GeoLocation) => {
// Reference: https://www.movable-type.co.uk/scripts/latlong.html
const φ1 = a.lat * Math.PI / 180; // φ, λ = lat, lon in radians
const φ2 = b.lat * Math.PI / 180;
const λ1 = a.lng * Math.PI / 180; // φ, λ = lat, lon in radians
const λ2 = b.lng * Math.PI / 180;
const y = Math.sin(λ2 - λ1) * Math.cos(φ2);
const x = Math.cos(φ1) * Math.sin(φ2) - Math.sin(φ1) * Math.cos(φ2) * Math.cos(λ2-λ1);
const θ = Math.atan2(y, x);
const brng = (θ * 180 / Math.PI + 360) % 360; // in degrees
return brng;
};
export const getBearing = (a: GeoLocation, b: GeoLocation) => {
// Reference: https://www.movable-type.co.uk/scripts/latlong.html
const φ1 = a.lat * Math.PI / 180, φ2 = b.lat * Math.PI / 180;
const λ1 = a.lng * Math.PI / 180, λ2 = b.lng * Math.PI / 180;
// Calculate the differences
const Δλ = λ2 - λ1;
const y = Math.sin(Δλ) * Math.cos(φ2);
const x = Math.cos(φ1) * Math.sin(φ2) - Math.sin(φ1) * Math.cos(φ2) * Math.cos(Δλ);
// Calculate the bearing
const θ = Math.atan2(y, x);
return (θ * 180 / Math.PI + 360) % 360; // in degrees
};


export const DEFAULT_GEOLOCATION: GeoLocation = {
lat: 22.302711,
lng: 114.177216,
Expand Down
Loading