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 london_underground #8272

Merged
merged 15 commits into from Jul 2, 2017

Conversation

Projects
None yet
6 participants
@robmarkcole
Contributor

robmarkcole commented Jul 1, 2017

Description:

This sensor displays the current status of London Underground tube lines. If there is a disruption, a description of the disruption is viewable as an attribute.
This pull request replaces #8235

Example entry for configuration.yaml:

sensor:
  - platform: london_underground
    line:
      - Bakerloo
      - Central
      - Circle
      - District
      - DLR
      - Hammersmith & City
      - Jubilee
      - London Overground
      - Metropolitan
      - Northern
      - Piccadilly
      - TfL Rail
      - Victoria
      - Waterloo & City

Pull request in home-assistant.github.io (if applicable): home-assistant/home-assistant.io#2902

Checklist:

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

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

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass

If the code does not interact with devices:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • Tests have been added to verify that the new code works.
Add tube_state
Add tube_state sensor
'Waterloo & City']
URL = 'https://api.tfl.gov.uk/line/mode/tube,overground,dlr,tflrail/status'
CONFIG_SCHEMA = vol.Schema({

This comment has been minimized.

@balloob

balloob Jul 1, 2017

Member

You are a platform so please define a platform schema:

PLATFORM_SCHEMA = vol.Schema({
    vol.Required(CONF_LINE): vol.In(TUBE_LINES),
})
ATTRIBUTION = "Powered by TfL Open Data"
CONF_LINE = 'line'
DOMAIN = 'tube_state'

This comment has been minimized.

@balloob

balloob Jul 1, 2017

Member

platforms don't have a domain. Please remove this line.

sensors.append(LondonTubeSensor(line, data))
add_devices(sensors, True)
_LOGGER.info("The tube_state component is ready!")

This comment has been minimized.

@balloob

balloob Jul 1, 2017

Member

Please remove these two log lines.

def update(self):
"""Get the latest data from TFL."""
response = requests.get(URL)
_LOGGER.info("TFL Request made")

This comment has been minimized.

@balloob

balloob Jul 1, 2017

Member

Please don't log this as info. If it needs to be logged at all, make it debug

"""Stop everything that was started."""
self.hass.stop()
def test_setup_with_config(self):

This comment has been minimized.

@balloob

balloob Jul 1, 2017

Member

Please merge this test with the one below. So instead of tube_state.setup_platform(…) just call setup_component(…).

Then you can read the state from the state machine.

state = self.hass.states.get('sensor.london_overground')
assert state.state == 'bla'
assert state.attributes.get('Description') == 'Something'

@balloob balloob added this to the 0.48 milestone Jul 1, 2017

Make corrections
Correct PLATFORM_SCHEMA
self.hass.stop()
@requests_mock.Mocker()

This comment has been minimized.

@houndci-bot

houndci-bot Jul 1, 2017

too many blank lines (2)

@balloob

balloob approved these changes Jul 1, 2017

@balloob

This comment has been minimized.

Member

balloob commented Jul 1, 2017

🎉 🎉 🐬

robmarkcole added some commits Jul 1, 2017

Correct format of test
Test still failing, don’t understand why
@lwis

This comment has been minimized.

Member

lwis commented Jul 1, 2017

@robmarkcole self.hass.states.get('sensor.london_overground') is returning None.

@robmarkcole

This comment has been minimized.

Contributor

robmarkcole commented Jul 1, 2017

I don't understand why self.hass.states.get('sensor.london_overground') is returning None..
Test does pass with:

sensor = self.entities[0]
self.assertEqual(sensor.name, 'London Overground')
self.assertEqual(sensor.state, 'Minor Delays')
self.assertEqual(sensor.device_state_attributes['Description'], 'something')
Make test pass
Preferred method below returns None

state = self.hass.states.get('sensor.london_overground')
sensor = self.entities[0]
self.assertEqual(sensor.name, 'London Overground')
self.assertEqual(sensor.state, 'Minor Delays')
self.assertEqual(sensor.device_state_attributes['Description'], 'something')

This comment has been minimized.

@houndci-bot

houndci-bot Jul 1, 2017

line too long (84 > 79 characters)

setup_component(self.hass, 'sensor', {'tube_state': self.config}))
#state = self.hass.states.get('sensor.london_overground')
#assert state.state == 'Minor Delays'
#assert state.attributes.get('Description') == 'something'

This comment has been minimized.

@houndci-bot

houndci-bot Jul 1, 2017

block comment should start with '# '

self.assertTrue(
setup_component(self.hass, 'sensor', {'tube_state': self.config}))
#state = self.hass.states.get('sensor.london_overground')
#assert state.state == 'Minor Delays'

This comment has been minimized.

@houndci-bot

houndci-bot Jul 1, 2017

block comment should start with '# '

self.assertEqual(len(self.entities), 1)
self.assertTrue(
setup_component(self.hass, 'sensor', {'tube_state': self.config}))
#state = self.hass.states.get('sensor.london_overground')

This comment has been minimized.

@houndci-bot

houndci-bot Jul 1, 2017

block comment should start with '# '

self.assertEqual(sensor.name, 'London Overground')
self.assertEqual(sensor.state, 'Minor Delays')
self.assertEqual(sensor.device_state_attributes['Description'],
'something')

This comment has been minimized.

@houndci-bot

houndci-bot Jul 1, 2017

continuation line over-indented for visual indent

self.assertEqual(len(self.entities), 1)
self.assertTrue(
setup_component(self.hass, 'sensor', {'tube_state': self.config}))
# state = self.hass.states.get('sensor.london_overground')

This comment has been minimized.

@robmarkcole

robmarkcole Jul 1, 2017

Contributor

self.hass.states.get Returns None

# assert state.state == 'Minor Delays'
# assert state.attributes.get('Description') == 'something'
sensor = self.entities[0]

This comment has been minimized.

@robmarkcole

robmarkcole Jul 1, 2017

Contributor

This passes. Copied technique from test_wsdot.py

This comment has been minimized.

@balloob

balloob Jul 1, 2017

Member

This technique should not be used. please remove.

@balloob

The test needs fixing.

class TestLondonTubeSensor(unittest.TestCase):
"""Test the tube_state platform."""
def add_entities(self, new_entities, update_before_add=False):

This comment has been minimized.

@balloob

balloob Jul 1, 2017

Member

Remove this.

"""Initialize values for this testcase class."""
self.hass = get_test_home_assistant()
self.config = {CONF_LINE: ['London Overground']}
self.entities = []

This comment has been minimized.

@balloob

balloob Jul 1, 2017

Member

Remove this.

def test_setup(self, mock_req):
"""Test for operational tube_state sensor with proper attributes."""
mock_req.get(URL, text=load_fixture('tube_state.json'))
tube_state.setup_platform(self.hass, self.config, self.add_entities)

This comment has been minimized.

@balloob

balloob Jul 1, 2017

Member

Remove this.

"""Test for operational tube_state sensor with proper attributes."""
mock_req.get(URL, text=load_fixture('tube_state.json'))
tube_state.setup_platform(self.hass, self.config, self.add_entities)
self.assertEqual(len(self.entities), 1)

This comment has been minimized.

@balloob

balloob Jul 1, 2017

Member

And this.

self.assertEqual(len(self.entities), 1)
self.assertTrue(
setup_component(self.hass, 'sensor', {'tube_state': self.config}))
# state = self.hass.states.get('sensor.london_overground')

This comment has been minimized.

@balloob

balloob Jul 1, 2017

Member

Instead of commenting it out, check what the entity name is. It's right there in the test logs.

# assert state.state == 'Minor Delays'
# assert state.attributes.get('Description') == 'something'
sensor = self.entities[0]

This comment has been minimized.

@balloob

balloob Jul 1, 2017

Member

This technique should not be used. please remove.

@balloob balloob removed this from the 0.48 milestone Jul 1, 2017

Make requested changes to test, not working
Test fails with:

AssertionError: assert 0 > 0
where 0 = len([])

Surely I need tube_state.setup_platform ?
import unittest
import requests_mock
from homeassistant.components.sensor import tube_state

This comment has been minimized.

@houndci-bot

houndci-bot Jul 2, 2017

'homeassistant.components.sensor.tube_state' imported but unused

setup_component(self.hass, 'sensor', {'tube_state': self.config}))
ids = self.hass.states.entity_ids()
assert len(ids) > 0

