-
Notifications
You must be signed in to change notification settings - Fork 90
Feat: Qt keycard support with nfc and pcsc #19545
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
base: master
Are you sure you want to change the base?
Conversation
✔️ status-desktop/prs/linux/x86_64/tests-ui/PR-19545#1 🔹 ~13 min 🔹 8bb7613 🔹 📦 tests/ui package |
✔️ status-desktop/e2e/prspr19545 🔹 ~15 min 🔹 8bb7613 🔹 📦 tests/e2e package |
✔️ status-desktop/prs/linux/x86_64/tests-ui/PR-19545#2 🔹 ~14 min 🔹 450a51f 🔹 📦 tests/ui package |
✔️ status-desktop/e2e/prspr19545 🔹 ~16 min 🔹 450a51f 🔹 📦 tests/e2e package |
✔️ status-desktop/prs/android/arm64/package/PR-19545#3 🔹 ~11 min 🔹 77b27f38 🔹 📦 android/arm64 package |
✔️ status-desktop/prs/linux/x86_64/tests-ui/PR-19545#3 🔹 ~13 min 🔹 adadfcd 🔹 📦 tests/ui package |
✔️ status-desktop/e2e/prspr19545 🔹 ~32 min 🔹 adadfcd 🔹 📦 tests/e2e package |
✔️ status-desktop/e2e/prs-windowsPR19545 🔹 ~23 min 🔹 adadfcd 🔹 📦 tests/e2e-windows package |
✔️ status-desktop/prs/android/arm64/package/PR-19545#4 🔹 ~11 min 🔹 e6338a7a 🔹 📦 android/arm64 package |
✔️ status-app/prs/linux/x86_64/tests-ui/PR-19545#4 🔹 ~1 hr 6 min 🔹 7e8edd3 🔹 📦 tests/ui package |
✔️ status-app/prs/linux/x86_64/tests-nim/PR-19545#5 🔹 ~10 min 🔹 f23b896 🔹 📦 tests/nim package |
✔️ status-app/prs/linux/x86_64/tests-ui/PR-19545#5 🔹 ~14 min 🔹 f23b896 🔹 📦 tests/ui package |
Jenkins BuildsClick to see older builds (73)
|
| KEYCARD_LIB := $(STATUSKEYCARD_QT_LIB) | ||
| KEYCARD_LIBDIR := $(STATUSKEYCARD_QT_LIBDIR) | ||
| KEYCARD_LINKNAME := status-keycard-qt | ||
| KEYCARD_DYLIB_NAME := libstatus-keycard-qt.dylib |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That looks like a macOS specific libname
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Linux will look like "libstatus-keycard-qt.so"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should use a variant of STATUSKEYCARD_QT_LIB?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still a valid question imo :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ahh, lol! Just pushed the fix to the next PR that sits on top of this. 6352ad8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, up to you and how you want to deal with merging the whole chain :)
This commit adds the status-keycard-qt submodule and updates the build configuration to support it. In this initial version the `status-keycard-qt` option can be enabled by using the `USE_STATUS_KEYCARD_QT` flag. - desktop: `USE_STATUS_KEYCARD_QT` will enable/disable the qt implementation. By default the go version is used - ios: `USE_STATUS_KEYCARD_QT` will enable/disable the qt implementation - android: the qt implementation is enabled by default
If the app needs the nfc channel at startup qt will enter in a race-condition. The result is that qt cannot enable the reader since the java infrastructure-setup is not ready for qt. The nfc can be enabled afterwards only by placing the app in the background and then back to foreground. To workaround this issue we'll make sure the foregroundDispatch gets enabled at startup.
GlobalPlatform crypto needs the legacy providers. While it will potentially work OOTB with android since we're using dynamic linking, for IOS we need to compile with `no-module`. This will bundle the providers in the lib.
f23b896 to
8a34d73
Compare
+ Fix lib extention
|
Thanks @caybro. Updated the makefile and added some documentation for the makefile flags we use. |
| KEYCARD_LIB := $(STATUSKEYCARD_QT_LIB) | ||
| KEYCARD_LIBDIR := $(STATUSKEYCARD_QT_LIBDIR) | ||
| KEYCARD_LINKNAME := status-keycard-qt | ||
| KEYCARD_DYLIB_NAME := libstatus-keycard-qt.dylib |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still a valid question imo :)
What does the PR do
Add https://github.com/status-im/status-keycard-qt lib as a keycard integration option.
This commit adds the status-keycard-qt submodule and updates the build configuration to support it.
In this initial version the
status-keycard-qtoption can be enabled by using theUSE_STATUS_KEYCARD_QTflag.USE_STATUS_KEYCARD_QTwill enable/disable the qt implementation. By default the go version is usedUSE_STATUS_KEYCARD_QTwill enable/disable the qt implementation. On IOS the developer experience is quite bad and I've tried not to make it worse here. When compiling the app with keycard support a paid apple developer account is needed to sign the app. For this reason theUSE_STATUS_KEYCARD_QTis disabled by default in the mobile makefile. People with apple accounts can use theDEVELOPMENT_TEAMflag to configure the signing before running the app.USE_STATUS_KEYCARD_QTAcceptance criteria
NOTE: Tests can be done in the final PR #19549