-
Notifications
You must be signed in to change notification settings - Fork 397
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
fix: play_card function with second_swipe support #2330
fix: play_card function with second_swipe support #2330
Conversation
Regarding "second swipe": That's definitely another behaviour known from V2. Nevertheless would it not make more sense that it is an option on the folder/card, then an overall state? So different cards could have different settings? |
I agree
I don't think that's the case. See below explanation.
I haven't deep dived into the v2 implementation much.
I agree Intuitively, we opted for the commands Initially, I questioned the necessity of Nonetheless, combining the if(second_swipe): next() # Avoid triggering play_album() This is likely the rationale behind the original implementation within the Player, as it was the only entity capable of managing this logic. I am dissatisfied with the current implementation of the second swipe functionality and propose its removal, with the intention of reintroducing it in a separate PR. |
#1698 is related and probably needs to be considered. |
as my current setup (v3/devlop, place_not_swipe=true) does not resume after replacing, but playing the track again, i finally found this thread. However, it looks like a lot of people already dug into this topic in enhancement calls etc. from my point of view (not sure if its correct) the second swipe action is dealt with in the mpd_player. however, why isn't this put into the cards itself like: How do you think about it? |
see d9d40bf for a card-wrapper and the updated mpdplayer-script |
Pull Request Test Coverage Report for Build 11652970324Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
Here is an explanation why Second Swipe is part of RPi-Jukebox-RFID/src/jukebox/components/playermpd/__init__.py Lines 56 to 82 in 33fec64
|
Abandoning in favor of #2452 |
Until now, 3 functions existed to be registered to a card:
play_single
,play_album
andplay_folder
. None of these functions supported "Second Swipe". Instead, another function existed, calledplay_card
which had Second Swipe support, but it ended up to callplay_folder
, ingoring the other 2 functions all togetherThis PR aims at solving this problem and to prepare for hoffie@c4805ce
play_card
inPlayerMPD
play_card
command instead ofplay_single
,play_album
orplay_folder
play_card