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

[Qt 5.9] qrscanner: enable by default #1816

Open
wants to merge 1 commit into
base: master
from

Conversation

@mmbyday
Copy link
Contributor

commented Dec 13, 2018

Lack of QR scanning capability is a major inconvenience when moving funds. i.e. sending from GUI to a mobile wallet or paper wallet/offline wallet.

Before this is merged,

  • verify working on buildbots / build environment
  • verify readme works on MacOS as is, or if further updates required.
  • depends on #1807

Thoughts?

@mmbyday mmbyday force-pushed the mmbyday:enable-qr-scanner branch from 64efd90 to a9e0be9 Dec 13, 2018

@selsta

This comment has been minimized.

Copy link
Contributor

commented Dec 13, 2018

Don’t really know how to get this to compile on macOS. (brew install zbar didn’t work). I’ll look into it tomorrow.

@xmrdsc

This comment has been minimized.

Copy link
Contributor

commented Dec 17, 2018

Cool! UX needs some work though (buttons look weird). Don't you agree?

@mmbyday

This comment has been minimized.

Copy link
Contributor Author

commented Dec 17, 2018

@xmrdc are we lboth looking at the "after" with qrcode icons? The "before" did look weird ;-)

@selsta

This comment has been minimized.

Copy link
Contributor

commented Dec 20, 2018

/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang++ -c -pipe -stdlib=libc++ -fPIC -fstack-protector -fstack-protector-strong -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1 -Wformat -Wformat-security -O2 -std=gnu++11  -arch x86_64 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk -mmacosx-version-min=10.12 -Wall -W -fPIC -DWITH_SCANNER -DQT_NO_DEBUG -DQT_QUICK_LIB -DQT_WIDGETS_LIB -DQT_MULTIMEDIA_LIB -DQT_GUI_LIB -DQT_QML_LIB -DQT_NETWORK_LIB -DQT_CORE_LIB -I../../monero-gui -I. -I../monero/include -I../src/libwalletqt -I../src/QR-Code-generator -I../src -I../monero/src -I../src/QR-Code-scanner -I/usr/local/Cellar/qt/5.12.0/lib/QtQuick.framework/Headers -I/usr/local/Cellar/qt/5.12.0/lib/QtWidgets.framework/Headers -I/usr/local/Cellar/qt/5.12.0/lib/QtMultimedia.framework/Headers -I/usr/local/Cellar/qt/5.12.0/lib/QtGui.framework/Headers -I/usr/local/Cellar/qt/5.12.0/lib/QtQml.framework/Headers -I/usr/local/Cellar/qt/5.12.0/lib/QtNetwork.framework/Headers -I/usr/local/Cellar/qt/5.12.0/lib/QtCore.framework/Headers -I. -I/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/System/Library/Frameworks/OpenGL.framework/Headers -I/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/System/Library/Frameworks/AGL.framework/Headers -I/usr/local/Cellar/qt/5.12.0/mkspecs/macx-clang -F/usr/local/Cellar/qt/5.12.0/lib -o QrScanThread.o ../src/QR-Code-scanner/QrScanThread.cpp
In file included from ../src/QR-Code-scanner/QrScanThread.cpp:29:
../src/QR-Code-scanner/QrScanThread.h:38:10: fatal error: 'zbar.h' file not found
#include <zbar.h>
         ^~~~~~~~
1 error generated.
make: *** [QrScanThread.o] Error 1

zbar.h is in /usr/local/include, yet it still fails.

@selsta

This comment has been minimized.

Copy link
Contributor

commented Dec 20, 2018

diff --git a/monero-wallet-gui.pro b/monero-wallet-gui.pro
index 5caf019..fcc407c 100644
--- a/monero-wallet-gui.pro
+++ b/monero-wallet-gui.pro
@@ -184,6 +184,7 @@ CONFIG(WITH_SCANNER) {
             INCLUDEPATH += $$PWD/../ZBar/include
             LIBS += -lzbarjni -liconv
         } else {
+            INCLUDEPATH += /usr/local/include
             LIBS += -lzbar
         }
     } else {

Got it to compile like this (not sure if this is the correct solution), but I have no QR code button in the GUI.

We also have to think about if #611 is still relevant.

@mmbyday

This comment has been minimized.

Copy link
Contributor Author

commented Feb 1, 2019

@selsta Can you try cleaning the build first? Delete everything in the build folder. I found compiling works, but sometimes a clean compile is required for the QR button to show up. Thanks for your help!

@selsta

This comment has been minimized.

Copy link
Contributor

commented Feb 3, 2019

@mmbyday cleaning the build first worked, thanks. Still, I’m against merging this until we fix #611 and improve the overall UI (there is no obvious way to close the scanner apart from double clicking).

@mmbyday mmbyday force-pushed the mmbyday:enable-qr-scanner branch 2 times, most recently from 399aef1 to 36fbf6d Apr 12, 2019

@mmbyday

This comment has been minimized.

Copy link
Contributor Author

commented Apr 12, 2019

@selsta #611 should be fixed now. Camera no longer loads for a split second during startup.

@mmbyday mmbyday force-pushed the mmbyday:enable-qr-scanner branch from 36fbf6d to 6954c7f Apr 12, 2019

@mmbyday mmbyday changed the title qrscanner: enable by default [Qt 5.9] qrscanner: enable by default Apr 12, 2019

@selsta

This comment has been minimized.

Copy link
Contributor

commented Apr 16, 2019

@mmbyday Can confirm :) IMO we should look into adding this after v0.14.1.0 is out, as it will require build bot changes.

@mmbyday mmbyday force-pushed the mmbyday:enable-qr-scanner branch 2 times, most recently from bd0d244 to 47228d5 Apr 17, 2019

@mmbyday

This comment has been minimized.

Copy link
Contributor Author

commented Apr 17, 2019

@selsta Great :) Also added a font awesome button so there's an obvious way to close the scanner.

@mmbyday mmbyday force-pushed the mmbyday:enable-qr-scanner branch from 47228d5 to b4adcf0 Apr 17, 2019

@mmbyday mmbyday force-pushed the mmbyday:enable-qr-scanner branch from b4adcf0 to cbfae9a May 15, 2019

@jonathancross

This comment has been minimized.

Copy link
Contributor

commented Jul 16, 2019

IMO we should look into adding this after v0.14.1.0 is out, as it will require build bot changes.

Friendly ping... v0.14.1.0 is out :-)

@selsta

This comment has been minimized.

Copy link
Contributor

commented Jul 22, 2019

soon™ :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.