Skip to content
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

Improve Swiss public transport #30715

Closed

Conversation

agners
Copy link
Member

@agners agners commented Jan 12, 2020

Description:

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

Example entry for configuration.yaml (if applicable):

- platform: swiss_public_transport
  name: Zürich Hardbrücke         
  limit: 6                        
  stationboard:                   
  - Zürich Hardbrücke, Bahnhof    
  - Zürich Hardbrücke             

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.
  • I have followed the development checklist

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

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

  • The manifest file has all fields filled out correctly. Update and include derived files by running python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

Those information are available from OpendataTransport object, so no
need to pass it to the sensor separately.
Add a list of all upcomming connections. Also make the count of
connections configureable through a new config parameter "limit".
This can be useful for custom lovelace cards which display more
than one connection.
@probot-home-assistant
Copy link

Hey there @fabaff, mind taking a look at this pull request as its been labeled with a integration (swiss_public_transport) you are listed as a codeowner for? Thanks!

The Swiss public transport API supports stationboard type requests to
get all departing connections from a given station. Add this type of
sensor to the swiss_public_transport integration. This is especially
useful for smaller train or bus station to just display when the next
departure is and whether the departure is delayed.
@agners agners force-pushed the swiss-public-transport-improvements branch from ba08d0c to b1e311f Compare January 12, 2020 21:38
async def async_setup_platform(hass, config, async_add_entities, discovery_info=None):
"""Set up the Swiss public transport sensor."""

name = config.get(CONF_NAME)
start = config.get(CONF_START)
destination = config.get(CONF_DESTINATION)
stationboard = config.get(CONF_STATIONBOARD)
limit = config.get(CONF_LIMIT, 5)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default should be set in the validation -> vol.Optional(CONF_LIMIT, default=5): cv.positive_int,

vol.Optional(CONF_DESTINATION): cv.string,
vol.Optional(CONF_START): cv.string,
vol.Optional(CONF_STATIONBOARD): vol.All(cv.ensure_list, [cv.string]),
vol.Optional(CONF_LIMIT): cv.positive_int,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should work with vol.Inclusive/vol.Exclusive here. Like CONF_START and CONF_DESTINATION together.

Perhaps it could make sense to combine CONF_START and CONF_STATIONBOARD and if the CONF_DESTINATION is not given show the stationboard.

This is a breaking change but that's inevitable.


async def async_update(self):
"""Get the latest data from opendata.ch and update the states."""
_LOGGER.info("async_update called")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like something that is only useful for developing.

@@ -103,6 +186,9 @@ def device_state_attributes(self):
self._opendata.connections[0]["departure"]
) - dt_util.as_local(dt_util.utcnow())

# Convert to a list since this is more approprate
connections = [c[1] for c in sorted(self._opendata.connections.items())]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use proper variable name aka not c.

"""Initialize the sensor."""
self._opendata = opendata
self._name = name
self._remaining_time = ""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
self._remaining_time = ""
self._remaining_time = None

@springstan springstan changed the title Swiss public transport improvements Improve Swiss public transport Feb 12, 2020
@stale
Copy link

stale bot commented Mar 21, 2020

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days.
Thank you for your contributions.

@stale stale bot added the stale label Mar 21, 2020
@balloob balloob closed this Mar 25, 2020
@lock lock bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants