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 with svc-gis & Refactor #39

Merged
merged 7 commits into from
Oct 11, 2023

Conversation

amsmith-pro
Copy link
Contributor

@amsmith-pro amsmith-pro commented Sep 21, 2023

⚠️ This is not the R3 final review

Major refactor:

  • API handlers moved to their own files
  • Handle all unwraps
  • Removed all original router_utils
  • Timeslot approach
    • First identify matching timeslots accommodating the required loading and unloading times, with a call to best_path to determine the true distance (around no fly zones) to the target
    • Then pick an aircraft, using best path for each deadhead flight (pre and post)
    • storage record for flight plans does not indicate a block of time, or time needed to load/unload. Should be included in R4, and scheduler will be updated to include it

Some compromises in R3:

  • By default, send the aircraft back to the vertiport it was initially parked at after completion of flight
  • This will map better to hangars and rest stops in R4
    • In R3 the starting vertiport of the aircraft will be their "hangar" and they can return to any pad there
    • Should be the default for aircraft that don't have any immediate plans; go home, don't occupy a vertipad needlessly
    • Currently no way to book an indefinite term timeslot for a vertiport, which is what the return home will do
  • In R4 we'll introduce chaining, where an aircraft can cancel its return home in favor of another flight request, if it is predicted to have sufficient charge

@cla-bot cla-bot bot added the cla-signed label Sep 21, 2023
@github-actions
Copy link

github-actions bot commented Sep 21, 2023

This PR will generate the following release notes when merged:

Release

✨ Features

  • use svc-gis for routing (f9d5298)

🐛 Fixes

  • schedule bug (f7bf638)
  • unit test confirm cancel datetime bug (40bf0b6)

🔥 Refactorings

  • vertipad timeslots and best path (86d83ed)

🛠 Maintenance

@github-actions
Copy link

github-actions bot commented Sep 21, 2023

🪲 Unit Testing Coverage 🔎

|| Tested/Total Lines:
|| client-grpc/src/client.rs: 39/39
|| client-grpc/tests/integration_test.rs: 37/37
|| server/src/config.rs: 52/56
|| server/src/grpc/api/cancel_itinerary.rs: 34/40
|| server/src/grpc/api/confirm_itinerary.rs: 90/94
|| server/src/grpc/api/mod.rs: 32/45
|| server/src/grpc/api/query_flight.rs: 133/184
|| server/src/grpc/client.rs: 38/38
|| server/src/grpc/server.rs: 28/48
|| server/src/lib.rs: 4/12
|| server/src/router/flight_plan.rs: 63/90
|| server/src/router/itinerary.rs: 422/456
|| server/src/router/mod.rs: 21/31
|| server/src/router/schedule.rs: 236/266
|| server/src/router/vehicle.rs: 208/232
|| server/src/router/vertiport.rs: 329/363
|| server/src/test_util.rs: 202/206
||
87.97% coverage, 1968/2237 lines covered

👉 View in Coveralls.io 👈

Copy link
Contributor

@owlot owlot left a comment

Choose a reason for hiding this comment

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

I know this is still in Draft, but I quickly went over it already and marked some things I noticed

