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

[WIP] Iroh share UI #1649

Closed
wants to merge 2 commits into from
Closed

[WIP] Iroh share UI #1649

wants to merge 2 commits into from

Conversation

cyBerta
Copy link
Contributor

@cyBerta cyBerta commented Jul 13, 2022

this pr adds a "Link Device" button atop of the settings that shows a QR code. scanning this code from a new device setup transfers the account without prompting for further credentials.

the pr needs core-branch deltachat/deltachat-core-rust#3489 to be checked out at deltachat-ios/libraries/deltachat-core-rust 1

known issues below the image and video ...

video_2022-07-14_22-23-05.mp4

... known issues:

  • showing the qr code works only once, if the "Link Device" screen is left and re-entered, no qr code is shown; reason is unclear, may be related to core, @cyBerta knows more

  • when scanning the QR code and confirming "linking devices", apples shows a promt "Delta Chat would like to find and connect to devices on your local network.", see https://support.apple.com/en-us/HT211870 . the prompt is triggered by some network request in core, unfortunately, even when granted, core does not continue with the request (timeout?) and fails with "dial error". when the app is restarted (swipe away and restart), the prompt is no longer needed and things work. it would be nice to trigger the prompt before calling dc_receive_backup(), however, i did not found a way to do this yet. EDIT: fixed, the prompt is shown before calling dc_receive_backup() now.

these are the known UI bugs, there be dragons in the core 🐉 exciting things in the core

Footnotes

  1. the core-branch it is not part of the pr to make updates, rebasing and switching ios-branches easier

@@ -30,6 +30,8 @@
30E8F2482449C98600CE2C90 /* UIView+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 30E8F2472449C98600CE2C90 /* UIView+Extensions.swift */; };
30E8F24B2449CF6500CE2C90 /* InitialsBadge.swift in Sources */ = {isa = PBXBuildFile; fileRef = 30E8F24A2449CF6500CE2C90 /* InitialsBadge.swift */; };
30E8F24D2449D30200CE2C90 /* DcColors.swift in Sources */ = {isa = PBXBuildFile; fileRef = 30E8F24C2449D30200CE2C90 /* DcColors.swift */; };
78A8733F287766FA00690A0B /* libc++.tbd in Frameworks */ = {isa = PBXBuildFile; fileRef = 78A8733E287766FA00690A0B /* libc++.tbd */; };
78A873412877679B00690A0B /* SystemConfiguration.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 78A873402877679B00690A0B /* SystemConfiguration.framework */; };
Copy link
Member

Choose a reason for hiding this comment

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

what is this for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that was automatically added when updating the targets I think.

@r10s
Copy link
Member

r10s commented Jul 13, 2022

i know, this is wip, but in case you did not recognize: if the "Setup New Device" screen is entered a second time, the qr code is not shown.

@cyBerta
Copy link
Contributor Author

cyBerta commented Jul 14, 2022

i know, this is wip, but in case you did not recognize: if the "Setup New Device" screen is entered a second time, the qr code is not shown.

after debugging it, I think it is a core issue. In the last commit I added a lot of logs. I think it shows that self.dcContext.send_backup fails to release a lock used in the imex process.

@r10s
Copy link
Member

r10s commented Jul 14, 2022

after debugging it, I think it is a core issue. In the last commit I added a lot of logs. I think it shows that self.dcContext.send_backup fails to release a lock used in the imex process.

k, let's leave that as is for now and move to scanning, i have a commit here that ask for scanning on DCBACKUP, shall i push it? EDIT: done

@Simon-Laux
Copy link
Member

Simon-Laux commented Jul 14, 2022

this FAQ could be helpful for understanding the local network permission thing:
https://developer.apple.com/forums/thread/663858

probably the actionable items are:

  • set a good reason string so its less likely user denies it
  • try to find a way to find out whether we have the permission before starting the transfer process in core (and possibly start it right after we got/get the permission)

@r10s
Copy link
Member

r10s commented Jul 15, 2022

here is a workaround that could be used to trigger the permission thing so it does not interfere with core, ohh, apple ...

https://developer.apple.com/forums/thread/663768

@dignifiedquire we can build that in, however, maybe core also does sth. wrong that the ios prompt result core in returning ☎️ "dial error"
(i would assume if the user taps "Allow", the request is continued, but maybe that part is also buggy from ios side :)

@r10s
Copy link
Member

r10s commented Aug 31, 2022

i rebases the pr and removed the submodule from the various commits as they are hard/impossible to update and to rebase; also finally we want to merge that without submodule anyway.

there were some weird Pod changes that are not mergable, i used the Pod from master, that seems to be okay.

however, now there is the error:

failed to run custom build command for `iroh-rpc-types v0.1.0 (https://github.com/n0-computer/iroh?branch=main#5187773b)`

@r10s
Copy link
Member

r10s commented Aug 31, 2022

however, now there is the error:

failed to run custom build command for `iroh-rpc-types v0.1.0 (https://github.com/n0-computer/iroh?branch=main#5187773b)`

that needs protoc to be installed with:

brew install protobuf

@r10s r10s mentioned this pull request Mar 7, 2023
18 tasks
@r10s
Copy link
Member

r10s commented Mar 7, 2023

replaced by #1822

@r10s r10s closed this Mar 7, 2023
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.

3 participants