From 16b561f200d3ac79c19d5df93f113c80b8d242cb Mon Sep 17 00:00:00 2001 From: tstabrawa <59430211+tstabrawa@users.noreply.github.com> Date: Tue, 17 Nov 2020 22:42:52 -0600 Subject: [PATCH 01/54] Add scene tests for Dimmers with ramp rates --- tests/test_Scenes.py | 75 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/tests/test_Scenes.py b/tests/test_Scenes.py index fd24a324..312d8a03 100644 --- a/tests/test_Scenes.py +++ b/tests/test_Scenes.py @@ -11,6 +11,7 @@ import insteon_mqtt.db.DeviceEntry as DeviceEntry import insteon_mqtt.db.Modem as ModemDB import insteon_mqtt.device.Base as Base +import insteon_mqtt.device.Dimmer as Dimmer class Test_Scenes: @@ -168,6 +169,80 @@ def test_set_group(self): scenes.entries[0].controllers[0].group = 2 assert scenes.data[0]['controllers'][0]['dev - a1.b1.c1']['group'] == 2 + def test_Dimmer_scenes_same_ramp_rate(self): + modem = MockModem() + dimmer = Dimmer(modem.protocol, modem, Address("11.22.33"), "Dimmer") + modem.devices[str(dimmer.addr)] = dimmer + device = modem.find(Address("aa.bb.cc")) + modem.devices[device.label] = device + scenes = Scenes.SceneManager(modem, None) + scenes.data = [{'controllers': [{'aa.bb.cc': {'group': 22}}], + 'responders': ['11.22.33']}, + {'controllers': [{'aa.bb.cc': {'group': 33}}], + 'responders': ['11.22.33']}] + scenes._init_scene_entries() + entry1 = DeviceEntry.from_json({"addr": "aa.bb.cc", + "group": 22, + "mem_loc" : 8119, + "db_flags": {"is_last_rec": False, + "in_use": True, + "is_controller": False}, + "data": [255, 23, 0]}) + scenes.add_or_update(dimmer, entry1) + entry2 = DeviceEntry.from_json({"addr": "aa.bb.cc", + "group": 33, + "mem_loc" : 8119, + "db_flags": {"is_last_rec": False, + "in_use": True, + "is_controller": False}, + "data": [255, 23, 0]}) + scenes.add_or_update(dimmer, entry2) + scenes.compress_controllers() + print(str(scenes.data)) + # We should end up with a single scene with: + # - 2 controller entries: aa.bb.cc, group 22, group 23 + # - 1 responder entry: 11.22.33, ramp_rate 19 seconds + assert len(scenes.entries) == 1 + assert len(scenes.data[0]['controllers']) == 2 + assert len(scenes.data[0]['responders']) == 1 + assert scenes.data[0]['responders'][0]['Dimmer']['ramp_rate'] == 19 + + def test_Dimmer_scenes_different_ramp_rates(self): + modem = MockModem() + dimmer = Dimmer(modem.protocol, modem, Address("11.22.33"), "Dimmer") + modem.devices[str(dimmer.addr)] = dimmer + device = modem.find(Address("aa.bb.cc")) + modem.devices[device.label] = device + scenes = Scenes.SceneManager(modem, None) + scenes.data = [{'controllers': [{'aa.bb.cc': {'group': 22}}], + 'responders': ['11.22.33']}, + {'controllers': [{'aa.bb.cc': {'group': 33}}], + 'responders': ['11.22.33']}] + scenes._init_scene_entries() + entry1 = DeviceEntry.from_json({"addr": "aa.bb.cc", + "group": 22, + "mem_loc" : 8119, + "db_flags": {"is_last_rec": False, + "in_use": True, + "is_controller": False}, + "data": [255, 23, 0]}) + scenes.add_or_update(dimmer, entry1) + entry2 = DeviceEntry.from_json({"addr": "aa.bb.cc", + "group": 33, + "mem_loc" : 8119, + "db_flags": {"is_last_rec": False, + "in_use": True, + "is_controller": False}, + "data": [255, 13, 0]}) + scenes.add_or_update(dimmer, entry2) + scenes.compress_controllers() + print(str(scenes.data)) + # We should end up with 2 scenes: + # - Controller aa.bb.cc, group 22 -> Dimmer w/ 19 second ramp_rate + # - Controller aa.bb.cc, group 33 -> Dimmer w/ 47 second ramp_rate + # (Just checking # of scenes should be adequate for this test.) + assert len(scenes.entries) == 2 + class MockModem(): def __init__(self): self.save_path = '' From 60cc79c1087784d75c2d1fc1fd1e3f4e7c4d7a46 Mon Sep 17 00:00:00 2001 From: tstabrawa <59430211+tstabrawa@users.noreply.github.com> Date: Wed, 15 Jul 2020 22:56:53 -0500 Subject: [PATCH 02/54] Make link_data_from_pretty match "to" variant for ramp_rate Fixes problem where scenes are being combined by scenes.compress_controllers despite having different ramp rates for responders. --- insteon_mqtt/device/Dimmer.py | 4 ++-- insteon_mqtt/device/FanLinc.py | 4 ++-- insteon_mqtt/device/KeypadLinc.py | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/insteon_mqtt/device/Dimmer.py b/insteon_mqtt/device/Dimmer.py index 5287a3fe..7ccad070 100644 --- a/insteon_mqtt/device/Dimmer.py +++ b/insteon_mqtt/device/Dimmer.py @@ -447,10 +447,10 @@ def link_data_from_pretty(self, is_controller, data): if 'data_3' in data: data_3 = data['data_3'] if not is_controller: - if 'ramp' in data: + if 'ramp_rate' in data: data_2 = 0x1f for ramp_key, ramp_value in self.ramp_pretty.items(): - if data['ramp'] >= ramp_value: + if data['ramp_rate'] >= ramp_value: data_2 = ramp_key break if 'on_level' in data: diff --git a/insteon_mqtt/device/FanLinc.py b/insteon_mqtt/device/FanLinc.py index 8a85a7de..44f37479 100644 --- a/insteon_mqtt/device/FanLinc.py +++ b/insteon_mqtt/device/FanLinc.py @@ -498,10 +498,10 @@ def link_data_from_pretty(self, is_controller, data): if not is_controller: if 'group' in data: data_3 = data['group'] - if 'ramp' in data and data['group'] <= 0x01: + if 'ramp_rate' in data and data['group'] <= 0x01: data_2 = 0x1f for ramp_key, ramp_value in Dimmer.ramp_pretty: - if data['ramp'] >= ramp_value: + if data['ramp_rate'] >= ramp_value: data_2 = ramp_key if 'on_level' in data: data_1 = int(data['on_level'] * 2.55 + .5) diff --git a/insteon_mqtt/device/KeypadLinc.py b/insteon_mqtt/device/KeypadLinc.py index 32e66479..8cd8c227 100644 --- a/insteon_mqtt/device/KeypadLinc.py +++ b/insteon_mqtt/device/KeypadLinc.py @@ -636,10 +636,10 @@ def link_data_from_pretty(self, is_controller, data): if 'group' in data: data_3 = data['group'] if not is_controller and self.is_dimmer: - if 'ramp' in data: + if 'ramp_rate' in data: data_2 = 0x1f for ramp_key, ramp_value in Dimmer.ramp_pretty: - if data['ramp'] >= ramp_value: + if data['ramp_rate'] >= ramp_value: data_2 = ramp_key if 'on_level' in data: data_1 = int(data['on_level'] * 2.55 + .5) From 103341a6fb73385c18551238e7b8bf88e66925a0 Mon Sep 17 00:00:00 2001 From: tstabrawa <59430211+tstabrawa@users.noreply.github.com> Date: Tue, 17 Nov 2020 23:09:10 -0600 Subject: [PATCH 03/54] Add scene tests for FanLinc & KeypadLinc with ramp rates --- tests/test_Scenes.py | 152 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 152 insertions(+) diff --git a/tests/test_Scenes.py b/tests/test_Scenes.py index 312d8a03..f3b70f9f 100644 --- a/tests/test_Scenes.py +++ b/tests/test_Scenes.py @@ -12,6 +12,8 @@ import insteon_mqtt.db.Modem as ModemDB import insteon_mqtt.device.Base as Base import insteon_mqtt.device.Dimmer as Dimmer +import insteon_mqtt.device.FanLinc as FanLinc +import insteon_mqtt.device.KeypadLinc as KeypadLinc class Test_Scenes: @@ -243,6 +245,156 @@ def test_Dimmer_scenes_different_ramp_rates(self): # (Just checking # of scenes should be adequate for this test.) assert len(scenes.entries) == 2 + def test_FanLinc_scenes_same_ramp_rate(self): + modem = MockModem() + fanlinc = FanLinc(modem.protocol, modem, Address("11.22.33"), "FanLinc") + modem.devices[str(fanlinc.addr)] = fanlinc + device = modem.find(Address("aa.bb.cc")) + modem.devices[device.label] = device + scenes = Scenes.SceneManager(modem, None) + scenes.data = [{'controllers': [{'aa.bb.cc': {'group': 22}}], + 'responders': ['11.22.33']}, + {'controllers': [{'aa.bb.cc': {'group': 33}}], + 'responders': ['11.22.33']}] + scenes._init_scene_entries() + entry1 = DeviceEntry.from_json({"addr": "aa.bb.cc", + "group": 22, + "mem_loc" : 8119, + "db_flags": {"is_last_rec": False, + "in_use": True, + "is_controller": False}, + "data": [255, 23, 1]}) + scenes.add_or_update(fanlinc, entry1) + entry2 = DeviceEntry.from_json({"addr": "aa.bb.cc", + "group": 33, + "mem_loc" : 8119, + "db_flags": {"is_last_rec": False, + "in_use": True, + "is_controller": False}, + "data": [255, 23, 1]}) + scenes.add_or_update(fanlinc, entry2) + scenes.compress_controllers() + print(str(scenes.data)) + # We should end up with a single scene with: + # - 2 controller entries: aa.bb.cc, group 22, group 23 + # - 1 responder entry: 11.22.33, ramp_rate 19 seconds + assert len(scenes.entries) == 1 + assert len(scenes.data[0]['controllers']) == 2 + assert len(scenes.data[0]['responders']) == 1 + assert scenes.data[0]['responders'][0]['FanLinc']['ramp_rate'] == 19 + + def test_FanLinc_scenes_different_ramp_rates(self): + modem = MockModem() + fanlinc = FanLinc(modem.protocol, modem, Address("11.22.33"), "FanLinc") + modem.devices[str(fanlinc.addr)] = fanlinc + device = modem.find(Address("aa.bb.cc")) + modem.devices[device.label] = device + scenes = Scenes.SceneManager(modem, None) + scenes.data = [{'controllers': [{'aa.bb.cc': {'group': 22}}], + 'responders': ['11.22.33']}, + {'controllers': [{'aa.bb.cc': {'group': 33}}], + 'responders': ['11.22.33']}] + scenes._init_scene_entries() + entry1 = DeviceEntry.from_json({"addr": "aa.bb.cc", + "group": 22, + "mem_loc" : 8119, + "db_flags": {"is_last_rec": False, + "in_use": True, + "is_controller": False}, + "data": [255, 23, 1]}) + scenes.add_or_update(fanlinc, entry1) + entry2 = DeviceEntry.from_json({"addr": "aa.bb.cc", + "group": 33, + "mem_loc" : 8119, + "db_flags": {"is_last_rec": False, + "in_use": True, + "is_controller": False}, + "data": [255, 13, 1]}) + scenes.add_or_update(fanlinc, entry2) + scenes.compress_controllers() + print(str(scenes.data)) + # We should end up with 2 scenes: + # - Controller aa.bb.cc, group 22 -> FanLinc w/ 19 second ramp_rate + # - Controller aa.bb.cc, group 33 -> FanLinc w/ 47 second ramp_rate + # (Just checking # of scenes should be adequate for this test.) + assert len(scenes.entries) == 2 + + def test_KeypadLinc_scenes_same_ramp_rate(self): + modem = MockModem() + keypadlinc = KeypadLinc(modem.protocol, modem, Address("11.22.33"), + "KeypadLinc") + modem.devices[str(keypadlinc.addr)] = keypadlinc + device = modem.find(Address("aa.bb.cc")) + modem.devices[device.label] = device + scenes = Scenes.SceneManager(modem, None) + scenes.data = [{'controllers': [{'aa.bb.cc': {'group': 22}}], + 'responders': ['11.22.33']}, + {'controllers': [{'aa.bb.cc': {'group': 33}}], + 'responders': ['11.22.33']}] + scenes._init_scene_entries() + entry1 = DeviceEntry.from_json({"addr": "aa.bb.cc", + "group": 22, + "mem_loc" : 8119, + "db_flags": {"is_last_rec": False, + "in_use": True, + "is_controller": False}, + "data": [255, 23, 1]}) + scenes.add_or_update(keypadlinc, entry1) + entry2 = DeviceEntry.from_json({"addr": "aa.bb.cc", + "group": 33, + "mem_loc" : 8119, + "db_flags": {"is_last_rec": False, + "in_use": True, + "is_controller": False}, + "data": [255, 23, 1]}) + scenes.add_or_update(keypadlinc, entry2) + scenes.compress_controllers() + print(str(scenes.data)) + # We should end up with a single scene with: + # - 2 controller entries: aa.bb.cc, group 22, group 23 + # - 1 responder entry: 11.22.33, ramp_rate 19 seconds + assert len(scenes.entries) == 1 + assert len(scenes.data[0]['controllers']) == 2 + assert len(scenes.data[0]['responders']) == 1 + assert scenes.data[0]['responders'][0]['KeypadLinc']['ramp_rate'] == 19 + + def test_KeypadLinc_scenes_different_ramp_rates(self): + modem = MockModem() + keypadlinc = KeypadLinc(modem.protocol, modem, Address("11.22.33"), + "KeypadLinc") + modem.devices[str(keypadlinc.addr)] = keypadlinc + device = modem.find(Address("aa.bb.cc")) + modem.devices[device.label] = device + scenes = Scenes.SceneManager(modem, None) + scenes.data = [{'controllers': [{'aa.bb.cc': {'group': 22}}], + 'responders': ['11.22.33']}, + {'controllers': [{'aa.bb.cc': {'group': 33}}], + 'responders': ['11.22.33']}] + scenes._init_scene_entries() + entry1 = DeviceEntry.from_json({"addr": "aa.bb.cc", + "group": 22, + "mem_loc" : 8119, + "db_flags": {"is_last_rec": False, + "in_use": True, + "is_controller": False}, + "data": [255, 23, 1]}) + scenes.add_or_update(keypadlinc, entry1) + entry2 = DeviceEntry.from_json({"addr": "aa.bb.cc", + "group": 33, + "mem_loc" : 8119, + "db_flags": {"is_last_rec": False, + "in_use": True, + "is_controller": False}, + "data": [255, 13, 1]}) + scenes.add_or_update(keypadlinc, entry2) + scenes.compress_controllers() + print(str(scenes.data)) + # We should end up with 2 scenes: + # - Controller aa.bb.cc, group 22 -> KeypadLinc w/ 19 second ramp_rate + # - Controller aa.bb.cc, group 33 -> KeypadLinc w/ 47 second ramp_rate + # (Just checking # of scenes should be adequate for this test.) + assert len(scenes.entries) == 2 + class MockModem(): def __init__(self): self.save_path = '' From b5454978944ab52140be7f5c02092f4aa5232ebe Mon Sep 17 00:00:00 2001 From: tstabrawa <59430211+tstabrawa@users.noreply.github.com> Date: Wed, 15 Jul 2020 23:29:57 -0500 Subject: [PATCH 04/54] Iterate on Dimmer.ramp_pretty.items(), add missing break Fixes "TypeError: 'int' object is not iterable" in KeypadLinc.py / link_data_from_pretty. --- insteon_mqtt/device/FanLinc.py | 3 ++- insteon_mqtt/device/KeypadLinc.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/insteon_mqtt/device/FanLinc.py b/insteon_mqtt/device/FanLinc.py index 44f37479..572f9833 100644 --- a/insteon_mqtt/device/FanLinc.py +++ b/insteon_mqtt/device/FanLinc.py @@ -500,9 +500,10 @@ def link_data_from_pretty(self, is_controller, data): data_3 = data['group'] if 'ramp_rate' in data and data['group'] <= 0x01: data_2 = 0x1f - for ramp_key, ramp_value in Dimmer.ramp_pretty: + for ramp_key, ramp_value in Dimmer.ramp_pretty.items(): if data['ramp_rate'] >= ramp_value: data_2 = ramp_key + break if 'on_level' in data: data_1 = int(data['on_level'] * 2.55 + .5) return [data_1, data_2, data_3] diff --git a/insteon_mqtt/device/KeypadLinc.py b/insteon_mqtt/device/KeypadLinc.py index 8cd8c227..8b5cfdf5 100644 --- a/insteon_mqtt/device/KeypadLinc.py +++ b/insteon_mqtt/device/KeypadLinc.py @@ -638,9 +638,10 @@ def link_data_from_pretty(self, is_controller, data): if not is_controller and self.is_dimmer: if 'ramp_rate' in data: data_2 = 0x1f - for ramp_key, ramp_value in Dimmer.ramp_pretty: + for ramp_key, ramp_value in Dimmer.ramp_pretty.items(): if data['ramp_rate'] >= ramp_value: data_2 = ramp_key + break if 'on_level' in data: data_1 = int(data['on_level'] * 2.55 + .5) return [data_1, data_2, data_3] From 7b25c9d2936e56f11c54d6da29d3647380777a49 Mon Sep 17 00:00:00 2001 From: tstabrawa <59430211+tstabrawa@users.noreply.github.com> Date: Wed, 18 Nov 2020 00:13:33 -0600 Subject: [PATCH 05/54] Add scene tests for preserving group-0 scenes Group 0 can be used by hubs. --- tests/test_Scenes.py | 71 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/tests/test_Scenes.py b/tests/test_Scenes.py index f3b70f9f..67c70b0f 100644 --- a/tests/test_Scenes.py +++ b/tests/test_Scenes.py @@ -395,6 +395,77 @@ def test_KeypadLinc_scenes_different_ramp_rates(self): # (Just checking # of scenes should be adequate for this test.) assert len(scenes.entries) == 2 + def test_foreign_hub_group_0(self): + modem = MockModem() + device = modem.find(Address("aa.bb.cc")) + modem.devices[device.label] = device + scenes = Scenes.SceneManager(modem, None) + # We'll build the following via DeviceEntrys: + #scenes.data = [{'controllers': [{'aa.bb.cc': 0}], + # 'responders': ['cc.bb.22', 'cc.bb.aa']}] + scenes._init_scene_entries() + entry = DeviceEntry.from_json({"data": [3, 0, 239], + "mem_loc" : 8119, + "group": 0, + "db_flags": {"is_last_rec": False, + "in_use": True, + "is_controller": False}, + "addr": "aa.bb.cc"}) + device = modem.find("cc.bb.22") + scenes.add_or_update(device, entry) + device = modem.find("cc.bb.aa") + scenes.add_or_update(device, entry) + print(str(scenes.data)) + # Check that group == 0 + assert scenes.entries[0].controllers[0].group == 0 + assert scenes.entries[0].controllers[0].style == 1 + assert scenes.data[0]['controllers'][0]['dev - aa.bb.cc'] == 0 + + def test_foreign_hub_set_group_0(self): + modem = MockModem() + scenes = Scenes.SceneManager(modem, None) + scenes.data = [{'controllers': [{'a1.b1.c1': {'data_1': 0}}], + 'responders': ['cc.bb.22'], + 'name': 'test'}] + scenes._init_scene_entries() + scenes.entries[0].controllers[0].group = 0 + print(str(scenes.data)) + assert scenes.data[0]['controllers'][0]['dev - a1.b1.c1']['group'] == 0 + + def test_foreign_hub_group_0_and_1(self): + modem = MockModem() + device = modem.find(Address("aa.bb.cc")) + modem.devices[device.label] = device + scenes = Scenes.SceneManager(modem, None) + # We'll build the following via DeviceEntrys: + #scenes.data = [{'controllers': [{'aa.bb.cc': 0}, 'aa.bb.cc'], + # 'responders': ['cc.bb.aa']}] + scenes._init_scene_entries() + entry1 = DeviceEntry.from_json({"data": [3, 0, 239], + "mem_loc" : 8119, + "group": 1, + "db_flags": {"is_last_rec": False, + "in_use": True, + "is_controller": False}, + "addr": "aa.bb.cc"}) + device = modem.find("cc.bb.aa") + scenes.add_or_update(device, entry1) + entry2 = DeviceEntry.from_json({"data": [3, 0, 239], + "mem_loc" : 8119, + "group": 0, + "db_flags": {"is_last_rec": False, + "in_use": True, + "is_controller": False}, + "addr": "aa.bb.cc"}) + device = modem.find("cc.bb.aa") + scenes.add_or_update(device, entry2) + scenes.compress_controllers() + print(str(scenes.data)) + # Check that we have two controller entries & 1 responder + assert len(scenes.entries) == 1 + assert len(scenes.data[0]['controllers']) == 2 + assert len(scenes.data[0]['responders']) == 1 + class MockModem(): def __init__(self): self.save_path = '' From c49791b18ac70f287d0fd761175400e4ff43e641 Mon Sep 17 00:00:00 2001 From: tstabrawa <59430211+tstabrawa@users.noreply.github.com> Date: Thu, 16 Jul 2020 00:07:38 -0500 Subject: [PATCH 06/54] Don't treat group-0 and group-1 as equivalent Fixes scene sync trying to replace group-0 entries with group number changed to 1. --- insteon_mqtt/Scenes.py | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/insteon_mqtt/Scenes.py b/insteon_mqtt/Scenes.py index 34a4ba6b..160f2d53 100644 --- a/insteon_mqtt/Scenes.py +++ b/insteon_mqtt/Scenes.py @@ -522,7 +522,7 @@ def from_link_entry(scene_manager, device, entry): if not found_ctrl: # We know nothing about the controller so set to default values # the link-data may be wrong, but it doesn't seem to matter - if entry.group > 0x01: + if entry.group != 0x01: scene['controllers'].append({entry_addr: entry.group}) else: scene['controllers'].append(entry_addr) @@ -731,16 +731,13 @@ def __eq__(self, other): primarily by the compress_* functions ''' ret = False - self_group = self.group if self.group > 0x00 else 0x01 - other_group = other.group if other.group > 0x00 else 0x01 - if (self.addr == other.addr and self_group == other_group and + if (self.addr == other.addr and self.group == other.group and self.link_data == other.link_data): ret = True return ret def __str__(self): - self_group = self.group if self.group > 0x00 else 0x01 - subs = (self.addr, self_group, self.link_data) + subs = (self.addr, self.group, self.link_data) return 'Dev Addr: %s Group: %s Data1-3: %s' % subs def __hash__(self): @@ -822,20 +819,20 @@ def group(self, value): Args: value: (int)The group value """ - if self.style == 0 and value > 0x01: + if self.style == 0 and value != 0x01: if ('group' not in self._yaml_data[self.label] or self._yaml_data[self.label]['group'] != value): self._yaml_data[self.label]['group'] = value - if self.style == 1 and value > 0x01: + if self.style == 1 and value != 0x01: self._yaml_data[self.label] = value - if self.style == 2 and value > 0x01: + if self.style == 2 and value != 0x01: self._yaml_data = {self.label: value} - # Remove group entry in yaml_data if default value of 0x00 or 0x01 + # Remove group entry in yaml_data if default value of 0x01 if (self.style == 0 and 'group' in self._yaml_data[self.label] and - value <= 0x01): + value == 0x01): del self._yaml_data[self.label]['group'] - elif self.style == 1 and value <= 0x01: + elif self.style == 1 and value == 0x01: self._yaml_data = self.label self.update_device() @@ -891,7 +888,7 @@ def link_data(self): # Group is a bit special and gets added to link_data # a few responders (kpl) need to know group to set data_3 link_dict = self._yaml_data[self.label].copy() - if self.group > 0x01: + if self.group != 0x01: link_dict['group'] = self.group # Convert data values from human readable form pretty_data = self.device.link_data_from_pretty( From 8634bf63da0dbb35ec7767844e0f3f6df7b29bd8 Mon Sep 17 00:00:00 2001 From: tstabrawa <59430211+tstabrawa@users.noreply.github.com> Date: Thu, 19 Nov 2020 01:20:42 -0600 Subject: [PATCH 07/54] Add simple scene tests for Remotes --- tests/test_Scenes.py | 65 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/tests/test_Scenes.py b/tests/test_Scenes.py index 67c70b0f..2228a67d 100644 --- a/tests/test_Scenes.py +++ b/tests/test_Scenes.py @@ -14,6 +14,7 @@ import insteon_mqtt.device.Dimmer as Dimmer import insteon_mqtt.device.FanLinc as FanLinc import insteon_mqtt.device.KeypadLinc as KeypadLinc +import insteon_mqtt.device.Remote as Remote class Test_Scenes: @@ -466,6 +467,70 @@ def test_foreign_hub_group_0_and_1(self): assert len(scenes.data[0]['controllers']) == 2 assert len(scenes.data[0]['responders']) == 1 + def test_mini_remote_button_config_no_data3(self): + modem = MockModem() + remote = Remote(modem.protocol, modem, Address("11.22.33"), "Remote", 4) + modem.devices[str(remote.addr)] = remote + device = modem.find(Address("aa.bb.cc")) + modem.devices[device.label] = device + scenes = Scenes.SceneManager(modem, None) + # We'll build the following via DeviceEntrys: + #scenes.data = [{'controllers': [{'11.22.33': 2}], + # 'responders': ['aa.bb.cc']}] + scenes._init_scene_entries() + # The following data values are taken from an actual Mini Remote + entry = DeviceEntry.from_json({"addr": "aa.bb.cc", + "group": 2, + "mem_loc" : 8119, + "db_flags": {"is_last_rec": False, + "in_use": True, + "is_controller": True}, + "data": [3, 0, 0]}) + scenes.add_or_update(remote, entry) + print(str(scenes.data)) + # We should end up with a single scene with: + # - 1 controller entry: 11.22.33, group 2 (no data_3 value) + # - 1 responder entry: aa.bb.cc + assert len(scenes.entries) == 1 + assert len(scenes.data[0]['controllers']) == 1 + assert len(scenes.data[0]['responders']) == 1 + assert scenes.entries[0].controllers[0].group == 2 + assert scenes.entries[0].controllers[0].link_data == [3, 0, 0] + assert scenes.entries[0].controllers[0].style == 1 + + def test_mini_remote_button_config_with_data3(self): + modem = MockModem() + remote = Remote(modem.protocol, modem, Address("11.22.33"), "Remote", 4) + modem.devices[str(remote.addr)] = remote + device = modem.find(Address("aa.bb.cc")) + modem.devices[device.label] = device + scenes = Scenes.SceneManager(modem, None) + # We'll build the following via DeviceEntrys: + #scenes.data = [{'controllers': [{'11.22.33': {group: 2, data_3: 2}], + # 'responders': ['aa.bb.cc']}] + scenes._init_scene_entries() + # Preserve data_3 values if present + entry = DeviceEntry.from_json({"addr": "aa.bb.cc", + "group": 2, + "mem_loc" : 8119, + "db_flags": {"is_last_rec": False, + "in_use": True, + "is_controller": True}, + "data": [3, 0, 2]}) + scenes.add_or_update(remote, entry) + print(str(scenes.data)) + # We should end up with a single scene with: + # - 1 controller entry: 11.22.33, group 2, data_3 = 2 + # - 1 responder entry: aa.bb.cc + assert len(scenes.entries) == 1 + assert len(scenes.data[0]['controllers']) == 1 + assert len(scenes.data[0]['responders']) == 1 + assert scenes.entries[0].controllers[0].group == 2 + assert scenes.entries[0].controllers[0].link_data == [3, 0, 2] + assert scenes.entries[0].controllers[0].style == 0 + assert scenes.data[0]['controllers'][0]['Remote']['group'] == 2 + assert scenes.data[0]['controllers'][0]['Remote']['data_3'] == 2 + class MockModem(): def __init__(self): self.save_path = '' From 5e0e032bfe6e38804138379c017d35da9feede0e Mon Sep 17 00:00:00 2001 From: tstabrawa <59430211+tstabrawa@users.noreply.github.com> Date: Thu, 16 Jul 2020 23:41:20 -0500 Subject: [PATCH 08/54] Change default data_3 value for mini remotes Scenes created on mini-remote don't have group # as data_3 value. --- insteon_mqtt/device/Remote.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/insteon_mqtt/device/Remote.py b/insteon_mqtt/device/Remote.py index a511fe3b..aa89860c 100644 --- a/insteon_mqtt/device/Remote.py +++ b/insteon_mqtt/device/Remote.py @@ -196,9 +196,8 @@ def link_data(self, is_controller, group, data=None): Returns: bytes[3]: Returns a list of 3 bytes to use as D1,D2,D3. """ - # Most of this is from looking through Misterhouse bug reports. if is_controller: - defaults = [0x03, 0x00, group] + defaults = [0x03, 0x00, 0x00] # Responder data is always link dependent. Since nothing was given, # assume the user wants to turn the device on (0xff). @@ -224,7 +223,7 @@ def link_data_to_pretty(self, is_controller, data): Returns: list[3]: list, containing a dict of the human readable values """ - ret = [{'data_1': data[0]}, {'data_2': data[1]}, {'group': data[2]}] + ret = [{'data_1': data[0]}, {'data_2': data[1]}, {'data_3': data[2]}] return ret #----------------------------------------------------------------------- @@ -251,8 +250,6 @@ def link_data_from_pretty(self, is_controller, data): data_3 = None if 'data_3' in data: data_3 = data['data_3'] - if 'group' in data: - data_3 = data['group'] return [data_1, data_2, data_3] #----------------------------------------------------------------------- From c9d94648d3fa2543827fb9933901da61e71d656e Mon Sep 17 00:00:00 2001 From: tstabrawa <59430211+tstabrawa@users.noreply.github.com> Date: Tue, 21 Jul 2020 22:06:20 -0500 Subject: [PATCH 09/54] Fix typos in comments --- insteon_mqtt/Scenes.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/insteon_mqtt/Scenes.py b/insteon_mqtt/Scenes.py index 160f2d53..d90d9ef6 100644 --- a/insteon_mqtt/Scenes.py +++ b/insteon_mqtt/Scenes.py @@ -124,7 +124,7 @@ def compress_controllers(self): controllers are merged. This is a companion to compress_responders and compress_n_way. These - are seperate functions to that they can be called seperately using + are seperate functions so that they can be called seperately using Stacks. This function only processes the scenes in a single pass. If it runs @@ -164,7 +164,7 @@ def compress_responders(self): merged. This is a companion to compress_controllers and compress_n_way. - These are seperate functions to that they can be called seperately + These are seperate functions so that they can be called seperately using Stacks. This function only processes the scenes in a single pass. If it runs @@ -205,7 +205,7 @@ def compress_n_way(self): definition.. This is a companion to compress_responders and compress_controllers. - These are seperate functions to that they can be called seperately + These are seperate functions so that they can be called seperately using Stacks. This function only processes the scenes in a single pass. If it runs From 8ff32ba355c044a47d61ae770454711fb2d16dc4 Mon Sep 17 00:00:00 2001 From: tstabrawa <59430211+tstabrawa@users.noreply.github.com> Date: Fri, 20 Nov 2020 01:16:57 -0600 Subject: [PATCH 10/54] Add scene test involving multiple groups from same device --- tests/test_Scenes.py | 75 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/tests/test_Scenes.py b/tests/test_Scenes.py index 2228a67d..9bda611a 100644 --- a/tests/test_Scenes.py +++ b/tests/test_Scenes.py @@ -8,6 +8,8 @@ import insteon_mqtt as IM import insteon_mqtt.Scenes as Scenes import insteon_mqtt.Address as Address +import insteon_mqtt.CommandSeq as CommandSeq +import insteon_mqtt.db.Device as Device import insteon_mqtt.db.DeviceEntry as DeviceEntry import insteon_mqtt.db.Modem as ModemDB import insteon_mqtt.device.Base as Base @@ -531,6 +533,79 @@ def test_mini_remote_button_config_with_data3(self): assert scenes.data[0]['controllers'][0]['Remote']['group'] == 2 assert scenes.data[0]['controllers'][0]['Remote']['data_3'] == 2 + def test_foreign_hub_keypad_button_backlights_scene(self): + modem = MockModem() + keypad = KeypadLinc(modem.protocol, modem, Address("11.22.33"), + "Keypad") + modem.devices[str(keypad.addr)] = keypad + device = modem.find(Address("aa.bb.cc")) + modem.devices[device.label] = device + scenes = Scenes.SceneManager(modem, None) + # Define multiple KeypadLinc scenes and matching DB entries + scenes.data = [ + {'controllers': [{'aa.bb.cc': 19}], + 'responders': [{'11.22.33': 3}, + {'11.22.33': {'group': 4, 'on_level': 0.0}}, + {'11.22.33': {'group': 5, 'on_level': 0.0}}, + {'11.22.33': {'group': 6, 'on_level': 0.0}}]}] + keypad_db = Device.from_json( + { "address": "11.22.33", + "delta": 0, + "engine": None, + "dev_cat": 1, + "sub_cat": 66, + "firmware": 69, + "used":[ + {"addr": "aa.bb.cc", + "group": 19, + "mem_loc" : 8119, + "db_flags": {"is_last_rec": False, + "in_use": True, + "is_controller": False}, + "data": [255, 0x1f, 3]}, + {"addr": "aa.bb.cc", + "group": 19, + "mem_loc" : 8219, + "db_flags": {"is_last_rec": False, + "in_use": True, + "is_controller": False}, + "data": [0, 0x1f, 4]}, + {"addr": "aa.bb.cc", + "group": 19, + "mem_loc" : 8319, + "db_flags": {"is_last_rec": False, + "in_use": True, + "is_controller": False}, + "data": [0, 0x1f, 5]}, + {"addr": "aa.bb.cc", + "group": 19, + "mem_loc" : 8419, + "db_flags": {"is_last_rec": False, + "in_use": True, + "is_controller": False}, + "data": [0, 0x1f, 6]}], + "unused": [], + "last": {"addr": "00.00.00", + "group": 0, + "mem_loc": 8519, + "db_flags": {"is_last_rec": True, + "in_use": False, + "is_controller": False}, + "data": [0, 0, 0]}, + "meta": {} }, None, keypad) + keypad.db = keypad_db + scenes._init_scene_entries() + scenes.populate_scenes() + print(str(scenes.data)) + # Compute if any DB changes needed to implement scenes + seq = CommandSeq(modem.protocol, "Sync complete") + keypad.sync(dry_run=True, refresh=False, sequence=seq) + # Uncomment the next two lines to see what sequence would do: + #IM.log.initialize() + #seq.run() + # No changes to DB should be needed + assert len(seq.calls) == 0 + class MockModem(): def __init__(self): self.save_path = '' From b611ce38c12ca7e0816927effb0eee9d10afe2f8 Mon Sep 17 00:00:00 2001 From: tstabrawa <59430211+tstabrawa@users.noreply.github.com> Date: Tue, 21 Jul 2020 22:18:01 -0500 Subject: [PATCH 11/54] Force match on local_group in Device.diff() Fixes problem where identical entries are removed and re-added by scene sync command. --- insteon_mqtt/db/Device.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/insteon_mqtt/db/Device.py b/insteon_mqtt/db/Device.py index 445a5c0f..f77780ad 100644 --- a/insteon_mqtt/db/Device.py +++ b/insteon_mqtt/db/Device.py @@ -558,7 +558,8 @@ def diff(self, rhs): delta = DbDiff(self.addr) for entry in self.entries.values(): - rhsEntry = rhs.find(entry.addr, entry.group, entry.is_controller) + rhsEntry = rhs.find(entry.addr, entry.group, entry.is_controller, + entry.data[2]) # RHS is missing this entry or has different data bytes we need # to update. From 0a4e9369d5b24d75a97932b5b5d5d93e8848b0d7 Mon Sep 17 00:00:00 2001 From: tstabrawa <59430211+tstabrawa@users.noreply.github.com> Date: Fri, 20 Nov 2020 23:28:11 -0600 Subject: [PATCH 12/54] Add scene test for preserving FanLinc dimmer ramp rates --- tests/test_Scenes.py | 50 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/tests/test_Scenes.py b/tests/test_Scenes.py index 9bda611a..8348b9bf 100644 --- a/tests/test_Scenes.py +++ b/tests/test_Scenes.py @@ -606,6 +606,56 @@ def test_foreign_hub_keypad_button_backlights_scene(self): # No changes to DB should be needed assert len(seq.calls) == 0 + def test_fanlinc_dimmer_ramp_rate_scene(self): + modem = MockModem() + fanlinc = FanLinc(modem.protocol, modem, Address("11.22.33"), "FanLinc") + modem.devices[str(fanlinc.addr)] = fanlinc + device = modem.find(Address("aa.bb.cc")) + modem.devices[device.label] = device + scenes = Scenes.SceneManager(modem, None) + # Define a FanLinc scene with ramp rate and a matching DB entry + scenes.data = [ + {'controllers': [{'aa.bb.cc': 22}], + 'responders': [{'11.22.33': {'ramp_rate': 19, 'group': 1}}]}] + fanlinc_db = Device.from_json( + { "address": "11.22.33", + "delta": 0, + "engine": None, + "dev_cat": 1, + "sub_cat": 46, + "firmware": 69, + "used":[ + {"addr": "aa.bb.cc", + "group": 22, + "mem_loc" : 8119, + "db_flags": {"is_last_rec": False, + "in_use": True, + "is_controller": False}, + "data": [255, 23, 1]}], + "unused": [], + "last": {"addr": "00.00.00", + "group": 0, + "mem_loc": 8519, + "db_flags": {"is_last_rec": True, + "in_use": False, + "is_controller": False}, + "data": [0, 0, 0]}, + "meta": {} }, None, fanlinc) + fanlinc.db = fanlinc_db + scenes._init_scene_entries() + scenes.populate_scenes() + print(str(scenes.data)) + # Make sure link data matches scene config & DB entry: + assert scenes.entries[0].responders[0].link_data == [255, 23, 1] + # Compute if any DB changes needed to implement scenes + seq = CommandSeq(modem.protocol, "Sync complete") + fanlinc.sync(dry_run=True, refresh=False, sequence=seq) + # Uncomment the next two lines to see what sequence would do: + #IM.log.initialize() + #seq.run() + # No changes to DB should be needed + assert len(seq.calls) == 0 + class MockModem(): def __init__(self): self.save_path = '' From 4bd65dd8ffb414e9a00cce33b43deed917778158 Mon Sep 17 00:00:00 2001 From: tstabrawa <59430211+tstabrawa@users.noreply.github.com> Date: Wed, 22 Jul 2020 00:01:13 -0500 Subject: [PATCH 13/54] Allow FanLinc to parse ramp_rate if group is default Fixes problem where FanLinc scenes read from yaml would all have ramp rate set to 0x1f. --- insteon_mqtt/device/FanLinc.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/insteon_mqtt/device/FanLinc.py b/insteon_mqtt/device/FanLinc.py index 572f9833..3687e935 100644 --- a/insteon_mqtt/device/FanLinc.py +++ b/insteon_mqtt/device/FanLinc.py @@ -498,12 +498,12 @@ def link_data_from_pretty(self, is_controller, data): if not is_controller: if 'group' in data: data_3 = data['group'] - if 'ramp_rate' in data and data['group'] <= 0x01: - data_2 = 0x1f - for ramp_key, ramp_value in Dimmer.ramp_pretty.items(): - if data['ramp_rate'] >= ramp_value: - data_2 = ramp_key - break + if 'ramp_rate' in data and (data_3 is None or data_3 == 0x01): + data_2 = 0x1f + for ramp_key, ramp_value in Dimmer.ramp_pretty.items(): + if data['ramp_rate'] >= ramp_value: + data_2 = ramp_key + break if 'on_level' in data: data_1 = int(data['on_level'] * 2.55 + .5) return [data_1, data_2, data_3] From 6fa889b97d6c0a3620b99f4b8fbfd2ac3623fb85 Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Wed, 25 Nov 2020 12:17:50 -0800 Subject: [PATCH 14/54] Wait on Receiving Pre NAK If receive a Pre NAK, add to wait time and wait for actual NAK. This is a strange NAK which is sent when the device db is large and searching it would take more time that permitted in normal message ACK. This may fix #231 --- insteon_mqtt/handler/DeviceDbModify.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/insteon_mqtt/handler/DeviceDbModify.py b/insteon_mqtt/handler/DeviceDbModify.py index 73c632a1..517af79c 100644 --- a/insteon_mqtt/handler/DeviceDbModify.py +++ b/insteon_mqtt/handler/DeviceDbModify.py @@ -76,10 +76,16 @@ def msg_received(self, protocol, msg): self.entry) elif msg.flags.type == Msg.Flags.Type.DIRECT_NAK: - LOG.error("%s db mod NAK: %s, Message: %s", self.db.addr, - msg.nak_str(), msg) - self.on_done(False, "Device database update failed. " + - msg.nak_str(), None) + if msg.cmd2 == 0xFC: + # This is a "Pre NAK in case database search takes + # too long". This happens when the device database is + # large. Just ignore it, add more wait time and wait. + return Msg.CONTINUE + else: + LOG.error("%s db mod NAK: %s, Message: %s", self.db.addr, + msg.nak_str(), msg) + self.on_done(False, "Device database update failed. " + + msg.nak_str(), None) else: LOG.error("%s db mod unexpected msg type: %s", From cce5bfe7f2eee53229589d3c28c5f873ffac036f Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Wed, 25 Nov 2020 13:33:10 -0800 Subject: [PATCH 15/54] Add Log Warning on Skipping Pre-NAK --- insteon_mqtt/handler/DeviceDbModify.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/insteon_mqtt/handler/DeviceDbModify.py b/insteon_mqtt/handler/DeviceDbModify.py index 517af79c..445cc9ba 100644 --- a/insteon_mqtt/handler/DeviceDbModify.py +++ b/insteon_mqtt/handler/DeviceDbModify.py @@ -80,6 +80,8 @@ def msg_received(self, protocol, msg): # This is a "Pre NAK in case database search takes # too long". This happens when the device database is # large. Just ignore it, add more wait time and wait. + LOG.warning("%s Pre-NAK: %s, Message: %s", self.db.addr, + msg.nak_str(), msg) return Msg.CONTINUE else: LOG.error("%s db mod NAK: %s, Message: %s", self.db.addr, From 3cd8211962000c14cc5e364b629477980d76d4fc Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Wed, 25 Nov 2020 14:27:23 -0800 Subject: [PATCH 16/54] Handle Pre-NAK in DeviceDBGet Apparently this NAK can also be sent on a db_get request. --- insteon_mqtt/handler/DeviceDbGet.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/insteon_mqtt/handler/DeviceDbGet.py b/insteon_mqtt/handler/DeviceDbGet.py index 0d4cad2f..d6cf91a7 100644 --- a/insteon_mqtt/handler/DeviceDbGet.py +++ b/insteon_mqtt/handler/DeviceDbGet.py @@ -86,11 +86,19 @@ def msg_received(self, protocol, msg): return Msg.CONTINUE elif msg.flags.type == Msg.Flags.Type.DIRECT_NAK: - LOG.error("%s device NAK error: %s, Message: %s", - msg.from_addr, msg.nak_str(), msg) - self.on_done(False, "Database command NAK. " + msg.nak_str(), - None) - return Msg.FINISHED + if msg.cmd2 == 0xFC: + # This is a "Pre NAK in case database search takes + # too long". This happens when the device database is + # large. Just ignore it, add more wait time and wait. + LOG.warning("%s Pre-NAK: %s, Message: %s", msg.from_addr, + msg.nak_str(), msg) + return Msg.CONTINUE + else: + LOG.error("%s device NAK error: %s, Message: %s", + msg.from_addr, msg.nak_str(), msg) + self.on_done(False, "Database command NAK. " + msg.nak_str(), + None) + return Msg.FINISHED else: LOG.warning("%s device unexpected msg: %s", msg.from_addr, msg) From 7f86cbdc19471204e938acbaf6c45e2e0c9a17cf Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Wed, 25 Nov 2020 16:45:12 -0800 Subject: [PATCH 17/54] Add Pre NAK Handling Wherever DIRECT_NAK is Handled It seems that this NAK can be sent whenever. So anyplace where DIRECT_NAKs result in a cancellation of sending, code has been added to ignore the Pre NAKs. --- insteon_mqtt/handler/BroadcastCmdResponse.py | 14 +++++++++++--- insteon_mqtt/handler/ExtendedCmdResponse.py | 18 +++++++++++++----- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/insteon_mqtt/handler/BroadcastCmdResponse.py b/insteon_mqtt/handler/BroadcastCmdResponse.py index ef0e412c..dd91ea55 100644 --- a/insteon_mqtt/handler/BroadcastCmdResponse.py +++ b/insteon_mqtt/handler/BroadcastCmdResponse.py @@ -82,9 +82,17 @@ def msg_received(self, protocol, msg): return Msg.CONTINUE elif msg.flags.type == Msg.Flags.Type.DIRECT_NAK: - LOG.error("%s device NAK error: %s", msg.from_addr, msg) - self.on_done(False, "Device command NAK", None) - return Msg.FINISHED + if msg.cmd2 == 0xFC: + # This is a "Pre NAK in case database search takes + # too long". This happens when the device database is + # large. Just ignore it, add more wait time and wait. + LOG.warning("%s Pre-NAK: %s, Message: %s", msg.from_addr, + msg.nak_str(), msg) + return Msg.CONTINUE + else: + LOG.error("%s device NAK error: %s", msg.from_addr, msg) + self.on_done(False, "Device command NAK", None) + return Msg.FINISHED else: LOG.warning("%s device unexpected msg: %s", msg.from_addr, msg) diff --git a/insteon_mqtt/handler/ExtendedCmdResponse.py b/insteon_mqtt/handler/ExtendedCmdResponse.py index 1f064e40..39e6edb0 100644 --- a/insteon_mqtt/handler/ExtendedCmdResponse.py +++ b/insteon_mqtt/handler/ExtendedCmdResponse.py @@ -88,11 +88,19 @@ def msg_received(self, protocol, msg): return Msg.CONTINUE elif msg.flags.type == Msg.Flags.Type.DIRECT_NAK: - LOG.error("%s device NAK error: %s, Message: %s", - msg.from_addr, msg.nak_str(), msg) - self.on_done(False, "Device command NAK. " + msg.nak_str(), - None) - return Msg.FINISHED + if msg.cmd2 == 0xFC: + # This is a "Pre NAK in case database search takes + # too long". This happens when the device database is + # large. Just ignore it, add more wait time and wait. + LOG.warning("%s Pre-NAK: %s, Message: %s", msg.from_addr, + msg.nak_str(), msg) + return Msg.CONTINUE + else: + LOG.error("%s device NAK error: %s, Message: %s", + msg.from_addr, msg.nak_str(), msg) + self.on_done(False, "Device command NAK. " + msg.nak_str(), + None) + return Msg.FINISHED else: LOG.warning("%s device unexpected msg: %s", msg.from_addr, msg) From 9ba2ffe6f81e7d63c2de56fdf49afcc1a6df32dc Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Thu, 3 Dec 2020 11:29:20 -0800 Subject: [PATCH 18/54] Fif Flake Errors, Lines too Long --- insteon_mqtt/handler/DeviceDbGet.py | 4 ++-- insteon_mqtt/handler/DeviceDbModify.py | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/insteon_mqtt/handler/DeviceDbGet.py b/insteon_mqtt/handler/DeviceDbGet.py index d6cf91a7..77c9f112 100644 --- a/insteon_mqtt/handler/DeviceDbGet.py +++ b/insteon_mqtt/handler/DeviceDbGet.py @@ -96,8 +96,8 @@ def msg_received(self, protocol, msg): else: LOG.error("%s device NAK error: %s, Message: %s", msg.from_addr, msg.nak_str(), msg) - self.on_done(False, "Database command NAK. " + msg.nak_str(), - None) + self.on_done(False, "Database command NAK. " + + msg.nak_str(), None) return Msg.FINISHED else: diff --git a/insteon_mqtt/handler/DeviceDbModify.py b/insteon_mqtt/handler/DeviceDbModify.py index 445cc9ba..5a27aade 100644 --- a/insteon_mqtt/handler/DeviceDbModify.py +++ b/insteon_mqtt/handler/DeviceDbModify.py @@ -80,12 +80,12 @@ def msg_received(self, protocol, msg): # This is a "Pre NAK in case database search takes # too long". This happens when the device database is # large. Just ignore it, add more wait time and wait. - LOG.warning("%s Pre-NAK: %s, Message: %s", self.db.addr, - msg.nak_str(), msg) + LOG.warning("%s Pre-NAK: %s, Message: %s", + self.db.addr, msg.nak_str(), msg) return Msg.CONTINUE else: - LOG.error("%s db mod NAK: %s, Message: %s", self.db.addr, - msg.nak_str(), msg) + LOG.error("%s db mod NAK: %s, Message: %s", + self.db.addr, msg.nak_str(), msg) self.on_done(False, "Device database update failed. " + msg.nak_str(), None) From 33ac3759ec5398d506f9946e199f40716acdcdb9 Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Thu, 3 Dec 2020 11:36:39 -0800 Subject: [PATCH 19/54] Add Tests for Pre Nak --- tests/handler/test_BroadcastCmdResponse.py | 6 ++++++ tests/handler/test_DeviceDbGet.py | 6 ++++++ tests/handler/test_ExtendedCmdResponse.py | 6 ++++++ 3 files changed, 18 insertions(+) diff --git a/tests/handler/test_BroadcastCmdResponse.py b/tests/handler/test_BroadcastCmdResponse.py index 3a58f537..7d7f708a 100644 --- a/tests/handler/test_BroadcastCmdResponse.py +++ b/tests/handler/test_BroadcastCmdResponse.py @@ -67,6 +67,12 @@ def callback(msg, on_done=None): r = handler.msg_received(proto, msg) assert r == Msg.FINISHED + # direct Pre NAK + flags = Msg.Flags(Msg.Flags.Type.DIRECT_NAK, False) + msg = Msg.InpStandard(addr, addr, flags, 0x10, 0xFC) + r = handler.msg_received(proto, msg) + assert r == Msg.CONTINUE + # unexpected flags = Msg.Flags(Msg.Flags.Type.ALL_LINK_BROADCAST, False) msg = Msg.InpStandard(addr, addr, flags, 0x10, 0x00) diff --git a/tests/handler/test_DeviceDbGet.py b/tests/handler/test_DeviceDbGet.py index 0441b5ed..f6b4810e 100644 --- a/tests/handler/test_DeviceDbGet.py +++ b/tests/handler/test_DeviceDbGet.py @@ -36,6 +36,12 @@ def callback(success, msg, value): r = handler.msg_received(proto, std_ack) assert r == Msg.UNKNOWN + # direct Pre NAK + flags = Msg.Flags(Msg.Flags.Type.DIRECT_NAK, False) + msg = Msg.InpStandard(addr, addr, flags, 0x2f, 0xFC) + r = handler.msg_received(proto, msg) + assert r == Msg.CONTINUE + # Try w/ an extended msg. ext_data = bytes(14) ext_ack = Msg.OutExtended.direct(addr, 0x2f, 0x00, ext_data) diff --git a/tests/handler/test_ExtendedCmdResponse.py b/tests/handler/test_ExtendedCmdResponse.py index 56ddc283..1e76a421 100644 --- a/tests/handler/test_ExtendedCmdResponse.py +++ b/tests/handler/test_ExtendedCmdResponse.py @@ -69,6 +69,12 @@ def callback(msg, on_done=None): r = handler.msg_received(proto, msg) assert r == Msg.FINISHED + # direct Pre NAK + flags = Msg.Flags(Msg.Flags.Type.DIRECT_NAK, False) + msg = Msg.InpStandard(addr, addr, flags, 0x2e, 0xFC) + r = handler.msg_received(proto, msg) + assert r == Msg.CONTINUE + # unexpected flags = Msg.Flags(Msg.Flags.Type.BROADCAST, False) msg = Msg.InpStandard(addr, addr, flags, 0x2e, 0x00) From 497071cf29c042f88b2dad63aee5c799e0c0c58c Mon Sep 17 00:00:00 2001 From: tstabrawa <59430211+tstabrawa@users.noreply.github.com> Date: Wed, 9 Dec 2020 00:11:25 -0600 Subject: [PATCH 20/54] Also parse FanLinc ramp rate if group is 0 Changed per code-review suggestion. --- insteon_mqtt/device/FanLinc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/insteon_mqtt/device/FanLinc.py b/insteon_mqtt/device/FanLinc.py index 3687e935..1a61d9f6 100644 --- a/insteon_mqtt/device/FanLinc.py +++ b/insteon_mqtt/device/FanLinc.py @@ -498,7 +498,7 @@ def link_data_from_pretty(self, is_controller, data): if not is_controller: if 'group' in data: data_3 = data['group'] - if 'ramp_rate' in data and (data_3 is None or data_3 == 0x01): + if 'ramp_rate' in data and (data_3 is None or data_3 <= 0x01): data_2 = 0x1f for ramp_key, ramp_value in Dimmer.ramp_pretty.items(): if data['ramp_rate'] >= ramp_value: From efeffbe25dad57446df03b22043aba781fb41f0b Mon Sep 17 00:00:00 2001 From: tstabrawa <59430211+tstabrawa@users.noreply.github.com> Date: Sat, 12 Dec 2020 14:39:15 -0600 Subject: [PATCH 21/54] Also handle Pre-NAK in StandardCmd Fixes #247 --- insteon_mqtt/handler/StandardCmd.py | 9 +++++++++ tests/handler/test_StandardCmd.py | 6 ++++++ 2 files changed, 15 insertions(+) diff --git a/insteon_mqtt/handler/StandardCmd.py b/insteon_mqtt/handler/StandardCmd.py index 76b8fe3a..3afe8896 100644 --- a/insteon_mqtt/handler/StandardCmd.py +++ b/insteon_mqtt/handler/StandardCmd.py @@ -86,6 +86,15 @@ def msg_received(self, protocol, msg): # If this message matches our address and command, it's probably # the ACK we're expecting. if msg.from_addr == self.addr and msg.cmd1 == self.cmd: + if (msg.flags.type == Msg.Flags.Type.DIRECT_NAK and + msg.cmd2 == 0xFC): + # This is a "Pre NAK in case database search takes + # too long". This happens when the device database is + # large. Just ignore it, add more wait time and wait. + LOG.warning("%s Pre-NAK: %s, Message: %s", msg.from_addr, + msg.nak_str(), msg) + return Msg.CONTINUE + # Run the callback - it's up to the callback to check if this # is really the ACK or not. self.callback(msg, on_done=self.on_done) diff --git a/tests/handler/test_StandardCmd.py b/tests/handler/test_StandardCmd.py index d1c89109..f68aa77c 100644 --- a/tests/handler/test_StandardCmd.py +++ b/tests/handler/test_StandardCmd.py @@ -41,6 +41,12 @@ def callback(msg, on_done=None): r = handler.msg_received(proto, out) assert r == Msg.UNKNOWN + # direct Pre NAK + flags = Msg.Flags(Msg.Flags.Type.DIRECT_NAK, False) + msg = Msg.InpStandard(addr, addr, flags, 0x11, 0xFC) + r = handler.msg_received(proto, msg) + assert r == Msg.CONTINUE + # Now pass in the input message. # expected input meesage From 01321f4ff3c30318c218debdc13280119ef558a5 Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Mon, 14 Dec 2020 14:09:38 -0800 Subject: [PATCH 22/54] Define NAK Types as Enum; Use Enum When Testing Nak Type Makes things easier to read. --- insteon_mqtt/handler/BroadcastCmdResponse.py | 2 +- insteon_mqtt/handler/DeviceDbGet.py | 2 +- insteon_mqtt/handler/DeviceDbModify.py | 2 +- insteon_mqtt/handler/ExtendedCmdResponse.py | 2 +- insteon_mqtt/message/InpStandard.py | 9 +++++++++ 5 files changed, 13 insertions(+), 4 deletions(-) diff --git a/insteon_mqtt/handler/BroadcastCmdResponse.py b/insteon_mqtt/handler/BroadcastCmdResponse.py index dd91ea55..f82805a1 100644 --- a/insteon_mqtt/handler/BroadcastCmdResponse.py +++ b/insteon_mqtt/handler/BroadcastCmdResponse.py @@ -82,7 +82,7 @@ def msg_received(self, protocol, msg): return Msg.CONTINUE elif msg.flags.type == Msg.Flags.Type.DIRECT_NAK: - if msg.cmd2 == 0xFC: + if msg.cmd2 == msg.NakType.PRE_NAK: # This is a "Pre NAK in case database search takes # too long". This happens when the device database is # large. Just ignore it, add more wait time and wait. diff --git a/insteon_mqtt/handler/DeviceDbGet.py b/insteon_mqtt/handler/DeviceDbGet.py index 062e91ce..f19ba494 100644 --- a/insteon_mqtt/handler/DeviceDbGet.py +++ b/insteon_mqtt/handler/DeviceDbGet.py @@ -103,7 +103,7 @@ def msg_received(self, protocol, msg): return Msg.CONTINUE elif msg.flags.type == Msg.Flags.Type.DIRECT_NAK: - if msg.cmd2 == 0xFC: + if msg.cmd2 == msg.NakType.PRE_NAK: # This is a "Pre NAK in case database search takes # too long". This happens when the device database is # large. Just ignore it, add more wait time and wait. diff --git a/insteon_mqtt/handler/DeviceDbModify.py b/insteon_mqtt/handler/DeviceDbModify.py index 5c530167..8d0ad5f7 100644 --- a/insteon_mqtt/handler/DeviceDbModify.py +++ b/insteon_mqtt/handler/DeviceDbModify.py @@ -78,7 +78,7 @@ def msg_received(self, protocol, msg): self.entry) elif msg.flags.type == Msg.Flags.Type.DIRECT_NAK: - if msg.cmd2 == 0xFC: + if msg.cmd2 == msg.NakType.PRE_NAK: # This is a "Pre NAK in case database search takes # too long". This happens when the device database is # large. Just ignore it, add more wait time and wait. diff --git a/insteon_mqtt/handler/ExtendedCmdResponse.py b/insteon_mqtt/handler/ExtendedCmdResponse.py index 39e6edb0..fb26475e 100644 --- a/insteon_mqtt/handler/ExtendedCmdResponse.py +++ b/insteon_mqtt/handler/ExtendedCmdResponse.py @@ -88,7 +88,7 @@ def msg_received(self, protocol, msg): return Msg.CONTINUE elif msg.flags.type == Msg.Flags.Type.DIRECT_NAK: - if msg.cmd2 == 0xFC: + if msg.cmd2 == msg.NakType.PRE_NAK: # This is a "Pre NAK in case database search takes # too long". This happens when the device database is # large. Just ignore it, add more wait time and wait. diff --git a/insteon_mqtt/message/InpStandard.py b/insteon_mqtt/message/InpStandard.py index 5d894616..f627d663 100644 --- a/insteon_mqtt/message/InpStandard.py +++ b/insteon_mqtt/message/InpStandard.py @@ -3,6 +3,7 @@ # Input insteon standard and extended message. # #=========================================================================== +import enum import io import time from ..Address import Address @@ -23,6 +24,14 @@ class InpStandard(Base): msg_code = 0x50 fixed_msg_size = 11 + # NAK types + class NakType(enum.IntEnum): + SENDER_NOT_IN_DB = 0xFF + NO_LOAD = 0xFE + BAD_CHECKSUM = 0xFD + PRE_NAK = 0xFC + ILLEGAL_VALUE = 0xFB + #----------------------------------------------------------------------- @classmethod def from_bytes(cls, raw): From ba318d7819107f00d6fd2481bfed9a40e6a40a3a Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Mon, 14 Dec 2020 14:10:13 -0800 Subject: [PATCH 23/54] Catch All NAKs at StandardCmd Level --- insteon_mqtt/handler/StandardCmd.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/insteon_mqtt/handler/StandardCmd.py b/insteon_mqtt/handler/StandardCmd.py index 3afe8896..9b3db41c 100644 --- a/insteon_mqtt/handler/StandardCmd.py +++ b/insteon_mqtt/handler/StandardCmd.py @@ -86,14 +86,20 @@ def msg_received(self, protocol, msg): # If this message matches our address and command, it's probably # the ACK we're expecting. if msg.from_addr == self.addr and msg.cmd1 == self.cmd: - if (msg.flags.type == Msg.Flags.Type.DIRECT_NAK and - msg.cmd2 == 0xFC): - # This is a "Pre NAK in case database search takes - # too long". This happens when the device database is - # large. Just ignore it, add more wait time and wait. - LOG.warning("%s Pre-NAK: %s, Message: %s", msg.from_addr, - msg.nak_str(), msg) - return Msg.CONTINUE + if msg.flags.type == Msg.Flags.Type.DIRECT_NAK: + if msg.cmd2 == msg.NakType.PRE_NAK: + # This is a "Pre NAK in case database search takes + # too long". This happens when the device database is + # large. Just ignore it, add more wait time and wait. + LOG.warning("%s Pre-NAK: %s, Message: %s", + msg.from_addr, msg.nak_str(), msg) + return Msg.CONTINUE + else: + LOG.error("%s device NAK error: %s, Message: %s", + msg.from_addr, msg.nak_str(), msg) + self.on_done(False, "Command failed. " + + msg.nak_str(), None) + return Msg.FINISHED # Run the callback - it's up to the callback to check if this # is really the ACK or not. From 6ff7db21bfb1ad6555155a481f6505307f8337c6 Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Mon, 14 Dec 2020 14:53:44 -0800 Subject: [PATCH 24/54] Create StandardCmd Child that Passes on NAKs Basically only used for handle_engine. This could have been written around via wraping the on_done callback, but the log messaging to the user was not as intuitive. --- insteon_mqtt/device/Base.py | 8 ++------ insteon_mqtt/handler/StandardCmd.py | 3 +-- insteon_mqtt/handler/__init__.py | 1 + insteon_mqtt/message/InpStandard.py | 2 +- 4 files changed, 5 insertions(+), 9 deletions(-) diff --git a/insteon_mqtt/device/Base.py b/insteon_mqtt/device/Base.py index cae31418..500b9faa 100644 --- a/insteon_mqtt/device/Base.py +++ b/insteon_mqtt/device/Base.py @@ -454,7 +454,7 @@ def get_engine(self, on_done=None): # Send the get_engine_version request. msg = Msg.OutStandard.direct(self.addr, 0x0D, 0x00) - msg_handler = handler.StandardCmd(msg, self.handle_engine, on_done) + msg_handler = handler.StandardCmdNAK(msg, self.handle_engine, on_done) self.send(msg, msg_handler) #----------------------------------------------------------------------- @@ -1092,11 +1092,7 @@ def handle_linking(self, msg, on_done=None): completed. Signature is: on_done(success, msg, data) """ on_done = util.make_callback(on_done) - - if msg.flags.type == Msg.Flags.Type.DIRECT_ACK: - on_done(True, "Entered linking mode", None) - else: - on_done(False, "Linking mode failed", None) + on_done(True, "Entered linking mode", None) #----------------------------------------------------------------------- def _db_update(self, local_group, is_controller, remote_addr, remote_group, diff --git a/insteon_mqtt/handler/StandardCmd.py b/insteon_mqtt/handler/StandardCmd.py index 9b3db41c..e9033b41 100644 --- a/insteon_mqtt/handler/StandardCmd.py +++ b/insteon_mqtt/handler/StandardCmd.py @@ -101,8 +101,7 @@ def msg_received(self, protocol, msg): msg.nak_str(), None) return Msg.FINISHED - # Run the callback - it's up to the callback to check if this - # is really the ACK or not. + # Run the callback self.callback(msg, on_done=self.on_done) # Indicate no more messages are expected. diff --git a/insteon_mqtt/handler/__init__.py b/insteon_mqtt/handler/__init__.py index 39d8c27f..e869cdbf 100644 --- a/insteon_mqtt/handler/__init__.py +++ b/insteon_mqtt/handler/__init__.py @@ -44,5 +44,6 @@ from .ModemReset import ModemReset from .ModemScene import ModemScene from .StandardCmd import StandardCmd +from .StandardCmdNAK import StandardCmdNAK from .ThermostatCmd import ThermostatCmd from .BroadcastCmdResponse import BroadcastCmdResponse diff --git a/insteon_mqtt/message/InpStandard.py b/insteon_mqtt/message/InpStandard.py index f627d663..8893534e 100644 --- a/insteon_mqtt/message/InpStandard.py +++ b/insteon_mqtt/message/InpStandard.py @@ -113,7 +113,7 @@ def nak_str(self): """ ret = "" naks = { - 0xFF: "Senders ID not in responders db. Try linking again.", + 0xFF: "Senders ID not in responders db. Try running 'join' again.", 0xFE: "Load sense detects no load", 0xFD: "Checksum is incorrect", 0xFC: "Pre NAK in case database search takes too long", From d259464f041a1c3dda69348223abc227e41ab446 Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Mon, 14 Dec 2020 17:10:13 -0800 Subject: [PATCH 25/54] Move All Ack/NAK Processing into StandardCmd I don't see anything that legitimately needs to see a NAK in here it is all logging messages or unnecessary things. --- insteon_mqtt/device/Base.py | 2 +- insteon_mqtt/device/Dimmer.py | 77 +++++++------------- insteon_mqtt/device/FanLinc.py | 19 ++--- insteon_mqtt/device/IOLinc.py | 26 +++---- insteon_mqtt/device/KeypadLinc.py | 115 +++++++++++------------------- insteon_mqtt/device/Outlet.py | 44 ++++-------- insteon_mqtt/device/Switch.py | 48 ++++--------- insteon_mqtt/device/Thermostat.py | 26 ++----- 8 files changed, 117 insertions(+), 240 deletions(-) diff --git a/insteon_mqtt/device/Base.py b/insteon_mqtt/device/Base.py index 500b9faa..fd29ad6f 100644 --- a/insteon_mqtt/device/Base.py +++ b/insteon_mqtt/device/Base.py @@ -268,7 +268,7 @@ def join(self, on_done=None): LOG.info("Join Device %s", self.addr) # Using a sequence so we can pass the on_done function through. - seq = CommandSeq(self, "Operation Complete", on_done) + seq = CommandSeq(self, "Device joined.", on_done) # First get the engine version. This process only works and is # necessary on I2CS devices. diff --git a/insteon_mqtt/device/Dimmer.py b/insteon_mqtt/device/Dimmer.py index a47dc2bf..0c37d605 100644 --- a/insteon_mqtt/device/Dimmer.py +++ b/insteon_mqtt/device/Dimmer.py @@ -577,10 +577,7 @@ def handle_backlight(self, msg, on_done): on_done: Finished callback. This is called when the command has completed. Signature is: on_done(success, msg, data) """ - if msg.flags.type == Msg.Flags.Type.DIRECT_ACK: - on_done(True, "Backlight level updated", None) - else: - on_done(False, "Backlight level failed", None) + on_done(True, "Backlight level updated", None) #----------------------------------------------------------------------- def handle_on_level(self, msg, on_done): @@ -595,10 +592,7 @@ def handle_on_level(self, msg, on_done): on_done: Finished callback. This is called when the command has completed. Signature is: on_done(success, msg, data) """ - if msg.flags.type == Msg.Flags.Type.DIRECT_ACK: - on_done(True, "Button on level updated", None) - else: - on_done(False, "Button on level failed", None) + on_done(True, "Button on level updated", None) #----------------------------------------------------------------------- def handle_broadcast(self, msg): @@ -706,20 +700,13 @@ def handle_ack(self, msg, on_done, reason=""): """ # If this it the ACK we're expecting, update the internal state and # emit our signals. - if msg.flags.type == Msg.Flags.Type.DIRECT_ACK: - LOG.debug("Dimmer %s ACK: %s", self.addr, msg) - - _is_on, mode = on_off.Mode.decode(msg.cmd1) - reason = reason if reason else on_off.REASON_COMMAND - self._set_level(msg.cmd2, mode, reason) - on_done(True, "Dimmer state updated to %s" % self._level, - msg.cmd2) + LOG.debug("Dimmer %s ACK: %s", self.addr, msg) - elif msg.flags.type == Msg.Flags.Type.DIRECT_NAK: - LOG.error("Dimmer %s NAK error: %s, Message: %s", self.addr, - msg.nak_str(), msg) - on_done(False, "Dimmer state update failed. " + msg.nak_str(), - None) + _is_on, mode = on_off.Mode.decode(msg.cmd1) + reason = reason if reason else on_off.REASON_COMMAND + self._set_level(msg.cmd2, mode, reason) + on_done(True, "Dimmer state updated to %s" % self._level, + msg.cmd2) #----------------------------------------------------------------------- def handle_scene(self, msg, on_done, reason=""): @@ -741,21 +728,14 @@ def handle_scene(self, msg, on_done, reason=""): # Call the callback. We don't change state here - the device will # send a regular broadcast message which will run handle_broadcast # which will then update the state. - if msg.flags.type == Msg.Flags.Type.DIRECT_ACK: - LOG.debug("Dimmer %s ACK: %s", self.addr, msg) - - # Reason is device because we're simulating a button press. We - # can't really pass this around because we just get a broadcast - # message later from the device. So we set a temporary variable - # here and use it in handle_broadcast() to output the reason. - self.broadcast_reason = reason if reason else on_off.REASON_DEVICE - on_done(True, "Scene triggered", None) - - elif msg.flags.type == Msg.Flags.Type.DIRECT_NAK: - LOG.error("Dimmer %s NAK error: %s, Message: %s", self.addr, - msg.nak_str(), msg) - on_done(False, "Scene trigger failed failed. " + msg.nak_str(), - None) + LOG.debug("Dimmer %s ACK: %s", self.addr, msg) + + # Reason is device because we're simulating a button press. We + # can't really pass this around because we just get a broadcast + # message later from the device. So we set a temporary variable + # here and use it in handle_broadcast() to output the reason. + self.broadcast_reason = reason if reason else on_off.REASON_DEVICE + on_done(True, "Scene triggered", None) #----------------------------------------------------------------------- def handle_increment(self, msg, on_done, delta, reason=""): @@ -777,22 +757,15 @@ def handle_increment(self, msg, on_done, delta, reason=""): """ # If this it the ACK we're expecting, update the internal state and # emit our signals. - if msg.flags.type == Msg.Flags.Type.DIRECT_ACK: - LOG.debug("Dimmer %s ACK: %s", self.addr, msg) - - # Add the delta and bound at [0, 255] - level = min(self._level + delta, 255) - level = max(level, 0) - self._set_level(level, reason=reason) - - s = "Dimmer %s state updated to %s" % (self.addr, self._level) - on_done(True, s, msg.cmd2) - - elif msg.flags.Dimmer == Msg.Flags.Type.DIRECT_NAK: - LOG.error("Dimmer %s NAK error: %s, Message: %s", self.addr, - msg.nak_str(), msg) - on_done(False, "Dimmer %s state update failed. " + msg.nak_str(), - None) + LOG.debug("Dimmer %s ACK: %s", self.addr, msg) + + # Add the delta and bound at [0, 255] + level = min(self._level + delta, 255) + level = max(level, 0) + self._set_level(level, reason=reason) + + s = "Dimmer %s state updated to %s" % (self.addr, self._level) + on_done(True, s, msg.cmd2) #----------------------------------------------------------------------- def handle_group_cmd(self, addr, msg): diff --git a/insteon_mqtt/device/FanLinc.py b/insteon_mqtt/device/FanLinc.py index e2567a38..b52b31cd 100644 --- a/insteon_mqtt/device/FanLinc.py +++ b/insteon_mqtt/device/FanLinc.py @@ -338,19 +338,12 @@ def handle_speed(self, msg, on_done=None, reason=""): # If this it the ACK we're expecting, update the internal state and # emit our signals. - if msg.flags.type == Msg.Flags.Type.DIRECT_ACK: - LOG.debug("FanLinc fan %s ACK: %s", self.addr, msg) - - reason = reason if reason else on_off.REASON_COMMAND - self._set_fan_speed(msg.cmd2, reason) - on_done(True, "Fan %s state updated to %s" % - (self.addr, self._fan_speed), msg.cmd2) - - elif msg.flags.type == Msg.Flags.Type.DIRECT_NAK: - LOG.error("FanLinc fan %s NAK error: %s, Message: %s", self.addr, - msg.nak_str(), msg) - on_done(False, "Fan %s state update failed. " + msg.nak_str(), - None) + LOG.debug("FanLinc fan %s ACK: %s", self.addr, msg) + + reason = reason if reason else on_off.REASON_COMMAND + self._set_fan_speed(msg.cmd2, reason) + on_done(True, "Fan %s state updated to %s" % + (self.addr, self._fan_speed), msg.cmd2) #----------------------------------------------------------------------- def handle_group_cmd(self, addr, msg): diff --git a/insteon_mqtt/device/IOLinc.py b/insteon_mqtt/device/IOLinc.py index 0254ae15..92e02ec0 100644 --- a/insteon_mqtt/device/IOLinc.py +++ b/insteon_mqtt/device/IOLinc.py @@ -838,24 +838,18 @@ def handle_ack(self, msg, on_done): completed. Signature is: on_done(success, msg, data) """ # This state is for the relay. - if msg.flags.type == Msg.Flags.Type.DIRECT_ACK: - LOG.debug("IOLinc %s ACK: %s", self.addr, msg) - on_done(True, "IOLinc command complete", None) + LOG.debug("IOLinc %s ACK: %s", self.addr, msg) + on_done(True, "IOLinc command complete", None) - # On command. 0x11: on - if msg.cmd1 == 0x11: - LOG.info("IOLinc %s relay ON", self.addr) - self._set_relay_is_on(True) - - # Off command. 0x13: off - elif msg.cmd1 == 0x13: - LOG.info("IOLinc %s relay OFF", self.addr) - self._set_relay_is_on(False) + # On command. 0x11: on + if msg.cmd1 == 0x11: + LOG.info("IOLinc %s relay ON", self.addr) + self._set_relay_is_on(True) - elif msg.flags.type == Msg.Flags.Type.DIRECT_NAK: - LOG.error("IOLinc %s NAK error: %s, Message: %s", self.addr, - msg.nak_str(), msg) - on_done(False, "IOLinc command failed. " + msg.nak_str(), None) + # Off command. 0x13: off + elif msg.cmd1 == 0x13: + LOG.info("IOLinc %s relay OFF", self.addr) + self._set_relay_is_on(False) #----------------------------------------------------------------------- def handle_group_cmd(self, addr, msg): diff --git a/insteon_mqtt/device/KeypadLinc.py b/insteon_mqtt/device/KeypadLinc.py index c57b88fd..30acd881 100644 --- a/insteon_mqtt/device/KeypadLinc.py +++ b/insteon_mqtt/device/KeypadLinc.py @@ -1150,10 +1150,7 @@ def handle_ack(self, msg, on_done, task=""): completed. Signature is: on_done(success, msg, data) task (str): The message to report. """ - if msg.flags.type == Msg.Flags.Type.DIRECT_ACK: - on_done(True, "%s updated" % task, None) - else: - on_done(False, "%s failed" % task, None) + on_done(True, "%s updated" % task, None) #----------------------------------------------------------------------- def handle_backlight_on(self, msg, on_done, is_on): @@ -1166,11 +1163,8 @@ def handle_backlight_on(self, msg, on_done, is_on): is_on (bool): True if the backlight is being turned on, False for off. """ - if msg.flags.type == Msg.Flags.Type.DIRECT_ACK: - on_done(True, "backlight set to %s" % is_on, None) - self._backlight = is_on - else: - on_done(False, "backlight set failed", None) + on_done(True, "backlight set to %s" % is_on, None) + self._backlight = is_on #----------------------------------------------------------------------- def handle_refresh(self, msg): @@ -1218,27 +1212,20 @@ def handle_button_led(self, msg, on_done, group, is_on, led_bits, """ # If this is the ACK we're expecting, update the internal state and # emit our signals. - if msg.flags.type == Msg.Flags.Type.DIRECT_ACK: - LOG.debug("KeypadLinc LED %s group %s ACK: %s", self.addr, group, - msg) - - # Update the LED bit for the updated group. - self._led_bits = led_bits - LOG.ui("KeypadLinc %s LED's changed to %s", self.addr, - "{:08b}".format(self._led_bits)) + LOG.debug("KeypadLinc LED %s group %s ACK: %s", self.addr, group, + msg) - # Change the level and emit the active signal. - self._set_level(group, 0xff if is_on else 0x00, reason=reason) + # Update the LED bit for the updated group. + self._led_bits = led_bits + LOG.ui("KeypadLinc %s LED's changed to %s", self.addr, + "{:08b}".format(self._led_bits)) - msg = "KeypadLinc %s LED group %s updated to %s" % \ - (self.addr, group, is_on) - on_done(True, msg, is_on) + # Change the level and emit the active signal. + self._set_level(group, 0xff if is_on else 0x00, reason=reason) - elif msg.flags.type == Msg.Flags.Type.DIRECT_NAK: - LOG.error("KeypadLinc LED %s NAK error: %s, Message: %s", - self.addr, msg.nak_str(), msg) - on_done(False, "KeypadLinc %s LED update failed. " + msg.nak_str(), - None) + msg = "KeypadLinc %s LED group %s updated to %s" % \ + (self.addr, group, is_on) + on_done(True, msg, is_on) #----------------------------------------------------------------------- def handle_load_attach(self, msg, on_done, is_attached): @@ -1252,20 +1239,15 @@ def handle_load_attach(self, msg, on_done, is_attached): """ # If this it the ACK we're expecting, update the internal state and # emit our signals. - if msg.flags.type == Msg.Flags.Type.DIRECT_ACK: - LOG.debug("KeypadLinc %s ACK: %s", self.addr, msg) - if is_attached: - self._load_group = 1 - else: - self._load_group = 9 - - LOG.ui("Keypadlinc %s, setting load to group %s", self.addr, - self._load_group) - on_done(True, "Load set to group: %s" % self._load_group, None) + LOG.debug("KeypadLinc %s ACK: %s", self.addr, msg) + if is_attached: + self._load_group = 1 + else: + self._load_group = 9 - elif msg.flags.type == Msg.Flags.Type.DIRECT_NAK: - LOG.error("KeypadLinc %s NAK error: %s", self.addr, msg) - on_done(False, "Changing the load group failed", None) + LOG.ui("Keypadlinc %s, setting load to group %s", self.addr, + self._load_group) + on_done(True, "Load set to group: %s" % self._load_group, None) #----------------------------------------------------------------------- def handle_refresh_led(self, msg): @@ -1458,17 +1440,12 @@ def handle_set_load(self, msg, on_done, reason=""): """ # If this is the ACK we're expecting, update the internal state and # emit our signals. - if msg.flags.type == Msg.Flags.Type.DIRECT_ACK: - LOG.debug("KeypadLinc %s ACK: %s", self.addr, msg) - - _is_on, mode = on_off.Mode.decode(msg.cmd1) - self._set_level(self._load_group, msg.cmd2, mode, reason) - on_done(True, "KeypadLinc state updated to %s" % self._level, - msg.cmd2) + LOG.debug("KeypadLinc %s ACK: %s", self.addr, msg) - elif msg.flags.type == Msg.Flags.Type.DIRECT_NAK: - LOG.error("KeypadLinc %s NAK error: %s", self.addr, msg) - on_done(False, "KeypadLinc state update failed", None) + _is_on, mode = on_off.Mode.decode(msg.cmd1) + self._set_level(self._load_group, msg.cmd2, mode, reason) + on_done(True, "KeypadLinc state updated to %s" % self._level, + msg.cmd2) #----------------------------------------------------------------------- def handle_scene(self, msg, on_done, reason=""): @@ -1490,19 +1467,14 @@ def handle_scene(self, msg, on_done, reason=""): # Call the callback. We don't change state here - the device will # send a regular broadcast message which will run handle_broadcast # which will then update the state. - if msg.flags.type == Msg.Flags.Type.DIRECT_ACK: - LOG.debug("KeypadLinc %s ACK: %s", self.addr, msg) + LOG.debug("KeypadLinc %s ACK: %s", self.addr, msg) - # Reason is device because we're simulating a button press. We - # can't really pass this around because we just get a broadcast - # message later from the device. So we set a temporary variable - # here and use it in handle_broadcast() to output the reason. - self.broadcast_reason = reason if reason else on_off.REASON_DEVICE - on_done(True, "Scene triggered", None) - - elif msg.flags.type == Msg.Flags.Type.DIRECT_NAK: - LOG.error("KeypadLinc %s NAK error: %s", self.addr, msg) - on_done(False, "Scene trigger failed failed", None) + # Reason is device because we're simulating a button press. We + # can't really pass this around because we just get a broadcast + # message later from the device. So we set a temporary variable + # here and use it in handle_broadcast() to output the reason. + self.broadcast_reason = reason if reason else on_off.REASON_DEVICE + on_done(True, "Scene triggered", None) #----------------------------------------------------------------------- def handle_increment(self, msg, on_done, delta, reason=""): @@ -1523,20 +1495,15 @@ def handle_increment(self, msg, on_done, delta, reason=""): """ # If this it the ACK we're expecting, update the internal state and # emit our signals. - if msg.flags.type == Msg.Flags.Type.DIRECT_ACK: - LOG.debug("KeypadLinc %s ACK: %s", self.addr, msg) - - # Add the delta and bound at [0, 255] - level = min(self._level + delta, 255) - level = max(level, 0) - self._set_level(self._load_group, level, reason=reason) + LOG.debug("KeypadLinc %s ACK: %s", self.addr, msg) - s = "KeypadLinc %s state updated to %s" % (self.addr, self._level) - on_done(True, s, msg.cmd2) + # Add the delta and bound at [0, 255] + level = min(self._level + delta, 255) + level = max(level, 0) + self._set_level(self._load_group, level, reason=reason) - elif msg.flags.type == Msg.Flags.Type.DIRECT_NAK: - LOG.error("KeypadLinc %s NAK error: %s", self.addr, msg) - on_done(False, "KeypadLinc %s state update failed", None) + s = "KeypadLinc %s state updated to %s" % (self.addr, self._level) + on_done(True, s, msg.cmd2) #----------------------------------------------------------------------- def handle_group_cmd(self, addr, msg): diff --git a/insteon_mqtt/device/Outlet.py b/insteon_mqtt/device/Outlet.py index 2e136f2e..c1dbc7a5 100644 --- a/insteon_mqtt/device/Outlet.py +++ b/insteon_mqtt/device/Outlet.py @@ -435,10 +435,7 @@ def handle_backlight(self, msg, on_done): on_done: Finished callback. This is called when the command has completed. Signature is: on_done(success, msg, data) """ - if msg.flags.type == Msg.Flags.Type.DIRECT_ACK: - on_done(True, "Backlight level updated", None) - else: - on_done(False, "Backlight level failed", None) + on_done(True, "Backlight level updated", None) #----------------------------------------------------------------------- def handle_broadcast(self, msg): @@ -563,18 +560,13 @@ def handle_ack(self, msg, on_done, reason=""): # If this it the ACK we're expecting, update the internal # state and emit our signals. - if msg.flags.type == Msg.Flags.Type.DIRECT_ACK: - LOG.debug("Outlet %s grp: %s ACK: %s", self.addr, group, msg) - - is_on, mode = on_off.Mode.decode(msg.cmd1) - reason = reason if reason else on_off.REASON_COMMAND - self._set_is_on(group, is_on, mode, reason) - on_done(True, "Outlet state updated to on=%s" % self._is_on, - self._is_on) + LOG.debug("Outlet %s grp: %s ACK: %s", self.addr, group, msg) - elif msg.flags.type == Msg.Flags.Type.DIRECT_NAK: - LOG.error("Outlet %s NAK error: %s", self.addr, msg) - on_done(False, "Outlet state update failed", None) + is_on, mode = on_off.Mode.decode(msg.cmd1) + reason = reason if reason else on_off.REASON_COMMAND + self._set_is_on(group, is_on, mode, reason) + on_done(True, "Outlet state updated to on=%s" % self._is_on, + self._is_on) #----------------------------------------------------------------------- def handle_scene(self, msg, on_done, reason=""): @@ -596,20 +588,14 @@ def handle_scene(self, msg, on_done, reason=""): # Call the callback. We don't change state here - the device will # send a regular broadcast message which will run handle_broadcast # which will then update the state. - if msg.flags.type == Msg.Flags.Type.DIRECT_ACK: - LOG.debug("Outlet %s ACK: %s", self.addr, msg) - - # Reason is device because we're simulating a button press. We - # can't really pass this around because we just get a broadcast - # message later from the device. So we set a temporary variable - # here and use it in handle_broadcast() to output the reason. - self.broadcast_reason = reason if reason else on_off.REASON_DEVICE - on_done(True, "Scene triggered", None) - - elif msg.flags.type == Msg.Flags.Type.DIRECT_NAK: - LOG.error("Outlet %s NAK error: %s", self.addr, msg) - self.broadcast_reason = None - on_done(False, "Scene trigger failed failed", None) + LOG.debug("Outlet %s ACK: %s", self.addr, msg) + + # Reason is device because we're simulating a button press. We + # can't really pass this around because we just get a broadcast + # message later from the device. So we set a temporary variable + # here and use it in handle_broadcast() to output the reason. + self.broadcast_reason = reason if reason else on_off.REASON_DEVICE + on_done(True, "Scene triggered", None) #----------------------------------------------------------------------- def handle_group_cmd(self, addr, msg): diff --git a/insteon_mqtt/device/Switch.py b/insteon_mqtt/device/Switch.py index 20d51fb0..23822f03 100644 --- a/insteon_mqtt/device/Switch.py +++ b/insteon_mqtt/device/Switch.py @@ -366,10 +366,7 @@ def handle_backlight(self, msg, on_done): on_done: Finished callback. This is called when the command has completed. Signature is: on_done(success, msg, data) """ - if msg.flags.type == Msg.Flags.Type.DIRECT_ACK: - on_done(True, "Backlight level updated", None) - else: - on_done(False, "Backlight level failed", None) + on_done(True, "Backlight level updated", None) #----------------------------------------------------------------------- def handle_broadcast(self, msg): @@ -477,20 +474,13 @@ def handle_ack(self, msg, on_done, reason=""): """ # If this it the ACK we're expecting, update the internal state and # emit our signals. - if msg.flags.type == Msg.Flags.Type.DIRECT_ACK: - LOG.debug("Switch %s ACK: %s", self.addr, msg) - - is_on, mode = on_off.Mode.decode(msg.cmd1) - reason = reason if reason else on_off.REASON_COMMAND - self._set_is_on(is_on, mode, reason) - on_done(True, "Switch state updated to on=%s" % self._is_on, - self._is_on) + LOG.debug("Switch %s ACK: %s", self.addr, msg) - elif msg.flags.type == Msg.Flags.Type.DIRECT_NAK: - LOG.error("Switch %s NAK error: %s, Message: %s", self.addr, - msg.nak_str(), msg) - on_done(False, "Switch state update failed. " + msg.nak_str(), - None) + is_on, mode = on_off.Mode.decode(msg.cmd1) + reason = reason if reason else on_off.REASON_COMMAND + self._set_is_on(is_on, mode, reason) + on_done(True, "Switch state updated to on=%s" % self._is_on, + self._is_on) #----------------------------------------------------------------------- def handle_scene(self, msg, on_done, reason=""): @@ -512,22 +502,14 @@ def handle_scene(self, msg, on_done, reason=""): # Call the callback. We don't change state here - the device will # send a regular broadcast message which will run handle_broadcast # which will then update the state. - if msg.flags.type == Msg.Flags.Type.DIRECT_ACK: - LOG.debug("Switch %s ACK: %s", self.addr, msg) - - # Reason is device because we're simulating a button press. We - # can't really pass this around because we just get a broadcast - # message later from the device. So we set a temporary variable - # here and use it in handle_broadcast() to output the reason. - self.broadcast_reason = reason if reason else on_off.REASON_DEVICE - on_done(True, "Scene triggered", None) - - elif msg.flags.type == Msg.Flags.Type.DIRECT_NAK: - LOG.error("Switch %s NAK error: %s, Message: %s", self.addr, - msg.nak_str(), msg) - self.broadcast_reason = None - on_done(False, "Scene trigger failed failed. " + msg.nak_str(), - None) + LOG.debug("Switch %s ACK: %s", self.addr, msg) + + # Reason is device because we're simulating a button press. We + # can't really pass this around because we just get a broadcast + # message later from the device. So we set a temporary variable + # here and use it in handle_broadcast() to output the reason. + self.broadcast_reason = reason if reason else on_off.REASON_DEVICE + on_done(True, "Scene triggered", None) #----------------------------------------------------------------------- def handle_group_cmd(self, addr, msg): diff --git a/insteon_mqtt/device/Thermostat.py b/insteon_mqtt/device/Thermostat.py index a31736f8..dbe79400 100644 --- a/insteon_mqtt/device/Thermostat.py +++ b/insteon_mqtt/device/Thermostat.py @@ -454,14 +454,8 @@ def handle_generic_ack(self, msg, on_done=None): """ on_done = util.make_callback(on_done) - if msg.flags.type == Msg.Flags.Type.DIRECT_NAK: - LOG.error("%s NAK: %s, Message: %s", self.db.addr, msg.nak_str(), - msg) - on_done(False, "Thermostat command NAK. " + msg.nak_str(), None) - - else: - LOG.debug("Thermostat %s generic ack recevied", self.addr) - on_done(True, "Thermostat generic ack recevied", None) + LOG.debug("Thermostat %s generic ack recevied", self.addr) + on_done(True, "Thermostat generic ack recevied", None) #----------------------------------------------------------------------- def handle_broadcast(self, msg): @@ -538,13 +532,7 @@ def handle_mode_command(self, msg, on_done=None): """ on_done = util.make_callback(on_done) - if msg.flags.type == Msg.Flags.Type.DIRECT_NAK: - LOG.error("%s mode command NAK: %s, Message: %s", self.db.addr, - msg.nak_str(), msg) - on_done(False, "Thermostat mode command NAK. " + msg.nak_str(), - None) - - elif msg.cmd1 == 0x6b: + if msg.cmd1 == 0x6b: self.signal_mode_change.emit(self, Thermostat.ModeCommands(msg.cmd2)) on_done(True, "Thermostat recevied mode command", None) @@ -586,13 +574,7 @@ def handle_fan_command(self, msg, on_done=None): """ on_done = util.make_callback(on_done) - if msg.flags.type == Msg.Flags.Type.DIRECT_NAK: - LOG.error("%s fan command NAK: %s, Message: %s", self.db.addr, - msg.nak_str(), msg) - on_done(False, "Thermostat fan command NAK. " + msg.nak_str(), - None) - - elif msg.cmd1 == 0x6b: + if msg.cmd1 == 0x6b: self.signal_fan_mode_change.emit(self, Thermostat.FanCommands(msg.cmd2)) on_done(True, "Thermostat recevied fan mode command", None) From 4163ac031407fae3bf25195dde5d4bf92c81bfd6 Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Mon, 14 Dec 2020 17:22:56 -0800 Subject: [PATCH 26/54] Fix Bug in Function Call --- insteon_mqtt/db/DeviceModifyManagerI1.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/insteon_mqtt/db/DeviceModifyManagerI1.py b/insteon_mqtt/db/DeviceModifyManagerI1.py index 7cdaa101..71a7ac37 100644 --- a/insteon_mqtt/db/DeviceModifyManagerI1.py +++ b/insteon_mqtt/db/DeviceModifyManagerI1.py @@ -165,7 +165,7 @@ def handle_lsb_write_response(self, msg, on_done): """ # Increment the delta 1 self.db.set_delta(self.db.delta + 1) - self.handle_lsb_response(self, msg, on_done) + self.handle_lsb_response(msg, on_done) #------------------------------------------------------------------- def write_lsb_byte(self, on_done): From 508cfc46c2977a326a6fc53194ad2c70405cc5f4 Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Mon, 14 Dec 2020 17:29:41 -0800 Subject: [PATCH 27/54] NAKs are No Longer Sent to Handle_Ack --- tests/device/test_IOLinc.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/device/test_IOLinc.py b/tests/device/test_IOLinc.py index d0d15fc1..a146b553 100644 --- a/tests/device/test_IOLinc.py +++ b/tests/device/test_IOLinc.py @@ -278,7 +278,6 @@ def test_handle_refresh_sensor(self, test_iolinc, cmd2, expected): @pytest.mark.parametrize("cmd1, type, expected", [ (0x11, IM.message.Flags.Type.DIRECT_ACK, True), (0X13, IM.message.Flags.Type.DIRECT_ACK, False), - (0X11, IM.message.Flags.Type.DIRECT_NAK, None), ]) def test_handle_ack(self, test_iolinc, cmd1, type, expected): with mock.patch.object(IM.Signal, 'emit'): @@ -288,11 +287,8 @@ def test_handle_ack(self, test_iolinc, cmd1, type, expected): msg = IM.message.InpStandard(from_addr, to_addr, flags, cmd1, 0x01) test_iolinc.handle_ack(msg, lambda success, msg, cmd: True) calls = IM.Signal.emit.call_args_list - if expected is not None: - assert calls[0][0][2] == expected - assert IM.Signal.emit.call_count == 1 - else: - assert IM.Signal.emit.call_count == 0 + assert calls[0][0][2] == expected + assert IM.Signal.emit.call_count == 1 @pytest.mark.parametrize("cmd1, entry_d1, mode, sensor, expected", [ (0x11, None, IM.device.IOLinc.Modes.LATCHING, False, None), From 3e4d3860a7873e62132ab9250bfb127509df1203 Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Mon, 14 Dec 2020 17:37:28 -0800 Subject: [PATCH 28/54] Linter Fixes --- insteon_mqtt/device/IOLinc.py | 2 +- insteon_mqtt/mqtt/Leak.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/insteon_mqtt/device/IOLinc.py b/insteon_mqtt/device/IOLinc.py index 92e02ec0..decb9968 100644 --- a/insteon_mqtt/device/IOLinc.py +++ b/insteon_mqtt/device/IOLinc.py @@ -878,7 +878,7 @@ def handle_group_cmd(self, addr, msg): # This reflects a change in the relay state. # Handle on/off commands codes. if on_off.Mode.is_valid(msg.cmd1): - is_on, mode = on_off.Mode.decode(msg.cmd1) + is_on = on_off.Mode.decode(msg.cmd1)[0] if self.mode == IOLinc.Modes.MOMENTARY_A: # In Momentary A the relay only turns on if the cmd matches # the responder link D1, else it always turns off. Even if diff --git a/insteon_mqtt/mqtt/Leak.py b/insteon_mqtt/mqtt/Leak.py index 01e291d4..ac93f99e 100644 --- a/insteon_mqtt/mqtt/Leak.py +++ b/insteon_mqtt/mqtt/Leak.py @@ -3,7 +3,6 @@ # MQTT leak sensor device # #=========================================================================== -import time from .. import log from .BatterySensor import BatterySensor from .MsgTemplate import MsgTemplate From 9a2598ac4d09778a9fe3ce0f7286535b2c5c308a Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Tue, 15 Dec 2020 09:24:49 -0800 Subject: [PATCH 29/54] Add StandardCmdNak Handler --- insteon_mqtt/handler/StandardCmdNAK.py | 72 ++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 insteon_mqtt/handler/StandardCmdNAK.py diff --git a/insteon_mqtt/handler/StandardCmdNAK.py b/insteon_mqtt/handler/StandardCmdNAK.py new file mode 100644 index 00000000..6d5cd6ac --- /dev/null +++ b/insteon_mqtt/handler/StandardCmdNAK.py @@ -0,0 +1,72 @@ +#=========================================================================== +# +# Insteon broadcast message handler +# +#=========================================================================== +from .. import log +from .. import message as Msg +from .StandardCmd import StandardCmd + +LOG = log.get_logger() + + +class StandardCmdNAK(StandardCmd): + """Insteon standard input mesage handler that passes on NAKs + + The Standard command handler will process and not pass on Naks to the + callback function. + + This is handler is identical in everyway, but will pass on a NAK so that + the callback can process it. This is rarely needed + """ + + #----------------------------------------------------------------------- + def msg_received(self, protocol, msg): + """See if we can handle the message. + + See if the message is the expected ACK of our output or the expected + InpStandard reply message. If we get a reply, pass it to the + callback. + + Args: + protocol (Protocol): The Insteon Protocol object + msg: Insteon message object that was read. + + Returns: + Msg.UNKNOWN if we can't handle this message. + Msg.CONTINUE if we handled the message and expect more. + Msg.FINISHED if we handled the message and are done. + """ + # Probably an echo back of our sent message. + if isinstance(msg, Msg.OutStandard): + # If the message is the echo back of our message, then continue + # waiting for a reply. + if msg.to_addr == self.addr and msg.cmd1 == self.cmd: + if not msg.is_ack: + LOG.error("%s NAK response", self.addr) + + LOG.debug("%s got msg ACK", self.addr) + return Msg.CONTINUE + + # Message didn't match the expected addr/cmd. + LOG.debug("%s handler unknown msg", self.addr) + return Msg.UNKNOWN + + # See if this is the standard message ack/nak we're expecting. + elif isinstance(msg, Msg.InpStandard): + # If this message matches our address and command, it's probably + # the ACK we're expecting. + if msg.from_addr == self.addr and msg.cmd1 == self.cmd: + # Run the callback it decides what to do with an ACK or NAK + self.callback(msg, on_done=self.on_done) + + # Indicate no more messages are expected. + return Msg.FINISHED + else: + LOG.info("Possible unexpected message from %s cmd %#04x but " + "expected %s cmd %#04x", msg.from_addr, msg.cmd1, + self.addr, self.cmd) + + return Msg.UNKNOWN + + #----------------------------------------------------------------------- From c2d9fcc6261bab7e2166f88dfeeccc826570b84a Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Tue, 15 Dec 2020 09:34:52 -0800 Subject: [PATCH 30/54] Only pass on ACKs; Improve StandardCmd Flow --- insteon_mqtt/handler/StandardCmd.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/insteon_mqtt/handler/StandardCmd.py b/insteon_mqtt/handler/StandardCmd.py index e9033b41..9f389dcf 100644 --- a/insteon_mqtt/handler/StandardCmd.py +++ b/insteon_mqtt/handler/StandardCmd.py @@ -101,15 +101,16 @@ def msg_received(self, protocol, msg): msg.nak_str(), None) return Msg.FINISHED - # Run the callback - self.callback(msg, on_done=self.on_done) - - # Indicate no more messages are expected. - return Msg.FINISHED - else: - LOG.info("Possible unexpected message from %s cmd %#04x but " - "expected %s cmd %#04x", msg.from_addr, msg.cmd1, - self.addr, self.cmd) + elif msg.flags.type == Msg.Flags.Type.DIRECT_ACK: + # Run the callback + self.callback(msg, on_done=self.on_done) + # Indicate no more messages are expected. + return Msg.FINISHED + + # Only make it here if this is a bad msg. + LOG.info("Possible unexpected message from %s cmd %#04x but " + "expected %s cmd %#04x", msg.from_addr, msg.cmd1, + self.addr, self.cmd) return Msg.UNKNOWN From a9e34b413c16547bff917ddb00032336beedbb9b Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Tue, 15 Dec 2020 09:40:04 -0800 Subject: [PATCH 31/54] Only display unexpected message if addr and cmd1 match A different no-handler found message will be displayed otherwise. --- insteon_mqtt/handler/StandardCmd.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/insteon_mqtt/handler/StandardCmd.py b/insteon_mqtt/handler/StandardCmd.py index 9f389dcf..5d6e4d31 100644 --- a/insteon_mqtt/handler/StandardCmd.py +++ b/insteon_mqtt/handler/StandardCmd.py @@ -107,10 +107,10 @@ def msg_received(self, protocol, msg): # Indicate no more messages are expected. return Msg.FINISHED - # Only make it here if this is a bad msg. - LOG.info("Possible unexpected message from %s cmd %#04x but " - "expected %s cmd %#04x", msg.from_addr, msg.cmd1, - self.addr, self.cmd) + # Only make it here if this is a bad msg. + LOG.info("Possible unexpected message from %s cmd %#04x but " + "expected %s cmd %#04x", msg.from_addr, msg.cmd1, + self.addr, self.cmd) return Msg.UNKNOWN From a9197eb7d87b141157de8a995b7e85aa31ff298b Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Tue, 15 Dec 2020 10:46:46 -0800 Subject: [PATCH 32/54] On Expire Increase Hops by 1 Maximum hops allowed is 3. --- insteon_mqtt/handler/Base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/insteon_mqtt/handler/Base.py b/insteon_mqtt/handler/Base.py index 13e0b32a..853e53df 100644 --- a/insteon_mqtt/handler/Base.py +++ b/insteon_mqtt/handler/Base.py @@ -117,7 +117,7 @@ def is_expired(self, protocol, t): # Increase the hop count if we can. if isinstance(self._msg, Msg.OutStandard): # also handles OutExtended - num_hops = max(3, self._msg.flags.max_hops) + num_hops = min(3, self._msg.flags.max_hops + 1) LOG.debug("Increasing max_hops to %d", num_hops) self._msg.flags.set_hops(num_hops) From 35bb0463401f2a5f0629a104d14ce20540c91a8c Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Tue, 15 Dec 2020 11:10:22 -0800 Subject: [PATCH 33/54] Add History --- HISTORY.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/HISTORY.md b/HISTORY.md index f49847b5..49fd1c33 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -44,6 +44,8 @@ unnecessary refresh requirements, particularly around pairing. ([PR 248][P248]) +- Minor fix to the calculation of hops on resent messages. ([PR 259][P259]) + ## [0.7.3] Fixing a number of small bugs in preparation for upcoming releases which @@ -452,3 +454,4 @@ will add new features. [P239]: https://github.com/TD22057/insteon-mqtt/pull/239 [P244]: https://github.com/TD22057/insteon-mqtt/pull/244 [P235]: https://github.com/TD22057/insteon-mqtt/pull/235 +[P259]: https://github.com/TD22057/insteon-mqtt/pull/259 From 2d661e8be247107c4b567a9e7e595a0d84572a16 Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Tue, 15 Dec 2020 11:53:36 -0800 Subject: [PATCH 34/54] Fix Pytest --- tests/test_Scenes.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/test_Scenes.py b/tests/test_Scenes.py index 0b99f411..1b264b1b 100644 --- a/tests/test_Scenes.py +++ b/tests/test_Scenes.py @@ -694,8 +694,13 @@ def link_data_to_pretty(self, is_controller, data): class MockProto: def __init__(self): self.msgs = [] + self.signal_msg_finished = MockSignal() def send(self, msg, handler, high_priority=False, after=None): self.msgs.append(msg) +class MockSignal: + def connect(self, *args, **kwargs): + pass + #=========================================================================== From 4d5944aae6ac53d739f9bce82950dbeb64d77f01 Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Tue, 15 Dec 2020 12:02:08 -0800 Subject: [PATCH 35/54] Upate History --- HISTORY.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/HISTORY.md b/HISTORY.md index 49fd1c33..52d7c4f1 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -40,6 +40,9 @@ ### Fixes +- Major fixes to a number of bugs in the Scenes management functions. + (thanks @tstabrawa)([PR 234][P234]) + - Database delta is updated on database writes. This eliminates a number of unnecessary refresh requirements, particularly around pairing. ([PR 248][P248]) @@ -455,3 +458,4 @@ will add new features. [P244]: https://github.com/TD22057/insteon-mqtt/pull/244 [P235]: https://github.com/TD22057/insteon-mqtt/pull/235 [P259]: https://github.com/TD22057/insteon-mqtt/pull/259 +[P234]: https://github.com/TD22057/insteon-mqtt/pull/234 From f5e3f5f5d93ab900940e3f88fd3a0f21917dddab Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Tue, 15 Dec 2020 12:30:54 -0800 Subject: [PATCH 36/54] Use Enum --- insteon_mqtt/message/InpStandard.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/insteon_mqtt/message/InpStandard.py b/insteon_mqtt/message/InpStandard.py index 8893534e..8810d657 100644 --- a/insteon_mqtt/message/InpStandard.py +++ b/insteon_mqtt/message/InpStandard.py @@ -113,11 +113,13 @@ def nak_str(self): """ ret = "" naks = { - 0xFF: "Senders ID not in responders db. Try running 'join' again.", - 0xFE: "Load sense detects no load", - 0xFD: "Checksum is incorrect", - 0xFC: "Pre NAK in case database search takes too long", - 0xFB: "Illegal value in command" + self.NakType.SENDER_NOT_IN_DB: + "Senders ID not in responders db. Try running 'join' again.", + self.NakType.NO_LOAD: "Load sense detects no load", + self.NakType.BAD_CHECKSUM: "Checksum is incorrect", + self.NakType.PRE_NAK: + "Pre NAK in case database search takes too long", + self.NakType.ILLEGAL_VALUE: "Illegal value in command" } if (self.flags.type == Flags.Type.DIRECT_NAK and self.cmd2 in naks.keys()): From 34868be62dfe33b618514594690f861b5d311a41 Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Tue, 15 Dec 2020 12:51:46 -0800 Subject: [PATCH 37/54] If PLM NAK don't show ACK log message also --- insteon_mqtt/handler/StandardCmd.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/insteon_mqtt/handler/StandardCmd.py b/insteon_mqtt/handler/StandardCmd.py index 5d6e4d31..630dc16b 100644 --- a/insteon_mqtt/handler/StandardCmd.py +++ b/insteon_mqtt/handler/StandardCmd.py @@ -73,8 +73,8 @@ def msg_received(self, protocol, msg): if msg.to_addr == self.addr and msg.cmd1 == self.cmd: if not msg.is_ack: LOG.error("%s NAK response", self.addr) - - LOG.debug("%s got msg ACK", self.addr) + else: + LOG.debug("%s got msg ACK", self.addr) return Msg.CONTINUE # Message didn't match the expected addr/cmd. From d6c8592e8c4acd3e2818ac896ffd492299049c96 Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Tue, 15 Dec 2020 12:59:23 -0800 Subject: [PATCH 38/54] Don't Treat PLM NAK as Error; Fix Duplicat Logging PLM NAK should be reported as a warning otherwise the command line UI will report it as a fail when it isn't. PLM NAK will likely just lead to a message timeout and retry. Also fix some other logging that was incorrectly reporting an ACK when a NAK had arrived. --- insteon_mqtt/handler/BroadcastCmdResponse.py | 6 +++--- insteon_mqtt/handler/DeviceDbGet.py | 2 +- insteon_mqtt/handler/DeviceRefresh.py | 7 +++---- insteon_mqtt/handler/ExtendedCmdResponse.py | 2 +- insteon_mqtt/handler/StandardCmd.py | 4 ++-- insteon_mqtt/handler/StandardCmdNAK.py | 6 +++--- 6 files changed, 13 insertions(+), 14 deletions(-) diff --git a/insteon_mqtt/handler/BroadcastCmdResponse.py b/insteon_mqtt/handler/BroadcastCmdResponse.py index f82805a1..865f2b5a 100644 --- a/insteon_mqtt/handler/BroadcastCmdResponse.py +++ b/insteon_mqtt/handler/BroadcastCmdResponse.py @@ -60,9 +60,9 @@ def msg_received(self, protocol, msg): # waiting for a reply. if msg.to_addr == self.addr and msg.cmd1 == self.cmd: if not msg.is_ack: - LOG.error("%s NAK response", self.addr) - - LOG.debug("%s got msg ACK", self.addr) + LOG.warning("%s PLM NAK response", self.addr) + else: + LOG.debug("%s got PLM ACK", self.addr) return Msg.CONTINUE # Message didn't match the expected addr/cmd. diff --git a/insteon_mqtt/handler/DeviceDbGet.py b/insteon_mqtt/handler/DeviceDbGet.py index f19ba494..65705b30 100644 --- a/insteon_mqtt/handler/DeviceDbGet.py +++ b/insteon_mqtt/handler/DeviceDbGet.py @@ -83,7 +83,7 @@ def msg_received(self, protocol, msg): if isinstance(msg, (Msg.OutExtended, Msg.OutStandard)): if msg.to_addr == self.db.addr and msg.cmd1 == 0x2f: if not msg.is_ack: - LOG.error("%s NAK response", self.db.addr) + LOG.warning("%s PLM NAK response", self.db.addr) return Msg.CONTINUE return Msg.UNKNOWN diff --git a/insteon_mqtt/handler/DeviceRefresh.py b/insteon_mqtt/handler/DeviceRefresh.py index 86464ec4..6cc3ec9f 100644 --- a/insteon_mqtt/handler/DeviceRefresh.py +++ b/insteon_mqtt/handler/DeviceRefresh.py @@ -69,12 +69,11 @@ def msg_received(self, protocol, msg): # Probably an echo back of our sent message. if isinstance(msg, Msg.OutStandard) and msg.to_addr == self.addr: if msg.is_ack: - LOG.debug("%s ACK response", self.addr) + LOG.debug("%s PLM ACK response", self.addr) return Msg.CONTINUE else: - LOG.error("%s NAK response", self.addr) - self.on_done(False, "NAK response", None) - return Msg.FINISHED + LOG.warning("%s PLM NAK response", self.addr) + return Msg.CONTINUE # See if this is the standard message ack/nak we're expecting. elif isinstance(msg, Msg.InpStandard) and msg.from_addr == self.addr: diff --git a/insteon_mqtt/handler/ExtendedCmdResponse.py b/insteon_mqtt/handler/ExtendedCmdResponse.py index fb26475e..9b7c50f3 100644 --- a/insteon_mqtt/handler/ExtendedCmdResponse.py +++ b/insteon_mqtt/handler/ExtendedCmdResponse.py @@ -71,7 +71,7 @@ def msg_received(self, protocol, msg): if isinstance(msg, (Msg.OutExtended, Msg.OutStandard)): if msg.to_addr == self.addr and msg.cmd1 == self.cmd: if not msg.is_ack: - LOG.error("%s NAK response", self.addr) + LOG.warning("%s PLM NAK response", self.addr) return Msg.CONTINUE return Msg.UNKNOWN diff --git a/insteon_mqtt/handler/StandardCmd.py b/insteon_mqtt/handler/StandardCmd.py index 630dc16b..057a9f5b 100644 --- a/insteon_mqtt/handler/StandardCmd.py +++ b/insteon_mqtt/handler/StandardCmd.py @@ -72,9 +72,9 @@ def msg_received(self, protocol, msg): # waiting for a reply. if msg.to_addr == self.addr and msg.cmd1 == self.cmd: if not msg.is_ack: - LOG.error("%s NAK response", self.addr) + LOG.warning("%s PLM NAK response", self.addr) else: - LOG.debug("%s got msg ACK", self.addr) + LOG.debug("%s got PLM ACK", self.addr) return Msg.CONTINUE # Message didn't match the expected addr/cmd. diff --git a/insteon_mqtt/handler/StandardCmdNAK.py b/insteon_mqtt/handler/StandardCmdNAK.py index 6d5cd6ac..0fecf178 100644 --- a/insteon_mqtt/handler/StandardCmdNAK.py +++ b/insteon_mqtt/handler/StandardCmdNAK.py @@ -43,9 +43,9 @@ def msg_received(self, protocol, msg): # waiting for a reply. if msg.to_addr == self.addr and msg.cmd1 == self.cmd: if not msg.is_ack: - LOG.error("%s NAK response", self.addr) - - LOG.debug("%s got msg ACK", self.addr) + LOG.warning("%s PLM NAK response", self.addr) + else: + LOG.debug("%s got PLM ACK", self.addr) return Msg.CONTINUE # Message didn't match the expected addr/cmd. From b9e719d4b9d8aa9159473bf42f648a42fc87ccae Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Tue, 15 Dec 2020 13:15:46 -0800 Subject: [PATCH 39/54] Fix Comments --- insteon_mqtt/handler/StandardCmd.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/insteon_mqtt/handler/StandardCmd.py b/insteon_mqtt/handler/StandardCmd.py index 057a9f5b..9d212e36 100644 --- a/insteon_mqtt/handler/StandardCmd.py +++ b/insteon_mqtt/handler/StandardCmd.py @@ -24,7 +24,7 @@ class StandardCmd(Base): When we get the InptStandard message we expect to see, it will be passed to the callback set in the constructor which is usually a method on the - device to handle the result (or the ACK that the command went through). + device to handle the ACK (or the ACK that the command went through). """ def __init__(self, msg, callback, on_done=None, num_retry=3): """Constructor @@ -34,7 +34,7 @@ def __init__(self, msg, callback, on_done=None, num_retry=3): reply must match the address and msg.cmd1 field to be processed by this handler. callback: The message handler callback. This is called when a - matching message is read. Calling signature: + matching ACK is read. Calling signature: callback( msg, on_done ) on_done: The finished callback. Calling signature: on_done( bool success, str message, data ) From 4d424d4ccb8fed7e0a89980a9f6a9ccb9c80d471 Mon Sep 17 00:00:00 2001 From: Kevin Robert Keegan Date: Tue, 15 Dec 2020 14:06:29 -0800 Subject: [PATCH 40/54] Update insteon_mqtt/handler/StandardCmd.py Co-authored-by: tstabrawa <59430211+tstabrawa@users.noreply.github.com> --- insteon_mqtt/handler/StandardCmd.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/insteon_mqtt/handler/StandardCmd.py b/insteon_mqtt/handler/StandardCmd.py index 9d212e36..776a8fda 100644 --- a/insteon_mqtt/handler/StandardCmd.py +++ b/insteon_mqtt/handler/StandardCmd.py @@ -24,7 +24,7 @@ class StandardCmd(Base): When we get the InptStandard message we expect to see, it will be passed to the callback set in the constructor which is usually a method on the - device to handle the ACK (or the ACK that the command went through). + device to handle the ACK. """ def __init__(self, msg, callback, on_done=None, num_retry=3): """Constructor From 68c928a0f9b7b58307b85129f1db4a4dd7b8ac6f Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Tue, 15 Dec 2020 17:34:12 -0800 Subject: [PATCH 41/54] Make Modem Write in Log Prettier --- insteon_mqtt/Protocol.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/insteon_mqtt/Protocol.py b/insteon_mqtt/Protocol.py index 50857c2d..c49b7dab 100644 --- a/insteon_mqtt/Protocol.py +++ b/insteon_mqtt/Protocol.py @@ -523,7 +523,7 @@ def _send_next_msg(self): msg_bytes = out.msg.to_bytes() LOG.info("Write message to modem: %s", out.msg) - LOG.debug("Write bytes to modem: %s", msg_bytes) + LOG.debug("Write bytes to modem: %s", msg_bytes.hex()) # Write the message to the PLM modem. The message will only be sent # when the current time is after the next write time as tracked by From ee1535cd1ef83b699193e93e1482585e00e8ec4e Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Wed, 16 Dec 2020 16:22:50 -0800 Subject: [PATCH 42/54] Add Pytest --- tests/message/test_Timed.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/message/test_Timed.py b/tests/message/test_Timed.py index e43b1dbf..35b5e752 100644 --- a/tests/message/test_Timed.py +++ b/tests/message/test_Timed.py @@ -5,6 +5,7 @@ #=========================================================================== from unittest import mock import insteon_mqtt as IM +import insteon_mqtt.message as Msg class Test_Timed: @@ -20,8 +21,8 @@ def test_active(self): #----------------------------------------------------------------------- def test_send(self): t0 = 1000 - msg = mock.Mock() - msg.to_bytes.return_value = "123" + addr = IM.Address(0x48, 0x3d, 0x46) + msg = Msg.OutStandard.direct(addr, 0x11, 0x25) handler = mock.Mock() obj = IM.message.Timed(msg, handler, False, t0) @@ -31,7 +32,7 @@ def test_send(self): obj.send(protocol) last = link.mock_calls[-1] - assert last == mock.call.write("123", 0) + assert last == mock.call.write(msg.to_bytes(), 0) #----------------------------------------------------------------------- From 8114c58b90b0fa2130612554711d3416379afce6 Mon Sep 17 00:00:00 2001 From: Kevin Robert Keegan Date: Wed, 16 Dec 2020 18:46:55 -0800 Subject: [PATCH 43/54] Attempt at PR Tests Well, let's see what happens here. --- .github/workflows/python-app.yml | 36 ++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 .github/workflows/python-app.yml diff --git a/.github/workflows/python-app.yml b/.github/workflows/python-app.yml new file mode 100644 index 00000000..3fcc2927 --- /dev/null +++ b/.github/workflows/python-app.yml @@ -0,0 +1,36 @@ +# This workflow will install Python dependencies, run tests and lint with a single version of Python +# For more information see: https://help.github.com/actions/language-and-framework-guides/using-python-with-github-actions + +name: Insteon-MQTT + +on: + pull_request: + branches: + - dev + +jobs: + build: + + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v2 + - name: Set up Python 3.8 + uses: actions/setup-python@v2 + with: + python-version: 3.8 + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install flake8 pytest + if [ -f requirements.txt ]; then pip install -r requirements.txt; fi + if [ -f requirements-test.txt ]; then pip install -r requirements-test.txt; fi + - name: Lint with flake8 + run: | + # stop the build if there are Python syntax errors or undefined names + flake8 insteon_mqtt --count --select=E9,F63,F7,F82 --show-source --statistics + # exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide + flake8 insteon_mqtt --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics + - name: Test with pytest + run: | + pytest tests From 1b988993ecda790de5fac7fc421b16bc7b80ae41 Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Wed, 16 Dec 2020 18:57:10 -0800 Subject: [PATCH 44/54] Fix Typo --- insteon_mqtt/cmd_line/device.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/insteon_mqtt/cmd_line/device.py b/insteon_mqtt/cmd_line/device.py index 938a9fa8..b8484bbe 100644 --- a/insteon_mqtt/cmd_line/device.py +++ b/insteon_mqtt/cmd_line/device.py @@ -206,7 +206,7 @@ def pair(args, config): if reply["status"]: print("Pairing may fail if the modem db is out of date. Try running") print("the following and then re-try the pair command.") - print(" insteont-mqtt config.py refresh modem") + print(" insteon-mqtt config.py refresh modem") return reply["status"] From 68cfc6ca8f3c795d44edd3978d1cdb3077fc6978 Mon Sep 17 00:00:00 2001 From: tstabrawa <59430211+tstabrawa@users.noreply.github.com> Date: Thu, 17 Dec 2020 00:23:25 -0600 Subject: [PATCH 45/54] IOLinc config.yaml fixes Home Assistant expects upper-case on/off payloads. Also, update HA MQTT examples for recent IOLinc changes. --- config.yaml | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/config.yaml b/config.yaml index 149e43db..82cfcf93 100644 --- a/config.yaml +++ b/config.yaml @@ -851,8 +851,13 @@ mqtt: # In Home Assistant use MQTT switch with a configuration like: # switch: # - platform: mqtt - # state_topic: 'insteon/aa.bb.cc/state' - # command_topic: 'insteon/aa.bb.cc/scene' + # state_topic: 'insteon/aa.bb.cc/relay' + # command_topic: 'insteon/aa.bb.cc/set' + # + # To monitor the sensor, use an MQTT binary_sensor with a configuration like: + # binary_sensor: + # - platform: mqtt + # state_topic: 'insteon/aa.bb.cc/sensor' io_linc: # Output state change topic and template. This message is sent whenever # the device sensor or device relay state changes. Available variables for @@ -864,7 +869,7 @@ mqtt: # sensor_on_str = 'off'/'on' # relay_on_str = 'off'/'on' state_topic: 'insteon/{{address}}/state' - state_payload: '{ "sensor" : "{{sensor_on_str.lower()}}", "relay" : {{relay_on_str.lower()}} }' + state_payload: '{ "sensor" : "{{sensor_on_str.upper()}}", "relay" : {{relay_on_str.upper()}} }' # Output relay state change topic and template. This message is sent # whenever the device relay state changes. Available variables for @@ -874,7 +879,7 @@ mqtt: # relay_on = 0/1 # relay_on_str = 'off'/'on' relay_state_topic: 'insteon/{{address}}/relay' - relay_state_payload: '{{relay_on_str.lower()}}' + relay_state_payload: '{{relay_on_str.upper()}}' # Output sensor state change topic and template. This message is sent # whenever the device sensor state changes. Available variables for @@ -884,7 +889,7 @@ mqtt: # sensor_on = 0/1 # sensor_on = 'off'/'on' sensor_state_topic: 'insteon/{{address}}/sensor' - sensor_state_payload: '{{sensor_on_str.lower()}}' + sensor_state_payload: '{{sensor_on_str.upper()}}' # Input on/off command. This forces the relay on/off and ignores the # momentary-A,B,C setting. Use this to force the relay to respond. From 7ad4d7c459ba83e6c4c763fa4747ccc1924490da Mon Sep 17 00:00:00 2001 From: Kevin Robert Keegan Date: Thu, 17 Dec 2020 15:26:49 -0800 Subject: [PATCH 46/54] Annotate flake8 messages --- .github/workflows/python-app.yml | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/.github/workflows/python-app.yml b/.github/workflows/python-app.yml index 3fcc2927..990ed12e 100644 --- a/.github/workflows/python-app.yml +++ b/.github/workflows/python-app.yml @@ -1,7 +1,7 @@ # This workflow will install Python dependencies, run tests and lint with a single version of Python # For more information see: https://help.github.com/actions/language-and-framework-guides/using-python-with-github-actions -name: Insteon-MQTT +name: Linting and Pytest on: pull_request: @@ -25,12 +25,17 @@ jobs: pip install flake8 pytest if [ -f requirements.txt ]; then pip install -r requirements.txt; fi if [ -f requirements-test.txt ]; then pip install -r requirements-test.txt; fi - - name: Lint with flake8 + - name: Lint with flake8 exit on errors run: | # stop the build if there are Python syntax errors or undefined names flake8 insteon_mqtt --count --select=E9,F63,F7,F82 --show-source --statistics - # exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide - flake8 insteon_mqtt --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics + - name: Lint with flake8 warnings annotation + uses: TrueBrain/actions-flake8@v1.4.1 + with: + path: insteon_mqtt + ignore: E265, E203, E123, E722, E127, E131, E731, W504 + max_line_length: 79 + only_warn: 1 - name: Test with pytest run: | pytest tests From 8c4257ec79bee678c6884aa3cddb32daf602f13b Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Thu, 17 Dec 2020 15:27:21 -0800 Subject: [PATCH 47/54] Fix Lint Warnings --- insteon_mqtt/cmd_line/device.py | 2 + insteon_mqtt/db/Device.py | 3 +- insteon_mqtt/db/ModemEntry.py | 3 +- insteon_mqtt/device/Dimmer.py | 8 +-- insteon_mqtt/device/EZIO4O.py | 86 +++++++++++++++++-------------- insteon_mqtt/device/IOLinc.py | 7 +-- insteon_mqtt/device/KeypadLinc.py | 4 +- insteon_mqtt/mqtt/EZIO4O.py | 32 ++++++------ insteon_mqtt/mqtt/Leak.py | 1 - insteon_mqtt/network/Mqtt.py | 3 +- insteon_mqtt/network/poll.py | 7 +-- insteon_mqtt/network/select.py | 7 +-- insteon_mqtt/util.py | 3 +- 13 files changed, 90 insertions(+), 76 deletions(-) diff --git a/insteon_mqtt/cmd_line/device.py b/insteon_mqtt/cmd_line/device.py index b8484bbe..ecd1a2bc 100644 --- a/insteon_mqtt/cmd_line/device.py +++ b/insteon_mqtt/cmd_line/device.py @@ -315,6 +315,7 @@ def import_scenes(args, config): reply = util.send(config, topic, payload, args.quiet) return reply["status"] + #=========================================================================== def awake(args, config): topic = "%s/%s" % (args.topic, args.address) @@ -325,6 +326,7 @@ def awake(args, config): reply = util.send(config, topic, payload, args.quiet) return reply["status"] + #=========================================================================== def get_battery_voltage(args, config): topic = "%s/%s" % (args.topic, args.address) diff --git a/insteon_mqtt/db/Device.py b/insteon_mqtt/db/Device.py index 686d7a52..e8b868a4 100644 --- a/insteon_mqtt/db/Device.py +++ b/insteon_mqtt/db/Device.py @@ -786,7 +786,8 @@ def _add_using_new(self, addr, group, is_controller, data, LOG.info("Device %s appending new record at mem %#06x", self.addr, self.last.mem_loc) - seq = CommandSeq(self.device, "Device database update complete", on_done) + seq = CommandSeq(self.device, "Device database update complete", + on_done) # Shift the current last record down 8 bytes. Make a copy - we'll # only update our member var if the write works. diff --git a/insteon_mqtt/db/ModemEntry.py b/insteon_mqtt/db/ModemEntry.py index 04bf4762..8b19db05 100644 --- a/insteon_mqtt/db/ModemEntry.py +++ b/insteon_mqtt/db/ModemEntry.py @@ -121,7 +121,8 @@ def __lt__(self, rhs): #----------------------------------------------------------------------- def __str__(self): return ("ID: %-25s grp: %3s type: %s data: %#04x %#04x %#04x" % - (self.label[:25], self.group, util.ctrl_str(self.is_controller), + (self.label[:25], self.group, + util.ctrl_str(self.is_controller), self.data[0], self.data[1], self.data[2])) #----------------------------------------------------------------------- diff --git a/insteon_mqtt/device/Dimmer.py b/insteon_mqtt/device/Dimmer.py index 4507a3d2..0b8008dd 100644 --- a/insteon_mqtt/device/Dimmer.py +++ b/insteon_mqtt/device/Dimmer.py @@ -46,7 +46,7 @@ class Dimmer(Base): def __init__(self, protocol, modem, address, name=None): """Constructor - + Args: protocol (Protocol): The Protocol object used to communicate with the Insteon network. This is needed to allow the @@ -525,8 +525,8 @@ def set_on_level(self, level, on_done=None): def set_ramp_rate(self, rate, on_done=None): """Set the device default ramp rate. - This changes the dimmer default ramp rate of how quickly it will - turn on or off. This rate can be between 0.1 seconds and up to 9 + This changes the dimmer default ramp rate of how quickly it will + turn on or off. This rate can be between 0.1 seconds and up to 9 minutes. Args: @@ -536,7 +536,7 @@ def set_ramp_rate(self, rate, on_done=None): """ LOG.info("Dimmer %s setting ramp rate to %s", self.label, rate) - data_3 = 0x1c #the default ramp rate is .5 + data_3 = 0x1c # the default ramp rate is .5 for ramp_key, ramp_value in self.ramp_pretty.items(): if rate >= ramp_value: data_3 = ramp_key diff --git a/insteon_mqtt/device/EZIO4O.py b/insteon_mqtt/device/EZIO4O.py index 0abef7b7..28b0d70b 100644 --- a/insteon_mqtt/device/EZIO4O.py +++ b/insteon_mqtt/device/EZIO4O.py @@ -19,7 +19,8 @@ # EZIOxx Flags settings definition EZIO4xx_flags = { "analog-input": { - "options": {"none": 0b00000000, "command": 0b00000001, "interval": 0b00000011}, + "options": {"none": 0b00000000, "command": 0b00000001, + "interval": 0b00000011}, "mask": 0b00000011, "default": "none", }, @@ -207,9 +208,8 @@ def refresh(self, force=False, on_done=None): seq.run() #----------------------------------------------------------------------- - def on( - self, group=0x01, level=None, mode=on_off.Mode.NORMAL, reason="", on_done=None - ): + def on(self, group=0x01, level=None, mode=on_off.Mode.NORMAL, reason="", + on_done=None): """Turn the device on. NOTE: This does NOT simulate a button press on the device - it just @@ -222,8 +222,8 @@ def on( the state changed signals. Args: - group (int): The group to send the command to. Group 1 to 4 matching - output 1 to 4. + group (int): The group to send the command to. Group 1 to 4 + matching output 1 to 4. level (int): If non zero, turn the device on. Should be in the range 0 to 255. Only dimmers use the intermediate values, all other devices look at level=0 or level>0. @@ -239,7 +239,8 @@ def on( assert level >= 0 and level <= 0xFF assert isinstance(mode, on_off.Mode) - # Use a standard message to send "output on" (0x45) command for the output + # Use a standard message to send "output on" (0x45) command for the + # output msg = Msg.OutStandard.direct(self.addr, 0x45, group - 1) # Use the standard command handler which will notify us when @@ -254,7 +255,8 @@ def on( self.send(msg, msg_handler) #----------------------------------------------------------------------- - def off(self, group=0x01, mode=on_off.Mode.NORMAL, reason="", on_done=None): + def off(self, group=0x01, mode=on_off.Mode.NORMAL, reason="", + on_done=None): """Turn the device off. NOTE: This does NOT simulate a button press on the device - it just @@ -267,8 +269,8 @@ def off(self, group=0x01, mode=on_off.Mode.NORMAL, reason="", on_done=None): the state changed signals. Args: - group (int): The group to send the command to. Group 1 to 4 matching - output 1 to 4. + group (int): The group to send the command to. Group 1 to 4 + matching output 1 to 4. 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 @@ -280,7 +282,8 @@ def off(self, group=0x01, mode=on_off.Mode.NORMAL, reason="", on_done=None): assert 1 <= group <= 4 assert isinstance(mode, on_off.Mode) - # Use a standard message to send "output off" (0x46) command for the output + # Use a standard message to send "output off" (0x46) command for the + # output msg = Msg.OutStandard.direct(self.addr, 0x46, group - 1) # Use the standard command handler which will notify us when the @@ -295,7 +298,8 @@ def off(self, group=0x01, mode=on_off.Mode.NORMAL, reason="", on_done=None): self.send(msg, msg_handler) #----------------------------------------------------------------------- - def set(self, level, group=0x01, mode=on_off.Mode.NORMAL, reason="", on_done=None): + def set(self, level, group=0x01, mode=on_off.Mode.NORMAL, reason="", + on_done=None): """Turn the device on or off. Level zero will be off. NOTE: This does NOT simulate a button press on the device - it just @@ -311,8 +315,8 @@ def set(self, level, group=0x01, mode=on_off.Mode.NORMAL, reason="", on_done=Non level (int): If non zero, turn the device on. Should be in the range 0 to 255. Only dimmers use the intermediate values, all other devices look at level=0 or level>0. - group (int): The group to send the command to. Group 1 to 4 matching - output 1 to 4. + group (int): The group to send the command to. Group 1 to 4 + matching output 1 to 4. mode (on_off.Mode): The type of command to send (normal, fast, etc). on_done: Finished callback. This is called when the command has completed. Signature is: on_done(success, msg, data) @@ -441,9 +445,11 @@ def link_data_to_pretty(self, is_controller, data): list[3]: list, containing a dict of the human readable values """ if is_controller: - ret = [{"data_1": data[0]}, {"data_2": data[1]}, {"group": data[2]}] + ret = [{"data_1": data[0]}, {"data_2": data[1]}, + {"group": data[2]}] else: - ret = [{"data_1": data[0]}, {"data_2": data[1]}, {"group": data[2] + 1}] + ret = [{"data_1": data[0]}, {"data_2": data[1]}, + {"group": data[2] + 1}] return ret #----------------------------------------------------------------------- @@ -516,10 +522,12 @@ def set_flags(self, on_done, **kwargs): LOG.info("EZIO4O %s cmd: set flags", self.label) # TODO initialize flags on first run - # Initialise flag value by reading the device Configuration Port settings + # Initialise flag value by reading the device Configuration Port + # settings if self._flag_value is None: LOG.info( - "EZIO4O %s cmd: flags not initialized - run get-flags first", self.label + "EZIO4O %s cmd: flags not initialized - run get-flags first", + self.label ) return @@ -557,7 +565,8 @@ def set_flags(self, on_done, **kwargs): % (option, field, EZIO4xx_flags[field]["options"].keys()) ) - # Use a standard message to send "write configuration to port" (0x4D) command + # Use a standard message to send "write configuration to port" (0x4D) + # command msg = Msg.OutStandard.direct(self.addr, 0x4D, new_flag_value) # Use the standard command handler which will notify us when the @@ -585,7 +594,8 @@ def get_flags(self, on_done=None): # Check the input flags to make sure only ones we can understand were # passed in. - # Use a standard message to send "read configuration to port" (0x4E) command + # Use a standard message to send "read configuration to port" (0x4E) + # command msg = Msg.OutStandard.direct(self.addr, 0x4E, 0x00) # Use the standard command handler which will notify us when the @@ -600,9 +610,9 @@ def get_flags(self, on_done=None): def handle_flags(self, msg, on_done): """Callback for flags settings commanded messages. - This callback is run when we get a reply back from set or read flags commands. - If the command was ACK'ed, we know it worked so we'll update the internal - state of flags. + This callback is run when we get a reply back from set or read flags + commands. If the command was ACK'ed, we know it worked so we'll update + the internal state of flags. Args: msg (message.InpStandard): The reply message from the device. @@ -621,12 +631,14 @@ def handle_flags(self, msg, on_done): LOG.debug("EZIO4O %s Flag ACK: %s", self.label, msg) bits = msg.cmd2 self._flag_value = bits - LOG.ui("EZIO4O %s operating flags: %s", self.label, "{:08b}".format(bits)) + LOG.ui("EZIO4O %s operating flags: %s", self.label, + "{:08b}".format(bits)) for field in EZIO4xx_flags: flag_bits = bits & EZIO4xx_flags[field]["mask"] option = "unknown" - for flag_option, option_bits in EZIO4xx_flags[field]["options"].items(): + flags_opts = EZIO4xx_flags[field]["options"].items() + for flag_option, option_bits in flags_opts: if flag_bits == option_bits: option = flag_option LOG.ui("%s : %s", field, option) @@ -696,10 +708,7 @@ def handle_refresh(self, msg): if 0x00 <= msg.cmd2 <= 0x0F: for i in range(4): - if util.bit_get(msg.cmd2, i): - is_on = True - else: - is_on = False + is_on = bool(util.bit_get(msg.cmd2, i)) # State change for output if is_on != self._is_on[i]: @@ -734,7 +743,8 @@ def handle_ack(self, msg, on_done, reason=""): # us which output it was so we have to track it here. See __init__ # code comments for more info. if not self._which_output: - LOG.error("EZIO4O %s ACK error. No output ID's were saved", self.label) + LOG.error("EZIO4O %s ACK error. No output ID's were saved", + self.label) on_done(False, "EZIO4O update failed - no ID's saved", None) return @@ -746,17 +756,13 @@ def handle_ack(self, msg, on_done, reason=""): LOG.debug("EZIO4O %s ACK: %s", self.label, msg) for i in range(4): - if util.bit_get(msg.cmd2, i): - is_on = True - else: - is_on = False + is_on = bool(util.bit_get(msg.cmd2, i)) # State change for the output and all outputs with state change if is_on != self._is_on[i] or i == group - 1: self._set_is_on(i + 1, is_on, reason=on_off.REASON_REFRESH) - on_done( - True, "EZIO4O state %s updated to: %s" % (i + 1, is_on), None - ) + on_done(True, "EZIO4O state %s updated to: %s" % + (i + 1, is_on), None) elif msg.flags.type == Msg.Flags.Type.DIRECT_NAK: LOG.error("EZIO4O %s NAK error: %s", self.label, msg) @@ -818,7 +824,8 @@ def handle_group_cmd(self, addr, msg): entry = self.db.find(addr, msg.group, is_controller=False) if not entry: LOG.error( - "EZIO4O %s has no group %s entry from %s", self.label, msg.group, addr + "EZIO4O %s has no group %s entry from %s", + self.label, msg.group, addr ) return @@ -831,7 +838,8 @@ def handle_group_cmd(self, addr, msg): self._set_is_on(localGroup, is_on, mode, on_off.REASON_SCENE) else: - LOG.warning("EZIO4O %s unknown group cmd %#04x", self.label, msg.cmd1) + LOG.warning("EZIO4O %s unknown group cmd %#04x", self.label, + msg.cmd1) #----------------------------------------------------------------------- def _set_is_on(self, group, is_on, mode=on_off.Mode.NORMAL, reason=""): diff --git a/insteon_mqtt/device/IOLinc.py b/insteon_mqtt/device/IOLinc.py index 0254ae15..e15b230f 100644 --- a/insteon_mqtt/device/IOLinc.py +++ b/insteon_mqtt/device/IOLinc.py @@ -884,15 +884,12 @@ def handle_group_cmd(self, addr, msg): # This reflects a change in the relay state. # Handle on/off commands codes. if on_off.Mode.is_valid(msg.cmd1): - is_on, mode = on_off.Mode.decode(msg.cmd1) + is_on = on_off.Mode.decode(msg.cmd1)[0] if self.mode == IOLinc.Modes.MOMENTARY_A: # In Momentary A the relay only turns on if the cmd matches # the responder link D1, else it always turns off. Even if # the momentary time has not elapsed. - if is_on == bool(entry.data[0]): - is_on = True - else: - is_on = False + is_on = is_on == bool(entry.data[0]) elif self.mode == IOLinc.Modes.MOMENTARY_B: # In Momentary B, either On or Off will turn on the Relay is_on = True diff --git a/insteon_mqtt/device/KeypadLinc.py b/insteon_mqtt/device/KeypadLinc.py index 565c7d74..3bacb1de 100644 --- a/insteon_mqtt/device/KeypadLinc.py +++ b/insteon_mqtt/device/KeypadLinc.py @@ -891,7 +891,7 @@ def set_ramp_rate(self, rate, on_done=None): LOG.info("Dimmer %s setting ramp rate to %s", self.label, rate) - data_3 = 0x1c #the default ramp rate is .5 + data_3 = 0x1c # the default ramp rate is .5 for ramp_key, ramp_value in Dimmer.ramp_pretty.items(): if rate >= ramp_value: data_3 = ramp_key @@ -971,7 +971,7 @@ def set_flags(self, on_done, **kwargs): seq.add(self.set_on_level, on_level) if FLAG_RAMP_RATE in kwargs: - rate = util.input_float(kwargs,FLAG_RAMP_RATE) + rate = util.input_float(kwargs, FLAG_RAMP_RATE) seq.add(self.set_ramp_rate, rate) if FLAG_FOLLOW_MASK in kwargs: diff --git a/insteon_mqtt/mqtt/EZIO4O.py b/insteon_mqtt/mqtt/EZIO4O.py index 69f0c6b7..036bc829 100644 --- a/insteon_mqtt/mqtt/EZIO4O.py +++ b/insteon_mqtt/mqtt/EZIO4O.py @@ -20,8 +20,8 @@ class EZIO4O: allow input MQTT messages to change the state of the device. EZIO4O will report their state (state/output) and can be commanded to turn - on and off (set/output topic) or trigger a device scene with which the modem - is a responder (scene/output topic). + on and off (set/output topic) or trigger a device scene with which the + modem is a responder (scene/output topic). """ def __init__(self, mqtt, device): @@ -36,7 +36,8 @@ def __init__(self, mqtt, device): # Output state change reporting template. self.msg_state = MsgTemplate( - topic="insteon/{{address}}/state/{{button}}", payload="{{on_str.lower()}}" + topic="insteon/{{address}}/state/{{button}}", + payload="{{on_str.lower()}}" ) # Input on/off command template. @@ -58,7 +59,7 @@ def load_config(self, config, qos=None): """Load values from a configuration data object. Args: - config (dict: The configuration dictionary to load from. The object + config (dict: The configuration dictionary to load from. The object config is stored in config['ezio4o']. qos (int): The default quality of service level to use. """ @@ -67,7 +68,8 @@ def load_config(self, config, qos=None): return self.msg_state.load_config(data, "state_topic", "state_payload", qos) - self.msg_on_off.load_config(data, "on_off_topic", "on_off_payload", qos) + self.msg_on_off.load_config(data, "on_off_topic", + "on_off_payload", qos) self.msg_scene.load_config(data, "scene_topic", "scene_payload", qos) #----------------------------------------------------------------------- @@ -112,9 +114,8 @@ def unsubscribe(self, link): link.unsubscribe(topic) #----------------------------------------------------------------------- - def template_data( - self, is_on=None, button=None, mode=on_off.Mode.NORMAL, reason=None - ): + def template_data(self, is_on=None, button=None, mode=on_off.Mode.NORMAL, + reason=None): """Create the Jinja templating data variables for on/off messages. Args: @@ -129,10 +130,8 @@ def template_data( Returns: dict: Returns a dict with the variables available for templating. """ - data = { - "address": self.device.addr.hex, - "name": self.device.name if self.device.name else self.device.addr.hex, - } + name = self.device.name if self.device.name else self.device.addr.hex + data = {"address": self.device.addr.hex, "name": name} if button is not None: data["button"] = button @@ -148,7 +147,8 @@ def template_data( return data #----------------------------------------------------------------------- - def _insteon_on_off(self, device, group, is_on, mode=on_off.Mode.NORMAL, reason=""): + def _insteon_on_off(self, device, group, is_on, + mode=on_off.Mode.NORMAL, reason=""): """Device active on/off callback. This is triggered via signal when the Insteon device turns on or off. @@ -189,7 +189,8 @@ def _input_on_off(self, client, data, message, group): message: MQTT message - has attrs: topic, payload, qos, retain. """ LOG.debug( - "EZIO4O output %s message %s %s", group, message.topic, message.payload + "EZIO4O output %s message %s %s", group, message.topic, + message.payload ) # Parse the input MQTT message. @@ -218,7 +219,8 @@ def _input_scene(self, client, data, message, group): message: MQTT message - has attrs: topic, payload, qos, retain. """ LOG.debug( - "EZIO4O output %s message %s %s", group, message.topic, message.payload + "EZIO4O output %s message %s %s", group, message.topic, + message.payload ) # Parse the input MQTT message. diff --git a/insteon_mqtt/mqtt/Leak.py b/insteon_mqtt/mqtt/Leak.py index 01e291d4..ac93f99e 100644 --- a/insteon_mqtt/mqtt/Leak.py +++ b/insteon_mqtt/mqtt/Leak.py @@ -3,7 +3,6 @@ # MQTT leak sensor device # #=========================================================================== -import time from .. import log from .BatterySensor import BatterySensor from .MsgTemplate import MsgTemplate diff --git a/insteon_mqtt/network/Mqtt.py b/insteon_mqtt/network/Mqtt.py index 6289a239..3cc21e8c 100644 --- a/insteon_mqtt/network/Mqtt.py +++ b/insteon_mqtt/network/Mqtt.py @@ -179,7 +179,8 @@ def poll(self, t): passed in so that all clients receive the same "current" time instead of each calling time.time() and getting a different value. """ - # This is required to handle keepalive messages and detect disconnections. + # This is required to handle keepalive messages and detect + # disconnections. rc = self.client.loop_misc() if rc == paho.MQTT_ERR_NO_CONN: self._on_disconnect(self.client, None, rc) diff --git a/insteon_mqtt/network/poll.py b/insteon_mqtt/network/poll.py index aa718505..3264dfd5 100644 --- a/insteon_mqtt/network/poll.py +++ b/insteon_mqtt/network/poll.py @@ -162,8 +162,8 @@ def close_all(self): # items from that dict when the callbacks happen. So copy the value # and iterate over that. links = list(self.links.values()) - for l in links: - l.close() + for link in links: + link.close() #----------------------------------------------------------------------- def select(self, time_out=None): @@ -247,7 +247,8 @@ def select(self, time_out=None): # user reports. So copy the links before iterating since closing the # link mods the dict which isn't allowed. This isn't ideal but the # number of links should be small so it probably doesn't matter. - for link in itertools.chain(list(self.links.values()), self.poll_links): + for link in itertools.chain(list(self.links.values()), + self.poll_links): link.poll(t) #----------------------------------------------------------------------- diff --git a/insteon_mqtt/network/select.py b/insteon_mqtt/network/select.py index 173f589a..f7f0d31c 100644 --- a/insteon_mqtt/network/select.py +++ b/insteon_mqtt/network/select.py @@ -161,8 +161,8 @@ def close_all(self): This wlil call Link.close() to shut the links down. """ links = self.links.values() - for l in links: - l.close() + for link in links: + link.close() #----------------------------------------------------------------------- def select(self, time_out=None): @@ -242,7 +242,8 @@ def select(self, time_out=None): # user reports. So copy the links before iterating since closing the # link mods the dict which isn't allowed. This isn't ideal but the # number of links should be small so it probably doesn't matter. - for link in itertools.chain(list(self.links.values()), self.poll_links): + for link in itertools.chain(list(self.links.values()), + self.poll_links): link.poll(t) #----------------------------------------------------------------------- diff --git a/insteon_mqtt/util.py b/insteon_mqtt/util.py index 145d6bd7..9f3591e0 100644 --- a/insteon_mqtt/util.py +++ b/insteon_mqtt/util.py @@ -281,6 +281,7 @@ def input_byte(inputs, field): msg = "Invalid %s input. Valid inputs are 0-255" % input raise ValueError(msg) + #=========================================================================== def input_float(inputs, field): """Convert an input field to an float. @@ -311,4 +312,4 @@ def input_float(inputs, field): return float(value) except ValueError: msg = "Invalid %s input. Valid inputs are float values." % input - raise ValueError(msg) \ No newline at end of file + raise ValueError(msg) From 550d50c040a912b356509b6a09cb87751af7a167 Mon Sep 17 00:00:00 2001 From: Kevin Robert Keegan Date: Thu, 17 Dec 2020 15:44:52 -0800 Subject: [PATCH 48/54] Update python-app.yml no spaces in ignore. --- .github/workflows/python-app.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/python-app.yml b/.github/workflows/python-app.yml index 990ed12e..05dec390 100644 --- a/.github/workflows/python-app.yml +++ b/.github/workflows/python-app.yml @@ -33,7 +33,7 @@ jobs: uses: TrueBrain/actions-flake8@v1.4.1 with: path: insteon_mqtt - ignore: E265, E203, E123, E722, E127, E131, E731, W504 + ignore: E265,E203,E123,E722,E127,E131,E731,W504 max_line_length: 79 only_warn: 1 - name: Test with pytest From 376da21b12007cc63458689d4ffd54101c4d9c18 Mon Sep 17 00:00:00 2001 From: Kevin Robert Keegan Date: Thu, 17 Dec 2020 17:24:54 -0800 Subject: [PATCH 49/54] Add Coverage Comments to PRs --- .github/workflows/python-app.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/python-app.yml b/.github/workflows/python-app.yml index 05dec390..8b65d8dd 100644 --- a/.github/workflows/python-app.yml +++ b/.github/workflows/python-app.yml @@ -39,3 +39,10 @@ jobs: - name: Test with pytest run: | pytest tests + - name: Build coverage file + run: | + # This skips priting files with 100% coverage + pytest --cache-clear --cov-report term:skip-covered --cov=insteon_mqtt tests/ > pytest-coverage.txt + - name: Comment coverage + uses: coroo/pytest-coverage-commentator@v1.0.2 + From 17b587ebbc59cd1e27e4ea3bffb2e56f230b3545 Mon Sep 17 00:00:00 2001 From: Kevin Robert Keegan Date: Thu, 17 Dec 2020 17:48:51 -0800 Subject: [PATCH 50/54] Fix Coverage Commenter --- .github/workflows/python-app.yml | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/.github/workflows/python-app.yml b/.github/workflows/python-app.yml index 8b65d8dd..a01b60c6 100644 --- a/.github/workflows/python-app.yml +++ b/.github/workflows/python-app.yml @@ -10,9 +10,7 @@ on: jobs: build: - runs-on: ubuntu-latest - steps: - uses: actions/checkout@v2 - name: Set up Python 3.8 @@ -38,11 +36,10 @@ jobs: only_warn: 1 - name: Test with pytest run: | - pytest tests - - name: Build coverage file - run: | - # This skips priting files with 100% coverage - pytest --cache-clear --cov-report term:skip-covered --cov=insteon_mqtt tests/ > pytest-coverage.txt + # Skips creating coverage stats for covered items + pytest --cache-clear --cov-report term:skip-covered --cov=insteon_mqtt tests/ | tee pytest-coverage.txt - name: Comment coverage uses: coroo/pytest-coverage-commentator@v1.0.2 + with: + pytest-coverage: pytest-coverage.txt From 2c610262617c50475186ffa4e1f620636637f716 Mon Sep 17 00:00:00 2001 From: Kevin Robert Keegan Date: Thu, 17 Dec 2020 17:55:40 -0800 Subject: [PATCH 51/54] Remove Coverage Comment Might lack permissions to post a comment? --- .github/workflows/python-app.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/python-app.yml b/.github/workflows/python-app.yml index a01b60c6..47b7e9bc 100644 --- a/.github/workflows/python-app.yml +++ b/.github/workflows/python-app.yml @@ -38,8 +38,8 @@ jobs: run: | # Skips creating coverage stats for covered items pytest --cache-clear --cov-report term:skip-covered --cov=insteon_mqtt tests/ | tee pytest-coverage.txt - - name: Comment coverage - uses: coroo/pytest-coverage-commentator@v1.0.2 - with: - pytest-coverage: pytest-coverage.txt +# - name: Comment coverage +# uses: coroo/pytest-coverage-commentator@v1.0.2 +# with: +# pytest-coverage: pytest-coverage.txt From 534180944a0360c812a81a1fce2c7b9f94ca5054 Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Fri, 18 Dec 2020 12:58:05 -0800 Subject: [PATCH 52/54] =?UTF-8?q?Bump=20version:=200.7.3=20=E2=86=92=200.7?= =?UTF-8?q?.4?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .bumpversion.cfg | 2 +- README.md | 2 +- hassio/config.json | 2 +- insteon_mqtt/__init__.py | 2 +- setup.py | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.bumpversion.cfg b/.bumpversion.cfg index d89f2a37..d8172498 100644 --- a/.bumpversion.cfg +++ b/.bumpversion.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 0.7.3 +current_version = 0.7.4 commit = True tag = False diff --git a/README.md b/README.md index 1d8d8be3..6dee4356 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ My initial intent with this package is better integrate Insteon into Home Assistant and make it easier and more understandable to add new features and devices. -Version: 0.7.3 ([History](HISTORY.md)) +Version: 0.7.4 ([History](HISTORY.md)) ### Breaking changes from last version: diff --git a/hassio/config.json b/hassio/config.json index 8abec6db..fc963ae1 100644 --- a/hassio/config.json +++ b/hassio/config.json @@ -2,7 +2,7 @@ "name": "Insteon MQTT", "description": "Python Insteon PLM <-> MQTT bridge", "slug": "insteon-mqtt", - "version": "0.7.3", + "version": "0.7.4", "startup": "services", "arch": ["amd64","armhf","aarch64","i386"], "boot": "auto", diff --git a/insteon_mqtt/__init__.py b/insteon_mqtt/__init__.py index aa9a07d3..3a94e73c 100644 --- a/insteon_mqtt/__init__.py +++ b/insteon_mqtt/__init__.py @@ -10,7 +10,7 @@ For docs, see: https://www.github.com/TD22057/insteon-mqtt """ -__version__ = "0.7.3" +__version__ = "0.7.4" #=========================================================================== diff --git a/setup.py b/setup.py index e3d486a8..b2f59fc3 100644 --- a/setup.py +++ b/setup.py @@ -8,7 +8,7 @@ setuptools.setup( name = 'insteon-mqtt', - version = '0.7.3', + version = '0.7.4', description = "Insteon <-> MQTT bridge server", long_description = readme, author = "Ted Drain", From 13d9d39907fde59cc493f17934c879be1202a3e2 Mon Sep 17 00:00:00 2001 From: Kevin Robert Keegan Date: Fri, 18 Dec 2020 13:05:01 -0800 Subject: [PATCH 53/54] Update README.md --- README.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 6dee4356..77d9af63 100644 --- a/README.md +++ b/README.md @@ -11,9 +11,11 @@ features and devices. Version: 0.7.4 ([History](HISTORY.md)) -### Breaking changes from last version: +### Recent Breaking Changes -- KeypadLinc now supports both dimmer and on/off device types. This required +- 0.7.4 - IOLinc, the scene_topic has been elimited, please see the documentation + for the replaces functionality. +- 0.7.2 - KeypadLinc now supports both dimmer and on/off device types. This required changing the KeypadLinc inputs in the MQTT portion of the config.yaml file. See the file in the repository for the new input fields. ([Issue #33][I33]). From 0314d6a65afc1fd61228f181035f324bf4dc1f9b Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Fri, 18 Dec 2020 13:45:59 -0800 Subject: [PATCH 54/54] Update History --- HISTORY.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/HISTORY.md b/HISTORY.md index 52d7c4f1..fd402c43 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,5 +1,14 @@ # Revision Change History +## [0.7.5] + +### Additions + +### Fixes + +- Improved message handling and processing of Pre_NAK messages. + ([PR 236][P236]) + ## [0.7.4] ### Additions @@ -459,3 +468,4 @@ will add new features. [P235]: https://github.com/TD22057/insteon-mqtt/pull/235 [P259]: https://github.com/TD22057/insteon-mqtt/pull/259 [P234]: https://github.com/TD22057/insteon-mqtt/pull/234 +[P236]: https://github.com/TD22057/insteon-mqtt/pull/236