server/src/config.rs Outdated Show resolved Hide resolved
server/src/grpc/api/mod.rs Outdated Show resolved Hide resolved
server/src/grpc/api/mod.rs Outdated Show resolved Hide resolved
server/src/grpc/api/mod.rs Outdated Show resolved Hide resolved
proto/grpc.proto Outdated Show resolved Hide resolved
@amsmith-pro amsmith-pro force-pushed the am-smith/dw-1103/use-svc-gis-for-routin branch from 3a2b13e to 432f3f6 Compare October 2, 2023 23:05
@amsmith-pro amsmith-pro force-pushed the am-smith/dw-1103/use-svc-gis-for-routin branch from 432f3f6 to 86d83ed Compare October 2, 2023 23:05
@arrow-github-bot arrow-github-bot force-pushed the am-smith/dw-1103/use-svc-gis-for-routin branch from 86d83ed to 432f3f6 Compare October 2, 2023 23:05
@amsmith-pro amsmith-pro force-pushed the am-smith/dw-1103/use-svc-gis-for-routin branch 2 times, most recently from 297efa9 to b98f89a Compare October 2, 2023 23:20
@amsmith-pro amsmith-pro force-pushed the am-smith/dw-1103/use-svc-gis-for-routin branch 2 times, most recently from 01f0834 to 57d2da4 Compare October 3, 2023 04:05
@amsmith-pro amsmith-pro marked this pull request as ready for review October 3, 2023 04:05
@amsmith-pro amsmith-pro requested a review from a team as a code owner October 3, 2023 04:05
@amsmith-pro amsmith-pro force-pushed the am-smith/dw-1103/use-svc-gis-for-routin branch from 57d2da4 to 40506a2 Compare October 3, 2023 04:13
@amsmith-pro amsmith-pro force-pushed the am-smith/dw-1103/use-svc-gis-for-routin branch from 8195c06 to 25b5e6d Compare October 5, 2023 23:22
@amsmith-pro amsmith-pro force-pushed the am-smith/dw-1103/use-svc-gis-for-routin branch from 25b5e6d to 103b258 Compare October 6, 2023 00:00
@amsmith-pro
Copy link
Contributor Author

leg_1
leg_2
leg_3

@amsmith-pro
Copy link
Contributor Author

@owlot Ready for review

@amsmith-pro amsmith-pro requested a review from owlot October 6, 2023 00:40
Copy link
Contributor

@owlot owlot left a comment

Choose a reason for hiding this comment

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

I pulled the branch locally and ran make rust-test to do some validations. Unfortunately the test_confirm_and_cancel_itinerary was failing for me.
To confirm, I re-ran the rust-tests action, and the tests are failing there now as well.

I suspect it has something to do with the fact that the current date is being used for this test case while the test data is injected with fixed dates.

log information with debug enabled:

lotjuh@roo:~/git/Arrow-air/svc-scheduler:2>cat server/logs/tests.log | jq -c '. | { message: .message, thread_id: .thread_id, date: .time}' | grep test_confirm_and_cancel_itinerary
{"message":"(test_confirm_and_cancel_itinerary) start","thread_id":140706676972320,"date":"2023-10-09T14:02:25.521723157+00:00"}
{"message":"(test_confirm_and_cancel_itinerary) query_flight result: Ok(\n    Response {\n        metadata: MetadataMap {\n            headers: {},\n        },\n        message: QueryFlightResponse {\n            itineraries: [],\n        },\n        extensions: Extensions,\n    },\n)","thread_id":140706676972320,"date":"2023-10-09T14:02:25.541989562+00:00"}

