Skip to content

Commit

Permalink
routing: Support alternatives for single-mode routing calculations
Browse files Browse the repository at this point in the history
Add a `RouteParameters` type to replace the `MapMatchParameters` in the
`route` function, as those 2 parameters types are not expected to be the
same and this clarifies the difference. Add a `withAlternatives`
property to this new type.

The `TripRoutingQueryAttributes` object is added as a parameter to the
`getRouteByMode` function call, which allows to set the
`withAlternatives` property of the `RouteParameters` object.
  • Loading branch information
tahini committed Dec 18, 2024
1 parent 39c63e8 commit 7fab3e2
Show file tree
Hide file tree
Showing 12 changed files with 85 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,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 @@ -88,7 +88,7 @@ describe('Routing Calculations', () => {

expect(mockedRouteFunction).toHaveBeenCalledTimes(1);
expect(mockGetRouteByMode).toHaveBeenCalledTimes(1);
expect(mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'walking');
expect(mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'walking', attributes);

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

expect(mockedRouteFunction).toHaveBeenCalledTimes(1);
expect(mockGetRouteByMode).toHaveBeenCalledTimes(1);
expect(mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'walking');
expect(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 @@ -146,7 +146,7 @@ describe('Routing Calculations', () => {

expect(mockedRouteFunction).toHaveBeenCalledTimes(1);
expect(mockGetRouteByMode).toHaveBeenCalledTimes(1);
expect(mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'walking');
expect(mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'walking', attributes);

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

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

expect(mockedRouteFunction).toHaveBeenCalledTimes(1);
expect(mockGetRouteByMode).toHaveBeenCalledTimes(1);
expect(mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'walking');
expect(mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'walking', attributes);

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

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

expect(mockedRouteFunction).toHaveBeenCalledTimes(1);
expect(mockGetRouteByMode).toHaveBeenCalledTimes(1);
expect(mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'walking');
expect(mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'walking', attributes);

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

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

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

expect(mockedRouteFunction).toHaveBeenCalledTimes(1);
expect(mockGetRouteByMode).toHaveBeenCalledTimes(3);
expect(mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'walking');
expect(mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'cycling');
expect(mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'rail');
expect(mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'walking', attributes);
expect(mockGetRouteByMode).toHaveBeenCalledWith(attributes.originGeojson, attributes.destinationGeojson, 'cycling', attributes);
expect(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: 4 additions & 4 deletions packages/chaire-lib-backend/src/utils/osrm/OSRMMode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,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 { TransitionRouteOptions, TransitionMatchOptions } from 'chaire-lib-common/lib/api/OSRMRouting';

//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 +66,14 @@ class OSRMMode {
}
}

public async route(params: transitionRouteOptions): Promise<Status.Status<osrm.RouteResults>> {
public async route(params: TransitionRouteOptions): Promise<Status.Status<osrm.RouteResults>> {
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: params?.withAlternatives === true ? true : false,
steps: false,
annotations: false, // use "nodes" to get all osm nodes ids
geometries: 'geojson',
Expand Down Expand Up @@ -102,7 +102,7 @@ class OSRMMode {
}
}

public async match(params: transitionMatchOptions): Promise<Status.Status<osrm.MatchResults>> {
public async match(params: TransitionMatchOptions): Promise<Status.Status<osrm.MatchResults>> {
this.validateParamMode(params.mode);

//TODO validate that points is not empty?? Does it make sense ?
Expand Down
6 changes: 3 additions & 3 deletions packages/chaire-lib-backend/src/utils/osrm/OSRMService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import osrm from 'osrm';

import * as Status from 'chaire-lib-common/lib/utils/Status';
import { transitionRouteOptions, transitionMatchOptions } from 'chaire-lib-common/lib/api/OSRMRouting';
import { TransitionRouteOptions, TransitionMatchOptions } from 'chaire-lib-common/lib/api/OSRMRouting';
import * as RoutingService from 'chaire-lib-common/lib/services/routing/RoutingService';
import { RoutingMode } from 'chaire-lib-common/lib/config/routingModes';

Expand All @@ -20,11 +20,11 @@ class OSRMService {
this._modeRegistry = {} as Record<RoutingMode, OSRMMode>;
}

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

public match(parameters: transitionMatchOptions): Promise<Status.Status<osrm.MatchResults>> {
public match(parameters: TransitionMatchOptions): Promise<Status.Status<osrm.MatchResults>> {
return this.getMode(parameters.mode).match(parameters);
}

Expand Down
7 changes: 5 additions & 2 deletions packages/chaire-lib-common/src/api/OSRMRouting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,16 @@ 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 {
export interface TransitionMatchOptions extends osrm.MatchOptions {
mode: RoutingMode;
points: GeoJSON.Feature<GeoJSON.Point>[];
}

export interface transitionRouteOptions extends osrm.RouteOptions {
export interface TransitionRouteOptions extends osrm.RouteOptions {
mode: RoutingMode;
points: GeoJSON.Feature<GeoJSON.Point>[];
routingAttributes?: TripRoutingQueryAttributes;
withAlternatives?: boolean;
}
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,7 @@ 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.RouteParameters): 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 @@ -101,7 +101,7 @@ export default class OSRMRoutingService extends RoutingService.default {
}

private async callOsrmMap(
params: OSRMRoutingAPI.transitionMatchOptions
params: OSRMRoutingAPI.TransitionMatchOptions
): Promise<Status.Status<osrm.MatchResults>> {
//console.log("osrm params", params);
return new Promise((resolve) => {
Expand All @@ -117,7 +117,7 @@ export default class OSRMRoutingService extends RoutingService.default {
}

private async callOsrmRoute(
params: OSRMRoutingAPI.transitionRouteOptions
params: OSRMRoutingAPI.TransitionRouteOptions
): Promise<Status.Status<osrm.RouteResults>> {
//console.log("osrm params", params);
return new Promise((resolve) => {
Expand Down Expand Up @@ -182,7 +182,7 @@ export default class OSRMRoutingService extends RoutingService.default {
return this.processMapMatchingResult(routingResult);
}

public async route(params: RoutingService.MapMatchParameters): Promise<RoutingService.RouteResults> {
public async route(params: RoutingService.RouteParameters): Promise<RoutingService.RouteResults> {
const routingResult = await this.callOsrmRoute({
mode: params.mode,
points: params.points.features,
Expand All @@ -192,7 +192,8 @@ export default class OSRMRoutingService extends RoutingService.default {
// gaps : 'ignore',
continue_straight:
// TODO: Migrate this value from preferences to config like the osrm modes. See issue #1140
Preferences.current.osrmRouting.useContinueStraightForMapMatching === true ? true : undefined // see comment in chaire-lib-common/lib/config/defaultPreferences.config.js
Preferences.current.osrmRouting.useContinueStraightForMapMatching === true ? true : undefined, // see comment in chaire-lib-common/lib/config/defaultPreferences.config.js
withAlternatives: params.withAlternatives === true ? true : false
});
return this.processRoutingResult(routingResult);
}
Expand Down
23 changes: 21 additions & 2 deletions packages/chaire-lib-common/src/services/routing/RoutingService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
*/
import GeoJSON from 'geojson';
import { RoutingMode } from '../../config/routingModes';
import { TripRoutingQueryAttributes } from './types';

// FIXME Make sure these parameters are those that apply to map matching as the name implies
export interface MapMatchParameters {
mode: RoutingMode;
/**
Expand All @@ -22,6 +24,23 @@ export interface MapMatchParameters {
overview?: string;
}

// FIXME See if those parameters can/should be merged with TripRoutingQueryAttributes, using this one instead
export interface RouteParameters {
mode: RoutingMode;
/**
* The collection of features by which the path should pass.
*/
points: GeoJSON.FeatureCollection<GeoJSON.Point>;
defaultRunningSpeed?: number | undefined | null; // we need default running speed for manual routing
/**
* Whether to return steps for each route
*/
showSteps?: boolean;
annotations?: boolean;
overview?: string;
withAlternatives?: boolean;
}

export interface TableFromParameters {
mode: RoutingMode;
origin: GeoJSON.Feature<GeoJSON.Point>;
Expand Down Expand Up @@ -117,10 +136,10 @@ export interface RoutingService {
* The array of 'points' parameter can contain all the features in the
* route. Various implementations may use them differently.
*
* @param params
* @param params The query parameters.
* @returns Route matching result
*/
route(params: MapMatchParameters): Promise<RouteResults>;
route(params: RouteParameters): Promise<RouteResults>;

/**
* Compute the durations and distances of the fastest route between the
Expand Down
19 changes: 17 additions & 2 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,31 @@
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] }
points: { type: 'FeatureCollection', features: [origin, destination] },
withAlternatives: routingAttributes?.withAlternatives || false
});
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 @@ -87,14 +86,12 @@ test('Route', async () => {
const mockRoute = jest.fn().mockImplementation((params, callback) => callback(Status.createOk(osrmResponse)));
eventManager.on('service.osrmRouting.route', mockRoute);


const routeResult = await routingService.route({mode: 'walking', points: { type: 'FeatureCollection', features: stubPlaces}});
const routeResult = await routingService.route({ mode: 'walking', points: { type: 'FeatureCollection', features: stubPlaces}, withAlternatives: 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", withAlternatives: true }, expect.any(Function))
expect(routeResult).toEqual(expectedResponse);
});


test('Route fail', async () => {

const mockRoute = jest.fn().mockImplementation((params, callback) => callback(Status.createError("BAD RESULTS")));
Expand All @@ -109,7 +106,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", withAlternatives: false }, expect.any(Function))
expect(routeResult).toBeUndefined();
expect(haveThrown).toBe(true);
});
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { ExecutableJob } from '../../services/executableJob/ExecutableJob';
import jobsDbQueries from '../../models/db/jobs.db.queries';
import Users from 'chaire-lib-backend/lib/services/users/users';
import { TransitAccessibilityMapCalculator } from '../../services/accessibilityMap/TransitAccessibilityMapCalculator';
import { TripRoutingQueryAttributes } from 'chaire-lib-common/src/services/routing/types';

const socketStub = new EventEmitter();
routes(socketStub, 1);
Expand Down Expand Up @@ -74,7 +75,8 @@ describe('osrm service routes', () => {
// Route and match parameters are the same here for this test
const routeParameters = {
mode: 'walking',
points: [TestUtils.makePoint([-73, 45]), TestUtils.makePoint([ -73.1, 45.1 ])]
points: [TestUtils.makePoint([-73, 45]), TestUtils.makePoint([ -73.1, 45.1 ])],
withAlternatives: true
};

test('Route correctly', (done) => {
Expand Down
Loading

0 comments on commit 7fab3e2

Please sign in to comment.