Skip to content

Add vehicle-specific maximum speed profiles with configurable upper bounds#7383

Merged
DennisOSRM merged 6 commits intomasterfrom
claude/add-vehicle-specific-speed-profiles
Feb 27, 2026
Merged

Add vehicle-specific maximum speed profiles with configurable upper bounds#7383
DennisOSRM merged 6 commits intomasterfrom
claude/add-vehicle-specific-speed-profiles

Conversation

@Claude
Copy link
Copy Markdown
Contributor

@Claude Claude AI commented Feb 27, 2026 •
edited
Loading

No description provided.

@Claude Claude AI changed the title [WIP] Add vehicle specific upper speed to speed profiles Add vehicle-specific maximum speed profiles with configurable upper bounds Feb 27, 2026
@DennisOSRM
Copy link
Copy Markdown
Collaborator

DennisOSRM commented Feb 27, 2026

@claude[agent] start the implementation of the feature and associated tests for when the vehicle max speed is set and als when it is unset.

@Claude @DennisOSRM
Implement vehicle-specific maximum speed profiles with configurable u…
de6086a 
…pper bounds

Co-authored-by: DennisOSRM <1067895+DennisOSRM@users.noreply.github.com>
@Claude
Copy link
Copy Markdown
Contributor Author

Claude AI commented Feb 27, 2026

@claude[agent] start the implementation of the feature and associated tests for when the vehicle max speed is set and als when it is unset.

Implementation complete in commit de6086a. Added:

  • vehicle_max_speed property to car.lua (defaults to nil for backward compatibility)
  • WayHandlers.vehicle_speed_cap() handler that caps speeds using math.min()
  • 3 cucumber test scenarios covering both set (87 km/h cap) and unset (nil) cases

The feature is non-intrusive and preserves default car profile behavior when vehicle_max_speed is nil.

@Claude Claude AI changed the title Add vehicle-specific maximum speed profiles with configurable upper bounds Addressing PR comments Feb 27, 2026
@DennisOSRM

This comment was marked as resolved.

Sign in to view
@Claude @DennisOSRM
Fix vehicle_speed_cap handler placement and motorway test expectation
e84d00a 
Co-authored-by: DennisOSRM <1067895+DennisOSRM@users.noreply.github.com>
@DennisOSRM

This comment was marked as resolved.

Sign in to view
@Claude @DennisOSRM
Mark trunk maxspeed test with ~1% tolerance for rounding
58b70e0 
Co-authored-by: DennisOSRM <1067895+DennisOSRM@users.noreply.github.com>
@Claude

This comment was marked as resolved.

Sign in to view
@DennisOSRM
Copy link
Copy Markdown
Collaborator

DennisOSRM commented Feb 27, 2026

@claude[agent] the build is failing in an unexpected way. Analyse the failure and verify that it doesn't come from a rounding error in the calculation of the offset, which can exceed 1% depending on how it's calculated.

1) Scenario: Car - Vehicle maximum speed with forward/backward directions # features/car/maxspeed.feature:217
   ✔ Before # file:/home/runner/work/osrm-backend/osrm-backend/features/support/hooks.js:37
   ✔ Given the profile "car" # file:/home/runner/work/osrm-backend/osrm-backend/features/step_definitions/data.js:329
   ✔ Given a grid size of 1000 meters # file:/home/runner/work/osrm-backend/osrm-backend/features/step_definitions/data.js:35
   ✔ Given the profile file "car" initialized with # file:/home/runner/work/osrm-backend/osrm-backend/features/step_definitions/data.js:334
       """
       profile.vehicle_max_speed = 87
       """
   ✔ Given a grid size of 100 meters # file:/home/runner/work/osrm-backend/osrm-backend/features/step_definitions/data.js:35
   ✖ Then routability should be # file:/home/runner/work/osrm-backend/osrm-backend/features/step_definitions/routability.js:8
       | highway  | maxspeed | maxspeed:forward | maxspeed:backward | forw        | backw       |
       | motorway |          |                  |                   | 87 km/h     |             |
       | trunk    | 120      |                  |                   | 87 km/h ~1% | 87 km/h ~1% |
       | primary  | 80       |                  |                   | 64 km/h     | 64 km/h     |
       | primary  |          | 120              |                   | 87 km/h     | 65 km/h     |
       | primary  |          |                  | 120               | 65 km/h     | 87 km/h     |
       | primary  | 15       | 120              |                   | 87 km/h     | 12 km/h     |
   
       Tables were not identical:
       | highway  | maxspeed | maxspeed:forward | maxspeed:backward |     forw        |     backw       |
       | motorway |          |                  |                   |     87 km/h     |                 |
       | trunk    | 120      |                  |                   | (-) 87 km/h ~1% | (-) 87 km/h ~1% |
       | trunk    | 120      |                  |                   | (+) 88 km/h     | (+) 88 km/h     |
       | primary  | 80       |                  |                   |     64 km/h     |     64 km/h     |
       | primary  |          | 120              |                   |     87 km/h     |     65 km/h     |
       | primary  |          |                  | 120               |     65 km/h     |     87 km/h     |
       | primary  | 15       | 120              |                   |     87 km/h     |     12 km/h     |
   ✔ After # file:/home/runner/work/osrm-backend/osrm-backend/features/support/hooks.js:55
       Attachment (application/osm+xml): 217_car_vehicle_maximum_speed_with_forwardbackward_directions.osm
       Attachment (text/x.cucumber.log+plain)

