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

improved skill settings #1151

Merged
merged 5 commits into from
Oct 13, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion mycroft/skills/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ def settings(self):
try:
return self._settings
except:
self._settings = SkillSettings(self._dir)
self._settings = SkillSettings(self._dir, self.name)
return self._settings

def bind(self, emitter):
Expand Down
250 changes: 182 additions & 68 deletions mycroft/skills/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,31 +27,28 @@

import json
from threading import Timer

from os.path import isfile, join
from os.path import isfile, join, expanduser

from mycroft.api import DeviceApi
from mycroft.util.log import LOG
from mycroft.configuration import ConfigurationManager


SKILLS_DIR = "/opt/mycroft/skills"


# TODO: allow deleting skill when skill is deleted
class SkillSettings(dict):
"""
SkillSettings creates a dictionary that can easily be stored
""" SkillSettings creates a dictionary that can easily be stored
to file, serialized as json. It also syncs to the backend for
skill settings

Args:
settings_file (str): Path to storage file
"""

def __init__(self, directory):
def __init__(self, directory, name):
super(SkillSettings, self).__init__()
self.api = DeviceApi()
self._device_identity = self.api.identity.uuid
self.config = ConfigurationManager.get()
self.name = name
# set file paths
self._settings_path = join(directory, 'settings.json')
self._meta_path = join(directory, 'settingsmeta.json')
Expand All @@ -60,12 +57,47 @@ def __init__(self, directory):
self.loaded_hash = hash(str(self))

# if settingsmeta.json exists
# this block of code is a control flow for
# different scenarios that may arises with settingsmeta
if isfile(self._meta_path):
self.settings_meta = self._load_settings_meta()
self.settings = self._get_settings()
self._send_settings_meta()
# start polling timer
Timer(60, self._poll_skill_settings).start()
LOG.info("settingsmeta.json exist for {}".format(self.name))
settings_meta = self._load_settings_meta()
hashed_meta = hash(str(settings_meta)+str(self._device_identity))
# check if hash is different from the saved hashed
if self._is_new_hash(hashed_meta):
LOG.info("looks like settingsmeta.json" +
"has changed for {}".format(self.name))
# TODO: once the delete api for device is created uncomment
# if self._uuid_exist():
# LOG.info("a uuid exist for {}".format(self.name) +
# "deleting old one")
# old_uuid = self._load_uuid()
# self._delete_metatdata(old_uuid)
LOG.info("sending settingsmeta.json for {}".format(self.name) +
"to home.mycroft.ai")
new_uuid = self._send_settings_meta(settings_meta, hashed_meta)
self._save_uuid(new_uuid)
self._save_hash(hashed_meta)
else: # if hash is old
should_exist_in_backend = False
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be clearer to rename to something like found_in_backend?

settings = self._get_settings()
# checks backend if th settings have been deleted
# through web ui
for skill in settings:
if skill["identifier"] == str(hashed_meta):
should_exist_in_backend = True
# if it's been deleted from web ui
# resend the settingsmeta.json
if should_exist_in_backend is False:
LOG.info("seems like it got deleted from home... " +
"sending settingsmeta.json for " +
"{}".format(self.name))
new_uuid = self._send_settings_meta(
settings_meta, hashed_meta)
self._save_uuid(new_uuid)
self._save_hash(hashed_meta)

Timer(60, self._poll_skill_settings, [hashed_meta]).start()

self.load_skill_settings()

Expand All @@ -74,80 +106,154 @@ def _is_stored(self):
return hash(str(self)) == self.loaded_hash

def __getitem__(self, key):
""" Get key
"""
Copy link
Contributor

@MatthewScholefield MatthewScholefield Oct 12, 2017

Choose a reason for hiding this comment

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

How about """ Get key """ on one line? (Same below)

return super(SkillSettings, self).__getitem__(key)

def __setitem__(self, key, value):
"""
Add/Update key.
""" Add/Update key.
"""
return super(SkillSettings, self).__setitem__(key, value)

def _load_settings_meta(self):
""" loads settings metadata from skills path
"""
with open(self._meta_path) as f:
data = json.load(f)
return data

def _skill_exist_in_backend(self):
"""
see if skill settings already exist in the backend
"""
skill_identity = self._get_skill_identity()
for skill_setting in self.settings:
if skill_identity == skill_setting["identifier"]:
return True
return False
def _send_settings_meta(self, settings_meta, hashed_meta):
""" send settingsmeta.json to the backend

def _send_settings_meta(self):
"""
send settingsmeta.json to the backend if skill doesn't
already exist
Args:
param1 (dict): dictionary of the current settings meta data
param1 (int): hashed settings meta data

Returns:
uuid (str): a unique id for the setting meta data
"""
try:
if self._skill_exist_in_backend() is False:
response = self._put_metadata(self.settings_meta)
settings_meta["identifier"] = str(hashed_meta)
self._put_metadata(settings_meta)
settings = self._get_settings()
skill_identity = str(hashed_meta)
uuid = None
# TODO: note uuid should be returned from the put request
for skill_setting in settings:
if skill_setting['identifier'] == skill_identity:
uuid = skill_setting["uuid"]
return uuid
except Exception as e:
LOG.error(e)

def _poll_skill_settings(self):
def _load_uuid(self):
""" loads uuid

