akloeckner / hacs-hafas

Port of kilimnik's HAFAS client for HACS
5 stars 2 forks source link

Improve state and attributes of integration (re: Attributes like HVV) #4

Closed risiko79 closed 9 months ago

risiko79 commented 1 year ago

Hi akloeckner,

many thanks for your investigations taking the source from @kilimnik and put it into this repro. I'm searching for a DB integration for HA. I started with https://github.com/faserf/ha-deutschebahn but this implementation uses schiene too.

After pyhafas was fixed hopefully this project will get more boost.

Now my request:

It is possible to implement the atrributes of the sensor like the HVV integration? https://www.home-assistant.io/integrations/hvv_departures

This hopefully enables the possibility to use the hvv card https://github.com/nilstgmd/hvv-card

akloeckner commented 1 year ago

You mean, the following attributes should be filled with meaningfull data?

Attribute | Description -- | -- line | Line number of the next departure origin | The station where the transport started from direction | The station where the transport ends type | Type of the transportation, for example, Bus or S id | A unique identifier for the line. In most cases, line is sufficient to identify the line delay | Real-time data about the delay of the transport in seconds. Add this to the departure time to get the real departure time next | A list of the upcoming departures. Each element has the above attributes and departure containing the timestamp

I checked the HVV card, and it uses the next attribute to display the list of departures: https://github.com/nilstgmd/hvv-card/blob/5d73c08bd035957284a76fd80cc41288b3e0b75e/hvv-card.js#L81

However, the next attribute is already filled with just the single next departure. So, this cannot be changed to be a list, unless we break compatibility with the old db integration.

However, I also would like to have a list of departures in an attribute. So, maybe this can be added as a new attribute and HVV card can be enhanced to accept this new attribute, too? Or, we accept the breaking change and take this as an opportunity to re-think the entire attributes. In the latter case, I would also switch the state to be an actual timestamp...

risiko79 commented 1 year ago

Hi. This is a new integration and from my point of view a breaking change should be acceptable. The old db integration is very simple and don't really support more departures. I come from FHEM and there the db integration is bretty much better.

Maybe HVV isn't the best example, but it supports more departures and a well card is available. From my point of view all departures/transport sensors in HA should be standardized (same attributes) like the weather sensors

MVG uses departures as list instead of next and you can define the number of departures with the attribute 'number' https://www.home-assistant.io/integrations/mvglive/

Do you know if @kilimnik will continue the development?

kilimnik commented 1 year ago

Hey, I am open to continue developing this further. The reason I used next, is for backwards comparability to the old db integration. But I think you're right, a list would be more usefull.

kilimnik commented 1 year ago

A standard transport system would be great. I'm not in contact with any HA Devs so I don't know if there is something planned.

akloeckner commented 1 year ago

Maybe we can have a specific look at other software (FHEM, HVV, HAFAS itself...) to see what would be a good format.

Also, it might be worth to start a discussion on the HA forums, such that other integration developers can find and join it more easily. If there are plans, someone on the forums should know. And if there aren't, we can propose something from scratch and discuss it.

I guess it will take a long time to agree on something, if that even ever happens. In the meantime, we can go forward with our own integration and find something that works for us.

akloeckner commented 1 year ago

I was waiting for my local transport company to become available in pyhafas, because I wouldn't have any meaningful use case without it. Now it is available and I have it working in HA. Shall we go on with this? As a start I created the forums topic: https://community.home-assistant.io/t/best-attributes-for-a-transport-integration/603036

risiko79 commented 1 year ago

Many thanks for your investigations. Hopefully we will get answers asap. Maybe also interesting

https://github.com/agners/swiss-public-transport-card https://github.com/vas3k/home-assistant-berlin-transport https://github.com/MrGauz/home-assistant-munich-transport https://community.home-assistant.io/t/lovelace-munich-public-transport-departure-card/59622 http://hasl.sorlov.com/

A lot of individual implementations - sensors and cards

akloeckner commented 1 year ago

Thanks for these pointers. There is nothing in the community yet.

I'd summarize the integrations as follows:

The pyhafas module returns data in the Friendly Public Transport Format. It seems a bit proprietary to the group around the module. We retrieve a list of Journey objects, which are in turn basically a list of Leg objects with the following fields:

My thoughts so far:

What do you think? (Note that this will not be compatible with the HVV card.)

akloeckner commented 1 year ago

The attributes should simply reproduce the returned Journey object. That's just what HaFaS returns.

I just tried that... it's not so easy, because the object seems to be recursing indefinitely. So, we'll have to make a choice which variables to return...

# see https://gist.github.com/sungitly/3f75cb297572dace2937?permalink_comment_id=4211196#gistcomment-4211196
def to_dict(item):
    match item:
        case dict():
            return {k: to_dict(v) for k, v in item.items()}
        case list() | tuple():
            return [to_dict(x) for x in item]
        case object(__dict__=_):
            return {k: to_dict(v) for k, v in vars(item).items()}
        case _:
            return item
risiko79 commented 1 year ago

Hi. Sorry I can't support so much. But I think such a generic solution will not work. You have to go through the results from pyhafas and have to create a own dict by yourself depending on the journey or object type.

In your proposed return I can't see a list for the journeys. An attribute should return a list of dict (the converted journey object)

sc-ampersand-p commented 1 year ago
  • Non-cancelled, because I noticed that DB also returns canceled departures, which then might show up as next departure.

I'd advise against filtering out cancelled connections. Especially when it comes to hourly connections, you might not even notice that the next connection you're looking at is an hour later than you expected it to be. Just my two cents, obviously, but I like the DB app for precisely that reason. In my area, the VVS app just removes connections from the search results instead of marking them as "they used to exist, but not today".

Obviously, this would mean somehow clearly marking cancelled connections. Not sure if that's properly feasible. Returning a connection that isn't even going to run without any indication that it's been cancelled is an obviously bad choice.

akloeckner commented 1 year ago

you might not even notice that the next connection you're looking at is an hour later than you expected it to be

Good point. I guess it also depends on the type of state. I'd opt for an actual datetime. It's unambiguous. It can easily be used programmatically. It shows in the front-end as relative time. So, this one would be obviously reporting "in 1 hour". If we were to stick with the legacy time format "hh:mm + d", that would be less obvious.

In the list of departures, we should keep the cancelled journeys, if the transport company provides them. They will be marked as cancelled, of course.

PS.

Returning a connection that isn't even going to run without any indication that it's been cancelled is an obviously bad choice.

That is what's happening right now with data from DB. There is an attribute canceled: true. But that is not visible directly in the front-end.

akloeckner commented 9 months ago

I'll close this via #7. Thanks for all your constructive feedback!