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

Adds folder sensor #12208

Merged
merged 20 commits into from
Feb 22, 2018
Merged

Adds folder sensor #12208

merged 20 commits into from
Feb 22, 2018

Conversation

robmarkcole
Copy link
Contributor

@robmarkcole robmarkcole commented Feb 6, 2018

Description:

Home-assistant custom component for monitoring the contents of a folder.

The state of the sensor is the size in MB of files within the folder that meet the filter criteria. The use case is detecting when a file is created in a folder . For example, I have a USB camera attached to my HA instance that saves a new timestamped photo when motion is detected. This sensor allows me to detect when another image is saved.
The number of files in the folder and the size in bytes are exposed in attributes.

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

Example entry for configuration.yaml (if applicable):

sensor:
  - platform: folder
    folder: /share/motion
    filter: '*capture.jpg'
  - platform: folder
    folder: /config

Checklist:

  • The code change is tested and works locally.

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
  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • [x ] New files were added to .coveragerc.

If the code does not interact with devices:

  • [x ] Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • [ x] Tests have been added to verify that the new code works.

The state of the sensor is the time that the most recently modified
file in a folder was modified.
return

def get_sorted_files_list(self, folder_path, filter_term):
"""Rerturn the sorted list of files in a directory, applying filter.
Copy link
Contributor

@arsaboo arsaboo Feb 6, 2018

Choose a reason for hiding this comment

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

Typo....return. Also, shorten the docstring.

return sorted_files_list

def get_last_updated(self, recent_modified_file):
"""Rerturn the datetime a file was last modified."""
Copy link
Contributor

Choose a reason for hiding this comment

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

Same...typo return

Makes the recommended edits to docstrings
@Qxlkdr
Copy link
Contributor

Qxlkdr commented Feb 6, 2018

Don't forget to add your file to .coveragerc as described in the pull request template.


def setup_platform(hass, config, add_devices, discovery_info=None):
"""Set up the folder sensor."""
folder = Folder(config.get(CONF_FOLDER_PATHS), config.get(CONF_FILTER))
Copy link
Member

Choose a reason for hiding this comment

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

All paths have to be whitelisted by checking it with hass.config.is_allowed_path(path)

self._last_updated = ''
self._name = folder_path.split("/")[-2]
self._unit_of_measurement = ''
self.update()
Copy link
Member

Choose a reason for hiding this comment

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

Remove this. It's already called during entity addition.


self._last_updated = self.get_last_updated(
self._recent_modified_file)
return
Copy link
Member

Choose a reason for hiding this comment

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

Remove this unnecessary return statement.

@property
def device_state_attributes(self):
"""Return other details about the sensor state."""
attrs = {}
Copy link
Member

Choose a reason for hiding this comment

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

Please construct the whole dict directly.

attr = {
    'folder': self._folder_path,
   ...
}

self._recent_modified_file = ''
self._last_updated = ''
self._name = folder_path.split("/")[-2]
self._unit_of_measurement = ''
Copy link
Member

Choose a reason for hiding this comment

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

This is not an allowed unit of measurement.

self._sorted_files_list = []
self._number_of_files = None
self._recent_modified_file = ''
self._last_updated = ''
Copy link
Member

Choose a reason for hiding this comment

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

Init as None.

robmarkcole added a commit to robmarkcole/HASS-folder-sensor that referenced this pull request Feb 12, 2018
Address requests changes
Copy link
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Nice! Can be merged when build passes.

Copy link
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Sorry, I forgot about tests. This should be tested since it's not integrating a 3rd party device.

setup_component(self.hass, 'sensor', config))
assert len(self.hass.states.entity_ids()) == 1
state = self.hass.states.get('sensor.test_folder')
#assert state.state == '0.0'

Choose a reason for hiding this comment

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

block comment should start with '# '

@robmarkcole
Copy link
Contributor Author

robmarkcole commented Feb 20, 2018

@MartinHjelmare @arsaboo @balloob @fabaff I've made all the required changes now, don't know what that strange error from Travis is about. Cheers

Fix setup with else statement
self._number_of_files = None
self._recent_modified_file = None
self._last_updated = None
self._name = folder_path.split("/")[-2]
Copy link
Member

Choose a reason for hiding this comment

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

You must not use Windows

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As in, this will not work on Windows? OK will try something else

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

'filter': self._filter_term,
'modified_file': self._recent_modified_file.split('/')[-1],
'number_of_files': len(self._sorted_files_list),
'files': [f.split('/')[-1] for f in self._sorted_files_list]
Copy link
Member

Choose a reason for hiding this comment

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

seperator

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

'filter': self._filter_term,
'modified_file': os.path.split(self._recent_modified_file)[1],
'number_of_files': len(self._sorted_files_list),
'files': [os.path.split(f)[1] for f in self._sorted_files_list]
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 not do this. This is way too much data for in the state machine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

attr = {
'path': self._folder_path,
'filter': self._filter_term,
'modified_file': os.path.split(self._recent_modified_file)[1],
Copy link
Member

Choose a reason for hiding this comment

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

Let's drop this attribute. If people want this info, we should get a watchdog component to monitor folders/files and fire events. If people want to automate on this, it will not work if 2 files get added between 2 updates. Then they want to have that fixed etc. I think that we should have people do the right thing right away by not including this, so they will look for the watchdog component instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK


def update(self):
"""Update the sensor."""
self._sorted_files_list = get_sorted_files_list(
Copy link
Member

Choose a reason for hiding this comment

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

It's better if we don't keep this in memory but instead only set the instance variables that we want to derive from it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK

@robmarkcole
Copy link
Contributor Author

@balloob and @MartinHjelmare all changes have beeb made I believe

@balloob balloob merged commit 4d7fb2c into home-assistant:dev Feb 22, 2018
@robmarkcole robmarkcole deleted the folder-sensor branch February 22, 2018 07:24
@balloob balloob mentioned this pull request Feb 22, 2018
@home-assistant home-assistant locked and limited conversation to collaborators May 29, 2018
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