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

routing: Support alternatives for single-mode routing calculations #1001

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ const calculateRoute = async (
routingMode: RoutingMode
): Promise<RouteResults> => {
// FIXME This code path will use a fake socket route to do the calculation. Move this code to the backend too
return await getRouteByMode(od[0], od[1], routingMode);
return await getRouteByMode(od[0], od[1], routingMode, routingAttributes);
};

export class Routing {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ describe('Routing Calculations', () => {

expect(mockedTrRouting.mockRouteFunction).toHaveBeenCalledTimes(1);
expect(mockedRoutingUtils.mockGetRouteByMode).toHaveBeenCalledTimes(1);
expect(mockedRoutingUtils.mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'walking');
expect(mockedRoutingUtils.mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'walking', attributes);

expect(Object.keys(result)).toEqual(['transit', 'walking']);
const transitResult = result['transit'] as TransitRoutingResultData;
Expand All @@ -106,7 +106,7 @@ describe('Routing Calculations', () => {

expect(mockedTrRouting.mockRouteFunction).toHaveBeenCalledTimes(1);
expect(mockedRoutingUtils.mockGetRouteByMode).toHaveBeenCalledTimes(1);
expect(mockedRoutingUtils.mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'walking');
expect(mockedRoutingUtils.mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'walking', attributes);

expect(Object.keys(result)).toEqual(['transit', 'walking']);
const transitResult = result['transit'] as TransitRoutingResultData;
Expand Down Expand Up @@ -137,7 +137,7 @@ describe('Routing Calculations', () => {

expect(mockedTrRouting.mockRouteFunction).toHaveBeenCalledTimes(1);
expect(mockedRoutingUtils.mockGetRouteByMode).toHaveBeenCalledTimes(1);
expect(mockedRoutingUtils.mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'walking');
expect(mockedRoutingUtils.mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'walking', attributes);

expect(transitResult.paths.length).toEqual(1);

Expand Down Expand Up @@ -166,7 +166,7 @@ describe('Routing Calculations', () => {

expect(mockedTrRouting.mockRouteFunction).toHaveBeenCalledTimes(1);
expect(mockedRoutingUtils.mockGetRouteByMode).toHaveBeenCalledTimes(1);
expect(mockedRoutingUtils.mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'walking');
expect(mockedRoutingUtils.mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'walking', attributes);

expect(transitResult.paths.length).toEqual(2);

Expand All @@ -183,7 +183,7 @@ describe('Routing Calculations', () => {

expect(mockedTrRouting.mockRouteFunction).toHaveBeenCalledTimes(1);
expect(mockedRoutingUtils.mockGetRouteByMode).toHaveBeenCalledTimes(1);
expect(mockedRoutingUtils.mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'walking');
expect(mockedRoutingUtils.mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'walking', attributes);

expect(Object.keys(result)).toEqual(['transit', 'walking']);
const transitResult = result['transit'] as TransitRoutingResultData;
Expand All @@ -205,9 +205,9 @@ describe('Routing Calculations', () => {

expect(mockedTrRouting.mockRouteFunction).toHaveBeenCalledTimes(1);
expect(mockedRoutingUtils.mockGetRouteByMode).toHaveBeenCalledTimes(3);
expect(mockedRoutingUtils.mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'walking');
expect(mockedRoutingUtils.mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'cycling');
expect(mockedRoutingUtils.mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'rail');
expect(mockedRoutingUtils.mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'walking', attributes);
expect(mockedRoutingUtils.mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'cycling', attributes);
expect(mockedRoutingUtils.mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'rail', attributes);

expect(Object.keys(result)).toEqual(routingModes);
for (let i = 0; i < routingModes.length; i++) {
Expand All @@ -228,9 +228,9 @@ describe('Routing Calculations', () => {

expect(mockedTrRouting.mockRouteFunction).toHaveBeenCalledTimes(1);
expect(mockedRoutingUtils.mockGetRouteByMode).toHaveBeenCalledTimes(3);
expect(mockedRoutingUtils.mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'walking');
expect(mockedRoutingUtils.mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'cycling');
expect(mockedRoutingUtils.mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'rail');
expect(mockedRoutingUtils.mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'walking', attributes);
expect(mockedRoutingUtils.mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'cycling', attributes);
expect(mockedRoutingUtils.mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'rail', attributes);

expect(Object.keys(result)).toEqual(routingModes);
for (let i = 0; i < routingModes.length; i++) {
Expand Down
8 changes: 6 additions & 2 deletions packages/chaire-lib-backend/src/utils/osrm/OSRMMode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import osrm from 'osrm'; // Types from the osrm API definition see https://githu
import * as RoutingService from 'chaire-lib-common/lib/services/routing/RoutingService';
import * as Status from 'chaire-lib-common/lib/utils/Status';
import { transitionRouteOptions, transitionMatchOptions } from 'chaire-lib-common/lib/api/OSRMRouting';
import { TripRoutingQueryAttributes } from 'chaire-lib-common/lib/services/routing/types';

//TODO replace this fetch-retry library with one compatible with TS
const fetch = require('@zeit/fetch-retry')(require('node-fetch'));
Expand Down Expand Up @@ -66,14 +67,17 @@ class OSRMMode {
}
}

public async route(params: transitionRouteOptions): Promise<Status.Status<osrm.RouteResults>> {
public async route(
params: transitionRouteOptions,
routingAttributes?: TripRoutingQueryAttributes
): Promise<Status.Status<osrm.RouteResults>> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of adding a new parameter, we already have the params object which include the boolean for alternatives. We should probably fill it in there before calling this function.

this.validateParamMode(params.mode);
//TODO validate that points is not empty?? Does it make sense ?

// Fill in default values if they are not defined
const parameters = _merge(
{
alternatives: false,
alternatives: routingAttributes?.withAlternatives === true ? true : false,
steps: false,
annotations: false, // use "nodes" to get all osm nodes ids
geometries: 'geojson',
Expand Down
8 changes: 6 additions & 2 deletions packages/chaire-lib-backend/src/utils/osrm/OSRMService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import * as RoutingService from 'chaire-lib-common/lib/services/routing/RoutingS
import { RoutingMode, routingModes } from 'chaire-lib-common/lib/config/routingModes';

import OSRMMode from './OSRMMode';
import { TripRoutingQueryAttributes } from 'chaire-lib-common/lib/services/routing/types';

class OSRMService {
private _modeRegistry: Record<RoutingMode, OSRMMode>;
Expand All @@ -22,8 +23,11 @@ class OSRMService {
this._modeRegistry = {} as Record<RoutingMode, OSRMMode>;
}

public route(parameters: transitionRouteOptions): Promise<Status.Status<osrm.RouteResults>> {
return this.getMode(parameters.mode).route(parameters);
public route(
parameters: transitionRouteOptions,
routingAttributes?: TripRoutingQueryAttributes
): Promise<Status.Status<osrm.RouteResults>> {
return this.getMode(parameters.mode).route(parameters, routingAttributes);
}

public match(parameters: transitionMatchOptions): Promise<Status.Status<osrm.MatchResults>> {
Expand Down
2 changes: 2 additions & 0 deletions packages/chaire-lib-common/src/api/OSRMRouting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/osrm/index.
*/

import { RoutingMode } from '../config/routingModes';
import { TripRoutingQueryAttributes } from '../services/routing/types';

export interface transitionMatchOptions extends osrm.MatchOptions {
mode: RoutingMode;
Expand All @@ -21,4 +22,5 @@ export interface transitionMatchOptions extends osrm.MatchOptions {
export interface transitionRouteOptions extends osrm.RouteOptions {
mode: RoutingMode;
points: GeoJSON.Feature<GeoJSON.Point>[];
routingAttributes?: TripRoutingQueryAttributes;
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import * as RoutingService from './RoutingService';
import * as turf from '@turf/turf';
import _cloneDeep from 'lodash/cloneDeep';
import { TripRoutingQueryAttributes } from './types';

/**
* This class provides routing services using manual routing. This mode will always return return results
Expand Down Expand Up @@ -82,7 +83,10 @@ export default class ManualRoutingService extends RoutingService.default {
return this.routeToMapResults(routingResult);
}

public async route(params: RoutingService.MapMatchParameters): Promise<RoutingService.RouteResults> {
public async route(
params: RoutingService.MapMatchParameters,
_routingAttributes?: TripRoutingQueryAttributes
): Promise<RoutingService.RouteResults> {
const routingResult = await this.directRouting(params.points.features, params.defaultRunningSpeed, true);

return routingResult;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { _isBlank } from '../../utils/LodashExtensions';
import Preferences from '../../config/Preferences';
import * as Status from '../../utils/Status';
import * as OSRMRoutingAPI from '../../api/OSRMRouting';
import { TripRoutingQueryAttributes } from './types';

const defaultRoutingRadiusMeters = 20;

Expand Down Expand Up @@ -116,13 +117,15 @@ export default class OSRMRoutingService extends RoutingService.default {
}

private async callOsrmRoute(
params: OSRMRoutingAPI.transitionRouteOptions
params: OSRMRoutingAPI.transitionRouteOptions,
routingAttributes?: TripRoutingQueryAttributes
): Promise<Status.Status<osrm.RouteResults>> {
//console.log("osrm params", params);
return new Promise((resolve) => {
serviceLocator.socketEventManager.emit(
'service.osrmRouting.route',
params,
routingAttributes,
(routingResult: Status.Status<osrm.RouteResults>) => {
//console.log('routingResult', routingResult);
resolve(routingResult);
Expand Down Expand Up @@ -181,17 +184,24 @@ export default class OSRMRoutingService extends RoutingService.default {
return this.processMapMatchingResult(routingResult);
}

public async route(params: RoutingService.MapMatchParameters): Promise<RoutingService.RouteResults> {
const routingResult = await this.callOsrmRoute({
mode: params.mode,
points: params.points.features,
steps: params.showSteps || false,
overview: params.overview === 'simplified' ? 'simplified' : params.overview === 'false' ? 'false' : 'full',
annotations: _isBlank(params.annotations) ? true : params.annotations,
// gaps : 'ignore',
continue_straight:
Preferences.current.osrmRouting.useContinueStraightForMapMatching === true ? true : undefined // see comment in chaire-lib-common/lib/config/defaultPreferences.config.js
});
public async route(
params: RoutingService.MapMatchParameters,
routingAttributes?: TripRoutingQueryAttributes
): Promise<RoutingService.RouteResults> {
const routingResult = await this.callOsrmRoute(
{
mode: params.mode,
points: params.points.features,
steps: params.showSteps || false,
overview:
params.overview === 'simplified' ? 'simplified' : params.overview === 'false' ? 'false' : 'full',
annotations: _isBlank(params.annotations) ? true : params.annotations,
// gaps : 'ignore',
continue_straight:
Preferences.current.osrmRouting.useContinueStraightForMapMatching === true ? true : undefined // see comment in chaire-lib-common/lib/config/defaultPreferences.config.js
},
routingAttributes
);
return this.processRoutingResult(routingResult);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/
import GeoJSON from 'geojson';
import { RoutingMode } from '../../config/routingModes';
import { TripRoutingQueryAttributes } from './types';

export interface MapMatchParameters {
mode: RoutingMode;
Expand Down Expand Up @@ -118,9 +119,11 @@ export interface RoutingService {
* route. Various implementations may use them differently.
*
* @param params
* @param routingAttributes Additional attributes related to the trip
* calculation
* @returns Route matching result
*/
route(params: MapMatchParameters): Promise<RouteResults>;
route(params: MapMatchParameters, routingAttributes?: TripRoutingQueryAttributes): Promise<RouteResults>;

/**
* Compute the durations and distances of the fastest route between the
Expand Down
27 changes: 22 additions & 5 deletions packages/chaire-lib-common/src/services/routing/RoutingUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,33 @@
import { RoutingMode } from '../../config/routingModes';
import { default as routingServiceManager, DEFAULT_ROUTING_ENGINE } from './RoutingServiceManager';
import { RouteResults } from './RoutingService';
import { TripRoutingQueryAttributes } from './types';

/**
* Calls the routing service to get a route between two points for a single mode
* of calculation
* @param { GeoJSON.Feature<GeoJSON.Point> } origin The origin point feature
* @param { GeoJSON.Feature<GeoJSON.Point> } destination The destination point
* feature
* @param { RoutingMode }mode The mode of routing calculation, defaults to
* 'walking'
* @param { TripRoutingQueryAttributes | undefined } routingAttributes Optional
* additional routing attributes for this trip calculation
* @returns
*/
export const getRouteByMode = async (
origin: GeoJSON.Feature<GeoJSON.Point>,
destination: GeoJSON.Feature<GeoJSON.Point>,
mode: RoutingMode = 'walking'
mode: RoutingMode = 'walking',
routingAttributes?: TripRoutingQueryAttributes
): Promise<RouteResults> => {
const routingService = routingServiceManager.getRoutingServiceForEngine(DEFAULT_ROUTING_ENGINE);
const routingResult = await routingService.route({
mode,
points: { type: 'FeatureCollection', features: [origin, destination] }
});
const routingResult = await routingService.route(
{
mode,
points: { type: 'FeatureCollection', features: [origin, destination] }
},
routingAttributes
);
return routingResult;
};
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,16 @@ import serviceLocator from '../../../utils/ServiceLocator';
import TestUtils from '../../../test/TestUtils';
import { EventEmitter } from 'events';
import * as Status from '../../../utils/Status';
import { TripRoutingQueryAttributes } from '../types';


let eventManager;
let routingService;
// Setup a fresh routingService and eventManager for each tests
const routingService = new OSRMRoutingService();
const eventManager = new EventEmitter();
serviceLocator.socketEventManager = eventManager;

beforeEach(() => {
// Setup a fresh routingService and eventManager for each tests
routingService = new OSRMRoutingService();
eventManager = new EventEmitter();
serviceLocator.socketEventManager = eventManager;

jest.clearAllMocks();
eventManager.removeAllListeners();
});

test('Table from', async () => {
Expand Down Expand Up @@ -84,13 +83,41 @@ test('Route', async () => {
// TODO fill the routes fields with something
const expectedResponse = {waypoints: [point1.geometry, point2.geometry], routes: []};
const osrmResponse = {waypoints: [osrmWaypoint1, osrmWaypoint2], routes: []};
const mockRoute = jest.fn().mockImplementation((params, callback) => callback(Status.createOk(osrmResponse)));
const mockRoute = jest.fn().mockImplementation((params, routingAttributes, callback) => callback(Status.createOk(osrmResponse)));
eventManager.on('service.osrmRouting.route', mockRoute);


const routeResult = await routingService.route({mode: 'walking', points: { type: 'FeatureCollection', features: stubPlaces}});
expect(mockRoute).toHaveBeenCalledTimes(1);
expect(mockRoute).toHaveBeenCalledWith({mode: 'walking', points: stubPlaces, annotations: true, steps: false, continue_straight: undefined, overview: "full", }, expect.anything())
expect(mockRoute).toHaveBeenCalledWith({mode: 'walking', points: stubPlaces, annotations: true, steps: false, continue_straight: undefined, overview: "full", }, undefined, expect.anything())
expect(routeResult).toEqual(expectedResponse);
});

test('Route with routingAttributes', async () => {
const point1 = TestUtils.makePoint([-73.1, 45.1]);
const point2 =TestUtils.makePoint([-73.1, 44.9]);

const osrmWaypoint1 = {location: [-73.1, 45.1]}
const osrmWaypoint2 = {location: [-73.1, 44.9]}

// TODO fill the routes fields with something
const expectedResponse = {waypoints: [point1.geometry, point2.geometry], routes: []};
const osrmResponse = {waypoints: [osrmWaypoint1, osrmWaypoint2], routes: []};
const mockRoute = jest.fn().mockImplementation((params, routingAttributes, callback) => callback(Status.createOk(osrmResponse)));
eventManager.on('service.osrmRouting.route', mockRoute);

const routingAttributes: TripRoutingQueryAttributes = {
routingModes: ['walking'],
withAlternatives: true,
timeSecondsSinceMidnight: 0,
timeType: 'arrival',
originGeojson: stubPlaces[0],
destinationGeojson: stubPlaces[1]
};

const routeResult = await routingService.route({mode: 'walking', points: { type: 'FeatureCollection', features: stubPlaces}}, routingAttributes);
expect(mockRoute).toHaveBeenCalledTimes(1);
expect(mockRoute).toHaveBeenCalledWith({mode: 'walking', points: stubPlaces, annotations: true, steps: false, continue_straight: undefined, overview: "full", }, routingAttributes, expect.anything())
expect(routeResult).toEqual(expectedResponse);
});

Expand All @@ -109,7 +136,7 @@ test('Route fail', async () => {
haveThrown = true;
}
expect(mockRoute).toHaveBeenCalledTimes(1);
expect(mockRoute).toHaveBeenCalledWith({mode: 'walking', points: stubPlaces, annotations: true, steps: false, continue_straight: undefined, overview: "full", }, expect.anything())
expect(mockRoute).toHaveBeenCalledWith({mode: 'walking', points: stubPlaces, annotations: true, steps: false, continue_straight: undefined, overview: "full", }, undefined, expect.anything())
expect(routeResult).toBeUndefined();
expect(haveThrown).toBe(true);
});
Loading
Loading