feat: adds working sync & load of queue with timer for auto sync#757
Open
eddyizm wants to merge 3 commits into
Open
feat: adds working sync & load of queue with timer for auto sync#757eddyizm wants to merge 3 commits into
eddyizm wants to merge 3 commits into
Conversation
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@app/src/main/java/com/cappielloantonio/tempo/ui/fragment/PlayerBottomSheetFragment.java`:
- Around line 343-347: runAutoSaveHandler currently posts autoSaveRunnable
without clearing previously queued callbacks, allowing duplicate delayed save
chains because autoSaveRunnable self-reschedules; fix by making scheduling
idempotent: in runAutoSaveHandler(boolean isPlaying) always null-check
autoSaveHandler, call autoSaveHandler.removeCallbacks(autoSaveRunnable) before
posting a new delayed callback, and only then call
autoSaveHandler.postDelayed(autoSaveRunnable,
Preferences.getSyncCountdownTimer() * 1000L) when isPlaying and
Preferences.isSyncronizationEnabled() is true; leave the existing
removeCallbacks branch as-is for the stopping case.
In
`@app/src/main/java/com/cappielloantonio/tempo/ui/fragment/PlayerCoverFragment.java`:
- Around line 149-156: The click handler for bind.innerButtonBottomRight
currently calls mediaBrowserListenableFuture.get() which can block the UI;
instead, change the handler to avoid blocking by checking
mediaBrowserListenableFuture for null and then using
mediaBrowserListenableFuture.addListener(...) (or Futures.addCallback) to obtain
the MediaBrowser off the UI thread, call mediaBrowser.getCurrentPosition()
inside that listener, and then on the main thread call
playerBottomSheetViewModel.savePlayQueue(position) and show the Snackbar
(guarding that getView() / requireActivity() still exist); also ensure the
future is created via buildAsync() in onStart() remains unchanged and handle
exceptions inside the listener without calling Thread-blocking get().
In
`@app/src/main/java/com/cappielloantonio/tempo/ui/fragment/PlayerQueueFragment.java`:
- Around line 462-473: The listener passed to
mediaBrowserListenableFuture.addListener (in handleSaveQueueClick) performs UI
work (Toast.makeText and toggleFabMenu) on the future's completion thread; wrap
those UI calls in a main-thread dispatch instead of running them directly.
Modify the listener body so you still call mediaBrowserListenableFuture.get()
and playerBottomSheetViewModel.savePlayQueue(position) on the current thread,
but when showing the Toast and calling toggleFabMenu() post a Runnable to the
main thread (e.g., requireActivity().runOnUiThread(...) or new
Handler(Looper.getMainLooper()).post(...)) so the UI interactions occur on the
main thread and keep the existing try/catch around the non-UI operations.
In
`@app/src/main/java/com/cappielloantonio/tempo/viewmodel/PlayerBottomSheetViewModel.java`:
- Around line 289-297: The saved playback position is being persisted by
savePlayQueue but never used when restoring the queue; update the restore/start
flow to pass the persisted position into playback startup and modify
MediaManager.startQueue to accept a positionMs parameter and seek to that value
instead of hardcoding 0. Specifically: change MediaManager.startQueue(...)
signature to startQueue(List<Child> queue, int startIndex, long positionMs) (or
equivalent), update its implementation to call seekTo(..., positionMs) rather
than seekTo(..., 0), and update every caller that starts a queue (the
queue-load/restore path that currently calls startQueue with index-only) to
fetch the saved position from queueRepository (or the value saved by
savePlayQueue) and pass it into the new startQueue(positionMs) parameter so
restored queues resume at the saved millisecond offset.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8bdea2db-822e-4434-8ced-aa3606deada3
📒 Files selected for processing (8)
app/src/main/java/com/cappielloantonio/tempo/ui/fragment/PlayerBottomSheetFragment.javaapp/src/main/java/com/cappielloantonio/tempo/ui/fragment/PlayerCoverFragment.javaapp/src/main/java/com/cappielloantonio/tempo/ui/fragment/PlayerQueueFragment.javaapp/src/main/java/com/cappielloantonio/tempo/util/Preferences.ktapp/src/main/java/com/cappielloantonio/tempo/viewmodel/PlayerBottomSheetViewModel.javaapp/src/main/res/layout/inner_fragment_player_queue.xmlapp/src/main/res/values/arrays.xmlapp/src/main/res/values/strings.xml
Comment on lines
+149
to
+156
| try { | ||
| MediaBrowser mediaBrowser = mediaBrowserListenableFuture.get(); | ||
| long position = mediaBrowser.getCurrentPosition(); | ||
| if (playerBottomSheetViewModel.savePlayQueue(position)) { | ||
| Snackbar.make(requireView(), R.string.player_queue_save_queue_success, Snackbar.LENGTH_LONG).show(); | ||
| } | ||
| } catch (Exception e) { | ||
| e.printStackTrace(); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="app/src/main/java/com/cappielloantonio/tempo/ui/fragment/PlayerCoverFragment.java"
echo "== Snippet around lines 120-190 =="
nl -ba "$file" | sed -n '120,190p'
echo
echo "== Find declaration/usages of mediaBrowserListenableFuture =="
rg -n "mediaBrowserListenableFuture" "$file"
echo
echo "== Show broader context for the click handler =="
# Grab a wider window to see the enclosing method and listener setup
nl -ba "$file" | sed -n '90,220p'Repository: eddyizm/tempus
Length of output: 154
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="app/src/main/java/com/cappielloantonio/tempo/ui/fragment/PlayerCoverFragment.java"
echo "== Snippet around lines 120-190 (with line numbers via awk) =="
awk 'NR>=120 && NR<=190 {printf "%4d\t%s\n", NR, $0}' "$file"
echo
echo "== Find declaration/usages of mediaBrowserListenableFuture (in this file) =="
rg -n "mediaBrowserListenableFuture" "$file" || true
echo
echo "== Wider context around listener setup (lines 90-220) =="
awk 'NR>=90 && NR<=220 {printf "%4d\t%s\n", NR, $0}' "$file"Repository: eddyizm/tempus
Length of output: 10822
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="app/src/main/java/com/cappielloantonio/tempo/ui/fragment/PlayerCoverFragment.java"
echo "== Call sites =="
rg -n "initializeBrowser\\(|bindMediaController\\(|initInnerButton\\(" "$file"
echo
echo "== Context around lifecycle/setup methods =="
# Print a larger window from the start; then narrow if needed
awk 'NR>=1 && NR<=240 {printf "%4d\t%s\n", NR, $0}' "$file" | sed -n '1,240p'
echo
echo "== Search for initInnerButton() definition already shown; check full file for where invoked =="
rg -n "private void initInnerButton\\(" "$file" && rg -n "initInnerButton\\(\\)" "$file"Repository: eddyizm/tempus
Length of output: 10311
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "class PlayerBottomSheetViewModel|getLiveMedia\\(" app/src/main/java/com/cappielloantonio/tempo -SRepository: eddyizm/tempus
Length of output: 994
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="app/src/main/java/com/cappielloantonio/tempo/viewmodel/PlayerBottomSheetViewModel.java"
echo "== getLiveMedia() and nearby code (lines 160-240) =="
awk 'NR>=160 && NR<=240 {printf "%4d\t%s\n", NR, $0}' "$file"
echo
echo "== Search where live media is set =="
rg -n "setValue\\(|postValue\\(|getLiveMedia\\(" "$file"
echo
echo "== Context around first setValue/postValue related to live media (if any) =="
# Print a bit around likely liveMedia field usages
rg -n "LiveData<Child>|liveMedia|MutableLiveData<Child>" "$file"Repository: eddyizm/tempus
Length of output: 5280
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "setLiveMedia\\(" app/src/main/java/com/cappielloantonio/tempo -SRepository: eddyizm/tempus
Length of output: 484
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="app/src/main/java/com/cappielloantonio/tempo/ui/fragment/PlayerBottomSheetFragment.java"
awk 'NR>=120 && NR<=240 {printf "%4d\t%s\n", NR, $0}' "$file"Repository: eddyizm/tempus
Length of output: 6831
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="app/src/main/java/com/cappielloantonio/tempo/ui/fragment/PlayerBottomSheetFragment.java"
awk 'NR>=120 && NR<=240 {printf "%4d\t%s\n", NR, $0}' "$file"Repository: eddyizm/tempus
Length of output: 6831
Avoid blocking mediaBrowserListenableFuture.get() in the bottom-right click handler.
mediaBrowserListenableFuture.get() is called directly inside bind.innerButtonBottomRight.setOnClickListener(...) (PlayerCoverFragment.java, ~148-158). The future is only created in onStart() via buildAsync(), so a click before it completes can block the UI thread (jank/ANR). Use a non-blocking approach (e.g., mediaBrowserListenableFuture.addListener(...)) and update the Snackbar on the main thread, with guards for mediaBrowserListenableFuture == null / getView() == null.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@app/src/main/java/com/cappielloantonio/tempo/ui/fragment/PlayerCoverFragment.java`
around lines 149 - 156, The click handler for bind.innerButtonBottomRight
currently calls mediaBrowserListenableFuture.get() which can block the UI;
instead, change the handler to avoid blocking by checking
mediaBrowserListenableFuture for null and then using
mediaBrowserListenableFuture.addListener(...) (or Futures.addCallback) to obtain
the MediaBrowser off the UI thread, call mediaBrowser.getCurrentPosition()
inside that listener, and then on the main thread call
playerBottomSheetViewModel.savePlayQueue(position) and show the Snackbar
(guarding that getView() / requireActivity() still exist); also ensure the
future is created via buildAsync() in onStart() remains unchanged and handle
exceptions inside the listener without calling Thread-blocking get().
Comment on lines
+462
to
+473
| mediaBrowserListenableFuture.addListener(() -> { | ||
| try { | ||
| MediaBrowser mediaBrowser = mediaBrowserListenableFuture.get(); | ||
| long position = mediaBrowser.getCurrentPosition(); | ||
| if (playerBottomSheetViewModel.savePlayQueue(position)) { | ||
| Toast.makeText(requireContext(), R.string.player_queue_save_queue_success, Toast.LENGTH_SHORT).show(); | ||
| } | ||
| } catch (Exception e) { | ||
| Log.e(TAG, "Error saving queue", e); | ||
| } | ||
| toggleFabMenu(); | ||
| }, MoreExecutors.directExecutor()); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate file and print relevant section with line numbers
FILE="app/src/main/java/com/cappielloantonio/tempo/ui/fragment/PlayerQueueFragment.java"
if [ ! -f "$FILE" ]; then
echo "Missing $FILE" >&2
exit 1
fi
nl -ba "$FILE" | sed -n '430,520p'
# Show toggleFabMenu implementation and any usage of it
rg -n "toggleFabMenu\\s*\\(" "$FILE"
# Print toggleFabMenu method body if found
if rg -q "void toggleFabMenu\\s*\\(" "$FILE"; then
# use sed to extract around method
START=$(rg -n "void toggleFabMenu\\s*\\(" "$FILE" | head -n1 | cut -d: -f1)
END=$((START+80))
nl -ba "$FILE" | sed -n "${START},${END}p"
fi
# Search for mediaBrowserListenableFuture creation / listener registration
rg -n "mediaBrowserListenableFuture" "$FILE"Repository: eddyizm/tempus
Length of output: 102
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="app/src/main/java/com/cappielloantonio/tempo/ui/fragment/PlayerQueueFragment.java"
nl -ba "$FILE" | sed -n '430,520p'
rg -n "toggleFabMenu\\s*\\(" "$FILE"
if rg -q "void toggleFabMenu\\s*\\(" "$FILE"; then
START=$(rg -n "void toggleFabMenu\\s*\\(" "$FILE" | head -n1 | cut -d: -f1)
END=$((START+90))
nl -ba "$FILE" | sed -n "${START},${END}p"
fi
rg -n "mediaBrowserListenableFuture" "$FILE"Repository: eddyizm/tempus
Length of output: 101
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="app/src/main/java/com/cappielloantonio/tempo/ui/fragment/PlayerQueueFragment.java"
echo "=== Context around lines 430-520 ==="
nl -ba "$FILE" | sed -n '430,520p'
echo "=== toggleFabMenu occurrences ==="
rg -n "toggleFabMenu\\s*\\(" "$FILE"
echo "=== toggleFabMenu implementation (if present) ==="
if rg -q "void toggleFabMenu\\s*\\(" "$FILE"; then
START=$(rg -n "void toggleFabMenu\\s*\\(" "$FILE" | head -n1 | cut -d: -f1)
END=$((START+120))
nl -ba "$FILE" | sed -n "${START},${END}p"
fi
echo "=== mediaBrowserListenableFuture occurrences ==="
rg -n "mediaBrowserListenableFuture" "$FILE"Repository: eddyizm/tempus
Length of output: 156
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="app/src/main/java/com/cappielloantonio/tempo/ui/fragment/PlayerQueueFragment.java"
if [ ! -f "$FILE" ]; then
echo "Missing $FILE" >&2
exit 1
fi
awk '{
printf "%6d\t%s\n", NR, $0
}' "$FILE" | sed -n '430,520p'
echo "=== toggleFabMenu occurrences ==="
rg -n "toggleFabMenu\\s*\\(" "$FILE" || true
echo "=== toggleFabMenu implementation (if present) ==="
if rg -q "void toggleFabMenu\\s*\\(" "$FILE"; then
START=$(rg -n "void toggleFabMenu\\s*\\(" "$FILE" | head -n1 | cut -d: -f1)
awk '{
printf "%6d\t%s\n", NR, $0
}' "$FILE" | sed -n "${START},$((START+140))p"
fi
echo "=== mediaBrowserListenableFuture occurrences ==="
rg -n "mediaBrowserListenableFuture" "$FILE" || trueRepository: eddyizm/tempus
Length of output: 13316
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="app/src/main/java/com/cappielloantonio/tempo/ui/fragment/PlayerQueueFragment.java"
echo "=== Context around lines 1-220 ==="
awk '{printf "%6d\t%s\n", NR, $0}' "$FILE" | sed -n '1,220p'
echo "=== Context around lines 220-420 ==="
awk '{printf "%6d\t%s\n", NR, $0}' "$FILE" | sed -n '220,420p'
echo "=== Direct executor addListener occurrences (for UI work patterns) ==="
rg -n "addListener\\(\\(\\) -> \\{|addListener\\([^\\)]*directExecutor\\(" "$FILE" || trueRepository: eddyizm/tempus
Length of output: 20862
Move Save Queue listener UI work to the main thread
mediaBrowserListenableFuture.addListener(..., MoreExecutors.directExecutor()) runs on the future’s completion thread, so Toast.makeText(...) and toggleFabMenu() in handleSaveQueueClick() (lines 462-473) can execute off the main thread and cause UI crashes. Dispatch those UI calls to the main thread (the same pattern is also used in the shuffle/clear listeners).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@app/src/main/java/com/cappielloantonio/tempo/ui/fragment/PlayerQueueFragment.java`
around lines 462 - 473, The listener passed to
mediaBrowserListenableFuture.addListener (in handleSaveQueueClick) performs UI
work (Toast.makeText and toggleFabMenu) on the future's completion thread; wrap
those UI calls in a main-thread dispatch instead of running them directly.
Modify the listener body so you still call mediaBrowserListenableFuture.get()
and playerBottomSheetViewModel.savePlayQueue(position) on the current thread,
but when showing the Toast and calling toggleFabMenu() post a Runnable to the
main thread (e.g., requireActivity().runOnUiThread(...) or new
Handler(Looper.getMainLooper()).post(...)) so the UI interactions occur on the
main thread and keep the existing try/catch around the non-UI operations.
Owner
Author
|
This still needs more testing and review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #47
Summary by CodeRabbit
Release Notes