This comment has been minimized.

@robmarkcole

robmarkcole Jul 2, 2017

Contributor

I have zero entities

robmarkcole added some commits Jul 2, 2017

Fixed test
Config was wrong
import unittest
import requests_mock
from homeassistant.components.sensor import london_tube

This comment has been minimized.

@houndci-bot

houndci-bot Jul 2, 2017

'homeassistant.components.sensor.london_tube' imported but unused

import unittest
import requests_mock
from homeassistant.components.sensor import london_tube

This comment has been minimized.

@houndci-bot

houndci-bot Jul 2, 2017

'homeassistant.components.sensor.london_tube' imported but unused

@robmarkcole robmarkcole changed the title from Add tube_state to Add london_tube Jul 2, 2017

Update name to london_underground
Make consistent
import unittest
import requests_mock
from homeassistant.components.sensor import london_underground

This comment has been minimized.

@houndci-bot

houndci-bot Jul 2, 2017

'homeassistant.components.sensor.london_underground' imported but unused

@robmarkcole robmarkcole changed the title from Add london_tube to Add london_underground Jul 2, 2017

@robmarkcole

This comment has been minimized.

Contributor

robmarkcole commented Jul 2, 2017

I'm not sure what needs to be done about the failing Travis @ https://travis-ci.org/home-assistant/home-assistant/jobs/249272537

@balloob

balloob approved these changes Jul 2, 2017

@balloob

This comment has been minimized.

Member

balloob commented Jul 2, 2017

Woohooo 🎉

@balloob balloob merged commit 865865c into home-assistant:dev Jul 2, 2017

3 of 4 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
cla-bot Everyone involved has signed the CLA
coverage/coveralls Coverage decreased (-0.03%) to 93.608%
Details
hound No violations found. Woof!
@robmarkcole

This comment has been minimized.

Contributor

robmarkcole commented Jul 2, 2017

Fantastic 🤗

@robmarkcole robmarkcole deleted the robmarkcole:london_tube_state branch Jul 2, 2017

@balloob balloob referenced this pull request Jul 13, 2017

Merged

0.49 #8468

dethpickle added a commit to dethpickle/home-assistant that referenced this pull request Aug 18, 2017

Add london_underground (home-assistant#8272)
* Add tube_state

Add tube_state sensor

* Final cleanup

* Make corrections

Correct PLATFORM_SCHEMA

* Fix space

* Make test pass

* Correct format of test

Test still failing, don’t understand why

* correct description

* Make test pass

Preferred method below returns None

state = self.hass.states.get('sensor.london_overground')

* Format for hound

* indent

* Make requested changes to test, not working

Test fails with:

AssertionError: assert 0 > 0
where 0 = len([])

Surely I need tube_state.setup_platform ?

* Fixed test

Config was wrong

* Change component name to london_tube

* Update name to london_underground

Make consistent

* cleanup

@home-assistant home-assistant locked and limited conversation to collaborators Oct 20, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.