-
-
Notifications
You must be signed in to change notification settings - Fork 29.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add RMV public transport sensor #15814
Changes from 1 commit
c3f61e0
b9b59aa
41c34ea
f9e88b6
221167e
504049b
db76dd0
2d9c122
485ae89
6fcc48d
741a711
1403293
f67f22e
74117c3
2accb78
661bad1
26f1227
b714364
8be8b07
d21cec3
4341b8a
72c3b7e
9f29d30
844c439
cacfd46
a9949a4
1642b47
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,7 +45,7 @@ | |
'IC': 'mdi:train', | ||
'ICE': 'mdi:train', | ||
'SEV': 'mdi:checkbox-blank-circle-outline', | ||
'-': 'mdi:clock' | ||
None: 'mdi:clock' | ||
} | ||
ATTRIBUTION = "Data provided by opendata.rmv.de" | ||
|
||
|
@@ -58,7 +58,7 @@ | |
vol.Optional(CONF_DIRECTIONS, default=['']): cv.ensure_list_csv, | ||
vol.Optional(CONF_LINES, default=['']): cv.ensure_list_csv, | ||
vol.Optional(CONF_PRODUCTS, default=DEFAULT_PRODUCT): | ||
cv.ensure_list_csv, | ||
vol.All(cv.ensure_list, [vol.In(DEFAULT_PRODUCT)]), | ||
vol.Optional(CONF_TIMEOFFSET, default=0): cv.positive_int, | ||
vol.Optional(CONF_MAXJOURNEYS, default=5): cv.positive_int, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do we call this in other sensor commuting platforms? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok. It would be good to come up with a descriptive name and then have all platforms use the same term. But it can wait to a different PR. But please separate words by underscore in variable and constant names and strings. So change to |
||
vol.Optional(CONF_NAME, default=DEFAULT_NAME): cv.string}] | ||
|
@@ -93,7 +93,7 @@ def __init__(self, station, destinations, directions, | |
self.data = RMVDepartureData(station, destinations, directions, | ||
lines, products, timeoffset, maxjourneys) | ||
self._state = STATE_UNKNOWN | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Init state as |
||
self._icon = ICONS['-'] | ||
self._icon = ICONS[None] | ||
|
||
@property | ||
def name(self): | ||
|
@@ -103,9 +103,7 @@ def name(self): | |
@property | ||
def available(self): | ||
"""Return True if entity is available.""" | ||
if self._state is STATE_UNKNOWN: | ||
return False | ||
return True | ||
return self._state != STATE_UNKNOWN | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't use return self._state is not None Is there no use to differ between unknown state and unavailable? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll check that. |
||
|
||
@property | ||
def state(self): | ||
|
@@ -119,10 +117,10 @@ def state_attributes(self): | |
try: | ||
result = { | ||
'next_departures': [val for val in self.data.departures[1:]], | ||
'direction': self.data.departures[0].get('direction', '-'), | ||
'line': self.data.departures[0].get('number', '-'), | ||
'minutes': self.data.departures[0].get('minutes', '-'), | ||
'product': self.data.departures[0].get('product', '-'), | ||
'direction': self.data.departures[0].get('direction', None), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
'line': self.data.departures[0].get('number', None), | ||
'minutes': self.data.departures[0].get('minutes', None), | ||
'product': self.data.departures[0].get('product', None), | ||
} | ||
except IndexError: | ||
pass | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could return empty dict here instead of defining it at the top. |
||
|
@@ -142,16 +140,16 @@ def update(self): | |
"""Get the latest data and update the state.""" | ||
self.data.update() | ||
if not self.data.departures: | ||
self._state = '-' | ||
self._icon = ICONS['-'] | ||
self._state = None | ||
self._icon = ICONS[None] | ||
return | ||
else: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. change to guard clause if not self.data.departures:
# something
return
#something else There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since you have return in the |
||
if self._name is DEFAULT_NAME: | ||
if self._name == DEFAULT_NAME: | ||
self._name = self.data.station | ||
else: | ||
self._name = self._name | ||
self._station = self.data.station | ||
self._state = self.data.departures[0].get('minutes', '-') | ||
self._icon = ICONS[self.data.departures[0].get('product', '-')] | ||
self._state = self.data.departures[0].get('minutes', None) | ||
self._icon = ICONS[self.data.departures[0].get('product', None)] | ||
return | ||
|
||
|
||
class RMVDepartureData: | ||
|
@@ -182,7 +180,7 @@ def update(self): | |
self.departures = {} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We set this as a list in init. We probably shouldn't change the type. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well spotted. Not sure when that happened. ;) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I rather initialise as a dict. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But we use |
||
_LOGGER.warning("Returned data not understood") | ||
return | ||
self.station = _data.get('station', '-') | ||
self.station = _data.get('station', None) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
_deps = [] | ||
for journey in _data['journeys']: | ||
# find the first departure meeting the criteria | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better validate each product user configured is in the default product list
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fixed that. Thanks for the hint.