Skip to content
This repository has been archived by the owner on Jan 5, 2022. It is now read-only.

Added optional automated confirmation for backups #12

Merged
merged 4 commits into from
Nov 8, 2015

Conversation

inktrap
Copy link
Contributor

@inktrap inktrap commented Nov 8, 2015

  • I implemented optional automated confirmation for backups.
  • Adebar's backup function wil be running in the background, so confirmation keycodes can be sent.
  • I tested the script-generation and the resulting script and they both work.
  • Also I tried to follow the Adebar coding-style, if you have any suggestions, tell me
  • I think this ability is very important and adds greatly to the usability of Adebar

@inktrap
Copy link
Contributor Author

inktrap commented Nov 8, 2015

Also I updated the config in my branch with a note that spaces are currently not supported in passwords, but this is not included in this PR. So feel free to add it.

Btw.: Making spaces work would require to split the password, then enter each segment, then the space via keyevent 62. (This won't work, see comment below)

@inktrap
Copy link
Contributor Author

inktrap commented Nov 8, 2015

  1. I tried implementing the fix for spaces in passwords and entering them works. But if there are more than two spaces, the keyboard will be visible and the following keycodes fail.

  2. I also tried including spaces inline "like%sthis", which is much nicer than 1) but that gave the same results. Therefore I won't be implementing this. No spaces for backup-passwords, keycodes are to fragile.

@IzzySoft
Copy link
Owner

IzzySoft commented Nov 8, 2015

Thanks a lot! This sounds good – an on a quick investigation also looks good, so I will merge this PR. It requires an additional change, though: As using a config file is optional, defaults for the two new settings need to be placed in adebar_cli as well. Until approved by more devices/users, I'll put AUTO_CONFIRM=0 there for now.

Speaking of which: May I ask you to add your device details to issue #7?

IzzySoft added a commit that referenced this pull request Nov 8, 2015
Added optional automated confirmation for backups
@IzzySoft IzzySoft merged commit 04a02d9 into IzzySoft:master Nov 8, 2015
@IzzySoft
Copy link
Owner

IzzySoft commented Nov 8, 2015

PS: Any plans to support this with the restore scripts as well?

@inktrap
Copy link
Contributor Author

inktrap commented Nov 8, 2015

re: restore scripts: sure, why not? Haven't looked at them yet
and have not tried restoring at all.

Excerpts from Izzy's message of 2015-11-08 15:18:08 +0100:

PS: Any plans to support this with the restore scripts as well?


Reply to this email directly or view it on GitHub:
#12 (comment)

@IzzySoft
Copy link
Owner

IzzySoft commented Nov 8, 2015

Thanks in advance! No need to "hurry" – whenever you find time is fine. Before compiling this into a new "release", I just think it should be "complete" :)

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

Successfully merging this pull request may close these issues.

2 participants