Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify Modem DB Delete by Using DB_Flags #363

Merged
merged 2 commits into from
Mar 18, 2021

Conversation

krkeegan
Copy link
Collaborator

@krkeegan krkeegan commented Mar 17, 2021

Proposed change

Contrary to the Insteon Specification, it appears the DB Flags can be used in the Delete command. While using 0x00 for the flags worked in most circumstances, it appears that in some cases for an unknown reason the Delete command would fail on USB Dongle style PLMs. It is possible that the command was also failing on other PLM styles as well and was not noticed or reported.

Using the DB_Flags is a net win, 1) it simplifies the delete functionality significantly and 2) it seems to solve the problem wherein entries could not be deleted.

This change removes the complicated Command_Seq wherein the modem first had to be searched for records to delete, the records were deleted and any necessary restored records were restored.

Now, the delete function simply takes an entry and deletes it. If a nack is return, it is likely that the entry does not exist on the device and a clear error message is returned to the user.

Additional information

Checklist

  • The code change is tested and works locally.
  • The code changes fix the issue for those who were experiencing it.
  • Local tests pass.
  • Tests have been added to verify that the new code works, mostly modified and simplified existing tests. Many tests for the old complicated process were removed.
  • Code documentation was added where necessary

Contrary to the Insteon Specification, the DB Flags can be used
in the Delete command.  While using 0x00 for the flags worked
in most circumstances, it appears that in some cases for an
unknown reason the Delete command would fail on USB Dongle style
PLMs.  It is possible that the command was also failing on other
PLM styles as well and was not noticed or reported.

Using the DB_Flags is a net win, 1) it simplifies the delete
functionality significantly and 2) it seems to solve the problem
wherein entries could not be deleted.

This hopefully fixes TD22057#351.
@krkeegan krkeegan linked an issue Mar 17, 2021 that may be closed by this pull request
Copy link
Contributor

@tstabrawa tstabrawa left a comment

Choose a reason for hiding this comment

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

Looks like you've got one new lint error in insteon_mqtt/db/Modem.py. Otherwise, the changes look good to me.

@zimmer62
Copy link

The checkbox "The code changes fix the issue for those who were experiencing it." on this pull request should be checked.
I can confirm that it did fix my problem. I tested using both the sync command, and the db-delete command.

@krkeegan krkeegan changed the title Simply Modem DB Delete by Using DB_Flags Simplify Modem DB Delete by Using DB_Flags Mar 18, 2021
@krkeegan krkeegan merged commit 139aa11 into TD22057:dev Mar 18, 2021
@krkeegan krkeegan deleted the Modem_Delete branch March 18, 2021 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error trying to delete modem link
3 participants