Add OASA Telematics greek public transport sensor component #22196

panosmz · 2019-03-19T21:03:10Z

Description:

Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#8978

Example entry for `configuration.yaml` (if applicable):

sensor:
  - platform: oasa_telematics
    stop_id: '090006'
    route_id: 1965

Checklist:

The code change is tested and works locally.
Local tests pass with tox. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated in home-assistant.io

If the code communicates with devices, web services, or third-party tools:

New dependencies have been added to the REQUIREMENTS variable (example).
New dependencies are only imported inside functions that use them (example).
New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
New files were added to .coveragerc.

houndci-bot · 2019-03-19T21:03:16Z

homeassistant/components/oasa_telematics/__init__.py

@@ -0,0 +1 @@
+"""The OASA Telematics component."""


blank line at end of file

bkbilly · 2019-03-20T00:20:17Z

Hi!
If you want take a look of my implementation of this because it seems simpler using coordinates:
https://gist.github.com/bkbilly/ee250990228f1acb3673b1b1d06574bb

panosmz · 2019-03-20T06:36:28Z

Hi @bkbilly, thanks that's interesting! However, I use a different approach. I find it simpler (and more useful) to get the upcoming arrivals for a given bus stop, mimicking the way the real bus stop displays work. Nonetheless we could definitely add the actual bus coordinates as a different mode.

MartinHjelmare

Please clarify what we mean with "due in" and "next arrival".

MartinHjelmare · 2019-04-03T10:49:05Z

homeassistant/components/oasa_telematics/sensor.py

+For more info on the API see:
+https://oasa-telematics-api.readthedocs.io/en/latest/
+For more details about this platform, please refer to the documentation at
+https://home-assistant.io/components/sensor.oasa_telematics/


We don't include the url in the docstring anymore. Just keep the very first line.

MartinHjelmare · 2019-04-03T10:49:32Z

homeassistant/components/oasa_telematics/sensor.py

+REQUIREMENTS = ['oasatelematics==0.2']
+_LOGGER = logging.getLogger(__name__)
+
+ATTR_STOP_ID = 'Stop ID'


State attribute keys should be lowercase snakecase.

MartinHjelmare · 2019-04-03T10:50:13Z

homeassistant/components/oasa_telematics/sensor.py

+})
+
+
+def setup_platform(hass, config, add_devices, discovery_info=None):


Please rename add_devices to add_entities.

MartinHjelmare · 2019-04-03T10:52:58Z

homeassistant/components/oasa_telematics/sensor.py

+        self._times = self.data.info
+        self._name_data = self.data.name_data
+        try:
+            self._state = self._times[0][ATTR_DUE_IN]


Instead of making the state minutes until departure, make it an absolute utc timestamp and set device_class property to timestamp.

This will allow the user to select a relative time representation of the state in the frontend.

https://developers.home-assistant.io/docs/en/entity_sensor.html#available-device-classes

MartinHjelmare · 2019-04-03T10:54:10Z

homeassistant/components/oasa_telematics/sensor.py

