From 69fef6f3ff1a7160b2c5af344cc0dc78e883c972 Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Tue, 11 May 2021 14:12:48 -0700 Subject: [PATCH 01/10] Validate Yaml Files on Startup; Add Validation Schemas This adds Cerberus as a required package. Cerberus is used to validate the configuration yaml against an included schema. It will also validate the the scenes yaml against an included schema if scenes are enabled. The validation is done first thing on startup, and if it fails it will print a nice clear message to the user and exit the program. I realize that exiting is a harsh result, but I strongly believe that it is better for users this way. Otherwise configuration errors may become latent errors that are hard to diagnose. The validation is moderately permissive. That is, I have allowed unkown keys to be present in a few places. Notably, I did this in the devices sections of the mqtt portion. This is because in the past we had named a few of the topic keys differently or allowed their use in places where they are no longer used. I believe, for 99.9% of users, if your installation is working currently. The validation will pass. However, there may be some odd situations that are being tolerated by the code that may cause a validation error. --- insteon_mqtt/cmd_line/main.py | 5 + insteon_mqtt/config.py | 223 +++++++++++++ insteon_mqtt/schemas/config-schema.yaml | 414 ++++++++++++++++++++++++ insteon_mqtt/schemas/scenes-schema.yaml | 89 +++++ requirements.txt | 1 + scenes.yaml | 8 +- 6 files changed, 736 insertions(+), 4 deletions(-) create mode 100644 insteon_mqtt/schemas/config-schema.yaml create mode 100644 insteon_mqtt/schemas/scenes-schema.yaml diff --git a/insteon_mqtt/cmd_line/main.py b/insteon_mqtt/cmd_line/main.py index ab60dd98..fc1daa48 100644 --- a/insteon_mqtt/cmd_line/main.py +++ b/insteon_mqtt/cmd_line/main.py @@ -430,6 +430,11 @@ def parse_args(args): def main(mqtt_converter=None): args = parse_args(sys.argv[1:]) + # Validate the configuration file + val_errors = config.validate(args.config) + if val_errors != "": + return val_errors + # Load the configuration file. cfg = config.load(args.config) diff --git a/insteon_mqtt/config.py b/insteon_mqtt/config.py index 0de2d7ea..ace1e725 100644 --- a/insteon_mqtt/config.py +++ b/insteon_mqtt/config.py @@ -9,7 +9,11 @@ #=========================================================================== import os.path +import re +import ipaddress import yaml +from cerberus import Validator +from cerberus.errors import BasicErrorHandler from . import device # Configuration file input description to class map. @@ -36,6 +40,88 @@ } +#=========================================================================== +def validate(path): + """Validates the configuration file against the defined schema + + Args: + path: The file to load + + Returns: + string: the failure message text or an empty string if no errors + """ + error = "" + + # Check the main config file first + document = None + with open(path, "r") as f: + document = yaml.load(f, Loader) + error += validate_file(document, 'config-schema.yaml', 'configuration') + + # Check the Scenes file + insteon = document.get('insteon', {}) + scenes_path = insteon.get('scenes', None) + if scenes_path is not None: + with open(scenes_path, "r") as f: + document = yaml.load(f, Loader) + # This is a bit hacky, we have to move the scenes contents into a + # root key because cerberus has to have a root key + scenes = {"scenes": document} + error += validate_file(scenes, 'scenes-schema.yaml', 'scenes') + + return error + + +#=========================================================================== +def validate_file(document, schema_file, name): + """ This is used to validate a generic yaml file. + + We use it to validate both the config and scenes files. + + Returns: + (str): An error message string, or an empty string if no errors. + """ + basepath = os.path.dirname(__file__) + schema = None + schema_file_path = os.path.join(basepath, 'schemas', schema_file) + with open(schema_file_path, "r") as f: + schema = yaml.load(f, Loader=yaml.Loader) + + v = IMValidator(schema, error_handler=MetaErrorHandler(schema=schema)) + valid = v.validate(document) + + if valid: + return "" + else: + return """ + ------- Validation Error ------- +An error occured while trying to validate your %s file. Please +review the errors below and fix the error. InsteonMQTT cannot run until this +error is fixed. + +""" % (name) + parse_validation_errors(v.errors) + + +#=========================================================================== +def parse_validation_errors(errors, indent=0): + """ This creates a nice presentation of the error for the User + + The error list looks a lot like the yaml document. Running it through + yaml.dump() was ok. However, doing it this way allows us to have + multiline error messages with nice indentations and such. + """ + error_msg = "" + for key in errors.keys(): + error_msg += " " * indent + str(key) + ": \n" + for item in errors[key]: + if isinstance(item, dict): + error_msg += parse_validation_errors(item, indent=indent + 2) + else: + item = item.replace("\n", "\n " + " " * (indent + 2)) + error_msg += " " * (indent) + "- " + str(item) + "\n" + return error_msg + + #=========================================================================== def load(path): """Load the configuration file. @@ -166,4 +252,141 @@ def rel_path(self, node): % str(node)) raise yaml.constructor.ConstructorError(msg) + +#=========================================================================== +class MetaErrorHandler(BasicErrorHandler): + """ Used for adding custom fail message for a better UX + + This is part of the Cerberus yaml validation schema. + + When a test fails, this will search the contents of each meta keyword + in each key of the search path starting from the root. If the meta + value contains a key with the failed test name appended with "_error" + that message will be used in place of the standard message. + + For example if the following regex fails it creates a custom error: + mqtt: + schema: + cmd_topic: + regex: '^[^/+][^+]*[^/+#]$' + meta: + regex_error: Custom regex error message + + """ + messages = BasicErrorHandler.messages + messages[0x92] = "zero or more than one rule validate, when exactly " + \ + "one is required" + + def __init__(self, schema=None, tree=None): + self.schema = schema + super().__init__(tree) + + def __iter__(self): + """ This is not implemented here either. + """ + raise NotImplementedError + + def _format_message(self, field, error): + """ Hijack the _format_message of the base class to insert our own + messages. + + """ + error_msg = self._find_meta_error(error.schema_path) + if error_msg is not None: + return error_msg + else: + return super()._format_message(field, error) + + def _find_meta_error(self, error_path): + """ Gets the meta error message if there is one + + This function recursively parses the search path for the meta keyword + starting at the root and working updwards, so that it always returns + the most specific meta value that it can find. + """ + schema_part = self.schema + error_msg = None + for iter in range(len(error_path)): + error_key = error_path[iter] + if isinstance(error_key, str): + schema_part = schema_part.get(error_key, None) + else: + # This is likely an int representing the position in a list + schema_part = schema_part[error_key] + if isinstance(schema_part, dict): + meta = schema_part.get('meta', None) + if meta is not None: + error_msg = self._find_error(meta, error_path, iter) + elif isinstance(schema_part, list): + continue + else: + break + return error_msg + + def _find_error(self, meta, error_path, iter): + """ Gets the failed test error message if there is one + + This function recursively parses the search path for the failed test + keyword starting at the base of meta and working updwards, the error + message the deepest into the error_path is returned. + """ + error_msg = None + for meta_iter in range(iter + 1, len(error_path)): + if not isinstance(meta, dict): + break + if error_path[meta_iter] + "_error" in meta: + meta = meta[error_path[meta_iter] + "_error"] + else: + break + if isinstance(meta, str): + error_msg = meta + return error_msg + + +#=========================================================================== +class IMValidator(Validator): + """ Adds a few check_with functions to validate specific settings + """ + def _check_with_valid_ip(self, field, value): + """ Tests whether value is a valid ipv4 or ipv6 address + + Uses the library ipaddress for accuracy + """ + try: + ipaddress.ip_address(value) + except ValueError: + self._error(field, "Invalid IP Address") + + def _check_with_valid_insteon_addr(self, field, value): + """ Tests whether value is a valid Insteon Address for Insteon MQTT + + Accepts any of the following: + - (int) in range of 0 - 0xFFFFFF + - (str) in any case: + - No seperators - AABBCC + - Space, dot or colon seperators - AA.BB.CC, AA BB CC, AA:BB:CC + """ + valid = False + # Try Integer First + try: + addr = int(value) + except ValueError: + pass + else: + if addr >= 0 and addr <= 0xFFFFFF: + valid = True + + # See if valid string form + if not valid: + addr = re.search( + r"[A-F0-9]{2}[ \.:]?[A-F0-9]{2}[ \.:]?[A-F0-9]{2}", + str(value), flags=re.IGNORECASE + ) + if addr is not None: + valid = True + + if not valid: + self._error(field, "Insteon Addresses can be represented as: \n" + "aa.bb.cc, aabbcc, or aa:bb:cc") + #=========================================================================== diff --git a/insteon_mqtt/schemas/config-schema.yaml b/insteon_mqtt/schemas/config-schema.yaml new file mode 100644 index 00000000..87df5762 --- /dev/null +++ b/insteon_mqtt/schemas/config-schema.yaml @@ -0,0 +1,414 @@ +## DO NOT EDIT THIS FILE +## +## This schema is used to validate the config settings before loading. +## The validation enforces certain rules for each setting including: +## - Type - Is the setting specified in the expected form +## - Presence - Is the setting defined when it should be +## - Content - Is the content of the setting something allowed +## Plus other unique tests are performed as well. This helps catch errors +## that may otherwise be hard to diagnose once the program is loaded. +## The schema is interpreted by Cerberus, which you can read more about +## here: https://docs.python-cerberus.org/en/stable/index.html +## +## Yaml anchors & and aliases * are used extensively here to avoid duplicate +## coding. Please read about them here +## https://ktomk.github.io/writing/yaml-anchor-alias-and-merge-key.html +## +## Merge keys were not used because they are already deprecated and not +## not supported in yaml 1.2. + + +logging: + type: dict + schema: + level: + type: integer + min: 5 + max: 40 +insteon: + type: dict + schema: + port: + type: string + baudrate: + type: integer + min: 110 + max: 256000 + use_hub: + type: boolean + oneof: + # If hub_enabled require hub fields, else require PLM port + - allowed: [True] + dependencies: ['hub_ip', 'hub_user', 'hub_password'] + - allowed: [False] + dependencies: ['port'] + hub_ip: + type: string + check_with: valid_ip + hub_port: + type: integer + min: 0 + max: 65535 + hub_user: + type: string + hub_password: + type: string + address: &insteon_addr + # A true integer could be validated with min/max here. But we already + # have code to handle a str representation of an integer, so might as + # well pass integers to the check_with anyways + type: ['string', 'integer'] + check_with: valid_insteon_addr + meta: + type_error: |- + Insteon Addresses can be represented as: + aa.bb.cc, aabbcc, or aa:bb:cc + storage: + type: string + startup_refresh: + type: boolean + scenes: # Scene file is validated in a separate schema + type: string + devices: + type: dict + keysrules: + type: string + allowed: ['switch', 'dimmer', 'battery_sensor', 'hidden_door', + 'motion', 'mini_remote1', 'mini_remote4', + 'mini_remote8', 'smoke_bridge', 'fan_linc', + 'keypad_linc', 'keypad_linc_sw', 'leak', 'io_linc', + 'outlet', 'thermostat', 'ezio4o'] + valuesrules: + type: list + nullable: True + schema: + oneof: + # Opt1 Only address specified + - &insteon_addr_short_error + type: ['string', 'integer'] + check_with: valid_insteon_addr + meta: + type_error: Not a valid Insteon Address format. + # Opt2 Address: Name + - type: dict + meta: + type_error: "Entry was not in 'address: name' format" + keysrules: *insteon_addr_short_error + valuesrules: + type: string + meta: + type_error: >- + Device entry value should be a name in string form. + # Opt3 Entry with config_extra settings, impossible to validate + # this here, so runtime code does all the testing. + # Setting minlength ensures this broad test only applies when + # config_extra keys are present + - type: dict + keysrules: + type: ['string', 'integer'] + meta: + type_error: Key field must be a string or integer + minlength: 2 + meta: + minlength_error: Entry did not have any extra config settings + type_error: "Entry was not in 'address: name' format" + meta: + oneof_error: | + This entry does not match a valid device entry format. + Insteon Addresses can be represented as: + aa.bb.cc, aabbcc, or aa:bb:cc + Device entries can use one of the following forms: + - aa.bb.cc + - aa.bb.cc: Device Name + - aa.bb.cc: Device Name + extra_setting: Extra configuration setting + +mqtt: + type: dict + meta: + schema_error: unknown field + allow_unknown: + ## The only unknown keys are user defined discovery_class settings + type: dict + schema: + discovery_entities: &discovery_entities + type: list + schema: + type: dict + schema: + component: + type: string + allowed: ['alarm_control_panel', 'binary_sensor', 'camera', + 'cover', 'device_tracker', 'device_trigger', 'fan', + 'climate', 'light', 'lock', 'scene', 'sensor', + 'switch', 'tag', 'vacuum'] + required: True + config: + type: string + required: True + schema: + broker: + type: string + check_with: valid_ip + required: True + port: + type: integer + required: True + min: 0 + max: 65535 + username: + type: string + password: + type: string + id: + type: string + keep_alive: + type: integer + min: 0 + qos: + type: integer + min: 0 + max: 2 + retain: + type: ['integer', 'boolean'] + allowed: [0, 1, True, False] + cmd_topic: &mqtt_topic + type: string + regex: '^[^/+][^+]*[^/+#]$' + meta: + regex_error: >- + MQTT Topics cannot start or end with / or # and cannot use + + enable_discovery: + type: boolean + discovery_topic_base: *mqtt_topic + discovery_ha_status: *mqtt_topic + device_info_template: + type: string + modem: + type: dict + allow_unknown: True + schema: + scene_topic: *mqtt_topic + scene_payload: + type: string + discovery_entities: *discovery_entities + switch: + type: dict + allow_unknown: True + schema: + state_topic: *mqtt_topic + state_payload: + type: string + on_off_topic: *mqtt_topic + on_off_payload: + type: string + scene_topic: *mqtt_topic + scene_payload: + type: string + discovery_entities: *discovery_entities + dimmer: + type: dict + allow_unknown: True + schema: + state_topic: *mqtt_topic + state_payload: + type: string + manual_state_topic: *mqtt_topic + manual_state_payload: + type: string + on_off_topic: *mqtt_topic + on_off_payload: + type: string + level_topic: *mqtt_topic + level_payload: + type: string + scene_topic: *mqtt_topic + scene_payload: + type: string + discovery_entities: *discovery_entities + battery_sensor: + type: dict + allow_unknown: True + schema: + state_topic: *mqtt_topic + state_payload: + type: string + low_battery_topic: *mqtt_topic + low_battery_payload: + type: string + heartbeat_topic: *mqtt_topic + heartbeat_payload: + type: string + discovery_entities: *discovery_entities + motion: + type: dict + allow_unknown: True + schema: + dawn_dusk_topic: *mqtt_topic + dawn_dusk_payload: + type: string + discovery_entities: *discovery_entities + hidden_door: + type: dict + allow_unknown: True + schema: + battery_voltage_topic: *mqtt_topic + battery_voltage_payload: + type: string + discovery_entities: *discovery_entities + leak: + type: dict + allow_unknown: True + schema: + wet_dry_topic: *mqtt_topic + wet_dry_payload: + type: string + discovery_entities: *discovery_entities + remote: + type: dict + allow_unknown: True + schema: + state_topic: *mqtt_topic + state_payload: + type: string + discovery_entities: *discovery_entities + smoke_bridge: + type: dict + allow_unknown: True + schema: + battery_topic: *mqtt_topic + battery_payload: + type: string + co_topic: *mqtt_topic + co_payload: + type: string + error_topic: *mqtt_topic + error_payload: + type: string + smoke_topic: *mqtt_topic + smoke_payload: + type: string + discovery_entities: *discovery_entities + thermostat: + type: dict + allow_unknown: True + schema: + ambient_temp_topic: *mqtt_topic + ambient_temp_payload: + type: string + cool_sp_command_topic: *mqtt_topic + cool_sp_payload: + type: string + cool_sp_state_topic: *mqtt_topic + cool_sp_state_payload: + type: string + energy_state_topic: *mqtt_topic + energy_state_payload: + type: string + fan_command_topic: *mqtt_topic + fan_command_payload: + type: string + fan_state_topic: *mqtt_topic + fan_state_payload: + type: string + heat_sp_command_topic: *mqtt_topic + heat_sp_payload: + type: string + heat_sp_state_topic: *mqtt_topic + heat_sp_state_payload: + type: string + hold_state_topic: *mqtt_topic + hold_state_payload: + type: string + humid_state_topic: *mqtt_topic + humid_state_payload: + type: string + mode_command_topic: *mqtt_topic + mode_command_payload: + type: string + mode_state_topic: *mqtt_topic + mode_state_payload: + type: string + status_state_topic: *mqtt_topic + status_state_payload: + type: string + discovery_entities: *discovery_entities + fan_linc: + type: dict + allow_unknown: True + schema: + fan_state_topic: *mqtt_topic + fan_state_payload: + type: string + fan_on_off_topic: *mqtt_topic + fan_on_off_payload: + type: string + fan_speed_topic: *mqtt_topic + fan_speed_payload: + type: string + fan_speed_set_topic: *mqtt_topic + fan_speed_set_payload: + type: string + discovery_entities: *discovery_entities + keypad_linc: + type: dict + allow_unknown: True + schema: + btn_state_topic: *mqtt_topic + btn_state_payload: + type: string + dimmer_state_topic: *mqtt_topic + dimmer_state_payload: + type: string + manual_state_topic: *mqtt_topic + manual_state_payload: + type: string + btn_on_off_topic: *mqtt_topic + btn_on_off_payload: + type: string + dimmer_level_topic: *mqtt_topic + dimmer_level_payload: + type: string + btn_scene_topic: *mqtt_topic + btn_scene_payload: + type: string + discovery_entities: *discovery_entities + io_linc: + type: dict + allow_unknown: True + schema: + state_topic: *mqtt_topic + state_payload: + type: string + relay_state_topic: *mqtt_topic + relay_state_payload: + type: string + sensor_state_topic: *mqtt_topic + sensor_state_payload: + type: string + on_off_topic: *mqtt_topic + on_off_payload: + type: string + discovery_entities: *discovery_entities + outlet: + type: dict + allow_unknown: True + schema: + state_topic: *mqtt_topic + state_payload: + type: string + on_off_topic: *mqtt_topic + on_off_payload: + type: string + discovery_entities: *discovery_entities + ezio4o: + type: dict + allow_unknown: True + schema: + state_topic: *mqtt_topic + state_payload: + type: string + on_off_topic: *mqtt_topic + on_off_payload: + type: string + discovery_entities: *discovery_entities diff --git a/insteon_mqtt/schemas/scenes-schema.yaml b/insteon_mqtt/schemas/scenes-schema.yaml new file mode 100644 index 00000000..199e1b8c --- /dev/null +++ b/insteon_mqtt/schemas/scenes-schema.yaml @@ -0,0 +1,89 @@ +## DO NOT EDIT THIS FILE +## +## This schema is used to validate the scenes file before loading. +## The validation enforces certain rules for each setting including: +## - Type - Is the setting specified in the expected form +## - Presence - Is the setting defined when it should be +## - Content - Is the content of the setting something allowed +## Plus other unique tests are performed as well. This helps catch errors +## that may otherwise be hard to diagnose once the program is loaded. +## The schema is interpreted by Cerberus, which you can read more about +## here: https://docs.python-cerberus.org/en/stable/index.html +## +## Yaml anchors & and aliases * are used extensively here to avoid duplicate +## coding. Please read about them here +## https://ktomk.github.io/writing/yaml-anchor-alias-and-merge-key.html +## +## Merge keys were not used because they are already deprecated and not +## not supported in yaml 1.2. +## +## I made an error when initially designing the scenes.yaml file. Apparently +## not having a key at the root is very uncommon, even though it works. +## However, it doesn't work for Cerberus. So before passing the document to +## validate we insert a root key of `scenes` and set it's value to the +## document contents + +scenes: + type: list + schema: + type: dict + allow_unknown: False + schema: + responders: &scene_device + type: list + required: True + schema: + oneof: + # Opt1 This is a device name or Insteon address only + - type: ['string', 'integer'] + meta: + type_error: Device address or name must be a string + # Opt2 This is a device with group + - type: dict + maxlength: 1 + keysrules: + type: ['string', 'integer'] + meta: + type_error: Device address or name must be a string + valuesrules: + type: integer + meta: + type_error: Group must be an integer + meta: + type_error: "Entry was not in 'device: group' format" + maxlength_error: "Entry was not in 'device: group' format" + # Opt3 This is a device with additional device attributes + - type: dict + keysrules: + type: ['string', 'integer'] + meta: + type_error: >- + Device address, name, and device attributes must be strings + valuesrules: + type: dict + schema: {} + allow_unknown: + type: number + meta: + type_error: Device attribute value must be a number + meta: + type_error: >- + Entry was not in 'device: {dev_attr: value}' format + meta: + oneof_error: | + This entry does not match a valid entry format. + Insteon Addresses can be represented as: + aa.bb.cc, aabbcc, or aa:bb:cc + Scene Device entries can use one of the following forms: + - aa.bb.cc + - Device Name + - [aa.bb.cc, Device Name]: Group (integer) + - aa.bb.cc: + device_attribute: Value (number) + + controllers: *scene_device + name: + required: False + type: string + meta: + type_error: Scene names must be strings diff --git a/requirements.txt b/requirements.txt index b0869d3d..7962ce1c 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,3 +6,4 @@ ruamel.yaml>=0.15 Flask>=1.1.2 Flask-SocketIO>=4.3.2 requests>=2.18.2 +cerberus>=1.3.4 diff --git a/scenes.yaml b/scenes.yaml index f6e0de6c..e28f8c7f 100644 --- a/scenes.yaml +++ b/scenes.yaml @@ -32,7 +32,7 @@ # # Scene Device Format # A scene device can be defined in a number of ways: -# - As a string of the device address such as aa.bbb.cc +# - As a string of the device address such as aa.bb.cc # - As a string of the device name as specified in the config file # - As a dict, where the key is the device name or address and the value is # the group as an int @@ -125,7 +125,7 @@ # Latching # On and off commands to the scene will turn the relay on and off -# Momentary_A +# Momentary_A # Only on commands will turn the relay on. # Momentary_B # Either on or off will turn on the relay. @@ -142,7 +142,7 @@ # Latching # On and off commands to the scene will turn the relay on and off -# Momentary_A +# Momentary_A # Only off commands will turn the relay on. # Momentary_B # Either on or off will turn on the relay. @@ -156,4 +156,4 @@ - modem responders: - iolinc: - on_off: 0 \ No newline at end of file + on_off: 0 From 03dfd1c1bcea6e2ecc607087bfb2631dd9be5f15 Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Wed, 12 May 2021 12:47:14 -0700 Subject: [PATCH 02/10] Add Unit Test to Validation; Fix Address Validation; Msg Improve Some basic tests to check validation. I am not going to test all possible bad configurations, just the basic concept Also check config-example.yaml so that it is always correct. Test the address validation and fix error in regex Improve validation messages --- insteon_mqtt/config.py | 2 +- insteon_mqtt/schemas/config-schema.yaml | 8 ++++ tests/configs/bad_plm.yaml | 2 + tests/test_config.py | 61 +++++++++++++++++++++++++ 4 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 tests/configs/bad_plm.yaml diff --git a/insteon_mqtt/config.py b/insteon_mqtt/config.py index ace1e725..882113de 100644 --- a/insteon_mqtt/config.py +++ b/insteon_mqtt/config.py @@ -379,7 +379,7 @@ def _check_with_valid_insteon_addr(self, field, value): # See if valid string form if not valid: addr = re.search( - r"[A-F0-9]{2}[ \.:]?[A-F0-9]{2}[ \.:]?[A-F0-9]{2}", + r"^[A-F0-9]{2}[ \.:]?[A-F0-9]{2}[ \.:]?[A-F0-9]{2}$", str(value), flags=re.IGNORECASE ) if addr is not None: diff --git a/insteon_mqtt/schemas/config-schema.yaml b/insteon_mqtt/schemas/config-schema.yaml index 87df5762..060f5317 100644 --- a/insteon_mqtt/schemas/config-schema.yaml +++ b/insteon_mqtt/schemas/config-schema.yaml @@ -40,8 +40,16 @@ insteon: # If hub_enabled require hub fields, else require PLM port - allowed: [True] dependencies: ['hub_ip', 'hub_user', 'hub_password'] + meta: + allowed_error: use_hub must be True - allowed: [False] dependencies: ['port'] + meta: + allowed_error: use_hub must be False + meta: + oneof_error: >- + Required PLM or Hub fields are not properly configured. One of the + two following definitions must be satisfied. hub_ip: type: string check_with: valid_ip diff --git a/tests/configs/bad_plm.yaml b/tests/configs/bad_plm.yaml new file mode 100644 index 00000000..9ef9128c --- /dev/null +++ b/tests/configs/bad_plm.yaml @@ -0,0 +1,2 @@ +insteon: + use_hub: False diff --git a/tests/test_config.py b/tests/test_config.py index 363d780e..2d8f1a63 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -6,6 +6,8 @@ #=========================================================================== import os import pytest +from unittest import mock +from unittest.mock import call import insteon_mqtt as IM @@ -63,6 +65,65 @@ def test_multi_error(self): with pytest.raises(Exception): IM.config.load(file) + #----------------------------------------------------------------------- + def test_validate_good(self): + file = os.path.join(os.path.dirname(os.path.realpath(__file__)), + 'configs', 'basic.yaml') + val = IM.config.validate(file) + assert val == "" + + #----------------------------------------------------------------------- + def test_validate_bad(self): + file = os.path.join(os.path.dirname(os.path.realpath(__file__)), + 'configs', 'bad_plm.yaml') + val = IM.config.validate(file) + assert val != "" + + #----------------------------------------------------------------------- + def test_validate_example(self): + file = os.path.join(os.path.dirname(os.path.realpath(__file__)), + '..', 'config-example.yaml') + val = IM.config.validate(file) + assert val == "" + + #----------------------------------------------------------------------- + def test_validate_addr(self): + validator = IM.config.IMValidator() + validator._error = mock.Mock() + + # Good + validator._check_with_valid_insteon_addr('test_field', 'aabbcc') + validator._error.assert_not_called() + + # Also good + validator._check_with_valid_insteon_addr('test_field', 'aa.bb.cc') + validator._error.assert_not_called() + + # Also good + validator._check_with_valid_insteon_addr('test_field', 'aa bb cc') + validator._error.assert_not_called() + + # Also good + validator._check_with_valid_insteon_addr('test_field', 'aa:bb:cc') + validator._error.assert_not_called() + + # Also good + validator._check_with_valid_insteon_addr('test_field', 'aa:bb.cc') + validator._error.assert_not_called() + + # Also good + validator._check_with_valid_insteon_addr('test_field', '5522') + validator._error.assert_not_called() + + # Also bad + validator._check_with_valid_insteon_addr('test_field', 'Error') + validator._error.assert_called_once() + validator._error.reset_mock() + + # Also bad + validator._check_with_valid_insteon_addr('test_field', 'aabbbcc') + validator._error.assert_called_once() + validator._error.reset_mock() #=========================================================================== class MockManager: From 067b3ddf4a67eeb30290d23d6165e2b0061b7248 Mon Sep 17 00:00:00 2001 From: Lloyd Ramey Date: Thu, 13 May 2021 22:38:00 -0400 Subject: [PATCH 03/10] add group 2 to fanlinc responders group --- insteon_mqtt/device/FanLinc.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/insteon_mqtt/device/FanLinc.py b/insteon_mqtt/device/FanLinc.py index 1899d466..8ccc2bb8 100644 --- a/insteon_mqtt/device/FanLinc.py +++ b/insteon_mqtt/device/FanLinc.py @@ -82,6 +82,9 @@ def __init__(self, protocol, modem, address, name=None, config_extra=None): # This is necessary to override the dimmer group_map of 0x01 self.group_map = {} + # List of responder group numbers + self.responder_groups = [0x01, 0x02] + #----------------------------------------------------------------------- def addRefreshData(self, seq, force=False): """Add commands to refresh any internal data required. From 5d5f018bacc7a523d63d39d3861bfb852ab8d2a4 Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Sun, 16 May 2021 10:16:22 -0700 Subject: [PATCH 04/10] Additional Yaml Validator Test --- tests/configs/good_hub.yaml | 5 +++++ tests/test_config.py | 7 +++++++ 2 files changed, 12 insertions(+) create mode 100644 tests/configs/good_hub.yaml diff --git a/tests/configs/good_hub.yaml b/tests/configs/good_hub.yaml new file mode 100644 index 00000000..e18f3d5f --- /dev/null +++ b/tests/configs/good_hub.yaml @@ -0,0 +1,5 @@ +insteon: + use_hub: True + hub_ip: 192.168.1.1 + hub_user: username + hub_password: password diff --git a/tests/test_config.py b/tests/test_config.py index 2d8f1a63..e940ceef 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -86,6 +86,13 @@ def test_validate_example(self): val = IM.config.validate(file) assert val == "" + #----------------------------------------------------------------------- + def test_good_hub(self): + file = os.path.join(os.path.dirname(os.path.realpath(__file__)), + 'configs', 'good_hub.yaml') + val = IM.config.validate(file) + assert val == "" + #----------------------------------------------------------------------- def test_validate_addr(self): validator = IM.config.IMValidator() From 60912fdc6281bee436568bc2334a1d8dc62019d6 Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Sun, 16 May 2021 10:32:30 -0700 Subject: [PATCH 05/10] Update Setup to Include Schema Files --- setup.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/setup.py b/setup.py index fb27eaf2..0f19a520 100644 --- a/setup.py +++ b/setup.py @@ -16,7 +16,10 @@ url = 'https://github.com/TD22057/insteon-mqtt', packages = setuptools.find_packages(exclude=["tests*"]), scripts = ['scripts/insteon-mqtt'], - include_package_data = True, + package_data = { + # include the schema files + "": ["schemas/*.yaml"], + }, install_requires = requirements, license = "GNU General Public License v3", classifiers = [ From 2a97df4f2fd01695dbfad059bdf2bc6b0b6ecc0b Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Sun, 16 May 2021 10:38:57 -0700 Subject: [PATCH 06/10] Update Schema for Logging File; Allow More Unknowns Make the Schema less strict. Less ideal as typos will no pass unnoticed, but it won't halt on startup for typos either. --- insteon_mqtt/schemas/config-schema.yaml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/insteon_mqtt/schemas/config-schema.yaml b/insteon_mqtt/schemas/config-schema.yaml index 060f5317..8cb3d6f6 100644 --- a/insteon_mqtt/schemas/config-schema.yaml +++ b/insteon_mqtt/schemas/config-schema.yaml @@ -20,12 +20,18 @@ logging: type: dict + allow_unknown: True schema: level: type: integer min: 5 max: 40 + file: + type: string + screen: + type: boolean insteon: + allow_unknown: True type: dict schema: port: @@ -78,6 +84,7 @@ insteon: scenes: # Scene file is validated in a separate schema type: string devices: + allow_unknown: False type: dict keysrules: type: string From 22f8b94bb666a401a41a3c0c5de2dcedb09f79d1 Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Sun, 16 May 2021 11:10:59 -0700 Subject: [PATCH 07/10] Add On Off Command Support to Fan Group on FanLinc --- insteon_mqtt/device/FanLinc.py | 57 +++++++++++++++++++++++++++++++++- 1 file changed, 56 insertions(+), 1 deletion(-) diff --git a/insteon_mqtt/device/FanLinc.py b/insteon_mqtt/device/FanLinc.py index 1899d466..6f6802ad 100644 --- a/insteon_mqtt/device/FanLinc.py +++ b/insteon_mqtt/device/FanLinc.py @@ -20,7 +20,7 @@ class FanLinc(Dimmer): """Insteon FanLinc fan speed control device. This class can be used to model a FanLinc module which is used to control - a ciling fan. The FanLinc can be on or off and supports three speeds + a ceiling fan. The FanLinc can be on or off and supports three speeds (LOW, MED, HIGH). The FanLinc is also a dimmer switch and has the same signals and methods as that class (Dimmer). @@ -184,6 +184,61 @@ def fan_off(self, reason="", on_done=None): msg_handler = handler.StandardCmd(msg, callback, on_done) self.send(msg, msg_handler) + #----------------------------------------------------------------------- + def on(self, group=0x01, level=None, mode=on_off.Mode.NORMAL, reason="", + transition=None, on_done=None): + """This extends the method in ResponderBase to handle the fan group. + + Args: + group (int): The group to send the command to. + level (int): If non-zero, turn the device on. The API is an int + to keep a consistent API with other devices. + mode (on_off.Mode): The type of command to send (normal, fast, etc). + transition (int): Transition time in seconds if supported. + reason (str): This is optional and is used to identify why the + command was sent. It is passed through to the output signal + when the state changes - nothing else is done with it. + on_done: Finished callback. This is called when the command has + completed. Signature is: on_done(success, msg, data) + """ + # If group 2, then this is for Fan + if group == 0x02: + speed = None + if level is not None: + if level >= 0 and level <= 85: + speed = FanLinc.Speed.LOW + elif level > 85 and level <= 170: + speed = FanLinc.Speed.MEDIUM + elif level > 170: + speed = FanLinc.Speed.HIGH + self.fan_on(speed=speed, reason=reason, on_done=on_done) + else: + # This is a regular on command pass to ResponderBase + super().on(group=group, level=level, mode=mode, reason=reason, + transition=transition, on_done=on_done) + + #----------------------------------------------------------------------- + def off(self, group=0x01, mode=on_off.Mode.NORMAL, reason="", + transition=None, on_done=None): + """This extends the method in ResponderBase to handle the fan group. + + Args: + group (int): The group to send the command to. + mode (on_off.Mode): The type of command to send (normal, fast, etc). + reason (str): This is optional and is used to identify why the + command was sent. It is passed through to the output signal + when the state changes - nothing else is done with it. + on_done: Finished callback. This is called when the command has + completed. Signature is: on_done(success, msg, data) + """ + # If group 2, then this is for Fan + if group == 0x02: + self.fan_off(reason=reason, on_done=on_done) + else: + # This is a regular on command pass to ResponderBase + super().off(group=group, mode=mode, reason=reason, + transition=transition, on_done=on_done) + #----------------------------------------------------------------------- def fan_set(self, speed, reason="", on_done=None): """Set the fan speed. From d563451fa502c626ea73cd92529f37f58c1156c4 Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Sun, 16 May 2021 11:30:23 -0700 Subject: [PATCH 08/10] Update ChangeLog; Add Notes About Breaking --- CHANGELOG.md | 10 ++++++++++ README.md | 6 +++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a52749ec..1e647938 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,14 @@ # Revision Change History +## [0.9.1] + +### Additions + +- Validate the user config file before starting. This should help catch some + bugs, and provides clear concise descriptions of mistakes. ([PR 397][P397]) + ** Potential Breaking Change ** If you have an error in your config.yaml + file you will get an error when trying to startup after upgrading. + ## [0.9.0] ### Discovery Platform! @@ -712,3 +721,4 @@ will add new features. [P390]: https://github.com/TD22057/insteon-mqtt/pull/390 [P392]: https://github.com/TD22057/insteon-mqtt/pull/392 [P393]: https://github.com/TD22057/insteon-mqtt/pull/393 +[P397]: https://github.com/TD22057/insteon-mqtt/pull/397 diff --git a/README.md b/README.md index 8f262f62..b99504b6 100644 --- a/README.md +++ b/README.md @@ -11,10 +11,12 @@ Version: 0.9.0 ([History](CHANGELOG.md)) ### Recent Breaking Changes +- 0.9.1 - A Yaml validation routine was added. If you have an error in your + config.yaml file, you will get an error on startup. - 0.8.3 - HomeAssistant version 2021.4.0 now only supports percentages for fan speeds. This means any fan entities in HomeAssistant that were configured to use "low", "medium", and "high" for the fan speed will no longer work. - See [config-example.yaml](https://github.com/TD22057/insteon-mqtt/blob/master/config-example.yaml) + See [config-example.yaml](https://github.com/TD22057/insteon-mqtt/blob/master/config-example.yaml) under the `mqtt -> fan` section for a suggest configuration in HomeAssistant. __Users not using HomeAssistant are unaffected.__ - 0.7.4 - IOLinc, the scene_topic has been elimited, please see the documentation @@ -112,8 +114,6 @@ There is still more work to do and here are a some of my plans for future enhancements: - Full suite of unit tests. -- YAML input configuration validation. - # Thanks From 29ded764bb85d5f89d9fe61c922efc98d275d1d1 Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Sun, 16 May 2021 11:32:53 -0700 Subject: [PATCH 09/10] Update Changelog --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e647938..788ce93a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,11 @@ ** Potential Breaking Change ** If you have an error in your config.yaml file you will get an error when trying to startup after upgrading. +### Fixes + +- Add support for `on` and `off` command for the fan group on fan_linc devices + Thanks @lnr0626 ([PR 399][P399],[PR 400][P400]) + ## [0.9.0] ### Discovery Platform! @@ -722,3 +727,5 @@ will add new features. [P392]: https://github.com/TD22057/insteon-mqtt/pull/392 [P393]: https://github.com/TD22057/insteon-mqtt/pull/393 [P397]: https://github.com/TD22057/insteon-mqtt/pull/397 +[P399]: https://github.com/TD22057/insteon-mqtt/pull/399 +[P400]: https://github.com/TD22057/insteon-mqtt/pull/400 From 6f473a23cfa1b69f716bb926e402df187346d26c Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Sun, 16 May 2021 11:33:41 -0700 Subject: [PATCH 10/10] =?UTF-8?q?Bump=20version:=200.9.0=20=E2=86=92=200.9?= =?UTF-8?q?.1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .bumpversion.cfg | 2 +- README.md | 2 +- config.json | 2 +- insteon_mqtt/const.py | 2 +- setup.py | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.bumpversion.cfg b/.bumpversion.cfg index 6841bde2..8e6d253a 100644 --- a/.bumpversion.cfg +++ b/.bumpversion.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 0.9.0 +current_version = 0.9.1 commit = True tag = False diff --git a/README.md b/README.md index b99504b6..5eb15158 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ integrated into and controlled from anything that can use MQTT. This package works well with HomeAssistant and can be easily [installed as an addon](docs/HA_Addon_Instructions.md) using the HomeAssistant Supervisor. -Version: 0.9.0 ([History](CHANGELOG.md)) +Version: 0.9.1 ([History](CHANGELOG.md)) ### Recent Breaking Changes diff --git a/config.json b/config.json index a8e7d5e8..9c0ee536 100644 --- a/config.json +++ b/config.json @@ -2,7 +2,7 @@ "name": "Insteon MQTT", "description": "Creates an MQTT interface to the Insteon protocol.", "slug": "insteon-mqtt", - "version": "0.9.0", + "version": "0.9.1", "startup": "services", "arch": ["amd64","armhf","aarch64","i386"], "boot": "auto", diff --git a/insteon_mqtt/const.py b/insteon_mqtt/const.py index 607b4b9e..436dce70 100644 --- a/insteon_mqtt/const.py +++ b/insteon_mqtt/const.py @@ -11,6 +11,6 @@ variable throughout the code without causing a cyclic import """ -__version__ = "0.9.0" +__version__ = "0.9.1" #=========================================================================== diff --git a/setup.py b/setup.py index 0f19a520..b85779b5 100644 --- a/setup.py +++ b/setup.py @@ -8,7 +8,7 @@ setuptools.setup( name = 'insteon-mqtt', - version = '0.9.0', + version = '0.9.1', description = "Insteon <-> MQTT bridge server", long_description = readme, author = "Ted Drain",