playing.txt will not deleted after track is done

[Raspfarbend]

I've recognice a strange behavior. I my case i played a track (playtime 2-3 sec.) and after playtime i want to trigger the track via rfid a secondtime - nothing happens.

Branch pr80-mpd

I short view in the "audiofolders" folders - playing.txt is still existing.

[MiczFlor]

Hi @zxa can you make sense of this?

[zxa]

Hi @MiczFlor
it's not "my" code, but it's because the current second swipe implementation lacks a "end of playlist" handling (i guess the playlist of Raspfarbend consists of only one track). I don't see an easy fix for that.

i wanted to replace that code anyway with something like i outlined in #94 as there are also other circumstances that code fails. But before, i finish a feature i'm personally needing the most (track selection via web app).

[Raspfarbend]

Good morning @MiczFlor , @zxa ,
thats correkt one playlist with one track.

[MiczFlor]

Hi @zxa Now I see. Sorry for the confusion. That's what happens when you merge from a hiking trail during holidays on your mobile...
Hi @Luegengladiator, I think this one is for you, based on these commits
0f51354
66da274

[Luegengladiator]

I'll have a look on that. Seems there is a wrong check.
It's if exists playing.txt instead of if mpc status is playing.
There should be no need of playing.txt handling.
Only keep it up to date by calling the correct startup function.
Hope to get it fixed tomorrow.

@zxa is there a timeframe for your new implementation?

[Luegengladiator]

With a deeper look in the code there is no need of the playing.txt file any more:
# Before we create a new playlist, we remove the old one from the folder.
# It's a workaround for resume playing as mpd doesn't know how its current playlist is named,
# so we only want the current playlist in the "mpc lsplaylists" output.

So "mpc lsplaylists" is not accidentally limited to the actual playlist.
I'll change the script. Additional there is no need to check the playlist if there is no playout. I'll add this check too.

[Luegengladiator]

Struggling around with the status of the player I figured out some issues with the order of the script parts.
@MiczFlor @zxa
There is always an sudo $PATHDATA/playout_controls.sh -c=playlistclear if player is running or not, if next action is "stop" (on second swipe) or not. I will send the fix for the playing.txt issue but we have to discuss this!

[Luegengladiator]

Fix with #125 please check and close if ok

[MiczFlor]

Hi @Raspfarbend @Luegengladiator
I merged the pull request:
#125
please check - and if it works, close this issue.
Thanks, m

[zxa]

The PR introduces a new problem:

• you have to call ./rfid_trigger_play.sh twice to play a new playlist if something is playing yet. It's because in line 333 it checks for "playing" which is true and in line 341 it checks if the loaded playlist is the same like the card playlist, which is also true as the playlist was loaded in the same script run before.

I'm no software professional with only limited self taught coding knowledge, so i wonder if it's good coding practice to set a flag in line 354 and then to check for it in line 357 (the negation makes it even harder to read) to only execute one line of code which could also be in line 354.
Plus: i hate these debug lines. For me it makes the code harder to read and if i'm chasing a bug, i can put echo lines where i want. For example the approx. 40 lines of code starting from line 333 can be condensed to 13 lines (haven't checked for functionality) which can be read on a single glance.

    if [ $mpd_status == "playing" ]
    then
       run_folder=$(mpc lsplaylists)
       # if same playlist, only stop the player
       if [ "$FOLDERNAME" != "$run_folder" ]
       then
           # if playlist is different, stop the running player and restart with new playlist
           $PATHDATA/playout_controls.sh -c=playlistaddplay -v="${FOLDERNAME}"
       fi
    else
          # load new playlist and play
          $PATHDATA/playout_controls.sh -c=playlistaddplay -v="${FOLDERNAME}"
    fi

[MiczFlor]

Quick question: what's the desired behaviour and what is the problem at the moment - from the users perspective. I didn't yet understand the issue and thought it will solve itself :)

[Luegengladiator]