lotjuh@roo:~/git/Arrow-air/svc-scheduler:0>cat server/logs/*.log | jq -c '[.] | sort_by(.time) | .[] | select(.thread_id==140706676972320) | .message'
"(search MOCK) vertiport client."
"(search MOCK) flight_plan client."
"(get_by_id MOCK) vertiport client."
"(search MOCK) vertipad client."
"(get_by_id MOCK) vertiport client."
"(search MOCK) vertipad client."
"(search MOCK) vehicle client."
"(confirm_itinerary) with id itinerary1"
"(query_flight) query_flight returning: 0 flight plans."
"(get_vertipads) proposed filter: AdvancedSearchFilter { filters: [FilterOption { search_field: \"vertiport_id\", search_value: [\"31e5275e-8f3f-45cc-a9e4-5a85ff619bff\"], predicate_operator: Equals, comparison_operator: None }, FilterOption { search_field: \"deleted_at\", search_value: [], predicate_operator: IsNull, comparison_operator: Some(And) }, FilterOption { search_field: \"enabled\", search_value: [\"1\"], predicate_operator: Equals, comparison_operator: Some(And) }], page_number: 0, results_per_page: -1, order_by: [] }"
"(get_vertipads) vertiport: \"31e5275e-8f3f-45cc-a9e4-5a85ff619bff\""
"(get_vertipads) response: Response { metadata: MetadataMap { headers: {} }, message: List { list: [Object { id: \"e55f5d19-c6ee-463e-8394-f875ec40db27\", data: Some(Data { vertiport_id: \"31e5275e-8f3f-45cc-a9e4-5a85ff619bff\", name: \"Mock vertipad for vertiport 31e5275e-8f3f-45cc-a9e4-5a85ff619bff\", geo_location: Some(GeoPoint { longitude: -122.4194, latitude: 37.7746 }), enabled: true, occupied: false, schedule: Some(\"DTSTART:20221020T180000Z;DURATION:PT24H\\nRRULE:FREQ=DAILY;BYDAY=MO,TU,WE,TH,FR\"), created_at: Some(Timestamp { seconds: 1647901800, nanos: 0 }), updated_at: Some(Timestamp { seconds: 1647901800, nanos: 0 }) }) }, Object { id: \"783d277d-25ce-4c20-88ae-1644ff7c6e6f\", data: Some(Data { vertiport_id: \"7227d2cb-41e0-47aa-909a-6462d992903f\", name: \"Mock vertipad for vertiport 7227d2cb-41e0-47aa-909a-6462d992903f\", geo_location: Some(GeoPoint { longitude: -122.4194, latitude: 37.7746 }), enabled: true, occupied: false, schedule: Some(\"DTSTART:20221020T180000Z;DURATION:PT24H\\nRRULE:FREQ=DAILY;BYDAY=MO,TU,WE,TH,FR\"), created_at: Some(Timestamp { seconds: 1661643000, nanos: 0 }), updated_at: Some(Timestamp { seconds: 1661643000, nanos: 0 }) }) }, Object { id: \"d76dd8c1-5ee0-4a27-8f26-31d5e21f4aa1\", data: Some(Data { vertiport_id: \"92fb3d23-8bcd-4237-912c-10dc8a6b923a\", name: \"Mock vertipad for vertiport 92fb3d23-8bcd-4237-912c-10dc8a6b923a\", geo_location: Some(GeoPoint { longitude: -122.4194, latitude: 37.7746 }), enabled: true, occupied: false, schedule: Some(\"DTSTART:20221020T180000Z;DURATION:PT24H\\nRRULE:FREQ=DAILY;BYDAY=MO,TU,WE,TH,FR\"), created_at: Some(Timestamp { seconds: 1681296600, nanos: 0 }), updated_at: Some(Timestamp { seconds: 1681296600, nanos: 0 }) }) }, Object { id: \"9336c7a8-e4b3-42a2-90cc-1e8c4f5acb63\", data: Some(Data { vertiport_id: \"e9e7ffc2-92a6-46d7-adad-2ae758110bc8\", name: \"Mock vertipad for vertiport e9e7ffc2-92a6-46d7-adad-2ae758110bc8\", geo_location: Some(GeoPoint { longitude: -122.4194, latitude: 37.7746 }), enabled: true, occupied: false, schedule: Some(\"DTSTART:20221020T180000Z;DURATION:PT24H\\nRRULE:FREQ=DAILY;BYDAY=MO,TU,WE,TH,FR\"), created_at: Some(Timestamp { seconds: 1626314700, nanos: 0 }), updated_at: Some(Timestamp { seconds: 1626314700, nanos: 0 }) }) }, Object { id: \"eee01fcd-289a-4d15-a391-490a599d99e0\", data: Some(Data { vertiport_id: \"e9e7ffc2-92a6-46d7-adad-2ae758110bc8\", name: \"Mock vertipad e9e7ffc2-92a6-46d7-adad-2ae758110bc8\", geo_location: Some(GeoPoint { longitude: -122.4194, latitude: 37.7746 }), enabled: true, occupied: false, schedule: Some(\"DTSTART:20221020T180000Z;DURATION:PT24H\\nRRULE:FREQ=DAILY;BYDAY=MO,TU,WE,TH,FR\"), created_at: Some(Timestamp { seconds: 1640170500, nanos: 0 }), updated_at: Some(Timestamp { seconds: 1640170500, nanos: 0 }) }) }] }, extensions: Extensions }"
"(get_vertipads) proposed filter: AdvancedSearchFilter { filters: [FilterOption { search_field: \"vertiport_id\", search_value: [\"7227d2cb-41e0-47aa-909a-6462d992903f\"], predicate_operator: Equals, comparison_operator: None }, FilterOption { search_field: \"deleted_at\", search_value: [], predicate_operator: IsNull, comparison_operator: Some(And) }, FilterOption { search_field: \"enabled\", search_value: [\"1\"], predicate_operator: Equals, comparison_operator: Some(And) }], page_number: 0, results_per_page: -1, order_by: [] }"
"(get_vertipads) vertiport: \"7227d2cb-41e0-47aa-909a-6462d992903f\""
"(get_vertipads) response: Response { metadata: MetadataMap { headers: {} }, message: List { list: [Object { id: \"e55f5d19-c6ee-463e-8394-f875ec40db27\", data: Some(Data { vertiport_id: \"31e5275e-8f3f-45cc-a9e4-5a85ff619bff\", name: \"Mock vertipad for vertiport 31e5275e-8f3f-45cc-a9e4-5a85ff619bff\", geo_location: Some(GeoPoint { longitude: -122.4194, latitude: 37.7746 }), enabled: true, occupied: false, schedule: Some(\"DTSTART:20221020T180000Z;DURATION:PT24H\\nRRULE:FREQ=DAILY;BYDAY=MO,TU,WE,TH,FR\"), created_at: Some(Timestamp { seconds: 1647901800, nanos: 0 }), updated_at: Some(Timestamp { seconds: 1647901800, nanos: 0 }) }) }, Object { id: \"783d277d-25ce-4c20-88ae-1644ff7c6e6f\", data: Some(Data { vertiport_id: \"7227d2cb-41e0-47aa-909a-6462d992903f\", name: \"Mock vertipad for vertiport 7227d2cb-41e0-47aa-909a-6462d992903f\", geo_location: Some(GeoPoint { longitude: -122.4194, latitude: 37.7746 }), enabled: true, occupied: false, schedule: Some(\"DTSTART:20221020T180000Z;DURATION:PT24H\\nRRULE:FREQ=DAILY;BYDAY=MO,TU,WE,TH,FR\"), created_at: Some(Timestamp { seconds: 1661643000, nanos: 0 }), updated_at: Some(Timestamp { seconds: 1661643000, nanos: 0 }) }) }, Object { id: \"d76dd8c1-5ee0-4a27-8f26-31d5e21f4aa1\", data: Some(Data { vertiport_id: \"92fb3d23-8bcd-4237-912c-10dc8a6b923a\", name: \"Mock vertipad for vertiport 92fb3d23-8bcd-4237-912c-10dc8a6b923a\", geo_location: Some(GeoPoint { longitude: -122.4194, latitude: 37.7746 }), enabled: true, occupied: false, schedule: Some(\"DTSTART:20221020T180000Z;DURATION:PT24H\\nRRULE:FREQ=DAILY;BYDAY=MO,TU,WE,TH,FR\"), created_at: Some(Timestamp { seconds: 1681296600, nanos: 0 }), updated_at: Some(Timestamp { seconds: 1681296600, nanos: 0 }) }) }, Object { id: \"9336c7a8-e4b3-42a2-90cc-1e8c4f5acb63\", data: Some(Data { vertiport_id: \"e9e7ffc2-92a6-46d7-adad-2ae758110bc8\", name: \"Mock vertipad for vertiport e9e7ffc2-92a6-46d7-adad-2ae758110bc8\", geo_location: Some(GeoPoint { longitude: -122.4194, latitude: 37.7746 }), enabled: true, occupied: false, schedule: Some(\"DTSTART:20221020T180000Z;DURATION:PT24H\\nRRULE:FREQ=DAILY;BYDAY=MO,TU,WE,TH,FR\"), created_at: Some(Timestamp { seconds: 1626314700, nanos: 0 }), updated_at: Some(Timestamp { seconds: 1626314700, nanos: 0 }) }) }, Object { id: \"eee01fcd-289a-4d15-a391-490a599d99e0\", data: Some(Data { vertiport_id: \"e9e7ffc2-92a6-46d7-adad-2ae758110bc8\", name: \"Mock vertipad e9e7ffc2-92a6-46d7-adad-2ae758110bc8\", geo_location: Some(GeoPoint { longitude: -122.4194, latitude: 37.7746 }), enabled: true, occupied: false, schedule: Some(\"DTSTART:20221020T180000Z;DURATION:PT24H\\nRRULE:FREQ=DAILY;BYDAY=MO,TU,WE,TH,FR\"), created_at: Some(Timestamp { seconds: 1640170500, nanos: 0 }), updated_at: Some(Timestamp { seconds: 1640170500, nanos: 0 }) }) }] }, extensions: Extensions }"
"(get_aircraft_gaps) Flight plan for unknown aircraft: 3369a641-b3b6-48f7-8af9-8a7306b9cc4f"
"(get_aircraft_gaps) Flight plan for unknown aircraft: 32a2a20c-88b6-4566-b5d7-1731a5a177e2"
"(get_aircraft_gaps) Flight plan for unknown aircraft: 32a2a20c-88b6-4566-b5d7-1731a5a177e2"
"(get_aircraft_gaps) Flight plan for unknown aircraft: bdf35f61-366f-49cd-b782-ef0ffcab804d"
"(get_aircraft_gaps) Flight plan for unknown aircraft: 3369a641-b3b6-48f7-8af9-8a7306b9cc4f"
"(get_aircraft_gaps) Flight plan for unknown aircraft: bdf35f61-366f-49cd-b782-ef0ffcab804d"
"(get_aircraft_gaps) Flight plan for unknown aircraft: 3369a641-b3b6-48f7-8af9-8a7306b9cc4f"
"(get_aircraft_gaps) Flight plan for unknown aircraft: 32a2a20c-88b6-4566-b5d7-1731a5a177e2"
"(get_aircraft_gaps) Flight plan for unknown aircraft: bdf35f61-366f-49cd-b782-ef0ffcab804d"
"(get_aircraft_gaps) Flight plan for unknown aircraft: 32a2a20c-88b6-4566-b5d7-1731a5a177e2"
"(get_itineraries) found 0 itineraries."
"(test_confirm_and_cancel_itinerary) start"
"(test_confirm_and_cancel_itinerary) query_flight result: Ok(\n    Response {\n        metadata: MetadataMap {\n            headers: {},\n        },\n        message: QueryFlightResponse {\n            itineraries: [],\n        },\n        extensions: Extensions,\n    },\n)"

@amsmith-pro amsmith-pro force-pushed the am-smith/dw-1103/use-svc-gis-for-routin branch from 9dc522b to 40bf0b6 Compare October 9, 2023 20:10
@amsmith-pro
Copy link
Contributor Author

@owlot Indeed it was using Utc::now, changed it to use a date in the future always

@amsmith-pro amsmith-pro requested a review from owlot October 9, 2023 20:11
@owlot owlot merged commit 27a5c1a into develop Oct 11, 2023
17 checks passed
@owlot owlot deleted the am-smith/dw-1103/use-svc-gis-for-routin branch October 11, 2023 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants