tofuSCHNITZEL / home-assistant-wienerlinien

A sensor that give you information about departures from a specified Wiener Linien stop.
MIT License
26 stars 8 forks source link

Integration miscalculates stations with multiple lines #5

Open BorisProhaska opened 2 years ago

BorisProhaska commented 2 years ago

When using this integration with a station id, that has multiple lines, it is completely being ignored which line is the first and next.

I think it's because this integration isn't actually looking into the data, but always takes the first line.

As an example:

I'd assume, that the first is 11A in 2min, and next is 10A in 7min.

What the integration does is: 10A (because first in the list) in 7min as first, and 10A in 37min as next.

Can you fix this please? :)

Thanks

jozefKruszynski commented 1 year ago

The reason for this is that the way the integration works, is that it only takes the first monitor in the index of the Json response.

I actually forked this project today, and made some changes to fix this issue, also added unique IDs to the created monitors, and modified the naming of them also.

If you want to take a look at it, you can find it here: https://github.com/jozefKruszynski/home-assistant-wienerlinien

The config is identical, but you will need to makes some changes to any dashboards due to the different naming and unique IDs

jozefKruszynski commented 1 year ago

@tofuSCHNITZEL I hope it's okay that I posted my fork in the above response.

BorisProhaska commented 1 year ago

@jozefKruszynski I'll have a look at it - thank you very much.

tofuSCHNITZEL commented 1 year ago

@jozefKruszynski would you create a pull request so I can make the improvements available for everybody?

jozefKruszynski commented 1 year ago

Will do

BorisProhaska commented 1 year ago

@jozefKruszynski @tofuSCHNITZEL this is way better now. Thank you very much - it does exactly what i wanted - you guys are awesome :)

jozefKruszynski commented 1 year ago

@tofuSCHNITZEL I’ll try to make the pull request tomorrow. Please give it a thorough review as I don’t usually program in Python so you might a few oddities.

tofuSCHNITZEL commented 1 year ago

thanks! I will have a look and test it also on my own setup.

jozefKruszynski commented 1 year ago

@tofuSCHNITZEL Do you have a specific merge request template that you would like me to use?

tofuSCHNITZEL commented 1 year ago

hm no I dont think so.

jozefKruszynski commented 1 year ago

@tofuSCHNITZEL in case you hadn't spotted it yet, here is a gentle nudge 😉 https://github.com/tofuSCHNITZEL/home-assistant-wienerlinien/pull/6

tofuSCHNITZEL commented 1 year ago

Yes I already looked it over, would you have time and would you be interested in a call / "digital meet up" to discuss the changes? I also want to look at the API first to get a feeling how the data is structured there...

jozefKruszynski commented 1 year ago

Sounds like a reasonable plan. I actually just managed to find the official documentation of the API last night. I have v1 and v1.3 although I'd really like to get an openapi doc rather than the pdfs that I found.

Edit: I probably have some time next week for a teams/zoom/something else call. Let me know when would suit...

tofuSCHNITZEL commented 1 year ago

teams would work for me... could you send me the links to the doc you found?

jozefKruszynski commented 1 year ago

Good morning, here is the link to the v1.3 api pdf. https://www.wienerlinien.at/ogd_realtime/doku/ogd/wienerlinien-echtzeitdaten-dokumentation.pdf

According to the pdf, v1.3 was released 03.09.2020, I haven't found anything more recent so far.

Here is the original v1.0: https://data.wien.gv.at/pdf/wienerlinien-echtzeitdaten-dokumentation.pdf

From a quick test this morning, it looks as though when you use the DIVA number for a stop instead of the RBL number, you get all info for that stop. I tested Floridsdorf for example, and got a response containing all the U6 departures and the buses, and trams going in each direction.

tofuSCHNITZEL commented 1 year ago