+        if self._times is not None:
+            next_arrival = None
+            if len(self._times) > 1:
+                next_arrival = ('{} min'.format(


What does next arrival mean? What is arriving where?

MartinHjelmare · 2019-04-03T10:56:17Z

homeassistant/components/oasa_telematics/sensor.py

+                next_arrival = ('{} min'.format(
+                    str(self._times[1][ATTR_DUE_IN])))
+            params.update({
+                ATTR_DUE_IN: str(self._times[0][ATTR_DUE_IN]),


What does due in mean?

I'm thinking we're interested in next departure time. I don't understand what we mean with the other terms here, next arrival and due in.

Seems like we use "due in" for state already. We shouldn't duplicate state in state attributes.

MartinHjelmare · 2019-04-03T11:01:03Z

homeassistant/components/oasa_telematics/sensor.py

+    @staticmethod
+    def empty_name_data():
+        """Object returned when no stop/route name data are found."""
+        return [{ATTR_STOP_NAME: 'n/a',


Instead of setting 'n/a', don't include these attributes in the state attributes at all if not known. If state isn't known we should set state to None.

MartinHjelmare · 2019-04-03T11:01:56Z

homeassistant/components/oasa_telematics/sensor.py

+            if route:
+                return route[0].get('route_departure_eng')
+        except TypeError:
+            _LOGGER.debug("Cannot get route name from OASA API")


Shouldn't this be a warning at least. How frequently could this occur?

panosmz · 2019-04-04T08:32:40Z

@MartinHjelmare This sensor fetches the next bus arrival (in minutes) of a given bus line to a given bus stop.

The estimated arrival time is what we poll the api for, but I agree that next_departure should be added. If we are only interested in next_departure, that's a different implementation.

due_in is the estimated arrival time (in minutes) of the bus to the stop, and next_arrival is the second next estimated arrival time (of the next bus in line heading to the stop). I will rename these for clarification.

MartinHjelmare · 2019-04-04T09:04:11Z

Isn't departure more interesting than arrival, if this will be used to know what bus to catch? I'm thinking we could replace arrival with departure. But I'll leave this up to you. Thanks for clarifying the context.

MartinHjelmare · 2019-04-05T22:11:58Z

homeassistant/components/oasa_telematics/sensor.py

+    @property
+    def unit_of_measurement(self):
+        """Return the unit this state is expressed in."""
+        return 'ISO8601'


The timestamp sensor device class doesn't use unit of measurement.

MartinHjelmare · 2019-04-05T22:15:16Z

homeassistant/components/oasa_telematics/sensor.py

+
+        for result in results:
+            arrival_min = int(result.get('btime2'))
+            if arrival_min is not None:


arrival_min can't be None at this point. There would have been an error at the line above where we copy to int in that case.

Move the check up before copying to int.

MartinHjelmare

Nice!

MartinHjelmare · 2019-04-05T22:35:03Z

Can be merged when the merge conflict is solved and the build passes.

panosmz · 2019-04-06T04:49:59Z

Thanks for the feedback @MartinHjelmare

homeassistant added cla-signed new-platform integration: oasa_telematics labels Mar 19, 2019

ghost added the in progress label Mar 19, 2019

houndci-bot reviewed Mar 19, 2019

View reviewed changes

panosmz mentioned this pull request Mar 19, 2019

Add documentation for OASA Telematics greek public transport sensor c home-assistant/home-assistant.io#8978

Merged

2 tasks

MartinHjelmare requested changes Apr 3, 2019

View reviewed changes

MartinHjelmare reviewed Apr 5, 2019

View reviewed changes

MartinHjelmare approved these changes Apr 5, 2019

View reviewed changes

panosmz added 6 commits April 6, 2019 01:56

add telematics sensor

fc645bb

add missing final newline

265758a

code cleanup & add manifest

89c2371

fixes from review

60de82f

fix flake8 warning

86518db

rerun gen_requirements_all.py script

9a97387

MartinHjelmare merged commit 6351c5c into home-assistant:dev Apr 6, 2019

ghost removed the in progress label Apr 6, 2019

panosmz mentioned this pull request May 19, 2019

OASA telematics unknown travel time #23542

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add OASA Telematics greek public transport sensor component #22196

Add OASA Telematics greek public transport sensor component #22196

panosmz commented Mar 19, 2019 •

edited

Loading

houndci-bot Mar 19, 2019

bkbilly commented Mar 20, 2019

panosmz commented Mar 20, 2019

MartinHjelmare left a comment

MartinHjelmare Apr 3, 2019

MartinHjelmare Apr 3, 2019

MartinHjelmare Apr 3, 2019

MartinHjelmare Apr 3, 2019

MartinHjelmare Apr 3, 2019

MartinHjelmare Apr 3, 2019 •

edited

Loading

MartinHjelmare Apr 3, 2019

MartinHjelmare Apr 3, 2019

MartinHjelmare Apr 3, 2019

panosmz commented Apr 4, 2019

MartinHjelmare commented Apr 4, 2019

MartinHjelmare Apr 5, 2019 •

edited

Loading

MartinHjelmare Apr 5, 2019 •

edited

Loading

MartinHjelmare left a comment

MartinHjelmare commented Apr 5, 2019

panosmz commented Apr 6, 2019

		})


		def setup_platform(hass, config, add_devices, discovery_info=None):

Add OASA Telematics greek public transport sensor component #22196

Add OASA Telematics greek public transport sensor component #22196

Conversation

panosmz commented Mar 19, 2019 • edited Loading

Description:

Example entry for configuration.yaml (if applicable):

Checklist:

Choose a reason for hiding this comment

bkbilly commented Mar 20, 2019

panosmz commented Mar 20, 2019

MartinHjelmare left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MartinHjelmare Apr 3, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

panosmz commented Apr 4, 2019

MartinHjelmare commented Apr 4, 2019

MartinHjelmare Apr 5, 2019 • edited Loading

Choose a reason for hiding this comment

MartinHjelmare Apr 5, 2019 • edited Loading

Choose a reason for hiding this comment

MartinHjelmare left a comment

Choose a reason for hiding this comment

MartinHjelmare commented Apr 5, 2019

panosmz commented Apr 6, 2019

panosmz commented Mar 19, 2019 •

edited

Loading

Example entry for `configuration.yaml` (if applicable):

MartinHjelmare Apr 3, 2019 •

edited

Loading

MartinHjelmare Apr 5, 2019 •

edited

Loading

MartinHjelmare Apr 5, 2019 •

edited

Loading