@zxa yes you are right. If Player is running the playlist is always the same so you have to swipe twice on playlistswitch.
As I mentioned before, we have to discuss the order of the script parts. For example creating an Playlist is only useful, if we need it.
@MiczFlor desired behaviour is 1st scan of Code: Play Playlist. 2nd scan of same Code: stop playing as it was already there with mpg123. The Problem is, mpd doesn't know of the Playlist. It only knows the Files of the List so it is hard to check if it is the 2nd swipe

[Luegengladiator]

To remove the issue @zxa mentioned I moved the status check to the beginning of rfid_trigger_play.sh and checking the playlist if status is playing:

if mpc status | awk 'NR==2' | grep playing > /dev/null;
then
mpd_status="playing"
run_folder=$(mpc lsplaylists)
else
mpd_status="offline"
fi

I tested the behaviour and it seems to work now. I'm not able to work on the code directly. Should I open a PR again?

[MiczFlor]

Hi
I added these commits:
d7e9be8
99d9755

Creating these vars, which might also help to solve this issue?

settings/Latest_RFID

Contains the latest RFID used, which could start an audio playout but might also control the system (e.g. volume up or shutdown).
Created in script rfid_trigger_play.sh

settings/Latest_RFID_Played

Contains the latest RFID that triggered an audio folder piped to playout_controls.sh -c=playlistaddplay -v=...
Created in script rfid_trigger_play.sh

settings/Latest_Folder_Played

Contains the last folder that was played. Created in playout_controls.sh AND rfid_trigger_play.sh (currently all play triggers - webapp and RFID are piped through rfid_trigger_play.sh. So it seems redundant. But who knows if at a later dev stage some scripts might use only playout_controls.sh).

Next week, I will walk through the code line by line and understand it better. It's great to see that I don't get it at first sight :) that means "I am not alone" :)

[Luegengladiator]

Tested the fix the hole day now. Seems to work.
So it's "only" a positioning issue. The first fix checked the "running" playlist, after the playlist was already changed. So it always checked actual RFID vs. Playlist of actual RFID which is always true. As a result the playout stopped instead of switching to the new Playlist.
@zxa "[...] Plus: i hate these debug lines [...]" the debug lines are not for developers. The main profit is, you can get logs from Endusers understanding their issues...

[MiczFlor]

I haven't tried yet, but I assume it should work for most cases. Not sure if it handles "resume play" for all possible cases

Haven't been near a Phoniebox yet, but I think it could be as simple as this:

# read the latest folder into var
Latest_Folder_Played=`cat $PATHDATA/../settings/Latest_Folder_Played`

# 1. if player is running: stop player
if [ $mpd_status == "playing" ]
    then
    # use the proper process to stop, because 
    # IF resume play: save current position
    $PATHDATA/playout_controls.sh -c=playerstop
fi

# 2. if a new playlist was swiped or pressed: play it
if [ "$Latest_Folder_Played" != "$FOLDERNAME" ]
    then
    $PATHDATA/playout_controls.sh -c=playlistaddplay -v="${FOLDERNAME}"
fi

[MiczFlor]

Hi @Luegengladiator @Raspfarbend @zxa
testing time, I pushed
39464e9
and I believe this fixes most of it.

• I had to add to check for MPD "paused" not only "playing", too.
• Also, I added the option to pass a dir name to the resume script (because once the new playlist was generated, the mpc lsplaylists to get the "current" folder would get confused as there are two now.
• There were also issues with the playing.txt and lastplayed.dat because sometimes (don't ask me when in hindsight...) they would not delete. So currently they are all created from all places as sudo
• Known issue: the web app does not know what you do with the cards, so if you swipe and start a new playlist, the player controls in the web app won't change.

I tested back and forth between

• RFID card swiping and web app
• with resume ON and OFF
• From playling and paused MPD state

But because this is tricky, I believe you will find something and we start from scratch :)

[MiczFlor]

I am closing this ticket, because of the launch of version 1.0 on master. If you have any issues, please try first if they are still happening on the latest version 1.0. If so, open another ticket, please.

• Use this thread for update issues if you are running stretch

The version 1.0 has not been tested on jessie

• Read this thread if you are running jessie

Version 1.0 is a major improvement (which will be full of bugs, for sure :) and I want to thank all the contributors and Phoniebox lovers with their input, pictures, bug fixes and suggestions.