From 055b4cca9e59f62311d284b316b5d30f6e7387bf Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Sun, 13 Dec 2020 15:37:49 -0800 Subject: [PATCH] Don't Assume Last Mem Location is Unused Insteon devices always keep the last memory location as unused and all 0x00s. However, it looks like ISY may use the last location. Our code also didn't treat the last_loc as unused even when it was marked as so. Now, the last_loc is only put in the unused array if it is unused. This means that to write to an existing mem_loc we need >1 unused address. To bring an ISY database into alignment with how we use it if the last loc is used, we do 3 things. 1. Change the last_loc flag on the current last entry to false 2. Write the new entry in the slot just below #1 marked as used and not last 3. Write a new all 0x00 entry marked unused and last below #2 Finally rearrange how we write new entrys so that: 1. Write the new entry in the slot currently marked as last 2. Write a new all 0x00 entry marked unused and last below #2 This could mean that if things fail before #2 happens a bunch of bizarre old links may reappear, but I think this is extremely unlikely, and organizing it this way enables the ISY fix to be done in only 3 database writes. --- insteon_mqtt/db/Device.py | 85 ++++++++++++++++++++++++--------------- 1 file changed, 53 insertions(+), 32 deletions(-) diff --git a/insteon_mqtt/db/Device.py b/insteon_mqtt/db/Device.py index eee146dd..875e9c9d 100644 --- a/insteon_mqtt/db/Device.py +++ b/insteon_mqtt/db/Device.py @@ -349,7 +349,7 @@ def add_on_device(self, addr, group, is_controller, data, on_done = util.make_callback(on_done) # See if we can fill in an unused entry in the db. - add_unused = len(self.unused) > 0 + add_unused = len(self.unused) > 1 # See if the entry already exists. Pass data[2] so we only match # entries with the same local group. @@ -665,6 +665,11 @@ def add_entry(self, entry, save=True): Args: entry: (DeviceEntry) The entry to add. """ + + # Entry is a new last record to use + if entry.db_flags.is_last_rec: + self.last = entry + # Entry is an active entry. if entry.db_flags.in_use: # NOTE: this relies on no-one keeping a handle to this entry @@ -681,10 +686,6 @@ def add_entry(self, entry, save=True): if entry not in responders: responders.append(entry) - # Entry is not in use and is a new last record to use - elif entry.db_flags.is_last_rec: - self.last = entry - # Entry is a normal record but is not in use. else: # NOTE: this relies on no one keeping a handle to this entry @@ -779,53 +780,73 @@ def _add_using_new(self, addr, group, is_controller, data, """ # pylint: disable=too-many-locals - # Start by moving the current last record down 8 bytes. Write out - # the new last record and then create a new record with the input - # data at the location of the old last record. - 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) - # Write the new last entry as all 00 which is how it appears on - # factory reset - flags = Msg.DbFlags(in_use=False, is_controller=False, - is_last_rec=True) - last = DeviceEntry(Address(0, 0, 0), 0, self.last.mem_loc, flags, - None, db=self) - last.mem_loc -= 0x08 + last_mem_loc = self.last.mem_loc + + if self.last.db_flags.in_use: + # We don't and insteon does not use the last mem_loc. The last loc + # is always all 0x00s. However ISY appears to use the last loc. + # So we need to fix that by first changing the last location flag + # on the current last entry. + flags = Msg.DbFlags(in_use=False, is_controller=False, + is_last_rec=False) + last = self.last + last.db_flags = flags + + # Update the last record + if self.engine == 0: + # on_done is passed by the sequence manager inside seq.add() + modify_manager = DeviceModifyManagerI1(self.device, self, + last, on_done=None, + num_retry=3) + seq.add(modify_manager.start_modify) + else: + ext_data = last.to_bytes() + msg = Msg.OutExtended.direct(self.addr, 0x2f, 0x00, ext_data) + msg_handler = handler.DeviceDbModify(self, last) + seq.add_msg(msg, msg_handler) + + # Update the last mem_loc to use + last_mem_loc = last_mem_loc - 0x08 + + # Create the new entry at the current last memory location. + db_flags = Msg.DbFlags(in_use=True, is_controller=is_controller, + is_last_rec=False) + entry = DeviceEntry(addr, group, last_mem_loc, db_flags, data, + db=self) - # Start by writing the last record - that way if it fails, we don't - # try and update w/ the new data record. if self.engine == 0: # on_done is passed by the sequence manager inside seq.add() modify_manager = DeviceModifyManagerI1(self.device, self, - last, on_done=None, + entry, on_done=None, num_retry=3) seq.add(modify_manager.start_modify) else: - ext_data = last.to_bytes() + # Add the call to update the data record. + ext_data = entry.to_bytes() msg = Msg.OutExtended.direct(self.addr, 0x2f, 0x00, ext_data) - msg_handler = handler.DeviceDbModify(self, last) + msg_handler = handler.DeviceDbModify(self, entry) seq.add_msg(msg, msg_handler) - # Create the new entry at the current last memory location. - db_flags = Msg.DbFlags(in_use=True, is_controller=is_controller, - is_last_rec=False) - entry = DeviceEntry(addr, group, self.last.mem_loc, db_flags, data, - db=self) + # Finally write the new last entry as all 00 which is how it appears on + # factory reset, to the address just below the current last_loc + # just in case something was actually here + flags = Msg.DbFlags(in_use=False, is_controller=False, + is_last_rec=True) + last = DeviceEntry(Address(0, 0, 0), 0, last_mem_loc - 0x08, flags, + None, db=self) if self.engine == 0: # on_done is passed by the sequence manager inside seq.add() modify_manager = DeviceModifyManagerI1(self.device, self, - entry, on_done=None, + last, on_done=None, num_retry=3) seq.add(modify_manager.start_modify) else: - # Add the call to update the data record. - ext_data = entry.to_bytes() + ext_data = last.to_bytes() msg = Msg.OutExtended.direct(self.addr, 0x2f, 0x00, ext_data) - msg_handler = handler.DeviceDbModify(self, entry) + msg_handler = handler.DeviceDbModify(self, last) seq.add_msg(msg, msg_handler) seq.run()