-
Couldn't load subscription status.
- Fork 51
Use node model state to update the android notification #331
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
Conversation
6e0d079 to
1ef10e1
Compare
d5411a7 to
eb53a46
Compare
|
Needs rebase. |
| this, &AndroidNotifier::onBlockTipHeightChanged); | ||
| QObject::connect(&node_model, &NodeModel::numOutboundPeersChanged, | ||
| this, &AndroidNotifier::onNumOutboundPeersChanged); | ||
| QObject::connect(&node_model, &NodeModel::pauseChanged, |
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.
Concept ACK, I've been running and testing this for a few days. The only issue I've noticed is that the "Paused" state won't be represented in the Notification if we were syncing before pressing paused. If we were connecting or any other state, it'll show. But not if we were syncing.
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.
What state does it show? I'm having a hard time reproducing
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.
Well now I also can't reproduce, may have just lagged in showing the update for whatever reason android decided.
| notificationBuilder.setContentTitle("Connecting..."); | ||
| } else if (!synced) { | ||
| if (verificationProgress < 0) { | ||
| notificationBuilder.setContentTitle(String.format("%.2f%% loaded...", verificationProgress)); |
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.
eb53a46 to
acd2d3e
Compare
The AndroidNotifier class connects to the NodeModel's state signals and uses JNI to send callbacks to the Android service managing the foreground notification.
acd2d3e to
b641df4
Compare
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.
Approach ACK
I've been learning more about what we really need to do here in order to interact with the Android and the JVM, and this code is looking good here.
The androidnotifier.cpp file contains the importance pieces of magic. We're gaining access to the JVM and storing it. Then we're setting up the wiring between nodeModel updates to calls into our BitcoinQt Service to update the notification text.
@johnny9 are you able to reproduce this: https://github.com/bitcoin-core/gui-qml/pull/331/files#r1211573422
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.
ACK b641df4
…droid notification 50c7340 android: use node model state to update notification (johnny9) Pull request description: The AndroidNotifier class connects to the NodeModel's state signals and uses JNI to send callbacks to the Android service managing the foreground notification. [](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/win64/unsecure_win_gui.zip?branch=pull/331) [](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/macos/unsecure_mac_gui.zip?branch=pull/331) [](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/macos_arm64/unsecure_mac_arm64_gui.zip?branch=pull/331) [](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/android/unsecure_android_apk.zip?branch=pull/331) ACKs for top commit: jarolrod: ACK 50c7340 Tree-SHA512: 1dfc95ade573f6620ba67871ff6f609e4bffe4cee877e03bd56c8917ac8cfb184198e5ca603ffda1962a3416da52921c0228eab5f0172011eca44de3d813e0d0
…droid notification 50c73409abb042fb20f00d803aee082981425398 android: use node model state to update notification (johnny9) Pull request description: The AndroidNotifier class connects to the NodeModel's state signals and uses JNI to send callbacks to the Android service managing the foreground notification. [](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/win64/unsecure_win_gui.zip?branch=pull/331) [](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/macos/unsecure_mac_gui.zip?branch=pull/331) [](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/macos_arm64/unsecure_mac_arm64_gui.zip?branch=pull/331) [](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/android/unsecure_android_apk.zip?branch=pull/331) ACKs for top commit: jarolrod: ACK 50c73409abb042fb20f00d803aee082981425398 Tree-SHA512: 1dfc95ade573f6620ba67871ff6f609e4bffe4cee877e03bd56c8917ac8cfb184198e5ca603ffda1962a3416da52921c0228eab5f0172011eca44de3d813e0d0



The AndroidNotifier class connects to the NodeModel's state signals and uses JNI to send callbacks to the Android service managing the foreground notification.