Skip to content

Commit

Permalink
Merge pull request #255 from krkeegan/Fix_250
Browse files Browse the repository at this point in the history 
Last Entry Can be Used
  • Loading branch information
@krkeegan
krkeegan authored Dec 18, 2020
2 parents 085a68d + 4261146 commit 0ddef77
Show file tree
Hide file tree
Showing 3 changed files with 159 additions and 37 deletions.
4 changes: 4 additions & 0 deletions HISTORY.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@

### Fixes

- Allow for used last entries in DB. Improve compatibility with devices
setup by ISY. ([PR 255][P255])

- Improved message handling and processing of Pre_NAK messages.
([PR 236][P236])

Expand Down Expand Up @@ -469,3 +472,4 @@ will add new features.
[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
[P255]: https://github.com/TD22057/insteon-mqtt/pull/255
86 changes: 49 additions & 37 deletions insteon_mqtt/db/Device.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,11 @@ def from_json(data, path, device):
for d in data['unused']:
obj.add_entry(DeviceEntry.from_json(d, db=obj), save=False)

# This isn't needed anymore, but here for compatibility with pre
# 0.7.4. The last entry appears in either the used or unused array
if "last" in data:
obj.last = DeviceEntry.from_json(data["last"], db=obj)
obj.add_entry(DeviceEntry.from_json(data["last"], db=obj),
save=False)

# When loading db's <= ver 0.6, no last field was saved to create
# one at the correct location.
Expand Down Expand Up @@ -349,7 +352,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.
Expand All @@ -367,8 +370,8 @@ def add_on_device(self, addr, group, is_controller, data,
on_done(True, "Entry already exists", entry)
return

LOG.info("Device %s adding db: %s grp %s %s %s", self.addr, addr,
group, util.ctrl_str(is_controller), data)
LOG.info("Device %s adding db: %s grp %s %s D: %s", self.addr, addr,
group, util.ctrl_str(is_controller), data.hex())

# If there are entries in the db that are mark unused, we can re-use
# those memory addresses and just update them w/ the correct
Expand Down Expand Up @@ -621,7 +624,6 @@ def to_json(self):
'firmware' : self.firmware,
'used' : used,
'unused' : unused,
'last' : self.last.to_json(),
'meta' : self._meta
}
if self.desc:
Expand Down Expand Up @@ -666,6 +668,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
Expand All @@ -682,10 +689,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
Expand Down Expand Up @@ -780,53 +783,62 @@ 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)

# Shift the current last record down 8 bytes. Make a copy - we'll
# only update our member var if the write works.
last = self.last.copy()
last.mem_loc -= 0x08
last_mem_loc = self.last.mem_loc

# 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,
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)
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.
last = self.last.copy()
last.db_flags.is_last_rec = False

# Update the last record
self._add_entry_seq(seq, last)

# Create the new entry at the current last memory location.
# Update the last mem_loc to use
last_mem_loc = last_mem_loc - 0x08

# Create the new entry at the 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,
entry = DeviceEntry(addr, group, last_mem_loc, db_flags, data,
db=self)
self._add_entry_seq(seq, entry)

# 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)
self._add_entry_seq(seq, last)

seq.run()

def _add_entry_seq(self, seq, entry):
"""Appends the Appropriate Modify Handler for the entry to the
sequence.
Args:
seq: (CommandSeq)The sequence to append the action to
entry: (DeviceEntry) The entry to modify
"""
# 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,
entry, 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()
msg = Msg.OutExtended.direct(self.addr, 0x2f, 0x00, ext_data)
msg_handler = handler.DeviceDbModify(self, entry)
seq.add_msg(msg, msg_handler)

seq.run()

#-----------------------------------------------------------------------
106 changes: 106 additions & 0 deletions tests/db/test_Device.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,13 +150,119 @@ def test_add_multi_group(self):
db_flags, data2, db=None)
assert right1 == val1

#-----------------------------------------------------------------------
def test_last_entry_used(self):
# tests _add_using_new
# This writing to a db where the last entry is used. It seems that
# ISY devices might do this
device = MockDevice()

