Skip to content

File sensor#7569

Merged
fabaff merged 5 commits into
home-assistant:devfrom
fabaff:file-sensor
May 15, 2017
Merged

File sensor#7569
fabaff merged 5 commits into
home-assistant:devfrom
fabaff:file-sensor

Conversation

@fabaff
Copy link
Copy Markdown
Member

@fabaff fabaff commented May 12, 2017

Description:

The file sensor reads the last entry of a plain text file. The entries can be simple values or JSON with is handled by our templating feature

Related issue (if applicable): fixes File sensors

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

Example entry for configuration.yaml (if applicable):

sensor:
  - platform: file
    file_path: /home/fab/.homeassistant/test/sensor-test.txt
    value_template: '{{ value_json.temperature }}'
    unit_of_measurement: '°C'

Checklist:

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

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.

Comment thread homeassistant/components/sensor/file.py Outdated
return self._state

@asyncio.coroutine
def async_update(self):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should not do I/O inside an async context. Since you're dealing with a file, best to just keep the method update

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks

Comment thread homeassistant/components/sensor/file.py Outdated
with open(self._file_path, 'r', encoding='utf-8') as file_data:
data = file_data.readlines()[-1].strip()
except (IndexError, FileNotFoundError, IsADirectoryError):
data = STATE_UNKNOWN
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please return None instead of STATE_UNKNOWN, the Entity helper class will set it to unknown

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also return when an error occurs. Templates don't need to run on None

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Changed

Comment thread tests/components/sensor/test_file.py Outdated
m_open = MockOpen()
with patch('homeassistant.components.sensor.file.open', m_open,
create=True):
content = open('mock.file1')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wait, why would you open the file? Shouldn't you instead trigger an update to the component here ?

Comment thread tests/components/sensor/test_file.py Outdated
}
}

assert setup_component(self.hass, 'sensor', config)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You should run this inside the patch context and block till done. The sensor updates on load, so it will hit your mock.

Comment thread tests/components/sensor/test_file.py Outdated

def create_file(path, data):
"""Create a sensor file."""
with open(path, 'w') as test_file:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should never do any actual I/O in tests. That's why we use mock_open.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks

Comment thread tests/components/sensor/test_file.py Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

expected 2 blank lines, found 1

Comment thread tests/components/sensor/test_file.py Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

'json' imported but unused

Copy link
Copy Markdown
Member

@balloob balloob left a comment

Choose a reason for hiding this comment

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

Ok to merge when tests pass 🐬

(if tests don't pass feel free to revert my optimization)

@fabaff
Copy link
Copy Markdown
Member Author

fabaff commented May 15, 2017

Thanks

@fabaff fabaff merged commit f25347d into home-assistant:dev May 15, 2017
@fabaff fabaff deleted the file-sensor branch May 15, 2017 12:26
@arsaboo
Copy link
Copy Markdown
Contributor

arsaboo commented Jul 12, 2017

Is it possible to write to the file using HA to be read by this sensor?

@Landrash
Copy link
Copy Markdown
Contributor

@arsaboo https://home-assistant.io/components/notify.command_line/ could be used in combination with echo.

@home-assistant home-assistant locked and limited conversation to collaborators Jul 14, 2017
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.

7 participants