okay I also checked it just now... and the stopid is different for different lines on the same station.. so if for example I take the stop id for schottentor 43 the 0 and 1 (=first and next) departures for line 0 correspond to the departure times for line 43 (confirmed with the wien mobil app) image if I want the U2 info for schottentor - they have a differnet stopid (but same diva)

tofuSCHNITZEL commented 1 year ago

When using this integration with a station id, that has multiple lines, it is completely being ignored which line is the first and next.

@BorisProhaska could you give me an example id? because different lines on the same station normally have a different station id alltogether

BorisProhaska commented 1 year ago

@tofuSCHNITZEL you can use 5812 for example. The lines 49A and 50A to Hütteldorf.

jozefKruszynski commented 1 year ago

I feel like I should probably close my merge request, as we would need to discuss what we really want to achieve. My changes were rushed and suited my needs, but they are probably not a great solution. Unfortunately work is super busy in the run up to Christmas, so I won't have a massive amount of time to invest in helping out.

BorisProhaska commented 1 year ago

@jozefKruszynski just to make it clear: Your "fix" made this integration usable. I guess this isn't time critical. For most people, who live at a station where there's only one line @tofuSCHNITZEL's integration works perfectly fine. It just struggles with stations where multiple lines are departing.

tofuSCHNITZEL commented 1 year ago

And I'm commited make the integration work for everybody - but first I need to understand the problem - and @jozefKruszynski code will also help me to do that. I'm also covered in work so I totally unterstand

jozefKruszynski commented 1 year ago

I will try to take a further look at the api on Wednesday. I can try to build a proper OpenAPI doc from the info in the pdf. I'll post a link when I get it done.

tofuSCHNITZEL commented 1 year ago

@tofuSCHNITZEL you can use 5812 for example. The lines 49A and 50A to Hütteldorf.

okay wait a minute... I think now I understand... since the stopID referres to the stop of a particular line, first AND next will correspond to that line and not "what departs next at this station"... so this extension works as intended... if you want the next departure of another transport line you need to add another sensor (with a different stopid) so @jozefKruszynski is right. we might need to talk strategy because providing what lines leaves next at this location is a totaly different approach to this whole thing...

jozefKruszynski commented 1 year ago

Essentially each of the "lines" are a separate "monitor" in the api. However, each "monitor" can have 0-n different lines contained within it. The nomenclature is rather confusing at best.

jozefKruszynski commented 1 year ago

I generated a class model from a sample response using the "diva" number "60200334". Had to name the file .txt as GitHub doesn't allow me to attach a .py file generated-model-from-response.txt Anyway, it might be useful to us.

tofuSCHNITZEL commented 1 year ago

okay so DIVA = STATION - so if you give the api a divaID you get a list of ALL lines from this station on the other hand stopID = line (with a direction) so it should contain only one monitor (but e.g. stationid 4203 has two monitor entries)

jozefKruszynski commented 1 year ago

It seems that a diva is a collection of stopIDs in the general vicinity. However stopid != Line as a stopId can have many lines.

tofuSCHNITZEL commented 1 year ago

really? but if I click through https://till.mabe.at/rbl and choose locations with more than one station every line at a station gets their own stopid... so what stopID do you know that is truly for different lines? (4203 for example both is for U2 - same direction but for the front of the train and the back of the train (I assume))

jozefKruszynski commented 1 year ago

2378 has lines 26 and 25 as an example

tofuSCHNITZEL commented 1 year ago

damn you are right! but I would say thats an error on the side of wiener linien right? because if stopid and diva is the same for line 25 and 26 at the station carminweg - how should we distinguish between them? the best would be probably to create a config flow where you enter the stopid (or better yet type in the name of the line and then select the station - and then if there is multiple monitors for a stopID you can choose)

but its still not very "robust" who is to say that with a certain stopid that returns monitors for two different lines - the one line will always be under monitor 0 and the other under monitor 1 (we would probably need to save the "lineid" and then always check the monitor if it also has this line id)

I also found this which might be usefull? https://www.wienerlinien.at/ogd_realtime/doku/