1401 scenarios (1 failed, 5 skipped, 1395 passed)
7375 steps (1 failed, 44 skipped, 7330 passed)
1m01.410s (executing steps: 4m49.627s)

@Claude
Copy link
Copy Markdown
Contributor Author

Claude AI commented Feb 27, 2026

@claude[agent] the build is failing in an unexpected way. Analyse the failure and verify that it doesn't come from a rounding error in the calculation of the offset, which can exceed 1% depending on h...

Confirmed: the rounding error is 1.15%, which exceeds the 1% tolerance. The fuzzy matching uses strict less-than comparison (100 * ratio < percentage), so 1.15% fails the 1% check.

Three fix options:

  1. ~2% tolerance (recommended) - Minimal change, mathematically sound
  2. +-1 km/h absolute - Clear bounds (86-88 acceptable)
  3. Remove 100m grid step - Keep 1000m segments for lower relative error

Which approach would you like me to implement?

@Claude @DennisOSRM
Use absolute tolerance +-1 for trunk maxspeed test
4b01358 
Co-authored-by: DennisOSRM <1067895+DennisOSRM@users.noreply.github.com>
@Claude Claude AI changed the title Addressing PR comments Add vehicle-specific maximum speed profiles with configurable upper bounds Feb 27, 2026
@DennisOSRM DennisOSRM marked this pull request as ready for review February 27, 2026 18:56
@DennisOSRM DennisOSRM requested a review from Copilot February 27, 2026 19:16
Copilot AI reviewed Feb 27, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR implements vehicle-specific maximum speed caps for routing profiles, addressing issue #6979 where signposted maxspeeds could override lower vehicle-specific speed limits (e.g., trucks being routed at speeds exceeding their legal limits).

Changes:

  • Added optional vehicle_max_speed profile property to enable vehicle-specific speed caps
  • Implemented WayHandlers.vehicle_speed_cap() handler to enforce the maximum speed limit
  • Added comprehensive test coverage with three scenarios validating capping behavior, backward compatibility, and directional maxspeed interactions

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
profiles/lib/way_handlers.lua Adds vehicle_speed_cap handler that applies math.min() capping when vehicle_max_speed is set
profiles/car.lua Declares optional vehicle_max_speed property (defaults to nil) and inserts handler in correct execution order
features/car/maxspeed.feature Adds three test scenarios covering vehicle_max_speed enabled, disabled, and directional maxspeed cases

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@DennisOSRM
Copy link
Copy Markdown
Collaborator

DennisOSRM commented Feb 27, 2026

@claude[agent] Add a line to the changelog summarising this change. List issue #6979 as the reference.

@Claude @DennisOSRM
Add changelog entry for vehicle-specific maximum speed profiles
4c8e1c6 
Co-authored-by: DennisOSRM <1067895+DennisOSRM@users.noreply.github.com>
@Claude Claude AI changed the title Add vehicle-specific maximum speed profiles with configurable upper bounds Addressing PR comments Feb 27, 2026
@DennisOSRM DennisOSRM changed the title Addressing PR comments Add vehicle-specific maximum speed profiles with configurable upper bounds Feb 27, 2026
@DennisOSRM DennisOSRM merged commit a74567b into master Feb 27, 2026
23 checks passed
@DennisOSRM DennisOSRM deleted the claude/add-vehicle-specific-speed-profiles branch February 27, 2026 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

@DennisOSRM DennisOSRM

@Claude Claude

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Claude @DennisOSRM