Bram-diederik commented 10 months ago

The problem

The ns api shows only one departure time.

Due to a feature in the NS APP. to show historic information the home assistant sensor shows departed information 9 out of 10 times. making the integration totally not usable for normal usage to lookup the next departing train.

please fix this issue by looking up the next train to depart or make a future + X option to allow an amount of next following trains. like the hacks ovapi integration.

I know this sounds lots like a feature request and i posted this as feature request but not much happened.

the general userstory I want a sensor for the next train to take to go to my destination fails. so its a bug. and a major one.

What version of Home Assistant Core has the issue?

core-2023.11.3

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant OS

Integration causing the issue

https://www.home-assistant.io/integrations/nederlandse_spoorwegen/

Link to integration documentation on our website

No response

Diagnostics information

No response

Example YAML snippet

No response

Anything in the logs that might be useful for us?

No response

Additional information

No response

home-assistant[bot] commented 10 months ago

Hey there @yarmom, mind taking a look at this issue as it has been labeled with an integration (nederlandse_spoorwegen) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of `nederlandse_spoorwegen` can trigger bot actions by commenting: - `@home-assistant close` Closes the issue. - `@home-assistant rename Awesome new title` Renames the issue. - `@home-assistant reopen` Reopen the issue. - `@home-assistant unassign nederlandse_spoorwegen` Removes the current integration label and assignees on the issue, add the integration domain after the command. - `@home-assistant add-label needs-more-information` Add a label (needs-more-information, problem in dependency, problem in custom component) to the issue. - `@home-assistant remove-label needs-more-information` Remove a label (needs-more-information, problem in dependency, problem in custom component) on the issue.

_{^{(message by CodeOwnersMention)}}

nederlandse_spoorwegen documentation nederlandse_spoorwegen source _{^{(message by IssueLinks)}}

Bram-diederik commented 10 months ago

Screenshot_20231207_160148_Home Assistant You cant say this is functional

itslechef commented 10 months ago

Screenshot_20231212-132359_Home_Assistant_1 Completely useless in it's current state. Next departures show trains 6-7 minutes in the past. I'd love for someone to fix this issue.

joopdo commented 8 months ago

I think this happens because the API doesn't get called often enough. The defined train sensor gets updated every 7 minutes for me, which leads to the perceived delay. Adding a scan_interval to a minute doesn't seem to increase the scan rate for some reason.

What I ended up doing is to make use of the attribute 'next'. If you want to do the same, create a sensor as follows:

sensor:
  - platform: template
    sensors:
      next_departure_time:
        friendly_name: "Next Departure Time"
        value_template: "{{ state_attr('sensor.<name of the sensor you created>', 'next') }}"

Or as a notification:

service: notify.signal_group
data:
  message: |
      🚆 Lutjebroek -> Gansdorp: {{states.sensor.lutjebroek_to_gansdorp.attributes.next}}
      🚆 Gansdorp -> Lutjebroek: {{states.sensor.gansdorp_to_lutjebroek.attributes.next}}

Paiusco commented 6 months ago

I don't get why the sensor doesn't switch to the "next" attribute until the next call to the API is done (by default is set to be called every 120seconds) Which I'm not so sure is true...

It's not that hard to understand the code from the integration, it gets harder when one needs to understand how the whole sensor thingy works behind the curtains

Bram-diederik commented 6 months ago

I think this happens because the API doesn't get called often enough. The defined train sensor gets updated every 7 minutes for me, which leads to the perceived delay. Adding a scan_interval to a minute doesn't seem to increase the scan rate for some reason.

What I ended up doing is to make use of the attribute 'next'. If you want to do the same, create a sensor as follows:
sensor:
  - platform: template
    sensors:
      next_departure_time:
        friendly_name: "Next Departure Time"
        value_template: "{{ state_attr('sensor.<name of the sensor you created>', 'next') }}"
Or as a notification:
service: notify.signal_group
data:
  message: |
      🚆 Lutjebroek -> Gansdorp: {{states.sensor.lutjebroek_to_gansdorp.attributes.next}}
      🚆 Gansdorp -> Lutjebroek: {{states.sensor.gansdorp_to_lutjebroek.attributes.next}}

Only next with out the other info like departer platform. I got a route where i can take a train any way. From haarlem to gouda. Over den haag or amsterdam. I need to know the platform.

dortatdtdx commented 4 months ago

I encoutered the same problem that the sensor isn't updated that often. However, after digging into the code and checking the NS API (and NS mobile app) I noticed that the NS API itself is supplying us a departure time that is in the past, in my case it is about 7 minutes after switching to the correct value.

Although I'm not a python expert, but I was able to adapt the HA nederlandse_spoorwegen component file sensor.py slightly https://github.com/home-assistant/core/blob/dev/homeassistant/components/nederlandse_spoorwegen/sensor.py

I've added a check whether the current time is later than the departure time. If that is the case than take the next train in the list. For me it works now :)

def extra_state_attributes(self):
    """Return the state attributes."""
    if not self._trips:
        return

    # check is current time is later than trips.departure_time_actual
    now = datetime.now()
    now_plus_delta = now - timedelta(minutes = 1)
    if self._trips[0].departure_time_actual is not None:
        departure_time = self._trips[0].departure_time_actual
    else:
        departure_time = self._trips[0].departure_time_planned

    if departure_time.timestamp() > now_plus_delta.timestamp():
        tripid = int('0')
    else:
        tripid = int('1')

    if self._trips[tripid].trip_parts:
        route = [self._trips[tripid].departure]
        for k in self._trips[tripid].trip_parts:
            route.append(k.destination)