local_addr = IM.Address(0x01, 0x02, 0x03)
db = IM.db.Device(local_addr, device=device)

# Initial database is empty. So add a record that is used and last
flags = Msg.DbFlags(in_use=True, is_controller=False,
is_last_rec=True)
orig_last = IM.db.DeviceEntry(IM.Address(0x12, 0x34, 0x56), 0x01,
0x0fff, flags, None, db=db)
db.add_entry(orig_last, save=False)

# Test that entry is there
assert db.last == orig_last
assert len(db.entries) == 1

# Now Add a new entry to the deivce
data = bytes([0xff, 0x00, 0x01])
is_controller = False
remote_addr = IM.Address(0x50, 0x51, 0x52)
remote_group = 0x30
flags = Msg.DbFlags(in_use=True, is_controller=is_controller,
is_last_rec=False)
new_entry = IM.db.DeviceEntry(remote_addr, remote_group, 0x0ff7,
flags, data, db=db)

db.add_on_device(remote_addr, remote_group, is_controller, data)

# 3 messages
assert len(device.sent) == 3
# First message updates orig last to be marked not last
fix_old = orig_last.copy()
fix_old.db_flags = Msg.DbFlags(in_use=True,
is_controller=fix_old.is_controller,
is_last_rec=False)
assert device.sent[0].msg.data == fix_old.to_bytes()
# Second message adds the new entry
assert device.sent[1].msg.data == new_entry.to_bytes()
# Third message should be a new blank last entry
flags = Msg.DbFlags(in_use=False, is_controller=False,
is_last_rec=True)
last = IM.db.DeviceEntry(IM.Address(0, 0, 0), 0, 0x0fef, flags, None,
db=db)
assert device.sent[2].msg.data == last.to_bytes()

# Check that our cache matches what we expect
assert db.last == last
assert len(db.unused) == 1
assert db.find_mem_loc(0x0fff) == fix_old
assert db.find_mem_loc(0x0ff7) == new_entry

#-----------------------------------------------------------------------
def test_last_entry_unused(self):
# tests _add_using_new
# This writing to a db where the last entry is unused, what we expect
device = MockDevice()

local_addr = IM.Address(0x01, 0x02, 0x03)
db = IM.db.Device(local_addr, device=device)

# Initial database is empty. So add a record that is used
flags = Msg.DbFlags(in_use=False, is_controller=False,
is_last_rec=False)
orig_entry = IM.db.DeviceEntry(IM.Address(0x12, 0x34, 0x56), 0x01,
0x0fff, flags, None, db=db)
db.add_entry(orig_entry, save=False)

# add a record that is empty and last
flags = Msg.DbFlags(in_use=False, is_controller=False,
is_last_rec=True)
orig_last = IM.db.DeviceEntry(IM.Address(0x00, 0x00, 0x00), 0x00,
0x0ff7, flags, None, db=db)
db.add_entry(orig_last, save=False)

# Test that entry is there
assert db.last == orig_last
assert len(db.entries) == 0
assert len(db.unused) == 2

# Now Add a new entry to the deivce
data = bytes([0xff, 0x00, 0x01])
is_controller = False
remote_addr = IM.Address(0x50, 0x51, 0x52)
remote_group = 0x30
flags = Msg.DbFlags(in_use=True, is_controller=is_controller,
is_last_rec=False)
new_entry = IM.db.DeviceEntry(remote_addr, remote_group, 0x0fff,
flags, data, db=db)

db.add_on_device(remote_addr, remote_group, is_controller, data)

# 1 messages
assert len(device.sent) == 1
# First message adds the new entry
assert device.sent[0].msg.data == new_entry.to_bytes()

# Check that our cache matches what we expect
assert db.last == orig_last
assert len(db.unused) == 1
assert db.find_mem_loc(0x0fff) == new_entry

#===========================================================================
class MockDevice:
"""Mock insteon_mqtt/Device class
"""
def __init__(self):
self.sent = []
self.modem = H.main.MockModem("")

def send(self, msg, handler, priority=None, after=None):
self.sent.append(H.Data(msg=msg, handler=handler))
Expand Down

0 comments on commit 0ddef77

Please sign in to comment.