Returns:
uuid (int): uuid of the previous settingsmeta
"""
If identifier exists for this skill poll to backend to
request settings and store it if it changes
TODO: implement as websocket
directory = self.config.get("skills")["directory"]
directory = join(directory, self.name)
directory = expanduser(directory)
uuid_file = join(directory, 'uuid')
if isfile(uuid_file):
with open(uuid_file, 'r') as f:
uuid = f.read()
return uuid

def _save_uuid(self, uuid):
""" saves uuid to path

Args:
param1 (int): uuid of new seetingsmeta
"""
if self._skill_exist_in_backend():
try:
# update settings
self.settings = self._get_settings()
skill_identity = self._get_skill_identity()
for skill_setting in self.settings:
if skill_setting['identifier'] == skill_identity:
sections = skill_setting['skillMetadata']['sections']
for section in sections:
for field in section["fields"]:
self.__setitem__(field["name"], field["value"])

# store value if settings has changed from backend
self.store()

except Exception as e:
LOG.error(e)

# poll backend every 60 seconds for new settings
Timer(60, self._poll_skill_settings).start()

def _get_skill_identity(self):
LOG.info("saving uuid {}".format(str(uuid)))
directory = self.config.get("skills")["directory"]
directory = join(directory, self.name)
directory = expanduser(directory)
uuid_file = join(directory, 'uuid')
with open(uuid_file, 'w') as f:
f.write(str(uuid))

def _save_hash(self, hashed_meta):
""" saves hashed_meta to path

Args:
param1 (int): hashed of new seetingsmeta
"""
returns the skill identifier
LOG.info("saving hash {}".format(str(hashed_meta)))
directory = self.config.get("skills")["directory"]
directory = join(directory, self.name)
directory = expanduser(directory)
hash_file = join(directory, 'hash')
with open(hash_file, 'w') as f:
f.write(str(hashed_meta))

def _uuid_exist(self):
""" checks if there is a uuid file

Returns:
bool: True if uuid file exist False otherwise
"""
directory = self.config.get("skills")["directory"]
directory = join(directory, self.name)
directory = expanduser(directory)
uuid_file = join(directory, 'uuid')
return isfile(uuid_file)

def _is_new_hash(self, hashed_meta):
""" checks if the stored hash is the same as current.
if the hashed file does not exist, usually in the
case of first load, then the create it and return True

Args:
param1 (int): hash of metadata and uuid

Returns:
bool: True if hash is new False otherwise
"""
directory = self.config.get("skills")["directory"]
directory = join(directory, self.name)
directory = expanduser(directory)
hash_file = join(directory, 'hash')
if isfile(hash_file):
with open(hash_file, 'r') as f:
current_hash = f.read()
return False if current_hash == str(hashed_meta) else True
return True

def _poll_skill_settings(self, hashed_meta):
""" If identifier exists for this skill poll to backend to
request settings and store it if it changes
TODO: implement as websocket

Args:
param1 (int): the hashed identifier

"""
LOG.info("getting settings from home.mycroft.ai")
try:
return self.settings_meta["identifier"]
# update settings
settings = self._get_settings()
skill_identity = str(hashed_meta)
for skill_setting in settings:
if skill_setting['identifier'] == skill_identity:
sections = skill_setting['skillMetadata']['sections']
for section in sections:
for field in section["fields"]:
self.__setitem__(field["name"], field["value"])
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure, but would it make sense to write it as:

self[field['name']] = field['value']


# store value if settings has changed from backend
self.store()

except Exception as e:
LOG.error(e)
return None

# poll backend every 60 seconds for new settings
Timer(60, self._poll_skill_settings, [hashed_meta]).start()

def load_skill_settings(self):
"""
If settings.json exist, open and read stored values into self
""" If settings.json exist, open and read stored values into self
"""
if isfile(self._settings_path):
with open(self._settings_path) as f:
Expand All @@ -161,17 +267,15 @@ def load_skill_settings(self):
LOG.error(e)

def _get_settings(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

This hasn't been changed in this PR, but should we rename this to _get_remote_settings for clarity?

"""
Get skill settings for this device from backend
""" Get skill settings for this device from backend
"""
return self.api.request({
"method": "GET",
"path": self._api_path
})

def _put_metadata(self, settings_meta):
"""
PUT settingsmeta to backend to be configured in home.mycroft.ai.
""" PUT settingsmeta to backend to be configured in home.mycroft.ai.
used in plcae of POST and PATCH
"""
return self.api.request({
Expand All @@ -180,9 +284,19 @@ def _put_metadata(self, settings_meta):
"json": settings_meta
})

def store(self, force=False):
def _delete_metatdata(self, uuid):
""" Deletes the current skill metadata

Args:
param1 (str): unique id of the skill
"""
Store dictionary to file if a change has occured.
return self.api.request({
"method": "DELETE",
"path": "/skill/{}".format(uuid)
})

def store(self, force=False):
""" Store dictionary to file if a change has occured.

Args:
force: Force write despite no change
Expand Down
Loading