The remaining file should be checked and self._trips[0] should be changed to self._trips[tripid]

DjehutiC commented 4 months ago

I encoutered the same problem that the sensor isn't updated that often. However, after digging into the code and checking the NS API (and NS mobile app) I noticed that the NS API itself is supplying us a departure time that is in the past, in my case it is about 7 minutes after switching to the correct value.

Although I'm not a python expert, but I was able to adapt the HA nederlandse_spoorwegen component file sensor.py slightly https://github.com/home-assistant/core/blob/dev/homeassistant/components/nederlandse_spoorwegen/sensor.py

I've added a check whether the current time is later than the departure time. If that is the case than take the next train in the list. For me it works now :)

Thanks for your work.

Not quite sure how I can implement your solution though. Would I need to sideload your sensor instead of the official integration? How would I go about that?

The remaining file should be checked and self._trips[0] should be changed to self._trips[tripid]

Can you elaborate on this? Or perhaps post a newer version?

Thanks again.

dortatdtdx commented 4 months ago

On my system I've modified the file sensor.py (see attachement). This file is located in [..]/site-packages/homeassistant/components/nederlandse_spoorwegen/ After you've modified this file, you should restart HA. I'm pretty sure there is a fancier method to do this, but as I mentioned I'm new to HA and python :)

I'm aware that when HA gets an update I need to adjust the file once again to implement my modifications.

sensor.txt

Bram-diederik commented 4 months ago

None of us knows how to fork hass and push this fix?

Op wo 19 jun 2024 15:07 schreef dortatdtdx @.***>:

On my system I've modified the file sensor.py (see attachement). This file is located in [..]/site-packages/homeassistant/components/nederlandse_spoorwegen/

I'm aware that when HA gets an update I need to adjust the file once again to implement my modifications.

sensor.txt https://github.com/user-attachments/files/15901224/sensor.txt

— Reply to this email directly, view it on GitHub https://github.com/home-assistant/core/issues/104754#issuecomment-2178679784, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMYKLXP3AVVAHAETZR5MDATZIF675AVCNFSM6AAAAABAAF23DKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNZYGY3TSNZYGQ . You are receiving this because you authored the thread.Message ID: @.***>

joostlek commented 4 months ago

I am currently waiting for sub config entries to make its way into HA and then I will look into this if I have time

gbcatrinoiu commented 3 months ago

@joostlek did you manage to do something?

joostlek commented 3 months ago

Still waiting on the sub entries

gbcatrinoiu commented 3 months ago

not sure what you mean. you mean the sensors created by the nederlandse_spoorwegen integration?

joostlek commented 3 months ago

no, a new way of doing configuration within home assistant. This integration suits that way really nicely so I want to use that for this integration

gbcatrinoiu commented 3 months ago

do you have a link for this where I can read a bit more? I might be handy for my setup.

joostlek commented 3 months ago

https://github.com/home-assistant/architecture/discussions/1070

rbouma commented 4 weeks ago

Great News! 🚆

I've updated the NS Integration to work with the new Home Assistant config flow! 🎉 I've got it running locally and just need to figure out the process for getting it merged into Home Assistant.

Features: ✅

Easily manage your routes
No need to look up station codes—just select from a list
Simple configuration directly via the modal

Coming Soon: 🏗️

Add delays to routes which was suggested here: https://github.com/home-assistant/core/pull/45984
Display a list of all upcoming trains for the selected route

Fill in your api key

CleanShot 2024-09-23 at 19 21 48@2x

No more need to find the station code!

CleanShot 2024-09-23 at 19 22 29@2x

Stations are now in a simple selectbox

CleanShot 2024-09-23 at 19 22 36@2x

A device with the route entity is created

CleanShot 2024-09-23 at 19 23 25@2x

all information from the api

CleanShot 2024-09-23 at 19 23 34@2x

lhautomation61 commented 1 day ago

That looks great @rbouma! Could you please share the yaml file? Hope you get it to HA soon, great work

DjehutiC commented 1 day ago

That looks like a lot of work with an excellent outcome, Rick! Thanks for the effort and please keep us updated on the rest of the process..!

Op zo 20 okt 2024, 18:45 schreef lhautomation61 @.***>:

That looks great @rbouma https://github.com/rbouma! Could you please share the yaml file? Hope you get it to HA soon, great work

— Reply to this email directly, view it on GitHub https://github.com/home-assistant/core/issues/104754#issuecomment-2425112512, or unsubscribe https://github.com/notifications/unsubscribe-auth/ASXZZMSMRGN6VPW6AG4PTYTZ4PMZLAVCNFSM6AAAAABAAF23DKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMRVGEYTENJRGI . You are receiving this because you commented.Message ID: @.***>

home-assistant / core

nederlandse_spoorwegen Not usable #104754

The problem

What version of Home Assistant Core has the issue?

What was the last working version of Home Assistant Core?

What type of installation are you running?

Integration causing the issue

Link to integration documentation on our website

Diagnostics information

Example YAML snippet

Anything in the logs that might be useful for us?

Additional information

Great News! 🚆

Features: ✅

Coming Soon: 🏗️

Fill in your api key

No more need to find the station code!

Stations are now in a simple selectbox

A device with the route entity is created

all information from the api