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.

Sign up for GitHub

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] Display 'Connecting...' when connection to daemon is lost #829

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[WIP] Display 'Connecting...' when connection to daemon is lost #829

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/src/main/java/com/m2049r/xmrwallet/WalletActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ private void onWalletRescan() {
final WalletFragment walletFragment = getWalletFragment();
getWallet().rescanBlockchainAsync();
synced = false;
walletFragment.unsync();
walletFragment.onStartRescan();
invalidateOptionsMenu();
} catch (ClassCastException ex) {
Timber.d(ex.getLocalizedMessage());
Expand Down
11 changes: 11 additions & 0 deletions app/src/main/java/com/m2049r/xmrwallet/WalletFragment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,10 @@ public void unsync() {
bSend.setEnabled(false);
}
if (isVisible()) enableAccountsList(false); //otherwise it is enabled in onResume()
}

public void onStartRescan() {
unsync();
firstBlock = 0;
Copy link
Owner

Choose a reason for hiding this comment

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

i am not sure not unsetting firstBlock in unsync() has no adverse effects

Copy link
Contributor Author

@j-berman j-berman May 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Is there another place unsync() is called I'm not seeing except in the place I added? Or do you mean that it should also unset the firstBlock even in that place I added the call to unsync()?

Copy link
Contributor Author

@j-berman j-berman May 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Will dig into it/test it some more. The issue I was thinking with resetting it in that new call to unsync I added is if it might cause continual resets of the progress bar before it's synchronized IIRC

Copy link
Owner

Choose a reason for hiding this comment

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

i don*t see another place either. yes, i mean also unsetting firstBlock - maybe even scrapping this method as its only used once and has no obvious reuse scenario (or?).

}

Expand Down Expand Up @@ -460,10 +464,17 @@ private void updateStatus(Wallet wallet) {
} else {
sync = getString(R.string.status_synced) + " " + formatter.format(wallet.getBlockChainHeight());
ivSynced.setVisibility(View.VISIBLE);
setProgress(-1);
}
} else {
sync = getString(R.string.status_wallet_connecting);
setProgress(101);
ivSynced.setVisibility(View.GONE);
}
if (wallet.isSynchronized()) {
onSynced();
} else {
unsync();
}
setProgress(sync);
// TODO show connected status somewhere
Expand Down
4 changes: 4 additions & 0 deletions app/src/main/java/com/m2049r/xmrwallet/model/Wallet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,10 @@ public void setSynchronized() {
this.synced = true;
}

public void setUnsynchronized() {
this.synced = false;
}

public static native String getDisplayAmount(long amount);

public static native long getAmountFromString(String amount);
Expand Down
37 changes: 26 additions & 11 deletions app/src/main/java/com/m2049r/xmrwallet/service/WalletService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,9 @@ public void newBlock(long height) {
Timber.d("newBlock() @ %d with observer %s", height, observer);
if (observer != null) {
boolean fullRefresh = false;
updateDaemonState(wallet, wallet.isSynchronized() ? height : 0);
if (!wallet.isSynchronized()) {
boolean receivedNewBlock = true;
boolean updatedWalletConnectionStatus = updateDaemonState(wallet, height, receivedNewBlock);
if (updatedWalletConnectionStatus || !wallet.isSynchronized()) {
updated = true;
// we want to see our transactions as they come in
wallet.refreshHistory();
Expand All @@ -146,13 +147,13 @@ public void updated() {
updated = true;
}

public void refreshed() { // this means it's synced
public void refreshed() {
Timber.d("refreshed()");
final Wallet wallet = getWallet();
if (wallet == null) throw new IllegalStateException("No wallet!");
wallet.setSynchronized();
if (updated) {
updateDaemonState(wallet, wallet.getBlockChainHeight());
boolean receivedNewBlock = false;
boolean updatedWalletConnectionStatus = updateDaemonState(wallet, wallet.getBlockChainHeight(), receivedNewBlock);
if (updated || updatedWalletConnectionStatus) {
wallet.refreshHistory();
if (observer != null) {
updated = !observer.onRefreshed(wallet, true);
Expand All @@ -164,16 +165,20 @@ public void refreshed() { // this means it's synced
private long lastDaemonStatusUpdate = 0;
private long daemonHeight = 0;
private Wallet.ConnectionStatus connectionStatus = Wallet.ConnectionStatus.ConnectionStatus_Disconnected;
private static final long STATUS_UPDATE_INTERVAL = 120000; // 120s (blocktime)
private static final long STATUS_UPDATE_INTERVAL_SYNCED = 120000; // 120s (blocktime)
private static final long STATUS_UPDATE_INTERVAL_SYNCING = 10000; // 10s

private void updateDaemonState(Wallet wallet, long height) {
private boolean updateDaemonState(Wallet wallet, long height, boolean receivedNewBlock) {
Wallet.ConnectionStatus startConnectionStatus = connectionStatus;
long t = System.currentTimeMillis();
if (height > 0) { // if we get a height, we are connected
daemonHeight = height;
if (daemonHeight > 0 && height > 0 && (height > daemonHeight || receivedNewBlock)) {
if (height > daemonHeight)
daemonHeight = height;
connectionStatus = Wallet.ConnectionStatus.ConnectionStatus_Connected;
lastDaemonStatusUpdate = t;
} else {
if (t - lastDaemonStatusUpdate > STATUS_UPDATE_INTERVAL) {
long statusUpdateInterval = wallet.isSynchronized() ? STATUS_UPDATE_INTERVAL_SYNCED : STATUS_UPDATE_INTERVAL_SYNCING;
if (daemonHeight == 0 || t - lastDaemonStatusUpdate > statusUpdateInterval) {
Copy link
Contributor Author

@j-berman j-berman May 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Also calling out this part. Notice how I added daemonHeight > 0 in the above if statement, and the first time updateDaemonState gets called, daemonHeight == 0, so it will fall into this if and make the request for the height. If the height is already cached, it won't be a problem.

The work over in monero-project/monero#8076 should minimize trips to the daemon and ideally get the height cached so the client shouldn't need to make a trip to the daemon here the first time this is called. Looking into this as well.

lastDaemonStatusUpdate = t;
// these calls really connect to the daemon - wasting time
daemonHeight = wallet.getDaemonBlockChainHeight();
Expand All @@ -185,6 +190,16 @@ private void updateDaemonState(Wallet wallet, long height) {
}
}
}
setWalletSyncState(wallet);
return startConnectionStatus != connectionStatus;
}

public void setWalletSyncState(Wallet wallet) {
if (daemonHeight > 0 && daemonHeight <= wallet.getBlockChainHeight()) {
wallet.setSynchronized();
} else {
wallet.setUnsynchronized();
}
}

public long getDaemonHeight() {
Expand Down