did you happen to find a stopid / diva with a monitor that contained more than one "lines"? image

jozefKruszynski commented 1 year ago

I think that this is simply the way the API is designed and doesn't appear to be an error. Every stopId that I have checked, where I know for sure that there are more than one line stopping there, has this same structure. The monitor arrays themselves are always in ascending numerical order from what I have observed.

jozefKruszynski commented 1 year ago

https://www.wienerlinien.at/ogd_realtime/doku/ogd/wienerlinien_ogd_Beschreibung.pdf

This doc seems to suggest that the DIVA number is indeed a collection of stops in the area.

tofuSCHNITZEL commented 1 year ago

Okay so the "quick fix" for this whole thing will be to introduce a "lineid" setting that you need to supply if a stopid has multiple lines

jozefKruszynski commented 1 year ago

That makes sense, and could have it optional, so that if it's empty, create a sensor for each line

tofuSCHNITZEL commented 1 year ago

yes - or fall back to only picking the first monitor so it stays backwards compatibale (also behaviourwise) with previous versions. and if you need multiple lines at the same stop you just add additional line ids or add a new sensor. and then for the next "big" update I would go with a config flow where you can interactivly choose the station and then it creates the senors as such that we also save on API requests because we get all the stations lines in one request

jozefKruszynski commented 1 year ago

Sounds like the beginnings of a plan

tofuSCHNITZEL commented 1 year ago

@BorisProhaska: please try the new beta version: https://github.com/tofuSCHNITZEL/home-assistant-wienerlinien/releases/tag/1.3.0-beta (enable "show beta versions" under "redownload" in hacs)

it enables you to specify the line you are ihterested in and should solve the problem you described

jozefKruszynski commented 1 year ago

Nice update, I will close my merge request. My one suggestion would be to use a list of "lines" within the "stop" config to avoid duplication within config.

tofuSCHNITZEL commented 1 year ago

Yes I already thought about that - I will try to add it before this version will go "stable" - I also thought about changing it from the LineID to the actual name of the line - they are fairly straight forward and have no spaces or special chars in them...

mr-p666 commented 1 year ago

@BorisProhaska: please try the new beta version: https://github.com/tofuSCHNITZEL/home-assistant-wienerlinien/releases/tag/1.3.0-beta (enable "show beta versions" under "redownload" in hacs)

it enables you to specify the line you are ihterested in and should solve the problem you described

Hej @tofuSCHNITZEL,

I tried your integration but still have issues with multiple lines at one station. If I use line '52B' and 'Ulmenstraße' in the direction of 'Hütteldorf', I get the same StopID as if I use line '43B' and 'Ulmenstraße' in the same direction. On the page from @mabe-at, at least the station number in the URL would be different.

I tried the 1.3.0-beta and my config looks like this:

platform: wienerlinien
stops:
  - stop: 3306
  - stop: 3307
    line: 243

Any suggestions or ideas? Would be very grateful for any help! :smiley:

chrbkr commented 11 months ago

I just tried the beta version. Unfortunately, it is still broken, probably in the same way, plus in some other ways.

I guess the entity names are generated dynamically? For example, for Klosterneuburger Straße/Wallensteinstraße, I got several entities with depots as the target (probably because I restarted late at night, when many lines were heading to the depot). When trying to add an entity for line 33 to a card, all 8 (!) entities are greyed out. For line 5, only the one with target Wexstraße (depot) is available (sensor.klosterneuburger_str_wallensteinstrasse_5_wexstrasse_betriebsbhf_brigittenau_first_departure), and only firs_departure, not next_departure.

But after adding that entity, when going into the details, the destination is actually Josefstädter Straße U, and the Line is 33 instead of 5...

tofuSCHNITZEL commented 11 months ago

Yes it's a huge mess. Especially because the api resturns differnt stuff depending on the time of day... it will need a rebuild from the ground up in a way that can handle all of this api madness