Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(ytm): Improve history handling #248

Merged
merged 7 commits into from
Jan 15, 2025
Merged

fix(ytm): Improve history handling #248

merged 7 commits into from
Jan 15, 2025

Conversation

FoxxMD
Copy link
Owner

@FoxxMD FoxxMD commented Jan 2, 2025

Checklist before requesting a review

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Describe your changes

  • Use diff change type to restrict type of diff results allowed for YTM history changes -- only allow when add/bump diffs are prepend ("new" tracks at top of history list)
  • Store some (4) recent history responses where order changed. Then, on add-prepend diffs check that a previous response does not exactly match. If it does this is probably an indicator a "newer" response actually has outdated data (bug: (YTM) With each scrobble, another song gets scrobbled #227) and we should ignore new tracks
  • Add tests for YTM history processing to test all of the above
  • Use shelf-playlist response instead of flat list when parsing plays so we preserve playlist name
    • Print playlist name to diff debug for more context when troubleshooting logs
  • Detect skipped plays based on reasonable time elapsed since last discovered track (bug: (YTM) Songs skipped 0-9 seconds after their scrobble cause following song to be discarded #226)
    • Newest track is always added as it could be playing now, n+1 tracks are added based on cumulative elapsed - duration
  • Interim, discovered tracks are given play date based on now - cumulative duration of all prior interim tracks

Issue number and link, if applicable

#227
#228
#226

FoxxMD added 4 commits January 2, 2025 18:25
* Diff type should always be prepend since we are checking for new tracks
  * Warn and log if another type is detected
* Warn if more than one track diffed for discovery
* Refactor and and make YTM history parsing testable

#227
Store responses when history changes and then check if YTM returns an outdated response after previously returning new tracks.

#227
@FoxxMD FoxxMD added bug Something isn't working safe to test trusted to build image labels Jan 2, 2025
@FoxxMD FoxxMD self-assigned this Jan 2, 2025
@FoxxMD FoxxMD changed the title feat: Include comment in play diff when present fix(ytm): Improve handling of temporally inconsistent history Jan 2, 2025
Copy link
Contributor

github-actions bot commented Jan 3, 2025

📦 A new release has been made for this pull request.

To play around with this PR, pull an image:

  • foxxmd/multi-scrobbler:pr-248

Images are available for x86_64 and ARM64.

Latest commit: 8b8f32d

@FoxxMD FoxxMD changed the title fix(ytm): Improve handling of temporally inconsistent history fix(ytm): Improve history handling Jan 3, 2025
@FoxxMD FoxxMD added this to the 0.9.0 milestone Jan 14, 2025
@FoxxMD FoxxMD merged commit 1ea1069 into master Jan 15, 2025
4 checks passed
@FoxxMD FoxxMD deleted the GH-227/ytmMusicalChairs branch January 16, 2025 19:49
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working safe to test trusted to build image
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant