Itinerary-id#1286
Conversation
| nullptr, // osr::lookup ** | ||
| stoptimes_ep.pl_, stoptimes_ep.tt_, stoptimes_ep.tags_, nullptr, nullptr, | ||
| rtt, stoptimes_ep.matches_, nullptr, | ||
| nullptr, // shapes ** |
There was a problem hiding this comment.
you will probably want shapes to return polylines?
traines-source
left a comment
There was a problem hiding this comment.
I think we can just merge src/itinerary_id.cc into endpoints/itinerary_id.cc ?
Other than that and apart from the already discussed issues and code style, LGTM
| string trip_id = 2; | ||
| string from_id = 3; | ||
| string to_id = 4; | ||
| string coords = 5; |
There was a problem hiding this comment.
should be 2x latlng, not string
There was a problem hiding this comment.
But isn't this polyline format more compact?
There was a problem hiding this comment.
depends on the binary representation of string
but probably not much more compact, so not a reasonable tradeoff
-> switch to 2x latlng
| type: string | ||
| toId: | ||
| type: string | ||
| coords: |
| struct leg_hint { | ||
| explicit leg_hint(proto_leg_t const& l) | ||
| : display_name_{l.display_name()}, | ||
| trip_id_{l.trip_id()}, | ||
| from_stop_id_{l.from_id()}, | ||
| to_stop_id_{l.to_id()}, | ||
| sched_start_{l.sched_start()}, | ||
| sched_end_{l.sched_end()}, | ||
| mode_{json::value_to<api::ModeEnum>(json::value{l.mode()})}, | ||
| scheduled_{l.scheduled()} { | ||
| if (!trip_id_.empty()) { | ||
| utl::verify(!display_name_.empty(), "itinerary id: display_name missing"); | ||
| utl::verify(!from_stop_id_.empty(), "itinerary id: from_id missing"); | ||
| utl::verify(!to_stop_id_.empty(), "itinerary id: to_id missing"); | ||
| } | ||
| decode_coords(l.coords()); | ||
| } | ||
|
|
||
| bool is_public_transport() const { return !trip_id_.empty(); } | ||
|
|
||
| std::string display_name_; | ||
| std::string trip_id_; | ||
| std::string from_stop_id_; | ||
| std::string to_stop_id_; | ||
| std::int64_t sched_start_; | ||
| std::int64_t sched_end_; | ||
| geo::latlng from_pos_; | ||
| geo::latlng to_pos_; | ||
| api::ModeEnum mode_; | ||
| bool scheduled_; | ||
|
|
||
| private: | ||
| void decode_coords(std::string_view const encoded_coords) { | ||
| auto const coords = geo::decode_polyline(encoded_coords); | ||
| utl::verify(coords.size() == 2, | ||
| "itinerary id: coords must decode to exactly 2 points, got {}", | ||
| coords.size()); | ||
|
|
||
| from_pos_ = coords[0]; | ||
| to_pos_ = coords[1]; | ||
| } | ||
| }; |
There was a problem hiding this comment.
this can be removed? - just use the generated type proto_leg_t
There was a problem hiding this comment.
I recently removed the other constructor that accepted json.
I think it's a nice abstraction to have and we can always easily remove it if we want to.
It also does some preprocessing and format checking.
| n::unixtime_t arr_; | ||
| }; | ||
|
|
||
| using leg_item = std::variant<resolved_run, walk_segment>; |
There was a problem hiding this comment.
why can't this be a nigiri::journey::leg directly?
|
|
||
| auto const fr = make_frun_from_trip_id( | ||
| stop_times_ep.tags_, stop_times_ep.tt_, rt.rtt_.get(), lh.trip_id_); | ||
| utl::verify(fr.valid(), |
There was a problem hiding this comment.
not found legs should become dummy legs that contain the information from the itinerary-id
if a user presses the refresh button and from 4 legs 1 could not be recovered, it should show the three legs that could be recovered and for the one missing leg, it should show that it could not be recovered
I think we have to add a refresh functionality in the MOTIS debug UI in this PR to test the API for real-world readiness before publishing
Replaces #1269
Related Issue: #998