Skip to content
This repository has been archived by the owner on Feb 10, 2018. It is now read-only.

Added commit confirm functionality #127

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

kderynski
Copy link
Contributor

Copy link
Member

@mirceaulinic mirceaulinic left a comment

Choose a reason for hiding this comment

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

This looks good to me. Thanks @kderynski!

Can you please confirm that it does not change the current behaviour? (calling commit_config() without any args commits directly?).

@kderynski
Copy link
Contributor Author

Hey @mirceaulinic,

Yes, I have tested it. commit_config() without additional argument confirmed=0 doesn't change current behaviour.

BTW I will create PR in a few minutes with brief description of this method in documentation.

@mirceaulinic
Copy link
Member

Looking at this, I'd like to test the following scenario:

  • connect using the config_lock optional arg set as False
  • commit confirmed x minutes
  • is the config DB still locked?
  • confirm the commit
  • is the config DB unlocked?

@kderynski
Copy link
Contributor Author

@mirceaulinic I have tested proposed scenario and config DB was unlocked during commit confirmed period. I have added small fix and now it works as it should, so config DB is locked when device waits for confirmation.

@mirceaulinic
Copy link
Member

Thanks @kderynski

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants