Web Sockets implementation test round 1

anithegregorian commented 1 year ago

Hi @albertkun @matikin9 ,

I tested the WebSocket implementation today, looks like it's still in the early stages and returning empty responses:

{"metadata": {"title": "Vehicle Positions"}, "type": "FeatureCollection", "features": []}

Perhaps this may clarify what could be an ideal implementation in FastAPI:

socket should only emit events when vehicle position changes (to reduce operational load/cost on the client)
moving marker plugin can be initialized with static/live data (preferably live data, but needs a fallback to static), which creates a designated route/path for the marker animation (which is the current approach), it only calls pause() and resume() methods randomly to simulate animation, it also limits changing the route/path
for a live moving vehicle this would mean refactoring the moving marker code to progressively position the marker based on the current longitude/latitude emitted from the socket server (which is the correct approach)
we should add longitude/latitude progressively for moving marker only after the socket is connected for every subscriber

You should check out the documentation for moving the marker as it says:

Getter Methods

isRunning(): return true if the marker is currently moving.
isPaused(): return true if the marker is paused
isEnded(): return true if the marker is arrived to the last position or it has been stopped manually
isStarted(): return true if the marker has started

Note: Marker.getLatLng() still works and give the current position

Setter Methods

start(): the marker begins its path or resumes if it is paused.
stop(): manually stops the marker, if you call `start after, the marker starts again the polyline at the beginning.
pause(): just pauses the marker
resume(): the marker resumes its animation
addLatLng(latlng, duration): adds a point to the polyline. Useful, if we have to set the path one by one.
moveTo(latlng, duration): stops current animation and make the marker move to latlng in duration ms.
addStation(pointIndex, duration): the marker will stop at the pointIndexth points of the polyline during duration ms. You can't add a station at the first or last point of the polyline.

I'd be interested in addLatLng(latlng, duration) and maybe addStation(pointIndex, duration) if you have an idea of avg. time a vehicle would have a pitstop.

matikin9 commented 1 year ago

What's the endpoint you're using? We just pushed some updates yesterday and I'm getting data:

wss://api.metro.net/LACMTA/live/trip_detail/route_code/720?geojson=True

I'll need some time to go over the rest of your post. I have no idea what it looks like to integrate the websocket connection on the frontend, so this is useful! :D

anithegregorian commented 1 year ago

@matikin9 its working now, wasn't showing up yesterday...

anithegregorian commented 1 year ago

@albertkun @matikin9

I've run into one small issue with grouping bus routes in series. The API response returns an unsorted array of route codes, which makes it difficult to group. One of the route codes SOFI is also concerning because it's a string instead of an integer.

I was able to piece together a rather complex math expression to check for series:

matchSeries(index){
    const current = store.agencies_bus_filtered[index];
    if (index === 0){
        return true
    }
    const previous = store.agencies_bus_filtered[index - 1];
    return Math.floor(parseInt(current.route_code) / 100) * 100 !== Math.floor(parseInt(previous.route_code) / 100) * 100
}

This goes at the template level with v-if to print the series label. Would it be possible to sort the response in ascending order by route_code?

Don't want to create new issues, as it's a small fix...

albertkun commented 1 year ago

Hello @anithegregorian hmm, I can look into to it (and yes, i should be pretty simple!) but will probably have something out later this week (maybe Wednesday at earliest). i might be able to get it by descending order if that's OK. let me check in with @matikin9 to see how fast we can move this PR.

albertkun commented 1 year ago

@anithegregorian i have it currently running on dev here: https://dev-metro-api-v2.ofhq3vd1r7une.us-west-2.cs.amazonlightsail.com/

if this is the websocket endpoint, then you can test this to see if it is ordered according to your request?

wss://dev-metro-api-v2.ofhq3vd1r7une.us-west-2.cs.amazonlightsail.com/LACMTA/live/trip_detail/route_code/720?geojson=True

If not, let me know which endpoint you are referring to.

Also, for this issue and going forward, please provide the exact endpoint you are testing with so we can troubleshoot correctly, thanks!

anithegregorian commented 1 year ago

@albertkun that would be the route overview (https://api.metro.net/LACMTA/route_overview?route_code=all) endpoint, haven't been able to test live API's yet.

If you check this response tail (notice route_codes, SOFI, 10, 14 etc), the 1-100 series should be at the top of response according to Figma design:

{
  "route_long_name": null,
  "route_id": "SOFI",
  "route_type": 3,
  "route_text_color": null,
  "agency_id": "LACMTA",
  "alt_id": null,
  "description": null,
  "pdf_file_link": null,
  "route_code": "SOFI",
  "route_short_name": "C Line (Green) - SoFi Stadium",
  "route_desc": "The express shuttle will operate between the Metro C Line Hawthorne/Lennox Station and SoFi Stadium during SoFi Stadiums three pre-season and 18 regular-season NFL games. Shuttle service will run every 5-8 minutes roughly 3 hours before kick-off and roughly 1.5 hours after each game. During the game, shuttles will depart SoFi to the Metro C Line Hawthorne/Lennox Station as needed. Metro express shuttle service to SoFi Stadium is FREE, but normal fares apply for regular Metro Bus and Rail service and parking fees at Metro Park & Ride lots",
  "route_color": null,
  "route_url": "https://www.metro.net/riding/gameday/",
  "line_id": null,
  "long_name": null,
  "pdf_file_url": null,
  "iconography_url": null
},
{
  "route_long_name": "Metro Local Line",
  "route_id": "10-13167",
  "route_type": 3,
  "route_text_color": null,
  "agency_id": "LACMTA",
  "alt_id": "10/48",
  "description": "Northbound to DTLA-Southbound to Avalon Station ",
  "pdf_file_link": "https://www.metro.net/line-override/22-12-line-010-048/attachment/010-048_tt_12-11-22/",
  "route_code": "10",
  "route_short_name": "10/48",
  "route_desc": "W HOLLYWOOD-DTWN LA -AVALON STA VIA MELROSE-AVALON",
  "route_color": null,
  "route_url": null,
  "line_id": "10",
  "long_name": "",
  "pdf_file_url": "https://cdn.beta.metro.net/wp-content/uploads/2022/12/10171216/010-048_TT_12-11-22.pdf",
  "iconography_url": null
},
{
  "route_long_name": "Metro Local Line",
  "route_id": "14-13167",
  "route_type": 3,
  "route_text_color": null,
  "agency_id": "LACMTA",
  "alt_id": "14/37",
  "description": "Eastbound to DTLA-Westbound to Cedars-Sinai",
  "pdf_file_link": "https://www.metro.net/line-override/22-12-line-014-037/attachment/014-037_tt_12-11-22/",
  "route_code": "14",
  "route_short_name": "14/37",
  "route_desc": "CEDAR SINAI-DTWN LA-WASH/FAIRFAX VIA BEVERLY-ADAMS",
  "route_color": null,
  "route_url": null,
  "line_id": "14",
  "long_name": "",
  "pdf_file_url": "https://cdn.beta.metro.net/wp-content/uploads/2022/12/10171411/014-037_TT_12-11-22.pdf",
  "iconography_url": null
},
{
  "route_long_name": "Metro Local Line",
  "route_id": "35-13167",
  "route_type": 3,
  "route_text_color": null,
  "agency_id": "LACMTA",
  "alt_id": "35/38",
  "description": "Eastbound to DTLA-Westbound to Washington/Fairfax via Washington Bl",
  "pdf_file_link": "https://www.metro.net/line-override/22-12-line-035-038/attachment/035-038_tt_12-11-22/",
  "route_code": "35",
  "route_short_name": "35/38",
  "route_desc": "DOWNTOWN LA- WLA VIA WASHINGTON BL & JEFFERSON BL",
  "route_color": null,
  "route_url": null,
  "line_id": "35",
  "long_name": "",
  "pdf_file_url": "https://cdn.beta.metro.net/wp-content/uploads/2022/12/10170903/035-038_TT_12-11-22.pdf",
  "iconography_url": null
},
{
  "route_long_name": "Metro Local Line",
  "route_id": "211-13167",
  "route_type": 3,
  "route_text_color": null,
  "agency_id": "LACMTA",
  "alt_id": "211/215",
  "description": "Inglewood-South Bay Galleria via Prairie Av-Inglewood Av",
  "pdf_file_link": "https://www.metro.net/line-override/22-10-line-211-215/attachment/211-215_tt_02-19-23/",
  "route_code": "211",
  "route_short_name": "211/215",
  "route_desc": "INGLEWOOD-SOUTH BAY GALLERIA VIA PRAIRIE-INGLEWOOD",
  "route_color": null,
  "route_url": null,
  "line_id": "211",
  "long_name": "",
  "pdf_file_url": "https://cdn.beta.metro.net/wp-content/uploads/2023/02/21093622/211-215_TT_02-19-23.pdf",
  "iconography_url": null
},
{
  "route_long_name": "Metro Local Line",
  "route_id": "242-13167",
  "route_type": 3,
  "route_text_color": null,
  "agency_id": "LACMTA",
  "alt_id": "",
  "description": "Devonshire St - Woodland Hills via Tampa Av-Winnetka Av",
  "pdf_file_link": "https://www.metro.net/line-override/22-10-line-242-243/attachment/242-243_tt_10-23-22/",
  "route_code": "242",
  "route_short_name": "242/243",
  "route_desc": "NORTHRIDGE - TARZANA VIA TAMPA - WINNETKA AVS",
  "route_color": null,
  "route_url": null,
  "line_id": "242",
  "long_name": "242/243",
  "pdf_file_url": "https://cdn.beta.metro.net/wp-content/uploads/2022/10/19111037/242-243_TT_10-23-22.pdf",
  "iconography_url": null
},
{
  "route_long_name": "Metro Express Line",
  "route_id": "487-13167",
  "route_type": 3,
  "route_text_color": null,
  "agency_id": "LACMTA",
  "alt_id": "487/489",
  "description": "Downtown LA - Sierra Madre Villa Station - Temple City",
  "pdf_file_link": "https://www.metro.net/line-override/22-12-line-487-489/attachment/487-489-tt-12-22/",
  "route_code": "487",
  "route_short_name": "487/489",
  "route_desc": "DOWNTOWN LA - SIERRA MADRE VILLA STA",
  "route_color": null,
  "route_url": null,
  "line_id": "487",
  "long_name": "",
  "pdf_file_url": "https://cdn.beta.metro.net/wp-content/uploads/2022/12/08201004/487-489-TT-12-22.pdf",
  "iconography_url": null
}

albertkun commented 1 year ago

Thank you for providing the example endpoint @anithegregorian !

I made a PR into dev can you check this in a couple minutes? I won't be able to respond until Wednesday though!

https://dev-metro-api-v2.ofhq3vd1r7une.us-west-2.cs.amazonlightsail.com/LACMTA/route_overview?route_code=all

anithegregorian commented 1 year ago

@albertkun I checked, thanks for the quick response.

When grouped the series looks like this from both dev and prod API:

Maybe this will give proper context. It would also make sense to group the route overview as a series in 100s, 200s, 300s, and so on, from FastAPI instead of returning a flat response.

Partly because I have to pair them up in the Petite Vue template and you cannot write JavaScripts within templates to jailbreak from loops. Because of Vue's data-driven approach, the API response may need refactoring (grouping by route_code):

{
    "100": ["Route 14", "Route 25", "Route 35", "Route 99"],
    "200": ["Route 104", "Route 134", "Route 199"],
    "300": ["Route 301", "Route 333", "Route 378"]
}

Hope this helps...

albertkun commented 1 year ago

Thank you for the extra details and checking the endpoints @anithegregorian!! Also the screenshots were super helpful! So this seems to be an issue with how we return the route overview data in this Python code here:

https://github.com/LACMTA/metro-api-v2/blob/dev/fastapi/app/crud.py#L486-L495

The order_by worked for the query but not the json object we are creating. Should be an easy-ish fix, just needs some tinkering. Will get back to you tomorrow!

anithegregorian commented 1 year ago

That's looking good @albertkun

Does dev have these updates? I'm seeing it all jumbled up and non-sequential right now on dev or you're still tinkering to get it in the right order. If route_code is stored as a string we're in for a surprise when it comes to sorting!

The real challenge would be grouping series. I guess you're off today, will wait for further updates and keep moving with more template cleanup and schedule details page.

albertkun commented 1 year ago

@anithegregorian Thank you for the reply! Yeah, it was not ready for testing yet and I also gave you the wrong endpoint!

The correct URL is the following and is up on dev:

https://dev-metro-api-v2.ofhq3vd1r7une.us-west-2.cs.amazonlightsail.com/all/route_overview

Also, regarding your comment about route_code by a string, we created a new field, route_code_padded which is an integer! Feel free to use that for sorting purposes!

Let me know if you have any questions!

anithegregorian commented 1 year ago

Hi @albertkun

I tried with the refactored/additional field route_code_padded version API endpoint and it looks like this:

As you can see, grouping series won't work if the response is not sorted by route_code_padded, and would make sense to group them in series with the API response (instead of flat response)

{
  "LACMTA": [
    {
      "100": [
        {
          "route_code": 2,
          "route_desc": "Westwood"
        },
        {
          "route_code": 56,
          "route_desc": "ASTRAL CITY"
        },
        {
          "route_code": 99,
          "route_desc": "BE SOFT."
        }
      ],
      "200": [
        {
          "route_code": 201,
          "route_desc": "Aboa"
        },
        {
          "route_code": 252,
          "route_desc": "CAPIO"
        },
        {
          "route_code": 298,
          "route_desc": "BUXUM"
        }
      ],
      "300": [
        {
          "route_code": 311,
          "route_desc": "Lutetia"
        },
        {
          "route_code": 344,
          "route_desc": "AONIDES"
        },
        {
          "route_code": 389,
          "route_desc": "FISCINA"
        }
      ]
    }
  ]
}

This is just an example of a response, that is grouped by series of route_code, and it's also sorted in each series. We can have a quick discussion if this doesn't make sense.

Thanks

anithegregorian commented 1 year ago

@albertkun is the WebSocket implementation direction aware? Meaning if LACMTA_Rail route 801 is moving in the opposite direction, is there any way to differentiate and update the map?

albertkun commented 1 year ago

@anithegregorian because the direction varies by route, you should probably use the destination_code to identify the direction of travel. We do not currently include direction_id.

albertkun commented 1 year ago

Hi @albertkun

I tried with the refactored/additional field route_code_padded version API endpoint and it looks like this:

As you can see, grouping series won't work if the response is not sorted by route_code_padded, and would make sense to group them in series with the API response (instead of flat response)
{
  "LACMTA": [
    {
      "100": [
        {
          "route_code": 2,
          "route_desc": "Westwood"
        },
        {
          "route_code": 56,
          "route_desc": "ASTRAL CITY"
        },
        {
          "route_code": 99,
          "route_desc": "BE SOFT."
        }
      ],
      "200": [
        {
          "route_code": 201,
          "route_desc": "Aboa"
        },
        {
          "route_code": 252,
          "route_desc": "CAPIO"
        },
        {
          "route_code": 298,
          "route_desc": "BUXUM"
        }
      ],
      "300": [
        {
          "route_code": 311,
          "route_desc": "Lutetia"
        },
        {
          "route_code": 344,
          "route_desc": "AONIDES"
        },
        {
          "route_code": 389,
          "route_desc": "FISCINA"
        }
      ]
    }
  ]
}
This is just an example of a response, that is grouped by series of route_code, and it's also sorted in each series. We can have a quick discussion if this doesn't make sense.

Thanks

Sure, I think a quick discussion would be helpful! What is your availability on Monday? I know it is getting late in India, haha 😅

anithegregorian commented 1 year ago

I can do it between 10.00 - 10.30 AM PDT on Monday

albertkun commented 1 year ago

I can do it between 10.00 - 10.30 AM PDT on Monday

Perfect, lets do that! Thank you!

anithegregorian commented 1 year ago

@albertkun I'd also like to go over the frequency of WebSocket broadcasts, I did a barebones test, opened up a WebSocket connection to the 801 route, and waited for Firefox to collect resource information.

If I leave the browser window open for 20-25 mins the interaction time in Browser gets sketchy (there's a slight delay in response), and we'd have to go over this as well. Just giving you a head up.

It's also been highlighted in my first response on this thread:

Socket should only emit events when vehicle position changes (to reduce operational load/cost on the client)

Have a great weekend!

anithegregorian commented 1 year ago

Hey @albertkun

It seems like the live/dev API servers are down, live API was down since yesterday, getting this error:

503 Service Temporarily Unavailable

albertkun commented 1 year ago

@anithegregorian yeah, not sure how or why the api went down all of a sudden, but looking into it. i'm pushing a new version to dev right now #267 , with reverting some previous changes. hopefully it brings dev back up and i'll merge it to main

albertkun commented 1 year ago

@anithegregorian api should be back up now! Will monitor throughout the day for any issues!

anithegregorian commented 1 year ago

Thanks @albertkun , think you'll need some sort of robust monitoring if this thing goes down once it goes live

anithegregorian commented 1 year ago

@albertkun seems like WebSocket is not returning proper responses, for most lines it's empty for most it's closing the connection. Tested on live and dev...

albertkun commented 1 year ago

@anithegregorian very odd, I'll take a look at it later! Is it closing the connection more frequently?

albertkun commented 1 year ago

@anithegregorian I made a quick change to the time out on dev! Let me know if it's working again and I'll merge to dev!

albertkun commented 1 year ago

@anithegregorian I identified the issue, the socket was/closing because you were most likely using geojson=True as the parameter. Please set that to false. If you need the geojson format, I'll go ahead and fix it.

edit: also, please remember to provide us with the broken endpoint next time so we can replicate the issue on our side. Thanks!

anithegregorian commented 1 year ago

@albertkun Yes please, the entire codebase is parsing through GeoJSON since it's standard/portable across any mapping platform, will keep the broken mapping link in mind.

albertkun commented 1 year ago

@anithegregorian Ok, sounds good! i'll get around to adding that functionality on Friday (Saturday your time)! Sorry for the delay!

albertkun commented 1 year ago

@anithegregorian Should be up and fixed now! Let me know if you have any questions!

anithegregorian commented 1 year ago

@albertkun As we close in on this, two things that I noticed:

Server sockets are still closing connections
Is it possible to send a LineString in GeoJSON response for routes (currently its built with lng/ltd which looks ugly)

Rest looks steadfast! In conclusion, it would also benefit to watch out for any downtime in API service and how to mitigate them on the front end.

albertkun commented 1 year ago

Thank you @anithegregorian !

Unfortunately, yes, the server sockets are still closing.. I'm not sure why- except that it could still be related to a timeout setting...
For the LineString which GeoJSON endpoint would you like it for? The routes. I'm not sure if I can switch the formatting for the GeoJSON, but will try!

Rest does seem pretty good, as I'm still having trouble working with websockets (as you can see!!).

LACMTA / metro-api-v2

Web Sockets implementation test round 1 #244