Add offset option to sensor.gtfs#7980
Conversation
There was a problem hiding this comment.
About that, should I import datetime.timedelta at the top of the file, or go ahead and split this line?
There was a problem hiding this comment.
Like I said, it's the first time I ever wrote Python. I'm not sure what's @property meant for. I expected the offset to show up in the sensor attributes (on the web UI), but it doesn't. Is this property mandatory?
There was a problem hiding this comment.
Use device_state_attributes() to collect and present attributes in the frontend.
There was a problem hiding this comment.
This component actually already implements device_state_attributes, which returns self._attributes. self._attributes is updated inside update, so just make sure you set the attribute there.
There was a problem hiding this comment.
That being said, we can also just remove it. Maybe not so important to have it in the attributes?
There was a problem hiding this comment.
Having it in the attributes might be useful for some notifications or that kind of things. I'll fix this, thanks for the pointers!
balloob
left a comment
There was a problem hiding this comment.
Looks good. Ok to merge when line length issue addressed and property either added to self._attributes or removed.
There was a problem hiding this comment.
continuation line unaligned for hanging indent
* Add documentation for sensor.gtfs offset option Added in PR home-assistant/core#7980 * Add default
* Add offset option to sensor.gtfs * Fix long lines in sensor.gtfs * Expose GTFS offset as an attribute
Description:
The rationale behind this addition is this: if you have a 10 minutes walk to your bus stop, you're not interested in knowing the next bus is in 4 minutes. Instead, you're interested in knowing that the next one is in 12 minutes, so you'd better go and catch it.
The
offsetoption is meant to do this: add a duration, and all departures before this duration will be ignored.Note that I know this PR isn't ready for merge. It's my first PR in here, and I never wrote Python before, things are probably wrong. I'm not so sure about the name
offset, for a start. I'll update the documentation once everything else is fixed in this PR.Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#2883
Example entry for
configuration.yaml(if applicable):Checklist:
If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
toxrun successfully. Your PR cannot be merged unless tests passREQUIREMENTSvariable (example).requirements_all.txtby runningscript/gen_requirements_all.py..coveragerc.If the code does not interact with devices:
toxrun successfully. Your PR cannot be merged unless tests pass